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

lint request: ambiguous imports which include an element from the Dart SDK #58326

Open
srawlins opened this issue Feb 19, 2021 · 6 comments
Open
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@srawlins
Copy link
Member

srawlins commented Feb 19, 2021

Analyzer has a diagnostic for when two imports provide an element of the same name:

// a.dart ////////////////////
class File {}

// b.dart ////////////////////
class File {}

// c.dart ////////////////////
import 'a.dart';
import 'b.dart';

main() {
  File(); // error: ambiguous_import
}

But this error does not appear when a dart: element is at play.

// a.dart ////////////////////
class File {}

// c.dart ////////////////////
import 'a.dart';
import 'dart:io';

main() {
  File(); // MISSING ERROR
}

Flutter is hitting this in a few places, like https://github.com/flutter/flutter/blob/master/packages/flutter_goldens/test/flutter_goldens_test.dart with "package:file/file.dart" and "dart:io". It does not cause runtime problems, but I think is still confusing.

@srawlins
Copy link
Member Author

Maybe I'm missing something about the rules of ambiguous imports, because CFE matches analyzer's behavior. The first example results in this error:

c.dart:7:3: Error: 'File' is imported from both 'a.dart' and 'b.dart'.
  File();
  ^^^^

The second example results in no error.

@srawlins
Copy link
Member Author

@scheglov points out the exception is specified in the spec:

"a declaration from a non-system library shadows declarations from system libraries".

I imagine authors generally still want a check here though, so converting this to a lint request.

@srawlins srawlins transferred this issue from dart-lang/sdk Feb 19, 2021
@srawlins srawlins changed the title analyzer sometimes fails to report ambiguous_import lint request: ambiguous imports which include an element from the Dart SDK Feb 19, 2021
@bwilkerson
Copy link
Member

The exception was added so that the core libraries could be extended without breaking previously existing code.

Given that, why do you think users would want this lint? What broken code will this help to catch?

@srawlins
Copy link
Member Author

I think most users are unaware of this singular exception to library element name collisions. There are libraries in flutter's codebase which import both dart:io and package:file/file.dart, and reference File. I believe most developers wouldn't know which File is being used in such a library.

If I had files like that in my code, I would want a lint to let me know, so that I could hide names that are being shadowed.

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Mar 29, 2024
@mateusfccp
Copy link
Contributor

I agree with this. It's unexpected for me that it's not an error, but after the explanation of why this is this way, I can understand why.

Even so, having an opt-in would be interesting, as, most of the time, shadowing a core identifier is done without intention, and may be overlooked.

@devoncarew devoncarew added analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 19, 2024
@pq pq added the P3 A lower priority bug or feature request label Nov 20, 2024
@FMorschel
Copy link
Contributor

FMorschel commented Nov 22, 2024

I think most users are unaware of this singular exception to library element name collisions.

Agreed, happened to me. Found this while working on #56830.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants