Start separating ClassElementImpl for Class and Enum.

pkg/analyzer/lib/src/dart/element/element.dart

 // Copyright (c) 2014, 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.
 
 library analyzer.src.dart.element.element;
 
 import 'dart:collection';
 import 'dart:math' show min;
 
 import 'package:analyzer/dart/ast/ast.dart';
@@ skipping 44 matching lines @@
   /**
    * Initialize a newly created pair to have both the [staticElement] and the
    * [propagatedElement].
    */
   AuxiliaryElements(this.staticElement, this.propagatedElement);
 }
 
 /**
  * A concrete implementation of a [ClassElement].
  */
-class ClassElementImpl extends ElementImpl
+abstract class ClassElementImpl extends ElementImpl

[Brian Wilkerson, 2016/06/01 14:08:30]

I'll just note that this is a breaking change. I don't know whether we have any
clients that are creating class elements (I hope not), but if so this will break
them.

[Paul Berry, 2016/06/01 17:33:14]

As discussed in person, I'm ok with making this change.  IMHO it's a stretch to
call it a breaking change at all--the class is called "...Impl" and is inside
"src/", so any client that is broken by this change was already breaking pub
conventions and accessing analyzer internals.

     with TypeParameterizedElementMixin
     implements ClassElement {
   /**
-   * The unlinked representation of the class in the summary.
-   */
-  final UnlinkedClass _unlinkedClass;
-
-  /**
    * A list containing all of the accessors (getters and setters) contained in
    * this class.
    */
   List<PropertyAccessorElement> _accessors = PropertyAccessorElement.EMPTY_LIST;
 
   /**
    * For classes which are not mixin applications, a list containing all of the
    * constructors contained in this class, or `null` if the list of
    * constructors has not yet been built.
    *
@@ skipping 45 matching lines @@
   /**
    * A flag indicating whether the types associated with the instance members of
    * this class have been inferred.
    */
   bool hasBeenInferred = false;
 
   /**
    * Initialize a newly created class element to have the given [name] at the
    * given [offset] in the file that contains the declaration of this element.
    */
-  ClassElementImpl(String name, int offset)
-      : _unlinkedClass = null,
-        super(name, offset);
+  ClassElementImpl(String name, int offset) : super(name, offset);
 
   /**
    * Initialize a newly created class element to have the given [name].
    */
-  ClassElementImpl.forNode(Identifier name)
-      : _unlinkedClass = null,
-        super.forNode(name);
+  ClassElementImpl.forNode(Identifier name) : super.forNode(name);

[Paul Berry, 2016/06/01 17:33:13]

As a side note, we could reduce the impact of this change by changing this
constructor into a factory constructor that forwards to ClassElementImpl_Class
or ClassElementImpl_Enum as appropriate.

Your call, though.

 
   /**
    * Initialize using the given serialized information.
    */
-  ClassElementImpl.forSerialized(
-      this._unlinkedClass, CompilationUnitElementImpl enclosingUnit)
+  ClassElementImpl.forSerialized(CompilationUnitElementImpl enclosingUnit)
       : super.forSerialized(enclosingUnit);
 
   /**
    * Set whether this class is abstract.
    */
   void set abstract(bool isAbstract) {
-    assert(_unlinkedClass == null);
     setModifier(Modifier.ABSTRACT, isAbstract);
   }
 
   @override
   List<PropertyAccessorElement> get accessors => _accessors;
 
   /**
    * Set the accessors contained in this class to the given [accessors].
    */
   void set accessors(List<PropertyAccessorElement> accessors) {
     for (PropertyAccessorElement accessor in accessors) {
       (accessor as PropertyAccessorElementImpl).enclosingElement = this;
     }
     this._accessors = accessors;
   }
 
   @override
   List<InterfaceType> get allSupertypes {
     List<InterfaceType> list = new List<InterfaceType>();
     _collectAllSupertypes(list);
     return list;
   }
 
   @override
-  int get codeLength {
-    if (_unlinkedClass != null) {
-      return _unlinkedClass.codeRange?.length;
-    }
-    return super.codeLength;
-  }
-
-  @override
-  int get codeOffset {
-    if (_unlinkedClass != null) {
-      return _unlinkedClass.codeRange?.offset;
-    }
-    return super.codeOffset;
-  }
-
-  @override
   List<ConstructorElement> get constructors {
     if (!isMixinApplication) {
       assert(_constructors != null);
       return _constructors ?? ConstructorElement.EMPTY_LIST;
     }
     return _computeMixinAppConstructors();
   }
 
   /**
    * Set the constructors contained in this class to the given [constructors].
    *
    * Should only be used for class elements that are not mixin applications.
    */
   void set constructors(List<ConstructorElement> constructors) {
     assert(!isMixinApplication);
     for (ConstructorElement constructor in constructors) {
       (constructor as ConstructorElementImpl).enclosingElement = this;
     }
     this._constructors = constructors;
   }
 
   @override
   String get displayName => name;
 
-  @override
-  SourceRange get docRange {
-    if (_unlinkedClass != null) {
-      UnlinkedDocumentationComment comment =
-          _unlinkedClass.documentationComment;
-      return comment != null
-          ? new SourceRange(comment.offset, comment.length)
-          : null;
-    }
-    return super.docRange;
-  }
-
-  @override
-  String get documentationComment {
-    if (_unlinkedClass != null) {
-      return _unlinkedClass?.documentationComment?.text;
-    }
-    return super.documentationComment;
-  }
-
   /**
    * Return `true` if [CompileTimeErrorCode.MIXIN_HAS_NO_CONSTRUCTORS] should
    * be reported for this class.
    */
   bool get doesMixinLackConstructors {
     if (!isMixinApplication && mixins.isEmpty) {
       // This class is not a mixin application and it doesn't have a "with"
       // clause, so CompileTimeErrorCode.MIXIN_HAS_NO_CONSTRUCTORS is
       // inapplicable.
       return false;
@@ skipping 27 matching lines @@
         }
         nearestNonMixinClass = nearestNonMixinClass.supertype.element;
       }
     }
     return !nearestNonMixinClass.constructors.any(isSuperConstructorAccessible);
   }
 
   @override
   TypeParameterizedElementMixin get enclosingTypeParameterContext => null;
 
-  /**
-   * Set whether this class is defined by an enum declaration.
-   */
-  void set enum2(bool isEnum) {
-    setModifier(Modifier.ENUM, isEnum);
-  }
-
   @override
   List<FieldElement> get fields => _fields;
 
   /**
    * Set the fields contained in this class to the given [fields].
    */
   void set fields(List<FieldElement> fields) {
     for (FieldElement field in fields) {
       (field as FieldElementImpl).enclosingElement = this;
     }
@@ skipping 56 matching lines @@
     for (PropertyAccessorElement accessor in _accessors) {
       if (accessor.isStatic) {
         return true;
       }
     }
     return false;
   }
 
   @override
   List<InterfaceType> get interfaces {
-    if (_unlinkedClass != null && _interfaces == null) {
-      ResynthesizerContext context = enclosingUnit.resynthesizerContext;
-      _interfaces = _unlinkedClass.interfaces
-          .map((EntityRef t) => context.resolveTypeRef(t, this))
-          .toList(growable: false);
-    }
     return _interfaces ?? const <InterfaceType>[];
   }
 
   void set interfaces(List<InterfaceType> interfaces) {
-    assert(_unlinkedClass == null);
     _interfaces = interfaces;
   }
 
   @override
   bool get isAbstract {
-    if (_unlinkedClass != null) {
-      return _unlinkedClass.isAbstract;
-    }
     return hasModifier(Modifier.ABSTRACT);
   }
 
   @override
-  bool get isEnum => hasModifier(Modifier.ENUM);
+  bool get isEnum;
 
   @override
   bool get isMixinApplication {
-    if (_unlinkedClass != null) {
-      return _unlinkedClass.isMixinApplication;
-    }
     return hasModifier(Modifier.MIXIN_APPLICATION);

[Brian Wilkerson, 2016/06/01 17:29:44]

Seems like this (and several other members) ought to just get copied down to
ClassElementImpl_Class. There's no value in having those members call 'super'
when the superclass implementation won't be used by ClassElementImpl_Enum.

   }
 
   @override
   bool get isOrInheritsProxy =>
       _safeIsOrInheritsProxy(this, new HashSet<ClassElement>());
 
   @override
   bool get isProxy {
     for (ElementAnnotation annotation in metadata) {
       if (annotation.isProxy) {
@@ skipping 18 matching lines @@
         return false;
       }
     }
     return true;
   }
 
   @override
   ElementKind get kind => ElementKind.CLASS;
 
   @override
-  List<ElementAnnotation> get metadata {
-    if (_unlinkedClass != null) {
-      return _metadata ??=
-          _buildAnnotations(enclosingUnit, _unlinkedClass.annotations);
-    }
-    return super.metadata;
-  }
-
-  @override
   List<MethodElement> get methods => _methods;
 
   /**
    * Set the methods contained in this class to the given [methods].
    */
   void set methods(List<MethodElement> methods) {
     for (MethodElement method in methods) {
       (method as MethodElementImpl).enclosingElement = this;
     }
     this._methods = methods;
   }
 
   /**
    * Set whether this class is a mixin application.
    */
   void set mixinApplication(bool isMixinApplication) {
-    assert(_unlinkedClass == null);
     setModifier(Modifier.MIXIN_APPLICATION, isMixinApplication);
   }
 
   @override
   List<InterfaceType> get mixins {
-    if (_unlinkedClass != null && _mixins == null) {
-      ResynthesizerContext context = enclosingUnit.resynthesizerContext;
-      _mixins = _unlinkedClass.mixins
-          .map((EntityRef t) => context.resolveTypeRef(t, this))
-          .toList(growable: false);
-    }
     return _mixins ?? const <InterfaceType>[];
   }
 
   void set mixins(List<InterfaceType> mixins) {
-    assert(_unlinkedClass == null);
     _mixins = mixins;
   }
 
-  @override
-  String get name {
-    if (_unlinkedClass != null) {
-      return _unlinkedClass.name;
-    }
-    return super.name;
+  /**
+   * Return resynthesized type parameter elements.
+   */
+  List<TypeParameterElement> get resynthesizedTypeParameters {
+    return super.typeParameters;
   }
 
   @override
-  int get nameOffset {
-    if (_unlinkedClass != null) {
-      return _unlinkedClass.nameOffset;
-    }
-    return super.nameOffset;
-  }
-
-  @override
   TypeParameterizedElementMixin get typeParameterContext => this;
 
   @override
   List<TypeParameterElement> get typeParameters {
-    if (_unlinkedClass != null) {
-      return super.typeParameters;
-    }
     return _typeParameters;
   }
 
   /**
    * Set the type parameters defined for this class to the given
    * [typeParameters].
    */
   void set typeParameters(List<TypeParameterElement> typeParameters) {
-    assert(_unlinkedClass == null);
     for (TypeParameterElement typeParameter in typeParameters) {
       (typeParameter as TypeParameterElementImpl).enclosingElement = this;
     }
     this._typeParameters = typeParameters;
   }
 
   @override
-  List<UnlinkedTypeParam> get unlinkedTypeParams =>
-      _unlinkedClass.typeParameters;
+  List<UnlinkedTypeParam> get unlinkedTypeParams => const <UnlinkedTypeParam>[];
 
   @override
   ConstructorElement get unnamedConstructor {
     for (ConstructorElement element in constructors) {
       String name = element.displayName;
       if (name == null || name.isEmpty) {
         return element;
       }
     }
     return null;
@@ skipping 11 matching lines @@
       buffer.write('enum ');
     } else {
       buffer.write('class ');
     }
     String name = displayName;
     if (name == null) {
       buffer.write("{unnamed class}");
     } else {
       buffer.write(name);
     }
-    int variableCount = _typeParameters.length;
+    int variableCount = typeParameters.length;
     if (variableCount > 0) {
       buffer.write("<");
       for (int i = 0; i < variableCount; i++) {
         if (i > 0) {
           buffer.write(", ");
         }
-        (_typeParameters[i] as TypeParameterElementImpl).appendTo(buffer);
+        (typeParameters[i] as TypeParameterElementImpl).appendTo(buffer);
       }
       buffer.write(">");
     }
     if (supertype != null && !supertype.isObject) {
       buffer.write(' extends ');
       buffer.write(supertype.displayName);
     }
     if (mixins.isNotEmpty) {
       buffer.write(' with ');
       buffer.write(mixins.map((t) => t.displayName).join(', '));
@@ skipping 38 matching lines @@
       if (fieldImpl.identifier == identifier) {
         return fieldImpl;
       }
     }
     for (MethodElement method in _methods) {
       MethodElementImpl methodImpl = method;
       if (methodImpl.identifier == identifier) {
         return methodImpl;
       }
     }
-    for (TypeParameterElement typeParameter in _typeParameters) {
+    for (TypeParameterElement typeParameter in typeParameters) {
       TypeParameterElementImpl typeParameterImpl = typeParameter;
       if (typeParameterImpl.identifier == identifier) {
         return typeParameterImpl;
       }
     }
     return null;
   }
 
   @override
   FieldElement getField(String name) {
@@ skipping 429 matching lines @@
   static ClassElementImpl getImpl(ClassElement classElement) {
     if (classElement is ClassElementHandle) {
       classElement.ensureActualElementComplete();
       return getImpl(classElement.actualElement);
     }
     return classElement as ClassElementImpl;
   }
 }
 
 /**
+ * A [ClassElementImpl], which is a class.
+ */
+class ClassElementImpl_Class extends ClassElementImpl {
+  /**
+   * The unlinked representation of the class in the summary.
+   */
+  final UnlinkedClass _unlinkedClass;
+
+  /**
+   * Initialize a newly created class element to have the given [name] at the
+   * given [offset] in the file that contains the declaration of this element.
+   */
+  ClassElementImpl_Class(String name, int offset)
+      : _unlinkedClass = null,
+        super(name, offset);
+
+  /**
+   * Initialize a newly created class element to have the given [name].
+   */
+  ClassElementImpl_Class.forNode(Identifier name)
+      : _unlinkedClass = null,
+        super.forNode(name);
+
+  /**
+   * Initialize using the given serialized information.
+   */
+  ClassElementImpl_Class.forSerialized(
+      this._unlinkedClass, CompilationUnitElementImpl enclosingUnit)
+      : super.forSerialized(enclosingUnit);
+
+  /**
+   * Set whether this class is abstract.
+   */
+  void set abstract(bool isAbstract) {
+    assert(_unlinkedClass == null);
+    super.abstract = isAbstract;
+  }
+
+  @override
+  int get codeLength {
+    if (_unlinkedClass != null) {
+      return _unlinkedClass.codeRange?.length;
+    }
+    return super.codeLength;
+  }
+
+  @override
+  int get codeOffset {
+    if (_unlinkedClass != null) {
+      return _unlinkedClass.codeRange?.offset;
+    }
+    return super.codeOffset;
+  }
+
+  @override
+  SourceRange get docRange {
+    if (_unlinkedClass != null) {
+      UnlinkedDocumentationComment comment =
+          _unlinkedClass.documentationComment;
+      return comment != null
+          ? new SourceRange(comment.offset, comment.length)
+          : null;
+    }
+    return super.docRange;
+  }
+
+  @override
+  String get documentationComment {
+    if (_unlinkedClass != null) {
+      return _unlinkedClass?.documentationComment?.text;
+    }
+    return super.documentationComment;
+  }
+
+  @override
+  List<InterfaceType> get interfaces {
+    if (_unlinkedClass != null && _interfaces == null) {
+      ResynthesizerContext context = enclosingUnit.resynthesizerContext;
+      _interfaces = _unlinkedClass.interfaces
+          .map((EntityRef t) => context.resolveTypeRef(t, this))
+          .toList(growable: false);
+    }
+    return super.interfaces;
+  }
+
+  void set interfaces(List<InterfaceType> interfaces) {
+    assert(_unlinkedClass == null);
+    super.interfaces = interfaces;
+  }
+
+  @override
+  bool get isAbstract {
+    if (_unlinkedClass != null) {
+      return _unlinkedClass.isAbstract;
+    }
+    return super.isAbstract;
+  }
+
+  @override
+  bool get isEnum => false;
+
+  @override
+  bool get isMixinApplication {
+    if (_unlinkedClass != null) {
+      return _unlinkedClass.isMixinApplication;
+    }
+    return super.isMixinApplication;
+  }
+
+  @override
+  List<ElementAnnotation> get metadata {
+    if (_unlinkedClass != null) {
+      return _metadata ??=
+          _buildAnnotations(enclosingUnit, _unlinkedClass.annotations);
+    }
+    return super.metadata;
+  }
+
+  /**
+   * Set whether this class is a mixin application.
+   */
+  void set mixinApplication(bool isMixinApplication) {
+    assert(_unlinkedClass == null);
+    super.mixinApplication = isMixinApplication;
+  }
+
+  @override
+  List<InterfaceType> get mixins {
+    if (_unlinkedClass != null && _mixins == null) {
+      ResynthesizerContext context = enclosingUnit.resynthesizerContext;
+      _mixins = _unlinkedClass.mixins
+          .map((EntityRef t) => context.resolveTypeRef(t, this))
+          .toList(growable: false);
+    }
+    return super.mixins;
+  }
+
+  void set mixins(List<InterfaceType> mixins) {
+    assert(_unlinkedClass == null);
+    super.mixins = mixins;
+  }
+
+  @override
+  String get name {
+    if (_unlinkedClass != null) {
+      return _unlinkedClass.name;
+    }
+    return super.name;
+  }
+
+  @override
+  int get nameOffset {
+    if (_unlinkedClass != null) {
+      return _unlinkedClass.nameOffset;
+    }
+    return super.nameOffset;
+  }
+
+  @override
+  List<TypeParameterElement> get typeParameters {
+    if (_unlinkedClass != null) {
+      return resynthesizedTypeParameters;
+    }
+    return super.typeParameters;
+  }
+
+  /**
+   * Set the type parameters defined for this class to the given
+   * [typeParameters].
+   */
+  void set typeParameters(List<TypeParameterElement> typeParameters) {
+    assert(_unlinkedClass == null);
+    super.typeParameters = typeParameters;
+  }
+
+  @override
+  List<UnlinkedTypeParam> get unlinkedTypeParams =>
+      _unlinkedClass.typeParameters;
+}
+
+/**
+ * A [ClassElementImpl], which is an enum.
+ */
+class ClassElementImpl_Enum extends ClassElementImpl {
+  /**
+   * The unlinked representation of the enum in the summary.
+   */
+  final UnlinkedEnum _unlinkedEnum;
+
+  /**
+   * Initialize a newly created class element to have the given [name] at the
+   * given [offset] in the file that contains the declaration of this element.
+   */
+  ClassElementImpl_Enum(String name, int offset)
+      : _unlinkedEnum = null,
+        super(name, offset);
+
+  /**
+   * Initialize a newly created class element to have the given [name].
+   */
+  ClassElementImpl_Enum.forNode(Identifier name)
+      : _unlinkedEnum = null,
+        super.forNode(name);
+
+  /**
+   * Initialize using the given serialized information.
+   */
+  ClassElementImpl_Enum.forSerialized(
+      this._unlinkedEnum, CompilationUnitElementImpl enclosingUnit)
+      : super.forSerialized(enclosingUnit);
+
+  /**
+   * Set whether this class is abstract.
+   */
+  void set abstract(bool isAbstract) {
+    assert(_unlinkedEnum == null);
+    super.abstract = isAbstract;

[Brian Wilkerson, 2016/06/01 14:08:30]

Why isn't this an error?

[scheglov, 2016/06/01 17:25:28]

Fixed.

+  }
+
+  @override
+  int get codeLength {
+    if (_unlinkedEnum != null) {
+      return _unlinkedEnum.codeRange?.length;
+    }
+    return super.codeLength;
+  }
+
+  @override
+  int get codeOffset {
+    if (_unlinkedEnum != null) {
+      return _unlinkedEnum.codeRange?.offset;
+    }
+    return super.codeOffset;
+  }
+
+  @override
+  SourceRange get docRange {
+    if (_unlinkedEnum != null) {
+      UnlinkedDocumentationComment comment = _unlinkedEnum.documentationComment;
+      return comment != null
+          ? new SourceRange(comment.offset, comment.length)
+          : null;
+    }
+    return super.docRange;
+  }
+
+  @override
+  String get documentationComment {
+    if (_unlinkedEnum != null) {
+      return _unlinkedEnum?.documentationComment?.text;
+    }
+    return super.documentationComment;
+  }
+
+  @override
+  List<InterfaceType> get interfaces => const <InterfaceType>[];
+
+  void set interfaces(List<InterfaceType> interfaces) {
+    assert(false);

[Brian Wilkerson, 2016/06/01 14:08:30]

Do we really want to only throw in checked mode?

[scheglov, 2016/06/01 17:25:28]

Well, this is what we do everywhere.

+  }
+
+  @override
+  bool get isAbstract => false;
+
+  @override
+  bool get isEnum => true;
+
+  @override
+  bool get isMixinApplication => false;
+
+  @override
+  List<ElementAnnotation> get metadata {
+    if (_unlinkedEnum != null) {
+      return _metadata ??=
+          _buildAnnotations(enclosingUnit, _unlinkedEnum.annotations);
+    }
+    return super.metadata;
+  }
+
+  /**
+   * Set whether this class is a mixin application.
+   */
+  void set mixinApplication(bool isMixinApplication) {
+    assert(false);
+  }
+
+  @override
+  List<InterfaceType> get mixins => const <InterfaceType>[];
+
+  void set mixins(List<InterfaceType> mixins) {
+    assert(false);
+  }
+
+  @override
+  String get name {
+    if (_unlinkedEnum != null) {
+      return _unlinkedEnum.name;
+    }
+    return super.name;
+  }
+
+  @override
+  int get nameOffset {
+    if (_unlinkedEnum != null) {
+      return _unlinkedEnum.nameOffset;
+    }
+    return super.nameOffset;
+  }
+
+  @override
+  List<TypeParameterElement> get typeParameters =>
+      const <TypeParameterElement>[];
+
+  /**
+   * Set the type parameters defined for this class to the given
+   * [typeParameters].
+   */
+  void set typeParameters(List<TypeParameterElement> typeParameters) {
+    assert(false);
+  }
+}
+
+/**
  * A concrete implementation of a [CompilationUnitElement].
  */
 class CompilationUnitElementImpl extends UriReferencedElementImpl
     implements CompilationUnitElement {
   /**
    * The context in which this unit is resynthesized, or `null` if the
    * element is not resynthesized a summary.
    */
   final ResynthesizerContext resynthesizerContext;
 
@@ skipping 6320 matching lines @@
       }
     }
     return _typeParameterElements;
   }
 
   /**
    * Get a list of [TypeParameterType] objects corresponding to the
    * element's type parameters.
    */
   List<TypeParameterType> get typeParameterTypes {
-    return _typeParameterTypes ??= typeParameters
-        .map((TypeParameterElement e) => e.type)
-        .toList(growable: false);
+    var zzz = typeParameters;

[Brian Wilkerson, 2016/06/01 14:08:30]

nit: Perhaps a better name and an explicit type?

[scheglov, 2016/06/01 17:25:27]

Sorry, forgot to inline after debugging.
Fixed.

+    return _typeParameterTypes ??=
+        zzz.map((TypeParameterElement e) => e.type).toList(growable: false);
   }
 
   /**
    * Get the [UnlinkedTypeParam]s representing the type parameters declared by
    * this element.
    *
    * TODO(scheglov) make private after switching linker to Impl
    */
   List<UnlinkedTypeParam> get unlinkedTypeParams;
 
@@ skipping 264 matching lines @@
 
   @override
   void visitElement(Element element) {
     int offset = element.nameOffset;
     if (offset != -1) {
       map[offset] = element;
     }
     super.visitElement(element);
   }
 }