This repository was archived by the owner on Nov 20, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 167
add lint depend_on_referenced_packages #2659
Merged
pq
merged 9 commits into
dart-archive:master
from
jakemac53:always-depend-on-packages-you-use
May 19, 2021
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
438e206
add lint always_depend_on_packages_you_use
jakemac53 8270e2e
add entry to example/all.yaml
jakemac53 a6aae5c
code style updates
jakemac53 3c6fc1b
optimizations, dont use Uri.parse and hoist up the available deps cal…
jakemac53 8b640eb
clean up code a bit, handle the no slash case
jakemac53 afec512
remove unused context field
jakemac53 2e94c0f
rename to depend_on_referenced_packages
jakemac53 4563c70
a few more bits of cleanup from the rename
jakemac53 58709d8
organize imports
jakemac53 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
}); | ||
}); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 changes: 11 additions & 0 deletions
11
test_data/integration/depend_on_referenced_packages/bin/main.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
11 changes: 11 additions & 0 deletions
11
test_data/integration/depend_on_referenced_packages/lib/public.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
5 changes: 5 additions & 0 deletions
5
test_data/integration/depend_on_referenced_packages/pubspec.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
name: sample_project | ||
dependencies: | ||
meta: any | ||
dev_dependencies: | ||
test: any |
11 changes: 11 additions & 0 deletions
11
test_data/integration/depend_on_referenced_packages/test/private.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.