Refactor constructors of ModelElement (#2828)

* partial

* analyzes

* finish refactor

* rebuild and delete an unnecessary reference

* rebuild

* dartfmt

* Rename to ModelObjectBuilder so that we can build more than just ModelElements

* complete rename

Author: jcollins-g

lib/src/element_type.dart

@@ -10,11 +10,13 @@ import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
 import 'package:dartdoc/src/model/comment_referable.dart';
 import 'package:dartdoc/src/model/model.dart';
+import 'package:dartdoc/src/model/model_object_builder.dart';
 import 'package:dartdoc/src/render/element_type_renderer.dart';
 
 /// Base class representing a type in Dartdoc.  It wraps a [DartType], and
 /// may link to a [ModelElement].
-abstract class ElementType extends Privacy with CommentReferable, Nameable {
+abstract class ElementType extends Privacy
+    with CommentReferable, Nameable, ModelBuilder {
   final DartType _type;
   @override
   final PackageGraph packageGraph;
@@ -39,7 +41,7 @@ abstract class ElementType extends Privacy with CommentReferable, Nameable {
       }
       return UndefinedElementType(f, library, packageGraph, returnedFrom);
     } else {
-      var element = ModelElement.fromElement(f.element, packageGraph);
+      var element = packageGraph.modelBuilder.fromElement(f.element);
       // [TypeAliasElement.aliasElement] has different implications.
       // In that case it is an actual type alias of some kind (generic
       // or otherwise.   Here however aliasElement signals that this is a
@@ -172,7 +174,7 @@ class FunctionTypeElementType extends UndefinedElementType
 
   /// An unmodifiable list of this function element's type parameters.
   List<TypeParameter> get typeFormals => type.typeFormals
-      .map((p) => ModelElement.from(p, library, packageGraph) as TypeParameter)
+      .map((p) => packageGraph.modelBuilder.from(p, library) as TypeParameter)
       .toList(growable: false);
 
   @override
@@ -218,16 +220,16 @@ class ParameterizedElementType extends DefinedElementType with Rendered {
 }
 
 /// A [ElementType] whose underlying type was referrred to by a type alias.
-mixin Aliased implements ElementType {
+mixin Aliased implements ElementType, ModelBuilderInterface {
   @override
   String get name => type.alias.element.name;
 
   @override
   bool get isTypedef => true;
 
   ModelElement _aliasElement;
-  ModelElement get aliasElement => _aliasElement ??=
-      ModelElement.fromElement(type.alias.element, packageGraph);
+  ModelElement get aliasElement =>
+      _aliasElement ??= modelBuilder.fromElement(type.alias.element);
 
   Iterable<ElementType> _aliasArguments;
   Iterable<ElementType> get aliasArguments =>
@@ -374,7 +376,7 @@ abstract class DefinedElementType extends ElementType {
 /// unless it is an alias reference.
 mixin Callable implements ElementType {
   List<Parameter> get parameters => type.parameters
-      .map((p) => ModelElement.from(p, library, packageGraph) as Parameter)
+      .map((p) => modelBuilder.from(p, library) as Parameter)
       .toList(growable: false);
 
   ElementType _returnType;

lib/src/generator/templates.runtime_renderers.dart

@@ -14,6 +14,7 @@ import 'package:dartdoc/src/model/feature.dart';
 import 'package:dartdoc/src/model/feature_set.dart';
 import 'package:dartdoc/src/model/language_feature.dart';
 import 'package:dartdoc/src/model/model.dart';
+import 'package:dartdoc/src/model/model_object_builder.dart';
 import 'package:dartdoc/src/mustachio/parser.dart';
 import 'package:dartdoc/src/mustachio/renderer_base.dart';
 import 'package:dartdoc/src/warnings.dart';
@@ -369,6 +370,7 @@ class _Renderer_Annotation extends RendererBase<Annotation> {
           CT_,
           () => {
                 ..._Renderer_Feature.propertyMap<CT_>(),
+                ..._Renderer_ModelBuilder.propertyMap<CT_>(),
                 'annotation': Property(
                   getValue: (CT_ c) => c.annotation,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -868,6 +870,7 @@ class _Renderer_Category extends RendererBase<Category> {
                 ..._Renderer_LibraryContainer.propertyMap<CT_>(),
                 ..._Renderer_TopLevelContainer.propertyMap<CT_>(),
                 ..._Renderer_Indexable.propertyMap<CT_>(),
+                ..._Renderer_ModelBuilder.propertyMap<CT_>(),
                 'canonicalLibrary': Property(
                   getValue: (CT_ c) => c.canonicalLibrary,
                   renderVariable:
@@ -1929,19 +1932,6 @@ class _Renderer_CommentReferable extends RendererBase<CommentReferable> {
                         parent: r);
                   },
                 ),
-                'packageGraph': Property(
-                  getValue: (CT_ c) => c.packageGraph,
-                  renderVariable: (CT_ c, Property<CT_> self,
-                          List<String> remainingNames) =>
-                      self.renderSimpleVariable(
-                          c, remainingNames, 'PackageGraph'),
-                  isNullValue: (CT_ c) => c.packageGraph == null,
-                  renderValue: (CT_ c, RendererBase<CT_> r,
-                      List<MustachioNode> ast, StringSink sink) {
-                    renderSimple(c.packageGraph, ast, r.template, sink,
-                        parent: r, getters: _invisibleGetters['PackageGraph']);
-                  },
-                ),
                 'referenceChildren': Property(
                   getValue: (CT_ c) => c.referenceChildren,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -4124,6 +4114,7 @@ class _Renderer_ElementType extends RendererBase<ElementType> {
                 ..._Renderer_Privacy.propertyMap<CT_>(),
                 ..._Renderer_CommentReferable.propertyMap<CT_>(),
                 ..._Renderer_Nameable.propertyMap<CT_>(),
+                ..._Renderer_ModelBuilder.propertyMap<CT_>(),
                 'canHaveParameters': Property(
                   getValue: (CT_ c) => c.canHaveParameters,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -9323,6 +9314,55 @@ class _Renderer_MixinTemplateData extends RendererBase<MixinTemplateData> {
   }
 }
 
+class _Renderer_ModelBuilder extends RendererBase<ModelBuilder> {
+  static final Map<Type, Object> _propertyMapCache = {};
+  static Map<String, Property<CT_>> propertyMap<CT_ extends ModelBuilder>() =>
+      _propertyMapCache.putIfAbsent(
+          CT_,
+          () => {
+                'modelBuilder': Property(
+                  getValue: (CT_ c) => c.modelBuilder,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(
+                          c, remainingNames, 'ModelObjectBuilder'),
+                  isNullValue: (CT_ c) => c.modelBuilder == null,
+                  renderValue: (CT_ c, RendererBase<CT_> r,
+                      List<MustachioNode> ast, StringSink sink) {
+                    renderSimple(c.modelBuilder, ast, r.template, sink,
+                        parent: r,
+                        getters: _invisibleGetters['ModelObjectBuilder']);
+                  },
+                ),
+                'packageGraph': Property(
+                  getValue: (CT_ c) => c.packageGraph,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(
+                          c, remainingNames, 'PackageGraph'),
+                  isNullValue: (CT_ c) => c.packageGraph == null,
+                  renderValue: (CT_ c, RendererBase<CT_> r,
+                      List<MustachioNode> ast, StringSink sink) {
+                    renderSimple(c.packageGraph, ast, r.template, sink,
+                        parent: r, getters: _invisibleGetters['PackageGraph']);
+                  },
+                ),
+              });
+
+  _Renderer_ModelBuilder(ModelBuilder context, RendererBase<Object> parent,
+      Template template, StringSink sink)
+      : super(context, parent, template, sink);
+
+  @override
+  Property<ModelBuilder> getProperty(String key) {
+    if (propertyMap<ModelBuilder>().containsKey(key)) {
+      return propertyMap<ModelBuilder>()[key];
+    } else {
+      return null;
+    }
+  }
+}
+
 void _render_ModelElement(ModelElement context, List<MustachioNode> ast,
     Template template, StringSink sink,
     {RendererBase<Object> parent}) {
@@ -9346,6 +9386,7 @@ class _Renderer_ModelElement extends RendererBase<ModelElement> {
                 ..._Renderer_Indexable.propertyMap<CT_>(),
                 ..._Renderer_FeatureSet.propertyMap<CT_>(),
                 ..._Renderer_DocumentationComment.propertyMap<CT_>(),
+                ..._Renderer_ModelBuilder.propertyMap<CT_>(),
                 'allParameters': Property(
                   getValue: (CT_ c) => c.allParameters,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -10827,6 +10868,7 @@ class _Renderer_Package extends RendererBase<Package> {
                 ..._Renderer_Canonicalization.propertyMap<CT_>(),
                 ..._Renderer_Warnable.propertyMap<CT_>(),
                 ..._Renderer_CommentReferable.propertyMap<CT_>(),
+                ..._Renderer_ModelBuilder.propertyMap<CT_>(),
                 'allLibraries': Property(
                   getValue: (CT_ c) => c.allLibraries,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -11583,7 +11625,7 @@ class _Renderer_Package extends RendererBase<Package> {
   }
 }
 
-String renderError(PackageTemplateData context, Template template) {
+String renderIndex(PackageTemplateData context, Template template) {
   var buffer = StringBuffer();
   _render_PackageTemplateData(context, template.ast, template, buffer);
   return buffer.toString();
@@ -11785,7 +11827,7 @@ class _Renderer_PackageTemplateData extends RendererBase<PackageTemplateData> {
   }
 }
 
-String renderIndex(PackageTemplateData context, Template template) {
+String renderError(PackageTemplateData context, Template template) {
   var buffer = StringBuffer();
   _render_PackageTemplateData(context, template.ast, template, buffer);
   return buffer.toString();
@@ -15126,7 +15168,6 @@ const _invisibleGetters = {
     'unnamedConstructor'
   },
   'CommentReferable': {
-    'packageGraph',
     'scope',
     'href',
     'referenceChildren',
@@ -15610,6 +15651,7 @@ const _invisibleGetters = {
     'resourceProvider',
     'sourceCode'
   },
+  'ModelObjectBuilder': {'hashCode', 'runtimeType'},
   'PackageGraph': {
     'hashCode',
     'runtimeType',

lib/src/model/accessor.dart

@@ -51,7 +51,7 @@ class Accessor extends ModelElement implements EnclosedElement {
   GetterSetterCombo get definingCombo {
     if (_definingCombo == null) {
       var variable = element.variable;
-      _definingCombo = ModelElement.fromElement(variable, packageGraph);
+      _definingCombo = modelBuilder.fromElement(variable);
       assert(_definingCombo != null, 'Unable to find defining combo');
     }
     return _definingCombo;
@@ -141,7 +141,7 @@ class Accessor extends ModelElement implements EnclosedElement {
           element.enclosingElement.enclosingElement);
     }
 
-    return ModelElement.from(element.enclosingElement, library, packageGraph);
+    return modelBuilder.from(element.enclosingElement, library);
   }
 
   @override
@@ -196,12 +196,12 @@ class ContainerAccessor extends Accessor with ContainerMember, Inheritable {
     ContainerAccessor accessor;
     if (element == null) return null;
     if (inheritedAccessors.contains(element)) {
-      accessor = ModelElement.from(
-          element, enclosingContainer.library, enclosingContainer.packageGraph,
+      accessor = enclosingContainer.packageGraph.modelBuilder.from(
+          element, enclosingContainer.library,
           enclosingContainer: enclosingContainer);
     } else {
-      accessor = ModelElement.from(
-          element, enclosingContainer.library, enclosingContainer.packageGraph);
+      accessor = enclosingContainer.packageGraph.modelBuilder
+          .from(element, enclosingContainer.library);
     }
     return accessor;
   }
@@ -261,7 +261,7 @@ class ContainerAccessor extends Accessor with ContainerMember, Inheritable {
           if (accessor != null) {
             accessor = accessor.declaration;
             InheritingContainer parentContainer =
-                ModelElement.fromElement(t.element, packageGraph);
+                modelBuilder.fromElement(t.element);
             var possibleFields = <Field>[];
             possibleFields.addAll(parentContainer.instanceFields);
             possibleFields.addAll(parentContainer.staticFields);

lib/src/model/annotation.dart

@@ -7,14 +7,15 @@ import 'package:dartdoc/src/element_type.dart';
 import 'package:dartdoc/src/model/feature.dart';
 import 'package:dartdoc/src/model/getter_setter_combo.dart';
 import 'package:dartdoc/src/model/library.dart';
-import 'package:dartdoc/src/model/model_element.dart';
+import 'package:dartdoc/src/model/model_object_builder.dart';
 import 'package:dartdoc/src/model/package_graph.dart';
 
 /// Represents a Dart annotation, attached to an element in the source code with
 /// `@`.
-class Annotation extends Feature {
+class Annotation extends Feature with ModelBuilder {
   final ElementAnnotation annotation;
   final Library library;
+  @override
   final PackageGraph packageGraph;
 
   Annotation(this.annotation, this.library, this.packageGraph)
@@ -28,7 +29,7 @@ class Annotation extends Feature {
   /// Return the linked name of the annotation.
   @override
   String get linkedName => annotation.element is PropertyAccessorElement
-      ? ModelElement.fromElement(annotation.element, packageGraph).linkedName
+      ? modelBuilder.fromElement(annotation.element).linkedName
       // TODO(jcollins-g): consider linking to constructor instead of type?
       : modelType.linkedName;
 
@@ -40,10 +41,9 @@ class Annotation extends Feature {
         _modelType =
             ElementType.from(annotatedWith.returnType, library, packageGraph);
       } else if (annotatedWith is PropertyAccessorElement) {
-        _modelType =
-            (ModelElement.fromElement(annotatedWith.variable, packageGraph)
-                    as GetterSetterCombo)
-                .modelType;
+        _modelType = (modelBuilder.fromElement(annotatedWith.variable)
+                as GetterSetterCombo)
+            .modelType;
       } else {
         assert(false,
             'non-callable element used as annotation?: ${annotation.element}');

lib/src/model/category.dart

@@ -8,6 +8,7 @@ import 'package:dartdoc/src/comment_references/model_comment_reference.dart';
 import 'package:dartdoc/src/dartdoc_options.dart';
 import 'package:dartdoc/src/model/comment_referable.dart';
 import 'package:dartdoc/src/model/model.dart';
+import 'package:dartdoc/src/model/model_object_builder.dart';
 import 'package:dartdoc/src/render/category_renderer.dart';
 import 'package:dartdoc/src/warnings.dart';
 
@@ -22,7 +23,8 @@ class Category extends Nameable
         MarkdownFileDocumentation,
         LibraryContainer,
         TopLevelContainer,
-        Indexable
+        Indexable,
+        ModelBuilder
     implements Documentable {
   /// All libraries in [libraries] must come from [package].
   final Package _package;

lib/src/model/comment_referable.dart

@@ -14,9 +14,8 @@ import 'package:analyzer/dart/element/scope.dart';
 import 'package:dartdoc/src/model/accessor.dart';
 import 'package:dartdoc/src/model/container.dart';
 import 'package:dartdoc/src/model/library.dart';
-import 'package:dartdoc/src/model/model_element.dart';
+import 'package:dartdoc/src/model/model_object_builder.dart';
 import 'package:dartdoc/src/model/nameable.dart';
-import 'package:dartdoc/src/model/package_graph.dart';
 import 'package:meta/meta.dart';
 
 class ReferenceChildrenLookup {
@@ -78,9 +77,7 @@ extension CommentReferableEntryBuilder on Map<String, CommentReferable> {
 }
 
 /// Support comment reference lookups on a Nameable object.
-mixin CommentReferable implements Nameable {
-  PackageGraph packageGraph;
-
+mixin CommentReferable implements Nameable, ModelBuilderInterface {
   /// For any [CommentReferable] where an analyzer [Scope] exists (or can
   /// be constructed), implement this.  This will take priority over
   /// lookups via [referenceChildren].  Can be cached.
@@ -147,7 +144,7 @@ mixin CommentReferable implements Nameable {
       bool Function(CommentReferable) filter) {
     var resultElement = scope.lookupPreferGetter(referenceLookup.lookup);
     if (resultElement == null) return null;
-    var result = ModelElement.fromElement(resultElement, packageGraph);
+    var result = modelBuilder.fromElement(resultElement);
     if (result is Accessor) {
       result = (result as Accessor).enclosingCombo;
     }

lib/src/model/constructor.dart

@@ -36,7 +36,7 @@ class Constructor extends ModelElement
 
   @override
   ModelElement get enclosingElement =>
-      ModelElement.from(element.enclosingElement, library, packageGraph);
+      modelBuilder.from(element.enclosingElement, library);
 
   @override
   String get filePath =>
@@ -131,7 +131,7 @@ class Constructor extends ModelElement
       _referenceChildren.addEntries(allParameters.map((param) {
         var paramElement = param.element;
         if (paramElement is FieldFormalParameterElement) {
-          return ModelElement.fromElement(paramElement.field, packageGraph);
+          return modelBuilder.fromElement(paramElement.field);
         }
         return param;
       }).generateEntries());

lib/src/model/container_member.dart

@@ -22,7 +22,7 @@ mixin ContainerMember on ModelElement implements EnclosedElement {
 
   Container get definingEnclosingContainer {
     _definingEnclosingContainer ??=
-        ModelElement.fromElement(element.enclosingElement, packageGraph);
+        modelBuilder.fromElement(element.enclosingElement);
     return _definingEnclosingContainer;
   }
 

lib/src/model/documentation_comment.dart

@@ -4,7 +4,6 @@ import 'package:dartdoc/src/model/documentable.dart';
 import 'package:dartdoc/src/model/documentation.dart';
 import 'package:dartdoc/src/model/inheritable.dart';
 import 'package:dartdoc/src/model/locatable.dart';
-import 'package:dartdoc/src/model/model_element.dart';
 import 'package:dartdoc/src/model/source_code_mixin.dart';
 import 'package:dartdoc/src/render/model_element_renderer.dart';
 import 'package:dartdoc/src/utils.dart';
@@ -57,9 +56,7 @@ mixin DocumentationComment
             (this as Inheritable).overriddenElement != null) {
           return (this as Inheritable).overriddenElement.documentationFrom;
         } else if (this is Inheritable && (this as Inheritable).isInherited) {
-          var thisInheritable = (this as Inheritable);
-          var fromThis = ModelElement.fromElement(
-              element, thisInheritable.definingEnclosingContainer.packageGraph);
+          var fromThis = modelBuilder.fromElement(element);
           return fromThis.documentationFrom;
         } else {
           return [this];