Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add lint depend_on_referenced_packages #2659

Merged
merged 9 commits into from
May 19, 2021

Conversation

jakemac53
Copy link
Contributor

@jakemac53 jakemac53 commented May 19, 2021

Description

Adds a new lint, depend_on_referenced_packages. This applies to all imports and exports.

For files under the lib or bin dir, only packages in the dependencies section may be referenced. For all other files any package listed in dev_dependencies is also allowed.

Fixes #29

@google-cla google-cla bot added the cla: yes label May 19, 2021
@coveralls
Copy link

coveralls commented May 19, 2021

Coverage Status

Coverage increased (+0.03%) to 93.955% when pulling 58709d8 on jakemac53:always-depend-on-packages-you-use into 989ea4e on dart-lang:master.

@jakemac53 jakemac53 requested a review from pq May 19, 2021 17:46
lib/src/ast.dart Outdated Show resolved Hide resolved
lib/src/ast.dart Show resolved Hide resolved
lib/src/rules/always_depend_on_packages_you_use.dart Outdated Show resolved Hide resolved
lib/src/rules/always_depend_on_packages_you_use.dart Outdated Show resolved Hide resolved
lib/src/rules/always_depend_on_packages_you_use.dart Outdated Show resolved Hide resolved
test/integration/always_depend_on_packages_you_use.dart Outdated Show resolved Hide resolved
example/all.yaml Outdated
@@ -3,6 +3,7 @@
linter:
rules:
- always_declare_return_types
- always_depend_on_packages_you_use
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general I think we're trending away from "always" and similar prefixes. Bike shedding names...

  • direct_package_dependencies

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explicit_package_dependencies

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or depend_on_referenced_packages

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with any of these, I like depend_on_referenced_packages probably the most?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with that name for now, LMK if you want to change it

@pq
Copy link
Member

pq commented May 19, 2021

So awesome. Modulo name bike-shedding, this looks great!

EDIT: also a perf question.

@jakemac53 jakemac53 changed the title add lint always_depend_on_packages_you_use add lint depend_on_referenced_packages May 19, 2021
@pq
Copy link
Member

pq commented May 19, 2021

depend_on_referenced_packages works for me. @bwilkerson: WDYT?

Never mind!

@pq
Copy link
Member

pq commented May 19, 2021

And hey, nice perf improvement!

https://github.com/dart-lang/linter/pull/2659/checks?check_run_id=2624270876#step:5:111

Looks like this is now down from 15ms in benchmarks to 2!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint for used package not defined in pubspec
4 participants