eliminate crash on warnings generated by README.md (#1410)
* Fix readme warnings
* Update test to generate warnings from the readme
* dartfmt
* Shift around abstract classes to make this more understandable
* dartfmt
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0ef7c44..e811c1c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,6 @@
+## 0.11.2
+* Fix regression where warnings generated by the README could result in a fatal exception. #1409
+
## 0.11.1
* Fix regression where a property or top level variable can be listed twice
under some conditions. #1401
diff --git a/bin/dartdoc.dart b/bin/dartdoc.dart
index c368664..94dd366 100644
--- a/bin/dartdoc.dart
+++ b/bin/dartdoc.dart
@@ -177,10 +177,10 @@
print('\nSuccess! Docs generated into ${results.outDir.absolute.path}');
}, onError: (e, Chain chain) {
if (e is DartDocFailure) {
- stderr.writeln('Generation failed: ${e}.');
+ stderr.writeln('\nGeneration failed: ${e}.');
exit(1);
} else {
- stderr.writeln('Generation failed: ${e}\n${chain.terse}');
+ stderr.writeln('\nGeneration failed: ${e}\n${chain.terse}');
exit(255);
}
});
diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart
index 69d11da..b0026ce 100644
--- a/lib/dartdoc.dart
+++ b/lib/dartdoc.dart
@@ -44,7 +44,7 @@
const String name = 'dartdoc';
// Update when pubspec version changes.
-const String version = '0.11.1';
+const String version = '0.11.2';
final String defaultOutDir = path.join('doc', 'api');
@@ -241,7 +241,7 @@
}
if (referenceElement == null && source == 'index.html')
referenceElement = package;
- package.warn(referenceElement, kind, p);
+ package.warnOnElement(referenceElement, kind, p);
}
void _doOrphanCheck(Package package, String origin, Set<String> visited) {
diff --git a/lib/src/html/html_generator_instance.dart b/lib/src/html/html_generator_instance.dart
index 436e6a2..3de8f9b 100644
--- a/lib/src/html/html_generator_instance.dart
+++ b/lib/src/html/html_generator_instance.dart
@@ -189,7 +189,7 @@
stdout
.write('\ngenerating docs for library ${lib.name} from ${lib.path}...');
if (!lib.isAnonymous && !lib.hasDocumentation) {
- package.warn(lib, PackageWarning.noLibraryLevelDocs);
+ package.warnOnElement(lib, PackageWarning.noLibraryLevelDocs);
}
TemplateData data =
new LibraryTemplateData(this, package, lib, useCategories);
diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart
index 093f664..4c1f441 100644
--- a/lib/src/markdown_processor.dart
+++ b/lib/src/markdown_processor.dart
@@ -164,7 +164,11 @@
}
// TODO: this is in the wrong place
-NodeList<CommentReference> _getCommentRefs(ModelElement modelElement) {
+NodeList<CommentReference> _getCommentRefs(Documentable documentable) {
+ // Documentable items that aren't related to analyzer elements have no
+ // CommentReference list.
+ if (documentable is! ModelElement) return null;
+ ModelElement modelElement = documentable;
Class preferredClass = _getPreferredClass(modelElement);
ModelElement cModelElement = modelElement.package
.findCanonicalModelElementFor(modelElement.element,
@@ -219,7 +223,7 @@
/// Returns null if element is a parameter.
MatchingLinkResult _getMatchingLinkElement(
- String codeRef, ModelElement element, List<CommentReference> commentRefs) {
+ String codeRef, Documentable element, List<CommentReference> commentRefs) {
// By debugging inspection, it seems correct to not warn when we don't have
// CommentReferences; there's actually nothing that needs resolving in
// that case.
@@ -244,6 +248,9 @@
// dartdoc-canonicalization-aware?
if (refElement == null) {
refElement = _getRefElementFromCommentRefs(commentRefs, codeRef);
+ } else {
+ assert(refElement is! PropertyAccessorElement);
+ assert(refElement is! PrefixElement);
}
// Did not find it anywhere.
@@ -251,12 +258,6 @@
return new MatchingLinkResult(null, null);
}
- if (refElement is PropertyAccessorElement) {
- // yay we found an accessor that wraps a const, but we really
- // want the top-level field itself
- refElement = (refElement as PropertyAccessorElement).variable;
- }
-
// Ignore all parameters.
if (refElement is ParameterElement || refElement is TypeParameterElement)
return new MatchingLinkResult(null, null, warn: false);
@@ -300,7 +301,17 @@
bool isConstrElement = ref.identifier.staticElement is ConstructorElement;
// Constructors are now handled by library search.
if (!isConstrElement) {
- return ref.identifier.staticElement;
+ Element refElement = ref.identifier.staticElement;
+ if (refElement is PropertyAccessorElement) {
+ // yay we found an accessor that wraps a const, but we really
+ // want the top-level field itself
+ refElement = (refElement as PropertyAccessorElement).variable;
+ }
+ if (refElement is PrefixElement) {
+ // We found a prefix element, but what we really want is the library element.
+ refElement = (refElement as PrefixElement).enclosingElement;
+ }
+ return refElement;
}
}
}
@@ -340,6 +351,7 @@
// TODO(jcollins-g): A complex package winds up spending a lot of cycles in here. Optimize.
Element _findRefElementInLibrary(
String codeRef, ModelElement element, List<CommentReference> commentRefs) {
+ assert(element != null);
assert(element.package.allLibrariesAdded);
String codeRefChomped = codeRef.replaceFirst(isConstructor, '');
@@ -625,14 +637,13 @@
}
}
-String _linkDocReference(String codeRef, ModelElement element,
+String _linkDocReference(String codeRef, Documentable documentable,
NodeList<CommentReference> commentRefs) {
// TODO(jcollins-g): Refactor so that doc operations work on the
// documented element.
- element = element.overriddenDocumentedElement;
-
+ documentable = documentable.overriddenDocumentedElement;
MatchingLinkResult result;
- result = _getMatchingLinkElement(codeRef, element, commentRefs);
+ result = _getMatchingLinkElement(codeRef, documentable, commentRefs);
final ModelElement linkedElement = result.element;
final String label = result.label ?? codeRef;
if (linkedElement != null) {
@@ -649,18 +660,19 @@
}
} else {
if (result.warn) {
- element.warn(PackageWarning.unresolvedDocReference, codeRef);
+ documentable.warn(PackageWarning.unresolvedDocReference, codeRef);
}
return '<code>${HTML_ESCAPE.convert(label)}</code>';
}
}
-String _renderMarkdownToHtml(String text, [ModelElement element]) {
+String _renderMarkdownToHtml(Documentable element) {
md.Node _linkResolver(String name) {
NodeList<CommentReference> commentRefs = _getCommentRefs(element);
return new md.Text(_linkDocReference(name, element, commentRefs));
}
+ String text = element.documentation;
_showWarningsForGenericsOutsideSquareBracketsBlocks(text, element);
return md.markdownToHtml(text,
inlineSyntaxes: _markdown_syntaxes, linkResolver: _linkResolver);
@@ -676,7 +688,7 @@
// like a non HTML tag (a generic?) outside of a `[]` block.
// https://github.com/dart-lang/dartdoc/issues/1250#issuecomment-269257942
void _showWarningsForGenericsOutsideSquareBracketsBlocks(
- String text, ModelElement element) {
+ String text, Warnable element) {
List<int> tagPositions = findFreeHangingGenericsPositions(text);
if (tagPositions.isNotEmpty) {
tagPositions.forEach((int position) {
@@ -732,13 +744,8 @@
final String asHtml;
final String asOneLiner;
- factory Documentation(String markdown) {
- String tempHtml = _renderMarkdownToHtml(markdown);
- return new Documentation._internal(markdown, tempHtml);
- }
-
- factory Documentation.forElement(ModelElement element) {
- String tempHtml = _renderMarkdownToHtml(element.documentation, element);
+ factory Documentation.forElement(Documentable element) {
+ String tempHtml = _renderMarkdownToHtml(element);
return new Documentation._internal(element.documentation, tempHtml);
}
diff --git a/lib/src/model.dart b/lib/src/model.dart
index 6c710ea..8153456 100644
--- a/lib/src/model.dart
+++ b/lib/src/model.dart
@@ -895,11 +895,13 @@
/// Bridges the gap between model elements and packages,
/// both of which have documentation.
-abstract class Documentable {
+abstract class Documentable implements Warnable {
String get documentation;
String get documentationAsHtml;
bool get hasDocumentation;
String get oneLineDoc;
+ Documentable get overriddenDocumentedElement;
+ Package get package;
}
class Dynamic extends ModelElement {
@@ -1709,8 +1711,7 @@
/// 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
- implements Comparable, Nameable, Documentable, Locatable {
+abstract class ModelElement implements Comparable, Nameable, Documentable {
final Element _element;
final Library _library;
@@ -2152,6 +2153,7 @@
bool _overriddenDocumentedElementIsSet = false;
// TODO(jcollins-g): This method prefers canonical elements, but it isn't
// guaranteed and is probably the source of bugs or confusing warnings.
+ @override
ModelElement get overriddenDocumentedElement {
if (!_overriddenDocumentedElementIsSet) {
ModelElement found = this;
@@ -2180,6 +2182,7 @@
return _overriddenDepth;
}
+ @override
Package get package =>
(this is Library) ? (this as Library).package : this.library.package;
@@ -2236,6 +2239,7 @@
return _parameters;
}
+ @override
void warn(PackageWarning kind, [String message]) {
if (kind == PackageWarning.unresolvedDocReference &&
overriddenElement != null) {
@@ -2243,7 +2247,7 @@
// Attach the warning to that element to deduplicate.
overriddenElement.warn(kind, message);
} else {
- library.package.warn(this, kind, message);
+ library.package.warnOnElement(this, kind, message);
}
}
@@ -2763,6 +2767,11 @@
],
};
+// Something that package warnings can be called on.
+abstract class Warnable implements Locatable {
+ void warn(PackageWarning warning, [String message]);
+}
+
// Something that can be located for warning purposes.
abstract class Locatable {
String get fullyQualifiedName;
@@ -2889,7 +2898,7 @@
}
}
-class Package implements Nameable, Documentable, Locatable {
+class Package implements Nameable, Documentable {
// Library objects serving as entry points for documentation.
final List<Library> _libraries = [];
// All library objects related to this package; a superset of _libraries.
@@ -2911,6 +2920,12 @@
final PackageMeta packageMeta;
+ @override
+ Package get package => this;
+
+ @override
+ Documentable get overriddenDocumentedElement => this;
+
final Map<Element, Library> _elementToLibrary = {};
String _docsAsHtml;
final Map<String, String> _macros = {};
@@ -2958,29 +2973,33 @@
PackageWarningCounter get packageWarningCounter => _packageWarningCounter;
- void warn(Locatable modelElement, PackageWarning kind, [String message]) {
- if (modelElement != null) {
+ @override
+ void warn(PackageWarning kind, [String message]) {
+ warnOnElement(this, kind, message);
+ }
+
+ void warnOnElement(Warnable warnable, PackageWarning kind, [String message]) {
+ if (warnable != null) {
// This sort of warning is only applicable to top level elements.
if (kind == PackageWarning.ambiguousReexport) {
- Element topLevelElement = modelElement.element;
+ Element topLevelElement = warnable.element;
while (topLevelElement.enclosingElement is! CompilationUnitElement) {
topLevelElement = topLevelElement.enclosingElement;
}
- modelElement = new ModelElement.from(
+ warnable = new ModelElement.from(
topLevelElement, findOrCreateLibraryFor(topLevelElement));
}
- if (modelElement is Accessor) {
+ if (warnable is Accessor) {
// This might be part of a Field, if so, assign this warning to the field
// rather than the Accessor.
- if ((modelElement as Accessor).enclosingCombo != null)
- modelElement = (modelElement as Accessor).enclosingCombo;
+ if ((warnable as Accessor).enclosingCombo != null)
+ warnable = (warnable as Accessor).enclosingCombo;
}
} else {
// If we don't have an element, we need a message to disambiguate.
assert(message != null);
}
- if (_packageWarningCounter.hasWarning(
- modelElement?.element, kind, message)) {
+ if (_packageWarningCounter.hasWarning(warnable?.element, kind, message)) {
return;
}
// Elements that are part of the Dart SDK can have colons in their FQNs.
@@ -2991,9 +3010,9 @@
// them out doesn't work as well there since it might confuse
// the user, yet we still want IntelliJ to link properly.
String nameSplitFromColonPieces;
- if (modelElement != null) {
+ if (warnable != null) {
nameSplitFromColonPieces =
- modelElement.fullyQualifiedName.replaceFirst(':', '-');
+ warnable.fullyQualifiedName.replaceFirst(':', '-');
}
String warningMessage;
switch (kind) {
@@ -3014,7 +3033,7 @@
break;
case PackageWarning.noLibraryLevelDocs:
warningMessage =
- "${modelElement.fullyQualifiedName} has no library level documentation comments";
+ "${warnable.fullyQualifiedName} has no library level documentation comments";
break;
case PackageWarning.ambiguousDocReference:
warningMessage =
@@ -3047,9 +3066,9 @@
break;
}
String fullMessage =
- "${warningMessage} ${modelElement != null ? modelElement.elementLocation : ''}";
+ "${warningMessage} ${warnable != null ? warnable.elementLocation : ''}";
packageWarningCounter.addWarning(
- modelElement?.element, kind, message, fullMessage);
+ warnable?.element, kind, message, fullMessage);
}
static Package _withAutoIncludedDependencies(
@@ -3110,7 +3129,7 @@
// Help the user if they pass us a category that doesn't exist.
for (String categoryName in config.categoryOrder) {
if (!result.containsKey(categoryName))
- warn(null, PackageWarning.categoryOrderGivesMissingPackageName,
+ warnOnElement(null, PackageWarning.categoryOrderGivesMissingPackageName,
"${categoryName}, categories: ${result.keys.join(',')}");
}
List<PackageCategory> packageCategories = result.values.toList()..sort();
@@ -3167,8 +3186,7 @@
@override
String get documentationAsHtml {
if (_docsAsHtml != null) return _docsAsHtml;
-
- _docsAsHtml = new Documentation(documentation).asHtml;
+ _docsAsHtml = new Documentation.forElement(this).asHtml;
return _docsAsHtml;
}
diff --git a/pubspec.yaml b/pubspec.yaml
index 1192a83..ca637ef 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,6 +1,6 @@
name: dartdoc
# Also update the `version` field in lib/dartdoc.dart.
-version: 0.11.1
+version: 0.11.2
author: Dart Team <misc@dartlang.org>
description: A documentation generator for Dart.
homepage: https://github.com/dart-lang/dartdoc
diff --git a/testing/test_package/README.md b/testing/test_package/README.md
index e0b03ab..f507880 100644
--- a/testing/test_package/README.md
+++ b/testing/test_package/README.md
@@ -23,6 +23,8 @@
- 3.14
```
+It sometimes generates warnings in commentRefs like this: [unknownThingy.FromSomewhere]
+
Be sure to check out other awesome packages on [pub][].
[pub]: https://pub.dartlang.org
diff --git a/testing/test_package_docs/index.html b/testing/test_package_docs/index.html
index c18a607..51787a7 100644
--- a/testing/test_package_docs/index.html
+++ b/testing/test_package_docs/index.html
@@ -4,7 +4,7 @@
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1">
- <meta name="generator" content="made with love by dartdoc 0.11.1">
+ <meta name="generator" content="made with love by dartdoc 0.11.2">
<meta name="description" content="test_package API docs, for the Dart programming language.">
<title>test_package - Dart API docs</title>
@@ -92,6 +92,7 @@
- "value"
- 3.14
</code></pre>
+<p>It sometimes generates warnings in commentRefs like this: <code>unknownThingy.FromSomewhere</code></p>
<p>Be sure to check out other awesome packages on <a href="https://pub.dartlang.org">pub</a>.</p>
</section>
