dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
627 stars 172 forks source link

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

Open srawlins opened 3 years ago

srawlins commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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.

bwilkerson commented 3 years ago

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 commented 3 years ago

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.