fix #29426, class type alias was missing checks

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

Index: pkg/analyzer/lib/src/generated/error_verifier.dart
diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
index b9e67d69d6af065a387804904b1c4f0d9fb7734a..d4a99d45a6a97c05953889bbdaa7e6731166ba22 100644
--- a/pkg/analyzer/lib/src/generated/error_verifier.dart
+++ b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -463,33 +463,15 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
       _checkForMemberWithClassName();
       _checkForNoDefaultSuperConstructorImplicit(node);
       _checkForConflictingTypeVariableErrorCodes(node);
-      ExtendsClause extendsClause = node.extendsClause;
+      TypeName superclass = node.extendsClause?.superclass;
       ImplementsClause implementsClause = node.implementsClause;
       WithClause withClause = node.withClause;
+
       // Only do error checks on the clause nodes if there is a non-null clause
       if (implementsClause != null ||
-          extendsClause != null ||
+          superclass != null ||
           withClause != null) {
-        // Only check for all of the inheritance logic around clauses if there
-        // isn't an error code such as "Cannot extend double" already on the
-        // class.
-        if (!_checkForImplementsDisallowedClass(implementsClause) &&
-            !_checkForExtendsDisallowedClass(extendsClause) &&
-            !_checkForAllMixinErrorCodes(withClause)) {
-          _checkForExtendsDeferredClass(extendsClause);
-          _checkForImplementsDeferredClass(implementsClause);
-          _checkForNonAbstractClassInheritsAbstractMember(node.name);
-          _checkForInconsistentMethodInheritance();
-          _checkForRecursiveInterfaceInheritance(_enclosingClass);
-          _checkForConflictingGetterAndMethod();
-          _checkForConflictingInstanceGetterAndSuperclassMember();
-          _checkImplementsSuperClass(node);
-          _checkImplementsFunctionWithoutCall(node);
-          _checkForMixinHasNoConstructors(node);
-          if (_options.strongMode) {
-            _checkForMixinWithConflictingPrivateMember(node);
-          }
-        }
+        _checkClassInheritance(node, superclass, withClause, implementsClause);

[Jennifer Messerly, 2017/04/24 21:41:02]

... a bit of postmortem culture going on here :)

Rather than just fix the problem, take steps to make it less likely to happen in
the future. By factoring out a common method there's now a shared place for the
shared checks.

       }
       visitClassDeclarationIncrementally(node);
       _checkForFinalNotInitializedInClass(node);
@@ -533,19 +515,8 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     ClassElementImpl outerClassElement = _enclosingClass;
     try {
       _enclosingClass = AbstractClassElementImpl.getImpl(node.element);
-      ImplementsClause implementsClause = node.implementsClause;
-      // Only check for all of the inheritance logic around clauses if there
-      // isn't an error code such as "Cannot extend double" already on the
-      // class.
-      if (!_checkForExtendsDisallowedClassInTypeAlias(node) &&
-          !_checkForImplementsDisallowedClass(implementsClause) &&
-          !_checkForAllMixinErrorCodes(node.withClause)) {
-        _checkForExtendsDeferredClassInTypeAlias(node);

[Jennifer Messerly, 2017/04/24 21:41:02]

I also attempted to add tests for all the checks that were not present in this
code path.

-        _checkForImplementsDeferredClass(implementsClause);
-        _checkForRecursiveInterfaceInheritance(_enclosingClass);
-        _checkForNonAbstractClassInheritsAbstractMember(node.name);
-        _checkForMixinHasNoConstructors(node);
-      }
+      _checkClassInheritance(
+          node, node.superclass, node.withClause, node.implementsClause);
     } finally {
       _enclosingClass = outerClassElement;
     }
@@ -699,12 +670,6 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
   }
 
   @override
-  Object visitExtendsClause(ExtendsClause node) {
-    _checkForImplicitDynamicType(node.superclass);

[Jennifer Messerly, 2017/04/24 21:41:02]

done in _checkClassInheritance now

-    return super.visitExtendsClause(node);
-  }
-
-  @override
   Object visitFieldDeclaration(FieldDeclaration node) {
     _isInStaticVariableDeclaration = node.isStatic;
     _isInInstanceVariableDeclaration = !_isInStaticVariableDeclaration;
@@ -1310,6 +1275,39 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
   }
 
   /**
+   * Checks the class for problems with the superclass, mixins, or implemented
+   * interfaces.
+   */
+  void _checkClassInheritance(
+      NamedCompilationUnitMember node,
+      TypeName superclass,
+      WithClause withClause,
+      ImplementsClause implementsClause) {
+    // Only check for all of the inheritance logic around clauses if there
+    // isn't an error code such as "Cannot extend double" already on the
+    // class.
+    if (!_checkForExtendsDisallowedClass(superclass) &&
+        !_checkForImplementsDisallowedClass(implementsClause) &&
+        !_checkForAllMixinErrorCodes(withClause)) {
+      _checkForImplicitDynamicType(superclass);
+      _checkForExtendsDeferredClass(superclass);
+      _checkForImplementsDeferredClass(implementsClause);
+      _checkForNonAbstractClassInheritsAbstractMember(node.name);
+      _checkForInconsistentMethodInheritance();
+      _checkForRecursiveInterfaceInheritance(_enclosingClass);
+      _checkForConflictingGetterAndMethod();
+      _checkForConflictingInstanceGetterAndSuperclassMember();
+      _checkImplementsSuperClass(implementsClause);
+      _checkImplementsFunctionWithoutCall(node.name);
+      _checkForMixinHasNoConstructors(node);
+
+      if (_options.strongMode) {
+        _checkForMixinWithConflictingPrivateMember(withClause, superclass);
+      }
+    }
+  }
+
+  /**
    * Given a list of [directives] that have the same prefix, generate an error
    * if there is more than one import and any of those imports is deferred.
    *
@@ -3589,25 +3587,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
    *
    * See [CompileTimeErrorCode.EXTENDS_DEFERRED_CLASS].
    */
-  void _checkForExtendsDeferredClass(ExtendsClause clause) {
-    if (clause == null) {
-      return;
-    }
-    _checkForExtendsOrImplementsDeferredClass(
-        clause.superclass, CompileTimeErrorCode.EXTENDS_DEFERRED_CLASS);
-  }
-
-  /**
-   * Verify that the given type [alias] does not extend a deferred class.
-   *
-   * See [CompileTimeErrorCode.EXTENDS_DISALLOWED_CLASS].
-   */
-  void _checkForExtendsDeferredClassInTypeAlias(ClassTypeAlias alias) {
-    if (alias == null) {
+  void _checkForExtendsDeferredClass(TypeName superclass) {
+    if (superclass == null) {
       return;
     }
     _checkForExtendsOrImplementsDeferredClass(
-        alias.superclass, CompileTimeErrorCode.EXTENDS_DEFERRED_CLASS);
+        superclass, CompileTimeErrorCode.EXTENDS_DEFERRED_CLASS);
   }
 
   /**
@@ -3616,26 +3601,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
    *
    * See [CompileTimeErrorCode.EXTENDS_DISALLOWED_CLASS].
    */
-  bool _checkForExtendsDisallowedClass(ExtendsClause clause) {
-    if (clause == null) {
+  bool _checkForExtendsDisallowedClass(TypeName superclass) {
+    if (superclass == null) {
       return false;
     }
     return _checkForExtendsOrImplementsDisallowedClass(
-        clause.superclass, CompileTimeErrorCode.EXTENDS_DISALLOWED_CLASS);
-  }
-
-  /**
-   * Verify that the given type [alias] does not extend classes such as 'num' or
-   * 'String'.
-   *
-   * See [CompileTimeErrorCode.EXTENDS_DISALLOWED_CLASS].
-   */
-  bool _checkForExtendsDisallowedClassInTypeAlias(ClassTypeAlias alias) {
-    if (alias == null) {
-      return false;
-    }
-    return _checkForExtendsOrImplementsDisallowedClass(
-        alias.superclass, CompileTimeErrorCode.EXTENDS_DISALLOWED_CLASS);
+        superclass, CompileTimeErrorCode.EXTENDS_DISALLOWED_CLASS);
   }
 
   /**
@@ -4896,12 +4867,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
    * library that defines a private member that conflicts with a private name
    * from the same library but from a superclass or a different mixin.
    */
-  void _checkForMixinWithConflictingPrivateMember(ClassDeclaration node) {
-    WithClause withClause = node.withClause;
+  void _checkForMixinWithConflictingPrivateMember(
+      WithClause withClause, TypeName superclassName) {
     if (withClause == null) {
       return;
     }
-    DartType declaredSupertype = node.extendsClause?.superclass?.type;
+    DartType declaredSupertype = superclassName?.type;
     if (declaredSupertype is! InterfaceType) {
       return;
     }
@@ -6194,12 +6165,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
    *
    * See [StaticWarningCode.FUNCTION_WITHOUT_CALL].
    */
-  void _checkImplementsFunctionWithoutCall(ClassDeclaration declaration) {
-    if (declaration.isAbstract) {
+  void _checkImplementsFunctionWithoutCall(AstNode className) {
+    ClassElement classElement = _enclosingClass;
+    if (classElement == null) {
       return;
     }
-    ClassElement classElement = declaration.element;
-    if (classElement == null) {
+    if (classElement.isAbstract) {
       return;
     }
     if (!_typeSystem.isSubtypeOf(
@@ -6217,7 +6188,7 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
         callMethod is! MethodElement ||
         (callMethod as MethodElement).isAbstract) {
       _errorReporter.reportErrorForNode(
-          StaticWarningCode.FUNCTION_WITHOUT_CALL, declaration.name);
+          StaticWarningCode.FUNCTION_WITHOUT_CALL, className);
     }
   }
 
@@ -6227,14 +6198,13 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
    *
    * See [CompileTimeErrorCode.IMPLEMENTS_SUPER_CLASS].
    */
-  void _checkImplementsSuperClass(ClassDeclaration declaration) {
+  void _checkImplementsSuperClass(ImplementsClause implementsClause) {
     // prepare super type
     InterfaceType superType = _enclosingClass.supertype;
     if (superType == null) {
       return;
     }
     // prepare interfaces
-    ImplementsClause implementsClause = declaration.implementsClause;
     if (implementsClause == null) {
       return;
     }