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.25k stars 1.58k forks source link

Analyzer: misleading message for factory constructor signature error #37048

Closed iarkh closed 4 years ago

iarkh commented 5 years ago

[Edit Jun 12th 2019: This issue is now concerned with the fact that the analyzer emits a somewhat misleading error message for the example below: It says 'assignable', but the reported error is actually about a required subtype relation. Cf. this comment. Edited the title correspondingly.]


Dart SDK Version: 2.3.1-dev.0.0 OS: Windows 10 64 bit

Here is a sample code example:

class A<X extends A<X>> {
  A() {}
  factory A.foo1() = C<Null>;
  factory A.foo2() = C<A<Null>>; 
  factory A.foo3() = C<A<A<Null>>>;
  factory A.foo4() = C<A<A<A<Null>>>>;
}

class C<X extends A<X>> extends A<X> {
  C() {}
}

main() {}

Both dart and analyzer throw compile errors for foo2, foo3, foo4 declaring . Both pass without error with foo1.

Seems like all four cases should not throw errors here because A<Null>(), A<A<Null>>(), A<A<A<Null>>>(), etc. are valid A's constructor calls.

Sample output is:

$> dartanalyzer test.dart Analyzing test.dart... error - The return type 'C<A>' of the redirected constructor isn't assignable to 'A' at test.dart:4:22 - redirect_to_invalid_return_type error - The return type 'C<A<A>>' of the redirected constructor isn't assignable to 'A' at test.dart:5:22 - redirect_to_invalid_return_type error - The return type 'C<A<A<A>>>' of the redirected constructor isn't assignable to 'A' at test.dart:6:22 - redirect_to_invalid_return_type 3 errors found.

$> dart test.dart test.dart:4:22: Error: The constructor function type 'C<A> Function()' isn't a subtype of 'A Function()'.

  • 'C' is from 'test.dart'.
  • 'A' is from 'test.dart'. factory A.foo2() = C<A>; ^ test.dart:5:22: Error: The constructor function type 'C<A<A>> Function()' isn't a subtype of 'A Function()'.
  • 'C' is from 'test.dart'.
  • 'A' is from 'test.dart'. factory A.foo3() = C<A<A>>; ^ test.dart:6:22: Error: The constructor function type 'C<A<A<A>>> Function()' isn't a subtype of 'A Function()'.
  • 'C' is from 'test.dart'.
  • 'A' is from 'test.dart'. factory A.foo4() = C<A<A<A>>>; ^
eernstg commented 5 years ago

No, the errors are actually correct. For example, we cannot let A<Null>.foo2() return a C<A<Null>>, because that is not a subtype of A<Null>. So the redirections correspond to valid constructor calls, but the resulting object doesn't have the type that we promise for construction of an A<T>.

However, it is misleading that the error message from the analyzer talks about 'assignable' because the actual requirement is that we have a subtype, and also that it omits the type Null from the error messages.

iarkh commented 5 years ago

If errors should be thrown here, shouldn't Specification be updated somehow?

It reads:

It is a compile-time error if a parameterized type T is super-bounded when it is used in any of the following ways: ... • T is an immediate subterm of a redirecting factory constructor signature (10.6.2).

It does not stay it clearly, but it can be supposed that using well bounded constructor (for example, C<A<Null>>()) should not throw a error here.

eernstg commented 5 years ago

The error arises for a different reason, specified here. That's the sentence where it is required that a redirection will produce something that has the proper type. A more obvious violation of the same sentence would be this example:

class A {}
class B {
  factory B() = A;
}

The problem is that B() evaluates to an instance of A, and that is not a subtype of B. It terms of the above mentioned sentence the problem is that A Function() is not a subtype of B Function(), and the reason for looking at the whole function type (rather than just the return type) is that we also need to complain if the parameter list shapes or parameter types do not match up.

jensjoha commented 5 years ago

@eernstg does that mean this issue can be closed?

eernstg commented 5 years ago

@jensjoha wrote:

does that mean this issue can be closed?

I just retried the example and noticed that the common front end (dart, 92dabc8491e767839fe6269e62f663b1ebb56484) reports the subtype issue that I mentioned, so I believe that there is no CFE issue here.

For the analyzer, it is confusing that the error message talks about 'assignable' even though the specified requirement is a 'subtype match' for the formal parameter list. But the reported error is concerned with the correct types (e.g., C<A<Null>> vs. A<X> for foo2), and it is raised in those 3 cases where there is no subtype match (and not in the fourth case where we do have the subtype match).

So we can remove 'area-front-end' and rename the issue to indicate that it's about a slightly misleading error message, not about raising compile-time errors that it shouldn't or vice versa.