[tool] Provide better CI feedback for combo PRs

script/tool/lib/src/common/core.dart

@@ -33,6 +33,10 @@ const String platformWindows = 'windows';
 /// Key for enable experiment.
 const String kEnableExperiment = 'enable-experiment';
 
+/// A String to add to comments on temporarily-added changes that should not
+/// land (e.g., dependency overrides in federated plugin combination PRs).
+const String kDoNotLandWarning = 'DO NOT MERGE';
+
 /// Target platforms supported by Flutter.
 // ignore: public_member_api_docs
 enum FlutterPlatform { android, ios, linux, macos, web, windows }

script/tool/lib/src/federation_safety_check_command.dart

@@ -6,6 +6,7 @@ import 'package:file/file.dart';
 import 'package:path/path.dart' as p;
 import 'package:pub_semver/pub_semver.dart';
 
+import 'common/core.dart';
 import 'common/file_utils.dart';
 import 'common/git_version_finder.dart';
 import 'common/output_utils.dart';
@@ -112,6 +113,21 @@ class FederationSafetyCheckCommand extends PackageLoopingCommand {
           'Platform interface changes are not validated.');
     }
 
+    // Special-case combination PRs that are following repo process, so that
+    // they don't get an error that makes it sound like something is wrong with
+    // the PR (but is still an error so that the PR can't land without following
+    // the resolution process).
+    if (package.getExamples().any(_hasTemporaryDependencyOverrides)) {
+      printError('"$kDoNotLandWarning" found in pubspec.yaml, so this is '
+          'assumed to be the initial combination PR for a federated change, '
+          'following the standard repository procedure. This failure is '
+          'expected, in order to prevent accidentally landing the temporary '
+          'overrides, and will automatically be resolved when the temporary '
+          'overrides are replaced by dependency version bumps later in the '
+          'process.');
+      return PackageResult.fail(<String>['Unresolved combo PR.']);
+    }
+
     // Uses basename to match _changedPackageFiles.
     final String basePackageName = package.directory.parent.basename;
     final String platformInterfacePackageName =
@@ -216,4 +232,9 @@ class FederationSafetyCheckCommand extends PackageLoopingCommand {
     // against having the wrong (e.g., incorrectly empty) diff output.
     return foundComment;
   }
+
+  bool _hasTemporaryDependencyOverrides(RepositoryPackage package) {
+    final String pubspecContents = package.pubspecFile.readAsStringSync();
+    return pubspecContents.contains(kDoNotLandWarning);
+  }
 }

script/tool/lib/src/make_deps_path_based_command.dart

@@ -57,7 +57,7 @@ class MakeDepsPathBasedCommand extends PackageCommand {
   // Includes a reference to the docs so that reviewers who aren't familiar with
   // the federated plugin change process don't think it's a mistake.
   static const String _dependencyOverrideWarningComment =
-      '# FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE.\n'
+      '# FOR TESTING AND INITIAL REVIEW ONLY. $kDoNotLandWarning.\n'
       '# See https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changing-federated-plugins';
 
   @override

script/tool/test/federation_safety_check_command_test.dart

@@ -262,6 +262,89 @@ index abc123..def456 100644
     );
   });
 
+  test('fails with specific text for combo PRs using the recommended tooling',
+      () async {
+    final Directory pluginGroupDir = packagesDir.childDirectory('foo');
+    final RepositoryPackage appFacing = createFakePlugin('foo', pluginGroupDir);
+    final RepositoryPackage implementation =
+        createFakePlugin('foo_bar', pluginGroupDir);
+    final RepositoryPackage platformInterface =
+        createFakePlugin('foo_platform_interface', pluginGroupDir);
+
+    void addFakeTempPubspecOverrides(RepositoryPackage package) {
+      final String contents = package.pubspecFile.readAsStringSync();
+      package.pubspecFile.writeAsStringSync('''
+$contents
+
+# FOR TESTING AND INITIAL REVIEW ONLY. $kDoNotLandWarning.
+dependency_overrides:
+  foo_platform_interface:
+    path: ../../../foo/foo_platform_interface
+''');
+    }
+
+    addFakeTempPubspecOverrides(appFacing.getExamples().first);
+    addFakeTempPubspecOverrides(implementation.getExamples().first);
+
+    const String appFacingChanges = '''
+diff --git a/packages/foo/foo/lib/foo.dart b/packages/foo/foo/lib/foo.dart
+index abc123..def456 100644
+--- a/packages/foo/foo/lib/foo.dart
++++ b/packages/foo/foo/lib/foo.dart
+@@ -51,6 +51,9 @@ Future<bool> launchUrl(
+   return true;
+ }
+
++// This is a new method
++bool foo() => true;
++
+ // This in an existing method
+ void aMethod() {
+   // Do things.
+''';
+
+    final String changedFileOutput = <File>[
+      appFacing.libDirectory.childFile('foo.dart'),
+      implementation.libDirectory.childFile('foo.dart'),
+      platformInterface.pubspecFile,
+      platformInterface.libDirectory.childFile('foo.dart'),
+    ].map((File file) => file.path).join('\n');
+    processRunner.mockProcessesForExecutable['git-diff'] = <FakeProcessInfo>[
+      FakeProcessInfo(MockProcess(stdout: changedFileOutput)),
+      FakeProcessInfo(MockProcess(stdout: appFacingChanges),
+          <String>['', 'HEAD', '--', '/packages/foo/foo/lib/foo.dart']),
+      // The others diffs don't need to be specified, since empty diff is also
+      // treated as a non-comment change.
+    ];
+
+    Error? commandError;
+    final List<String> output = await runCapturingPrint(
+        runner, <String>['federation-safety-check'], errorHandler: (Error e) {
+      commandError = e;
+    });
+
+    expect(commandError, isA<ToolExit>());
+    expect(
+      output,
+      containsAllInOrder(<Matcher>[
+        contains('Running for foo/foo...'),
+        contains('"DO NOT MERGE" found in pubspec.yaml, so this is assumed to '
+            'be the initial combination PR for a federated change, following '
+            'the standard repository procedure. This failure is expected, in '
+            'order to prevent accidentally landing the temporary overrides, '
+            'and will automatically be resolved when the temporary overrides '
+            'are replaced by dependency version bumps later in the process.'),
+        contains('Running for foo_bar...'),
+        contains('"DO NOT MERGE" found in pubspec.yaml'),
+        contains('The following packages had errors:'),
+        contains('foo/foo:\n'
+            '    Unresolved combo PR.'),
+        contains('foo_bar:\n'
+            '    Unresolved combo PR.'),
+      ]),
+    );
+  });
+
   test('ignores test-only changes to interface packages', () async {
     final Directory pluginGroupDir = packagesDir.childDirectory('foo');
     final RepositoryPackage appFacing = createFakePlugin('foo', pluginGroupDir);