[tool] Remove --against-pub flag (#11550)

We haven't used `--against-pub` in CI in a very long time, and we don't expect to need it locally. We report pub versions as part of the publish step, but the source of truth for validation is the repo, not pub. This removes the flag to simplify the tool.

Author: stuartmorgan-g

script/tool/lib/src/version_check_command.dart

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 import 'package:file/file.dart';
-import 'package:http/http.dart' as http;
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
 import 'package:pub_semver/pub_semver.dart';
@@ -12,7 +11,6 @@ import 'common/git_version_finder.dart';
 import 'common/output_utils.dart';
 import 'common/package_looping_command.dart';
 import 'common/package_state_utils.dart';
-import 'common/pub_version_finder.dart';
 import 'common/repository_package.dart';
 
 /// Categories of version change types.
@@ -98,17 +96,7 @@ class VersionCheckCommand extends PackageLoopingCommand {
     super.processRunner,
     super.platform,
     super.gitDir,
-    http.Client? httpClient,
-  }) : _pubVersionFinder = PubVersionFinder(
-         httpClient: httpClient ?? http.Client(),
-       ) {
-    argParser.addFlag(
-      _againstPubFlag,
-      help:
-          'Whether the version check should run against the version on pub.\n'
-          'Defaults to false, which means the version check only run against '
-          'the previous version in code.',
-    );
+  }) {
     argParser.addOption(
       _prLabelsArg,
       help:
@@ -140,7 +128,6 @@ class VersionCheckCommand extends PackageLoopingCommand {
     );
   }
 
-  static const String _againstPubFlag = 'against-pub';
   static const String _prLabelsArg = 'pr-labels';
   static const String _checkForMissingChanges = 'check-for-missing-changes';
   static const String _ignorePlatformInterfaceBreaks =
@@ -161,8 +148,6 @@ class VersionCheckCommand extends PackageLoopingCommand {
   static const String _missingChangelogChangeOverrideLabel =
       'override: no changelog needed';
 
-  final PubVersionFinder _pubVersionFinder;
-
   late final GitVersionFinder _gitVersionFinder;
 
   late final Set<String> _prLabels = _getPRLabels();
@@ -184,7 +169,7 @@ class VersionCheckCommand extends PackageLoopingCommand {
   final String description =
       'Checks if the versions of packages have been incremented per pub specification.\n'
       '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.';
+      'This command requires "flutter" to be in your path.';
 
   @override
   bool get hasLongOutput => false;
@@ -303,33 +288,6 @@ class VersionCheckCommand extends PackageLoopingCommand {
         : PackageResult.fail(errors);
   }
 
-  @override
-  Future<void> completeRun() async {
-    _pubVersionFinder.httpClient.close();
-  }
-
-  /// Returns the previous published version of [package].
-  ///
-  /// [packageName] must be the actual name of the package as published (i.e.,
-  /// the name from pubspec.yaml, not the on disk name if different.)
-  Future<Version?> _fetchPreviousVersionFromPub(String packageName) async {
-    final PubVersionFinderResponse pubVersionFinderResponse =
-        await _pubVersionFinder.getPackageVersion(packageName: packageName);
-    switch (pubVersionFinderResponse.result) {
-      case PubVersionFinderResult.success:
-        return pubVersionFinderResponse.versions.first;
-      case PubVersionFinderResult.fail:
-        printError('''
-${indentation}Error fetching version on pub for $packageName.
-${indentation}HTTP Status ${pubVersionFinderResponse.httpResponse.statusCode}
-${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
-''');
-        return null;
-      case PubVersionFinderResult.noPackageFound:
-        return Version.none;
-    }
-  }
-
   /// Returns the version of [package] from git at the base comparison hash.
   Future<Version?> _getPreviousVersionFromGit(RepositoryPackage package) async {
     final File pubspecFile = package.pubspecFile;
@@ -352,28 +310,12 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
   }) async {
     // This method isn't called unless `version` is non-null.
     final Version currentVersion = pubspec.version!;
-    Version? previousVersion;
-    String previousVersionSource;
-    if (getBoolArg(_againstPubFlag)) {
-      previousVersionSource = 'pub';
-      previousVersion = await _fetchPreviousVersionFromPub(pubspec.name);
-      if (previousVersion == null) {
-        return _CurrentVersionState.unknown;
-      }
-      if (previousVersion != Version.none) {
-        print(
-          '$indentation${pubspec.name}: Current largest version on pub: $previousVersion',
-        );
-      }
-    } else {
-      previousVersionSource = baseSha;
-      previousVersion =
-          await _getPreviousVersionFromGit(package) ?? Version.none;
-    }
+    final Version previousVersion =
+        await _getPreviousVersionFromGit(package) ?? Version.none;
     if (previousVersion == Version.none) {
       print(
         '${indentation}Unable to find previous version '
-        '${getBoolArg(_againstPubFlag) ? 'on pub server' : 'at git base'}.',
+        'at git base.',
       );
       logWarning(
         '${indentation}If this package is not new, something has gone wrong.',
@@ -387,7 +329,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
     }
 
     // Check for reverts when doing local validation.
-    if (!getBoolArg(_againstPubFlag) && currentVersion < previousVersion) {
+    if (currentVersion < previousVersion) {
       // 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.
@@ -414,7 +356,7 @@ ${indentation}HTTP response: ${pubVersionFinderResponse.httpResponse.body}
     } else {
       printError(
         '${indentation}Incorrectly updated version.\n'
-        '${indentation}HEAD: $currentVersion, $previousVersionSource: $previousVersion.\n'
+        '${indentation}HEAD: $currentVersion, $baseSha: $previousVersion.\n'
         '${indentation}Allowed versions: $allowedNextVersions',
       );
       return _CurrentVersionState.invalidChange;

script/tool/test/version_check_command_test.dart

@@ -2,15 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:convert';
-
 import 'package:args/command_runner.dart';
 import 'package:file/file.dart';
 import 'package:flutter_plugin_tools/src/common/core.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:pub_semver/pub_semver.dart';
 import 'package:test/test.dart';
 
@@ -40,15 +36,11 @@ void testAllowedVersion(
 }
 
 void main() {
-  const indentation = '  ';
   group('VersionCheckCommand', () {
     late MockPlatform mockPlatform;
     late Directory packagesDir;
     late CommandRunner<void> runner;
     late RecordingProcessRunner gitProcessRunner;
-    // Ignored if mockHttpResponse is set.
-    int mockHttpStatus;
-    Map<String, dynamic>? mockHttpResponse;
 
     setUp(() {
       mockPlatform = MockPlatform();
@@ -57,22 +49,11 @@ void main() {
       (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) =
           configureBaseCommandMocks(platform: mockPlatform);
 
-      // Default to simulating the plugin never having been published.
-      mockHttpStatus = 404;
-      mockHttpResponse = null;
-      final mockClient = MockClient((http.Request request) async {
-        return http.Response(
-          json.encode(mockHttpResponse),
-          mockHttpResponse == null ? mockHttpStatus : 200,
-        );
-      });
-
       final command = VersionCheckCommand(
         packagesDir,
         processRunner: processRunner,
         platform: mockPlatform,
         gitDir: gitDir,
-        httpClient: mockClient,
       );
 
       runner = CommandRunner<void>(
@@ -448,7 +429,7 @@ void main() {
       Error? commandError;
       final List<String> output = await runCapturingPrint(
         runner,
-        <String>['version-check', '--base-sha=main', '--against-pub'],
+        <String>['version-check', '--base-sha=main'],
         errorHandler: (Error e) {
           commandError = e;
         },
@@ -594,7 +575,7 @@ void main() {
         var hasError = false;
         final List<String> output = await runCapturingPrint(
           runner,
-          <String>['version-check', '--base-sha=main', '--against-pub'],
+          <String>['version-check', '--base-sha=main'],
           errorHandler: (Error e) {
             expect(e, isA<ToolExit>());
             hasError = true;
@@ -669,7 +650,7 @@ void main() {
       var hasError = false;
       final List<String> output = await runCapturingPrint(
         runner,
-        <String>['version-check', '--base-sha=main', '--against-pub'],
+        <String>['version-check', '--base-sha=main'],
         errorHandler: (Error e) {
           expect(e, isA<ToolExit>());
           hasError = true;
@@ -1652,114 +1633,6 @@ packages/plugin/lib/plugin.dart
       });
     });
 
-    test('allows valid against pub', () async {
-      mockHttpResponse = <String, dynamic>{
-        'name': 'some_package',
-        'versions': <String>['0.0.1', '0.0.2', '1.0.0'],
-      };
-
-      createFakePlugin('plugin', packagesDir, version: '2.0.0');
-      final List<String> output = await runCapturingPrint(runner, <String>[
-        'version-check',
-        '--base-sha=main',
-        '--against-pub',
-      ]);
-
-      expect(
-        output,
-        containsAllInOrder(<Matcher>[
-          contains('plugin: Current largest version on pub: 1.0.0'),
-        ]),
-      );
-    });
-
-    test('denies invalid against pub', () async {
-      mockHttpResponse = <String, dynamic>{
-        'name': 'some_package',
-        'versions': <String>['0.0.1', '0.0.2'],
-      };
-
-      createFakePlugin('plugin', packagesDir, version: '2.0.0');
-
-      var hasError = false;
-      final List<String> result = await runCapturingPrint(
-        runner,
-        <String>['version-check', '--base-sha=main', '--against-pub'],
-        errorHandler: (Error e) {
-          expect(e, isA<ToolExit>());
-          hasError = true;
-        },
-      );
-      expect(hasError, isTrue);
-
-      expect(
-        result,
-        containsAllInOrder(<Matcher>[
-          contains(
-            '''
-${indentation}Incorrectly updated version.
-${indentation}HEAD: 2.0.0, pub: 0.0.2.
-${indentation}Allowed versions: {1.0.0: NextVersionType.BREAKING_MAJOR, 0.1.0: NextVersionType.MINOR, 0.0.3: NextVersionType.PATCH}''',
-          ),
-        ]),
-      );
-    });
-
-    test(
-      'throw and print error message if http request failed when checking against pub',
-      () async {
-        mockHttpStatus = 400;
-
-        createFakePlugin('plugin', packagesDir, version: '2.0.0');
-        var hasError = false;
-        final List<String> result = await runCapturingPrint(
-          runner,
-          <String>['version-check', '--base-sha=main', '--against-pub'],
-          errorHandler: (Error e) {
-            expect(e, isA<ToolExit>());
-            hasError = true;
-          },
-        );
-        expect(hasError, isTrue);
-
-        expect(
-          result,
-          containsAllInOrder(<Matcher>[
-            contains('''
-${indentation}Error fetching version on pub for plugin.
-${indentation}HTTP Status 400
-${indentation}HTTP response: null
-'''),
-          ]),
-        );
-      },
-    );
-
-    test(
-      'when checking against pub, allow any version if http status is 404.',
-      () async {
-        mockHttpStatus = 404;
-
-        createFakePlugin('plugin', packagesDir, version: '2.0.0');
-        gitProcessRunner.mockProcessesForExecutable['git-show'] =
-            <FakeProcessInfo>[
-              FakeProcessInfo(MockProcess(stdout: 'version: 1.0.0')),
-            ];
-        final List<String> result = await runCapturingPrint(runner, <String>[
-          'version-check',
-          '--base-sha=main',
-          '--against-pub',
-        ]);
-
-        expect(
-          result,
-          containsAllInOrder(<Matcher>[
-            contains('Unable to find previous version on pub server.'),
-          ]),
-        );
-      },
-    );
-
     group('prelease versions', () {
       test(
         'allow an otherwise-valid transition that also adds a pre-release component',