Skip to content

Lint for merging equal import/exports with different show combinators #59722

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

Open
FMorschel opened this issue Dec 16, 2024 · 8 comments
Open
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal P3 A lower priority bug or feature request

Comments

@FMorschel
Copy link
Contributor

FMorschel commented Dec 16, 2024

When you have duplicate import/exports like:

export 'lib1.dart' show A;
export 'lib1.dart' show B;

we could have a lint for merging them together.

Old description

Say you have a package (happened to mine - due_date). You have an API that should be exported separately outside the project in different libraries (one file with multiple declarations or a barrel file).

So you write down an export directive with show/hide. But later you add more things and you add another entry (forgot about the original one or didn't see it) with the same file but other show/hide values. This should be warned to make them a single export. Even more important if they are using the hide combinator since the other may show unwanted things.

export 'lib1.dart' show A;
export 'lib1.dart' show B;

export 'lib2.dart' hide C;
export 'lib2.dart' hide D;

Today if we have the exact same values for the combinators it shows:

Duplicate export. Try removing all but one export of the library. dart(duplicate_export)
@FMorschel FMorschel added the legacy-area-analyzer Use area-devexp instead. label Dec 16, 2024
@bwilkerson
Copy link
Member

This sounds to me like a style preference. There's nothing semantically wrong with having multiple exports of different parts of the same library. (Though I agree that the more concise form is closer to the Dart style.)

That makes it a lint rather than a warning, but we're essentially not adding any new lints at this time that aren't requested by the language team, typically in order to facilitate adoption of new language features. So I suspect that we won't want to add this one.

@bwilkerson bwilkerson added devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal P3 A lower priority bug or feature request labels Dec 16, 2024
@FMorschel
Copy link
Contributor Author

I guess this is somewhat the same as what was asked at #59907 from your comment on #56830, but for exports.

@bwilkerson
Copy link
Member

Yes, exports with hides have the same issue.

@FMorschel
Copy link
Contributor Author

FMorschel commented Jan 14, 2025

I've decided to leave #59907 as a request for handling the hide combinator warning.

Here I'm asking for an actual lint for merging equal imports/exports that differ show names.


@bwilkerson, do you think that if I have one import/export with a show combinator and another without any combinators at all like:

import 'b.dart';
import 'b.dart' show B; //does nothing since everything is shown above

We should add the warning here as a lint or at the current warnings for duplicate exports/imports? If you think it should be handled at the current warnings I can create a new issue for tracking that.


Edit

Just to point another similarly related issue: #56879

@FMorschel FMorschel changed the title Duplicate export should consider combinators Lint for merging import/exports with different show combinators Jan 14, 2025
@FMorschel FMorschel changed the title Lint for merging import/exports with different show combinators Lint for merging equal import/exports with different show combinators Jan 14, 2025
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
@bwilkerson
Copy link
Member

We already have an UNNECESSARY_IMPORT warning. It ought to be flagging the case from the previous comment. I wouldn't want to create two diagnostics for it.

Beyond that, I think we need to stop and think about a consistent way of handling all of the cases.

@FMorschel
Copy link
Contributor Author

We already have an UNNECESSARY_IMPORT warning. It ought to be flagging the case from the previous comment. I wouldn't want to create two diagnostics for it.

Image

It does nothing if you are using A. The comment above would actually fit under "redundant" imports (#44569).

@bwilkerson
Copy link
Member

True, "redundant" is probably the better diagnostic.

@FMorschel
Copy link
Contributor Author

I think this should be triggering on:

  • Two or more equal imports that show different elements
  • Two ore more equal exports that have any combinators (since it wouldn't be redundant imports)

It could be called something like diffuse_directives (better, I think), verbose_directives or something like that (English is not my first language so if you can think of a better adjective here it's completely fine).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal P3 A lower priority bug or feature request
Projects
None yet
Development

No branches or pull requests

2 participants