diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ef7c44c0c..e811c1c743 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 c368664f43..94dd366fd7 100644 --- a/bin/dartdoc.dart +++ b/bin/dartdoc.dart @@ -177,10 +177,10 @@ main(List arguments) async { 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 69d11da6ff..b0026ce6a9 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -44,7 +44,7 @@ export 'src/sdk.dart'; 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 @@ class DartDoc { } 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 visited) { diff --git a/lib/src/html/html_generator_instance.dart b/lib/src/html/html_generator_instance.dart index 436e6a234a..3de8f9b8a7 100644 --- a/lib/src/html/html_generator_instance.dart +++ b/lib/src/html/html_generator_instance.dart @@ -189,7 +189,7 @@ class HtmlGeneratorInstance implements HtmlOptions { 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 093f6647d1..4c1f441e54 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -164,7 +164,11 @@ ModelElement _getPreferredClass(ModelElement modelElement) { } // TODO: this is in the wrong place -NodeList _getCommentRefs(ModelElement modelElement) { +NodeList _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 @@ NodeList _getCommentRefs(ModelElement modelElement) { /// Returns null if element is a parameter. MatchingLinkResult _getMatchingLinkElement( - String codeRef, ModelElement element, List commentRefs) { + String codeRef, Documentable element, List 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 @@ MatchingLinkResult _getMatchingLinkElement( // 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 @@ MatchingLinkResult _getMatchingLinkElement( 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 @@ Element _getRefElementFromCommentRefs( 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 @@ Map> _findRefElementCache; // TODO(jcollins-g): A complex package winds up spending a lot of cycles in here. Optimize. Element _findRefElementInLibrary( String codeRef, ModelElement element, List commentRefs) { + assert(element != null); assert(element.package.allLibrariesAdded); String codeRefChomped = codeRef.replaceFirst(isConstructor, ''); @@ -625,14 +637,13 @@ void _getResultsForClass(Class tryClass, String codeRefChomped, } } -String _linkDocReference(String codeRef, ModelElement element, +String _linkDocReference(String codeRef, Documentable documentable, NodeList 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 @@ String _linkDocReference(String codeRef, ModelElement element, } } else { if (result.warn) { - element.warn(PackageWarning.unresolvedDocReference, codeRef); + documentable.warn(PackageWarning.unresolvedDocReference, codeRef); } return '${HTML_ESCAPE.convert(label)}'; } } -String _renderMarkdownToHtml(String text, [ModelElement element]) { +String _renderMarkdownToHtml(Documentable element) { md.Node _linkResolver(String name) { NodeList 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 @@ const maxPostContext = 30; // 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 tagPositions = findFreeHangingGenericsPositions(text); if (tagPositions.isNotEmpty) { tagPositions.forEach((int position) { @@ -732,13 +744,8 @@ class Documentation { 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 6c710ea393..8153456575 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -895,11 +895,13 @@ class Constructor extends ModelElement /// 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 @@ class Method extends ModelElement /// 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 @@ abstract class ModelElement 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 @@ abstract class ModelElement return _overriddenDepth; } + @override Package get package => (this is Library) ? (this as Library).package : this.library.package; @@ -2236,6 +2239,7 @@ abstract class ModelElement return _parameters; } + @override void warn(PackageWarning kind, [String message]) { if (kind == PackageWarning.unresolvedDocReference && overriddenElement != null) { @@ -2243,7 +2247,7 @@ abstract class ModelElement // 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 @@ Map> packageWarningText = { ], }; +// 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 PackageWarningCounter { } } -class Package implements Nameable, Documentable, Locatable { +class Package implements Nameable, Documentable { // Library objects serving as entry points for documentation. final List _libraries = []; // All library objects related to this package; a superset of _libraries. @@ -2911,6 +2920,12 @@ class Package implements Nameable, Documentable, Locatable { final PackageMeta packageMeta; + @override + Package get package => this; + + @override + Documentable get overriddenDocumentedElement => this; + final Map _elementToLibrary = {}; String _docsAsHtml; final Map _macros = {}; @@ -2958,29 +2973,33 @@ class Package implements Nameable, Documentable, Locatable { 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 @@ class Package implements Nameable, Documentable, Locatable { // 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 @@ class Package implements Nameable, Documentable, Locatable { 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 @@ class Package implements Nameable, Documentable, Locatable { 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 @@ class Package implements Nameable, Documentable, Locatable { // 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 packageCategories = result.values.toList()..sort(); @@ -3167,8 +3186,7 @@ class Package implements Nameable, Documentable, Locatable { @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 1192a831aa..ca637efd3c 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 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 e0b03ab81c..f507880160 100644 --- a/testing/test_package/README.md +++ b/testing/test_package/README.md @@ -23,6 +23,8 @@ and_yaml: - 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 c18a6073fc..51787a7668 100644 --- a/testing/test_package_docs/index.html +++ b/testing/test_package_docs/index.html @@ -4,7 +4,7 @@ - + test_package - Dart API docs @@ -92,6 +92,7 @@

Best Package

- "value" - 3.14 +

It sometimes generates warnings in commentRefs like this: unknownThingy.FromSomewhere

Be sure to check out other awesome packages on pub.