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.

Sign up for GitHub

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

Jump to bottom

implementation_imports linting rule only works in lib directory #4859

Open
akasha19 opened this issue Jan 31, 2024 · 3 comments
Open

implementation_imports linting rule only works in lib directory #4859

akasha19 opened this issue Jan 31, 2024 · 3 comments
Assignees
@srawlins
Labels
false-negative P2 A bug or feature request we're likely to work on set-recommended Affects a rule in the recommended Dart rule set type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@akasha19
Copy link

akasha19 commented Jan 31, 2024 •
edited
Loading

Describe the issue
The implementations_imports rule only checks files that are in the lib directory of a package. src imports in other directories like test or bin are not found so there is no warning.

To Reproduce

analysis_options.yaml

linter:
  rules:
    - implementation_imports

any file in directory other than lib

import 'package:foo/src/something.dart';

Expected behavior
As I don't understand why this rule would only be important in the lib directory and not other directories I'm assuming this is a bug.

Additional context
Came here from https://stackoverflow.com/questions/77909927/why-does-the-implementation-imports-linting-rule-only-work-in-lib-directory
@github-actions github-actions bot added the set-recommended Affects a rule in the recommended Dart rule set label Jan 31, 2024
@srawlins
Copy link
Member

srawlins commented Feb 6, 2024

Good catch. Big bug.

@srawlins srawlins added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) P2 A bug or feature request we're likely to work on false-negative labels Feb 6, 2024
@srawlins srawlins self-assigned this Feb 7, 2024
@srawlins
Copy link
Member

This will take a bit of work to fix. New violations in flutter/engine, flutter/flutter, flutter/packages, the Dart SDK.

@srawlins srawlins mentioned this issue Feb 16, 2024
Closed
8 tasks
This was referenced Feb 16, 2024
Merged
Merged
auto-submit bot pushed a commit to flutter/engine that referenced this issue Feb 16, 2024
@goderbauer
Fix implementation imports outside of lib (#50727)
3e230a8 
Work towards dart-lang/linter#4859

There are libraries outside a `lib/` directory, which violate `implementation_imports`.
auto-submit bot pushed a commit to flutter/flutter that referenced this issue Feb 16, 2024
@goderbauer
Fix implementation imports outside of lib (#143594)
546bdec 
Work towards dart-lang/linter#4859

There are libraries outside a `lib/` directory, which violate `implementation_imports`.

Supersedes #143560.
srawlins added a commit to flutter/packages that referenced this issue Feb 23, 2024
@srawlins
Work towards dart-lang/linter#4859
c48b2cd 
There are libraries outside a `lib/` directory, which violate
`implementation_imports`. The fix here is to always add an 'ignore' exception,
as the "implementation" being imported is the very package being exemplified.

(Maybe that's a perfect reason to re-examine the public API of the package; can
you create a good example / example test without importing the private
implementation libraries?)
@srawlins srawlins mentioned this issue Feb 23, 2024
Merged
11 tasks
srawlins added a commit to flutter/packages that referenced this issue May 7, 2024
@srawlins
[camera] Ignore implementation imports outside of lib (#6191)
8df8b5f 
Work towards dart-lang/linter#4859

There are libraries outside a `lib/` directory, which violate
`implementation_imports`. The fix here is to always add an 'ignore'
exception, as the "implementation" being imported is the very package
being exemplified.

(Maybe that's a perfect reason to re-examine the public API of the
package; can you create a good example / example test without importing
the private implementation libraries?)



## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [relevant style guides] and ran the
auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages
repo does use `dart format`.)
- [x] I signed the [CLA].
- [x] The title of the PR starts with the name of the package surrounded
by square brackets, e.g. `[shared_preferences]`
- [x] I [linked to at least one issue that this PR fixes] in the
description above.
- [x] I updated `pubspec.yaml` with an appropriate new version according
to the [pub versioning philosophy], or this PR is [exempt from version
changes].
- [ ] I updated `CHANGELOG.md` to add a description of the change,
[following repository CHANGELOG style].
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[relevant style guides]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
[linked to at least one issue that this PR fixes]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[pub versioning philosophy]: https://dart.dev/tools/pub/versioning
[exempt from version changes]:
https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#version-and-changelog-updates
[following repository CHANGELOG style]:
https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changelog-style
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this issue May 22, 2024
@srawlins @TecHaxter
[camera] Ignore implementation imports outside of lib (flutter#6191)
af59459 
Work towards dart-lang/linter#4859

There are libraries outside a `lib/` directory, which violate
`implementation_imports`. The fix here is to always add an 'ignore'
exception, as the "implementation" being imported is the very package
being exemplified.

(Maybe that's a perfect reason to re-examine the public API of the
package; can you create a good example / example test without importing
the private implementation libraries?)



## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [relevant style guides] and ran the
auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages
repo does use `dart format`.)
- [x] I signed the [CLA].
- [x] The title of the PR starts with the name of the package surrounded
by square brackets, e.g. `[shared_preferences]`
- [x] I [linked to at least one issue that this PR fixes] in the
description above.
- [x] I updated `pubspec.yaml` with an appropriate new version according
to the [pub versioning philosophy], or this PR is [exempt from version
changes].
- [ ] I updated `CHANGELOG.md` to add a description of the change,
[following repository CHANGELOG style].
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[relevant style guides]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
[linked to at least one issue that this PR fixes]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[pub versioning philosophy]: https://dart.dev/tools/pub/versioning
[exempt from version changes]:
https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#version-and-changelog-updates
[following repository CHANGELOG style]:
https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changelog-style
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
arc-yong pushed a commit to Arctuition/packages-arc that referenced this issue Jun 14, 2024
@srawlins @arc-yong
[camera] Ignore implementation imports outside of lib (flutter#6191)
86db083 
Work towards dart-lang/linter#4859

There are libraries outside a `lib/` directory, which violate
`implementation_imports`. The fix here is to always add an 'ignore'
exception, as the "implementation" being imported is the very package
being exemplified.

(Maybe that's a perfect reason to re-examine the public API of the
package; can you create a good example / example test without importing
the private implementation libraries?)



## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [relevant style guides] and ran the
auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages
repo does use `dart format`.)
- [x] I signed the [CLA].
- [x] The title of the PR starts with the name of the package surrounded
by square brackets, e.g. `[shared_preferences]`
- [x] I [linked to at least one issue that this PR fixes] in the
description above.
- [x] I updated `pubspec.yaml` with an appropriate new version according
to the [pub versioning philosophy], or this PR is [exempt from version
changes].
- [ ] I updated `CHANGELOG.md` to add a description of the change,
[following repository CHANGELOG style].
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[relevant style guides]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
[linked to at least one issue that this PR fixes]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[pub versioning philosophy]: https://dart.dev/tools/pub/versioning
[exempt from version changes]:
https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#version-and-changelog-updates
[following repository CHANGELOG style]:
https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changelog-style
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
@travissanderson-wf
Copy link

Agreed this is a big one, we use this to make sure changing package src structure and file names won't break consumers but if they're importing in tests it will break their CI. This lint only protects their builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@srawlins srawlins

Labels
false-negative P2 A bug or feature request we're likely to work on set-recommended Affects a rule in the recommended Dart rule set type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@srawlins @travissanderson-wf @akasha19