Turn on and fix mixin type inference checks when not in "supermixin" …

…mode.

Now that we have an explicit syntax for mixins that is available all
the time, we have to do mixin inference error checking all the time,
not just when the "--supermixin" flag is supplied.

This required fixing several minor bugs:

- ErrorVerifier._checkForMixinSuperInvokedMembers did not properly
  handle a mixinElement argument that was a ClassElementHandle.

- ErrorVerifier._checkMixinInference wasn't using the actual
  substituted mixin types, causing errors to be wrongly reported when
  a class declaration had multiple inferred mixins (this was the root
  cause of #34404).

- Type names in "with" clauses in the resolved AST weren't properly
  reflecting the mixin type arguments that had been inferred.

Fixes #34404.

Change-Id: Ia773233c66f8d9ab778f207689c73922e9e3a880
Reviewed-on: https://dart-review.googlesource.com/75792
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>

pkg/analyzer/lib/src/generated/error_verifier.dart

@@ -16,6 +16,7 @@ import 'package:analyzer/error/error.dart';
 import 'package:analyzer/error/listener.dart';
 import 'package:analyzer/src/dart/ast/ast.dart';
 import 'package:analyzer/src/dart/element/element.dart';
+import 'package:analyzer/src/dart/element/handle.dart';
 import 'package:analyzer/src/dart/element/member.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/dart/resolver/inheritance_manager.dart';
@@ -4273,9 +4274,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
   /// [mixinIndex] in the list of mixins of [_enclosingClass] has concrete
   /// implementations of all the super-invoked members of the [mixinElement].
   bool _checkForMixinSuperInvokedMembers(
-      int mixinIndex, TypeName mixinName, ClassElementImpl mixinElement) {
+      int mixinIndex, TypeName mixinName, ClassElement mixinElement) {
+    ClassElementImpl mixinElementImpl = mixinElement is ClassElementHandle
+        ? mixinElement.actualElement
+        : mixinElement;
     InterfaceTypeImpl enclosingType = _enclosingClass.type;
-    for (var name in mixinElement.superInvokedNames) {
+    for (var name in mixinElementImpl.superInvokedNames) {
       var superMember = enclosingType.lookUpInheritedMember(
           name, _currentLibrary,
           concrete: true,
@@ -5801,7 +5805,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
 
   void _checkMixinInference(
       NamedCompilationUnitMember node, WithClause withClause) {
-    if (withClause == null || !_options.enableSuperMixins) {
+    if (withClause == null) {
       return;
     }
     ClassElement classElement = node.declaredElement;
@@ -5811,7 +5815,8 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     ClassElementImpl.collectAllSupertypes(
         supertypesForMixinInference, supertype, type);
     for (var typeName in withClause.mixinTypes) {
-      var mixinElement = typeName.name.staticElement;
+      var mixinType = typeName.type;
+      var mixinElement = mixinType.element;
       if (mixinElement is ClassElement) {
         if (typeName.typeArguments == null) {
           var mixinSupertypeConstraints =
@@ -5840,7 +5845,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
           }
         }
         ClassElementImpl.collectAllSupertypes(
-            supertypesForMixinInference, mixinElement.type, type);
+            supertypesForMixinInference, mixinType, type);
       }
     }
   }

pkg/analyzer/lib/src/generated/resolver.dart

@@ -7546,10 +7546,18 @@ class TypeNameResolver {
   final Source source;
   final AnalysisErrorListener errorListener;
 
+  /// Indicates whether bare typenames in "with" clauses should have their type
+  /// inferred type arguments loaded from the element model.
+  ///
+  /// This is needed for mixin type inference, but is incompatible with the old
+  /// task model.
+  final bool shouldUseWithClauseInferredTypes;
+
   Scope nameScope;
 
   TypeNameResolver(this.typeSystem, TypeProvider typeProvider,
-      this.definingLibrary, this.source, this.errorListener)
+      this.definingLibrary, this.source, this.errorListener,
+      {this.shouldUseWithClauseInferredTypes: true})
       : dynamicType = typeProvider.dynamicType,
         undefinedType = typeProvider.undefinedType;
 
@@ -7831,8 +7839,35 @@ class TypeNameResolver {
         }
       }
     }
-    typeName.staticType = type;
-    node.type = type;
+    DartType refinedType;
+    if (shouldUseWithClauseInferredTypes) {
+      var parent = node.parent;
+      if (parent is WithClause &&
+          type is InterfaceType &&
+          type.element.typeParameters.isNotEmpty) {
+        // Get the (possibly inferred) mixin type from the element model.
+        var grandParent = parent.parent;
+        if (grandParent is ClassDeclaration) {
+          refinedType =
+              _getInferredMixinType(grandParent.declaredElement, type.element);
+        } else if (grandParent is ClassTypeAlias) {
+          refinedType =
+              _getInferredMixinType(grandParent.declaredElement, type.element);
+        } else {
+          assert(false, 'Unexpected context for "with" clause');
+        }
+      }
+    }
+    refinedType ??= type;
+    typeName.staticType = refinedType;
+    node.type = refinedType;
+  }
+
+  DartType _getInferredMixinType(
+      ClassElement classElement, ClassElement mixinElement) {
+    for (var candidateMixin in classElement.mixins) {
+      if (candidateMixin.element == mixinElement) return candidateMixin;
+    }
   }
 
   /// The number of type arguments in the given [typeName] does not match the
@@ -8863,14 +8898,17 @@ class TypeResolverVisitor extends ScopedVisitor {
   /// created based on [definingLibrary] and [typeProvider].
   TypeResolverVisitor(LibraryElement definingLibrary, Source source,
       TypeProvider typeProvider, AnalysisErrorListener errorListener,
-      {Scope nameScope, this.mode: TypeResolverMode.everything})
+      {Scope nameScope,
+      this.mode: TypeResolverMode.everything,
+      bool shouldUseWithClauseInferredTypes: true})
       : super(definingLibrary, source, typeProvider, errorListener,
             nameScope: nameScope) {
     _dynamicType = typeProvider.dynamicType;
     _undefinedType = typeProvider.undefinedType;
     _typeSystem = TypeSystem.create(definingLibrary.context);
     _typeNameResolver = new TypeNameResolver(
-        _typeSystem, typeProvider, definingLibrary, source, errorListener);
+        _typeSystem, typeProvider, definingLibrary, source, errorListener,
+        shouldUseWithClauseInferredTypes: shouldUseWithClauseInferredTypes);
   }
 
   @override

pkg/analyzer/lib/src/task/dart.dart

@@ -5104,7 +5104,8 @@ class ResolveUnitTypeNamesTask extends SourceBasedAnalysisTask {
     //
     RecordingErrorListener errorListener = new RecordingErrorListener();
     TypeResolverVisitor visitor = new TypeResolverVisitor(
-        library, unitElement.source, typeProvider, errorListener);
+        library, unitElement.source, typeProvider, errorListener,
+        shouldUseWithClauseInferredTypes: false);
     unit.accept(visitor);
     //
     // Re-write the AST to handle the optional new and const feature.

pkg/analyzer/test/generated/non_error_resolver_test.dart

@@ -74,6 +74,12 @@ class NonErrorResolverTest extends NonErrorResolverTestBase {
     return super.test_mixinInference_with_actual_mixins();
   }
 
+  @override
+  @failingTest // Does not work with old task model
+  test_mixinInference_with_actual_mixins_supermixins_enabled() {
+    return super.test_mixinInference_with_actual_mixins_supermixins_enabled();
+  }
+
   @override
   @failingTest
   test_null_callMethod() {
@@ -3887,6 +3893,35 @@ mixin M1<T> on I<T> {
 
 class A = I<int> with M0, M1;
 
+void main () {
+  var x = new A().foo();
+}
+''');
+    var result = await computeAnalysisResult(source);
+    assertNoErrors(source);
+    verify([source]);
+    var main = result.unit.declarations.last as FunctionDeclaration;
+    var mainBody = main.functionExpression.body as BlockFunctionBody;
+    var xDecl = mainBody.block.statements[0] as VariableDeclarationStatement;
+    var xElem = xDecl.variables.variables[0].declaredElement;
+    expect(xElem.type.toString(), 'int');
+  }
+
+  test_mixinInference_with_actual_mixins_supermixins_enabled() async {
+    AnalysisOptionsImpl options = new AnalysisOptionsImpl();
+    options.enableSuperMixins = true;
+    resetWith(options: options);
+    Source source = addSource('''
+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 () {
   var x = new A().foo();
 }

tests/language_2/issue34404_flutter_modified_test.dart

@@ -0,0 +1,33 @@
+// Copyright (c) 2018, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// This test case is a reduction of some Flutter code, modified to use the new
+// mixin syntax.  We wish to verify that the class _DismissibleState doesn't
+// have any type inference errors.
+
+class _DismissibleState extends State<Dismissible>
+    with TickerProviderStateMixin, AutomaticKeepAliveClientMixin {}
+
+abstract class State<T extends StatefulWidget> extends Diagnosticable {}
+
+abstract class StatefulWidget extends Widget {}
+
+abstract class Widget extends DiagnosticableTree {}
+
+abstract class DiagnosticableTree extends Diagnosticable {}
+
+abstract class Diagnosticable {}
+
+class Dismissible extends StatefulWidget {}
+
+mixin TickerProviderStateMixin<T extends StatefulWidget> on State<T>
+    implements TickerProvider {}
+
+abstract class TickerProvider {}
+
+mixin AutomaticKeepAliveClientMixin<T extends StatefulWidget> on State<T> {}
+
+main() {
+  new _DismissibleState();
+}

tests/language_2/issue34404_flutter_test.dart

@@ -0,0 +1,34 @@
+// Copyright (c) 2018, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+// SharedOptions=--supermixin
+
+// This test case is a reduction of some Flutter code.  We wish to verify that
+// the class _DismissibleState doesn't have any type inference errors.
+
+class _DismissibleState extends State<Dismissible>
+    with TickerProviderStateMixin, AutomaticKeepAliveClientMixin {}
+
+abstract class State<T extends StatefulWidget> extends Diagnosticable {}
+
+abstract class StatefulWidget extends Widget {}
+
+abstract class Widget extends DiagnosticableTree {}
+
+abstract class DiagnosticableTree extends Diagnosticable {}
+
+abstract class Diagnosticable {}
+
+class Dismissible extends StatefulWidget {}
+
+abstract class TickerProviderStateMixin<T extends StatefulWidget>
+    extends State<T> implements TickerProvider {}
+
+abstract class TickerProvider {}
+
+abstract class AutomaticKeepAliveClientMixin<T extends StatefulWidget>
+    extends State<T> {}
+
+main() {
+  new _DismissibleState();
+}

tests/language_2/language_2_analyzer.status

@@ -75,6 +75,8 @@ malformed2_test: Pass, MissingCompileTimeError # Flaky: issue 31056.
 mixin_declaration/mixin_declaration_inference_invalid_07_test: MissingCompileTimeError
 mixin_declaration/mixin_declaration_invalid_superinvocation_test/10: CompileTimeError # Issue 30552
 mixin_method_override_test/01: MissingCompileTimeError
+mixin_mixin6_test: CompileTimeError # TODO(paulberry): triage
+mixin_mixin7_test: CompileTimeError # TODO(paulberry): triage
 mixin_super_2_test/01: MissingCompileTimeError
 mixin_super_2_test/03: MissingCompileTimeError
 mixin_supertype_subclass2_test/02: MissingStaticWarning # Issue 25614

tests/language_2/language_2_dart2js.status

@@ -45,6 +45,7 @@ int64_literal_test/40: CompileTimeError, OK # Error if web int literal cannot be
 int64_literal_test/none: CompileTimeError, OK # Error if web int literal cannot be represented exactly, see http://dartbug.com/33351
 issue23244_test: RuntimeError # Isolates - enum canonicalization - Issue 23244
 issue32353_test: CompileTimeError
+issue34404_flutter_test: CompileTimeError # --supermixin not supported
 library_env_test/has_mirror_support: RuntimeError, OK
 library_env_test/has_no_html_support: RuntimeError, OK
 list_test: CompileTimeError, OK # Error if web int literal cannot be represented exactly, see http://dartbug.com/33351

tests/language_2/language_2_dartdevc.status

@@ -101,6 +101,8 @@ issue31596_super_test/none: CompileTimeError
 issue31596_tearoff_test: CompileTimeError
 issue31596_test: CompileTimeError
 issue32353_test: RuntimeError
+issue34404_flutter_modified_test: CompileTimeError # DDC doesn't support mixin inference
+issue34404_flutter_test: CompileTimeError # DDC doesn't support mixin inference
 issue34498_test: MissingCompileTimeError # Issue 34500
 label_test: RuntimeError
 left_shift_test: RuntimeError # Ints and doubles are unified.
@@ -143,6 +145,8 @@ mixin_declaration/mixin_declaration_inference_valid_C12_test: CompileTimeError #
 mixin_declaration/mixin_declaration_inference_valid_C13_test: CompileTimeError # Issue #34164
 mixin_declaration/mixin_declaration_invalid_superinvocation_test/10: CompileTimeError    # Analyzer chooses wrong(?) super method.
 mixin_method_override_test/01: MissingCompileTimeError # Issue 34235
+mixin_mixin6_test: CompileTimeError # TODO(paulberry): triage
+mixin_mixin7_test: CompileTimeError # TODO(paulberry): triage
 mixin_super_2_test/01: MissingCompileTimeError
 mixin_super_2_test/03: MissingCompileTimeError
 mixin_super_test: RuntimeError