From 8138082a051a16ae1f6ec02af2db513a038a2243 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Mon, 13 Nov 2017 09:05:52 -0800 Subject: [PATCH 1/4] first piece --- bin/dartdoc.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/dartdoc.dart b/bin/dartdoc.dart index a90346b271..a4aca6beb1 100644 --- a/bin/dartdoc.dart +++ b/bin/dartdoc.dart @@ -174,7 +174,7 @@ main(List arguments) async { assert(message.isNotEmpty); if (record.level < logging.Level.WARNING) { - if (message.endsWith('...')) { + if (showProgress && message.endsWith('...')) { // Assume there may be more progress to print, so omit the trailing // newline writingProgress = true; From 1e4c0fa546727ad49db5f14ea87bc3151faaf169 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 14 Nov 2017 12:01:01 -0800 Subject: [PATCH 2/4] Finish warnings breakout --- lib/src/markdown_processor.dart | 21 ++++++++++++----- lib/src/model.dart | 40 +++++++++++++++++++++------------ lib/src/warnings.dart | 14 ++++++++++-- 3 files changed, 54 insertions(+), 21 deletions(-) diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index 5957b580df..d8aa91340f 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -137,6 +137,8 @@ final RegExp isConstructor = new RegExp(r'^new[\s]+', multiLine: true); // Covers anything with leading digits/symbols, empty string, weird punctuation, spaces. final RegExp notARealDocReference = new RegExp(r'''(^[^\w]|^[\d]|[,"'/]|^$)'''); +final RegExp operatorPrefix = new RegExp(r'^operator[ ]*'); + final HtmlEscape htmlEscape = const HtmlEscape(HtmlEscapeMode.ELEMENT); final List _markdown_syntaxes = [ @@ -397,8 +399,8 @@ Element _findRefElementInLibrary(String codeRef, ModelElement element, final Set results = new Set(); // This might be an operator. Strip the operator prefix and try again. - if (results.isEmpty && codeRef.startsWith('operator')) { - String newCodeRef = codeRef.replaceFirst('operator', ''); + if (results.isEmpty && codeRef.startsWith(operatorPrefix)) { + String newCodeRef = codeRef.replaceFirst(operatorPrefix, ''); return _findRefElementInLibrary( newCodeRef, element, commentRefs, preferredClass); } @@ -594,9 +596,13 @@ Element _findRefElementInLibrary(String codeRef, ModelElement element, } else if (results.length == 1) { result = results.first.element; } else { - element.warn(PackageWarning.ambiguousDocReference, - message: - "[$codeRef] => ${results.map((r) => "'${r.fullyQualifiedName}'").join(", ")}"); + // Squelch ambiguous doc reference warnings for parameters, because we + // don't link those anyway. + if (!results.every((r) => r is Parameter)) { + element.warn(PackageWarning.ambiguousDocReference, + message: + "[$codeRef] => ${results.map((r) => "'${r.fullyQualifiedName}'").join(", ")}"); + } result = results.first.element; } return result; @@ -635,6 +641,11 @@ void _getResultsForClass(Class tryClass, String codeRefChomped, for (final modelElement in c.allModelElements) { if (!_ConsiderIfConstructor(codeRef, modelElement)) continue; String namePart = modelElement.fullyQualifiedName.split('.').last; + if (modelElement is Accessor) { + // TODO(jcollins-g): Individual classes should be responsible for + // this name comparison munging. + namePart = namePart.split('=').first; + } // TODO(jcollins-g): fix operators so we can use 'name' here or similar. if (codeRefChomped == namePart) { results.add(package.findCanonicalModelElementFor( diff --git a/lib/src/model.dart b/lib/src/model.dart index 5b1bcd892c..f9b818df74 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -85,7 +85,7 @@ int byFeatureOrdering(String a, String b) { return compareAsciiLowerCaseNatural(a, b); } -final RegExp _locationSplitter = new RegExp(r"(package:|[\\/;.])"); +final RegExp locationSplitter = new RegExp(r"(package:|[\\/;.])"); /// Mixin for subclasses of ModelElement representing Elements that can be /// inherited from one class to another. @@ -2151,7 +2151,7 @@ ModelElement resolveMultiplyInheritedElement( /// helps prevent subtle bugs as generated output for a non-canonical /// ModelElement will reference itself as part of the "wrong" [Library] /// from the public interface perspective. -abstract class ModelElement extends Nameable +abstract class ModelElement extends Nameable with Warnable implements Comparable, Documentable { final Element _element; final Library _library; @@ -2325,14 +2325,6 @@ abstract class ModelElement extends Nameable return library.package.libraryElementReexportedBy[this.element.library]; } - Set get locationPieces { - return new Set() - ..addAll(element.location - .toString() - .split(_locationSplitter) - .where((s) => s.isNotEmpty)); - } - // Use components of this element's location to return a score for library // location. ScoredCandidate scoreElementWithLibrary(Library lib) { @@ -3318,7 +3310,7 @@ abstract class Nameable { String get name; Set get namePieces => new Set() - ..addAll(name.split(_locationSplitter).where((s) => s.isNotEmpty)); + ..addAll(name.split(locationSplitter).where((s) => s.isNotEmpty)); } class Operator extends Method { @@ -3379,7 +3371,7 @@ class Operator extends Method { String get typeName => 'operator'; } -class Package extends Nameable implements Documentable { +class Package extends Nameable with Documentable, Warnable { // Library objects serving as entry points for documentation. final List _libraries = []; @@ -3483,10 +3475,30 @@ class Package extends Nameable implements Documentable { extendedDebug: extendedDebug); } + final Set> _warnAlreadySeen = + new Set(); void warnOnElement(Warnable warnable, PackageWarning kind, {String message, - Iterable referredFrom, - Iterable extendedDebug}) { + Iterable referredFrom, + Iterable extendedDebug}) { + var newEntry = new Tuple3(warnable?.element, kind, message); + if (_warnAlreadySeen.contains(newEntry)) { + return; + } + // Warnings can cause other warnings. Queue them up via the stack but + // don't allow warnings we're already working on to get in there. + _warnAlreadySeen.add(newEntry); + _warnOnElement(warnable, kind, + message: message, + referredFrom: referredFrom, + extendedDebug: extendedDebug); + _warnAlreadySeen.remove(newEntry); + } + + void _warnOnElement(Warnable warnable, PackageWarning kind, + {String message, + Iterable referredFrom, + Iterable extendedDebug}) { if (warnable != null) { // This sort of warning is only applicable to top level elements. if (kind == PackageWarning.ambiguousReexport) { diff --git a/lib/src/warnings.dart b/lib/src/warnings.dart index fd137ccb0c..dc656dedd0 100644 --- a/lib/src/warnings.dart +++ b/lib/src/warnings.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analyzer/dart/element/element.dart'; +import 'package:dartdoc/src/model.dart'; import 'package:tuple/tuple.dart'; import 'config.dart'; @@ -82,10 +83,19 @@ final Map packageWarningText = const { }; /// Something that package warnings can be called on. +/// TODO(jcollins-g): Complete object model refactoring for #1524. abstract class Warnable implements Locatable { void warn(PackageWarning warning, {String message, Iterable referredFrom}); Warnable get enclosingElement; + + Set get locationPieces { + return new Set() + ..addAll(element.location + .toString() + .split(locationSplitter) + .where((s) => s.isNotEmpty)); + } } /// Something that can be located for warning purposes. @@ -240,7 +250,7 @@ class PackageWarningCounter { int get errorCount { return _warningCounts.keys .map((w) => options.asErrors.contains(w) ? _warningCounts[w] : 0) - .reduce((a, b) => a + b); + .fold(0, (a, b) => a + b); } int get warningCount { @@ -249,7 +259,7 @@ class PackageWarningCounter { options.asWarnings.contains(w) && !options.asErrors.contains(w) ? _warningCounts[w] : 0) - .reduce((a, b) => a + b); + .fold(0, (a, b) => a + b); } @override From 5c4439b4451ed776b7e4139008f1487929f72f19 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Tue, 14 Nov 2017 12:01:22 -0800 Subject: [PATCH 3/4] dartfmt --- lib/src/model.dart | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/src/model.dart b/lib/src/model.dart index f9b818df74..b0ae49b9eb 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -2151,7 +2151,8 @@ ModelElement resolveMultiplyInheritedElement( /// helps prevent subtle bugs as generated output for a non-canonical /// ModelElement will reference itself as part of the "wrong" [Library] /// from the public interface perspective. -abstract class ModelElement extends Nameable with Warnable +abstract class ModelElement extends Nameable + with Warnable implements Comparable, Documentable { final Element _element; final Library _library; @@ -3479,8 +3480,8 @@ class Package extends Nameable with Documentable, Warnable { new Set(); void warnOnElement(Warnable warnable, PackageWarning kind, {String message, - Iterable referredFrom, - Iterable extendedDebug}) { + Iterable referredFrom, + Iterable extendedDebug}) { var newEntry = new Tuple3(warnable?.element, kind, message); if (_warnAlreadySeen.contains(newEntry)) { return; @@ -3497,8 +3498,8 @@ class Package extends Nameable with Documentable, Warnable { void _warnOnElement(Warnable warnable, PackageWarning kind, {String message, - Iterable referredFrom, - Iterable extendedDebug}) { + Iterable referredFrom, + Iterable extendedDebug}) { if (warnable != null) { // This sort of warning is only applicable to top level elements. if (kind == PackageWarning.ambiguousReexport) { From e203dd60eca7d0a7fe3cfde3e18e1be39534bb93 Mon Sep 17 00:00:00 2001 From: Janice Collins Date: Thu, 16 Nov 2017 13:52:56 -0800 Subject: [PATCH 4/4] Review comment --- lib/src/warnings.dart | 9 ++++----- pubspec.lock | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/src/warnings.dart b/lib/src/warnings.dart index dc656dedd0..d2ba8401d6 100644 --- a/lib/src/warnings.dart +++ b/lib/src/warnings.dart @@ -90,11 +90,10 @@ abstract class Warnable implements Locatable { Warnable get enclosingElement; Set get locationPieces { - return new Set() - ..addAll(element.location - .toString() - .split(locationSplitter) - .where((s) => s.isNotEmpty)); + return new Set.from(element.location + .toString() + .split(locationSplitter) + .where((s) => s.isNotEmpty)); } } diff --git a/pubspec.lock b/pubspec.lock index d044023445..e0e550ebfa 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -350,4 +350,4 @@ packages: source: hosted version: "2.1.13" sdks: - dart: ">=1.23.0 <=2.0.0-dev.6.0" + dart: ">=1.23.0 <=2.0.0-dev.7.0"