From 3ff376b7b733b8256fbcd459fe0198f5fa7b9520 Mon Sep 17 00:00:00 2001
From: Reid Baker <reidbaker@google.com>
Date: Wed, 3 Apr 2024 15:44:11 -0400
Subject: [PATCH] [Tool] Add ability to check dependencies independently of
 dev-dependencies, exclude integration_test from dependencies (#6446)

Create a linter that ensures that `integration_test` is not used in dependencies.

Will be paired with a change to documentation
```
If you are considering adding an external dependency:

Consider other options, and discuss with #hackers-ecosystem in Discord.
* If you add a dev_dependency on an external package, pin it to a specific version if at all possible.
* If you add a dependency on an external package in an example/, pin it to a specific version if at all possible.
* Some dependencies should only be linked as dev dependencies like integration_test
```

Related to flutter/flutter/issues/145992
---
 .../tool/lib/src/pubspec_check_command.dart   | 17 +++++-
 .../tool/test/pubspec_check_command_test.dart | 59 +++++++++++++++++++
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/script/tool/lib/src/pubspec_check_command.dart b/script/tool/lib/src/pubspec_check_command.dart
index 378dfc65caa8..008dc2df3b63 100644
--- a/script/tool/lib/src/pubspec_check_command.dart
+++ b/script/tool/lib/src/pubspec_check_command.dart
@@ -356,7 +356,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
             'a topic. Add "$topicName" to the "topics" section.';
       }
     }
-    
+
     // Validates topic names according to https://dart.dev/tools/pub/pubspec#topics
     final RegExp expectedTopicFormat = RegExp(r'^[a-z](?:-?[a-z0-9]+)*$');
     final Iterable<String> invalidTopics = topics.where((String topic) =>
@@ -542,6 +542,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
   // there are any that aren't allowed.
   String? _checkDependencies(Pubspec pubspec) {
     final Set<String> badDependencies = <String>{};
+    // Shipped dependencies.
     for (final Map<String, Dependency> dependencies
         in <Map<String, Dependency>>[
       pubspec.dependencies,
@@ -553,6 +554,19 @@ class PubspecCheckCommand extends PackageLoopingCommand {
         }
       });
     }
+
+    // Ensure that dev-only dependencies aren't in `dependencies`.
+    const List<String> devOnlyDependencies = <String>['integration_test'];
+    // Non-published packages like pidgeon subpackages are allowed to violate
+    // the dev only dependencies rule.
+    if (pubspec.publishTo != 'none') {
+      pubspec.dependencies.forEach((String name, Dependency dependency) {
+        if (devOnlyDependencies.contains(name)) {
+          badDependencies.add(name);
+        }
+      });
+    }
+
     if (badDependencies.isEmpty) {
       return null;
     }
@@ -563,6 +577,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
   }
 
   // Checks whether a given dependency is allowed.
+  // Defaults to false.
   bool _shouldAllowDependency(String name, Dependency dependency) {
     if (dependency is PathDependency || dependency is SdkDependency) {
       return true;
diff --git a/script/tool/test/pubspec_check_command_test.dart b/script/tool/test/pubspec_check_command_test.dart
index 8e99d720f57f..8427ea3dcbb0 100644
--- a/script/tool/test/pubspec_check_command_test.dart
+++ b/script/tool/test/pubspec_check_command_test.dart
@@ -1734,6 +1734,65 @@ ${_topicsSection()}
           ]),
         );
       });
+
+      test('fails when integration_test is used in non dev dependency',
+          () async {
+        final RepositoryPackage package =
+            createFakePackage('a_package', packagesDir, examples: <String>[]);
+
+        package.pubspecFile.writeAsStringSync('''
+${_headerSection('a_package')}
+${_environmentSection()}
+${_dependenciesSection(<String>['integration_test: \n    sdk: flutter'])}
+${_devDependenciesSection()}
+${_topicsSection()}
+''');
+
+        Error? commandError;
+        final List<String> output = await runCapturingPrint(runner, <String>[
+          'pubspec-check',
+        ], errorHandler: (Error e) {
+          commandError = e;
+        });
+
+        expect(commandError, isA<ToolExit>());
+        expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            contains(
+                'The following unexpected non-local dependencies were found:\n'
+                '  integration_test\n'
+                'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies '
+                'for more information and next steps.'),
+          ]),
+        );
+      });
+
+      test('passes when integration_test is used in non published package',
+          () async {
+        final RepositoryPackage package =
+            createFakePackage('a_package', packagesDir, examples: <String>[]);
+
+        package.pubspecFile.writeAsStringSync('''
+${_headerSection('a_package', publishable: false)}
+${_environmentSection()}
+${_dependenciesSection(<String>['integration_test: \n    sdk: flutter'])}
+${_devDependenciesSection()}
+${_topicsSection()}
+''');
+
+        final List<String> output = await runCapturingPrint(runner, <String>[
+          'pubspec-check',
+        ]);
+
+        expect(
+          output,
+          containsAllInOrder(<Matcher>[
+            contains('Running for a_package...'),
+            contains('Ran for'),
+          ]),
+        );
+      });
     });
   });