dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.2k stars 1.57k forks source link

Incorrect error message in extension body that invokes constructor of type that isn't imported #51021

Open jonahwilliams opened 1 year ago

jonahwilliams commented 1 year ago

Dart SDK version: 3.0.0-112.0.dev (dev) (Wed Jan 11 02:23:31 2023 -0800) on "windows_x64"

If an extension method on a nullable type invokes a constructor of a type that is not imported, then the error message presented will be "unchecked_use_of_nullable_value" instead of "undefined_method". Making the extension on the non-nullable Bar shows the correct error message. Because Bar does not have a Foo method, I believe this is an incorrect error message.

main.dart

class Bar {}

extension on Bar? {
  Foo makeFoo() {
    // Incorrect diagnostic: unchecked_use_of_nullable_value
    return Foo();
  }
}

other.dart (not imported)

class Foo {}
srawlins commented 1 year ago

Is this when the code is compiled, or analyzed in the IDE?

jonahwilliams commented 1 year ago

analyzed

srawlins commented 1 year ago

OK that is the weirdest behavior, seen in dartpad, toggling between on Bar? and on Bar. CC @scheglov

I guess the issue is mostly with the text. with on Bar, the error says, "The method 'Foo' isn't defined for the type 'Bar'." but of course Foo() doesn't have to be defined on Bar, and pragmatically speaking it's clear to me as a human that the developer was going for a constructor. It's still weird with the on Bar? case that Foo hasn't just resolved to dynamic...

bwilkerson commented 1 year ago

I agree that the message is not helpful.

The analyzer has no way of knowing what Foo is intended to refer to, so it seems odd that it's assuming that it was intended to be a member of Bar. The message should make it clear that the only thing we really know is that Foo isn't defined.

jonahwilliams commented 1 year ago

Also, because the error is not undefined_method there is no quickfix available for importing the class, which is frustrating

lrhn commented 4 months ago

The language defines an unqualified identifier id that it's not defined in lexical scope to mean this.id. Then it's an error if this.id would be an error.

That explains the behavior, but it's not the best possible messaging for it.

If this.id it's also unrefined (not a member on the type of this, and also not an extension on it), it might be better to just say that id is not available, and treat id and this.id differently for error reporting.