Skip to content
This repository was archived by the owner on Nov 20, 2024. It is now read-only.

add lint depend_on_referenced_packages #2659

Merged
merged 9 commits into from
May 19, 2021
Merged

add lint depend_on_referenced_packages #2659

merged 9 commits into from
May 19, 2021

Conversation

@jakemac53
Copy link
Contributor

@jakemac53 jakemac53 commented May 19, 2021 •
edited
Loading

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 dart-lang/sdk#57175

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

coveralls commented May 19, 2021 •
edited
Loading

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
bwilkerson
bwilkerson approved these changes May 19, 2021
View reviewed changes
pq
pq reviewed May 19, 2021
View reviewed changes
example/all.yaml Outdated
linter:
rules:
- always_declare_return_types
- always_depend_on_packages_you_use
Copy link
Contributor

@pq pq May 19, 2021

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
Contributor

@pq pq May 19, 2021

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
Contributor

@bwilkerson bwilkerson May 19, 2021

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

@jakemac53 jakemac53 May 19, 2021

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

@jakemac53 jakemac53 May 19, 2021

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
Contributor

pq commented May 19, 2021 •
edited
Loading

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

EDIT: also a perf question.

pq
pq reviewed May 19, 2021
View reviewed changes
@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
Contributor

pq commented May 19, 2021 •
edited
Loading

depend_on_referenced_packages works for me. @bwilkerson: WDYT?

Never mind!

@pq
Copy link
Contributor

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!

@pq pq merged commit d1c0862 into dart-archive:master May 19, 2021
@DanTup DanTup mentioned this pull request Jul 21, 2021
Closed
@DanTup DanTup mentioned this pull request Nov 1, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@bwilkerson bwilkerson bwilkerson approved these changes

@pq pq pq approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Lint for used package not defined in pubspec

4 participants

@jakemac53 @coveralls @pq @bwilkerson