Analyzer issues incorrect warning on inconsistent interfaces in mixin inference

[leafpetersen]

The analyzer issues the following warnings on the code below:

  error • Type parameters could not be inferred for the mixin 'M1' because the base class implements the mixin's supertype constraint 'I<T>' in multiple conflicting ways at /Users/leafp/tmp/ddctest.dart:10:33 • mixin_inference_inconsistent_matching_classes
  error • A value of type 'int' can't be assigned to a variable of type 'String' at /Users/leafp/tmp/ddctest.dart:13:14 • invalid_assignment

Note that the second (expected) error does imply that the mixin inference is succeeding and inferring int as the type argument, but nonetheless the first (unexpected) error is emitted as if inference had failed.

class I<X> {}

class M0<T> extends I<T> {}

class M1<T> extends I<T> {
  T foo() => null;
}

// M1 is inferred as M1<int>
class A extends I<int> with M0, M1 {}

void main () {
  String s = new A().foo();
}

[leafpetersen]

cc @bwilkerson @stereotype441 @yjbanov

This doesn't need to be fixed for the old super-mixin inference, but we need to be sure this works correctly with the new syntax since flutter uses this pattern. I'll try to get some tests landed for this.

[jmesserly]

FYI, this may have been fixed(?). Not sure though. The errors I see now are:

  error • The class 'M0' can't be used as a mixin because it extends a class other than Object at test.dart:10:29 • mixin_inherits_from_not_object
  error • The class 'M1' can't be used as a mixin because it extends a class other than Object at test.dart:10:33 • mixin_inherits_from_not_object
  error • A value of type 'int' can't be assigned to a variable of type 'String' at test.dart:13:14 • invalid_assignment

[jmesserly]

if I change extends to implements:

class I<X> {}
class M0<T> implements I<T> {}
class M1<T> implements I<T> {
  T foo() => null;
}
// M1 is inferred as M1<int>
class A extends I<int> with M0, M1 {}

void main () {
  String s = new A().foo();
}

I get this error:

  error • The class 'A' cannot implement both 'I<int>' and 'I<dynamic>' because the type arguments are different at test.dart:10:1 • conflicting_generic_interfaces

That makes me think the inference may not be working correctly.

[bwilkerson]

@stereotype441

[scheglov]

This problem exists even without mixins.

class I<T> {}

class A<T> implements I<T> {}

class B implements I<int>, A {}

Analyzer reports error: The class 'B' cannot implement both 'I<int>' and 'I<dynamic>' because the type arguments are different. (conflicting_generic_interfaces at [test] bin/test2.dart:5).

@leafpetersen Could you please point me at the section in the specification that governs the expected behavior?

[leafpetersen]

There is no inference for implements clauses, so the errors are expected for the class example. For mixins, the proposal here includes a section on inference, specified here. I just landed tests for this here.

[stereotype441]

I'll look into this

[leafpetersen]

This is the relevant reproduction for the new mixin syntax:

class I<X> {}

mixin M0<T> on I<T> {}

mixin M1<T> on I<T> {
  T foo() => null;
}

class A = I<int> with M0, M1;

void main () {
  String s = new A().foo();
}

I still see the incorrect error for this example (and actually don't see the correct error).

I believe that this case is covered by the tests that I added.

[leafpetersen]

Note that this will be a blocker for migrating flutter. cc @JekCharlsonYu

[leafpetersen]

cc @yjbanov

[yjbanov]

Thanks for looking into it! Here a place in Flutter where we have to ignore an analyzer warning when inference fails.

[stereotype441]

A fix for the repro in #34404 (comment) is out for review: https://dart-review.googlesource.com/c/sdk/+/75625.

There are still a few of Leaf's test cases failing, so I'm going to leave this issue open until I've addressed them.

[stereotype441]

With 2e3f17f, the analyzer now produces the expected behavior for Leaf's example in #34404 (comment). But that same example fails when the analyzer is run with the "--supermixin" flag is passed in. The example at the top of this bug report also sill fails.

@yjbanov's Your example from Flutter is in a similar situation: if I modify it to use "mixin" syntax and analyze it without "--supermixin", it works. But if I analyze it with "--supermixin", it fails (regardless of whether old or new syntax is used).

So I think we are no longer blocking flutter's migration to the new syntax. And I'm mindful of Leaf's statement that this doesn't need to be fixed for the old super-mixin inference. But I'm bothered by the fact that the presence of the "--supermixin" flag mysteriously affects the new mixin syntax as well--that really shouldn't happen. So I'm going to continue investigating for a bit longer.

[stereotype441]

Aha, glad I checked. Supermixin error checking is still not working, but it looks like it's working because it's disabled when the "--supermixin" flag is missing. So this bug still blocks Flutter's migration to the new syntax. Working on a fix.

[stereotype441]

Candidate fix out for review: https://dart-review.googlesource.com/c/sdk/+/75792. This fixes and enables the error checking, and as a bonus, it fixes the test case in the original bug report (that uses the old "--supermixin" syntax).