diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 45f7f527e2..df76e4819e 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,5 +1,6 @@ ## NEXT +- Adds a new `readme-check` command. - Updates `publish-plugin` command documentation. ## 0.8.2 diff --git a/script/tool/lib/src/common/repository_package.dart b/script/tool/lib/src/common/repository_package.dart index e0c4e4a83b..72e7f948fe 100644 --- a/script/tool/lib/src/common/repository_package.dart +++ b/script/tool/lib/src/common/repository_package.dart @@ -48,6 +48,9 @@ class RepositoryPackage { /// The package's top-level pubspec.yaml. File get pubspecFile => directory.childFile('pubspec.yaml'); + /// The package's top-level README. + File get readmeFile => directory.childFile('README.md'); + late final Pubspec _parsedPubspec = Pubspec.parse(pubspecFile.readAsStringSync()); @@ -62,6 +65,12 @@ class RepositoryPackage { directory.parent.basename != 'packages' && directory.basename.startsWith(directory.parent.basename); + /// True if this appears to be the app-facing package of a federated plugin, + /// according to repository conventions. + bool get isAppFacing => + directory.parent.basename != 'packages' && + directory.basename == directory.parent.basename; + /// True if this appears to be a platform interface package, according to /// repository conventions. bool get isPlatformInterface => diff --git a/script/tool/lib/src/main.dart b/script/tool/lib/src/main.dart index 5a71a0a888..aa1cf300bd 100644 --- a/script/tool/lib/src/main.dart +++ b/script/tool/lib/src/main.dart @@ -26,6 +26,7 @@ import 'native_test_command.dart'; import 'publish_check_command.dart'; import 'publish_plugin_command.dart'; import 'pubspec_check_command.dart'; +import 'readme_check_command.dart'; import 'test_command.dart'; import 'version_check_command.dart'; import 'xcode_analyze_command.dart'; @@ -65,6 +66,7 @@ void main(List args) { ..addCommand(PublishCheckCommand(packagesDir)) ..addCommand(PublishPluginCommand(packagesDir)) ..addCommand(PubspecCheckCommand(packagesDir)) + ..addCommand(ReadmeCheckCommand(packagesDir)) ..addCommand(TestCommand(packagesDir)) ..addCommand(VersionCheckCommand(packagesDir)) ..addCommand(XcodeAnalyzeCommand(packagesDir)); diff --git a/script/tool/lib/src/readme_check_command.dart b/script/tool/lib/src/readme_check_command.dart new file mode 100644 index 0000000000..99e271c733 --- /dev/null +++ b/script/tool/lib/src/readme_check_command.dart @@ -0,0 +1,152 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:file/file.dart'; +import 'package:git/git.dart'; +import 'package:platform/platform.dart'; +import 'package:pubspec_parse/pubspec_parse.dart'; +import 'package:yaml/yaml.dart'; + +import 'common/core.dart'; +import 'common/package_looping_command.dart'; +import 'common/process_runner.dart'; +import 'common/repository_package.dart'; + +/// A command to enforce README conventions across the repository. +class ReadmeCheckCommand extends PackageLoopingCommand { + /// Creates an instance of the README check command. + ReadmeCheckCommand( + Directory packagesDir, { + ProcessRunner processRunner = const ProcessRunner(), + Platform platform = const LocalPlatform(), + GitDir? gitDir, + }) : super( + packagesDir, + processRunner: processRunner, + platform: platform, + gitDir: gitDir, + ); + + // Standardized capitalizations for platforms that a plugin can support. + static const Map _standardPlatformNames = { + 'android': 'Android', + 'ios': 'iOS', + 'linux': 'Linux', + 'macos': 'macOS', + 'web': 'Web', + 'windows': 'Windows', + }; + + @override + final String name = 'readme-check'; + + @override + final String description = + 'Checks that READMEs follow repository conventions.'; + + @override + bool get hasLongOutput => false; + + @override + Future runForPackage(RepositoryPackage package) async { + final File readme = package.readmeFile; + + if (!readme.existsSync()) { + return PackageResult.fail(['Missing README.md']); + } + + final List errors = []; + + final Pubspec pubspec = package.parsePubspec(); + final bool isPlugin = pubspec.flutter?['plugin'] != null; + + if (isPlugin && (!package.isFederated || package.isAppFacing)) { + final String? error = _validateSupportedPlatforms(package, pubspec); + if (error != null) { + errors.add(error); + } + } + + return errors.isEmpty + ? PackageResult.success() + : PackageResult.fail(errors); + } + + /// Validates that the plugin has a supported platforms table following the + /// expected format, returning an error string if any issues are found. + String? _validateSupportedPlatforms( + RepositoryPackage package, Pubspec pubspec) { + final List contents = package.readmeFile.readAsLinesSync(); + + // Example table following expected format: + // | | Android | iOS | Web | + // |----------------|---------|----------|------------------------| + // | **Support** | SDK 21+ | iOS 10+* | [See `camera_web `][1] | + final int detailsLineNumber = + contents.indexWhere((String line) => line.startsWith('| **Support**')); + if (detailsLineNumber == -1) { + return 'No OS support table found'; + } + final int osLineNumber = detailsLineNumber - 2; + if (osLineNumber < 0 || !contents[osLineNumber].startsWith('|')) { + return 'OS support table does not have the expected header format'; + } + + // Utility method to convert an iterable of strings to a case-insensitive + // sorted, comma-separated string of its elements. + String sortedListString(Iterable entries) { + final List entryList = entries.toList(); + entryList.sort( + (String a, String b) => a.toLowerCase().compareTo(b.toLowerCase())); + return entryList.join(', '); + } + + // Validate that the supported OS lists match. + final dynamic platformsEntry = pubspec.flutter!['plugin']!['platforms']; + if (platformsEntry == null) { + logWarning('Plugin not support any platforms'); + return null; + } + final YamlMap platformSupportMaps = platformsEntry as YamlMap; + final Set actuallySupportedPlatform = + platformSupportMaps.keys.toSet().cast(); + final Iterable documentedPlatforms = contents[osLineNumber] + .split('|') + .map((String entry) => entry.trim()) + .where((String entry) => entry.isNotEmpty); + final Set documentedPlatformsLowercase = + documentedPlatforms.map((String entry) => entry.toLowerCase()).toSet(); + if (actuallySupportedPlatform.length != documentedPlatforms.length || + actuallySupportedPlatform + .intersection(documentedPlatformsLowercase) + .length != + actuallySupportedPlatform.length) { + printError(''' +${indentation}OS support table does not match supported platforms: +${indentation * 2}Actual: ${sortedListString(actuallySupportedPlatform)} +${indentation * 2}Documented: ${sortedListString(documentedPlatformsLowercase)} +'''); + return 'Incorrect OS support table'; + } + + // Enforce a standard set of capitalizations for the OS headings. + final Iterable incorrectCapitalizations = documentedPlatforms + .toSet() + .difference(_standardPlatformNames.values.toSet()); + if (incorrectCapitalizations.isNotEmpty) { + final Iterable expectedVersions = incorrectCapitalizations + .map((String name) => _standardPlatformNames[name.toLowerCase()]!); + printError(''' +${indentation}Incorrect OS capitalization: ${sortedListString(incorrectCapitalizations)} +${indentation * 2}Please use standard capitalizations: ${sortedListString(expectedVersions)} +'''); + return 'Incorrect OS support formatting'; + } + + // TODO(stuartmorgan): Add validation that the minimums in the table are + // consistent with what the current implementations require. See + // https://github.com/flutter/flutter/issues/84200 + return null; + } +} diff --git a/script/tool/test/common/repository_package_test.dart b/script/tool/test/common/repository_package_test.dart index 29e3b58321..0a8ea36eb9 100644 --- a/script/tool/test/common/repository_package_test.dart +++ b/script/tool/test/common/repository_package_test.dart @@ -126,6 +126,7 @@ void main() { test('all return false for a simple plugin', () { final Directory plugin = createFakePlugin('a_plugin', packagesDir); expect(RepositoryPackage(plugin).isFederated, false); + expect(RepositoryPackage(plugin).isAppFacing, false); expect(RepositoryPackage(plugin).isPlatformInterface, false); expect(RepositoryPackage(plugin).isFederated, false); }); @@ -134,6 +135,7 @@ void main() { final Directory plugin = createFakePlugin('a_plugin', packagesDir.childDirectory('a_plugin')); expect(RepositoryPackage(plugin).isFederated, true); + expect(RepositoryPackage(plugin).isAppFacing, true); expect(RepositoryPackage(plugin).isPlatformInterface, false); expect(RepositoryPackage(plugin).isPlatformImplementation, false); }); @@ -142,6 +144,7 @@ void main() { final Directory plugin = createFakePlugin('a_plugin_platform_interface', packagesDir.childDirectory('a_plugin')); expect(RepositoryPackage(plugin).isFederated, true); + expect(RepositoryPackage(plugin).isAppFacing, false); expect(RepositoryPackage(plugin).isPlatformInterface, true); expect(RepositoryPackage(plugin).isPlatformImplementation, false); }); @@ -152,6 +155,7 @@ void main() { final Directory plugin = createFakePlugin( 'a_plugin_foo', packagesDir.childDirectory('a_plugin')); expect(RepositoryPackage(plugin).isFederated, true); + expect(RepositoryPackage(plugin).isAppFacing, false); expect(RepositoryPackage(plugin).isPlatformInterface, false); expect(RepositoryPackage(plugin).isPlatformImplementation, true); }); diff --git a/script/tool/test/readme_check_command_test.dart b/script/tool/test/readme_check_command_test.dart new file mode 100644 index 0000000000..aec2fa0784 --- /dev/null +++ b/script/tool/test/readme_check_command_test.dart @@ -0,0 +1,278 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:args/command_runner.dart'; +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_plugin_tools/src/common/core.dart'; +import 'package:flutter_plugin_tools/src/common/plugin_utils.dart'; +import 'package:flutter_plugin_tools/src/readme_check_command.dart'; +import 'package:test/test.dart'; + +import 'mocks.dart'; +import 'util.dart'; + +void main() { + late CommandRunner runner; + late RecordingProcessRunner processRunner; + late FileSystem fileSystem; + late MockPlatform mockPlatform; + late Directory packagesDir; + + setUp(() { + fileSystem = MemoryFileSystem(); + mockPlatform = MockPlatform(); + packagesDir = fileSystem.currentDirectory.childDirectory('packages'); + createPackagesDirectory(parentDir: packagesDir.parent); + processRunner = RecordingProcessRunner(); + final ReadmeCheckCommand command = ReadmeCheckCommand( + packagesDir, + processRunner: processRunner, + platform: mockPlatform, + ); + + runner = CommandRunner( + 'readme_check_command', 'Test for readme_check_command'); + runner.addCommand(command); + }); + + test('fails when README is missing', () async { + createFakePackage('a_package', packagesDir); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['readme-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Missing README.md'), + ]), + ); + }); + + group('plugin OS support', () { + test( + 'does not check support table for anything other than app-facing plugin packages', + () async { + const String federatedPluginName = 'a_federated_plugin'; + final Directory federatedDir = + packagesDir.childDirectory(federatedPluginName); + final List packageDirectories = [ + // A non-plugin package. + createFakePackage('a_package', packagesDir), + // Non-app-facing parts of a federated plugin. + createFakePlugin( + '${federatedPluginName}_platform_interface', federatedDir), + createFakePlugin('${federatedPluginName}_android', federatedDir), + ]; + + for (final Directory package in packageDirectories) { + package.childFile('README.md').writeAsStringSync(''' +A very useful package. +'''); + } + + final List output = await runCapturingPrint(runner, [ + 'readme-check', + ]); + + expect( + output, + containsAll([ + contains('Running for a_package...'), + contains('Running for a_federated_plugin_platform_interface...'), + contains('Running for a_federated_plugin_android...'), + contains('No issues found!'), + ]), + ); + }); + + test('fails when non-federated plugin is missing an OS support table', + () async { + final Directory pluginDir = createFakePlugin('a_plugin', packagesDir); + + pluginDir.childFile('README.md').writeAsStringSync(''' +A very useful plugin. +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['readme-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('No OS support table found'), + ]), + ); + }); + + test( + 'fails when app-facing part of a federated plugin is missing an OS support table', + () async { + final Directory pluginDir = + createFakePlugin('a_plugin', packagesDir.childDirectory('a_plugin')); + + pluginDir.childFile('README.md').writeAsStringSync(''' +A very useful plugin. +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['readme-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('No OS support table found'), + ]), + ); + }); + + test('fails the OS support table is missing the header', () async { + final Directory pluginDir = createFakePlugin('a_plugin', packagesDir); + + pluginDir.childFile('README.md').writeAsStringSync(''' +A very useful plugin. + +| **Support** | SDK 21+ | iOS 10+* | [See `camera_web `][1] | +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['readme-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('OS support table does not have the expected header format'), + ]), + ); + }); + + test('fails if the OS support table is missing a supported OS', () async { + final Directory pluginDir = createFakePlugin( + 'a_plugin', + packagesDir, + platformSupport: { + platformAndroid: const PlatformDetails(PlatformSupport.inline), + platformIOS: const PlatformDetails(PlatformSupport.inline), + platformWeb: const PlatformDetails(PlatformSupport.inline), + }, + ); + + pluginDir.childFile('README.md').writeAsStringSync(''' +A very useful plugin. + +| | Android | iOS | +|----------------|---------|----------| +| **Support** | SDK 21+ | iOS 10+* | +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['readme-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains(' OS support table does not match supported platforms:\n' + ' Actual: android, ios, web\n' + ' Documented: android, ios'), + contains('Incorrect OS support table'), + ]), + ); + }); + + test('fails if the OS support table lists an extra OS', () async { + final Directory pluginDir = createFakePlugin( + 'a_plugin', + packagesDir, + platformSupport: { + platformAndroid: const PlatformDetails(PlatformSupport.inline), + platformIOS: const PlatformDetails(PlatformSupport.inline), + }, + ); + + pluginDir.childFile('README.md').writeAsStringSync(''' +A very useful plugin. + +| | Android | iOS | Web | +|----------------|---------|----------|------------------------| +| **Support** | SDK 21+ | iOS 10+* | [See `camera_web `][1] | +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['readme-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains(' OS support table does not match supported platforms:\n' + ' Actual: android, ios\n' + ' Documented: android, ios, web'), + contains('Incorrect OS support table'), + ]), + ); + }); + + test('fails if the OS support table has unexpected OS formatting', + () async { + final Directory pluginDir = createFakePlugin( + 'a_plugin', + packagesDir, + platformSupport: { + platformAndroid: const PlatformDetails(PlatformSupport.inline), + platformIOS: const PlatformDetails(PlatformSupport.inline), + platformMacOS: const PlatformDetails(PlatformSupport.inline), + platformWeb: const PlatformDetails(PlatformSupport.inline), + }, + ); + + pluginDir.childFile('README.md').writeAsStringSync(''' +A very useful plugin. + +| | android | ios | MacOS | web | +|----------------|---------|----------|-------|------------------------| +| **Support** | SDK 21+ | iOS 10+* | 10.11 | [See `camera_web `][1] | +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['readme-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains(' Incorrect OS capitalization: android, ios, MacOS, web\n' + ' Please use standard capitalizations: Android, iOS, macOS, Web\n'), + contains('Incorrect OS support formatting'), + ]), + ); + }); + }); +}