From 24a1a905472ff7368891d826e8e09d4ecd806c26 Mon Sep 17 00:00:00 2001 From: Parker Lougheed Date: Fri, 31 Dec 2021 01:34:20 -0600 Subject: [PATCH 1/3] Cleanup model_element migration and surrounding accesses --- lib/src/model/accessor.dart | 17 +--- lib/src/model/constructor.dart | 2 +- lib/src/model/field.dart | 49 +++++----- lib/src/model/method.dart | 2 +- lib/src/model/model_element.dart | 140 ++++++++++++--------------- lib/src/model/model_node.dart | 71 ++++++-------- lib/src/model/source_code_mixin.dart | 7 +- 7 files changed, 131 insertions(+), 157 deletions(-) diff --git a/lib/src/model/accessor.dart b/lib/src/model/accessor.dart index 2364570c82..50b2944fe9 100644 --- a/lib/src/model/accessor.dart +++ b/lib/src/model/accessor.dart @@ -55,20 +55,13 @@ class Accessor extends ModelElement implements EnclosedElement { late final GetterSetterCombo definingCombo = modelBuilder.fromElement(element.variable) as GetterSetterCombo; - String? _sourceCode; + late final String _sourceCode = isSynthetic + ? _sourceCodeRenderer.renderSourceCode( + packageGraph.getModelNodeFor(definingCombo.element)!.sourceCode) + : super.sourceCode; @override - String? get sourceCode { - if (_sourceCode == null) { - if (isSynthetic) { - _sourceCode = _sourceCodeRenderer.renderSourceCode( - packageGraph.getModelNodeFor(definingCombo.element)!.sourceCode!); - } else { - _sourceCode = super.sourceCode; - } - } - return _sourceCode; - } + String get sourceCode => _sourceCode; bool _documentationCommentComputed = false; String? _documentationComment; diff --git a/lib/src/model/constructor.dart b/lib/src/model/constructor.dart index 4b5f39b1e5..9980c331d5 100644 --- a/lib/src/model/constructor.dart +++ b/lib/src/model/constructor.dart @@ -105,7 +105,7 @@ class Constructor extends ModelElement Map get referenceChildren { if (_referenceChildren == null) { _referenceChildren = {}; - _referenceChildren!.addEntries(allParameters!.map((param) { + _referenceChildren!.addEntries(allParameters.map((param) { var paramElement = param.element; if (paramElement is FieldFormalParameterElement) { return modelBuilder.fromElement(paramElement.field!); diff --git a/lib/src/model/field.dart b/lib/src/model/field.dart index 60179047df..7558c01e63 100644 --- a/lib/src/model/field.dart +++ b/lib/src/model/field.dart @@ -137,34 +137,31 @@ class Field extends ModelElement SourceCodeRenderer get _sourceCodeRenderer => packageGraph.rendererFactory.sourceCodeRenderer; - String? _sourceCode; - - @override - String? get sourceCode { - if (_sourceCode == null) { - // We could use a set to figure the dupes out, but that would lose ordering. - var fieldSourceCode = modelNode!.sourceCode ?? ''; - var getterSourceCode = getter?.sourceCode ?? ''; - var setterSourceCode = setter?.sourceCode ?? ''; - var buffer = StringBuffer(); - if (fieldSourceCode.isNotEmpty) { - fieldSourceCode = _sourceCodeRenderer.renderSourceCode(fieldSourceCode); - buffer.write(fieldSourceCode); - } - if (buffer.isNotEmpty) buffer.write('\n\n'); - if (fieldSourceCode != getterSourceCode) { - if (getterSourceCode != setterSourceCode) { - buffer.write(getterSourceCode); - if (buffer.isNotEmpty) buffer.write('\n\n'); - } - } - if (fieldSourceCode != setterSourceCode) { - buffer.write(setterSourceCode); + late final String _sourceCode = () { + // We could use a set to figure the dupes out, but that would lose ordering. + var fieldSourceCode = modelNode?.sourceCode ?? ''; + var getterSourceCode = getter?.sourceCode ?? ''; + var setterSourceCode = setter?.sourceCode ?? ''; + var buffer = StringBuffer(); + if (fieldSourceCode.isNotEmpty) { + fieldSourceCode = _sourceCodeRenderer.renderSourceCode(fieldSourceCode); + buffer.write(fieldSourceCode); + } + if (buffer.isNotEmpty) buffer.write('\n\n'); + if (fieldSourceCode != getterSourceCode) { + if (getterSourceCode != setterSourceCode) { + buffer.write(getterSourceCode); + if (buffer.isNotEmpty) buffer.write('\n\n'); } - _sourceCode = buffer.toString(); } - return _sourceCode; - } + if (fieldSourceCode != setterSourceCode) { + buffer.write(setterSourceCode); + } + return buffer.toString(); + }(); + + @override + String get sourceCode => _sourceCode; @override Library get library => super.library!; diff --git a/lib/src/model/method.dart b/lib/src/model/method.dart index 14e5be8f7e..3aaf160187 100644 --- a/lib/src/model/method.dart +++ b/lib/src/model/method.dart @@ -133,7 +133,7 @@ class Method extends ModelElement _referenceChildren = {}; _referenceChildren!.addEntriesIfAbsent([ ...typeParameters.explicitOnCollisionWith(this), - ...allParameters!.explicitOnCollisionWith(this), + ...allParameters.explicitOnCollisionWith(this), ...modelType.typeArguments.explicitOnCollisionWith(this), ...modelType.returnType.typeArguments.explicitOnCollisionWith(this), ]); diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart index 268b814daa..a7e2373457 100644 --- a/lib/src/model/model_element.dart +++ b/lib/src/model/model_element.dart @@ -131,8 +131,6 @@ abstract class ModelElement extends Canonicalization final Member? _originalMember; final Library? _library; - String? _linkedName; - ModelElement(this._element, this._library, this._packageGraph, [this._originalMember]); @@ -169,8 +167,9 @@ abstract class ModelElement extends Canonicalization // Return the cached ModelElement if it exists. var key = Tuple3(e, library, enclosingContainer); - if (packageGraph.allConstructedModelElements.containsKey(key)) { - return packageGraph.allConstructedModelElements[key]!; + var cachedModelElement = packageGraph.allConstructedModelElements[key]; + if (cachedModelElement != null) { + return cachedModelElement; } ModelElement? newModelElement; @@ -249,9 +248,9 @@ abstract class ModelElement extends Canonicalization // Return the cached ModelElement if it exists. var key = Tuple3(e, library, enclosingContainer); - if (packageGraph.allConstructedModelElements.containsKey(key)) { - return packageGraph.allConstructedModelElements[ - key as Tuple3]!; + var cachedModelElement = packageGraph.allConstructedModelElements[key]; + if (cachedModelElement != null) { + return cachedModelElement; } var newModelElement = ModelElement._fromParameters(e, library, packageGraph, @@ -450,16 +449,15 @@ abstract class ModelElement extends Canonicalization DartdocOptionContext.fromContextElement( packageGraph.config, library!.element, packageGraph.resourceProvider); - Set? _locationPieces; + late final Set _locationPieces = Set.from(element!.location + .toString() + .split(locationSplitter) + .where((s) => s.isNotEmpty)); @override - Set get locationPieces => - _locationPieces ??= Set.from(element!.location - .toString() - .split(locationSplitter) - .where((s) => s.isNotEmpty)); + Set get locationPieces => _locationPieces; - static final Set _specialFeatures = { + static const Set _specialFeatures = { // Replace the @override annotation with a feature that explicitly // indicates whether an override has occurred. 'override', @@ -501,19 +499,13 @@ abstract class ModelElement extends Canonicalization preferredClass: preferredClass); } - ModelElement? _canonicalModelElement; - // Returns the canonical ModelElement for this ModelElement, or null - // if there isn't one. - ModelElement? get canonicalModelElement => - _canonicalModelElement ??= buildCanonicalModelElement(); + // The canonical ModelElement for this ModelElement, + // or null if there isn't one. + late final ModelElement? canonicalModelElement = buildCanonicalModelElement(); - bool get hasSourceHref => sourceHref!.isNotEmpty; - String? _sourceHref; + bool get hasSourceHref => sourceHref.isNotEmpty; - String? get sourceHref { - _sourceHref ??= SourceLinker.fromElement(this).href(); - return _sourceHref; - } + late final String sourceHref = SourceLinker.fromElement(this).href(); Library get definingLibrary { Library? library = modelBuilder.fromElement(element!.library!) as Library?; @@ -722,8 +714,9 @@ abstract class ModelElement extends Canonicalization 'Invalid location data for element: $fullyQualifiedName'); assert(lineInfo != null, 'No lineInfo data available for element: $fullyQualifiedName'); - if (element!.nameOffset >= 0) { - _characterLocation = lineInfo?.getLocation(element!.nameOffset); + var nameOffset = element!.nameOffset; + if (nameOffset >= 0) { + _characterLocation = lineInfo?.getLocation(nameOffset); } } return _characterLocation; @@ -759,7 +752,7 @@ abstract class ModelElement extends Canonicalization return null; } - String? get htmlId => name; + String get htmlId => name; bool get isAsynchronous => isExecutable && (element as ExecutableElement).isAsynchronous; @@ -778,14 +771,14 @@ abstract class ModelElement extends Canonicalization var setterDeprecated = pie.setter?.metadata.any((a) => a.isDeprecated); var deprecatedValues = - [getterDeprecated, setterDeprecated].where((a) => a != null).toList(); + [getterDeprecated, setterDeprecated].whereNotNull().toList(); // At least one of these should be non-null. Otherwise things are weird assert(deprecatedValues.isNotEmpty); // If there are both a setter and getter, only show the property as // deprecated if both are deprecated. - return deprecatedValues.every((d) => d!); + return deprecatedValues.every((d) => d); } return element!.metadata.any((a) => a.isDeprecated); } @@ -820,10 +813,7 @@ abstract class ModelElement extends Canonicalization // FIXME(nnbd): library should not have to be nullable just because of dynamic Library? get library => _library; - String get linkedName { - _linkedName ??= _calculateLinkedName(); - return _linkedName!; - } + late final String linkedName = _calculateLinkedName(); @visibleForTesting @override @@ -870,40 +860,35 @@ abstract class ModelElement extends Canonicalization bool get isPublicAndPackageDocumented => isPublic && package?.isDocumented == true; - List? _allParameters; - // TODO(jcollins-g): This is in the wrong place. Move parts to // [GetterSetterCombo], elsewhere as appropriate? - List? get allParameters { - if (_allParameters == null) { - var recursedParameters = {}; - var newParameters = {}; - if (this is GetterSetterCombo && - (this as GetterSetterCombo).setter != null) { - newParameters.addAll((this as GetterSetterCombo).setter!.parameters); - } else { - if (isCallable) newParameters.addAll(parameters); - } - // TODO(jcollins-g): This part probably belongs in [ElementType]. - while (newParameters.isNotEmpty) { - recursedParameters.addAll(newParameters); - newParameters.clear(); - for (var p in recursedParameters) { - var parameterModelType = p.modelType; - if (parameterModelType is Callable) { - newParameters.addAll(parameterModelType.parameters - .where((pm) => !recursedParameters.contains(pm))); - } - if (parameterModelType is AliasedElementType) { - newParameters.addAll(parameterModelType.aliasedParameters - .where((pm) => !recursedParameters.contains(pm))); - } + late final List allParameters = () { + var recursedParameters = {}; + var newParameters = {}; + if (this is GetterSetterCombo && + (this as GetterSetterCombo).setter != null) { + newParameters.addAll((this as GetterSetterCombo).setter!.parameters); + } else { + if (isCallable) newParameters.addAll(parameters); + } + // TODO(jcollins-g): This part probably belongs in [ElementType]. + while (newParameters.isNotEmpty) { + recursedParameters.addAll(newParameters); + newParameters.clear(); + for (var p in recursedParameters) { + var parameterModelType = p.modelType; + if (parameterModelType is Callable) { + newParameters.addAll(parameterModelType.parameters + .where((pm) => !recursedParameters.contains(pm))); + } + if (parameterModelType is AliasedElementType) { + newParameters.addAll(parameterModelType.aliasedParameters + .where((pm) => !recursedParameters.contains(pm))); } } - _allParameters = recursedParameters.toList(); } - return _allParameters; - } + return recursedParameters.toList(); + }(); @override path.Context get pathContext => packageGraph.resourceProvider.pathContext; @@ -937,8 +922,8 @@ abstract class ModelElement extends Canonicalization } return [ - ...params! - .map((p) => ModelElement._from(p, library, packageGraph) as Parameter) + ...?params?.map( + (p) => ModelElement._from(p, library, packageGraph) as Parameter) ]; }(); @@ -948,12 +933,11 @@ abstract class ModelElement extends Canonicalization @override bool get hasDocumentationComment => element!.documentationComment != null; - String? _sourceCode; + late final String _sourceCode = + _sourceCodeRenderer.renderSourceCode(super.sourceCode); + @override - String? get sourceCode { - return _sourceCode ??= - _sourceCodeRenderer.renderSourceCode(super.sourceCode!); - } + String get sourceCode => _sourceCode; @override int compareTo(dynamic other) { @@ -971,12 +955,13 @@ abstract class ModelElement extends Canonicalization e ??= this; fqName ??= e.name; - if (e is! EnclosedElement || e.enclosingElement == null) { + var enclosingElement = e.enclosingElement; + if (e is! EnclosedElement || enclosingElement == null) { return fqName; } - return _buildFullyQualifiedName(e.enclosingElement as ModelElement?, - '${e.enclosingElement!.name}.$fqName'); + return _buildFullyQualifiedName( + enclosingElement as ModelElement?, '${enclosingElement.name}.$fqName'); } String _calculateLinkedName() { @@ -999,7 +984,10 @@ abstract class ModelElement extends Canonicalization @internal @override - CommentReferable get definingCommentReferable => element == null - ? super.definingCommentReferable - : modelBuilder.fromElement(element!); + CommentReferable get definingCommentReferable { + var element = this.element; + return element == null + ? super.definingCommentReferable + : modelBuilder.fromElement(element); + } } diff --git a/lib/src/model/model_node.dart b/lib/src/model/model_node.dart index a2d8d568f7..1af25f38af 100644 --- a/lib/src/model/model_node.dart +++ b/lib/src/model/model_node.dart @@ -33,49 +33,42 @@ class ModelNode { return []; } - String? _sourceCode; - - String? get sourceCode { - if (_sourceCode == null) { - if (_sourceNode?.offset != null) { - var enclosingSourceNode = _sourceNode; + late final String sourceCode = () { + var enclosingSourceNode = _sourceNode; + if (enclosingSourceNode == null) { + return ''; + } - /// Get a node higher up the syntax tree that includes the semicolon. - /// In this case, it is either a [FieldDeclaration] or - /// [TopLevelVariableDeclaration]. (#2401) - if (_sourceNode is VariableDeclaration) { - enclosingSourceNode = _sourceNode!.parent!.parent; - assert(enclosingSourceNode is FieldDeclaration || - enclosingSourceNode is TopLevelVariableDeclaration); - } + /// Get a node higher up the syntax tree that includes the semicolon. + /// In this case, it is either a [FieldDeclaration] or + /// [TopLevelVariableDeclaration]. (#2401) + if (enclosingSourceNode is VariableDeclaration) { + enclosingSourceNode = enclosingSourceNode.parent!.parent; + assert(enclosingSourceNode is FieldDeclaration || + enclosingSourceNode is TopLevelVariableDeclaration); + } - var sourceEnd = enclosingSourceNode!.end; - var sourceOffset = enclosingSourceNode.offset; + var sourceEnd = enclosingSourceNode!.end; + var sourceOffset = enclosingSourceNode.offset; - var contents = - model_utils.getFileContentsFor(element, resourceProvider); - // Find the start of the line, so that we can line up all the indents. - var i = sourceOffset; - while (i > 0) { - i -= 1; - if (contents![i] == '\n' || contents[i] == '\r') { - i += 1; - break; - } - } + var contents = model_utils.getFileContentsFor(element, resourceProvider)!; + // Find the start of the line, so that we can line up all the indents. + var i = sourceOffset; + while (i > 0) { + i -= 1; + if (contents[i] == '\n' || contents[i] == '\r') { + i += 1; + break; + } + } - // Trim the common indent from the source snippet. - var start = sourceOffset - (sourceOffset - i); - var source = contents!.substring(start, sourceEnd); + // Trim the common indent from the source snippet. + var start = sourceOffset - (sourceOffset - i); + var source = contents.substring(start, sourceEnd); - source = model_utils.stripIndentFromSource(source); - source = model_utils.stripDartdocCommentsFromSource(source); + source = model_utils.stripIndentFromSource(source); + source = model_utils.stripDartdocCommentsFromSource(source); - _sourceCode = source.trim(); - } else { - _sourceCode = ''; - } - } - return _sourceCode; - } + return source.trim(); + }(); } diff --git a/lib/src/model/source_code_mixin.dart b/lib/src/model/source_code_mixin.dart index d5dfd98a0d..cdd050f1ea 100644 --- a/lib/src/model/source_code_mixin.dart +++ b/lib/src/model/source_code_mixin.dart @@ -13,9 +13,12 @@ abstract class SourceCodeMixin implements Documentable { Element? get element; - bool get hasSourceCode => config.includeSource && sourceCode!.isNotEmpty; + bool get hasSourceCode => config.includeSource && sourceCode.isNotEmpty; Library? get library; - String? get sourceCode => modelNode == null ? '' : modelNode!.sourceCode; + String get sourceCode { + var modelNode = this.modelNode; + return modelNode == null ? '' : modelNode.sourceCode; + } } From c11d87749a33ed3dc94f58063f5919a4877d454f Mon Sep 17 00:00:00 2001 From: Parker Lougheed Date: Fri, 31 Dec 2021 01:40:33 -0600 Subject: [PATCH 2/3] More minor improvements --- lib/src/model/model_element.dart | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart index a7e2373457..e0b48fb480 100644 --- a/lib/src/model/model_element.dart +++ b/lib/src/model/model_element.dart @@ -384,11 +384,10 @@ abstract class ModelElement extends Canonicalization return library!.packageGraph.libraryElementReexportedBy[element!.library!]; } - ModelNode? _modelNode; + late final ModelNode? _modelNode = packageGraph.getModelNodeFor(element); @override - ModelNode? get modelNode => - _modelNode ??= packageGraph.getModelNodeFor(element); + ModelNode? get modelNode => _modelNode; Iterable? _annotations; // Skips over annotations with null elements or that are otherwise @@ -690,14 +689,12 @@ abstract class ModelElement extends Canonicalization return (_fullyQualifiedName ??= _buildFullyQualifiedName()); } - String? _fullyQualifiedNameWithoutLibrary; + late final String _fullyQualifiedNameWithoutLibrary = + fullyQualifiedName.replaceFirst('${library!.fullyQualifiedName}.', ''); + @override - String? get fullyQualifiedNameWithoutLibrary { - // Remember, periods are legal in library names. - _fullyQualifiedNameWithoutLibrary ??= - fullyQualifiedName.replaceFirst('${library!.fullyQualifiedName}.', ''); - return _fullyQualifiedNameWithoutLibrary; - } + String get fullyQualifiedNameWithoutLibrary => + _fullyQualifiedNameWithoutLibrary; @override String get sourceFileName => element!.source!.fullName; From 2b1884ca23a542b5156c7912c46d886ba79d4e45 Mon Sep 17 00:00:00 2001 From: Parker Lougheed Date: Fri, 31 Dec 2021 01:45:33 -0600 Subject: [PATCH 3/3] Rebuild renderer for reduced null cases --- .../templates.runtime_renderers.dart | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart index 8434db8107..d7d5beabba 100644 --- a/lib/src/generator/templates.runtime_renderers.dart +++ b/lib/src/generator/templates.runtime_renderers.dart @@ -376,7 +376,7 @@ class _Renderer_Accessor extends RendererBase { nextProperty, [...remainingNames.skip(1)]); }, - isNullValue: (CT_ c) => c.sourceCode == null, + isNullValue: (CT_ c) => false, renderValue: (CT_ c, RendererBase r, List ast, StringSink sink) { _render_String(c.sourceCode, ast, r.template, sink, @@ -5872,7 +5872,7 @@ class _Renderer_Field extends RendererBase { nextProperty, [...remainingNames.skip(1)]); }, - isNullValue: (CT_ c) => c.sourceCode == null, + isNullValue: (CT_ c) => false, renderValue: (CT_ c, RendererBase r, List ast, StringSink sink) { _render_String(c.sourceCode, ast, r.template, sink, @@ -9930,11 +9930,10 @@ class _Renderer_ModelElement extends RendererBase { List remainingNames) => self.renderSimpleVariable( c, remainingNames, 'List'), - isNullValue: (CT_ c) => c.allParameters == null, - renderValue: (CT_ c, RendererBase r, + renderIterable: (CT_ c, RendererBase r, List ast, StringSink sink) { - renderSimple(c.allParameters, ast, r.template, sink, - parent: r, getters: _invisibleGetters['List']!); + return c.allParameters.map((e) => + _render_Parameter(e, ast, r.template, sink, parent: r)); }, ), 'annotations': Property( @@ -10339,8 +10338,7 @@ class _Renderer_ModelElement extends RendererBase { nextProperty, [...remainingNames.skip(1)]); }, - isNullValue: (CT_ c) => - c.fullyQualifiedNameWithoutLibrary == null, + isNullValue: (CT_ c) => false, renderValue: (CT_ c, RendererBase r, List ast, StringSink sink) { _render_String(c.fullyQualifiedNameWithoutLibrary, ast, @@ -10440,7 +10438,7 @@ class _Renderer_ModelElement extends RendererBase { nextProperty, [...remainingNames.skip(1)]); }, - isNullValue: (CT_ c) => c.htmlId == null, + isNullValue: (CT_ c) => false, renderValue: (CT_ c, RendererBase r, List ast, StringSink sink) { _render_String(c.htmlId, ast, r.template, sink, parent: r); @@ -10881,7 +10879,7 @@ class _Renderer_ModelElement extends RendererBase { nextProperty, [...remainingNames.skip(1)]); }, - isNullValue: (CT_ c) => c.sourceCode == null, + isNullValue: (CT_ c) => false, renderValue: (CT_ c, RendererBase r, List ast, StringSink sink) { _render_String(c.sourceCode, ast, r.template, sink, @@ -10925,7 +10923,7 @@ class _Renderer_ModelElement extends RendererBase { nextProperty, [...remainingNames.skip(1)]); }, - isNullValue: (CT_ c) => c.sourceHref == null, + isNullValue: (CT_ c) => false, renderValue: (CT_ c, RendererBase r, List ast, StringSink sink) { _render_String(c.sourceHref, ast, r.template, sink, @@ -13339,7 +13337,7 @@ class _Renderer_SourceCodeMixin extends RendererBase { nextProperty, [...remainingNames.skip(1)]); }, - isNullValue: (CT_ c) => c.sourceCode == null, + isNullValue: (CT_ c) => false, renderValue: (CT_ c, RendererBase r, List ast, StringSink sink) { _render_String(c.sourceCode, ast, r.template, sink,