Skip to content

NNBD: Analyzer allows inherit two interfaces with incompatible type parameters: Object? vs FutureOr, Object? vs FutureOr<FutureOr> #40454

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

Closed
iarkh opened this issue Feb 4, 2020 · 4 comments
Assignees
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. NNBD Issues related to NNBD Release

Comments

@iarkh
Copy link
Contributor

iarkh commented Feb 4, 2020

Dart VM version: 2.8.0-edge.40f23c735f04433e4fc334fbd674474bd3de0f8b (Tue Jan 28 01:14:48 2020 +0000) on "linux_x64"

This issue is similar with #40453, but dart and analyzer behave in the different ways here.

The following source code declares classes X1 and X2 which inherit classes with incompatible type arguments:

import "dart:async";

class A<T>{}
class B implements A<Object?> {}

class C implements A<FutureOr> {}
class D implements A<FutureOr<FutureOr<FutureOr>>> {}

class X1 extends B implements C {}
class X2 extends B implements D {}

main() {}

This code throws two compile errors with dart and passes with analyzer.
Seems like error should be thrown in both cases.

Sample output is:

$ dart --enable-experiment=non-nullable test.dart
test.dart:9:7: Error: 'X1' can't implement both 'A<Object?>' and 'A<FutureOr>'

  • 'A' is from 'test.dart'.
  • 'Object' is from 'dart:core'.
  • 'FutureOr' is from 'dart:async'.
    class X1 extends B implements C {}
    ^
    test.dart:10:7: Error: 'X2' can't implement both 'A<Object?>' and 'A<FutureOr<FutureOr<FutureOr>>>'
  • 'A' is from 'test.dart'.
  • 'Object' is from 'dart:core'.
  • 'FutureOr' is from 'dart:async'.
    class X2 extends B implements D {}
    ^
    $ dartanalyzer --enable-experiment=non-nullable test.dart
    Analyzing test.dart...
    No issues found!
@lrhn
Copy link
Member

lrhn commented Feb 4, 2020

Reading the spec, I believe the analyzer is correct if the code is null-safe. (I'm not so sure if it's legacy code, though).

The check for having the same instantiation of an interface uses NORM and NNBD_TOP_MERGE:

If a class C in an opted-in library implements the same generic class I more than once as I0, .., In, and at least one of the Ii is not syntactically equal to the others, then it is an error if NNBD_TOP_MERGE(S0, ..., Sn) is not defined where Si is NORM(Ii). Otherwise, for the purposes of runtime subtyping checks, C is considered to implement the canonical interface given by NNBD_TOP_MERGE(S0, ..., Sn).

The type NORM(FutureOr) is dynamic, as is NORM(FutureOr<FutureOr<FutureOr>>).
The NNBD_TOP_MERGE(Object?, dynamic) is Object?, which means it's defined and there is no error.

@a-siva a-siva added legacy-area-front-end Legacy: Use area-dart-model instead. NNBD Issues related to NNBD Release labels Feb 4, 2020
@johnniwinther johnniwinther self-assigned this Feb 5, 2020
@iarkh
Copy link
Contributor Author

iarkh commented Feb 6, 2020

The same problem is reproducible for the dynamic vs FutureOr case.

@eernstg
Copy link
Member

eernstg commented Feb 7, 2020

Agreeing that there is no error here, note that dart-lang/language#824 aims to make an adjustment to the definition of NNBD_TOP_MERGE. This will not change the ability of an opted-it class to have superinterfaces that differ in the ways that NNBD_TOP_MERGE eliminates, but it will make them implement a superinterface that has type argument Object? or dynamic in some situations where the current rules would yield void.

@chloestefantsova
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. NNBD Issues related to NNBD Release
Projects
None yet
Development

No branches or pull requests

6 participants