From 438e206ff7638dca4a6a128ecc1dbe51fef37e33 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 19 May 2021 10:13:44 -0700 Subject: [PATCH 1/9] add lint always_depend_on_packages_you_use --- lib/src/analyzer.dart | 4 +- lib/src/ast.dart | 12 ++ lib/src/rules.dart | 2 + .../always_depend_on_packages_you_use.dart | 112 ++++++++++++++++++ .../always_depend_on_packages_you_use.dart | 93 +++++++++++++++ test/integration_test.dart | 3 + .../bin/main.dart | 11 ++ .../lib/public.dart | 11 ++ .../pubspec.yaml | 5 + .../test/private.dart | 11 ++ 10 files changed, 263 insertions(+), 1 deletion(-) create mode 100644 lib/src/rules/always_depend_on_packages_you_use.dart create mode 100644 test/integration/always_depend_on_packages_you_use.dart create mode 100644 test_data/integration/always_depend_on_packages_you_use/bin/main.dart create mode 100644 test_data/integration/always_depend_on_packages_you_use/lib/public.dart create mode 100644 test_data/integration/always_depend_on_packages_you_use/pubspec.yaml create mode 100644 test_data/integration/always_depend_on_packages_you_use/test/private.dart diff --git a/lib/src/analyzer.dart b/lib/src/analyzer.dart index 1cb7cbf6e..7c65f9bb9 100644 --- a/lib/src/analyzer.dart +++ b/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; diff --git a/lib/src/ast.dart b/lib/src/ast.dart index 781f384b5..e5c8bb1f7 100644 --- a/lib/src/ast.dart +++ b/lib/src/ast.dart @@ -129,6 +129,18 @@ 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. +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) => diff --git a/lib/src/rules.dart b/lib/src/rules.dart index b08b651dc..ada1be63c 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -4,6 +4,7 @@ import 'analyzer.dart'; import 'rules/always_declare_return_types.dart'; +import 'rules/always_depend_on_packages_you_use.dart'; import 'rules/always_put_control_body_on_new_line.dart'; import 'rules/always_put_required_named_parameters_first.dart'; import 'rules/always_require_non_null_named_parameters.dart'; @@ -200,6 +201,7 @@ void registerLintRules({bool inTestMode = false}) { Analyzer.facade.cacheLinterVersion(); Analyzer.facade ..register(AlwaysDeclareReturnTypes()) + ..register(AlwaysDependOnPackagesYouUse()) ..register(AlwaysPutControlBodyOnNewLine()) ..register(AlwaysPutRequiredNamedParametersFirst()) ..register(AlwaysRequireNonNullNamedParameters()) diff --git a/lib/src/rules/always_depend_on_packages_you_use.dart b/lib/src/rules/always_depend_on_packages_you_use.dart new file mode 100644 index 000000000..010c2b204 --- /dev/null +++ b/lib/src/rules/always_depend_on_packages_you_use.dart @@ -0,0 +1,112 @@ +// 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 packages you use.'; + +const _details = r''' + +**DO** depend on packages you use. + +When importing a package, *always* add a dependency on it to your pubspec. + +Depending explicitly on packages that you use 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 imported from a public file (one under either `lib` or `bin`), or some other +file. + +**BAD:** +```dart +import 'package:a/a.dart'; +import 'package:b/b.dart'; +``` + +```yaml +dependencies: + a: ^1.0.0 +``` + +**GOOD:** +```dart +import 'package:a/a.dart'; +import 'package:b/b.dart'; +``` + +```yaml +dependencies: + a: ^1.0.0 + b: ^1.0.0 +``` + +'''; + +class AlwaysDependOnPackagesYouUse extends LintRule implements NodeLintRule { + AlwaysDependOnPackagesYouUse() + : super( + name: 'always_depend_on_packages_you_use', + 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 visitor = _Visitor(this, context, pubspec, + isPublicFile: isInPublicDir(context.currentUnit.unit, context.package)); + registry.addImportDirective(this, visitor); + registry.addExportDirective(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + final AlwaysDependOnPackagesYouUse rule; + final LinterContext context; + final Pubspec pubspec; + final bool isPublicFile; + late final availableDeps = [ + if (pubspec.dependencies != null) + for (var dep in pubspec.dependencies!) + if (dep.name?.text != null) dep.name!.text!, + if (!isPublicFile && pubspec.devDependencies != null) + for (var dep in pubspec.devDependencies!) + if (dep.name?.text != null) dep.name!.text! + ]; + + _Visitor(this.rule, this.context, this.pubspec, {required this.isPublicFile}); + + void _checkDirective(UriBasedDirective node) { + // Is it a package: import? + var importUriContent = node.uriContent; + if (importUriContent == null) return; + if (!importUriContent.startsWith('package:')) return; + + try { + var importUri = Uri.parse(importUriContent); + if (importUri.pathSegments.isEmpty) return; + var packageName = importUri.pathSegments.first; + if (availableDeps.contains(packageName)) return; + rule.reportLint(node.uri); + } on FormatException catch (_) {} + } + + @override + void visitImportDirective(ImportDirective node) => _checkDirective(node); + + @override + void visitExportDirective(ExportDirective node) => _checkDirective(node); +} diff --git a/test/integration/always_depend_on_packages_you_use.dart b/test/integration/always_depend_on_packages_you_use.dart new file mode 100644 index 000000000..0700eaf1c --- /dev/null +++ b/test/integration/always_depend_on_packages_you_use.dart @@ -0,0 +1,93 @@ +// Copyright (c) 2020, 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('always depend on packages you use', () { + 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/always_depend_on_packages_you_use/bin', + '--rules=always_depend_on_packages_you_use' + ]); + expect( + collectingOut.trim(), + stringContainsInOrder([ + "Depend on packages you use.", + "import 'package:test/test.dart'; // LINT", + "Depend on packages you use.", + "import 'package:matcher/matcher.dart'; // LINT", + "Depend on packages you use.", + "export 'package:test/test.dart'; // LINT", + "Depend on packages you use.", + "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/always_depend_on_packages_you_use/lib', + '--rules=always_depend_on_packages_you_use' + ]); + expect( + collectingOut.trim(), + stringContainsInOrder([ + "Depend on packages you use.", + "import 'package:test/test.dart'; // LINT", + "Depend on packages you use.", + "import 'package:matcher/matcher.dart'; // LINT", + "Depend on packages you use.", + "export 'package:test/test.dart'; // LINT", + "Depend on packages you use.", + "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/always_depend_on_packages_you_use/test', + '--rules=always_depend_on_packages_you_use' + ]); + expect( + collectingOut.trim(), + stringContainsInOrder([ + "Depend on packages you use.", + "import 'package:matcher/matcher.dart'; // LINT", + "Depend on packages you use.", + "export 'package:matcher/matcher.dart'; // LINT", + ])); + expect(exitCode, 1); + }); + }); +} diff --git a/test/integration_test.dart b/test/integration_test.dart index 80d5c8c5c..0d92995cd 100644 --- a/test/integration_test.dart +++ b/test/integration_test.dart @@ -13,6 +13,8 @@ import 'package:test/test.dart'; import 'package:yaml/yaml.dart'; import '../test_data/rules/experiments/experiments.dart'; +import 'integration/always_depend_on_packages_you_use.dart' + as always_depend_on_packages_you_use; import 'integration/always_require_non_null_named_parameters.dart' as always_require_non_null_named_parameters; import 'integration/avoid_private_typedef_functions.dart' @@ -190,6 +192,7 @@ void ruleTests() { flutter_style_todos.main(); lines_longer_than_80_chars.main(); only_throw_errors.main(); + always_depend_on_packages_you_use.main(); always_require_non_null_named_parameters.main(); prefer_asserts_in_initializer_lists.main(); prefer_const_constructors_in_immutables.main(); diff --git a/test_data/integration/always_depend_on_packages_you_use/bin/main.dart b/test_data/integration/always_depend_on_packages_you_use/bin/main.dart new file mode 100644 index 000000000..853dddc19 --- /dev/null +++ b/test_data/integration/always_depend_on_packages_you_use/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 diff --git a/test_data/integration/always_depend_on_packages_you_use/lib/public.dart b/test_data/integration/always_depend_on_packages_you_use/lib/public.dart new file mode 100644 index 000000000..853dddc19 --- /dev/null +++ b/test_data/integration/always_depend_on_packages_you_use/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 diff --git a/test_data/integration/always_depend_on_packages_you_use/pubspec.yaml b/test_data/integration/always_depend_on_packages_you_use/pubspec.yaml new file mode 100644 index 000000000..d7f39ad4d --- /dev/null +++ b/test_data/integration/always_depend_on_packages_you_use/pubspec.yaml @@ -0,0 +1,5 @@ +name: sample_project +dependencies: + meta: any +dev_dependencies: + test: any diff --git a/test_data/integration/always_depend_on_packages_you_use/test/private.dart b/test_data/integration/always_depend_on_packages_you_use/test/private.dart new file mode 100644 index 000000000..c8dd4655f --- /dev/null +++ b/test_data/integration/always_depend_on_packages_you_use/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 From 8270e2edc7c88104be19ef2547e223c03119687a Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 19 May 2021 10:46:25 -0700 Subject: [PATCH 2/9] add entry to example/all.yaml --- example/all.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/example/all.yaml b/example/all.yaml index c196291d3..71e1780a3 100644 --- a/example/all.yaml +++ b/example/all.yaml @@ -3,6 +3,7 @@ linter: rules: - always_declare_return_types + - always_depend_on_packages_you_use - always_put_control_body_on_new_line - always_put_required_named_parameters_first - always_require_non_null_named_parameters From a6aae5c34adf4d615b8985a80b0f5fb68ef2ebfb Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 19 May 2021 11:43:49 -0700 Subject: [PATCH 3/9] code style updates --- lib/src/ast.dart | 4 +- .../always_depend_on_packages_you_use.dart | 43 ++++++++++--------- .../always_depend_on_packages_you_use.dart | 2 +- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/lib/src/ast.dart b/lib/src/ast.dart index e5c8bb1f7..50630bd1e 100644 --- a/lib/src/ast.dart +++ b/lib/src/ast.dart @@ -129,9 +129,11 @@ bool isInLibDir(CompilationUnit node, WorkspacePackage? package) { return path.isWithin(libDir, cuPath); } -/// Return true if this compilation unit [node] is declared within a public +/// 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; diff --git a/lib/src/rules/always_depend_on_packages_you_use.dart b/lib/src/rules/always_depend_on_packages_you_use.dart index 010c2b204..497c3a0aa 100644 --- a/lib/src/rules/always_depend_on_packages_you_use.dart +++ b/lib/src/rules/always_depend_on_packages_you_use.dart @@ -27,24 +27,20 @@ file. **BAD:** ```dart import 'package:a/a.dart'; -import 'package:b/b.dart'; ``` ```yaml dependencies: - a: ^1.0.0 ``` **GOOD:** ```dart import 'package:a/a.dart'; -import 'package:b/b.dart'; ``` ```yaml dependencies: a: ^1.0.0 - b: ^1.0.0 ``` '''; @@ -78,27 +74,32 @@ class _Visitor extends SimpleAstVisitor { final LinterContext context; final Pubspec pubspec; final bool isPublicFile; - late final availableDeps = [ - if (pubspec.dependencies != null) - for (var dep in pubspec.dependencies!) - if (dep.name?.text != null) dep.name!.text!, - if (!isPublicFile && pubspec.devDependencies != null) - for (var dep in pubspec.devDependencies!) - if (dep.name?.text != null) dep.name!.text! - ]; - - _Visitor(this.rule, this.context, this.pubspec, {required this.isPublicFile}); + late final List availableDeps; + + _Visitor(this.rule, this.context, this.pubspec, + {required this.isPublicFile}) { + var dependencies = pubspec.dependencies; + var devDependencies = pubspec.devDependencies; + availableDeps = [ + if (dependencies != null) + for (var dep in dependencies) + if (dep.name?.text != null) dep.name!.text!, + if (!isPublicFile && devDependencies != null) + for (var dep in devDependencies) + if (dep.name?.text != null) dep.name!.text! + ]; + } void _checkDirective(UriBasedDirective node) { - // Is it a package: import? - var importUriContent = node.uriContent; - if (importUriContent == null) return; - if (!importUriContent.startsWith('package:')) return; + // Is it a package: uri? + var uriContent = node.uriContent; + if (uriContent == null) return; + if (!uriContent.startsWith('package:')) return; try { - var importUri = Uri.parse(importUriContent); - if (importUri.pathSegments.isEmpty) return; - var packageName = importUri.pathSegments.first; + var uri = Uri.parse(uriContent); + if (uri.pathSegments.isEmpty) return; + var packageName = uri.pathSegments.first; if (availableDeps.contains(packageName)) return; rule.reportLint(node.uri); } on FormatException catch (_) {} diff --git a/test/integration/always_depend_on_packages_you_use.dart b/test/integration/always_depend_on_packages_you_use.dart index 0700eaf1c..643012dad 100644 --- a/test/integration/always_depend_on_packages_you_use.dart +++ b/test/integration/always_depend_on_packages_you_use.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// 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. From 3c6fc1bd0ffffd2b79dc21bc11a2d1e3607b59d6 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 19 May 2021 12:07:24 -0700 Subject: [PATCH 4/9] optimizations, dont use Uri.parse and hoist up the available deps calculation --- .../always_depend_on_packages_you_use.dart | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/src/rules/always_depend_on_packages_you_use.dart b/lib/src/rules/always_depend_on_packages_you_use.dart index 497c3a0aa..7cfaf8171 100644 --- a/lib/src/rules/always_depend_on_packages_you_use.dart +++ b/lib/src/rules/always_depend_on_packages_you_use.dart @@ -62,8 +62,19 @@ class AlwaysDependOnPackagesYouUse extends LintRule implements NodeLintRule { var pubspec = package.pubspec; if (pubspec == null) return; - var visitor = _Visitor(this, context, pubspec, - isPublicFile: isInPublicDir(context.currentUnit.unit, context.package)); + 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, context, availableDeps); registry.addImportDirective(this, visitor); registry.addExportDirective(this, visitor); } @@ -72,23 +83,9 @@ class AlwaysDependOnPackagesYouUse extends LintRule implements NodeLintRule { class _Visitor extends SimpleAstVisitor { final AlwaysDependOnPackagesYouUse rule; final LinterContext context; - final Pubspec pubspec; - final bool isPublicFile; - late final List availableDeps; + final List availableDeps; - _Visitor(this.rule, this.context, this.pubspec, - {required this.isPublicFile}) { - var dependencies = pubspec.dependencies; - var devDependencies = pubspec.devDependencies; - availableDeps = [ - if (dependencies != null) - for (var dep in dependencies) - if (dep.name?.text != null) dep.name!.text!, - if (!isPublicFile && devDependencies != null) - for (var dep in devDependencies) - if (dep.name?.text != null) dep.name!.text! - ]; - } + _Visitor(this.rule, this.context, this.availableDeps); void _checkDirective(UriBasedDirective node) { // Is it a package: uri? @@ -97,9 +94,8 @@ class _Visitor extends SimpleAstVisitor { if (!uriContent.startsWith('package:')) return; try { - var uri = Uri.parse(uriContent); - if (uri.pathSegments.isEmpty) return; - var packageName = uri.pathSegments.first; + var firstSlash = uriContent.indexOf('/'); + var packageName = uriContent.substring(8, firstSlash); if (availableDeps.contains(packageName)) return; rule.reportLint(node.uri); } on FormatException catch (_) {} From 8b640ebe23e6fff2f68f4742ee076e92c6d8e0ad Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 19 May 2021 12:17:12 -0700 Subject: [PATCH 5/9] clean up code a bit, handle the no slash case --- .../rules/always_depend_on_packages_you_use.dart | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/src/rules/always_depend_on_packages_you_use.dart b/lib/src/rules/always_depend_on_packages_you_use.dart index 7cfaf8171..5b748d7cc 100644 --- a/lib/src/rules/always_depend_on_packages_you_use.dart +++ b/lib/src/rules/always_depend_on_packages_you_use.dart @@ -93,12 +93,13 @@ class _Visitor extends SimpleAstVisitor { if (uriContent == null) return; if (!uriContent.startsWith('package:')) return; - try { - var firstSlash = uriContent.indexOf('/'); - var packageName = uriContent.substring(8, firstSlash); - if (availableDeps.contains(packageName)) return; - rule.reportLint(node.uri); - } on FormatException catch (_) {} + // 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 From afec512268c8491a04db9daa7fe98fa9f5bc7816 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 19 May 2021 12:32:45 -0700 Subject: [PATCH 6/9] remove unused context field --- lib/src/rules/always_depend_on_packages_you_use.dart | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/src/rules/always_depend_on_packages_you_use.dart b/lib/src/rules/always_depend_on_packages_you_use.dart index 5b748d7cc..ded73500c 100644 --- a/lib/src/rules/always_depend_on_packages_you_use.dart +++ b/lib/src/rules/always_depend_on_packages_you_use.dart @@ -74,7 +74,7 @@ class AlwaysDependOnPackagesYouUse extends LintRule implements NodeLintRule { if (dep.name?.text != null) dep.name!.text!, ]; - var visitor = _Visitor(this, context, availableDeps); + var visitor = _Visitor(this, availableDeps); registry.addImportDirective(this, visitor); registry.addExportDirective(this, visitor); } @@ -82,10 +82,9 @@ class AlwaysDependOnPackagesYouUse extends LintRule implements NodeLintRule { class _Visitor extends SimpleAstVisitor { final AlwaysDependOnPackagesYouUse rule; - final LinterContext context; final List availableDeps; - _Visitor(this.rule, this.context, this.availableDeps); + _Visitor(this.rule, this.availableDeps); void _checkDirective(UriBasedDirective node) { // Is it a package: uri? From 2e94c0f008c6bf35128f2f7383e14cc8db83ce0f Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 19 May 2021 13:15:52 -0700 Subject: [PATCH 7/9] rename to depend_on_referenced_packages --- example/all.yaml | 2 +- lib/src/rules.dart | 4 +-- ...art => depend_on_referenced_packages.dart} | 24 ++++++------- ...art => depend_on_referenced_packages.dart} | 34 +++++++++---------- test/integration_test.dart | 6 ++-- .../bin/main.dart | 0 .../lib/public.dart | 0 .../pubspec.yaml | 0 .../test/private.dart | 0 9 files changed, 35 insertions(+), 35 deletions(-) rename lib/src/rules/{always_depend_on_packages_you_use.dart => depend_on_referenced_packages.dart} (79%) rename test/integration/{always_depend_on_packages_you_use.dart => depend_on_referenced_packages.dart} (72%) rename test_data/integration/{always_depend_on_packages_you_use => depend_on_referenced_packages}/bin/main.dart (100%) rename test_data/integration/{always_depend_on_packages_you_use => depend_on_referenced_packages}/lib/public.dart (100%) rename test_data/integration/{always_depend_on_packages_you_use => depend_on_referenced_packages}/pubspec.yaml (100%) rename test_data/integration/{always_depend_on_packages_you_use => depend_on_referenced_packages}/test/private.dart (100%) diff --git a/example/all.yaml b/example/all.yaml index 71e1780a3..8ae0a3a47 100644 --- a/example/all.yaml +++ b/example/all.yaml @@ -3,7 +3,6 @@ linter: rules: - always_declare_return_types - - always_depend_on_packages_you_use - always_put_control_body_on_new_line - always_put_required_named_parameters_first - always_require_non_null_named_parameters @@ -60,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 diff --git a/lib/src/rules.dart b/lib/src/rules.dart index ada1be63c..1d8fc8fc0 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -4,7 +4,7 @@ import 'analyzer.dart'; import 'rules/always_declare_return_types.dart'; -import 'rules/always_depend_on_packages_you_use.dart'; +import 'rules/depend_on_referenced_packages.dart'; import 'rules/always_put_control_body_on_new_line.dart'; import 'rules/always_put_required_named_parameters_first.dart'; import 'rules/always_require_non_null_named_parameters.dart'; @@ -201,7 +201,6 @@ void registerLintRules({bool inTestMode = false}) { Analyzer.facade.cacheLinterVersion(); Analyzer.facade ..register(AlwaysDeclareReturnTypes()) - ..register(AlwaysDependOnPackagesYouUse()) ..register(AlwaysPutControlBodyOnNewLine()) ..register(AlwaysPutRequiredNamedParametersFirst()) ..register(AlwaysRequireNonNullNamedParameters()) @@ -259,6 +258,7 @@ void registerLintRules({bool inTestMode = false}) { ..register(ConstantIdentifierNames()) ..register(ControlFlowInFinally()) ..register(CurlyBracesInFlowControlStructures()) + ..register(DependOnReferencedPackages()) ..register(DeprecatedConsistency()) ..register(DiagnosticsDescribeAllProperties()) ..register(DirectivesOrdering()) diff --git a/lib/src/rules/always_depend_on_packages_you_use.dart b/lib/src/rules/depend_on_referenced_packages.dart similarity index 79% rename from lib/src/rules/always_depend_on_packages_you_use.dart rename to lib/src/rules/depend_on_referenced_packages.dart index ded73500c..71cc82f81 100644 --- a/lib/src/rules/always_depend_on_packages_you_use.dart +++ b/lib/src/rules/depend_on_referenced_packages.dart @@ -8,21 +8,21 @@ import 'package:analyzer/dart/ast/visitor.dart'; import '../analyzer.dart'; import '../ast.dart'; -const _desc = r'Depend on packages you use.'; +const _desc = r'Depend on referenced packages.'; const _details = r''' -**DO** depend on packages you use. +**DO** Depend on referenced packages. -When importing a package, *always* add a dependency on it to your pubspec. +When importing a package, add a dependency on it to your pubspec. -Depending explicitly on packages that you use ensures they will always exist -and allows you to put a dependency constraint on them to guard you against -breaking changes. +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 imported from a public file (one under either `lib` or `bin`), or some other -file. +is referenced from a public file (one under either `lib` or `bin`), or some +other private file. **BAD:** ```dart @@ -45,10 +45,10 @@ dependencies: '''; -class AlwaysDependOnPackagesYouUse extends LintRule implements NodeLintRule { - AlwaysDependOnPackagesYouUse() +class DependOnReferencedPackages extends LintRule implements NodeLintRule { + DependOnReferencedPackages() : super( - name: 'always_depend_on_packages_you_use', + name: 'depend_on_referenced_packages', description: _desc, details: _details, group: Group.pub); @@ -81,7 +81,7 @@ class AlwaysDependOnPackagesYouUse extends LintRule implements NodeLintRule { } class _Visitor extends SimpleAstVisitor { - final AlwaysDependOnPackagesYouUse rule; + final DependOnReferencedPackages rule; final List availableDeps; _Visitor(this.rule, this.availableDeps); diff --git a/test/integration/always_depend_on_packages_you_use.dart b/test/integration/depend_on_referenced_packages.dart similarity index 72% rename from test/integration/always_depend_on_packages_you_use.dart rename to test/integration/depend_on_referenced_packages.dart index 643012dad..2fb8c7756 100644 --- a/test/integration/always_depend_on_packages_you_use.dart +++ b/test/integration/depend_on_referenced_packages.dart @@ -12,7 +12,7 @@ import '../mocks.dart'; import '../test_constants.dart'; void main() { - group('always depend on packages you use', () { + group('always Depend on referenced packages', () { var currentOut = outSink; var collectingOut = CollectingSink(); setUp(() { @@ -30,19 +30,19 @@ void main() { await cli.run([ '--packages', packagesFilePath, - '$integrationTestDir/always_depend_on_packages_you_use/bin', - '--rules=always_depend_on_packages_you_use' + '$integrationTestDir/depend_on_referenced_packages/bin', + '--rules=depend_on_referenced_packages' ]); expect( collectingOut.trim(), stringContainsInOrder([ - "Depend on packages you use.", + "Depend on referenced packages.", "import 'package:test/test.dart'; // LINT", - "Depend on packages you use.", + "Depend on referenced packages.", "import 'package:matcher/matcher.dart'; // LINT", - "Depend on packages you use.", + "Depend on referenced packages.", "export 'package:test/test.dart'; // LINT", - "Depend on packages you use.", + "Depend on referenced packages.", "export 'package:matcher/matcher.dart'; // LINT", ])); expect(exitCode, 1); @@ -53,19 +53,19 @@ void main() { await cli.run([ '--packages', packagesFilePath, - '$integrationTestDir/always_depend_on_packages_you_use/lib', - '--rules=always_depend_on_packages_you_use' + '$integrationTestDir/depend_on_referenced_packages/lib', + '--rules=depend_on_referenced_packages' ]); expect( collectingOut.trim(), stringContainsInOrder([ - "Depend on packages you use.", + "Depend on referenced packages.", "import 'package:test/test.dart'; // LINT", - "Depend on packages you use.", + "Depend on referenced packages.", "import 'package:matcher/matcher.dart'; // LINT", - "Depend on packages you use.", + "Depend on referenced packages.", "export 'package:test/test.dart'; // LINT", - "Depend on packages you use.", + "Depend on referenced packages.", "export 'package:matcher/matcher.dart'; // LINT", ])); expect(exitCode, 1); @@ -76,15 +76,15 @@ void main() { await cli.run([ '--packages', packagesFilePath, - '$integrationTestDir/always_depend_on_packages_you_use/test', - '--rules=always_depend_on_packages_you_use' + '$integrationTestDir/depend_on_referenced_packages/test', + '--rules=depend_on_referenced_packages' ]); expect( collectingOut.trim(), stringContainsInOrder([ - "Depend on packages you use.", + "Depend on referenced packages.", "import 'package:matcher/matcher.dart'; // LINT", - "Depend on packages you use.", + "Depend on referenced packages.", "export 'package:matcher/matcher.dart'; // LINT", ])); expect(exitCode, 1); diff --git a/test/integration_test.dart b/test/integration_test.dart index 0d92995cd..a03e2cd16 100644 --- a/test/integration_test.dart +++ b/test/integration_test.dart @@ -13,8 +13,8 @@ import 'package:test/test.dart'; import 'package:yaml/yaml.dart'; import '../test_data/rules/experiments/experiments.dart'; -import 'integration/always_depend_on_packages_you_use.dart' - as always_depend_on_packages_you_use; +import 'integration/depend_on_referenced_packages.dart' + as depend_on_referenced_packages; import 'integration/always_require_non_null_named_parameters.dart' as always_require_non_null_named_parameters; import 'integration/avoid_private_typedef_functions.dart' @@ -192,7 +192,7 @@ void ruleTests() { flutter_style_todos.main(); lines_longer_than_80_chars.main(); only_throw_errors.main(); - always_depend_on_packages_you_use.main(); + depend_on_referenced_packages.main(); always_require_non_null_named_parameters.main(); prefer_asserts_in_initializer_lists.main(); prefer_const_constructors_in_immutables.main(); diff --git a/test_data/integration/always_depend_on_packages_you_use/bin/main.dart b/test_data/integration/depend_on_referenced_packages/bin/main.dart similarity index 100% rename from test_data/integration/always_depend_on_packages_you_use/bin/main.dart rename to test_data/integration/depend_on_referenced_packages/bin/main.dart diff --git a/test_data/integration/always_depend_on_packages_you_use/lib/public.dart b/test_data/integration/depend_on_referenced_packages/lib/public.dart similarity index 100% rename from test_data/integration/always_depend_on_packages_you_use/lib/public.dart rename to test_data/integration/depend_on_referenced_packages/lib/public.dart diff --git a/test_data/integration/always_depend_on_packages_you_use/pubspec.yaml b/test_data/integration/depend_on_referenced_packages/pubspec.yaml similarity index 100% rename from test_data/integration/always_depend_on_packages_you_use/pubspec.yaml rename to test_data/integration/depend_on_referenced_packages/pubspec.yaml diff --git a/test_data/integration/always_depend_on_packages_you_use/test/private.dart b/test_data/integration/depend_on_referenced_packages/test/private.dart similarity index 100% rename from test_data/integration/always_depend_on_packages_you_use/test/private.dart rename to test_data/integration/depend_on_referenced_packages/test/private.dart From 4563c70c0bfce6f42c27e308d006567246cbf0e6 Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 19 May 2021 13:24:19 -0700 Subject: [PATCH 8/9] a few more bits of cleanup from the rename --- test/integration/depend_on_referenced_packages.dart | 2 +- test/integration_test.dart | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/depend_on_referenced_packages.dart b/test/integration/depend_on_referenced_packages.dart index 2fb8c7756..357a4959e 100644 --- a/test/integration/depend_on_referenced_packages.dart +++ b/test/integration/depend_on_referenced_packages.dart @@ -12,7 +12,7 @@ import '../mocks.dart'; import '../test_constants.dart'; void main() { - group('always Depend on referenced packages', () { + group('Depend on referenced packages', () { var currentOut = outSink; var collectingOut = CollectingSink(); setUp(() { diff --git a/test/integration_test.dart b/test/integration_test.dart index a03e2cd16..4427d07bc 100644 --- a/test/integration_test.dart +++ b/test/integration_test.dart @@ -13,8 +13,6 @@ import 'package:test/test.dart'; import 'package:yaml/yaml.dart'; import '../test_data/rules/experiments/experiments.dart'; -import 'integration/depend_on_referenced_packages.dart' - as depend_on_referenced_packages; import 'integration/always_require_non_null_named_parameters.dart' as always_require_non_null_named_parameters; import 'integration/avoid_private_typedef_functions.dart' @@ -27,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; @@ -187,12 +187,12 @@ 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(); lines_longer_than_80_chars.main(); only_throw_errors.main(); - depend_on_referenced_packages.main(); always_require_non_null_named_parameters.main(); prefer_asserts_in_initializer_lists.main(); prefer_const_constructors_in_immutables.main(); From 58709d8db546f2a5fa3232387097be6f0a07500f Mon Sep 17 00:00:00 2001 From: Jacob MacDonald Date: Wed, 19 May 2021 13:36:31 -0700 Subject: [PATCH 9/9] organize imports --- lib/src/rules.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/rules.dart b/lib/src/rules.dart index 1d8fc8fc0..6111ad78f 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -4,7 +4,6 @@ import 'analyzer.dart'; import 'rules/always_declare_return_types.dart'; -import 'rules/depend_on_referenced_packages.dart'; import 'rules/always_put_control_body_on_new_line.dart'; import 'rules/always_put_required_named_parameters_first.dart'; import 'rules/always_require_non_null_named_parameters.dart'; @@ -62,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';