From 3b511d52a0008751c03751dcab08d9316c6531d8 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 8 Jun 2017 12:03:16 -0700 Subject: [PATCH 01/25] a little debug code --- lib/src/model.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/src/model.dart b/lib/src/model.dart index 0c840cb7dd..8ca7055e75 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -481,6 +481,10 @@ class Class extends ModelElement implements EnclosedElement { Map imap = library.inheritanceManager.getMembersInheritedFromInterfaces(element); + if (cmap.length + imap.length > 10) { + 1+1; + } + // remove methods that exist on this class _methods.forEach((method) { cmap.remove(method.name); From aa57971512c7097c9fbf1e89954dde9a477b637d Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 12 Jun 2017 13:15:57 -0700 Subject: [PATCH 02/25] intermediate state: things mostly work, but a lot of private constants showing up as broken links. --- lib/dartdoc.dart | 20 +- lib/src/html/html_generator_instance.dart | 2 + lib/src/markdown_processor.dart | 2 +- lib/src/model.dart | 503 ++++++++++++++++++---- lib/src/model_utils.dart | 5 +- 5 files changed, 430 insertions(+), 102 deletions(-) diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart index 3f1eaccdfe..2ea2493da0 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -220,9 +220,8 @@ class DartDoc { {String referredFrom}) { // Ordinarily this would go in [Package.warn], but we don't actually know what // ModelElement to warn on yet. - Locatable referredFromElement; Locatable warnOnElement; - Set referredFromElements; + Set referredFromElements = new Set(); Set warnOnElements; // Make all paths relative to origin. @@ -234,18 +233,15 @@ class DartDoc { referredFrom = path.relative(referredFrom, from: origin); } // Source paths are always relative. - referredFromElements = _hrefs[referredFrom]; + if (_hrefs[referredFrom] != null) { + referredFromElements.addAll(_hrefs[referredFrom]); + } } warnOnElements = _hrefs[warnOn]; if (referredFromElements != null) { if (referredFromElements.any((e) => e.isCanonical)) { - referredFromElement = - referredFromElements.firstWhere((e) => e.isCanonical); - } else { - // If we don't have a canonical element, just pick one. - referredFromElement = - referredFromElements.isEmpty ? null : referredFromElements.first; + referredFromElements.removeWhere((e) => !e.isCanonical); } } if (warnOnElements != null) { @@ -257,12 +253,12 @@ class DartDoc { } } - if (referredFromElement == null && referredFrom == 'index.html') - referredFromElement = package; + if (referredFromElements.isEmpty && referredFrom == 'index.html') + referredFromElements.add(package); String message = warnOn; if (referredFrom == 'index.json') message = '$warnOn (from index.json)'; package.warnOnElement(warnOnElement, kind, - message: message, referredFrom: referredFromElement); + message: message, referredFrom: referredFromElements.toList()); } void _doOrphanCheck(Package package, String origin, Set visited) { diff --git a/lib/src/html/html_generator_instance.dart b/lib/src/html/html_generator_instance.dart index 7c12540193..566a3bd29a 100644 --- a/lib/src/html/html_generator_instance.dart +++ b/lib/src/html/html_generator_instance.dart @@ -101,7 +101,9 @@ class HtmlGeneratorInstance implements HtmlOptions { for (var clazz in lib.allClasses) { // TODO(jcollins-g): consider refactor so that only the canonical // ModelElements show up in these lists + //if (clazz.name != 'FlutterLogoDecoration') continue; if (!clazz.isCanonical) continue; + generateClass(package, lib, clazz); for (var constructor in clazz.constructors) { diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index 7147c199ef..40ad81109c 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -292,7 +292,7 @@ MatchingLinkResult _getMatchingLinkElement( refModelElement = new ModelElement.from(searchElement, refLibrary); if (!refModelElement.isCanonical) { refModelElement.warn(PackageWarning.noCanonicalFound, - referredFrom: element); + referredFrom: [element]); // Don't warn about doc references because that's covered by the no // canonical library found message. return new MatchingLinkResult(null, null, warn: false); diff --git a/lib/src/model.dart b/lib/src/model.dart index 8ca7055e75..6064c8e5cd 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -144,6 +144,8 @@ abstract class Inheritable { } } else { if (enclosingElement.isCanonical) { + if (enclosingElement is Library) + 1+1; _canonicalEnclosingClass = enclosingElement; } } @@ -167,27 +169,70 @@ abstract class Inheritable { } } +/// A getter or setter that is a member of a Class. +class InheritableAccessor extends Accessor with Inheritable { + + /// Factory will return an [InheritableAccessor] with isInherited = true + /// if [element] is in [inheritedAccessors]. + factory InheritableAccessor.from(PropertyAccessorElement element, Set inheritedAccessors, Class enclosingClass) { + if (element == null) return null; + if (inheritedAccessors.contains(element)) { + return new ModelElement.from(element, enclosingClass.library, enclosingClass: enclosingClass); + } + return new ModelElement.from(element, enclosingClass.library); + } + + ModelElement _enclosingElement; + bool _isInherited = false; + InheritableAccessor( + PropertyAccessorElement element, Library library) + : super(element, library); + + InheritableAccessor.inherited(PropertyAccessorElement element, Library library, this._enclosingElement) + : super(element, library) { + _isInherited = true; + } + + @override + bool get isInherited => _isInherited; + + @override + ModelElement get enclosingElement { + if (_enclosingElement == null) { + if (_accessor.enclosingElement is CompilationUnitElement) { + _enclosingElement = package + .findOrCreateLibraryFor(_accessor.enclosingElement.enclosingElement); + } else { + _enclosingElement = new ModelElement.from(_accessor.enclosingElement, library); + } + } + return _enclosingElement; + } +} + + /// Getters and setters. class Accessor extends ModelElement with SourceCodeMixin implements EnclosedElement { - ModelElement _enclosingCombo; + GetterSetterCombo _enclosingCombo; + Accessor( - PropertyAccessorElement element, Library library, this._enclosingCombo) + PropertyAccessorElement element, Library library) : super(element, library); + ModelElement get enclosingCombo => _enclosingCombo; - @override - bool get isCanonical => enclosingCombo.isCanonical; + /// Call exactly once to set the enclosing combo for this Accessor. + void set enclosingCombo(GetterSetterCombo combo) { + assert(_enclosingCombo == null); + _enclosingCombo = combo; + } @override - void warn(PackageWarning kind, {String message, Locatable referredFrom}) { - if (enclosingCombo != null) { - enclosingCombo.warn(kind, message: message, referredFrom: referredFrom); - } else { - super.warn(kind, message: message, referredFrom: referredFrom); - } + String get computeDocumentationComment { + return super.computeDocumentationComment; } @override @@ -200,6 +245,20 @@ class Accessor extends ModelElement return new ModelElement.from(_accessor.enclosingElement, library); } + @override + bool get isCanonical => enclosingCombo.isCanonical; + + bool get isInherited => false; + + @override + void warn(PackageWarning kind, {String message, List referredFrom}) { + if (enclosingCombo != null) { + enclosingCombo.warn(kind, message: message, referredFrom: referredFrom); + } else { + super.warn(kind, message: message, referredFrom: referredFrom); + } + } + @override String get href { if (canonicalLibrary == null) return null; @@ -209,6 +268,7 @@ class Accessor extends ModelElement bool get isGetter => _accessor.isGetter; ModelElement _overriddenElement; + @override Accessor get overriddenElement { assert(package.allLibrariesAdded); @@ -224,15 +284,42 @@ class Accessor extends ModelElement accessor = Package.getBasestElement(accessor); } Class parentClass = new ModelElement.from( - parent, package.findOrCreateLibraryFor(parent)); + t.element, package.findOrCreateLibraryFor(t.element)); List possibleFields = []; possibleFields.addAll(parentClass.allInstanceProperties); possibleFields.addAll(parentClass.staticProperties); String fieldName = accessor.name.replaceFirst('=', ''); + Field foundField = possibleFields.firstWhere((f) => f.element.name == fieldName); + if (this.isGetter) { + _overriddenElement = foundField.getter; + } else { + _overriddenElement = foundField.setter; + } + assert(!(_overriddenElement as Accessor).isInherited); + if (_overriddenElement != null) + 1+1; + break; + /* _overriddenElement = new ModelElement.from(accessor, library, enclosingCombo: possibleFields .firstWhere((f) => f.element.name == fieldName)); - break; + */ + + /*Field foundField; + foundField = possibleFields.firstWhere( + (f) => f.getter?.element == accessor || f.setter?.element == accessor, orElse: () => null); + if (foundField != null) { + print ('found something: ${accessor.name}'); + if (foundField.getter?.element == accessor) { + _overriddenElement = foundField.getter; + } + if (foundField.setter?.element == accessor) { + _overriddenElement = foundField.setter; + } + break; + }*/ + } else { + 1+1; } } } @@ -246,6 +333,7 @@ class Accessor extends ModelElement PropertyAccessorElement get _accessor => (element as PropertyAccessorElement); } + class Class extends ModelElement implements EnclosedElement { List _mixins; ElementType _supertype; @@ -496,8 +584,8 @@ class Class extends ModelElement implements EnclosedElement { Set uniqueNames = new Set(); instanceProperties.forEach((f) { - if (f._setter != null) uniqueNames.add(f._setter.name); - if (f._getter != null) uniqueNames.add(f._getter.name); + if (f.setter != null) uniqueNames.add(f.setter.element.name); + if (f.getter != null) uniqueNames.add(f.getter.element.name); }); for (String key in cmap.keys) { @@ -599,6 +687,12 @@ class Class extends ModelElement implements EnclosedElement { } List get inheritedProperties { + if (_inheritedProperties == null) { + _inheritedProperties = _allFields.where((f) => f.isInherited).toList()..sort(byName); + } + return _inheritedProperties; + } + /*List get inheritedProperties { if (_inheritedProperties != null) return _inheritedProperties; Map cmap = library.inheritanceManager.getMembersInheritedFromClasses(element); @@ -610,8 +704,8 @@ class Class extends ModelElement implements EnclosedElement { Set uniqueNames = new Set(); instanceProperties.forEach((f) { - if (f._setter != null) uniqueNames.add(f._setter.name); - if (f._getter != null) uniqueNames.add(f._getter.name); + if (f.setter != null) uniqueNames.add(f.setter.element.name); + if (f.getter != null) uniqueNames.add(f.getter.element.name); }); for (String key in cmap.keys) { @@ -663,7 +757,7 @@ class Class extends ModelElement implements EnclosedElement { _inheritedProperties.sort(byName); return _inheritedProperties; - } + }*/ List get instanceMethods { if (_instanceMethods != null) return _instanceMethods; @@ -809,6 +903,38 @@ class Class extends ModelElement implements EnclosedElement { ElementType get supertype => _supertype; + List get _allFields { + if (_fields != null) return _fields; + _fields = []; + Map cmap = + library.inheritanceManager.getMembersInheritedFromClasses(element); + Map imap = + library.inheritanceManager.getMembersInheritedFromInterfaces(element); + + Set inheritedAccessors = new Set(); + inheritedAccessors.addAll(cmap.values.where((e) => e is PropertyAccessorElement)); + inheritedAccessors.addAll(imap.values.where((e) => e is PropertyAccessorElement)); + + for (FieldElement f in _cls.fields.where(isPublic)) { + PropertyAccessorElement getterElement = f.getter; + PropertyAccessorElement setterElement = f.setter; + Accessor getter = new InheritableAccessor.from(getterElement, inheritedAccessors, this); + Accessor setter = new InheritableAccessor.from(setterElement, inheritedAccessors, this); + if ((getter != null && getter.isInherited) || (setter != null && setter.isInherited)) { + // Field is >=50% inherited. + // TODO(jcollins-g): make half-inherited fields half-italicized...? + _fields.add(new ModelElement.from(f, library, enclosingClass: this, + getter: getter, setter: setter)); + } else { + _fields.add(new ModelElement.from(f, library, + getter: getter, setter: setter)); + } + } + _fields.sort(byName); + return _fields; + } + + /* List get _allFields { if (_fields != null) return _fields; @@ -820,6 +946,7 @@ class Class extends ModelElement implements EnclosedElement { return _fields; } + */ ClassElement get _cls => (element as ClassElement); @@ -948,16 +1075,24 @@ class Enum extends Class { if (_enumFields != null) return _enumFields; // This is a hack to give 'values' an index of -1 and all other fields - // their expected indicies. https://github.com/dart-lang/dartdoc/issues/1176 + // their expected indices. https://github.com/dart-lang/dartdoc/issues/1176 var index = -1; + _enumFields = []; + for (FieldElement f in _cls.fields.where(isPublic).where((f) => f.isConst)) { + // Enums do not have inheritance. + Accessor accessor = new ModelElement.from(f.getter, library); + _enumFields.add(new ModelElement.from(f, library, index: index++, getter: accessor)); + } + _enumFields.sort(byName); + /* _enumFields = _cls.fields .where(isPublic) .where((f) => f.isConst) .map((field) => new ModelElement.from(field, library, index: index++)) .toList(growable: false) ..sort(byName); - + */ return _enumFields; } @@ -981,10 +1116,23 @@ class Enum extends Class { class EnumField extends Field { int _index; - EnumField(FieldElement element, Library library) : super(element, library); + EnumField(FieldElement element, Library library, Accessor getter, Accessor setter) : super(element, library, getter, setter); - EnumField.forConstant(this._index, FieldElement element, Library library) - : super(element, library); + EnumField.forConstant(this._index, FieldElement element, Library library, Accessor getter) + : super(element, library, getter, null); + + /* + Accessor get getter { + // EnumFields should never be trying to get getter/setters, they are + // all synthetic. + assert(false); + return null; + } + Accessor get setter { + assert(false); + return null; + } + */ @override String get constantValue { @@ -996,8 +1144,8 @@ class EnumField extends Field { } @override - EnumField get documentationFrom { - if (name == 'values' && name == 'index') return this; + List get documentationFrom { + if (name == 'values' && name == 'index') return [this]; return super.documentationFrom; } @@ -1046,18 +1194,24 @@ class Field extends ModelElement String _constantValue; bool _isInherited = false; Class _enclosingClass; + @override + final InheritableAccessor getter; + @override + final InheritableAccessor setter; - Field(FieldElement element, Library library) : super(element, library) { + Field(FieldElement element, Library library, this.getter, this.setter) : super(element, library) { _setModelType(); + if (getter != null) getter.enclosingCombo = this; + if (setter != null) setter.enclosingCombo = this; } - Field.inherited(FieldElement element, this._enclosingClass, Library library) - : super(element, library) { - _isInherited = true; - _setModelType(); + factory Field.inherited(FieldElement element, Class enclosingClass, Library library, Accessor getter, Accessor setter) { + Field newField = new Field(element, library, getter, setter); + newField._isInherited = true; // Can't set _isInherited to true if this is the defining element, because // that would mean it isn't inherited. - assert(enclosingElement != definingEnclosingElement); + assert(newField.enclosingElement != newField.definingEnclosingElement); + return newField; } @override @@ -1083,6 +1237,76 @@ class Field extends ModelElement String get constantValueTruncated => truncateString(constantValue, 200); + /* + @override + List get documentationFrom { + if (_documentationFrom == null) { + _documentationFrom = []; + if (getter != null) _documentationFrom.addAll(getter.documentationFrom); + if (setter != null) _documentationFrom.addAll(setter.documentationFrom); + if (_documentationFrom.length > 1) { + int c = 0; + for (ModelElement e in _documentationFrom) { + if (e.documentation != '') { + c += 1; + } + } + if (c == _documentationFrom.length) + 1+1; + } + } + return _documentationFrom; + } + + String _oneLineDoc; + @override + String get oneLineDoc { + if (_oneLineDoc == null) { + List oneLineDocs = []; + for (ModelElement e in [getter, setter]) { + if (e != null) + oneLineDocs.add(e.oneLineDoc); + } + } + } + */ + /* + // TODO(jcollins-g): documentationFrom doesn't really make sense in the case + // of split fields. + @override + ModelElement get documentationFrom { + if (_documentationFrom == null) { + if (computeDocumentationComment == null && + canOverride() && + overriddenElement != null) { + _documentationFrom = overriddenElement; + } else if (this is Inheritable && (this as Inheritable).isInherited) { + Inheritable thisInheritable = (this as Inheritable); + ModelElement fromThis = new ModelElement.from( + element, thisInheritable.definingEnclosingElement.library); + _documentationFrom = fromThis.documentationFrom; + } else { + _documentationFrom = this; + } + } + return _documentationFrom; + } + + String get _documentationLocal { + if (_rawDocs != null) return _rawDocs; + if (config.dropTextFrom.contains(element.library.name)) { + _rawDocs = ''; + } else { + _rawDocs = computeDocumentationComment ?? ''; + _rawDocs = stripComments(_rawDocs) ?? ''; + _rawDocs = _injectExamples(_rawDocs); + _rawDocs = _stripMacroTemplatesAndAddToIndex(_rawDocs); + _rawDocs = _injectMacros(_rawDocs); + } + return _rawDocs; + } + */ + @override ModelElement get enclosingElement { if (_enclosingClass == null) { @@ -1160,7 +1384,9 @@ class Field extends ModelElement } @override - String get _computeDocumentationComment { + String get computeDocumentationComment { + if (name == 'hashCode' && enclosingElement.name == 'FlutterLogoDecoration') + 1+1; String docs = getterSetterDocumentationComment; if (docs.isEmpty) return _field.documentationComment; return docs; @@ -1170,11 +1396,13 @@ class Field extends ModelElement String get _fileName => isConst ? '$name-constant.html' : '$name.html'; + /* @override PropertyAccessorElement get _getter => _field.getter; @override PropertyAccessorElement get _setter => _field.setter; + */ void _setModelType() { if (hasGetter) { @@ -1189,22 +1417,73 @@ class Field extends ModelElement /// Mixin for top-level variables and fields (aka properties) abstract class GetterSetterCombo implements ModelElement { - Accessor get getter { + Accessor get getter; + + /*{ return _getter == null ? null - : new ModelElement.from(_getter, library, enclosingCombo: this); + : new ModelElement.from(_getter, library, enclosingCombo: this, enclosingClass: isInherited ? enclosingElement : null); + }*/ + + ModelElement enclosingElement; + bool get isInherited; + + @override + List get documentationFrom { + if (_documentationFrom == null) { + _documentationFrom = []; + if (getter != null) _documentationFrom.addAll(getter.documentationFrom); + if (setter != null) _documentationFrom.addAll(setter.documentationFrom); + if (_documentationFrom.length > 1) { + int c = 0; + for (ModelElement e in _documentationFrom) { + if (e.documentation != '') { + c += 1; + } + } + if (c == _documentationFrom.length) + 1+1; + } + } + return _documentationFrom; + } + + String _oneLineDoc; + @override + String get oneLineDoc { + if (_oneLineDoc == null) { + StringBuffer buffer = new StringBuffer(); + bool addGetterSetterText = false; + if (getter != null && getter.oneLineDoc.isNotEmpty && + setter != null && setter.oneLineDoc.isNotEmpty) { + addGetterSetterText = true; + } + if (getter != null) { + buffer.writeln('${addGetterSetterText ? "On read: ": ""}${getter.oneLineDoc}'); + } + if (setter != null) { + buffer.writeln('${addGetterSetterText ? "On write: ": ""}${setter.oneLineDoc}'); + } + _oneLineDoc = buffer.toString(); + } + return _oneLineDoc; } String get getterSetterDocumentationComment { var buffer = new StringBuffer(); - if (hasGetter && !_getter.isSynthetic) { - String docs = stripComments(_getter.documentationComment); + if (hasGetter && getter == null) + 1+1; + + if (hasGetter && !getter.element.isSynthetic) { + assert(getter.documentationFrom.length == 1); + String docs = stripComments(getter.documentationFrom.first.computeDocumentationComment); if (docs != null) buffer.write(docs); } - if (hasSetter && !_setter.isSynthetic) { - String docs = stripComments(_setter.documentationComment); + if (hasSetter && !setter.element.isSynthetic) { + assert(setter.documentationFrom.length == 1); + String docs = stripComments(setter.documentationFrom.first.computeDocumentationComment); if (docs != null) { if (buffer.isNotEmpty) buffer.write('\n\n'); buffer.write(docs); @@ -1236,10 +1515,10 @@ abstract class GetterSetterCombo implements ModelElement { return null; } - bool get hasExplicitGetter => hasGetter && !_getter.isSynthetic; + bool get hasExplicitGetter => hasGetter && !getter.element.isSynthetic; - bool get hasExplicitSetter => hasSetter && !_setter.isSynthetic; - bool get hasImplicitSetter => hasSetter && _setter.isSynthetic; + bool get hasExplicitSetter => hasSetter && !setter.element.isSynthetic; + bool get hasImplicitSetter => hasSetter && setter.element.isSynthetic; bool get hasGetter; @@ -1267,18 +1546,20 @@ abstract class GetterSetterCombo implements ModelElement { bool get writeOnly => hasSetter && !hasGetter; - Accessor get setter { - return _setter == null + Accessor get setter; /*{ + return _setter == null ? null - : new ModelElement.from(_setter, library, enclosingCombo: this); - } + : new ModelElement.from(_setter, library, enclosingCombo: this, enclosingClass: isInherited ? enclosingElement : null); + }*/ + /* PropertyAccessorElement get _getter; // TODO: now that we have explicit getter and setters, we probably // want a cleaner way to do this. Only the one-liner is using this // now. The detail pages should be using getter and setter directly. PropertyAccessorElement get _setter; + */ } class Library extends ModelElement { @@ -1528,14 +1809,28 @@ class Library extends ModelElement { elements.addAll(cu.topLevelVariables); } _exportedNamespace.definedNames.values.forEach((element) { - if (element is PropertyAccessorElement) elements.add(element.variable); + if (element is PropertyAccessorElement) { + elements.add(element.variable); + } }); - _variables = elements + _variables = []; + for (TopLevelVariableElement element in elements) { + Accessor getter; + if (element.getter != null) + getter = new ModelElement.from(element.getter, this); + Accessor setter; + if (element.setter != null) + setter = new ModelElement.from(element.setter, this); + _variables.add(new ModelElement.from(element, this, getter: getter, setter: setter)); + } + + _variables.sort(byName); + /*elements .where(isPublic) .map((e) => new ModelElement.from(e, this)) .toList(growable: false) ..sort(byName); - + */ return _variables; } @@ -1753,16 +2048,16 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { // somehow. ModelElement(this._element, this._library); + // TODO(jcollins-g): this way of using the optional parameter is messy, + // clean that up. + // TODO(jcollins-g): Refactor this into class-specific factories that + // call this one. + // TODO(jcollins-g): enforce this. /// Do not construct any ModelElements unless they are from this constructor. - /// TODO(jcollins-g): enforce this. /// Specify enclosingClass only if this is to be an inherited object. /// Specify index only if this is to be an EnumField.forConstant. - /// Specify enclosingCombo (a GetterSetterCombo) only if this is to be an - /// Accessor. - /// TODO(jcollins-g): this way of using the optional parameter is messy, - /// clean that up. - factory ModelElement.from(Element e, Library library, - {Class enclosingClass, int index, ModelElement enclosingCombo}) { + factory ModelElement.from(Element e, Library library, + {Class enclosingClass, int index, Accessor getter, Accessor setter}) { // We don't need index in this key because it isn't a disambiguator. // It isn't a disambiguator because EnumFields are not inherited, ever. // TODO(jcollins-g): cleanup class hierarchy so that EnumFields aren't @@ -1770,8 +2065,7 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { if (e is Member) { e = Package.getBasestElement(e); } - Tuple4 key = - new Tuple4(e, library, enclosingClass, enclosingCombo); + Tuple3 key = new Tuple3(e, library, enclosingClass); ModelElement newModelElement; if (e.kind != ElementKind.DYNAMIC && library.package._allConstructedModelElements.containsKey(key)) { @@ -1805,16 +2099,19 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { if (e is FieldElement) { if (enclosingClass == null) { if (index != null) { - newModelElement = new EnumField.forConstant(index, e, library); + assert(getter != null); + newModelElement = new EnumField.forConstant(index, e, library, getter); } else { if (e.enclosingElement.isEnum) { - newModelElement = new EnumField(e, library); + newModelElement = new EnumField(e, library, getter, setter); } else { - newModelElement = new Field(e, library); + assert(getter != null || setter != null); + newModelElement = new Field(e, library, getter, setter); } } } else { - newModelElement = new Field.inherited(e, enclosingClass, library); + assert(getter != null || setter != null); + newModelElement = new Field.inherited(e, enclosingClass, library, getter, setter); } } if (e is ConstructorElement) { @@ -1833,10 +2130,18 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { newModelElement = new Method.inherited(e, enclosingClass, library); } if (e is TopLevelVariableElement) { - newModelElement = new TopLevelVariable(e, library); + assert(getter != null || setter != null); + newModelElement = new TopLevelVariable(e, library, getter, setter); } if (e is PropertyAccessorElement) { - newModelElement = new Accessor(e, library, enclosingCombo); + if (e.enclosingElement is ClassElement) { + if (enclosingClass == null) + newModelElement = new InheritableAccessor(e, library); + else + newModelElement = new InheritableAccessor.inherited(e, library, enclosingClass); + } else { + newModelElement = new Accessor(e, library); + } } if (e is TypeParameterElement) { newModelElement = new TypeParameter(e, library); @@ -1856,10 +2161,15 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { library.package._allInheritableElements[iKey].add(newModelElement); } } - if (newModelElement is Accessor) { - assert(newModelElement.enclosingCombo == enclosingCombo); - } else { - assert(enclosingCombo == null); + if (newModelElement is Field) { + if (getter != null && newModelElement.getter == null) + 1+1; + assert(getter == null || newModelElement.getter.enclosingCombo != null); + assert(setter == null || newModelElement.setter.enclosingCombo != null); + } + if (newModelElement is TopLevelVariable) { + assert(getter == null || newModelElement.getter.enclosingCombo != null); + assert(setter == null || newModelElement.setter.enclosingCombo != null); } return newModelElement; @@ -1932,7 +2242,7 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { bool get canHaveParameters => element is ExecutableElement || element is FunctionTypeAliasElement; - ModelElement _documentationFrom; + List _documentationFrom; /// Returns the ModelElement from which we will get documentation. /// @@ -1940,19 +2250,19 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { /// to find docs, if the current class doesn't have docs /// for this element. @override - ModelElement get documentationFrom { + List get documentationFrom { if (_documentationFrom == null) { - if (_computeDocumentationComment == null && + if (computeDocumentationComment == null && canOverride() && overriddenElement != null) { - _documentationFrom = overriddenElement; + _documentationFrom = [overriddenElement]; } else if (this is Inheritable && (this as Inheritable).isInherited) { Inheritable thisInheritable = (this as Inheritable); ModelElement fromThis = new ModelElement.from( element, thisInheritable.definingEnclosingElement.library); _documentationFrom = fromThis.documentationFrom; } else { - _documentationFrom = this; + _documentationFrom = [this]; } } return _documentationFrom; @@ -1963,7 +2273,7 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { if (config.dropTextFrom.contains(element.library.name)) { _rawDocs = ''; } else { - _rawDocs = _computeDocumentationComment ?? ''; + _rawDocs = computeDocumentationComment ?? ''; _rawDocs = stripComments(_rawDocs) ?? ''; _rawDocs = _injectExamples(_rawDocs); _rawDocs = _stripMacroTemplatesAndAddToIndex(_rawDocs); @@ -1974,7 +2284,9 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { /// Returns the docs, stripped of their leading comments syntax. @override - String get documentation => documentationFrom._documentationLocal; + String get documentation { + return documentationFrom.map((e) => e._documentationLocal).join('\n'); + } Library get definingLibrary => package.findOrCreateLibraryFor(element); @@ -2288,12 +2600,12 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { } @override - void warn(PackageWarning kind, {String message, Locatable referredFrom}) { + void warn(PackageWarning kind, {String message, List referredFrom}) { package.warnOnElement(this, kind, message: message, referredFrom: referredFrom); } - String get _computeDocumentationComment => element.documentationComment; + String get computeDocumentationComment => element.documentationComment; Documentation get _documentation { if (__documentation != null) return __documentation; @@ -2818,14 +3130,14 @@ Map> packageWarningText = { /// Something that package warnings can be called on. abstract class Warnable implements Locatable { - void warn(PackageWarning warning, {String message, Locatable referredFrom}); + void warn(PackageWarning warning, {String message, List referredFrom}); } /// Something that can be located for warning purposes. abstract class Locatable { String get fullyQualifiedName; String get href; - Locatable get documentationFrom; + List get documentationFrom; Element get element; String get elementLocation; Tuple2 get lineAndColumn; @@ -2958,7 +3270,7 @@ class Package implements Nameable, Documentable { final PackageWarningOptions _packageWarningOptions; PackageWarningCounter _packageWarningCounter; // All ModelElements constructed for this package; a superset of allModelElements. - final Map, ModelElement> + final Map, ModelElement> _allConstructedModelElements = new Map(); // Anything that might be inheritable, place here for later lookup. @@ -2977,7 +3289,7 @@ class Package implements Nameable, Documentable { Documentable get overriddenDocumentedElement => this; @override - Documentable get documentationFrom => this; + List get documentationFrom => [this]; @override bool get hasExtendedDocumentation => documentation.isNotEmpty; @@ -3030,7 +3342,7 @@ class Package implements Nameable, Documentable { PackageWarningCounter get packageWarningCounter => _packageWarningCounter; @override - void warn(PackageWarning kind, {String message, Locatable referredFrom}) { + void warn(PackageWarning kind, {String message, List referredFrom}) { warnOnElement(this, kind, message: message, referredFrom: referredFrom); } @@ -3046,7 +3358,7 @@ class Package implements Nameable, Documentable { } void warnOnElement(Warnable warnable, PackageWarning kind, - {String message, Locatable referredFrom}) { + {String message, List referredFrom}) { if (warnable != null) { // This sort of warning is only applicable to top level elements. if (kind == PackageWarning.ambiguousReexport) { @@ -3079,7 +3391,6 @@ class Package implements Nameable, Documentable { // the user, yet we still want IntelliJ to link properly. Tuple2 warnableStrings = nameAndLocation(warnable); String warnablePrefix = 'from'; - Tuple2 referredFromStrings = nameAndLocation(referredFrom); String referredFromPrefix = 'referred to by'; String name = warnableStrings.item1; String warningMessage; @@ -3143,9 +3454,14 @@ class Package implements Nameable, Documentable { if (warnable != null) messageParts.add( "${warnablePrefix} ${warnableStrings.item1}: ${warnableStrings.item2}"); - if (referredFrom != null && referredFrom != warnable) { - messageParts.add( - "${referredFromPrefix} ${referredFromStrings.item1}: ${referredFromStrings.item2}"); + if (referredFrom != null) { + for (Locatable referral in referredFrom) { + if (referral != warnable) { + Tuple2 referredFromStrings = nameAndLocation(referral); + messageParts.add( + "${referredFromPrefix} ${referredFromStrings.item1}: ${referredFromStrings.item2}"); + } + } } String fullMessage; if (messageParts.length <= 2) { @@ -3756,7 +4072,12 @@ abstract class SourceCodeMixin { class TopLevelVariable extends ModelElement with GetterSetterCombo implements EnclosedElement { - TopLevelVariable(TopLevelVariableElement element, Library library) + @override + final Accessor getter; + @override + final Accessor setter; + + TopLevelVariable(TopLevelVariableElement element, Library library, this.getter, this.setter) : super(element, library) { if (hasGetter) { var t = _variable.getter.returnType; @@ -3772,8 +4093,13 @@ class TopLevelVariable extends ModelElement new ModelElement.from( s.element, package.findOrCreateLibraryFor(s.element))); } + if (getter != null) getter.enclosingCombo = this; + if (setter != null) setter.enclosingCombo = this; } + @override + bool get isInherited => false; + String get constantValue { var v = _variable.computeNode().toSource(); if (v == null) return ''; @@ -3834,7 +4160,7 @@ class TopLevelVariable extends ModelElement } @override - String get _computeDocumentationComment { + String get computeDocumentationComment { String docs = getterSetterDocumentationComment; if (docs.isEmpty) return _variable.documentationComment; return docs; @@ -3842,11 +4168,12 @@ class TopLevelVariable extends ModelElement String get _fileName => isConst ? '$name-constant.html' : '$name.html'; - @override + /*@override PropertyAccessorElement get _getter => _variable.getter; @override PropertyAccessorElement get _setter => _variable.setter; + */ TopLevelVariableElement get _variable => (element as TopLevelVariableElement); } diff --git a/lib/src/model_utils.dart b/lib/src/model_utils.dart index 897b2c31cb..d474ff7204 100644 --- a/lib/src/model_utils.dart +++ b/lib/src/model_utils.dart @@ -58,11 +58,13 @@ bool isPrivate(Element e) => (e.identifier == 'dart:_internal' || e.identifier == 'dart:nativewrappers')); +// TODO(jcollins-g): this does not take into account inheritance +// FIXME(jcollins-g): reimplement @nodoc somewhere else bool isPublic(Element e) { if (isPrivate(e)) return false; // check to see if element is part of the public api, that is it does not // have a '' or '@nodoc' in the documentation comment - if (e is PropertyAccessorElement && e.isSynthetic) { + /*if (e is PropertyAccessorElement && e.isSynthetic) { var accessor = (e as PropertyAccessorElement); if (accessor.correspondingSetter != null && !accessor.correspondingSetter.isSynthetic) { @@ -79,6 +81,7 @@ bool isPublic(Element e) { if (docComment != null && (docComment.contains('') || docComment.contains('@nodoc'))) return false; + */ return true; } From dc43f6e0322f9d629ca85d4086c9aef46b9aa72a Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 13 Jun 2017 10:24:09 -0700 Subject: [PATCH 03/25] intermediate state: debugging angular --- lib/src/model.dart | 52 ++++++++++++++++++++++++++++++++++++---- lib/src/model_utils.dart | 3 +-- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 6064c8e5cd..9c638e9297 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -261,8 +261,9 @@ class Accessor extends ModelElement @override String get href { - if (canonicalLibrary == null) return null; - return '${canonicalLibrary.dirName}/${_accessor.enclosingElement.name}/${name}.html'; + return enclosingCombo.href; + //if (canonicalLibrary == null) return null; + //return '${canonicalLibrary.dirName}/${_accessor.enclosingElement.name}/${name}.html'; } bool get isGetter => _accessor.isGetter; @@ -1046,6 +1047,7 @@ abstract class Documentable implements Warnable { class Dynamic extends ModelElement { Dynamic(Element element, Library library) : super(element, library); + @override ModelElement get enclosingElement => throw new UnsupportedError(''); @override @@ -1379,7 +1381,18 @@ class Field extends ModelElement if (readOnly && !isFinal && !isConst) all_features.add('read-only'); if (writeOnly) all_features.add('write-only'); if (readWrite) all_features.add('read / write'); - if (isInherited) all_features.add('inherited'); + if (getter != null && setter != null) { + if (getter.isInherited && setter.isInherited) { + all_features.add('inherited'); + } else { + if (getter.isInherited) + all_features.add('inherited-getter'); + if (setter.isInherited) + all_features.add('inherited-setter'); + } + } else { + if (isInherited) all_features.add('inherited'); + } return all_features; } @@ -1425,6 +1438,7 @@ abstract class GetterSetterCombo implements ModelElement { : new ModelElement.from(_getter, library, enclosingCombo: this, enclosingClass: isInherited ? enclosingElement : null); }*/ + @override ModelElement enclosingElement; bool get isInherited; @@ -1600,6 +1614,7 @@ class Library extends ModelElement { String get dirName => name.replaceAll(':', '-'); /// Libraries are not enclosed by anything. + @override ModelElement get enclosingElement => null; List get enums { @@ -2130,6 +2145,10 @@ abstract class ModelElement implements Comparable, Nameable, Documentable { newModelElement = new Method.inherited(e, enclosingClass, library); } if (e is TopLevelVariableElement) { + if (getter == null && setter == null) { + // FIXME: so, what's wrong here? + 1+1; + } assert(getter != null || setter != null); newModelElement = new TopLevelVariable(e, library, getter, setter); } @@ -3131,6 +3150,7 @@ Map> packageWarningText = { /// Something that package warnings can be called on. abstract class Warnable implements Locatable { void warn(PackageWarning warning, {String message, List referredFrom}); + Warnable get enclosingElement; } /// Something that can be located for warning purposes. @@ -3291,6 +3311,9 @@ class Package implements Nameable, Documentable { @override List get documentationFrom => [this]; + @override + Warnable get enclosingElement => null; + @override bool get hasExtendedDocumentation => documentation.isNotEmpty; @@ -3362,12 +3385,19 @@ class Package implements Nameable, Documentable { if (warnable != null) { // This sort of warning is only applicable to top level elements. if (kind == PackageWarning.ambiguousReexport) { + if (warnable is TypeParameter) + 1+1; + while (warnable.enclosingElement is! Library && warnable.enclosingElement != null) { + warnable = warnable.enclosingElement; + } + /* Element topLevelElement = warnable.element; while (topLevelElement.enclosingElement is! CompilationUnitElement) { topLevelElement = topLevelElement.enclosingElement; } warnable = new ModelElement.from( topLevelElement, findOrCreateLibraryFor(topLevelElement)); + */ } if (warnable is Accessor) { // This might be part of a Field, if so, assign this warning to the field @@ -3743,6 +3773,8 @@ class Package implements Nameable, Documentable { lib = findCanonicalLibraryFor(preferredClass.element); } ModelElement modelElement; + // TODO(jcollins-g): Special cases are pretty large here. Refactor to split + // out into helpers. // TODO(jcollins-g): The data structures should be changed to eliminate guesswork // with member elements. if (e is ClassMemberElement || e is PropertyAccessorElement) { @@ -3805,7 +3837,16 @@ class Package implements Nameable, Documentable { assert(matches.length <= 1); if (!matches.isEmpty) modelElement = matches.first; } else { - if (lib != null) modelElement = new ModelElement.from(e, lib); + if (lib != null) { + Accessor getter; + Accessor setter; + if (e is PropertyInducingElement) { + if (e.getter != null) getter = new ModelElement.from(e.getter, lib); + if (e.setter != null) setter = new ModelElement.from(e.setter, lib); + 1+1; + } + modelElement = new ModelElement.from(e, lib, getter: getter, setter: setter); + } assert(modelElement is! Inheritable); if (modelElement != null && !modelElement.isCanonical) { modelElement = null; @@ -4238,6 +4279,9 @@ class TypeParameter extends ModelElement { _modelType = new ElementType(_typeParameter.type, this); } + @override + ModelElement get enclosingElement => new ModelElement.from(element.enclosingElement, library); + @override String get href { if (canonicalLibrary == null) return null; diff --git a/lib/src/model_utils.dart b/lib/src/model_utils.dart index d474ff7204..8565e65151 100644 --- a/lib/src/model_utils.dart +++ b/lib/src/model_utils.dart @@ -64,7 +64,7 @@ bool isPublic(Element e) { if (isPrivate(e)) return false; // check to see if element is part of the public api, that is it does not // have a '' or '@nodoc' in the documentation comment - /*if (e is PropertyAccessorElement && e.isSynthetic) { + if (e is PropertyAccessorElement && e.isSynthetic) { var accessor = (e as PropertyAccessorElement); if (accessor.correspondingSetter != null && !accessor.correspondingSetter.isSynthetic) { @@ -81,7 +81,6 @@ bool isPublic(Element e) { if (docComment != null && (docComment.contains('') || docComment.contains('@nodoc'))) return false; - */ return true; } From 1f47da9f28467a57e0d6a77c13c6d22f9cdc1407 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 13 Jun 2017 14:29:18 -0700 Subject: [PATCH 04/25] intermediate state. things more or less work but are messy. --- lib/src/model.dart | 19 +++++++++++++++++-- lib/src/model_utils.dart | 1 - 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 9b64423d6a..1c9c607498 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -2295,10 +2295,25 @@ abstract class ModelElement extends Nameable if (e is TopLevelVariableElement) { if (getter == null && setter == null) { // FIXME: so, what's wrong here? - 1+1; + if (e.getter != null) { + getter = new ModelElement.from(e.getter, library); + if (getter.enclosingCombo != null) { + newModelElement = getter.enclosingCombo; + } + } + if (e.setter != null) { + setter = new ModelElement.from(e.setter, library); + if (setter.enclosingCombo != null) { + if (newModelElement != null) { + assert(setter.enclosingCombo == getter.enclosingCombo); + } + newModelElement = setter.enclosingCombo; + } + } } assert(getter != null || setter != null); - newModelElement = new TopLevelVariable(e, library, getter, setter); + if (newModelElement == null) + newModelElement = new TopLevelVariable(e, library, getter, setter); } if (e is PropertyAccessorElement) { if (e.enclosingElement is ClassElement) { diff --git a/lib/src/model_utils.dart b/lib/src/model_utils.dart index 57bc01798b..916b47610a 100644 --- a/lib/src/model_utils.dart +++ b/lib/src/model_utils.dart @@ -71,7 +71,6 @@ bool isPrivate(Element e) { } // TODO(jcollins-g): this does not take into account inheritance -// FIXME(jcollins-g): reimplement @nodoc somewhere else bool isPublic(Element e) { if (isPrivate(e)) return false; // check to see if element is part of the public api, that is it does not From b85e25d172199bd94e3cdea593f34a079b2a79c0 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 14 Jun 2017 09:26:30 -0700 Subject: [PATCH 05/25] Screenshot worthy. --- lib/src/model.dart | 110 ++++++++++++++++++++++++++++----------------- 1 file changed, 70 insertions(+), 40 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 1c9c607498..e0b5dc8355 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -1227,6 +1227,8 @@ class Field extends ModelElement @override String get documentation { + if (name == 'darkColor' && enclosingElement.name == 'FlutterLogoDecoration') + 1+1; // Verify that hasSetter and hasGetterNoSetter are mutually exclusive, // to prevent displaying more or less than one summary. Set assertCheck = new Set()..addAll([hasSetter, hasGetterNoSetter]); @@ -1407,8 +1409,7 @@ class Field extends ModelElement @override String get computeDocumentationComment { - if (name == 'hashCode' && enclosingElement.name == 'FlutterLogoDecoration') - 1+1; + String docs = getterSetterDocumentationComment; if (docs.isEmpty) return _field.documentationComment; return docs; @@ -1451,21 +1452,39 @@ abstract class GetterSetterCombo implements ModelElement { ModelElement enclosingElement; bool get isInherited; + + /// Returns true if at least one accessor is synthetic. + bool get hasSyntheticAccessors { + if ((getter != null && getter.element.isSynthetic) || + (setter != null && setter.element.isSynthetic)) { + // No half-synthetic combos here, please. + assert(getter == null || getter.element.isSynthetic); + assert(setter == null || setter.element.isSynthetic); + return true; + } + return false; + } + @override List get documentationFrom { if (_documentationFrom == null) { _documentationFrom = []; - if (getter != null) _documentationFrom.addAll(getter.documentationFrom); - if (setter != null) _documentationFrom.addAll(setter.documentationFrom); - if (_documentationFrom.length > 1) { - int c = 0; - for (ModelElement e in _documentationFrom) { - if (e.documentation != '') { - c += 1; + if (hasSyntheticAccessors) { + // TODO(jcollins-g): untangle when mixins can use super + _documentationFrom = computeDocumentationFrom; + } else { + if (getter != null) _documentationFrom.addAll(getter.documentationFrom); + if (setter != null) _documentationFrom.addAll(setter.documentationFrom); + if (_documentationFrom.length > 1) { + int c = 0; + for (ModelElement e in _documentationFrom) { + if (e.documentation != '') { + c += 1; + } } + if (c == _documentationFrom.length) + 1+1; } - if (c == _documentationFrom.length) - 1+1; } } return _documentationFrom; @@ -1475,19 +1494,23 @@ abstract class GetterSetterCombo implements ModelElement { @override String get oneLineDoc { if (_oneLineDoc == null) { - StringBuffer buffer = new StringBuffer(); - bool addGetterSetterText = false; - if (getter != null && getter.oneLineDoc.isNotEmpty && - setter != null && setter.oneLineDoc.isNotEmpty) { - addGetterSetterText = true; - } - if (getter != null) { - buffer.writeln('${addGetterSetterText ? "On read: ": ""}${getter.oneLineDoc}'); - } - if (setter != null) { - buffer.writeln('${addGetterSetterText ? "On write: ": ""}${setter.oneLineDoc}'); + if (hasSyntheticAccessors) { + _oneLineDoc = _documentation.asOneLiner; + } else { + StringBuffer buffer = new StringBuffer(); + bool addGetterSetterText = false; + if (getter != null && getter.oneLineDoc.isNotEmpty && + setter != null && setter.oneLineDoc.isNotEmpty) { + addGetterSetterText = true; + } + if (getter != null) { + buffer.writeln('${addGetterSetterText ? "On read: ": ""}${getter.oneLineDoc}'); + } + if (setter != null) { + buffer.writeln('${addGetterSetterText ? "On write: ": ""}${setter.oneLineDoc}'); + } + _oneLineDoc = buffer.toString(); } - _oneLineDoc = buffer.toString(); } return _oneLineDoc; } @@ -1901,7 +1924,7 @@ class Library extends ModelElement { } }); _variables = []; - for (TopLevelVariableElement element in elements) { + for (TopLevelVariableElement element in elements.where(isPublic)) { Accessor getter; if (element.getter != null) getter = new ModelElement.from(element.getter, this); @@ -2478,6 +2501,15 @@ abstract class ModelElement extends Nameable element is ExecutableElement || element is FunctionTypeAliasElement; List _documentationFrom; + // TODO(jcollins-g): untangle when mixins can call super + @override + List get documentationFrom { + if (_documentationFrom == null) { + _documentationFrom = computeDocumentationFrom; + } + return _documentationFrom; + } + /// Returns the ModelElement(s) from which we will get documentation. /// Can be more than one if this is a Field composing documentation from /// multiple Accessors. @@ -2485,23 +2517,21 @@ abstract class ModelElement extends Nameable /// This getter will walk up the inheritance hierarchy /// to find docs, if the current class doesn't have docs /// for this element. - @override - List get documentationFrom { - if (_documentationFrom == null) { - if (computeDocumentationComment == null && - canOverride() && - overriddenElement != null) { - _documentationFrom = [overriddenElement]; - } else if (this is Inheritable && (this as Inheritable).isInherited) { - Inheritable thisInheritable = (this as Inheritable); - ModelElement fromThis = new ModelElement.from( - element, thisInheritable.definingEnclosingElement.library); - _documentationFrom = fromThis.documentationFrom; - } else { - _documentationFrom = [this]; - } + List get computeDocumentationFrom { + List docFrom; + if (computeDocumentationComment == null && + canOverride() && + overriddenElement != null) { + docFrom = [overriddenElement]; + } else if (this is Inheritable && (this as Inheritable).isInherited) { + Inheritable thisInheritable = (this as Inheritable); + ModelElement fromThis = new ModelElement.from( + element, thisInheritable.definingEnclosingElement.library); + docFrom = fromThis.documentationFrom; + } else { + docFrom = [this]; } - return _documentationFrom; + return docFrom; } String get _documentationLocal { From aeb44fb618eed5720366d8a6fc6335dec0a40bd6 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Fri, 16 Jun 2017 12:14:04 -0700 Subject: [PATCH 06/25] tests closer now --- lib/src/model.dart | 187 ++++++++++++++++++++++++++++++++------------- 1 file changed, 133 insertions(+), 54 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index e0b5dc8355..dd06feac40 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -199,14 +199,9 @@ class InheritableAccessor extends Accessor with Inheritable { bool get isInherited => _isInherited; @override - ModelElement get enclosingElement { + Class get enclosingElement { if (_enclosingElement == null) { - if (_accessor.enclosingElement is CompilationUnitElement) { - _enclosingElement = package - .findOrCreateLibraryFor(_accessor.enclosingElement.enclosingElement); - } else { - _enclosingElement = new ModelElement.from(_accessor.enclosingElement, library); - } + _enclosingElement = super.enclosingElement; } return _enclosingElement; } @@ -228,7 +223,7 @@ class Accessor extends ModelElement /// Call exactly once to set the enclosing combo for this Accessor. void set enclosingCombo(GetterSetterCombo combo) { - assert(_enclosingCombo == null); + assert(_enclosingCombo == null || combo == _enclosingCombo); _enclosingCombo = combo; } @@ -784,7 +779,7 @@ class Class extends ModelElement implements EnclosedElement { List get instanceProperties { if (_instanceFields != null) return _instanceFields; _instanceFields = _allFields - .where((f) => !f.isStatic) + .where((f) => !f.isStatic && !f.isInherited) .toList(growable: false) ..sort(byName); @@ -813,6 +808,9 @@ class Class extends ModelElement implements EnclosedElement { return _cls.allSupertypes.any(_doCheck); } + /// Returns true if [other] is a parent class or mixin for this class. + bool isInheritingFrom(Class other) => superChain.map((et) => (et.element as Class)).contains(other); + @override String get kind => 'class'; @@ -915,6 +913,8 @@ class Class extends ModelElement implements EnclosedElement { List get _allFields { if (_fields != null) return _fields; + if (this.name == 'AnimateElement') + 1+1; _fields = []; Map cmap = library.inheritanceManager.getMembersInheritedFromClasses(element); @@ -922,28 +922,94 @@ class Class extends ModelElement implements EnclosedElement { library.inheritanceManager.getMembersInheritedFromInterfaces(element); Set inheritedAccessors = new Set(); - inheritedAccessors.addAll(cmap.values.where((e) => e is PropertyAccessorElement)); - inheritedAccessors.addAll(imap.values.where((e) => e is PropertyAccessorElement)); - + inheritedAccessors.addAll(cmap.values.where((e) => e is PropertyAccessorElement) + .map((e) => Package.getBasestElement(e) as PropertyAccessorElement).where(isPublic)); + + // Interfaces are subordinate to members inherited from classes, so don't + // add this to our accessor set if we already have something inherited from classes. + inheritedAccessors.addAll(imap.values.where((e) => e is PropertyAccessorElement && !cmap.containsKey(e.name)) + .map((e) => Package.getBasestElement(e) as PropertyAccessorElement).where(isPublic)); + + assert(!inheritedAccessors.any((e) => e is Member)); + // This structure keeps track of inherited accessors. + Map> accessorMap = new Map(); + for (PropertyAccessorElement accessorElement in inheritedAccessors) { + String name = accessorElement.name.replaceFirst('=', ''); + accessorMap.putIfAbsent(name, () => []); + accessorMap[name].add(accessorElement); + } + + // For half-inherited fields, the analyzer only links the non-inherited + // to the [FieldElement]. Compose our [Field] class by hand by looking up + // inherited accessors that may be related. for (FieldElement f in _cls.fields.where(isPublic)) { PropertyAccessorElement getterElement = f.getter; + if (getterElement == null && accessorMap.containsKey(f.name)) { + getterElement = accessorMap[f.name].firstWhere((e) => e.isGetter, orElse: () => null); + } PropertyAccessorElement setterElement = f.setter; - Accessor getter = new InheritableAccessor.from(getterElement, inheritedAccessors, this); - Accessor setter = new InheritableAccessor.from(setterElement, inheritedAccessors, this); - if ((getter != null && getter.isInherited) || (setter != null && setter.isInherited)) { - // Field is >=50% inherited. - // TODO(jcollins-g): make half-inherited fields half-italicized...? - _fields.add(new ModelElement.from(f, library, enclosingClass: this, - getter: getter, setter: setter)); - } else { - _fields.add(new ModelElement.from(f, library, - getter: getter, setter: setter)); + if (setterElement == null && accessorMap.containsKey(f.name)) { + setterElement = accessorMap[f.name].firstWhere((e) => e.isSetter, orElse: () => null); } + _addSingleField(getterElement, setterElement, inheritedAccessors, f); + accessorMap.remove(f.name); } + + // Now we only have inherited accessors who aren't associated with + // anything in cls._fields. + for (String fieldName in accessorMap.keys) { + List elements = accessorMap[fieldName].map((e) => Package.getBasestElement(e)).toList(); + PropertyAccessorElement getterElement = elements.firstWhere((e) => e.isGetter, orElse: () => null); + PropertyAccessorElement setterElement = elements.firstWhere((e) => e.isSetter, orElse: () => null); + _addSingleField(getterElement, setterElement, inheritedAccessors); + } + _fields.sort(byName); return _fields; } + /// Add a single Field to _fields. + /// + /// Modifies accessorMap, removing the field added. + void _addSingleField(PropertyAccessorElement getterElement, + PropertyAccessorElement setterElement, + Set inheritedAccessors, + [FieldElement f]) { + Accessor getter = new InheritableAccessor.from(getterElement, inheritedAccessors, this); + Accessor setter = new InheritableAccessor.from(setterElement, inheritedAccessors, this); + assert(!(getter == null && setter == null)); + if (f == null) { + // Pick an appropriate FieldElement to represent this element. + // Only hard when dealing with a synthetic Field. + if (getter != null && setter == null) { + f = Package.getBasestElement(getterElement.variable); + } else if (getter == null && setter != null) { + f = Package.getBasestElement(setterElement.variable); + } else /* getter != null && setter != null */ { + // In cases where a Field is composed of two Accessors defined in + // different places in the inheritance chain, there are two FieldElements + // for this single Field we're trying to compose. If the setter + // is in a class inheriting from the getter's class, pick the setter's FieldElement -- + // otherwise, pick the getter. + if ((setter.enclosingElement as Class).isInheritingFrom(getter.enclosingElement)) { + f = Package.getBasestElement(setterElement.variable); + } else { + f = Package.getBasestElement(getterElement.variable); + } + } + } + if ((getter == null || getter.isInherited) && (setter == null || setter.isInherited)) { + // Field is 100% inherited. + _fields.add(new ModelElement.from(f, library, enclosingClass: this, + getter: getter, setter: setter)); + } else { + // Field is <100% inherited (could be half-inherited). + // TODO(jcollins-g): make half-inherited fields half-italicized...? + _fields.add(new ModelElement.from(f, library, + getter: getter, setter: setter)); + } + } + /* List get _allFields { if (_fields != null) return _fields; @@ -1219,6 +1285,7 @@ class Field extends ModelElement factory Field.inherited(FieldElement element, Class enclosingClass, Library library, Accessor getter, Accessor setter) { Field newField = new Field(element, library, getter, setter); newField._isInherited = true; + newField._enclosingClass = enclosingClass; // Can't set _isInherited to true if this is the defining element, because // that would mean it isn't inherited. assert(newField.enclosingElement != newField.definingEnclosingElement); @@ -1409,7 +1476,6 @@ class Field extends ModelElement @override String get computeDocumentationComment { - String docs = getterSetterDocumentationComment; if (docs.isEmpty) return _field.documentationComment; return docs; @@ -1453,13 +1519,10 @@ abstract class GetterSetterCombo implements ModelElement { bool get isInherited; - /// Returns true if at least one accessor is synthetic. + /// Returns true if both accessors are synthetic. bool get hasSyntheticAccessors { if ((getter != null && getter.element.isSynthetic) || (setter != null && setter.element.isSynthetic)) { - // No half-synthetic combos here, please. - assert(getter == null || getter.element.isSynthetic); - assert(setter == null || setter.element.isSynthetic); return true; } return false; @@ -1469,23 +1532,10 @@ abstract class GetterSetterCombo implements ModelElement { List get documentationFrom { if (_documentationFrom == null) { _documentationFrom = []; - if (hasSyntheticAccessors) { - // TODO(jcollins-g): untangle when mixins can use super + if (getter != null) _documentationFrom.addAll(getter.documentationFrom); + if (setter != null) _documentationFrom.addAll(setter.documentationFrom); + if (_documentationFrom.length == 0 || _documentationFrom.every((e) => e.documentation == '')) _documentationFrom = computeDocumentationFrom; - } else { - if (getter != null) _documentationFrom.addAll(getter.documentationFrom); - if (setter != null) _documentationFrom.addAll(setter.documentationFrom); - if (_documentationFrom.length > 1) { - int c = 0; - for (ModelElement e in _documentationFrom) { - if (e.documentation != '') { - c += 1; - } - } - if (c == _documentationFrom.length) - 1+1; - } - } } return _documentationFrom; } @@ -1494,20 +1544,22 @@ abstract class GetterSetterCombo implements ModelElement { @override String get oneLineDoc { if (_oneLineDoc == null) { - if (hasSyntheticAccessors) { + bool hasAccessorsWithDocs = (getter != null && getter.oneLineDoc.isNotEmpty || + setter != null && setter.oneLineDoc.isNotEmpty); + if (!hasAccessorsWithDocs) { _oneLineDoc = _documentation.asOneLiner; } else { StringBuffer buffer = new StringBuffer(); - bool addGetterSetterText = false; + bool getterSetterBothAvailable = false; if (getter != null && getter.oneLineDoc.isNotEmpty && setter != null && setter.oneLineDoc.isNotEmpty) { - addGetterSetterText = true; + getterSetterBothAvailable = true; } - if (getter != null) { - buffer.writeln('${addGetterSetterText ? "On read: ": ""}${getter.oneLineDoc}'); + if (getter != null && getter.oneLineDoc.isNotEmpty) { + buffer.write('${getter.oneLineDoc}'); } - if (setter != null) { - buffer.writeln('${addGetterSetterText ? "On write: ": ""}${setter.oneLineDoc}'); + if (setter != null && setter.oneLineDoc.isNotEmpty) { + buffer.write('${getterSetterBothAvailable ? "": setter.oneLineDoc}'); } _oneLineDoc = buffer.toString(); } @@ -1641,8 +1693,18 @@ class Library extends ModelElement { Iterable get allOriginalModelElementNames { if (_allOriginalModelElementNames == null) { _allOriginalModelElementNames = allModelElements.map((e) { + Accessor getter; + Accessor setter; + if (e is GetterSetterCombo) { + if (e.getter != null) { + getter = new ModelElement.from(e.getter.element, package.findOrCreateLibraryFor(e.getter.element)); + } + if (e.setter != null) { + setter = new ModelElement.from(e.setter.element, package.findOrCreateLibraryFor(e.setter.element)); + } + } return new ModelElement.from( - e.element, package.findOrCreateLibraryFor(e.element)) + e.element, package.findOrCreateLibraryFor(e.element), getter: getter, setter: setter) .fullyQualifiedName; }).toList(); } @@ -2291,6 +2353,8 @@ abstract class ModelElement extends Nameable if (e.enclosingElement.isEnum) { newModelElement = new EnumField(e, library, getter, setter); } else { + if (getter == null && setter == null) + 1+1; assert(getter != null || setter != null); newModelElement = new Field(e, library, getter, setter); } @@ -2525,8 +2589,19 @@ abstract class ModelElement extends Nameable docFrom = [overriddenElement]; } else if (this is Inheritable && (this as Inheritable).isInherited) { Inheritable thisInheritable = (this as Inheritable); + InheritableAccessor newGetter; + InheritableAccessor newSetter; + if (this is GetterSetterCombo) { + GetterSetterCombo thisAsCombo = this as GetterSetterCombo; + if (thisAsCombo.getter != null) { + newGetter = new ModelElement.from(thisAsCombo.getter.element, thisAsCombo.getter.definingLibrary); + } + if (thisAsCombo.setter != null) { + newSetter = new ModelElement.from(thisAsCombo.setter.element, thisAsCombo.setter.definingLibrary); + } + } ModelElement fromThis = new ModelElement.from( - element, thisInheritable.definingEnclosingElement.library); + element, thisInheritable.definingEnclosingElement.library, getter:newGetter, setter:newSetter); docFrom = fromThis.documentationFrom; } else { docFrom = [this]; @@ -4109,8 +4184,8 @@ class Package extends Nameable implements Documentable { } // TODO(jcollins-g): Revise when dart-lang/sdk#29600 is fixed. - static Element getBasestElement(Member member) { - Element element = member; + static Element getBasestElement(Element possibleMember) { + Element element = possibleMember; while (element is Member) { element = (element as Member).baseElement; } @@ -4510,6 +4585,8 @@ class TopLevelVariable extends ModelElement String get documentation { // Verify that hasSetter and hasGetterNoSetter are mutually exclusive, // to prevent displaying more or less than one summary. + if (name == 'incorrectDocReferenceFromEx') + 1+1; Set assertCheck = new Set()..addAll([hasSetter, hasGetterNoSetter]); assert(assertCheck.containsAll([true, false])); return super.documentation; @@ -4558,6 +4635,8 @@ class TopLevelVariable extends ModelElement @override String get computeDocumentationComment { String docs = getterSetterDocumentationComment; + if (name == 'incorrectDocReferenceFromEx') + 1+1; if (docs.isEmpty) return _variable.documentationComment; return docs; } From 9396b0c2292dce53297c5c462c0b8012c454b0fd Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Fri, 16 Jun 2017 13:21:40 -0700 Subject: [PATCH 07/25] Tests pass! --- lib/src/html/html_generator_instance.dart | 4 ++++ lib/src/markdown_processor.dart | 9 ++++++++- lib/src/model.dart | 8 ++++---- testing/test_package_docs/ex/B-class.html | 4 ++-- .../fake/ClassWithUnusualProperties-class.html | 4 ++-- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/src/html/html_generator_instance.dart b/lib/src/html/html_generator_instance.dart index 566a3bd29a..fbc7a1d0e6 100644 --- a/lib/src/html/html_generator_instance.dart +++ b/lib/src/html/html_generator_instance.dart @@ -104,6 +104,8 @@ class HtmlGeneratorInstance implements HtmlOptions { //if (clazz.name != 'FlutterLogoDecoration') continue; if (!clazz.isCanonical) continue; + if (clazz.name == 'ConstantCat') + 1+1; generateClass(package, lib, clazz); for (var constructor in clazz.constructors) { @@ -122,6 +124,8 @@ class HtmlGeneratorInstance implements HtmlOptions { } for (var property in clazz.propertiesForPages) { + if (property.name == 'hashCode') + 1+1; if (!property.isCanonical) continue; generateProperty(package, lib, clazz, property); } diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index 794bb57d61..aaf60acf23 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -292,7 +292,14 @@ MatchingLinkResult _getMatchingLinkElement( if (refModelElement != null) { return new MatchingLinkResult(refModelElement, null); } - refModelElement = new ModelElement.from(searchElement, refLibrary); + if (searchElement is ClassMemberElement) { + if (codeRef == 'Map.keys') + 1+1; + Class refClass = new ModelElement.from(searchElement.enclosingElement, refLibrary); + refModelElement = refClass.allModelElements.firstWhere((e) => e.element == searchElement); + } else { + refModelElement = new ModelElement.from(searchElement, refLibrary); + } if (!refModelElement.isCanonical) { refModelElement.warn(PackageWarning.noCanonicalFound, referredFrom: [element]); diff --git a/lib/src/model.dart b/lib/src/model.dart index dd06feac40..827b6a21ff 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -360,7 +360,6 @@ class Class extends ModelElement implements EnclosedElement { List _instanceFields; List _inheritedProperties; List _allInstanceProperties; - final Set _genPageProperties = new Set(); Class(ClassElement element, Library library) : super(element, library) { Package p = library.package; @@ -783,7 +782,6 @@ class Class extends ModelElement implements EnclosedElement { .toList(growable: false) ..sort(byName); - _genPageProperties.addAll(_instanceFields); return _instanceFields; } @@ -854,8 +852,10 @@ class Class extends ModelElement implements EnclosedElement { // now, we're creating a flat list. We're not paying attention to where // these methods are actually coming from. This might turn out to be a // problem if we want to show that info later. - List get propertiesForPages => - _genPageProperties.toList(growable: false); + List get propertiesForPages { + return []..addAll( + instanceProperties)..addAll(inheritedProperties)..sort(byName); + } List get staticMethods { if (_staticMethods != null) return _staticMethods; diff --git a/testing/test_package_docs/ex/B-class.html b/testing/test_package_docs/ex/B-class.html index 14d9355a2e..d86f78e292 100644 --- a/testing/test_package_docs/ex/B-class.html +++ b/testing/test_package_docs/ex/B-class.html @@ -174,8 +174,8 @@

Properties

→ String
- -
@override, read-only
+ The getter for s +
@override, inherited-setter, read-only
fieldWithTypedef diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties-class.html b/testing/test_package_docs/fake/ClassWithUnusualProperties-class.html index 505a1f686d..82e3d0dafc 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties-class.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties-class.html @@ -168,7 +168,7 @@

Properties

-
@override, read-only
+
@override, inherited-setter, read-only
explicitGetterSetter @@ -203,7 +203,7 @@

Properties

-
@override, write-only
+
@override, inherited-getter, write-only
implicitReadWrite From affded163781f8bd83c63d71973375f083276a09 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Fri, 16 Jun 2017 14:14:01 -0700 Subject: [PATCH 08/25] fix up element search --- lib/src/markdown_processor.dart | 2 +- lib/src/model.dart | 27 ++++++++++++++++----------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index aaf60acf23..e6a0a3ed2e 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -296,7 +296,7 @@ MatchingLinkResult _getMatchingLinkElement( if (codeRef == 'Map.keys') 1+1; Class refClass = new ModelElement.from(searchElement.enclosingElement, refLibrary); - refModelElement = refClass.allModelElements.firstWhere((e) => e.element == searchElement); + refModelElement = refClass.findModelElement(searchElement); } else { refModelElement = new ModelElement.from(searchElement, refLibrary); } diff --git a/lib/src/model.dart b/lib/src/model.dart index 827b6a21ff..1ccbd0a3b2 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -452,19 +452,24 @@ class Class extends ModelElement implements EnclosedElement { return _constants; } - final Set _allElements = new Set(); + Map _allElements; + Map get allElements { + if (_allElements == null) { + _allElements = new Map(); + for (ModelElement me in allModelElements) { + assert(!_allElements.containsKey(me.element)); + _allElements[me.element] = me; + } + } + return _allElements; + } - // TODO(jcollins-g): optimize this. bool contains(Element element) { - if (_allElements.isEmpty) { - _allElements.addAll(allInstanceMethods.map((e) => e.element)); - _allElements.addAll(allInstanceProperties.map((e) => e.element)); - _allElements.addAll(allOperators.map((e) => e.element)); - _allElements.addAll(constructors.map((e) => e.element)); - _allElements.addAll(staticMethods.map((e) => e.element)); - _allElements.addAll(staticProperties.map((e) => e.element)); - } - return _allElements.contains(element); + return allElements.containsKey(element); + } + + ModelElement findModelElement(Element element) { + return allElements[element]; } final Set _allModelElements = new Set(); From 0c99c38611f07ff8f490c275de72ae849107cf6b Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Fri, 16 Jun 2017 14:27:29 -0700 Subject: [PATCH 09/25] beginnings of unit tests --- testing/test_package/lib/fake.dart | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 24038205e0..2ff3d69e64 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -182,20 +182,40 @@ typedef void myCoolTypedef(Cool x, bool y); /// Names are actually wrong in this class, but when we extend it, /// they are correct. class ImplicitProperties { + /// Docs for implicitGetterExplicitSetter from ImplicitProperties. String implicitGetterExplicitSetter; + + /// Docs for explicitGetterImplicitSetter from ImplicitProperties. List explicitGetterImplicitSetter; + + /// A simple property to inherit. + int forInheriting; + + /// Explicit getter for inheriting. + int get explicitGetterSetterForInheriting => 12; + + /// Explicit setter for inheriting. + set explicitGetterSetterForInheriting(int foo) {} + + /// A constant integer from ImplicitProperties. + static const int aConstantInteger = 50; } /// Classes with unusual properties? I don't think they exist. +/// +/// Or rather, dartdoc used to think they didn't exist. Check the variations +/// on inheritance and overrides here. class ClassWithUnusualProperties extends ImplicitProperties { @override set implicitGetterExplicitSetter(String x) {} @override + /// Getter doc for explicitGetterImplicitSetter List get explicitGetterImplicitSetter => new List(); myCoolTypedef _aFunction; + /// Getter doc for explicitGetterSetter. myCoolTypedef get explicitGetterSetter { return _aFunction; } @@ -211,6 +231,7 @@ class ClassWithUnusualProperties extends ImplicitProperties { /// Set to [f], and don't warn about [bar] or [baz]. set explicitSetter(f(int bar, Cool baz, List macTruck)) {} + /// This property has some docs, too. final Set finalProperty = new Set(); Map implicitReadWrite; From e139b079586c6925ec42aab19fbe4058c2a124d8 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 19 Jun 2017 15:01:49 -0700 Subject: [PATCH 10/25] Add new tests, and they pass. --- lib/src/model.dart | 48 ++++++++++++------- test/model_test.dart | 28 ++++++++++- testing/test_package/lib/fake.dart | 3 -- testing/test_package_docs/ex/B-class.html | 7 +-- testing/test_package_docs/ex/B/s.html | 12 +++++ .../ClassWithUnusualProperties-class.html | 41 ++++++++++++---- .../ClassWithUnusualProperties.html | 2 + .../ClassWithUnusualProperties/aMethod.html | 2 + .../explicitGetter.html | 2 + .../explicitGetterImplicitSetter.html | 7 ++- .../explicitGetterSetter.html | 7 ++- .../explicitSetter.html | 2 + .../finalProperty.html | 5 ++ .../implicitGetterExplicitSetter.html | 7 ++- .../implicitReadWrite.html | 2 + .../fake/ImplicitProperties-class.html | 24 +++++++++- .../ImplicitProperties.html | 2 + .../explicitGetterImplicitSetter.html | 5 ++ .../fake/ImplicitProperties/hashCode.html | 2 + .../implicitGetterExplicitSetter.html | 5 ++ .../fake/ImplicitProperties/noSuchMethod.html | 2 + .../ImplicitProperties/operator_equals.html | 2 + .../fake/ImplicitProperties/runtimeType.html | 2 + .../fake/ImplicitProperties/toString.html | 2 + testing/test_package_docs/index.json | 22 +++++++++ 25 files changed, 206 insertions(+), 37 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 1ccbd0a3b2..c1cd1ead1f 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -229,7 +229,10 @@ class Accessor extends ModelElement @override String get computeDocumentationComment { - return super.computeDocumentationComment; + if (element.isSynthetic) { + return (element as PropertyAccessorElement).variable.documentationComment; + } + return stripComments(super.computeDocumentationComment); } @override @@ -980,6 +983,12 @@ class Class extends ModelElement implements EnclosedElement { PropertyAccessorElement setterElement, Set inheritedAccessors, [FieldElement f]) { + if (f != null && f.name == 'implicitGetterExplicitSetter') + 1+1; + if (getterElement != null && getterElement.name.contains('implicitGetterExplicitSetter')) + 1+1; + if (setterElement != null && setterElement.name.contains('implicitGetterExplicitSetter')) + 1+1; Accessor getter = new InheritableAccessor.from(getterElement, inheritedAccessors, this); Accessor setter = new InheritableAccessor.from(setterElement, inheritedAccessors, this); assert(!(getter == null && setter == null)); @@ -1400,11 +1409,11 @@ class Field extends ModelElement return _enclosingClass; } - @override - bool get hasGetter => _field.getter != null; + //@override + //bool get hasGetter => _field.getter != null; - @override - bool get hasSetter => _field.setter != null; + //@override + //bool get hasSetter => _field.setter != null; @override String get href { @@ -1500,7 +1509,7 @@ class Field extends ModelElement void _setModelType() { if (hasGetter) { - var t = _field.getter.returnType; + var t = (getter.element as PropertyAccessorElement).returnType; _modelType = new ElementType( t, new ModelElement.from( @@ -1537,8 +1546,11 @@ abstract class GetterSetterCombo implements ModelElement { List get documentationFrom { if (_documentationFrom == null) { _documentationFrom = []; - if (getter != null) _documentationFrom.addAll(getter.documentationFrom); - if (setter != null) _documentationFrom.addAll(setter.documentationFrom); + if (getter != null) { + _documentationFrom.addAll(getter.documentationFrom.where((e) => e.computeDocumentationComment != computeDocumentationComment)); + } + if (setter != null) + _documentationFrom.addAll(setter.documentationFrom.where((e) => e.computeDocumentationComment != computeDocumentationComment)); if (_documentationFrom.length == 0 || _documentationFrom.every((e) => e.documentation == '')) _documentationFrom = computeDocumentationFrom; } @@ -1549,6 +1561,8 @@ abstract class GetterSetterCombo implements ModelElement { @override String get oneLineDoc { if (_oneLineDoc == null) { + if (name == 'implicitGetterExplicitSetter') + 1+1; bool hasAccessorsWithDocs = (getter != null && getter.oneLineDoc.isNotEmpty || setter != null && setter.oneLineDoc.isNotEmpty); if (!hasAccessorsWithDocs) { @@ -1580,13 +1594,13 @@ abstract class GetterSetterCombo implements ModelElement { if (hasGetter && !getter.element.isSynthetic) { assert(getter.documentationFrom.length == 1); - String docs = stripComments(getter.documentationFrom.first.computeDocumentationComment); + String docs = getter.documentationFrom.first.computeDocumentationComment; if (docs != null) buffer.write(docs); } if (hasSetter && !setter.element.isSynthetic) { assert(setter.documentationFrom.length == 1); - String docs = stripComments(setter.documentationFrom.first.computeDocumentationComment); + String docs = setter.documentationFrom.first.computeDocumentationComment; if (docs != null) { if (buffer.isNotEmpty) buffer.write('\n\n'); buffer.write(docs); @@ -1623,11 +1637,11 @@ abstract class GetterSetterCombo implements ModelElement { bool get hasExplicitSetter => hasSetter && !setter.element.isSynthetic; bool get hasImplicitSetter => hasSetter && setter.element.isSynthetic; - bool get hasGetter; + bool get hasGetter => getter != null; bool get hasNoGetterSetter => !hasExplicitGetter && !hasExplicitSetter; - bool get hasSetter; + bool get hasSetter => setter != null; bool get hasGetterNoSetter => (hasGetter && !hasSetter); @@ -2631,6 +2645,8 @@ abstract class ModelElement extends Nameable /// Returns the docs, stripped of their leading comments syntax. @override String get documentation { + if (name == 'implicitGetterExplicitSetter') + 1+1; return documentationFrom.map((e) => e._documentationLocal).join('\n'); } @@ -4600,11 +4616,11 @@ class TopLevelVariable extends ModelElement @override ModelElement get enclosingElement => library; - @override - bool get hasGetter => _variable.getter != null; + //@override + //bool get hasGetter => _variable.getter != null; - @override - bool get hasSetter => _variable.setter != null; + //@override + //bool get hasSetter => _variable.setter != null; @override String get href { diff --git a/test/model_test.dart b/test/model_test.dart index b249f94435..18b043e159 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -1190,11 +1190,12 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, }); group('Field', () { - Class c, LongFirstLine, CatString; + Class c, LongFirstLine, CatString, UnusualProperties; Field f1, f2, constField, dynamicGetter, onlySetter; Field lengthX; Field sFromApple, mFromApple, mInB, autoCompress; Field isEmpty; + Field implicitGetterExplicitSetter, explicitGetterImplicitSetter; setUp(() { c = exLibrary.classes.firstWhere((c) => c.name == 'Apple'); @@ -1204,6 +1205,11 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, LongFirstLine = fakeLibrary.classes.firstWhere((c) => c.name == 'LongFirstLine'); CatString = exLibrary.classes.firstWhere((c) => c.name == 'CatString'); + + UnusualProperties = fakeLibrary.classes.firstWhere((c) => c.name == 'ClassWithUnusualProperties'); + implicitGetterExplicitSetter = UnusualProperties.allModelElements.firstWhere((e) => e.name == 'implicitGetterExplicitSetter'); + explicitGetterImplicitSetter = UnusualProperties.allModelElements.firstWhere((e) => e.name == 'explicitGetterImplicitSetter'); + isEmpty = CatString.allInstanceProperties .firstWhere((p) => p.name == 'isEmpty'); dynamicGetter = LongFirstLine.instanceProperties @@ -1234,6 +1240,26 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, .firstWhere((p) => p.name == 'autoCompress'); }); + test('split inheritance with explicit setter works', () { + expect(implicitGetterExplicitSetter.getter.isInherited, isTrue); + expect(implicitGetterExplicitSetter.setter.isInherited, isFalse); + expect(implicitGetterExplicitSetter.isInherited, isFalse); + expect(implicitGetterExplicitSetter.features.contains('inherited-getter'), isTrue); + expect(implicitGetterExplicitSetter.oneLineDoc, equals('Docs for implicitGetterExplicitSetter from ImplicitProperties.')); + expect(implicitGetterExplicitSetter.documentation, equals('Docs for implicitGetterExplicitSetter from ImplicitProperties.')); + }); + + test('split inheritance with explicit getter works', () { + expect(explicitGetterImplicitSetter.getter.isInherited, isFalse); + expect(explicitGetterImplicitSetter.setter.isInherited, isTrue); + expect(explicitGetterImplicitSetter.isInherited, isFalse); + expect(explicitGetterImplicitSetter.features.contains('inherited-setter'), isTrue); + expect(explicitGetterImplicitSetter.oneLineDoc, equals('Getter doc for explicitGetterImplicitSetter')); + // Even though we have some new setter docs, getter still takes priority. + expect(explicitGetterImplicitSetter.documentation, equals('Docs for explicitGetterImplicitSetter from ImplicitProperties.')); + }); + + test('has a fully qualified name', () { expect(lengthX.fullyQualifiedName, 'fake.WithGetterAndSetter.lengthX'); }); diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 2ff3d69e64..d16a5a7522 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -196,9 +196,6 @@ class ImplicitProperties { /// Explicit setter for inheriting. set explicitGetterSetterForInheriting(int foo) {} - - /// A constant integer from ImplicitProperties. - static const int aConstantInteger = 50; } /// Classes with unusual properties? I don't think they exist. diff --git a/testing/test_package_docs/ex/B-class.html b/testing/test_package_docs/ex/B-class.html index d86f78e292..862b7b766b 100644 --- a/testing/test_package_docs/ex/B-class.html +++ b/testing/test_package_docs/ex/B-class.html @@ -170,12 +170,13 @@

Properties

read / write
- s - → String + s + ↔ String something +
The getter for s -
@override, inherited-setter, read-only
+
@override, inherited-setter, read / write
fieldWithTypedef diff --git a/testing/test_package_docs/ex/B/s.html b/testing/test_package_docs/ex/B/s.html index 22116e35ed..6d76a56f83 100644 --- a/testing/test_package_docs/ex/B/s.html +++ b/testing/test_package_docs/ex/B/s.html @@ -91,6 +91,18 @@
class B
+
+ +
+ void + s=(String something) +
+ +
+

The setter for s

+
+
+ diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties-class.html b/testing/test_package_docs/fake/ClassWithUnusualProperties-class.html index 82e3d0dafc..9a85d3ea76 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties-class.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties-class.html @@ -120,6 +120,8 @@
library fake

Classes with unusual properties? I don't think they exist.

+

Or rather, dartdoc used to think they didn't exist. Check the variations +on inheritance and overrides here.

@@ -163,12 +165,13 @@

Properties

read-only
- explicitGetterImplicitSetter - → List<int> + explicitGetterImplicitSetter + ↔ List<int> +
- -
@override, inherited-setter, read-only
+ Getter doc for explicitGetterImplicitSetter +
@override, inherited-setter, read / write
explicitGetterSetter @@ -176,7 +179,7 @@

Properties
- This property is not synthetic, so it might reference f -- display it. + Getter doc for explicitGetterSetter.
read / write
@@ -193,17 +196,17 @@

Properties

→ Set
- + This property has some docs, too.
final
implicitGetterExplicitSetter - ← String x + ↔ String x
- -
@override, inherited-getter, write-only
+ Docs for implicitGetterExplicitSetter from ImplicitProperties. +
@override, inherited-getter, read / write
implicitReadWrite @@ -213,6 +216,24 @@

Properties

read / write
+
+
+ explicitGetterSetterForInheriting + ↔ int foo + +
+
+ Explicit getter for inheriting. +
read / write, inherited
+
+
+ forInheriting + ↔ int + +
+
+ A simple property to inherit. +
read / write, inherited
hashCode @@ -302,6 +323,8 @@
class ClassWithUnusualProperties
  • finalProperty
  • implicitGetterExplicitSetter
  • implicitReadWrite
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties/ClassWithUnusualProperties.html b/testing/test_package_docs/fake/ClassWithUnusualProperties/ClassWithUnusualProperties.html index a24cb6ae02..5b6c866ed3 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties/ClassWithUnusualProperties.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties/ClassWithUnusualProperties.html @@ -52,6 +52,8 @@
    class ClassWithUnusualProperties
  • finalProperty
  • implicitGetterExplicitSetter
  • implicitReadWrite
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties/aMethod.html b/testing/test_package_docs/fake/ClassWithUnusualProperties/aMethod.html index a710dc5443..bcdc5e5f65 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties/aMethod.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties/aMethod.html @@ -52,6 +52,8 @@
    class ClassWithUnusualProperties
  • finalProperty
  • implicitGetterExplicitSetter
  • implicitReadWrite
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetter.html b/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetter.html index 36a78c0d4f..d494724926 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetter.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetter.html @@ -52,6 +52,8 @@
    class ClassWithUnusualProperties
  • finalProperty
  • implicitGetterExplicitSetter
  • implicitReadWrite
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetterImplicitSetter.html b/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetterImplicitSetter.html index 9c644260ae..b546a63fce 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetterImplicitSetter.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetterImplicitSetter.html @@ -52,6 +52,8 @@
    class ClassWithUnusualProperties
  • finalProperty
  • implicitGetterExplicitSetter
  • implicitReadWrite
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • hashCode
  • runtimeType
    • @@ -77,7 +79,10 @@
    class ClassWithUnusualProperties
    List<int> explicitGetterImplicitSetter
    - +
    +

    Getter doc for explicitGetterImplicitSetter

    +
    + diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetterSetter.html b/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetterSetter.html index 48f514fb67..54e1180cc3 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetterSetter.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitGetterSetter.html @@ -52,6 +52,8 @@
    class ClassWithUnusualProperties
  • finalProperty
  • implicitGetterExplicitSetter
  • implicitReadWrite
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • hashCode
  • runtimeType
    • @@ -77,7 +79,10 @@
    class ClassWithUnusualProperties
    myCoolTypedef explicitGetterSetter - +
    +

    Getter doc for explicitGetterSetter.

    +
    +
    diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitSetter.html b/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitSetter.html index 406e3c5bd7..9056e53c68 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitSetter.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties/explicitSetter.html @@ -52,6 +52,8 @@
    class ClassWithUnusualProperties
  • finalProperty
  • implicitGetterExplicitSetter
  • implicitReadWrite
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties/finalProperty.html b/testing/test_package_docs/fake/ClassWithUnusualProperties/finalProperty.html index 0d50a154da..d7493cf9cc 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties/finalProperty.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties/finalProperty.html @@ -52,6 +52,8 @@
    class ClassWithUnusualProperties
  • finalProperty
  • implicitGetterExplicitSetter
  • implicitReadWrite
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • hashCode
  • runtimeType
    • @@ -75,6 +77,9 @@
    class ClassWithUnusualProperties
    finalProperty
    final
    +
    +

    This property has some docs, too.

    +
    diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties/implicitGetterExplicitSetter.html b/testing/test_package_docs/fake/ClassWithUnusualProperties/implicitGetterExplicitSetter.html index 211ffe7f77..75edcb7bf5 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties/implicitGetterExplicitSetter.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties/implicitGetterExplicitSetter.html @@ -52,6 +52,8 @@
    class ClassWithUnusualProperties
  • finalProperty
  • implicitGetterExplicitSetter
  • implicitReadWrite
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • hashCode
  • runtimeType
    • @@ -79,7 +81,10 @@
    class ClassWithUnusualProperties
    implicitGetterExplicitSetter=(String x) - +
    +

    Docs for implicitGetterExplicitSetter from ImplicitProperties.

    +
    + diff --git a/testing/test_package_docs/fake/ClassWithUnusualProperties/implicitReadWrite.html b/testing/test_package_docs/fake/ClassWithUnusualProperties/implicitReadWrite.html index fb1df7725a..d0848bb7bc 100644 --- a/testing/test_package_docs/fake/ClassWithUnusualProperties/implicitReadWrite.html +++ b/testing/test_package_docs/fake/ClassWithUnusualProperties/implicitReadWrite.html @@ -52,6 +52,8 @@
    class ClassWithUnusualProperties
  • finalProperty
  • implicitGetterExplicitSetter
  • implicitReadWrite
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ImplicitProperties-class.html b/testing/test_package_docs/fake/ImplicitProperties-class.html index bc49410aed..7b7219e4e1 100644 --- a/testing/test_package_docs/fake/ImplicitProperties-class.html +++ b/testing/test_package_docs/fake/ImplicitProperties-class.html @@ -159,7 +159,25 @@

    Properties

    - + Docs for explicitGetterImplicitSetter from ImplicitProperties. +
    read / write
    +
    +
    + explicitGetterSetterForInheriting + ↔ int foo + +
    +
    + Explicit getter for inheriting. +
    read / write
    +
    +
    + forInheriting + ↔ int + +
    +
    + A simple property to inherit.
    read / write
    @@ -168,7 +186,7 @@

    Properties

    - + Docs for implicitGetterExplicitSetter from ImplicitProperties.
    read / write
    @@ -244,6 +262,8 @@
    class ImplicitProperties
    Properties
  • explicitGetterImplicitSetter
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • implicitGetterExplicitSetter
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ImplicitProperties/ImplicitProperties.html b/testing/test_package_docs/fake/ImplicitProperties/ImplicitProperties.html index 7e58f73170..43474a0af6 100644 --- a/testing/test_package_docs/fake/ImplicitProperties/ImplicitProperties.html +++ b/testing/test_package_docs/fake/ImplicitProperties/ImplicitProperties.html @@ -46,6 +46,8 @@
    class ImplicitProperties
    Properties
  • explicitGetterImplicitSetter
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • implicitGetterExplicitSetter
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ImplicitProperties/explicitGetterImplicitSetter.html b/testing/test_package_docs/fake/ImplicitProperties/explicitGetterImplicitSetter.html index 5fb2ecabe3..98eb5907d1 100644 --- a/testing/test_package_docs/fake/ImplicitProperties/explicitGetterImplicitSetter.html +++ b/testing/test_package_docs/fake/ImplicitProperties/explicitGetterImplicitSetter.html @@ -46,6 +46,8 @@
    class ImplicitProperties
    Properties
  • explicitGetterImplicitSetter
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • implicitGetterExplicitSetter
  • hashCode
  • runtimeType
    • @@ -69,6 +71,9 @@
    class ImplicitProperties
    explicitGetterImplicitSetter
    read / write
    +
    +

    Docs for explicitGetterImplicitSetter from ImplicitProperties.

    +
    diff --git a/testing/test_package_docs/fake/ImplicitProperties/hashCode.html b/testing/test_package_docs/fake/ImplicitProperties/hashCode.html index 6fcea923f3..b904ac04c9 100644 --- a/testing/test_package_docs/fake/ImplicitProperties/hashCode.html +++ b/testing/test_package_docs/fake/ImplicitProperties/hashCode.html @@ -46,6 +46,8 @@
    class ImplicitProperties
    Properties
  • explicitGetterImplicitSetter
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • implicitGetterExplicitSetter
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ImplicitProperties/implicitGetterExplicitSetter.html b/testing/test_package_docs/fake/ImplicitProperties/implicitGetterExplicitSetter.html index 80296112fa..65fd06e0b0 100644 --- a/testing/test_package_docs/fake/ImplicitProperties/implicitGetterExplicitSetter.html +++ b/testing/test_package_docs/fake/ImplicitProperties/implicitGetterExplicitSetter.html @@ -46,6 +46,8 @@
    class ImplicitProperties
    Properties
  • explicitGetterImplicitSetter
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • implicitGetterExplicitSetter
  • hashCode
  • runtimeType
    • @@ -69,6 +71,9 @@
    class ImplicitProperties
    implicitGetterExplicitSetter
    read / write
    +
    +

    Docs for implicitGetterExplicitSetter from ImplicitProperties.

    +
    diff --git a/testing/test_package_docs/fake/ImplicitProperties/noSuchMethod.html b/testing/test_package_docs/fake/ImplicitProperties/noSuchMethod.html index 1d0fa6fade..ac0da0ec06 100644 --- a/testing/test_package_docs/fake/ImplicitProperties/noSuchMethod.html +++ b/testing/test_package_docs/fake/ImplicitProperties/noSuchMethod.html @@ -46,6 +46,8 @@
    class ImplicitProperties
    Properties
  • explicitGetterImplicitSetter
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • implicitGetterExplicitSetter
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ImplicitProperties/operator_equals.html b/testing/test_package_docs/fake/ImplicitProperties/operator_equals.html index 491ef8672f..01a4351e84 100644 --- a/testing/test_package_docs/fake/ImplicitProperties/operator_equals.html +++ b/testing/test_package_docs/fake/ImplicitProperties/operator_equals.html @@ -46,6 +46,8 @@
    class ImplicitProperties
    Properties
  • explicitGetterImplicitSetter
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • implicitGetterExplicitSetter
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ImplicitProperties/runtimeType.html b/testing/test_package_docs/fake/ImplicitProperties/runtimeType.html index f82345b20e..256299bcd4 100644 --- a/testing/test_package_docs/fake/ImplicitProperties/runtimeType.html +++ b/testing/test_package_docs/fake/ImplicitProperties/runtimeType.html @@ -46,6 +46,8 @@
    class ImplicitProperties
    Properties
  • explicitGetterImplicitSetter
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • implicitGetterExplicitSetter
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/fake/ImplicitProperties/toString.html b/testing/test_package_docs/fake/ImplicitProperties/toString.html index 19ec203997..6dfe4d25f4 100644 --- a/testing/test_package_docs/fake/ImplicitProperties/toString.html +++ b/testing/test_package_docs/fake/ImplicitProperties/toString.html @@ -46,6 +46,8 @@
    class ImplicitProperties
    Properties
  • explicitGetterImplicitSetter
    • +
  • explicitGetterSetterForInheriting
    • +
  • forInheriting
  • implicitGetterExplicitSetter
  • hashCode
  • runtimeType
    • diff --git a/testing/test_package_docs/index.json b/testing/test_package_docs/index.json index 3ba526d744..7e925df6fe 100644 --- a/testing/test_package_docs/index.json +++ b/testing/test_package_docs/index.json @@ -4199,6 +4199,28 @@ "type": "class" } }, + { + "name": "explicitGetterSetterForInheriting", + "qualifiedName": "fake.ImplicitProperties.explicitGetterSetterForInheriting", + "href": "fake/ImplicitProperties/explicitGetterSetterForInheriting.html", + "type": "property", + "overriddenDepth": 0, + "enclosedBy": { + "name": "ImplicitProperties", + "type": "class" + } + }, + { + "name": "forInheriting", + "qualifiedName": "fake.ImplicitProperties.forInheriting", + "href": "fake/ImplicitProperties/forInheriting.html", + "type": "property", + "overriddenDepth": 0, + "enclosedBy": { + "name": "ImplicitProperties", + "type": "class" + } + }, { "name": "hashCode", "qualifiedName": "fake.ImplicitProperties.hashCode", From bf6ef0572c8aad5f26b1f224ee8686d2db9cb8f1 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 19 Jun 2017 15:08:40 -0700 Subject: [PATCH 11/25] Add stranded files. --- .../explicitGetterSetterForInheriting.html | 123 ++++++++++++++++++ .../ImplicitProperties/forInheriting.html | 108 +++++++++++++++ 2 files changed, 231 insertions(+) create mode 100644 testing/test_package_docs/fake/ImplicitProperties/explicitGetterSetterForInheriting.html create mode 100644 testing/test_package_docs/fake/ImplicitProperties/forInheriting.html diff --git a/testing/test_package_docs/fake/ImplicitProperties/explicitGetterSetterForInheriting.html b/testing/test_package_docs/fake/ImplicitProperties/explicitGetterSetterForInheriting.html new file mode 100644 index 0000000000..c719875796 --- /dev/null +++ b/testing/test_package_docs/fake/ImplicitProperties/explicitGetterSetterForInheriting.html @@ -0,0 +1,123 @@ + + + + + + + + explicitGetterSetterForInheriting property - ImplicitProperties class - fake library - Dart API + + + + + + + + + + + + +
    + +
    + + +
    explicitGetterSetterForInheriting
    + +
    + +
    + +
    +
    class ImplicitProperties
    +
      +
    1. Constructors
      2. +
    2. ImplicitProperties
      3. + +
    3. + Properties +
      4. +
    4. explicitGetterImplicitSetter
      5. +
    5. explicitGetterSetterForInheriting
      6. +
    6. forInheriting
      7. +
    7. implicitGetterExplicitSetter
      8. +
    8. hashCode
      9. +
    9. runtimeType
      10. + +
    10. Methods
      11. +
    11. noSuchMethod
      12. +
    12. toString
      13. + +
    13. Operators
      14. +
    14. operator ==
      15. + + + +
    +
    + +
    + + +
    + +
    + int + explicitGetterSetterForInheriting
    + +
    +

    Explicit getter for inheriting.

    +
    +
    + + +
    + +
    + void + explicitGetterSetterForInheriting=(int foo) +
    + +
    +

    Explicit setter for inheriting.

    +
    +
    + + +
    + +
    +
    property explicitGetterSetterForInheriting
    +
    + +
    + +
    + + test_package 0.0.1 + + • + + cc license + + +
    + + + + + + + + + + + diff --git a/testing/test_package_docs/fake/ImplicitProperties/forInheriting.html b/testing/test_package_docs/fake/ImplicitProperties/forInheriting.html new file mode 100644 index 0000000000..1091af5790 --- /dev/null +++ b/testing/test_package_docs/fake/ImplicitProperties/forInheriting.html @@ -0,0 +1,108 @@ + + + + + + + + forInheriting property - ImplicitProperties class - fake library - Dart API + + + + + + + + + + + + +
    + +
    + + +
    forInheriting
    + +
    + +
    + +
    +
    class ImplicitProperties
    +
      +
    1. Constructors
      2. +
    2. ImplicitProperties
      3. + +
    3. + Properties +
      4. +
    4. explicitGetterImplicitSetter
      5. +
    5. explicitGetterSetterForInheriting
      6. +
    6. forInheriting
      7. +
    7. implicitGetterExplicitSetter
      8. +
    8. hashCode
      9. +
    9. runtimeType
      10. + +
    10. Methods
      11. +
    11. noSuchMethod
      12. +
    12. toString
      13. + +
    13. Operators
      14. +
    14. operator ==
      15. + + + +
    +
    + +
    + +
    + int + forInheriting
    read / write
    +
    + +
    +

    A simple property to inherit.

    +
    + + + +
    + +
    +
    property forInheriting
    +
    + +
    + +
    + + test_package 0.0.1 + + • + + cc license + + +
    + + + + + + + + + + + From 34128bd67a7efb70a926deca3d156da5b820bb75 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 19 Jun 2017 16:02:23 -0700 Subject: [PATCH 12/25] Cleanup + dartfmt --- lib/dartdoc.dart | 2 +- lib/src/html/html_generator_instance.dart | 5 - lib/src/markdown_processor.dart | 10 +- lib/src/model.dart | 498 ++++++---------------- test/model_test.dart | 34 +- testing/test_package/lib/fake.dart | 1 + 6 files changed, 164 insertions(+), 386 deletions(-) diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart index 2ea2493da0..c093d4a787 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -253,7 +253,7 @@ class DartDoc { } } - if (referredFromElements.isEmpty && referredFrom == 'index.html') + if (referredFromElements.isEmpty && referredFrom == 'index.html') referredFromElements.add(package); String message = warnOn; if (referredFrom == 'index.json') message = '$warnOn (from index.json)'; diff --git a/lib/src/html/html_generator_instance.dart b/lib/src/html/html_generator_instance.dart index fbc7a1d0e6..bddcb589dd 100644 --- a/lib/src/html/html_generator_instance.dart +++ b/lib/src/html/html_generator_instance.dart @@ -101,11 +101,8 @@ class HtmlGeneratorInstance implements HtmlOptions { for (var clazz in lib.allClasses) { // TODO(jcollins-g): consider refactor so that only the canonical // ModelElements show up in these lists - //if (clazz.name != 'FlutterLogoDecoration') continue; if (!clazz.isCanonical) continue; - if (clazz.name == 'ConstantCat') - 1+1; generateClass(package, lib, clazz); for (var constructor in clazz.constructors) { @@ -124,8 +121,6 @@ class HtmlGeneratorInstance implements HtmlOptions { } for (var property in clazz.propertiesForPages) { - if (property.name == 'hashCode') - 1+1; if (!property.isCanonical) continue; generateProperty(package, lib, clazz, property); } diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index e6a0a3ed2e..096dc08d93 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -293,16 +293,16 @@ MatchingLinkResult _getMatchingLinkElement( return new MatchingLinkResult(refModelElement, null); } if (searchElement is ClassMemberElement) { - if (codeRef == 'Map.keys') - 1+1; - Class refClass = new ModelElement.from(searchElement.enclosingElement, refLibrary); + if (codeRef == 'Map.keys') 1 + 1; + Class refClass = + new ModelElement.from(searchElement.enclosingElement, refLibrary); refModelElement = refClass.findModelElement(searchElement); } else { refModelElement = new ModelElement.from(searchElement, refLibrary); } if (!refModelElement.isCanonical) { - refModelElement.warn(PackageWarning.noCanonicalFound, - referredFrom: [element]); + refModelElement + .warn(PackageWarning.noCanonicalFound, referredFrom: [element]); // Don't warn about doc references because that's covered by the no // canonical library found message. return new MatchingLinkResult(null, null, warn: false); diff --git a/lib/src/model.dart b/lib/src/model.dart index c1cd1ead1f..d5a1842330 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -146,8 +146,6 @@ abstract class Inheritable { } } else { if (enclosingElement.isCanonical) { - if (enclosingElement is Library) - 1+1; _canonicalEnclosingClass = enclosingElement; } } @@ -173,24 +171,25 @@ abstract class Inheritable { /// A getter or setter that is a member of a Class. class InheritableAccessor extends Accessor with Inheritable { - /// Factory will return an [InheritableAccessor] with isInherited = true /// if [element] is in [inheritedAccessors]. - factory InheritableAccessor.from(PropertyAccessorElement element, Set inheritedAccessors, Class enclosingClass) { + factory InheritableAccessor.from(PropertyAccessorElement element, + Set inheritedAccessors, Class enclosingClass) { if (element == null) return null; if (inheritedAccessors.contains(element)) { - return new ModelElement.from(element, enclosingClass.library, enclosingClass: enclosingClass); + return new ModelElement.from(element, enclosingClass.library, + enclosingClass: enclosingClass); } return new ModelElement.from(element, enclosingClass.library); } ModelElement _enclosingElement; bool _isInherited = false; - InheritableAccessor( - PropertyAccessorElement element, Library library) + InheritableAccessor(PropertyAccessorElement element, Library library) : super(element, library); - InheritableAccessor.inherited(PropertyAccessorElement element, Library library, this._enclosingElement) + InheritableAccessor.inherited( + PropertyAccessorElement element, Library library, this._enclosingElement) : super(element, library) { _isInherited = true; } @@ -207,18 +206,15 @@ class InheritableAccessor extends Accessor with Inheritable { } } - /// Getters and setters. class Accessor extends ModelElement with SourceCodeMixin implements EnclosedElement { GetterSetterCombo _enclosingCombo; - Accessor( - PropertyAccessorElement element, Library library) + Accessor(PropertyAccessorElement element, Library library) : super(element, library); - ModelElement get enclosingCombo => _enclosingCombo; /// Call exactly once to set the enclosing combo for this Accessor. @@ -237,7 +233,9 @@ class Accessor extends ModelElement @override void warn(PackageWarning kind, - {String message, List referredFrom, List extendedDebug}) { + {String message, + List referredFrom, + List extendedDebug}) { if (enclosingCombo != null) { enclosingCombo.warn(kind, message: message, @@ -269,8 +267,6 @@ class Accessor extends ModelElement @override String get href { return enclosingCombo.href; - //if (canonicalLibrary == null) return null; - //return '${canonicalLibrary.dirName}/${_accessor.enclosingElement.name}/${name}.html'; } bool get isGetter => _accessor.isGetter; @@ -297,37 +293,15 @@ class Accessor extends ModelElement possibleFields.addAll(parentClass.allInstanceProperties); possibleFields.addAll(parentClass.staticProperties); String fieldName = accessor.name.replaceFirst('=', ''); - Field foundField = possibleFields.firstWhere((f) => f.element.name == fieldName); + Field foundField = + possibleFields.firstWhere((f) => f.element.name == fieldName); if (this.isGetter) { _overriddenElement = foundField.getter; } else { _overriddenElement = foundField.setter; } assert(!(_overriddenElement as Accessor).isInherited); - if (_overriddenElement != null) - 1+1; break; - /* - _overriddenElement = new ModelElement.from(accessor, library, - enclosingCombo: possibleFields - .firstWhere((f) => f.element.name == fieldName)); - */ - - /*Field foundField; - foundField = possibleFields.firstWhere( - (f) => f.getter?.element == accessor || f.setter?.element == accessor, orElse: () => null); - if (foundField != null) { - print ('found something: ${accessor.name}'); - if (foundField.getter?.element == accessor) { - _overriddenElement = foundField.getter; - } - if (foundField.setter?.element == accessor) { - _overriddenElement = foundField.setter; - } - break; - }*/ - } else { - 1+1; } } } @@ -341,7 +315,6 @@ class Accessor extends ModelElement PropertyAccessorElement get _accessor => (element as PropertyAccessorElement); } - class Class extends ModelElement implements EnclosedElement { List _mixins; ElementType _supertype; @@ -456,7 +429,7 @@ class Class extends ModelElement implements EnclosedElement { } Map _allElements; - Map get allElements { + Map get allElements { if (_allElements == null) { _allElements = new Map(); for (ModelElement me in allModelElements) { @@ -581,10 +554,6 @@ class Class extends ModelElement implements EnclosedElement { Map imap = library.inheritanceManager.getMembersInheritedFromInterfaces(element); - if (cmap.length + imap.length > 10) { - 1+1; - } - // remove methods that exist on this class _methods.forEach((method) { cmap.remove(method.name); @@ -700,76 +669,11 @@ class Class extends ModelElement implements EnclosedElement { List get inheritedProperties { if (_inheritedProperties == null) { - _inheritedProperties = _allFields.where((f) => f.isInherited).toList()..sort(byName); + _inheritedProperties = _allFields.where((f) => f.isInherited).toList() + ..sort(byName); } return _inheritedProperties; } - /*List get inheritedProperties { - if (_inheritedProperties != null) return _inheritedProperties; - Map cmap = - library.inheritanceManager.getMembersInheritedFromClasses(element); - Map imap = - library.inheritanceManager.getMembersInheritedFromInterfaces(element); - - _inheritedProperties = []; - List values = []; - Set uniqueNames = new Set(); - - instanceProperties.forEach((f) { - if (f.setter != null) uniqueNames.add(f.setter.element.name); - if (f.getter != null) uniqueNames.add(f.getter.element.name); - }); - - for (String key in cmap.keys) { - // XXX: if we care about showing a hierarchy with our inherited methods, - // then don't do this - if (uniqueNames.contains(key)) continue; - - uniqueNames.add(key); - values.add(cmap[key]); - } - - for (String key in imap.keys) { - // XXX: if we care about showing a hierarchy with our inherited methods, - // then don't do this - if (uniqueNames.contains(key)) continue; - - uniqueNames.add(key); - values.add(imap[key]); - } - values - .removeWhere((it) => instanceProperties.any((i) => it.name == i.name)); - - for (var value in values) { - if (value != null && - value is PropertyAccessorElement && - isPublic(value) && - value.enclosingElement != null) { - // This seems to be here to deal with half-field inheritance, where - // we inherit a getter but not a setter, or vice-versa. (Or if a parent - // class has the same trouble). In that case, just drop any duplicate - // names we see. This probably results in bugs. - // TODO(jcollins-g): deal with half-inherited fields better - var e = value.variable; - - if (instanceProperties.any((f) => f.element.name == e.name)) continue; - if (_inheritedProperties.any((f) => f.element.name == e.name)) continue; - - if (!package.isDocumented(value.enclosingElement)) { - Field f = new ModelElement.from(e, library, enclosingClass: this); - _inheritedProperties.add(f); - _genPageProperties.add(f); - } else { - _inheritedProperties - .add(new ModelElement.from(e, library, enclosingClass: this)); - } - } - } - - _inheritedProperties.sort(byName); - - return _inheritedProperties; - }*/ List get instanceMethods { if (_instanceMethods != null) return _instanceMethods; @@ -815,7 +719,8 @@ class Class extends ModelElement implements EnclosedElement { } /// Returns true if [other] is a parent class or mixin for this class. - bool isInheritingFrom(Class other) => superChain.map((et) => (et.element as Class)).contains(other); + bool isInheritingFrom(Class other) => + superChain.map((et) => (et.element as Class)).contains(other); @override String get kind => 'class'; @@ -860,9 +765,11 @@ class Class extends ModelElement implements EnclosedElement { // now, we're creating a flat list. We're not paying attention to where // these methods are actually coming from. This might turn out to be a // problem if we want to show that info later. - List get propertiesForPages { - return []..addAll( - instanceProperties)..addAll(inheritedProperties)..sort(byName); + List get propertiesForPages { + return [] + ..addAll(instanceProperties) + ..addAll(inheritedProperties) + ..sort(byName); } List get staticMethods { @@ -921,8 +828,6 @@ class Class extends ModelElement implements EnclosedElement { List get _allFields { if (_fields != null) return _fields; - if (this.name == 'AnimateElement') - 1+1; _fields = []; Map cmap = library.inheritanceManager.getMembersInheritedFromClasses(element); @@ -930,13 +835,17 @@ class Class extends ModelElement implements EnclosedElement { library.inheritanceManager.getMembersInheritedFromInterfaces(element); Set inheritedAccessors = new Set(); - inheritedAccessors.addAll(cmap.values.where((e) => e is PropertyAccessorElement) - .map((e) => Package.getBasestElement(e) as PropertyAccessorElement).where(isPublic)); + inheritedAccessors.addAll(cmap.values + .where((e) => e is PropertyAccessorElement) + .map((e) => Package.getBasestElement(e) as PropertyAccessorElement) + .where(isPublic)); // Interfaces are subordinate to members inherited from classes, so don't // add this to our accessor set if we already have something inherited from classes. - inheritedAccessors.addAll(imap.values.where((e) => e is PropertyAccessorElement && !cmap.containsKey(e.name)) - .map((e) => Package.getBasestElement(e) as PropertyAccessorElement).where(isPublic)); + inheritedAccessors.addAll(imap.values + .where((e) => e is PropertyAccessorElement && !cmap.containsKey(e.name)) + .map((e) => Package.getBasestElement(e) as PropertyAccessorElement) + .where(isPublic)); assert(!inheritedAccessors.any((e) => e is Member)); // This structure keeps track of inherited accessors. @@ -953,11 +862,13 @@ class Class extends ModelElement implements EnclosedElement { for (FieldElement f in _cls.fields.where(isPublic)) { PropertyAccessorElement getterElement = f.getter; if (getterElement == null && accessorMap.containsKey(f.name)) { - getterElement = accessorMap[f.name].firstWhere((e) => e.isGetter, orElse: () => null); + getterElement = accessorMap[f.name] + .firstWhere((e) => e.isGetter, orElse: () => null); } PropertyAccessorElement setterElement = f.setter; if (setterElement == null && accessorMap.containsKey(f.name)) { - setterElement = accessorMap[f.name].firstWhere((e) => e.isSetter, orElse: () => null); + setterElement = accessorMap[f.name] + .firstWhere((e) => e.isSetter, orElse: () => null); } _addSingleField(getterElement, setterElement, inheritedAccessors, f); accessorMap.remove(f.name); @@ -966,9 +877,13 @@ class Class extends ModelElement implements EnclosedElement { // Now we only have inherited accessors who aren't associated with // anything in cls._fields. for (String fieldName in accessorMap.keys) { - List elements = accessorMap[fieldName].map((e) => Package.getBasestElement(e)).toList(); - PropertyAccessorElement getterElement = elements.firstWhere((e) => e.isGetter, orElse: () => null); - PropertyAccessorElement setterElement = elements.firstWhere((e) => e.isSetter, orElse: () => null); + List elements = accessorMap[fieldName] + .map((e) => Package.getBasestElement(e)) + .toList(); + PropertyAccessorElement getterElement = + elements.firstWhere((e) => e.isGetter, orElse: () => null); + PropertyAccessorElement setterElement = + elements.firstWhere((e) => e.isSetter, orElse: () => null); _addSingleField(getterElement, setterElement, inheritedAccessors); } @@ -979,18 +894,15 @@ class Class extends ModelElement implements EnclosedElement { /// Add a single Field to _fields. /// /// Modifies accessorMap, removing the field added. - void _addSingleField(PropertyAccessorElement getterElement, + void _addSingleField( + PropertyAccessorElement getterElement, PropertyAccessorElement setterElement, Set inheritedAccessors, [FieldElement f]) { - if (f != null && f.name == 'implicitGetterExplicitSetter') - 1+1; - if (getterElement != null && getterElement.name.contains('implicitGetterExplicitSetter')) - 1+1; - if (setterElement != null && setterElement.name.contains('implicitGetterExplicitSetter')) - 1+1; - Accessor getter = new InheritableAccessor.from(getterElement, inheritedAccessors, this); - Accessor setter = new InheritableAccessor.from(setterElement, inheritedAccessors, this); + Accessor getter = + new InheritableAccessor.from(getterElement, inheritedAccessors, this); + Accessor setter = + new InheritableAccessor.from(setterElement, inheritedAccessors, this); assert(!(getter == null && setter == null)); if (f == null) { // Pick an appropriate FieldElement to represent this element. @@ -1002,42 +914,29 @@ class Class extends ModelElement implements EnclosedElement { } else /* getter != null && setter != null */ { // In cases where a Field is composed of two Accessors defined in // different places in the inheritance chain, there are two FieldElements - // for this single Field we're trying to compose. If the setter - // is in a class inheriting from the getter's class, pick the setter's FieldElement -- - // otherwise, pick the getter. - if ((setter.enclosingElement as Class).isInheritingFrom(getter.enclosingElement)) { + // for this single Field we're trying to compose. Pick the one closest + // to this class on the inheritance chain. + if ((setter.enclosingElement as Class) + .isInheritingFrom(getter.enclosingElement)) { f = Package.getBasestElement(setterElement.variable); } else { f = Package.getBasestElement(getterElement.variable); } } } - if ((getter == null || getter.isInherited) && (setter == null || setter.isInherited)) { + if ((getter == null || getter.isInherited) && + (setter == null || setter.isInherited)) { // Field is 100% inherited. - _fields.add(new ModelElement.from(f, library, enclosingClass: this, - getter: getter, setter: setter)); + _fields.add(new ModelElement.from(f, library, + enclosingClass: this, getter: getter, setter: setter)); } else { // Field is <100% inherited (could be half-inherited). // TODO(jcollins-g): make half-inherited fields half-italicized...? - _fields.add(new ModelElement.from(f, library, - getter: getter, setter: setter)); + _fields.add( + new ModelElement.from(f, library, getter: getter, setter: setter)); } } - /* - List get _allFields { - if (_fields != null) return _fields; - - _fields = _cls.fields - .where(isPublic) - .map((e) => new ModelElement.from(e, library)) - .toList(growable: false) - ..sort(byName); - - return _fields; - } - */ - ClassElement get _cls => (element as ClassElement); List get _methods { @@ -1170,20 +1069,15 @@ class Enum extends Class { var index = -1; _enumFields = []; - for (FieldElement f in _cls.fields.where(isPublic).where((f) => f.isConst)) { + for (FieldElement f + in _cls.fields.where(isPublic).where((f) => f.isConst)) { // Enums do not have inheritance. Accessor accessor = new ModelElement.from(f.getter, library); - _enumFields.add(new ModelElement.from(f, library, index: index++, getter: accessor)); + _enumFields.add( + new ModelElement.from(f, library, index: index++, getter: accessor)); } _enumFields.sort(byName); - /* - _enumFields = _cls.fields - .where(isPublic) - .where((f) => f.isConst) - .map((field) => new ModelElement.from(field, library, index: index++)) - .toList(growable: false) - ..sort(byName); - */ + return _enumFields; } @@ -1207,24 +1101,14 @@ class Enum extends Class { class EnumField extends Field { int _index; - EnumField(FieldElement element, Library library, Accessor getter, Accessor setter) : super(element, library, getter, setter); + EnumField( + FieldElement element, Library library, Accessor getter, Accessor setter) + : super(element, library, getter, setter); - EnumField.forConstant(this._index, FieldElement element, Library library, Accessor getter) + EnumField.forConstant( + this._index, FieldElement element, Library library, Accessor getter) : super(element, library, getter, null); - /* - Accessor get getter { - // EnumFields should never be trying to get getter/setters, they are - // all synthetic. - assert(false); - return null; - } - Accessor get setter { - assert(false); - return null; - } - */ - @override String get constantValue { if (name == 'values') { @@ -1290,13 +1174,15 @@ class Field extends ModelElement @override final InheritableAccessor setter; - Field(FieldElement element, Library library, this.getter, this.setter) : super(element, library) { + Field(FieldElement element, Library library, this.getter, this.setter) + : super(element, library) { _setModelType(); if (getter != null) getter.enclosingCombo = this; if (setter != null) setter.enclosingCombo = this; } - factory Field.inherited(FieldElement element, Class enclosingClass, Library library, Accessor getter, Accessor setter) { + factory Field.inherited(FieldElement element, Class enclosingClass, + Library library, Accessor getter, Accessor setter) { Field newField = new Field(element, library, getter, setter); newField._isInherited = true; newField._enclosingClass = enclosingClass; @@ -1308,8 +1194,6 @@ class Field extends ModelElement @override String get documentation { - if (name == 'darkColor' && enclosingElement.name == 'FlutterLogoDecoration') - 1+1; // Verify that hasSetter and hasGetterNoSetter are mutually exclusive, // to prevent displaying more or less than one summary. Set assertCheck = new Set()..addAll([hasSetter, hasGetterNoSetter]); @@ -1331,76 +1215,6 @@ class Field extends ModelElement String get constantValueTruncated => truncateString(constantValue, 200); - /* - @override - List get documentationFrom { - if (_documentationFrom == null) { - _documentationFrom = []; - if (getter != null) _documentationFrom.addAll(getter.documentationFrom); - if (setter != null) _documentationFrom.addAll(setter.documentationFrom); - if (_documentationFrom.length > 1) { - int c = 0; - for (ModelElement e in _documentationFrom) { - if (e.documentation != '') { - c += 1; - } - } - if (c == _documentationFrom.length) - 1+1; - } - } - return _documentationFrom; - } - - String _oneLineDoc; - @override - String get oneLineDoc { - if (_oneLineDoc == null) { - List oneLineDocs = []; - for (ModelElement e in [getter, setter]) { - if (e != null) - oneLineDocs.add(e.oneLineDoc); - } - } - } - */ - /* - // TODO(jcollins-g): documentationFrom doesn't really make sense in the case - // of split fields. - @override - ModelElement get documentationFrom { - if (_documentationFrom == null) { - if (computeDocumentationComment == null && - canOverride() && - overriddenElement != null) { - _documentationFrom = overriddenElement; - } else if (this is Inheritable && (this as Inheritable).isInherited) { - Inheritable thisInheritable = (this as Inheritable); - ModelElement fromThis = new ModelElement.from( - element, thisInheritable.definingEnclosingElement.library); - _documentationFrom = fromThis.documentationFrom; - } else { - _documentationFrom = this; - } - } - return _documentationFrom; - } - - String get _documentationLocal { - if (_rawDocs != null) return _rawDocs; - if (config.dropTextFrom.contains(element.library.name)) { - _rawDocs = ''; - } else { - _rawDocs = computeDocumentationComment ?? ''; - _rawDocs = stripComments(_rawDocs) ?? ''; - _rawDocs = _injectExamples(_rawDocs); - _rawDocs = _stripMacroTemplatesAndAddToIndex(_rawDocs); - _rawDocs = _injectMacros(_rawDocs); - } - return _rawDocs; - } - */ - @override ModelElement get enclosingElement { if (_enclosingClass == null) { @@ -1409,12 +1223,6 @@ class Field extends ModelElement return _enclosingClass; } - //@override - //bool get hasGetter => _field.getter != null; - - //@override - //bool get hasSetter => _field.setter != null; - @override String get href { String retval; @@ -1477,10 +1285,8 @@ class Field extends ModelElement if (getter.isInherited && setter.isInherited) { all_features.add('inherited'); } else { - if (getter.isInherited) - all_features.add('inherited-getter'); - if (setter.isInherited) - all_features.add('inherited-setter'); + if (getter.isInherited) all_features.add('inherited-getter'); + if (setter.isInherited) all_features.add('inherited-setter'); } } else { if (isInherited) all_features.add('inherited'); @@ -1499,14 +1305,6 @@ class Field extends ModelElement String get _fileName => isConst ? '$name-constant.html' : '$name.html'; - /* - @override - PropertyAccessorElement get _getter => _field.getter; - - @override - PropertyAccessorElement get _setter => _field.setter; - */ - void _setModelType() { if (hasGetter) { var t = (getter.element as PropertyAccessorElement).returnType; @@ -1522,17 +1320,10 @@ class Field extends ModelElement abstract class GetterSetterCombo implements ModelElement { Accessor get getter; - /*{ - return _getter == null - ? null - : new ModelElement.from(_getter, library, enclosingCombo: this, enclosingClass: isInherited ? enclosingElement : null); - }*/ - @override ModelElement enclosingElement; bool get isInherited; - /// Returns true if both accessors are synthetic. bool get hasSyntheticAccessors { if ((getter != null && getter.element.isSynthetic) || @@ -1547,11 +1338,14 @@ abstract class GetterSetterCombo implements ModelElement { if (_documentationFrom == null) { _documentationFrom = []; if (getter != null) { - _documentationFrom.addAll(getter.documentationFrom.where((e) => e.computeDocumentationComment != computeDocumentationComment)); + _documentationFrom.addAll(getter.documentationFrom.where((e) => + e.computeDocumentationComment != computeDocumentationComment)); } if (setter != null) - _documentationFrom.addAll(setter.documentationFrom.where((e) => e.computeDocumentationComment != computeDocumentationComment)); - if (_documentationFrom.length == 0 || _documentationFrom.every((e) => e.documentation == '')) + _documentationFrom.addAll(setter.documentationFrom.where((e) => + e.computeDocumentationComment != computeDocumentationComment)); + if (_documentationFrom.length == 0 || + _documentationFrom.every((e) => e.documentation == '')) _documentationFrom = computeDocumentationFrom; } return _documentationFrom; @@ -1561,17 +1355,18 @@ abstract class GetterSetterCombo implements ModelElement { @override String get oneLineDoc { if (_oneLineDoc == null) { - if (name == 'implicitGetterExplicitSetter') - 1+1; - bool hasAccessorsWithDocs = (getter != null && getter.oneLineDoc.isNotEmpty || - setter != null && setter.oneLineDoc.isNotEmpty); + bool hasAccessorsWithDocs = + (getter != null && getter.oneLineDoc.isNotEmpty || + setter != null && setter.oneLineDoc.isNotEmpty); if (!hasAccessorsWithDocs) { _oneLineDoc = _documentation.asOneLiner; } else { StringBuffer buffer = new StringBuffer(); bool getterSetterBothAvailable = false; - if (getter != null && getter.oneLineDoc.isNotEmpty && - setter != null && setter.oneLineDoc.isNotEmpty) { + if (getter != null && + getter.oneLineDoc.isNotEmpty && + setter != null && + setter.oneLineDoc.isNotEmpty) { getterSetterBothAvailable = true; } if (getter != null && getter.oneLineDoc.isNotEmpty) { @@ -1589,9 +1384,6 @@ abstract class GetterSetterCombo implements ModelElement { String get getterSetterDocumentationComment { var buffer = new StringBuffer(); - if (hasGetter && getter == null) - 1+1; - if (hasGetter && !getter.element.isSynthetic) { assert(getter.documentationFrom.length == 1); String docs = getter.documentationFrom.first.computeDocumentationComment; @@ -1663,20 +1455,7 @@ abstract class GetterSetterCombo implements ModelElement { bool get writeOnly => hasSetter && !hasGetter; - Accessor get setter; /*{ - return _setter == null - ? null - : new ModelElement.from(_setter, library, enclosingCombo: this, enclosingClass: isInherited ? enclosingElement : null); - }*/ - - /* - PropertyAccessorElement get _getter; - - // TODO: now that we have explicit getter and setters, we probably - // want a cleaner way to do this. Only the one-liner is using this - // now. The detail pages should be using getter and setter directly. - PropertyAccessorElement get _setter; - */ + Accessor get setter; } class Library extends ModelElement { @@ -1716,14 +1495,17 @@ class Library extends ModelElement { Accessor setter; if (e is GetterSetterCombo) { if (e.getter != null) { - getter = new ModelElement.from(e.getter.element, package.findOrCreateLibraryFor(e.getter.element)); + getter = new ModelElement.from(e.getter.element, + package.findOrCreateLibraryFor(e.getter.element)); } if (e.setter != null) { - setter = new ModelElement.from(e.setter.element, package.findOrCreateLibraryFor(e.setter.element)); + setter = new ModelElement.from(e.setter.element, + package.findOrCreateLibraryFor(e.setter.element)); } } return new ModelElement.from( - e.element, package.findOrCreateLibraryFor(e.element), getter: getter, setter: setter) + e.element, package.findOrCreateLibraryFor(e.element), + getter: getter, setter: setter) .fullyQualifiedName; }).toList(); } @@ -2012,16 +1794,11 @@ class Library extends ModelElement { Accessor setter; if (element.setter != null) setter = new ModelElement.from(element.setter, this); - _variables.add(new ModelElement.from(element, this, getter: getter, setter: setter)); + _variables.add( + new ModelElement.from(element, this, getter: getter, setter: setter)); } _variables.sort(byName); - /*elements - .where(isPublic) - .map((e) => new ModelElement.from(e, this)) - .toList(growable: false) - ..sort(byName); - */ return _variables; } @@ -2323,7 +2100,7 @@ abstract class ModelElement extends Nameable /// Do not construct any ModelElements unless they are from this constructor. /// Specify enclosingClass only if this is to be an inherited object. /// Specify index only if this is to be an EnumField.forConstant. - factory ModelElement.from(Element e, Library library, + factory ModelElement.from(Element e, Library library, {Class enclosingClass, int index, Accessor getter, Accessor setter}) { // We don't need index in this key because it isn't a disambiguator. // It isn't a disambiguator because EnumFields are not inherited, ever. @@ -2332,7 +2109,8 @@ abstract class ModelElement extends Nameable if (e is Member) { e = Package.getBasestElement(e); } - Tuple3 key = new Tuple3(e, library, enclosingClass); + Tuple3 key = + new Tuple3(e, library, enclosingClass); ModelElement newModelElement; if (e.kind != ElementKind.DYNAMIC && library.package._allConstructedModelElements.containsKey(key)) { @@ -2367,20 +2145,20 @@ abstract class ModelElement extends Nameable if (enclosingClass == null) { if (index != null) { assert(getter != null); - newModelElement = new EnumField.forConstant(index, e, library, getter); + newModelElement = + new EnumField.forConstant(index, e, library, getter); } else { if (e.enclosingElement.isEnum) { newModelElement = new EnumField(e, library, getter, setter); } else { - if (getter == null && setter == null) - 1+1; assert(getter != null || setter != null); newModelElement = new Field(e, library, getter, setter); } } } else { assert(getter != null || setter != null); - newModelElement = new Field.inherited(e, enclosingClass, library, getter, setter); + newModelElement = + new Field.inherited(e, enclosingClass, library, getter, setter); } } if (e is ConstructorElement) { @@ -2426,7 +2204,8 @@ abstract class ModelElement extends Nameable if (enclosingClass == null) newModelElement = new InheritableAccessor(e, library); else - newModelElement = new InheritableAccessor.inherited(e, library, enclosingClass); + newModelElement = + new InheritableAccessor.inherited(e, library, enclosingClass); } else { newModelElement = new Accessor(e, library); } @@ -2450,8 +2229,6 @@ abstract class ModelElement extends Nameable } } if (newModelElement is Field) { - if (getter != null && newModelElement.getter == null) - 1+1; assert(getter == null || newModelElement.getter.enclosingCombo != null); assert(setter == null || newModelElement.setter.enclosingCombo != null); } @@ -2586,7 +2363,7 @@ abstract class ModelElement extends Nameable List _documentationFrom; // TODO(jcollins-g): untangle when mixins can call super @override - List get documentationFrom { + List get documentationFrom { if (_documentationFrom == null) { _documentationFrom = computeDocumentationFrom; } @@ -2613,14 +2390,17 @@ abstract class ModelElement extends Nameable if (this is GetterSetterCombo) { GetterSetterCombo thisAsCombo = this as GetterSetterCombo; if (thisAsCombo.getter != null) { - newGetter = new ModelElement.from(thisAsCombo.getter.element, thisAsCombo.getter.definingLibrary); + newGetter = new ModelElement.from( + thisAsCombo.getter.element, thisAsCombo.getter.definingLibrary); } if (thisAsCombo.setter != null) { - newSetter = new ModelElement.from(thisAsCombo.setter.element, thisAsCombo.setter.definingLibrary); + newSetter = new ModelElement.from( + thisAsCombo.setter.element, thisAsCombo.setter.definingLibrary); } } ModelElement fromThis = new ModelElement.from( - element, thisInheritable.definingEnclosingElement.library, getter:newGetter, setter:newSetter); + element, thisInheritable.definingEnclosingElement.library, + getter: newGetter, setter: newSetter); docFrom = fromThis.documentationFrom; } else { docFrom = [this]; @@ -2645,8 +2425,6 @@ abstract class ModelElement extends Nameable /// Returns the docs, stripped of their leading comments syntax. @override String get documentation { - if (name == 'implicitGetterExplicitSetter') - 1+1; return documentationFrom.map((e) => e._documentationLocal).join('\n'); } @@ -2987,7 +2765,9 @@ abstract class ModelElement extends Nameable @override void warn(PackageWarning kind, - {String message, List referredFrom, List extendedDebug}) { + {String message, + List referredFrom, + List extendedDebug}) { package.warnOnElement(this, kind, message: message, referredFrom: referredFrom, @@ -3548,7 +3328,8 @@ Map packageWarningText = { /// Something that package warnings can be called on. abstract class Warnable implements Locatable { - void warn(PackageWarning warning, {String message, List referredFrom}); + void warn(PackageWarning warning, + {String message, List referredFrom}); Warnable get enclosingElement; } @@ -3804,7 +3585,9 @@ class Package extends Nameable implements Documentable { @override void warn(PackageWarning kind, - {String message, List referredFrom, List extendedDebug}) { + {String message, + List referredFrom, + List extendedDebug}) { warnOnElement(this, kind, message: message, referredFrom: referredFrom, @@ -3823,23 +3606,16 @@ class Package extends Nameable implements Documentable { } void warnOnElement(Warnable warnable, PackageWarning kind, - {String message, List referredFrom, List extendedDebug}) { + {String message, + List referredFrom, + List extendedDebug}) { if (warnable != null) { // This sort of warning is only applicable to top level elements. if (kind == PackageWarning.ambiguousReexport) { - if (warnable is TypeParameter) - 1+1; - while (warnable.enclosingElement is! Library && warnable.enclosingElement != null) { + while (warnable.enclosingElement is! Library && + warnable.enclosingElement != null) { warnable = warnable.enclosingElement; } - /* - Element topLevelElement = warnable.element; - while (topLevelElement.enclosingElement is! CompilationUnitElement) { - topLevelElement = topLevelElement.enclosingElement; - } - warnable = new ModelElement.from( - topLevelElement, findOrCreateLibraryFor(topLevelElement)); - */ } if (warnable is Accessor) { // This might be part of a Field, if so, assign this warning to the field @@ -3933,7 +3709,8 @@ class Package extends Nameable implements Documentable { if (referredFrom != null) { for (Locatable referral in referredFrom) { if (referral != warnable) { - Tuple2 referredFromStrings = nameAndLocation(referral); + Tuple2 referredFromStrings = + nameAndLocation(referral); messageParts.add( "${referredFromPrefix} ${referredFromStrings.item1}: ${referredFromStrings.item2}"); } @@ -4293,9 +4070,9 @@ class Package extends Nameable implements Documentable { if (e is PropertyInducingElement) { if (e.getter != null) getter = new ModelElement.from(e.getter, lib); if (e.setter != null) setter = new ModelElement.from(e.setter, lib); - 1+1; } - modelElement = new ModelElement.from(e, lib, getter: getter, setter: setter); + modelElement = + new ModelElement.from(e, lib, getter: getter, setter: setter); } assert(modelElement is! Inheritable); if (modelElement != null && !modelElement.isCanonical) { @@ -4569,7 +4346,8 @@ class TopLevelVariable extends ModelElement @override final Accessor setter; - TopLevelVariable(TopLevelVariableElement element, Library library, this.getter, this.setter) + TopLevelVariable(TopLevelVariableElement element, Library library, + this.getter, this.setter) : super(element, library) { if (hasGetter) { var t = _variable.getter.returnType; @@ -4606,8 +4384,6 @@ class TopLevelVariable extends ModelElement String get documentation { // Verify that hasSetter and hasGetterNoSetter are mutually exclusive, // to prevent displaying more or less than one summary. - if (name == 'incorrectDocReferenceFromEx') - 1+1; Set assertCheck = new Set()..addAll([hasSetter, hasGetterNoSetter]); assert(assertCheck.containsAll([true, false])); return super.documentation; @@ -4656,21 +4432,12 @@ class TopLevelVariable extends ModelElement @override String get computeDocumentationComment { String docs = getterSetterDocumentationComment; - if (name == 'incorrectDocReferenceFromEx') - 1+1; if (docs.isEmpty) return _variable.documentationComment; return docs; } String get _fileName => isConst ? '$name-constant.html' : '$name.html'; - /*@override - PropertyAccessorElement get _getter => _variable.getter; - - @override - PropertyAccessorElement get _setter => _variable.setter; - */ - TopLevelVariableElement get _variable => (element as TopLevelVariableElement); } @@ -4735,7 +4502,8 @@ class TypeParameter extends ModelElement { } @override - ModelElement get enclosingElement => new ModelElement.from(element.enclosingElement, library); + ModelElement get enclosingElement => + new ModelElement.from(element.enclosingElement, library); @override String get href { diff --git a/test/model_test.dart b/test/model_test.dart index 18b043e159..fca72cb75d 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -1206,9 +1206,12 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, fakeLibrary.classes.firstWhere((c) => c.name == 'LongFirstLine'); CatString = exLibrary.classes.firstWhere((c) => c.name == 'CatString'); - UnusualProperties = fakeLibrary.classes.firstWhere((c) => c.name == 'ClassWithUnusualProperties'); - implicitGetterExplicitSetter = UnusualProperties.allModelElements.firstWhere((e) => e.name == 'implicitGetterExplicitSetter'); - explicitGetterImplicitSetter = UnusualProperties.allModelElements.firstWhere((e) => e.name == 'explicitGetterImplicitSetter'); + UnusualProperties = fakeLibrary.classes + .firstWhere((c) => c.name == 'ClassWithUnusualProperties'); + implicitGetterExplicitSetter = UnusualProperties.allModelElements + .firstWhere((e) => e.name == 'implicitGetterExplicitSetter'); + explicitGetterImplicitSetter = UnusualProperties.allModelElements + .firstWhere((e) => e.name == 'explicitGetterImplicitSetter'); isEmpty = CatString.allInstanceProperties .firstWhere((p) => p.name == 'isEmpty'); @@ -1244,22 +1247,33 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, expect(implicitGetterExplicitSetter.getter.isInherited, isTrue); expect(implicitGetterExplicitSetter.setter.isInherited, isFalse); expect(implicitGetterExplicitSetter.isInherited, isFalse); - expect(implicitGetterExplicitSetter.features.contains('inherited-getter'), isTrue); - expect(implicitGetterExplicitSetter.oneLineDoc, equals('Docs for implicitGetterExplicitSetter from ImplicitProperties.')); - expect(implicitGetterExplicitSetter.documentation, equals('Docs for implicitGetterExplicitSetter from ImplicitProperties.')); + expect(implicitGetterExplicitSetter.features.contains('inherited-getter'), + isTrue); + expect( + implicitGetterExplicitSetter.oneLineDoc, + equals( + 'Docs for implicitGetterExplicitSetter from ImplicitProperties.')); + expect( + implicitGetterExplicitSetter.documentation, + equals( + 'Docs for implicitGetterExplicitSetter from ImplicitProperties.')); }); test('split inheritance with explicit getter works', () { expect(explicitGetterImplicitSetter.getter.isInherited, isFalse); expect(explicitGetterImplicitSetter.setter.isInherited, isTrue); expect(explicitGetterImplicitSetter.isInherited, isFalse); - expect(explicitGetterImplicitSetter.features.contains('inherited-setter'), isTrue); - expect(explicitGetterImplicitSetter.oneLineDoc, equals('Getter doc for explicitGetterImplicitSetter')); + expect(explicitGetterImplicitSetter.features.contains('inherited-setter'), + isTrue); + expect(explicitGetterImplicitSetter.oneLineDoc, + equals('Getter doc for explicitGetterImplicitSetter')); // Even though we have some new setter docs, getter still takes priority. - expect(explicitGetterImplicitSetter.documentation, equals('Docs for explicitGetterImplicitSetter from ImplicitProperties.')); + expect( + explicitGetterImplicitSetter.documentation, + equals( + 'Docs for explicitGetterImplicitSetter from ImplicitProperties.')); }); - test('has a fully qualified name', () { expect(lengthX.fullyQualifiedName, 'fake.WithGetterAndSetter.lengthX'); }); diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index d16a5a7522..23450dc779 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -207,6 +207,7 @@ class ClassWithUnusualProperties extends ImplicitProperties { set implicitGetterExplicitSetter(String x) {} @override + /// Getter doc for explicitGetterImplicitSetter List get explicitGetterImplicitSetter => new List(); From 81d8f499af211abb4c2116a34dc7dce9d9e5d323 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 20 Jun 2017 08:53:09 -0700 Subject: [PATCH 13/25] Minor cleanups --- lib/dartdoc.dart | 2 +- lib/src/model.dart | 19 +++++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart index c093d4a787..002cc55b7c 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -258,7 +258,7 @@ class DartDoc { String message = warnOn; if (referredFrom == 'index.json') message = '$warnOn (from index.json)'; package.warnOnElement(warnOnElement, kind, - message: message, referredFrom: referredFromElements.toList()); + message: message, referredFrom: referredFromElements); } void _doOrphanCheck(Package package, String origin, Set visited) { diff --git a/lib/src/model.dart b/lib/src/model.dart index d5a1842330..06dca32c72 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -234,8 +234,8 @@ class Accessor extends ModelElement @override void warn(PackageWarning kind, {String message, - List referredFrom, - List extendedDebug}) { + Iterable referredFrom, + Iterable extendedDebug}) { if (enclosingCombo != null) { enclosingCombo.warn(kind, message: message, @@ -2766,8 +2766,8 @@ abstract class ModelElement extends Nameable @override void warn(PackageWarning kind, {String message, - List referredFrom, - List extendedDebug}) { + Iterable referredFrom, + Iterable extendedDebug}) { package.warnOnElement(this, kind, message: message, referredFrom: referredFrom, @@ -3329,7 +3329,7 @@ Map packageWarningText = { /// Something that package warnings can be called on. abstract class Warnable implements Locatable { void warn(PackageWarning warning, - {String message, List referredFrom}); + {String message, Iterable referredFrom}); Warnable get enclosingElement; } @@ -3586,8 +3586,8 @@ class Package extends Nameable implements Documentable { @override void warn(PackageWarning kind, {String message, - List referredFrom, - List extendedDebug}) { + Iterable referredFrom, + Iterable extendedDebug}) { warnOnElement(this, kind, message: message, referredFrom: referredFrom, @@ -3607,8 +3607,8 @@ class Package extends Nameable implements Documentable { void warnOnElement(Warnable warnable, PackageWarning kind, {String message, - List referredFrom, - List extendedDebug}) { + Iterable referredFrom, + Iterable extendedDebug}) { if (warnable != null) { // This sort of warning is only applicable to top level elements. if (kind == PackageWarning.ambiguousReexport) { @@ -4086,7 +4086,6 @@ class Package extends Nameable implements Documentable { /// a documentation entry point (for elements that have no Library within the /// set of canonical Libraries). Library findOrCreateLibraryFor(Element e) { - if (e == null) 1 + 1; // This is just a cache to avoid creating lots of libraries over and over. if (allLibraries.containsKey(e.library)) { return allLibraries[e.library]; From 9cb509d6bcdfc14fd1e9333e0465a60c7471a149 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 20 Jun 2017 12:07:33 -0700 Subject: [PATCH 14/25] More cleanups and simplification --- lib/src/markdown_processor.dart | 11 +++------ lib/src/model.dart | 40 +++++++++++---------------------- 2 files changed, 16 insertions(+), 35 deletions(-) diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index 096dc08d93..dadbd3ea55 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -292,14 +292,9 @@ MatchingLinkResult _getMatchingLinkElement( if (refModelElement != null) { return new MatchingLinkResult(refModelElement, null); } - if (searchElement is ClassMemberElement) { - if (codeRef == 'Map.keys') 1 + 1; - Class refClass = - new ModelElement.from(searchElement.enclosingElement, refLibrary); - refModelElement = refClass.findModelElement(searchElement); - } else { - refModelElement = new ModelElement.from(searchElement, refLibrary); - } + // From this point on, we haven't been able to find a canonical ModelElement. + // So in this case, just find any ModelElement we can. + refModelElement = new ModelElement.from(searchElement, refLibrary); if (!refModelElement.isCanonical) { refModelElement .warn(PackageWarning.noCanonicalFound, referredFrom: [element]); diff --git a/lib/src/model.dart b/lib/src/model.dart index 06dca32c72..925174882d 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -848,7 +848,8 @@ class Class extends ModelElement implements EnclosedElement { .where(isPublic)); assert(!inheritedAccessors.any((e) => e is Member)); - // This structure keeps track of inherited accessors. + // This structure keeps track of inherited accessors, allowing lookup + // by field name (stripping the '=' from setters). Map> accessorMap = new Map(); for (PropertyAccessorElement accessorElement in inheritedAccessors) { String name = accessorElement.name.replaceFirst('=', ''); @@ -893,7 +894,9 @@ class Class extends ModelElement implements EnclosedElement { /// Add a single Field to _fields. /// - /// Modifies accessorMap, removing the field added. + /// If [f] is not specified, pick the FieldElement from the PropertyAccessorElement + /// closer to the leaf of the inheritance tree (or the getter in case of a tie), + /// and construct a Field using that. void _addSingleField( PropertyAccessorElement getterElement, PropertyAccessorElement setterElement, @@ -931,7 +934,9 @@ class Class extends ModelElement implements EnclosedElement { enclosingClass: this, getter: getter, setter: setter)); } else { // Field is <100% inherited (could be half-inherited). - // TODO(jcollins-g): make half-inherited fields half-italicized...? + // TODO(jcollins-g): Navigation is probably still confusing for + // half-inherited fields when traversing the inheritance tree. Make + // this better, somehow. _fields.add( new ModelElement.from(f, library, getter: getter, setter: setter)); } @@ -2178,26 +2183,11 @@ abstract class ModelElement extends Nameable } if (e is TopLevelVariableElement) { if (getter == null && setter == null) { - // FIXME: so, what's wrong here? - if (e.getter != null) { - getter = new ModelElement.from(e.getter, library); - if (getter.enclosingCombo != null) { - newModelElement = getter.enclosingCombo; - } - } - if (e.setter != null) { - setter = new ModelElement.from(e.setter, library); - if (setter.enclosingCombo != null) { - if (newModelElement != null) { - assert(setter.enclosingCombo == getter.enclosingCombo); - } - newModelElement = setter.enclosingCombo; - } - } - } - assert(getter != null || setter != null); - if (newModelElement == null) + List allVariables = []..addAll(library.properties)..addAll(library.constants); + newModelElement = allVariables.firstWhere((v) => v.element == e); + } else { newModelElement = new TopLevelVariable(e, library, getter, setter); + } } if (e is PropertyAccessorElement) { if (e.enclosingElement is ClassElement) { @@ -2228,11 +2218,7 @@ abstract class ModelElement extends Nameable library.package._allInheritableElements[iKey].add(newModelElement); } } - if (newModelElement is Field) { - assert(getter == null || newModelElement.getter.enclosingCombo != null); - assert(setter == null || newModelElement.setter.enclosingCombo != null); - } - if (newModelElement is TopLevelVariable) { + if (newModelElement is GetterSetterCombo) { assert(getter == null || newModelElement.getter.enclosingCombo != null); assert(setter == null || newModelElement.setter.enclosingCombo != null); } From e572d077c27489095bb4d67fc686ee8be00420f5 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 21 Jun 2017 13:11:15 -0700 Subject: [PATCH 15/25] rebuild docs --- testing/test_package_docs/ex/Apple-class.html | 2 +- testing/test_package_docs/fake/fake-library.html | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/test_package_docs/ex/Apple-class.html b/testing/test_package_docs/ex/Apple-class.html index 81e2024800..7f69cd04fd 100644 --- a/testing/test_package_docs/ex/Apple-class.html +++ b/testing/test_package_docs/ex/Apple-class.html @@ -166,7 +166,7 @@

    Properties

    - The getter for s [...] + The getter for s
    read / write
    diff --git a/testing/test_package_docs/fake/fake-library.html b/testing/test_package_docs/fake/fake-library.html index eff709ce89..2662c0c910 100644 --- a/testing/test_package_docs/fake/fake-library.html +++ b/testing/test_package_docs/fake/fake-library.html @@ -104,7 +104,7 @@

    Classes

    ClassWithUnusualProperties
    - Classes with unusual properties? I don't think they exist. + Classes with unusual properties? I don't think they exist. [...]
    ConstantClass @@ -399,7 +399,7 @@

    Properties

    - The getter for setAndGet. [...] + The getter for setAndGet.
    read / write
    From e6627d6c477362377ef1d27b9e7f5b78e2b2e6cd Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 21 Jun 2017 14:03:18 -0700 Subject: [PATCH 16/25] Add a test for extended docs for explicit getter/setter fields --- test/model_test.dart | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/model_test.dart b/test/model_test.dart index afadc7a3dd..de11c71026 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -1289,6 +1289,13 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, expect(lengthX.fullyQualifiedName, 'fake.WithGetterAndSetter.lengthX'); }); + test('has extended documentation', () { + expect(lengthX.hasExtendedDocumentation, isTrue); + expect(lengthX.oneLineDoc, equals('Returns a length. [...]')); + expect(lengthX.documentation, contains('the fourth dimension')); + expect(lengthX.documentation, isNot(contains('[...]'))); + }); + test('has valid documentation', () { expect(mFromApple.hasDocumentation, isTrue); expect(mFromApple.documentation, "The read-write field `m`."); From 13041a9780d9b769f533e4748a12590fcd856e68 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 21 Jun 2017 14:32:55 -0700 Subject: [PATCH 17/25] Test tweaks and dartfmt --- lib/src/model.dart | 24 ++++++++++-------------- test/model_test.dart | 9 ++++++++- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index e7c9aba1b1..1ca00c4d70 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -439,13 +439,9 @@ class Class extends ModelElement implements EnclosedElement { return _allElements; } - bool contains(Element element) { - return allElements.containsKey(element); - } + bool contains(Element element) => allElements.containsKey(element); - ModelElement findModelElement(Element element) { - return allElements[element]; - } + ModelElement findModelElement(Element element) => allElements[element]; final Set _allModelElements = new Set(); List get allModelElements { @@ -764,12 +760,10 @@ class Class extends ModelElement implements EnclosedElement { // now, we're creating a flat list. We're not paying attention to where // these methods are actually coming from. This might turn out to be a // problem if we want to show that info later. - List get propertiesForPages { - return [] - ..addAll(instanceProperties) - ..addAll(inheritedProperties) - ..sort(byName); - } + List get propertiesForPages => [] + ..addAll(instanceProperties) + ..addAll(inheritedProperties) + ..sort(byName); List get staticMethods { if (_staticMethods != null) return _staticMethods; @@ -894,7 +888,7 @@ class Class extends ModelElement implements EnclosedElement { /// Add a single Field to _fields. /// /// If [f] is not specified, pick the FieldElement from the PropertyAccessorElement - /// closer to the leaf of the inheritance tree (or the getter in case of a tie), + /// whose enclosing class inherits from the other (defaulting to the getter) /// and construct a Field using that. void _addSingleField( PropertyAccessorElement getterElement, @@ -2182,7 +2176,9 @@ abstract class ModelElement extends Nameable } if (e is TopLevelVariableElement) { if (getter == null && setter == null) { - List allVariables = []..addAll(library.properties)..addAll(library.constants); + List allVariables = [] + ..addAll(library.properties) + ..addAll(library.constants); newModelElement = allVariables.firstWhere((v) => v.element == e); } else { newModelElement = new TopLevelVariable(e, library, getter, setter); diff --git a/test/model_test.dart b/test/model_test.dart index de11c71026..506d5a7245 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -1260,6 +1260,8 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, expect(implicitGetterExplicitSetter.isInherited, isFalse); expect(implicitGetterExplicitSetter.features.contains('inherited-getter'), isTrue); + expect(implicitGetterExplicitSetter.features.contains('read / write'), + isTrue); expect( implicitGetterExplicitSetter.oneLineDoc, equals( @@ -1276,6 +1278,8 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, expect(explicitGetterImplicitSetter.isInherited, isFalse); expect(explicitGetterImplicitSetter.features.contains('inherited-setter'), isTrue); + expect(explicitGetterImplicitSetter.features.contains('read / write'), + isTrue); expect(explicitGetterImplicitSetter.oneLineDoc, equals('Getter doc for explicitGetterImplicitSetter')); // Even though we have some new setter docs, getter still takes priority. @@ -1291,7 +1295,10 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans, test('has extended documentation', () { expect(lengthX.hasExtendedDocumentation, isTrue); - expect(lengthX.oneLineDoc, equals('Returns a length. [...]')); + expect( + lengthX.oneLineDoc, + equals( + 'Returns a length. [...]')); expect(lengthX.documentation, contains('the fourth dimension')); expect(lengthX.documentation, isNot(contains('[...]'))); }); From b65e7755ba46f0bcd3b069379227be43e1a874bc Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 21 Jun 2017 14:44:35 -0700 Subject: [PATCH 18/25] simplify an if statement --- lib/dartdoc.dart | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart index 548b6397b0..5f4852cf4f 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -239,10 +239,8 @@ class DartDoc { } warnOnElements = _hrefs[warnOn]; - if (referredFromElements != null) { - if (referredFromElements.any((e) => e.isCanonical)) { - referredFromElements.removeWhere((e) => !e.isCanonical); - } + if (referredFromElements.any((e) => e.isCanonical)) { + referredFromElements.removeWhere((e) => !e.isCanonical); } if (warnOnElements != null) { if (warnOnElements.any((e) => e.isCanonical)) { From 93f04ee4dce661dc2064aa07b706c90c32874503 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 22 Jun 2017 08:48:36 -0700 Subject: [PATCH 19/25] recover some performance --- lib/src/model.dart | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 1ca00c4d70..ac301e0f12 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -760,10 +760,16 @@ class Class extends ModelElement implements EnclosedElement { // now, we're creating a flat list. We're not paying attention to where // these methods are actually coming from. This might turn out to be a // problem if we want to show that info later. - List get propertiesForPages => [] - ..addAll(instanceProperties) - ..addAll(inheritedProperties) - ..sort(byName); + List _propertiesForPages; + List get propertiesForPages { + if (_propertiesForPages == null) { + _propertiesForPages = [] + ..addAll(instanceProperties) + ..addAll(inheritedProperties) + ..sort(byName); + } + return _propertiesForPages; + } List get staticMethods { if (_staticMethods != null) return _staticMethods; From e2624fafd25f29b22834b4c56ba2e5294680963a Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 22 Jun 2017 09:54:02 -0700 Subject: [PATCH 20/25] get back a little more performance --- lib/src/model.dart | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index ac301e0f12..f6f0d91ab4 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -1524,9 +1524,13 @@ class Library extends ModelElement { .toList(growable: false); } + List _constants; List get constants { - return _getVariables().where((v) => v.isConst).toList(growable: false) - ..sort(byName); + if (_constants == null) { + // _getVariables() is already sorted. + _constants = _getVariables().where((v) => v.isConst).toList(growable: false); + } + return _constants; } String get dirName => name.replaceAll(':', '-'); @@ -1624,7 +1628,7 @@ class Library extends ModelElement { bool get hasClasses => classes.isNotEmpty; - bool get hasConstants => _getVariables().any((v) => v.isConst); + bool get hasConstants => constants.isNotEmpty; bool get hasEnums => enums.isNotEmpty; @@ -1706,10 +1710,13 @@ class Library extends ModelElement { String get path => _libraryElement.definingCompilationUnit.name; + List _properties; /// All variables ("properties") except constants. List get properties { - return _getVariables().where((v) => !v.isConst).toList(growable: false) - ..sort(byName); + if (_properties == null) { + _properties = _getVariables().where((v) => !v.isConst).toList(growable: false); + } + return _properties; } List get typedefs { From d66ec18e1dc9300e2cc43f47bb4845b7da7bd70a Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 22 Jun 2017 11:56:03 -0700 Subject: [PATCH 21/25] dartfmt --- lib/src/model.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index f6f0d91ab4..dcfc50a0d6 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -1528,7 +1528,8 @@ class Library extends ModelElement { List get constants { if (_constants == null) { // _getVariables() is already sorted. - _constants = _getVariables().where((v) => v.isConst).toList(growable: false); + _constants = + _getVariables().where((v) => v.isConst).toList(growable: false); } return _constants; } @@ -1711,10 +1712,12 @@ class Library extends ModelElement { String get path => _libraryElement.definingCompilationUnit.name; List _properties; + /// All variables ("properties") except constants. List get properties { if (_properties == null) { - _properties = _getVariables().where((v) => !v.isConst).toList(growable: false); + _properties = + _getVariables().where((v) => !v.isConst).toList(growable: false); } return _properties; } From 9df8a572c0bfdaef5089b3d4ff18717526bc1b4d Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 22 Jun 2017 13:12:11 -0700 Subject: [PATCH 22/25] Fix minor problem with sdk build --- lib/src/model.dart | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index dcfc50a0d6..ad552563cd 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -270,12 +270,13 @@ class Accessor extends ModelElement bool get isGetter => _accessor.isGetter; + bool _overriddenElementIsSet = false; ModelElement _overriddenElement; - @override Accessor get overriddenElement { assert(package.allLibrariesAdded); - if (_overriddenElement == null) { + if (!_overriddenElementIsSet) { + _overriddenElementIsSet = true; Element parent = element.enclosingElement; if (parent is ClassElement) { for (InterfaceType t in getAllSupertypes(parent)) { @@ -293,14 +294,16 @@ class Accessor extends ModelElement possibleFields.addAll(parentClass.staticProperties); String fieldName = accessor.name.replaceFirst('=', ''); Field foundField = - possibleFields.firstWhere((f) => f.element.name == fieldName); - if (this.isGetter) { - _overriddenElement = foundField.getter; - } else { - _overriddenElement = foundField.setter; + possibleFields.firstWhere((f) => f.element.name == fieldName, orElse: () => null); + if (foundField != null) { + if (this.isGetter) { + _overriddenElement = foundField.getter; + } else { + _overriddenElement = foundField.setter; + } + assert(!(_overriddenElement as Accessor).isInherited); + break; } - assert(!(_overriddenElement as Accessor).isInherited); - break; } } } From 5aaf681b39b0e5c6192d2e70bf0c018f9d8ebda2 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 22 Jun 2017 13:12:30 -0700 Subject: [PATCH 23/25] dartfmt --- lib/src/model.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index ad552563cd..7a0bb39795 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -293,8 +293,9 @@ class Accessor extends ModelElement possibleFields.addAll(parentClass.allInstanceProperties); possibleFields.addAll(parentClass.staticProperties); String fieldName = accessor.name.replaceFirst('=', ''); - Field foundField = - possibleFields.firstWhere((f) => f.element.name == fieldName, orElse: () => null); + Field foundField = possibleFields.firstWhere( + (f) => f.element.name == fieldName, + orElse: () => null); if (foundField != null) { if (this.isGetter) { _overriddenElement = foundField.getter; From dc5aa11cdbf30461bdef9bd95b4189325c714a99 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 22 Jun 2017 14:25:25 -0700 Subject: [PATCH 24/25] Fix warning generation on non-canonical FieldElements --- lib/src/markdown_processor.dart | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index dadbd3ea55..15a3292b70 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -294,7 +294,20 @@ MatchingLinkResult _getMatchingLinkElement( } // From this point on, we haven't been able to find a canonical ModelElement. // So in this case, just find any ModelElement we can. - refModelElement = new ModelElement.from(searchElement, refLibrary); + Accessor getter; + Accessor setter; + if (searchElement is FieldElement) { + // TODO(jcollins-g): consolidate field element construction with inheritance + // checking. + if (searchElement.getter != null) { + getter = new ModelElement.from(searchElement.getter, refLibrary); + } + if (searchElement.setter != null) { + setter = new ModelElement.from(searchElement.setter, refLibrary); + } + } + refModelElement = new ModelElement.from(searchElement, refLibrary, + getter: getter, setter: setter); if (!refModelElement.isCanonical) { refModelElement .warn(PackageWarning.noCanonicalFound, referredFrom: [element]); From 18779c1a975e0379466db93e90e2997810bd7b95 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Wed, 5 Jul 2017 14:00:05 -0700 Subject: [PATCH 25/25] Add review comments --- lib/src/model.dart | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index 7a0bb39795..e646c37f03 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -2114,7 +2114,10 @@ abstract class ModelElement extends Nameable // clean that up. // TODO(jcollins-g): Refactor this into class-specific factories that // call this one. - // TODO(jcollins-g): enforce this. + // TODO(jcollins-g): Enforce construction restraint. + // TODO(jcollins-g): Allow e to be null and drop extraneous null checks. + // TODO(jcollins-g): Auto-vivify element's defining library for library + // parameter when given a null. /// Do not construct any ModelElements unless they are from this constructor. /// Specify enclosingClass only if this is to be an inherited object. /// Specify index only if this is to be an EnumField.forConstant. @@ -4405,12 +4408,6 @@ class TopLevelVariable extends ModelElement @override ModelElement get enclosingElement => library; - //@override - //bool get hasGetter => _variable.getter != null; - - //@override - //bool get hasSetter => _variable.setter != null; - @override String get href { if (canonicalLibrary == null) return null;