add lint depend_on_referenced_packages (#2659)

* add lint always_depend_on_packages_you_use

* add entry to example/all.yaml

* code style updates

* optimizations, dont use Uri.parse and hoist up the available deps calculation

* clean up code a bit, handle the no slash case

* remove unused context field

* rename to depend_on_referenced_packages

* a few more bits of cleanup from the rename

* organize imports

example/all.yaml

@@ -59,6 +59,7 @@ linter:
     - constant_identifier_names
     - control_flow_in_finally
     - curly_braces_in_flow_control_structures
+    - depend_on_referenced_packages
     - deprecated_consistency
     - diagnostic_describe_all_properties
     - directives_ordering

lib/src/analyzer.dart

@@ -43,9 +43,11 @@ export 'package:analyzer/src/lint/linter.dart'
         NodeLintRule;
 export 'package:analyzer/src/lint/project.dart'
     show DartProject, ProjectVisitor;
-export 'package:analyzer/src/lint/pub.dart' show PubspecVisitor, PSEntry;
+export 'package:analyzer/src/lint/pub.dart'
+    show PubspecVisitor, PSEntry, Pubspec;
 export 'package:analyzer/src/lint/util.dart' show Spelunker;
 export 'package:analyzer/src/services/lint.dart' show lintRegistry;
+export 'package:analyzer/src/workspace/pub.dart' show PubWorkspacePackage;
 
 const loggedAnalyzerErrorExitCode = 63;
 

lib/src/ast.dart

@@ -129,6 +129,20 @@ bool isInLibDir(CompilationUnit node, WorkspacePackage? package) {
   return path.isWithin(libDir, cuPath);
 }
 
+/// Return `true` if this compilation unit [node] is declared within a public
+/// directory in the given [package]'s directory tree. Public dirs are the
+/// `lib` and `bin` dirs.
+//
+//  TODO: move into WorkspacePackage
+bool isInPublicDir(CompilationUnit node, WorkspacePackage? package) {
+  if (package == null) return false;
+  var cuPath = node.declaredElement?.library.source.fullName;
+  if (cuPath == null) return false;
+  var libDir = path.join(package.root, 'lib');
+  var binDir = path.join(package.root, 'bin');
+  return path.isWithin(libDir, cuPath) || path.isWithin(binDir, cuPath);
+}
+
 /// Returns `true` if the keyword associated with the given [token] matches
 /// [keyword].
 bool isKeyword(Token token, Keyword keyword) =>

lib/src/rules.dart

@@ -61,6 +61,7 @@ import 'rules/comment_references.dart';
 import 'rules/constant_identifier_names.dart';
 import 'rules/control_flow_in_finally.dart';
 import 'rules/curly_braces_in_flow_control_structures.dart';
+import 'rules/depend_on_referenced_packages.dart';
 import 'rules/deprecated_consistency.dart';
 import 'rules/diagnostic_describe_all_properties.dart';
 import 'rules/directives_ordering.dart';
@@ -257,6 +258,7 @@ void registerLintRules({bool inTestMode = false}) {
     ..register(ConstantIdentifierNames())
     ..register(ControlFlowInFinally())
     ..register(CurlyBracesInFlowControlStructures())
+    ..register(DependOnReferencedPackages())
     ..register(DeprecatedConsistency())
     ..register(DiagnosticsDescribeAllProperties())
     ..register(DirectivesOrdering())

lib/src/rules/depend_on_referenced_packages.dart

@@ -0,0 +1,109 @@
+// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
+// for details. 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:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+
+import '../analyzer.dart';
+import '../ast.dart';
+
+const _desc = r'Depend on referenced packages.';
+
+const _details = r'''
+
+**DO** Depend on referenced packages.
+
+When importing a package, add a dependency on it to your pubspec.
+
+Depending explicitly on packages that you reference ensures they will always
+exist and allows you to put a dependency constraint on them to guard you
+against breaking changes.
+
+Whether this should be a regular dependency or dev_dependency depends on if it
+is referenced from a public file (one under either `lib` or `bin`), or some
+other private file.
+
+**BAD:**
+```dart
+import 'package:a/a.dart';
+```
+
+```yaml
+dependencies:
+```
+
+**GOOD:**
+```dart
+import 'package:a/a.dart';
+```
+
+```yaml
+dependencies:
+  a: ^1.0.0
+```
+
+''';
+
+class DependOnReferencedPackages extends LintRule implements NodeLintRule {
+  DependOnReferencedPackages()
+      : super(
+            name: 'depend_on_referenced_packages',
+            description: _desc,
+            details: _details,
+            group: Group.pub);
+
+  @override
+  void registerNodeProcessors(
+      NodeLintRegistry registry, LinterContext context) {
+    // Only lint if we have a pubspec.
+    var package = context.package;
+    if (package is! PubWorkspacePackage) return;
+    var pubspec = package.pubspec;
+    if (pubspec == null) return;
+
+    var dependencies = pubspec.dependencies;
+    var devDependencies = pubspec.devDependencies;
+    var availableDeps = [
+      if (dependencies != null)
+        for (var dep in dependencies)
+          if (dep.name?.text != null) dep.name!.text!,
+      if (devDependencies != null &&
+          !isInPublicDir(context.currentUnit.unit, context.package))
+        for (var dep in devDependencies)
+          if (dep.name?.text != null) dep.name!.text!,
+    ];
+
+    var visitor = _Visitor(this, availableDeps);
+    registry.addImportDirective(this, visitor);
+    registry.addExportDirective(this, visitor);
+  }
+}
+
+class _Visitor extends SimpleAstVisitor {
+  final DependOnReferencedPackages rule;
+  final List<String> availableDeps;
+
+  _Visitor(this.rule, this.availableDeps);
+
+  void _checkDirective(UriBasedDirective node) {
+    // Is it a package: uri?
+    var uriContent = node.uriContent;
+    if (uriContent == null) return;
+    if (!uriContent.startsWith('package:')) return;
+
+    // The package name is the first segment of the uri, find the first slash.
+    var firstSlash = uriContent.indexOf('/');
+    if (firstSlash == -1) return;
+
+    var packageName = uriContent.substring(8, firstSlash);
+    if (availableDeps.contains(packageName)) return;
+    rule.reportLint(node.uri);
+  }
+
+  @override
+  void visitImportDirective(ImportDirective node) => _checkDirective(node);
+
+  @override
+  void visitExportDirective(ExportDirective node) => _checkDirective(node);
+}

test/integration/depend_on_referenced_packages.dart

@@ -0,0 +1,93 @@
+// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:io';
+
+import 'package:analyzer/src/lint/io.dart';
+import 'package:linter/src/cli.dart' as cli;
+import 'package:test/test.dart';
+
+import '../mocks.dart';
+import '../test_constants.dart';
+
+void main() {
+  group('Depend on referenced packages', () {
+    var currentOut = outSink;
+    var collectingOut = CollectingSink();
+    setUp(() {
+      exitCode = 0;
+      outSink = collectingOut;
+    });
+    tearDown(() {
+      collectingOut.buffer.clear();
+      outSink = currentOut;
+      exitCode = 0;
+    });
+
+    test('lints files under bin', () async {
+      var packagesFilePath = File('.packages').absolute.path;
+      await cli.run([
+        '--packages',
+        packagesFilePath,
+        '$integrationTestDir/depend_on_referenced_packages/bin',
+        '--rules=depend_on_referenced_packages'
+      ]);
+      expect(
+          collectingOut.trim(),
+          stringContainsInOrder([
+            "Depend on referenced packages.",
+            "import 'package:test/test.dart'; // LINT",
+            "Depend on referenced packages.",
+            "import 'package:matcher/matcher.dart'; // LINT",
+            "Depend on referenced packages.",
+            "export 'package:test/test.dart'; // LINT",
+            "Depend on referenced packages.",
+            "export 'package:matcher/matcher.dart'; // LINT",
+          ]));
+      expect(exitCode, 1);
+    });
+
+    test('lints files under lib', () async {
+      var packagesFilePath = File('.packages').absolute.path;
+      await cli.run([
+        '--packages',
+        packagesFilePath,
+        '$integrationTestDir/depend_on_referenced_packages/lib',
+        '--rules=depend_on_referenced_packages'
+      ]);
+      expect(
+          collectingOut.trim(),
+          stringContainsInOrder([
+            "Depend on referenced packages.",
+            "import 'package:test/test.dart'; // LINT",
+            "Depend on referenced packages.",
+            "import 'package:matcher/matcher.dart'; // LINT",
+            "Depend on referenced packages.",
+            "export 'package:test/test.dart'; // LINT",
+            "Depend on referenced packages.",
+            "export 'package:matcher/matcher.dart'; // LINT",
+          ]));
+      expect(exitCode, 1);
+    });
+
+    test('lints files under test', () async {
+      var packagesFilePath = File('.packages').absolute.path;
+      await cli.run([
+        '--packages',
+        packagesFilePath,
+        '$integrationTestDir/depend_on_referenced_packages/test',
+        '--rules=depend_on_referenced_packages'
+      ]);
+      expect(
+          collectingOut.trim(),
+          stringContainsInOrder([
+            "Depend on referenced packages.",
+            "import 'package:matcher/matcher.dart'; // LINT",
+            "Depend on referenced packages.",
+            "export 'package:matcher/matcher.dart'; // LINT",
+          ]));
+      expect(exitCode, 1);
+    });
+  });
+}

test/integration_test.dart

@@ -25,6 +25,8 @@ import 'integration/avoid_web_libraries_in_flutter.dart'
     as avoid_web_libraries_in_flutter;
 import 'integration/cancel_subscriptions.dart' as cancel_subscriptions;
 import 'integration/close_sinks.dart' as close_sinks;
+import 'integration/depend_on_referenced_packages.dart'
+    as depend_on_referenced_packages;
 import 'integration/directives_ordering.dart' as directives_ordering;
 import 'integration/exhaustive_cases.dart' as exhaustive_cases;
 import 'integration/file_names.dart' as file_names;
@@ -185,6 +187,7 @@ void ruleTests() {
     overridden_fields.main();
     close_sinks.main();
     cancel_subscriptions.main();
+    depend_on_referenced_packages.main();
     directives_ordering.main();
     file_names.main();
     flutter_style_todos.main();

test_data/integration/depend_on_referenced_packages/bin/main.dart

@@ -0,0 +1,11 @@
+// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
+// for details. 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:meta/meta.dart'; // OK
+import 'package:test/test.dart'; // LINT
+import 'package:matcher/matcher.dart'; // LINT
+
+export 'package:meta/meta.dart'; // OK
+export 'package:test/test.dart'; // LINT
+export 'package:matcher/matcher.dart'; // LINT

test_data/integration/depend_on_referenced_packages/lib/public.dart

@@ -0,0 +1,11 @@
+// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
+// for details. 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:meta/meta.dart'; // OK
+import 'package:test/test.dart'; // LINT
+import 'package:matcher/matcher.dart'; // LINT
+
+export 'package:meta/meta.dart'; // OK
+export 'package:test/test.dart'; // LINT
+export 'package:matcher/matcher.dart'; // LINT

test_data/integration/depend_on_referenced_packages/pubspec.yaml

@@ -0,0 +1,5 @@
+name: sample_project
+dependencies:
+  meta: any
+dev_dependencies:
+  test: any

test_data/integration/depend_on_referenced_packages/test/private.dart

@@ -0,0 +1,11 @@
+// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
+// for details. 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:meta/meta.dart'; // OK
+import 'package:test/test.dart'; // OK
+import 'package:matcher/matcher.dart'; // LINT
+
+export 'package:meta/meta.dart'; // OK
+export 'package:test/test.dart'; // OK
+export 'package:matcher/matcher.dart'; // LINT