diff --git a/script/tool/lib/src/common.dart b/script/tool/lib/src/common.dart index 20e8479e6d..5d653ad0ed 100644 --- a/script/tool/lib/src/common.dart +++ b/script/tool/lib/src/common.dart @@ -165,11 +165,10 @@ bool isLinuxPlugin(FileSystemEntity entity) { return pluginSupportsPlatform(kPlatformFlagLinux, entity); } -/// Throws a [ToolExit] with `exitCode` and log the `errorMessage` in red. -void printErrorAndExit({required String errorMessage, int exitCode = 1}) { +/// Prints `errorMessage` in red. +void printError(String errorMessage) { final Colorize redError = Colorize(errorMessage)..red(); print(redError); - throw ToolExit(exitCode); } /// Error thrown when a command needs to exit with a non-zero exit code. @@ -456,9 +455,10 @@ abstract class PluginCommand extends Command { GitDir? baseGitDir = gitDir; if (baseGitDir == null) { if (!await GitDir.isGitDir(rootDir)) { - printErrorAndExit( - errorMessage: '$rootDir is not a valid Git repository.', - exitCode: 2); + printError( + '$rootDir is not a valid Git repository.', + ); + throw ToolExit(2); } baseGitDir = await GitDir.fromExisting(rootDir); } @@ -599,7 +599,7 @@ class ProcessRunner { /// passing [workingDir]. /// /// Returns the started [io.Process]. - Future start(String executable, List args, + Future start(String executable, List args, {Directory? workingDirectory}) async { final io.Process process = await io.Process.start(executable, args, workingDirectory: workingDirectory?.path); @@ -641,12 +641,12 @@ class PubVersionFinder { if (response.statusCode == 404) { return PubVersionFinderResponse( - versions: null, + versions: [], result: PubVersionFinderResult.noPackageFound, httpResponse: response); } else if (response.statusCode != 200) { return PubVersionFinderResponse( - versions: null, + versions: [], result: PubVersionFinderResult.fail, httpResponse: response); } @@ -666,9 +666,12 @@ class PubVersionFinder { /// Represents a response for [PubVersionFinder]. class PubVersionFinderResponse { /// Constructor. - PubVersionFinderResponse({this.versions, this.result, this.httpResponse}) { - if (versions != null && versions!.isNotEmpty) { - versions!.sort((Version a, Version b) { + PubVersionFinderResponse( + {required this.versions, + required this.result, + required this.httpResponse}) { + if (versions.isNotEmpty) { + versions.sort((Version a, Version b) { // TODO(cyanglaz): Think about how to handle pre-release version with [Version.prioritize]. // https://github.com/flutter/flutter/issues/82222 return b.compareTo(a); @@ -680,13 +683,13 @@ class PubVersionFinderResponse { /// /// This is sorted by largest to smallest, so the first element in the list is the largest version. /// Might be `null` if the [result] is not [PubVersionFinderResult.success]. - final List? versions; + final List versions; /// The result of the version finder. - final PubVersionFinderResult? result; + final PubVersionFinderResult result; /// The response object of the http request. - final http.Response? httpResponse; + final http.Response httpResponse; } /// An enum representing the result of [PubVersionFinder]. diff --git a/script/tool/lib/src/publish_check_command.dart b/script/tool/lib/src/publish_check_command.dart index fa229cabef..b77eceecbf 100644 --- a/script/tool/lib/src/publish_check_command.dart +++ b/script/tool/lib/src/publish_check_command.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart=2.9 - import 'dart:async'; import 'dart:convert'; import 'dart:io' as io; @@ -11,7 +9,6 @@ import 'dart:io' as io; import 'package:colorize/colorize.dart'; import 'package:file/file.dart'; import 'package:http/http.dart' as http; -import 'package:meta/meta.dart'; import 'package:pub_semver/pub_semver.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; @@ -23,7 +20,7 @@ class PublishCheckCommand extends PluginCommand { PublishCheckCommand( Directory packagesDir, { ProcessRunner processRunner = const ProcessRunner(), - this.httpClient, + http.Client? httpClient, }) : _pubVersionFinder = PubVersionFinder(httpClient: httpClient ?? http.Client()), super(packagesDir, processRunner: processRunner) { @@ -65,9 +62,6 @@ class PublishCheckCommand extends PluginCommand { final String description = 'Checks to make sure that a plugin *could* be published.'; - /// The custom http client used to query versions on pub. - final http.Client httpClient; - final PubVersionFinder _pubVersionFinder; // The output JSON when the _machineFlag is on. @@ -135,7 +129,7 @@ class PublishCheckCommand extends PluginCommand { } } - Pubspec _tryParsePubspec(Directory package) { + Pubspec? _tryParsePubspec(Directory package) { final File pubspecFile = package.childFile('pubspec.yaml'); try { @@ -205,7 +199,7 @@ class PublishCheckCommand extends PluginCommand { final String packageName = package.basename; print('Checking that $packageName can be published.'); - final Pubspec pubspec = _tryParsePubspec(package); + final Pubspec? pubspec = _tryParsePubspec(package); if (pubspec == null) { print('no pubspec'); return _PublishCheckResult._error; @@ -214,7 +208,7 @@ class PublishCheckCommand extends PluginCommand { return _PublishCheckResult._published; } - final Version version = pubspec.version; + final Version? version = pubspec.version; final _PublishCheckResult alreadyPublishedResult = await _checkIfAlreadyPublished( packageName: packageName, version: version); @@ -238,29 +232,24 @@ class PublishCheckCommand extends PluginCommand { // Check if `packageName` already has `version` published on pub. Future<_PublishCheckResult> _checkIfAlreadyPublished( - {String packageName, Version version}) async { + {required String packageName, required Version? version}) async { final PubVersionFinderResponse pubVersionFinderResponse = await _pubVersionFinder.getPackageVersion(package: packageName); - _PublishCheckResult result; switch (pubVersionFinderResponse.result) { case PubVersionFinderResult.success: - result = pubVersionFinderResponse.versions.contains(version) + return pubVersionFinderResponse.versions.contains(version) ? _PublishCheckResult._published : _PublishCheckResult._notPublished; - break; case PubVersionFinderResult.fail: print(''' Error fetching version on pub for $packageName. HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode} HTTP response: ${pubVersionFinderResponse.httpResponse.body} '''); - result = _PublishCheckResult._error; - break; + return _PublishCheckResult._error; case PubVersionFinderResult.noPackageFound: - result = _PublishCheckResult._notPublished; - break; + return _PublishCheckResult._notPublished; } - return result; } void _setStatus(String status) { @@ -272,7 +261,7 @@ HTTP response: ${pubVersionFinderResponse.httpResponse.body} return const JsonEncoder.withIndent(' ').convert(_machineOutput); } - void _printImportantStatusMessage(String message, {@required bool isError}) { + void _printImportantStatusMessage(String message, {required bool isError}) { final String statusMessage = '${isError ? 'ERROR' : 'SUCCESS'}: $message'; if (getBoolArg(_machineFlag)) { print(statusMessage); diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart index 6b837cb3f4..6a215064cf 100644 --- a/script/tool/lib/src/publish_plugin_command.dart +++ b/script/tool/lib/src/publish_plugin_command.dart @@ -507,10 +507,11 @@ Safe to ignore if the package is deleted in this commit. } final String credential = io.Platform.environment[_pubCredentialName]; if (credential == null) { - printErrorAndExit(errorMessage: ''' + printError(''' No pub credential available. Please check if `~/.pub-cache/credentials.json` is valid. If running this command on CI, you can set the pub credential content in the $_pubCredentialName environment variable. '''); + throw ToolExit(1); } credentialFile.openSync(mode: FileMode.writeOnlyAppend) ..writeStringSync(credential) diff --git a/script/tool/lib/src/version_check_command.dart b/script/tool/lib/src/version_check_command.dart index d74c7dad3c..6baa38e465 100644 --- a/script/tool/lib/src/version_check_command.dart +++ b/script/tool/lib/src/version_check_command.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart=2.9 - import 'dart:async'; import 'package:file/file.dart'; @@ -37,7 +35,7 @@ enum NextVersionType { /// those have different semver rules. @visibleForTesting Map getAllowedNextVersions( - Version masterVersion, Version headVersion) { + {required Version masterVersion, required Version headVersion}) { final Map allowedNextVersions = { masterVersion.nextMajor: NextVersionType.BREAKING_MAJOR, @@ -75,8 +73,8 @@ class VersionCheckCommand extends PluginCommand { VersionCheckCommand( Directory packagesDir, { ProcessRunner processRunner = const ProcessRunner(), - GitDir gitDir, - this.httpClient, + GitDir? gitDir, + http.Client? httpClient, }) : _pubVersionFinder = PubVersionFinder(httpClient: httpClient ?? http.Client()), super(packagesDir, processRunner: processRunner, gitDir: gitDir) { @@ -100,9 +98,6 @@ class VersionCheckCommand extends PluginCommand { 'Also checks if the latest version in CHANGELOG matches the version in pubspec.\n\n' 'This command requires "pub" and "flutter" to be in your path.'; - /// The http client used to query pub server. - final http.Client httpClient; - final PubVersionFinder _pubVersionFinder; @override @@ -112,6 +107,8 @@ class VersionCheckCommand extends PluginCommand { final List changedPubspecs = await gitVersionFinder.getChangedPubSpecs(); + final List badVersionChangePubspecs = []; + const String indentation = ' '; for (final String pubspecPath in changedPubspecs) { print('Checking versions for $pubspecPath...'); @@ -126,15 +123,16 @@ class VersionCheckCommand extends PluginCommand { continue; } - final Version headVersion = + final Version? headVersion = await gitVersionFinder.getPackageVersion(pubspecPath, gitRef: 'HEAD'); if (headVersion == null) { - printErrorAndExit( - errorMessage: '${indentation}No version found. A package that ' - 'intentionally has no version should be marked ' - '"publish_to: none".'); + printError('${indentation}No version found. A package that ' + 'intentionally has no version should be marked ' + '"publish_to: none".'); + badVersionChangePubspecs.add(pubspecPath); + continue; } - Version sourceVersion; + Version? sourceVersion; if (getBoolArg(_againstPubFlag)) { final String packageName = pubspecFile.parent.basename; final PubVersionFinderResponse pubVersionFinderResponse = @@ -146,12 +144,13 @@ class VersionCheckCommand extends PluginCommand { '$indentation$packageName: Current largest version on pub: $sourceVersion'); break; case PubVersionFinderResult.fail: - printErrorAndExit(errorMessage: ''' + printError(''' ${indentation}Error fetching version on pub for $packageName. ${indentation}HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode} ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} '''); - break; + badVersionChangePubspecs.add(pubspecPath); + continue; case PubVersionFinderResult.noPackageFound: sourceVersion = null; break; @@ -180,7 +179,8 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} // Check for reverts when doing local validation. if (!getBoolArg(_againstPubFlag) && headVersion < sourceVersion) { final Map possibleVersionsFromNewVersion = - getAllowedNextVersions(headVersion, sourceVersion); + getAllowedNextVersions( + masterVersion: headVersion, headVersion: sourceVersion); // Since this skips validation, try to ensure that it really is likely // to be a revert rather than a typo by checking that the transition // from the lower version to the new version would have been valid. @@ -192,14 +192,16 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} } final Map allowedNextVersions = - getAllowedNextVersions(sourceVersion, headVersion); + getAllowedNextVersions( + masterVersion: sourceVersion, headVersion: headVersion); if (!allowedNextVersions.containsKey(headVersion)) { final String source = (getBoolArg(_againstPubFlag)) ? 'pub' : 'master'; - final String error = '${indentation}Incorrectly updated version.\n' + printError('${indentation}Incorrectly updated version.\n' '${indentation}HEAD: $headVersion, $source: $sourceVersion.\n' - '${indentation}Allowed versions: $allowedNextVersions'; - printErrorAndExit(errorMessage: error); + '${indentation}Allowed versions: $allowedNextVersions'); + badVersionChangePubspecs.add(pubspecPath); + continue; } else { print('$indentation$headVersion -> $sourceVersion'); } @@ -208,38 +210,65 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} pubspec.name.endsWith('_platform_interface'); if (isPlatformInterface && allowedNextVersions[headVersion] == NextVersionType.BREAKING_MAJOR) { - final String error = '$pubspecPath breaking change detected.\n' - 'Breaking changes to platform interfaces are strongly discouraged.\n'; - printErrorAndExit(errorMessage: error); + printError('$pubspecPath breaking change detected.\n' + 'Breaking changes to platform interfaces are strongly discouraged.\n'); + badVersionChangePubspecs.add(pubspecPath); + continue; + } + } + _pubVersionFinder.httpClient.close(); + + // TODO(stuartmorgan): Unify the way iteration works for these checks; the + // two checks shouldn't be operating independently on different lists. + final List mismatchedVersionPlugins = []; + await for (final Directory plugin in getPlugins()) { + if (!(await _checkVersionsMatch(plugin))) { + mismatchedVersionPlugins.add(plugin.basename); } } - await for (final Directory plugin in getPlugins()) { - await _checkVersionsMatch(plugin); + bool passed = true; + if (badVersionChangePubspecs.isNotEmpty) { + passed = false; + printError(''' +The following pubspecs failed validaton: +$indentation${badVersionChangePubspecs.join('\n$indentation')} +'''); + } + if (mismatchedVersionPlugins.isNotEmpty) { + passed = false; + printError(''' +The following pubspecs have different versions in pubspec.yaml and CHANGELOG.md: +$indentation${mismatchedVersionPlugins.join('\n$indentation')} +'''); + } + if (!passed) { + throw ToolExit(1); } - _pubVersionFinder.httpClient.close(); print('No version check errors found!'); } - Future _checkVersionsMatch(Directory plugin) async { + /// Returns whether or not the pubspec version and CHANGELOG version for + /// [plugin] match. + Future _checkVersionsMatch(Directory plugin) async { // get version from pubspec final String packageName = plugin.basename; print('-----------------------------------------'); print( 'Checking the first version listed in CHANGELOG.md matches the version in pubspec.yaml for $packageName.'); - final Pubspec pubspec = _tryParsePubspec(plugin); + final Pubspec? pubspec = _tryParsePubspec(plugin); if (pubspec == null) { - const String error = 'Cannot parse version from pubspec.yaml'; - printErrorAndExit(errorMessage: error); + printError('Cannot parse version from pubspec.yaml'); + return false; } - final Version fromPubspec = pubspec.version; + final Version? fromPubspec = pubspec.version; // get first version from CHANGELOG final File changelog = plugin.childFile('CHANGELOG.md'); final List lines = changelog.readAsLinesSync(); - String firstLineWithText; + String? firstLineWithText; final Iterator iterator = lines.iterator; while (iterator.moveNext()) { if (iterator.current.trim().isNotEmpty) { @@ -248,7 +277,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} } } // Remove all leading mark down syntax from the version line. - String versionString = firstLineWithText.split(' ').last; + String? versionString = firstLineWithText?.split(' ').last; // Skip validation for the special NEXT version that's used to accumulate // changes that don't warrant publishing on their own. @@ -266,51 +295,48 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body} } } - final Version fromChangeLog = Version.parse(versionString); + final Version? fromChangeLog = + versionString == null ? null : Version.parse(versionString); if (fromChangeLog == null) { - final String error = - 'Cannot find version on the first line of ${plugin.path}/CHANGELOG.md'; - printErrorAndExit(errorMessage: error); + printError( + 'Cannot find version on the first line of ${plugin.path}/CHANGELOG.md'); + return false; } if (fromPubspec != fromChangeLog) { - final String error = ''' + printError(''' versions for $packageName in CHANGELOG.md and pubspec.yaml do not match. The version in pubspec.yaml is $fromPubspec. The first version listed in CHANGELOG.md is $fromChangeLog. -'''; - printErrorAndExit(errorMessage: error); +'''); + return false; } // If NEXT wasn't the first section, it should not exist at all. if (!hasNextSection) { final RegExp nextRegex = RegExp(r'^#+\s*NEXT\s*$'); if (lines.any((String line) => nextRegex.hasMatch(line))) { - printErrorAndExit(errorMessage: ''' + printError(''' When bumping the version for release, the NEXT section should be incorporated into the new version's release notes. '''); + return false; } } print('$packageName passed version check'); + return true; } - Pubspec _tryParsePubspec(Directory package) { + Pubspec? _tryParsePubspec(Directory package) { final File pubspecFile = package.childFile('pubspec.yaml'); try { final Pubspec pubspec = Pubspec.parse(pubspecFile.readAsStringSync()); - if (pubspec == null) { - final String error = - 'Failed to parse `pubspec.yaml` at ${pubspecFile.path}'; - printErrorAndExit(errorMessage: error); - } return pubspec; } on Exception catch (exception) { - final String error = - 'Failed to parse `pubspec.yaml` at ${pubspecFile.path}: $exception}'; - printErrorAndExit(errorMessage: error); + printError( + 'Failed to parse `pubspec.yaml` at ${pubspecFile.path}: $exception}'); } return null; } diff --git a/script/tool/test/common_test.dart b/script/tool/test/common_test.dart index 0516fab1da..a51182d91f 100644 --- a/script/tool/test/common_test.dart +++ b/script/tool/test/common_test.dart @@ -457,10 +457,10 @@ file2/file2.cc final PubVersionFinderResponse response = await finder.getPackageVersion(package: 'some_package'); - expect(response.versions, isNull); + expect(response.versions, isEmpty); expect(response.result, PubVersionFinderResult.noPackageFound); - expect(response.httpResponse!.statusCode, 404); - expect(response.httpResponse!.body, ''); + expect(response.httpResponse.statusCode, 404); + expect(response.httpResponse.body, ''); }); test('HTTP error when getting versions from pub', () async { @@ -471,10 +471,10 @@ file2/file2.cc final PubVersionFinderResponse response = await finder.getPackageVersion(package: 'some_package'); - expect(response.versions, isNull); + expect(response.versions, isEmpty); expect(response.result, PubVersionFinderResult.fail); - expect(response.httpResponse!.statusCode, 400); - expect(response.httpResponse!.body, ''); + expect(response.httpResponse.statusCode, 400); + expect(response.httpResponse.body, ''); }); test('Get a correct list of versions when http response is OK.', () async { @@ -517,8 +517,8 @@ file2/file2.cc Version.parse('0.0.1'), ]); expect(response.result, PubVersionFinderResult.success); - expect(response.httpResponse!.statusCode, 200); - expect(response.httpResponse!.body, json.encode(httpResponse)); + expect(response.httpResponse.statusCode, 200); + expect(response.httpResponse.body, json.encode(httpResponse)); }); }); diff --git a/script/tool/test/publish_check_command_test.dart b/script/tool/test/publish_check_command_test.dart index 11d703177d..e5722567f2 100644 --- a/script/tool/test/publish_check_command_test.dart +++ b/script/tool/test/publish_check_command_test.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart=2.9 - import 'dart:collection'; import 'dart:convert'; import 'dart:io' as io; @@ -23,9 +21,9 @@ import 'util.dart'; void main() { group('$PublishCheckProcessRunner tests', () { FileSystem fileSystem; - Directory packagesDir; - PublishCheckProcessRunner processRunner; - CommandRunner runner; + late Directory packagesDir; + late PublishCheckProcessRunner processRunner; + late CommandRunner runner; setUp(() { fileSystem = MemoryFileSystem(); @@ -177,7 +175,7 @@ void main() { } else if (request.url.pathSegments.last == 'no_publish_b.json') { return http.Response(json.encode(httpResponseB), 200); } - return null; + return http.Response('', 500); }); final PublishCheckCommand command = PublishCheckCommand(packagesDir, processRunner: processRunner, httpClient: mockClient); @@ -241,7 +239,7 @@ void main() { } else if (request.url.pathSegments.last == 'no_publish_b.json') { return http.Response(json.encode(httpResponseB), 200); } - return null; + return http.Response('', 500); }); final PublishCheckCommand command = PublishCheckCommand(packagesDir, processRunner: processRunner, httpClient: mockClient); @@ -308,7 +306,7 @@ void main() { } else if (request.url.pathSegments.last == 'no_publish_b.json') { return http.Response(json.encode(httpResponseB), 200); } - return null; + return http.Response('', 500); }); final PublishCheckCommand command = PublishCheckCommand(packagesDir, processRunner: processRunner, httpClient: mockClient); diff --git a/script/tool/test/version_check_test.dart b/script/tool/test/version_check_test.dart index fd33c21b2d..1199c27064 100644 --- a/script/tool/test/version_check_test.dart +++ b/script/tool/test/version_check_test.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart=2.9 - import 'dart:async'; import 'dart:convert'; import 'dart:io' as io; @@ -13,24 +11,25 @@ import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_plugin_tools/src/common.dart'; import 'package:flutter_plugin_tools/src/version_check_command.dart'; -import 'package:git/git.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; import 'package:mockito/mockito.dart'; import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart'; + +import 'common_test.mocks.dart'; import 'util.dart'; void testAllowedVersion( String masterVersion, String headVersion, { bool allowed = true, - NextVersionType nextVersionType, + NextVersionType? nextVersionType, }) { final Version master = Version.parse(masterVersion); final Version head = Version.parse(headVersion); final Map allowedVersions = - getAllowedNextVersions(master, head); + getAllowedNextVersions(masterVersion: master, headVersion: head); if (allowed) { expect(allowedVersions, contains(head)); if (nextVersionType != null) { @@ -41,8 +40,6 @@ void testAllowedVersion( } } -class MockGitDir extends Mock implements GitDir {} - class MockProcessResult extends Mock implements io.ProcessResult {} const String _redColorMessagePrefix = '\x1B[31m'; @@ -57,13 +54,13 @@ void main() { const String indentation = ' '; group('$VersionCheckCommand', () { FileSystem fileSystem; - Directory packagesDir; - CommandRunner runner; - RecordingProcessRunner processRunner; - List> gitDirCommands; + late Directory packagesDir; + late CommandRunner runner; + late RecordingProcessRunner processRunner; + late List> gitDirCommands; String gitDiffResponse; Map gitShowResponses; - MockGitDir gitDir; + late MockGitDir gitDir; setUp(() { fileSystem = MemoryFileSystem(); @@ -77,17 +74,19 @@ void main() { gitDirCommands.add(invocation.positionalArguments[0] as List); final MockProcessResult mockProcessResult = MockProcessResult(); if (invocation.positionalArguments[0][0] == 'diff') { - when(mockProcessResult.stdout as String) + when(mockProcessResult.stdout as String?) .thenReturn(gitDiffResponse); } else if (invocation.positionalArguments[0][0] == 'show') { - final String response = + final String? response = gitShowResponses[invocation.positionalArguments[0][1]]; if (response == null) { throw const io.ProcessException('git', ['show']); } - when(mockProcessResult.stdout as String).thenReturn(response); + when(mockProcessResult.stdout as String?) + .thenReturn(response); } else if (invocation.positionalArguments[0][0] == 'merge-base') { - when(mockProcessResult.stdout as String).thenReturn('abc123'); + when(mockProcessResult.stdout as String?) + .thenReturn('abc123'); } return Future.value(mockProcessResult); });