diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart index 844012e232..9c83b85068 100644 --- a/lib/src/dartdoc_options.dart +++ b/lib/src/dartdoc_options.dart @@ -1291,9 +1291,6 @@ class DartdocOptionContext extends DartdocOptionContextBase List get excludePackages => optionSet['excludePackages'].valueAt(context); - bool get enhancedReferenceLookup => - optionSet['enhancedReferenceLookup'].valueAt(context); - String get flutterRoot => optionSet['flutterRoot'].valueAt(context); bool get hideSdkText => optionSet['hideSdkText'].valueAt(context); @@ -1427,17 +1424,6 @@ Future> createDartdocOptions( help: 'Library names to ignore.', splitCommas: true), DartdocOptionArgOnly>('excludePackages', [], resourceProvider, help: 'Package names to ignore.', splitCommas: true), - DartdocOptionArgFile( - 'enhancedReferenceLookup', true, resourceProvider, - hide: true, - help: - 'Use the new comment reference lookup system instead of the legacy ' - 'version. Please file a bug referencing this flag if you have to ' - 'change it to false -- this flag and associated behavior will ' - 'disappear in a future version.', - negatable: true), - // This could be a ArgOnly, but trying to not provide too many ways - // to set the flutter root. DartdocOptionSyntheticOnly( 'flutterRoot', (DartdocSyntheticOption option, Folder dir) => resourceProvider diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart index 1c0afd7366..e70e49223a 100644 --- a/lib/src/generator/templates.runtime_renderers.dart +++ b/lib/src/generator/templates.runtime_renderers.dart @@ -3357,20 +3357,6 @@ class _Renderer_Container extends RendererBase { parent: r)); }, ), - 'allModelElementsByNamePart': Property( - getValue: (CT_ c) => c.allModelElementsByNamePart, - renderVariable: (CT_ c, Property self, - List remainingNames) => - self.renderSimpleVariable( - c, remainingNames, 'Map>'), - isNullValue: (CT_ c) => c.allModelElementsByNamePart == null, - renderValue: (CT_ c, RendererBase r, - List ast, StringSink sink) { - renderSimple( - c.allModelElementsByNamePart, ast, r.template, sink, - parent: r, getters: _invisibleGetters['Map']); - }, - ), 'constantFields': Property( getValue: (CT_ c) => c.constantFields, renderVariable: (CT_ c, Property self, @@ -7460,19 +7446,6 @@ class _Renderer_Library extends RendererBase { parent: r, getters: _invisibleGetters['HashMap']); }, ), - 'modelElementsNameMap': Property( - getValue: (CT_ c) => c.modelElementsNameMap, - renderVariable: (CT_ c, Property self, - List remainingNames) => - self.renderSimpleVariable(c, remainingNames, - 'HashMap>'), - isNullValue: (CT_ c) => c.modelElementsNameMap == null, - renderValue: (CT_ c, RendererBase r, - List ast, StringSink sink) { - renderSimple(c.modelElementsNameMap, ast, r.template, sink, - parent: r, getters: _invisibleGetters['HashMap']); - }, - ), 'name': Property( getValue: (CT_ c) => c.name, renderVariable: @@ -14803,6 +14776,7 @@ const _invisibleGetters = { 'DartType': { 'hashCode', 'runtimeType', + 'alias', 'aliasArguments', 'aliasElement', 'displayName', @@ -14842,7 +14816,6 @@ const _invisibleGetters = { 'examplePathPrefix', 'exclude', 'excludePackages', - 'enhancedReferenceLookup', 'flutterRoot', 'hideSdkText', 'include', @@ -15112,6 +15085,7 @@ const _invisibleGetters = { 'LibraryElement': { 'hashCode', 'runtimeType', + 'accessibleExtensions', 'definingCompilationUnit', 'entryPoint', 'exportedLibraries', @@ -15239,7 +15213,6 @@ const _invisibleGetters = { 'implementors', 'documentedExtensions', 'extensions', - 'findRefElementCache', 'defaultPackageName', 'defaultPackage', 'hasFooterVersion', diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart index 74d7f9c2ea..07a4ca7562 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -8,9 +8,7 @@ library dartdoc.markdown_processor; import 'dart:convert'; import 'dart:math'; -import 'package:analyzer/dart/element/element.dart'; import 'package:dartdoc/src/comment_references/model_comment_reference.dart'; -import 'package:dartdoc/src/element_type.dart'; import 'package:dartdoc/src/matching_link_result.dart'; import 'package:dartdoc/src/model/comment_referable.dart'; import 'package:dartdoc/src/model/model.dart'; @@ -18,7 +16,7 @@ import 'package:dartdoc/src/warnings.dart'; import 'package:markdown/markdown.dart' as md; import 'package:meta/meta.dart'; -const validHtmlTags = [ +const _validHtmlTags = [ 'a', 'abbr', 'address', @@ -119,41 +117,10 @@ const validHtmlTags = [ 'wbr' ]; -final RegExp nonHTML = - RegExp(" ])\\w+[> ]"); +final RegExp _nonHTML = + RegExp(" ])\\w+[> ]"); -/// Things to ignore at the end of a doc reference. -/// -/// This is intended to catch type parameters/arguments, and function call -/// parentheses. -final _trailingIgnorePattern = RegExp(r'(<.*>|\(.*\)|\?|!)$'); - -/// Things to ignore at the beginning of a doc reference. -/// -/// This is intended to catch various keywords that a developer may include in -/// front of a variable name. -// TODO(srawlins): I cannot find any tests asserting we can resolve such -// references. -final _leadingIgnorePattern = - RegExp(r'^(const|final|var)[\s]+', multiLine: true); - -/// If found, this pattern may indicate a reference to a constructor. -final _constructorIndicationPattern = - RegExp(r'(^new[\s]+|\(\)$)', multiLine: true); - -/// A pattern indicating that text which looks like a doc reference is not -/// intended to be. -/// -/// This covers anything with leading digits/symbols, empty strings, weird -/// punctuation, spaces. -/// -/// The idea is to catch such cases and not produce warnings about the contents. -// TODO(jcollins-g): consider being more strict here. -final RegExp notARealDocReference = RegExp(r'''(^[^\w]|^[\d]|[,"'/]|^$)'''); - -final RegExp operatorPrefix = RegExp(r'^operator[ ]*'); - -final HtmlEscape htmlEscape = const HtmlEscape(HtmlEscapeMode.element); +final HtmlEscape _htmlEscape = const HtmlEscape(HtmlEscapeMode.element); final List _markdownSyntaxes = [ _InlineCodeSyntax(), @@ -173,8 +140,8 @@ final List _markdownBlockSyntaxes = [ // Remove these schemas from the display text for hyperlinks. final RegExp _hideSchemes = RegExp('^(http|https)://'); -class IterableBlockParser extends md.BlockParser { - IterableBlockParser(List lines, md.Document document) +class _IterableBlockParser extends md.BlockParser { + _IterableBlockParser(List lines, md.Document document) : super(lines, document); Iterable parseLinesGenerator() sync* { @@ -190,17 +157,6 @@ class IterableBlockParser extends md.BlockParser { } } -// Calculate a class hint for findCanonicalModelElementFor. -ModelElement _getPreferredClass(ModelElement modelElement) { - if (modelElement is EnclosedElement && - (modelElement as EnclosedElement).enclosingElement is Container) { - return (modelElement as EnclosedElement).enclosingElement; - } else if (modelElement is Class) { - return modelElement; - } - return null; -} - /// Return false if the passed [referable] is a default [Constructor], /// or if it is shadowing another type of element, or is a parameter of /// one of the above. @@ -250,7 +206,7 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable( } else if (commentReference.hasConstructorHint && commentReference.hasCallableHint) { // This takes precedence over the callable hint if both are present -- - // pick a constructor and only constructor if we see `new`. + // pick a constructor if and only constructor if we see `new`. filter = _requireConstructor; } else if (commentReference.hasCallableHint) { // Trailing parens indicate we are looking for a callable. @@ -272,646 +228,9 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable( return MatchingLinkResult(lookupResult); } -/// Returns null if element is a parameter. -MatchingLinkResult _getMatchingLinkElementLegacy( - String codeRef, Warnable warnable) { - if (!codeRef.contains(_constructorIndicationPattern) && - codeRef.contains(notARealDocReference)) { - // Don't waste our time on things we won't ever find. - return MatchingLinkResult(null, warn: false); - } - - ModelElement refModelElement; - - // Try expensive not-scoped lookup. - if (refModelElement == null && warnable is ModelElement) { - Container preferredClass = _getPreferredClass(warnable); - // We might still get here in comparison mode, don't complain if that's - // the case. - if (preferredClass is Extension && - warnable.config.enhancedReferenceLookup == false) { - warnable.warn(PackageWarning.notImplemented, - message: - 'Comment reference resolution inside extension methods is not yet implemented'); - } else { - refModelElement = - _MarkdownCommentReference(codeRef, warnable, preferredClass) - .computeReferredElement(); - } - } - - // Did not find it anywhere. - if (refModelElement == null) { - // TODO(jcollins-g): remove squelching of non-canonical warnings here - // once we no longer process full markdown for - // oneLineDocs (#1417) - return MatchingLinkResult(null, warn: warnable.isCanonical); - } - - // Ignore all parameters. - if (refModelElement is Parameter || refModelElement is TypeParameter) { - return MatchingLinkResult(null, warn: false); - } - - // There have been places in the code which helpfully cache entities - // regardless of what package they are associated with. This assert - // will protect us from reintroducing that. - assert(refModelElement == null || - refModelElement.packageGraph == warnable.packageGraph); - if (refModelElement != null) { - return MatchingLinkResult(refModelElement); - } - // From this point on, we haven't been able to find a canonical ModelElement. - if (!refModelElement.isCanonical) { - if (refModelElement.library.isPublicAndPackageDocumented) { - refModelElement - .warn(PackageWarning.noCanonicalFound, referredFrom: [warnable]); - } - // Don't warn about doc references because that's covered by the no - // canonical library found message. - return MatchingLinkResult(null, warn: false); - } - // We should never get here unless there's a bug in findCanonicalModelElementFor. - // findCanonicalModelElementFor(searchElement, preferredClass: preferredClass) - // should only return null if ModelElement.from(searchElement, refLibrary) - // would return a non-canonical element. However, outside of checked mode, - // at least we have a canonical element, so proceed. - assert(false); - return MatchingLinkResult(refModelElement); -} - -/// Get the element referred by the [codeRef] in analyzer. -/// Deletes constructors and otherwise messes with the output for the rest -/// of the heuristics. -Element _getRefElementFromCommentRefs(Warnable element, String codeRef) { - if (element.commentRefs != null) { - var ref = element.commentRefs[codeRef]; - // ref can be null here if the analyzer failed to recognize this as a - // comment reference (e.g. [Foo.operator []]). - if (ref != null) { - // Constructors are now handled by library search. - if (ref.staticElement is! ConstructorElement) { - var refElement = ref.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; - } - } - } - return null; -} - -/// Represents a single comment reference. -class _MarkdownCommentReference { - /// The code reference text. - final String codeRef; - - /// The element containing the code reference. - final Warnable element; - - /// Disambiguate inheritance with this class. - final Class preferredClass; - - /// Current results. Input/output of all _find and _reduce methods. - Set results; - - /// codeRef with any leading constructor string, stripped. - String codeRefChomped; - - /// Library associated with this element. - Library library; - - /// PackageGraph associated with this element. - PackageGraph packageGraph; - - _MarkdownCommentReference(this.codeRef, this.element, this.preferredClass) { - assert(element != null); - assert(element.packageGraph.allLibrariesAdded); - - codeRefChomped = codeRef.replaceAll(_constructorIndicationPattern, ''); - library = - element is ModelElement ? (element as ModelElement).library : null; - packageGraph = library.packageGraph; - } - - String __impliedUnnamedConstructor; - - /// [_impliedUnnamedConstructor] is memoized in [__impliedUnnamedConstructor], - /// but even after it is initialized, it may be null. This bool represents the - /// initialization state. - bool __impliedUnnamedConstructorIsSet = false; - - /// Returns the name of the implied unnamed constructor if there is one, or - /// null if not. - /// - /// Unnamed constructors are a special case in dartdoc. If we look up a name - /// within a class of that class itself, the first thing we find is the - /// unnamed constructor. But we determine whether that's what they actually - /// intended (vs. the enclosing class) by context -- whether they seem - /// to be calling it with () or have a 'new' in front of it, or - /// whether the name is repeated. - /// - /// Similarly, referencing a class by itself might actually refer to its - /// unnamed constructor based on these same heuristics. - /// - /// With the name of the implied unnamed constructor, other methods can - /// determine whether or not the constructor and/or class we resolved to - /// is actually matching the user's intent. - String get _impliedUnnamedConstructor { - if (!__impliedUnnamedConstructorIsSet) { - __impliedUnnamedConstructorIsSet = true; - if (codeRef.contains(_constructorIndicationPattern) || - (codeRefChompedParts.length >= 2 && - codeRefChompedParts[codeRefChompedParts.length - 1] == - codeRefChompedParts[codeRefChompedParts.length - 2])) { - // If the last two parts of the code reference are equal, this is probably a default constructor. - __impliedUnnamedConstructor = codeRefChompedParts.last; - } - } - return __impliedUnnamedConstructor; - } - - /// Calculate reference to a ModelElement. - /// - /// Uses a series of calls to the _find* methods in this class to get one - /// or more possible [ModelElement] matches, then uses the _reduce* methods - /// in this class to try to bring it to a single ModelElement. Calls - /// [element.warn] for [PackageWarning.ambiguousDocReference] if there - /// are more than one, but does not warn otherwise. - ModelElement computeReferredElement() { - results = {}; - // TODO(jcollins-g): A complex package winds up spending a lot of cycles in - // here. Optimize. - for (var findMethod in [ - // This might be an operator. Strip the operator prefix and try again. - _findWithoutOperatorPrefix, - // Oh, and someone might have thrown on a 'const' or 'final' in front. - _findWithoutLeadingIgnoreStuff, - // Maybe this ModelElement has parameters, and this is one of them. - // We don't link these, but this keeps us from emitting warnings. Be sure - // to get members of parameters too. - _findParameters, - // Maybe this ModelElement has type parameters, and this is one of them. - _findTypeParameters, - // This could be local to the class, look there first. - _findWithinTryClasses, - // This could be a reference to a renamed library. - _findReferenceFromPrefixes, - // We now need the ref element cache to keep from repeatedly searching - // [Package.allModelElements]. But if not, look for a fully qualified - // match. (That only makes sense if the code reference might be - // qualified, and contains periods.) - _findWithinRefElementCache, - // Only look for partially qualified matches if we didn't find a fully - // qualified one. - _findPartiallyQualifiedMatches, - // Only look for partially qualified matches if we didn't find a fully - // qualified one. - _findGlobalWithinRefElementCache, - // This could conceivably be a reference to an enum member. They don't - // show up in [allModelElements]. - _findEnumReferences, - // Oh, and someone might have some type parameters or other garbage. - // After finding within classes because sometimes parentheses are used to - // imply constructors. - _findWithoutTrailingIgnoreStuff, - // Use the analyzer to resolve a comment reference. - _findAnalyzerReferences, - ]) { - findMethod(); - // Remove any "null" objects after each step of trying to add to results. - // TODO(jcollins-g): Eliminate all situations where nulls can be added - // to the results set. - results.remove(null); - if (results.isNotEmpty) break; - } - - if (results.length > 1) { - // This isn't C++. References to class methods are slightly expensive - // in Dart so don't build that list unless you need to. - for (var reduceMethod in [ - // If a result is actually in this library, prefer that. - _reducePreferResultsInSameLibrary, - // If a result is accessible in this library, prefer that. - _reducePreferResultsAccessibleInSameLibrary, - // This may refer to an element with the same name in multiple libraries - // in an external package, e.g. Matrix4 in vector_math and - // vector_math_64. Disambiguate by attempting to figure out which of - // them our package is actually using by checking the import/export - // graph. - _reducePreferLibrariesInLocalImportExportGraph, - // If a result's fully qualified name has pieces of the comment - // reference, prefer that. - _reducePreferReferencesIncludingFullyQualifiedName, - // If the reference is indicated to be a constructor, prefer - // constructors. This is not as generic as it sounds; very few naming - // conflicts are allowed, but an instance field is allowed to have the - // same name as a named constructor. - _reducePreferConstructorViaIndicators, - // Prefer Fields/TopLevelVariables to accessors. - // TODO(jcollins-g): Remove after fixing dart-lang/dartdoc#2396 or - // exclude Accessors from all lookup tables. - _reducePreferCombos, - // Prefer the Dart analyzer's resolution of comment references. We - // can't start from this because of the differences in Dartdoc - // canonicalization. - _reducePreferAnalyzerResolution, - ]) { - reduceMethod(); - if (results.length <= 1) break; - } - } - - ModelElement result; - // TODO(jcollins-g): further disambiguations based on package information? - if (results.isEmpty) { - result = null; - } else if (results.length == 1) { - result = results.first; - } else { - // Squelch ambiguous doc reference warnings for parameters, because we - // don't link those anyway. - if (!results.every((r) => r is Parameter)) { - var elementNames = results.map((r) => "'${r.fullyQualifiedName}'"); - element.warn(PackageWarning.ambiguousDocReference, - message: '[$codeRef] => ${elementNames.join(', ')}'); - } - result = results.first; - } - return result; - } - - List _codeRefParts; - - List get codeRefParts => _codeRefParts ??= codeRef.split('.'); - - List _codeRefChompedParts; - - List get codeRefChompedParts => - _codeRefChompedParts ??= codeRefChomped.split('.'); - - void _reducePreferAnalyzerResolution() { - var refElement = _getRefElementFromCommentRefs(element, codeRef); - if (results.any((me) => me.element == refElement)) { - results.removeWhere((me) => me.element != refElement); - } - } - - void _reducePreferConstructorViaIndicators() { - if (codeRef.contains(_constructorIndicationPattern) && - codeRefChompedParts.length >= 2) { - results.removeWhere((r) => r is! Constructor); - } - } - - void _reducePreferReferencesIncludingFullyQualifiedName() { - var startName = '${element.fullyQualifiedName}.'; - var realName = '${element.fullyQualifiedName}.$codeRefChomped'; - if (results.any((r) => r.fullyQualifiedName == realName)) { - results.removeWhere((r) => r.fullyQualifiedName != realName); - } - if (results.any((r) => r.fullyQualifiedName.startsWith(startName))) { - results.removeWhere((r) => !r.fullyQualifiedName.startsWith(startName)); - } - } - - void _reducePreferLibrariesInLocalImportExportGraph() { - if (results.any( - (r) => library.packageImportedExportedLibraries.contains(r.library))) { - results.removeWhere( - (r) => !library.packageImportedExportedLibraries.contains(r.library)); - } - } - - void _reducePreferResultsAccessibleInSameLibrary() { - // TODO(jcollins-g): we could have saved ourselves some work by using the analyzer - // to search the namespace, somehow. Do that instead. - if (element is ModelElement && - results.any((r) => r.element - .isAccessibleIn((element as ModelElement).library.element))) { - results.removeWhere((r) => - !r.element.isAccessibleIn((element as ModelElement).library.element)); - } - } - - void _reducePreferResultsInSameLibrary() { - if (results.any((r) => r.library?.packageName == library.packageName)) { - results.removeWhere((r) => r.library?.packageName != library.packageName); - } - } - - void _reducePreferCombos() { - var accessors = results.whereType().toList(); - accessors.forEach((a) { - results.remove(a); - results.add(a.enclosingCombo); - }); - } - - void _findTypeParameters() { - if (element is TypeParameters) { - results.addAll((element as TypeParameters).typeParameters.where((p) => - p.name == codeRefChomped || codeRefChomped.startsWith('${p.name}.'))); - } - } - - void _findParameters() { - if (element is ModelElement) { - results.addAll((element as ModelElement).allParameters.where((p) => - p.name == codeRefChomped || codeRefChomped.startsWith('${p.name}.'))); - } - } - - void _findWithoutLeadingIgnoreStuff() { - if (codeRef.contains(_leadingIgnorePattern)) { - var newCodeRef = codeRef.replaceFirst(_leadingIgnorePattern, ''); - results.add(_MarkdownCommentReference(newCodeRef, element, preferredClass) - .computeReferredElement()); - } - } - - void _findWithoutTrailingIgnoreStuff() { - if (codeRef.contains(_trailingIgnorePattern)) { - var newCodeRef = codeRef.replaceFirst(_trailingIgnorePattern, ''); - results.add(_MarkdownCommentReference(newCodeRef, element, preferredClass) - .computeReferredElement()); - } - } - - void _findWithoutOperatorPrefix() { - if (codeRef.startsWith(operatorPrefix)) { - var newCodeRef = codeRef.replaceFirst(operatorPrefix, ''); - results.add(_MarkdownCommentReference(newCodeRef, element, preferredClass) - .computeReferredElement()); - } - } - - void _findEnumReferences() { - // TODO(jcollins-g): Put enum members in allModelElements with useful hrefs without blowing up other assumptions about what that means. - // TODO(jcollins-g): This doesn't provide good warnings if an enum and class have the same name in different libraries in the same package. Fix that. - if (codeRefChompedParts.length >= 2) { - var maybeEnumName = codeRefChompedParts - .sublist(0, codeRefChompedParts.length - 1) - .join('.'); - var maybeEnumMember = codeRefChompedParts.last; - if (packageGraph.findRefElementCache.containsKey(maybeEnumName)) { - for (final modelElement - in packageGraph.findRefElementCache[maybeEnumName]) { - if (modelElement is Enum) { - if (modelElement.constantFields - .any((e) => e.name == maybeEnumMember)) { - results.add(modelElement); - break; - } - } - } - } - } - } - - /// Returns the unnamed constructor for class [toConvert] or the class for - /// constructor [toConvert], or just [toConvert], based on heuristics. - /// - /// * If an unnamed constructor is implied in the comment reference, and - /// [toConvert] is a class with the same name, the class's unnamed - /// constructor is returned. - /// * Otherwise, if [toConvert] is an unnamed constructor, its enclosing - /// class is returned. - /// * Otherwise, [toConvert] is returned. - ModelElement _convertConstructors(ModelElement toConvert) { - if (_impliedUnnamedConstructor != null) { - if (toConvert is Class && toConvert.name == _impliedUnnamedConstructor) { - return toConvert.unnamedConstructor; - } - return toConvert; - } else { - if (toConvert is Constructor && - (toConvert.enclosingElement as Class).unnamedConstructor == - toConvert) { - return toConvert.enclosingElement; - } - return toConvert; - } - } - - void _findReferenceFromPrefixes() { - if (element is! ModelElement) return; - var prefixToLibrary = - (element as ModelElement).definingLibrary.prefixToLibrary; - if (prefixToLibrary.containsKey(codeRefChompedParts.first)) { - if (codeRefChompedParts.length == 1) { - results.addAll(prefixToLibrary[codeRefChompedParts.first]); - } else { - var lookup = codeRefChompedParts.sublist(1).join('.'); - prefixToLibrary[codeRefChompedParts.first]?.forEach((l) => l - .modelElementsNameMap[lookup] - ?.map(_convertConstructors) - ?.forEach((m) => _addCanonicalResult(m))); - } - } - } - - void _findGlobalWithinRefElementCache() { - if (packageGraph.findRefElementCache.containsKey(codeRefChomped)) { - for (final modelElement - in packageGraph.findRefElementCache[codeRefChomped]) { - if (codeRefChomped == modelElement.fullyQualifiedNameWithoutLibrary || - (modelElement is Library && - codeRefChomped == modelElement.fullyQualifiedName)) { - _addCanonicalResult(_convertConstructors(modelElement)); - } - } - } - } - - void _findPartiallyQualifiedMatches() { - // Only look for partially qualified matches if we didn't find a fully qualified one. - if (library.modelElementsNameMap.containsKey(codeRefChomped)) { - for (final modelElement in library.modelElementsNameMap[codeRefChomped]) { - _addCanonicalResult(_convertConstructors(modelElement)); - } - } - } - - void _findWithinRefElementCache() { - // We now need the ref element cache to keep from repeatedly searching [Package.allModelElements]. - // But if not, look for a fully qualified match. (That only makes sense - // if the codeRef might be qualified, and contains periods.) - if (codeRefChomped.contains('.') && - packageGraph.findRefElementCache.containsKey(codeRefChomped)) { - for (var modelElement - in packageGraph.findRefElementCache[codeRefChomped]) { - _addCanonicalResult(_convertConstructors(modelElement)); - } - } - } - - void _findWithinTryClasses() { - // Maybe this is local to a class. - // TODO(jcollins-g): tryClasses is a strict subset of the superclass chain. Optimize. - var tryClasses = [preferredClass]; - var realClass = tryClasses.first; - if (element is Inheritable) { - var overriddenElement = (element as Inheritable).overriddenElement; - while (overriddenElement != null) { - tryClasses - .add((element as Inheritable).overriddenElement.enclosingElement); - overriddenElement = overriddenElement.overriddenElement; - } - } - - for (var tryClass in tryClasses) { - if (tryClass != null) { - if (codeRefChomped.contains('.') && - !codeRefChomped.startsWith(tryClass.name)) { - continue; - } - _getResultsForClass(tryClass); - } - results.remove(null); - if (results.isNotEmpty) break; - } - - if (results.isEmpty && realClass != null) { - for (var superClass - in realClass.publicSuperChain.map((et) => et.modelElement)) { - if (!tryClasses.contains(superClass)) { - _getResultsForClass(superClass); - } - results.remove(null); - if (results.isNotEmpty) break; - } - } - } - - void _findAnalyzerReferences() { - var refElement = _getRefElementFromCommentRefs(element, codeRef); - if (refElement == null) return; - - ModelElement refModelElement; - if (refElement is MultiplyDefinedElement) { - var elementNames = refElement.conflictingElements - .map((e) => "'${_fullyQualifiedElementName(e)}'"); - element.warn(PackageWarning.ambiguousDocReference, - message: '[$codeRef] => [${elementNames.join(', ')}]'); - refModelElement = ModelElement.fromElement( - // Continue; just use the first conflicting element. - refElement.conflictingElements.first, - element.packageGraph); - } else { - refModelElement = - ModelElement.fromElement(refElement, element.packageGraph); - } - if (refModelElement is Accessor) { - refModelElement = (refModelElement as Accessor).enclosingCombo; - } - refModelElement = refModelElement.canonicalModelElement ?? refModelElement; - results.add(refModelElement); - } - - /// Generates a fully-qualified name, similar to that of - /// [ModelElement.fullyQualifiedName], for an Element. - static String _fullyQualifiedElementName(Element element) { - var enclosingElement = element.enclosingElement; - - var enclosingName = enclosingElement == null - ? null - : _fullyQualifiedElementName(enclosingElement); - var name = element.name; - if (name == null) { - if (element is ExtensionElement) { - name = ''; - } else if (element is LibraryElement) { - name = ''; - } else if (element is CompilationUnitElement) { - return enclosingName; - } else { - name = ''; - } - } - - return enclosingName == null ? name : '$enclosingName.$name'; - } - - // Add a result, but make it canonical. - void _addCanonicalResult(ModelElement modelElement) { - results.add(modelElement.canonicalModelElement); - } - - /// _getResultsForClass assumes codeRefChomped might be a member of tryClass (inherited or not) - /// and will add to [results] - void _getResultsForClass(Class tryClass) { - // This might be part of the type arguments for the class, if so, add them. - // Otherwise, search the class. - if ((tryClass.modelType.typeArguments.map((e) => e.name)) - .contains(codeRefChomped)) { - results.add((tryClass.modelType.typeArguments.firstWhere( - (e) => e.name == codeRefChomped && e is DefinedElementType) - as DefinedElementType) - .modelElement); - } else { - // People like to use 'this' in docrefs too. - if (codeRef == 'this') { - _addCanonicalResult(tryClass); - } else { - // TODO(jcollins-g): get rid of reimplementation of identifier resolution - // or integrate into ModelElement in a simpler way. - var superChain = [tryClass]; - superChain.addAll(tryClass.interfaces.map((t) => t.modelElement)); - // This seems duplicitous with our caller, but the preferredClass - // hint matters with findCanonicalModelElementFor. - // TODO(jcollins-g): This makes our caller ~O(n^2) vs length of superChain. - // Fortunately superChains are short, but optimize this if it matters. - superChain.addAll(tryClass.superChain.map((t) => t.modelElement)); - for (final c in superChain) { - _getResultsForSuperChainElement(c); - if (results.isNotEmpty) break; - } - } - } - } - - /// Get any possible results for this class in the superChain. Returns - /// true if we found something. - void _getResultsForSuperChainElement(Class c) { - var membersToCheck = (c.allModelElementsByNamePart[codeRefChomped] ?? []) - .map(_convertConstructors); - for (var modelElement in membersToCheck) { - // [thing], a member of this class - _addCanonicalResult(modelElement); - } - if (codeRefChompedParts.length < 2 || - codeRefChompedParts[codeRefChompedParts.length - 2] == c.name) { - membersToCheck = - (c.allModelElementsByNamePart[codeRefChompedParts.last] ?? - []) - .map(_convertConstructors); - membersToCheck.forEach((m) => _addCanonicalResult(m)); - } - results.remove(null); - if (results.isNotEmpty) return; - if (c.fullyQualifiedNameWithoutLibrary == codeRefChomped) { - results.add(c); - } - } -} - -const _referenceLookupWarnings = { - PackageWarning.referenceLookupDiffersWithNew, - PackageWarning.referenceLookupFoundWithNew, - PackageWarning.referenceLookupMissingWithNew, -}; - md.Node _makeLinkNode(String codeRef, Warnable warnable) { var result = getMatchingLinkElement(warnable, codeRef); - var textContent = htmlEscape.convert(codeRef); + var textContent = _htmlEscape.convert(codeRef); var linkedElement = result.commentReferable; if (linkedElement != null) { if (linkedElement.href != null) { @@ -940,54 +259,8 @@ md.Node _makeLinkNode(String codeRef, Warnable warnable) { } @visibleForTesting -MatchingLinkResult getMatchingLinkElement(Warnable warnable, String codeRef, - {bool enhancedReferenceLookup}) { - enhancedReferenceLookup ??= warnable.config.enhancedReferenceLookup; - MatchingLinkResult result, resultOld, resultNew; - // Do a comparison between result types only if the warnings for them are - // enabled, because there's a significant performance penalty. - var doComparison = warnable.config.packageWarningOptions.warningModes.entries - .where((entry) => _referenceLookupWarnings.contains(entry.key)) - .any((entry) => entry.value != PackageWarningMode.ignore); - if (doComparison) { - resultNew = _getMatchingLinkElementCommentReferable(codeRef, warnable); - resultOld = _getMatchingLinkElementLegacy(codeRef, warnable); - if (resultNew.commentReferable != null) { - markdownStats.resolvedNewLookupReferences++; - } - result = enhancedReferenceLookup ? resultNew : resultOld; - if (resultOld.commentReferable != null) { - markdownStats.resolvedOldLookupReferences++; - } - } else { - if (enhancedReferenceLookup) { - result = _getMatchingLinkElementCommentReferable(codeRef, warnable); - } else { - result = _getMatchingLinkElementLegacy(codeRef, warnable); - } - } - if (doComparison) { - if (resultOld.isEquivalentTo(resultNew)) { - markdownStats.resolvedEquivalentlyReferences++; - } else { - if (resultNew.commentReferable == null && - resultOld.commentReferable != null) { - warnable.warn(PackageWarning.referenceLookupMissingWithNew, - message: '[$codeRef] => ' + resultOld.toString(), - referredFrom: warnable.documentationFrom); - } else if (resultNew.commentReferable != null && - resultOld.commentReferable == null) { - warnable.warn(PackageWarning.referenceLookupFoundWithNew, - message: '[$codeRef] => ' + resultNew.toString(), - referredFrom: warnable.documentationFrom); - } else { - warnable.warn(PackageWarning.referenceLookupDiffersWithNew, - message: - '[$codeRef] => new: ${resultNew.toString()} old: ${resultOld.toString()}', - referredFrom: warnable.documentationFrom); - } - } - } +MatchingLinkResult getMatchingLinkElement(Warnable warnable, String codeRef) { + var result = _getMatchingLinkElementCommentReferable(codeRef, warnable); markdownStats.totalReferences++; if (result.commentReferable != null) markdownStats.resolvedReferences++; return result; @@ -1031,7 +304,7 @@ Iterable findFreeHangingGenericsPositions(String string) sync* { while (true) { final nextOpenBracket = string.indexOf('[', currentPosition); final nextCloseBracket = string.indexOf(']', currentPosition); - final nextNonHTMLTag = string.indexOf(nonHTML, currentPosition); + final nextNonHTMLTag = string.indexOf(_nonHTML, currentPosition); final nextPositions = [nextOpenBracket, nextCloseBracket, nextNonHTMLTag] .where((p) => p != -1); @@ -1090,7 +363,7 @@ class MarkdownDocument extends md.Document { var lines = LineSplitter.split(text).toList(); md.Node firstNode; var nodes = []; - for (var node in IterableBlockParser(lines, this).parseLinesGenerator()) { + for (var node in _IterableBlockParser(lines, this).parseLinesGenerator()) { if (firstNode != null) { hasExtendedContent = true; if (!processFullText) break; @@ -1133,7 +406,7 @@ class _InlineCodeSyntax extends md.InlineSyntax { @override bool onMatch(md.InlineParser parser, Match match) { - var element = md.Element.text('code', htmlEscape.convert(match[1] /*!*/)); + var element = md.Element.text('code', _htmlEscape.convert(match[1] /*!*/)); parser.addNode(element); return true; } @@ -1143,7 +416,7 @@ class _AutolinkWithoutScheme extends md.AutolinkSyntax { @override bool onMatch(md.InlineParser parser, Match match) { var url = match[1] /*!*/; - var text = htmlEscape.convert(url).replaceFirst(_hideSchemes, ''); + var text = _htmlEscape.convert(url).replaceFirst(_hideSchemes, ''); var anchor = md.Element.text('a', text); anchor.attributes['href'] = url; parser.addNode(anchor); diff --git a/lib/src/matching_link_result.dart b/lib/src/matching_link_result.dart index 5cd1130916..a800f32260 100644 --- a/lib/src/matching_link_result.dart +++ b/lib/src/matching_link_result.dart @@ -2,7 +2,6 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -import 'package:dartdoc/src/element_type.dart'; import 'package:dartdoc/src/model/comment_referable.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:dartdoc/src/quiver.dart'; @@ -23,56 +22,6 @@ class MatchingLinkResult { @override int get hashCode => hash2(commentReferable, warn); - bool isEquivalentTo(MatchingLinkResult other) { - var compareThis = commentReferable; - var compareOther = other.commentReferable; - - if (compareThis is DefinedElementType) { - compareThis = (compareThis as DefinedElementType).modelElement; - } - - if (compareOther is DefinedElementType) { - compareOther = (compareOther as DefinedElementType).modelElement; - } - - if (compareThis is Accessor) { - compareThis = (compareThis as Accessor).enclosingCombo; - } - - if (compareOther is Accessor) { - compareOther = (compareOther as Accessor).enclosingCombo; - } - - if (compareThis is ModelElement && - compareThis.canonicalModelElement != null) { - compareThis = (compareThis as ModelElement).canonicalModelElement; - } - if (compareOther is ModelElement && - compareOther.canonicalModelElement != null) { - compareOther = (compareOther as ModelElement).canonicalModelElement; - } - if (compareThis == compareOther) return true; - // The old implementation just throws away Parameter matches to avoid - // problems with warning unnecessarily at higher levels of the code. - // I'd like to fix this at a different layer with the new lookup, so treat - // this as equivalent to a null type. - if (compareOther is Parameter && compareThis == null) { - return true; - } - if (compareThis is Parameter && compareOther == null) { - return true; - } - // Same with TypeParameter. - if (compareOther is TypeParameter && compareThis == null) { - return true; - } - if (compareThis is TypeParameter && compareOther == null) { - return true; - } - - return false; - } - @override String toString() { return 'element: [${commentReferable is Constructor ? 'new ' : ''}${commentReferable?.fullyQualifiedName}] warn: $warn'; @@ -82,9 +31,6 @@ class MatchingLinkResult { class _MarkdownStats { int totalReferences = 0; int resolvedReferences = 0; - int resolvedNewLookupReferences = 0; - int resolvedOldLookupReferences = 0; - int resolvedEquivalentlyReferences = 0; String _valueAndPercent(int references) { return '$references (${references.toDouble() / totalReferences.toDouble() * 100}%)'; @@ -96,18 +42,6 @@ class _MarkdownStats { report.writeln('total references: $totalReferences'); report.writeln( 'resolved references: ${_valueAndPercent(resolvedReferences)}'); - if (resolvedNewLookupReferences > 0) { - report.writeln( - 'resolved references with new lookup: $resolvedNewLookupReferences (${resolvedNewLookupReferences / totalReferences * 100}%)'); - } - if (resolvedOldLookupReferences > 0) { - report.writeln( - 'resolved references with old lookup: $resolvedOldLookupReferences (${resolvedOldLookupReferences / totalReferences * 100}%)'); - } - if (resolvedEquivalentlyReferences > 0) { - report.writeln( - 'resolved references with equivalent links: $resolvedEquivalentlyReferences (${resolvedEquivalentlyReferences / totalReferences * 100}%)'); - } return report.toString(); } } diff --git a/lib/src/model/container.dart b/lib/src/model/container.dart index 75becea76b..f75ffc8217 100644 --- a/lib/src/model/container.dart +++ b/lib/src/model/container.dart @@ -202,21 +202,6 @@ abstract class Container extends ModelElement with TypeParameters { return member; } - Map> _allModelElementsByNamePart; - - /// Helper for `_MarkdownCommentReference._getResultsForClass`. - Map> get allModelElementsByNamePart { - if (_allModelElementsByNamePart == null) { - _allModelElementsByNamePart = {}; - for (var me in allModelElements) { - _allModelElementsByNamePart.update( - me.namePart, (List v) => v..add(me), - ifAbsent: () => [me]); - } - } - return _allModelElementsByNamePart; - } - bool get hasPublicStaticFields => publicStaticFieldsSorted.isNotEmpty; Iterable get publicStaticFields => diff --git a/lib/src/model/library.dart b/lib/src/model/library.dart index b79109b623..0647cb66ea 100644 --- a/lib/src/model/library.dart +++ b/lib/src/model/library.dart @@ -579,26 +579,6 @@ class Library extends ModelElement with Categorization, TopLevelContainer { return name; } - /*late final*/ HashMap> _modelElementsNameMap; - - /// Map of [fullyQualifiedNameWithoutLibrary] to all matching [ModelElement]s - /// in this library. Used for legacy code reference lookups. - HashMap> get modelElementsNameMap { - if (_modelElementsNameMap == null) { - _modelElementsNameMap = HashMap>(); - allModelElements.forEach((ModelElement modelElement) { - // [definingLibrary] may be null if [element] has been imported or - // exported with a non-normalized URI, like "src//a.dart". - if (modelElement.definingLibrary == null) return; - _modelElementsNameMap - .putIfAbsent( - modelElement.fullyQualifiedNameWithoutLibrary, () => {}) - .add(modelElement); - }); - } - return _modelElementsNameMap; - } - /*late final*/ HashMap> _modelElementsMap; HashMap> get modelElementsMap { diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index 7ac8c07122..94f7c2d547 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -198,25 +198,6 @@ class PackageGraph with CommentReferable, Nameable { return _extensions; } - HashMap> _findRefElementCache; - HashMap> get findRefElementCache { - if (_findRefElementCache == null) { - assert(packageGraph.allLibrariesAdded); - _findRefElementCache = HashMap>(); - for (final modelElement - in utils.filterHasCanonical(packageGraph.allModelElements)) { - _findRefElementCache - .putIfAbsent( - modelElement.fullyQualifiedNameWithoutLibrary, () => {}) - .add(modelElement); - _findRefElementCache - .putIfAbsent(modelElement.fullyQualifiedName, () => {}) - .add(modelElement); - } - } - return _findRefElementCache; - } - // All library objects related to this package; a superset of _libraries. // Keyed by [LibraryElement.Source.fullName] to resolve multiple URIs // referring to the same location to the same [Library]. This isn't how @@ -464,20 +445,6 @@ class PackageGraph with CommentReferable, Nameable { case PackageWarning.missingCodeBlockLanguage: warningMessage = 'missing code block language: $message'; break; - case PackageWarning.referenceLookupFoundWithNew: - warningMessage = 'reference lookup found with new: $message'; - referredFromPrefix = 'from documentation for symbol'; - break; - case PackageWarning.referenceLookupMissingWithNew: - warningMessage = - 'reference lookup found only in old lookup code: $message'; - referredFromPrefix = 'from documentation for symbol'; - break; - case PackageWarning.referenceLookupDiffersWithNew: - warningMessage = - 'reference lookup resolution differs between lookup implementations: $message'; - referredFromPrefix = 'from documentation for symbol'; - break; } var messageParts = [warningMessage]; diff --git a/lib/src/warnings.dart b/lib/src/warnings.dart index 505b8d5bea..f0b7fc7424 100644 --- a/lib/src/warnings.dart +++ b/lib/src/warnings.dart @@ -268,21 +268,6 @@ const Map packageWarningDefinitions = 'specify ```dart or ~~~dart.' ], defaultWarningMode: PackageWarningMode.ignore), - PackageWarning.referenceLookupFoundWithNew: PackageWarningDefinition( - PackageWarning.referenceLookupFoundWithNew, - 'reference-lookup-found-with-new', - 'A code reference that was previously invalid is now found in the new lookup code.', - defaultWarningMode: PackageWarningMode.ignore), - PackageWarning.referenceLookupMissingWithNew: PackageWarningDefinition( - PackageWarning.referenceLookupMissingWithNew, - 'reference-lookup-missing-with-new', - 'A code reference found by the old lookup code is no longer found with the new lookup code.', - defaultWarningMode: PackageWarningMode.ignore), - PackageWarning.referenceLookupDiffersWithNew: PackageWarningDefinition( - PackageWarning.referenceLookupDiffersWithNew, - 'reference-lookup-not-found-with-new', - 'A code reference points to a different object in the new lookup code than the old.', - defaultWarningMode: PackageWarningMode.ignore), }; /// Something that package warnings can be called on. Optionally associated @@ -335,9 +320,6 @@ enum PackageWarning { missingConstantConstructor, missingExampleFile, missingCodeBlockLanguage, - referenceLookupFoundWithNew, - referenceLookupMissingWithNew, - referenceLookupDiffersWithNew, } /// Used to declare defaults for a particular package warning. diff --git a/test/end2end/model_special_cases_test.dart b/test/end2end/model_special_cases_test.dart index edabd0aaab..947d76043e 100644 --- a/test/end2end/model_special_cases_test.dart +++ b/test/end2end/model_special_cases_test.dart @@ -134,41 +134,41 @@ void main() { }); test('reference regression', () { - expect(newLookup(constructorTearoffs, 'A.A'), + expect(referenceLookup(constructorTearoffs, 'A.A'), equals(MatchingLinkResult(Anew))); - expect(newLookup(constructorTearoffs, 'new A()'), + expect(referenceLookup(constructorTearoffs, 'new A()'), equals(MatchingLinkResult(Anew))); - expect(newLookup(constructorTearoffs, 'A()'), + expect(referenceLookup(constructorTearoffs, 'A()'), equals(MatchingLinkResult(Anew))); - expect(newLookup(constructorTearoffs, 'B.B'), + expect(referenceLookup(constructorTearoffs, 'B.B'), equals(MatchingLinkResult(Bnew))); - expect(newLookup(constructorTearoffs, 'new B()'), + expect(referenceLookup(constructorTearoffs, 'new B()'), equals(MatchingLinkResult(Bnew))); - expect(newLookup(constructorTearoffs, 'B()'), + expect(referenceLookup(constructorTearoffs, 'B()'), equals(MatchingLinkResult(Bnew))); - expect(newLookup(constructorTearoffs, 'C.C'), + expect(referenceLookup(constructorTearoffs, 'C.C'), equals(MatchingLinkResult(Cnew))); - expect(newLookup(constructorTearoffs, 'new C()'), + expect(referenceLookup(constructorTearoffs, 'new C()'), equals(MatchingLinkResult(Cnew))); - expect(newLookup(constructorTearoffs, 'C()'), + expect(referenceLookup(constructorTearoffs, 'C()'), equals(MatchingLinkResult(Cnew))); - expect(newLookup(constructorTearoffs, 'D.D'), + expect(referenceLookup(constructorTearoffs, 'D.D'), equals(MatchingLinkResult(Dnew))); - expect(newLookup(constructorTearoffs, 'new D()'), + expect(referenceLookup(constructorTearoffs, 'new D()'), equals(MatchingLinkResult(Dnew))); - expect(newLookup(constructorTearoffs, 'D()'), + expect(referenceLookup(constructorTearoffs, 'D()'), equals(MatchingLinkResult(Dnew))); - expect(newLookup(constructorTearoffs, 'E.E'), + expect(referenceLookup(constructorTearoffs, 'E.E'), equals(MatchingLinkResult(Enew))); - expect(newLookup(constructorTearoffs, 'new E()'), + expect(referenceLookup(constructorTearoffs, 'new E()'), equals(MatchingLinkResult(Enew))); - expect(newLookup(constructorTearoffs, 'E()'), + expect(referenceLookup(constructorTearoffs, 'E()'), equals(MatchingLinkResult(Enew))); - expect(newLookup(constructorTearoffs, 'F.F'), + expect(referenceLookup(constructorTearoffs, 'F.F'), equals(MatchingLinkResult(Fnew))); - expect(newLookup(constructorTearoffs, 'new F()'), + expect(referenceLookup(constructorTearoffs, 'new F()'), equals(MatchingLinkResult(Fnew))); - expect(newLookup(constructorTearoffs, 'F()'), + expect(referenceLookup(constructorTearoffs, 'F()'), equals(MatchingLinkResult(Fnew))); }); }, skip: !_constructorTearoffsAllowed.allows(utils.platformVersion)); diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 9193be767a..bc153c1398 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -25,7 +25,7 @@ import 'package:dartdoc/src/warnings.dart'; import 'package:test/test.dart'; import '../src/utils.dart' - show bootBasicPackage, bothLookup, kTestPackagePublicLibraries, newLookup; + show bootBasicPackage, referenceLookup, kTestPackagePublicLibraries; final _testPackageGraphMemo = AsyncMemoizer(); Future get testPackageGraph async => @@ -1853,11 +1853,6 @@ void main() { expect(GenericMixin.characterLocation, isNotNull); }); - test('Verify mixin member is available in findRefElementCache', () { - expect(packageGraph.findRefElementCache['GenericMixin.mixinMember'], - isNotEmpty); - }); - test('Verify inheritance/mixin structure and type inference', () { expect( TypeInferenceMixedIn.mixedInTypes @@ -2254,22 +2249,22 @@ void main() { }); test('Verify basic ability to link anything', () { - expect(bothLookup(T0, 'C2'), equals(MatchingLinkResult(C2))); - expect(bothLookup(T2, 'C2'), equals(MatchingLinkResult(C2))); - expect(bothLookup(T5, 'C2'), equals(MatchingLinkResult(C2))); - expect(bothLookup(T8, 'C2'), equals(MatchingLinkResult(C2))); + expect(referenceLookup(T0, 'C2'), equals(MatchingLinkResult(C2))); + expect(referenceLookup(T2, 'C2'), equals(MatchingLinkResult(C2))); + expect(referenceLookup(T5, 'C2'), equals(MatchingLinkResult(C2))); + expect(referenceLookup(T8, 'C2'), equals(MatchingLinkResult(C2))); }); test('Verify ability to link to type parameters', () { var T2X = T2.typeParameters.firstWhere((t) => t.name == 'X'); - expect(bothLookup(T2, 'X'), equals(MatchingLinkResult(T2X))); + expect(referenceLookup(T2, 'X'), equals(MatchingLinkResult(T2X))); var T5X = T5.typeParameters.firstWhere((t) => t.name == 'X'); - expect(bothLookup(T5, 'X'), equals(MatchingLinkResult(T5X))); + expect(referenceLookup(T5, 'X'), equals(MatchingLinkResult(T5X))); }); test('Verify ability to link to parameters', () { var T5name = T5.parameters.firstWhere((t) => t.name == 'name'); - expect(bothLookup(T5, 'name'), equals(MatchingLinkResult(T5name))); + expect(referenceLookup(T5, 'name'), equals(MatchingLinkResult(T5name))); }); }); @@ -2326,49 +2321,44 @@ void main() { }); test('Grandparent override in container members', () { - // Original lookup fails unless the variable is reexported via other - // means, but to do that would not be testing the same case on both. - expect(newLookup(aField, 'aNotReexportedVariable'), + expect(referenceLookup(aField, 'aNotReexportedVariable'), equals(MatchingLinkResult(aNotReexportedVariable))); // Verify that documentationFrom cases work. Just having the doc // in the base class is enough to trigger [documentationFrom] and this // feature. - expect(bothLookup(anotherField, 'aNotReexportedVariable'), + expect(referenceLookup(anotherField, 'aNotReexportedVariable'), equals(MatchingLinkResult(aNotReexportedVariable))); }); // TODO(jcollins-g): dart-lang/dartdoc#2698 test('Linking for static/constructor inheritance across libraries', () { - expect(bothLookup(ExtendingAgain, 'aStaticField'), + expect(referenceLookup(ExtendingAgain, 'aStaticField'), equals(MatchingLinkResult(aStaticField))); - expect(bothLookup(ExtendingAgain, 'aStaticMethod'), + expect(referenceLookup(ExtendingAgain, 'aStaticMethod'), equals(MatchingLinkResult(aStaticMethod))); - expect(bothLookup(ExtendingAgain, 'aConstructor'), + expect(referenceLookup(ExtendingAgain, 'aConstructor'), equals(MatchingLinkResult(aConstructor))); }); test('Linking for inherited field from reexport context', () { - // While this can work in the old code at times, it is using the - // analyzer and the new test harness can't leverage the analyzer - // when testing the old lookup code. - expect(newLookup(aField, 'anotherNotReexportedVariable'), + expect(referenceLookup(aField, 'anotherNotReexportedVariable'), equals(MatchingLinkResult(anotherNotReexportedVariable))); }); // TODO(jcollins-g): dart-lang/dartdoc#2696 test('Allow non-explicit export namespace linking', () { expect( - bothLookup(BaseWithMembers, 'aSymbolOnlyAvailableInExportContext'), + referenceLookup( + BaseWithMembers, 'aSymbolOnlyAvailableInExportContext'), equals(MatchingLinkResult(aSymbolOnlyAvailableInExportContext))); }); test('Link to definingLibrary for class rather than its export context', () { - // This is an improvement over the original. - expect(newLookup(ExtendingAgain, 'someConflictingNameSymbol'), + expect(referenceLookup(ExtendingAgain, 'someConflictingNameSymbol'), equals(MatchingLinkResult(someConflictingNameSymbol))); - expect(newLookup(two_exports, 'someConflictingNameSymbol'), + expect(referenceLookup(two_exports, 'someConflictingNameSymbol'), equals(MatchingLinkResult(someConflictingNameSymbolTwoExports))); }); }); @@ -2433,56 +2423,54 @@ void main() { }); test('on extension targeting an unbound type', () { - expect(newLookup(UnboundTypeTargetExtension, 'doesNotCrash'), + expect(referenceLookup(UnboundTypeTargetExtension, 'doesNotCrash'), equals(MatchingLinkResult(doesNotCrash))); }); test('on inherited documentation', () { - expect(newLookup(aMethodExtended, 'ATypeParam'), + expect(referenceLookup(aMethodExtended, 'ATypeParam'), equals(MatchingLinkResult(ATypeParam))); - expect(newLookup(aMethodExtended, 'BTypeParam'), + expect(referenceLookup(aMethodExtended, 'BTypeParam'), equals(MatchingLinkResult(BTypeParam))); - expect(newLookup(aMethodExtended, 'CTypeParam'), + expect(referenceLookup(aMethodExtended, 'CTypeParam'), equals(MatchingLinkResult(CTypeParam))); // Disallowed, because Q does not exist where the docs originated from. // The old code forgave this most of the time. - expect(newLookup(aMethodExtended, 'QTypeParam'), + expect(referenceLookup(aMethodExtended, 'QTypeParam'), equals(MatchingLinkResult(null))); // We get an inverse situation on the extendedQ class. - expect(newLookup(aMethodExtendedQ, 'ATypeParam'), + expect(referenceLookup(aMethodExtendedQ, 'ATypeParam'), equals(MatchingLinkResult(null))); - expect(newLookup(aMethodExtendedQ, 'BTypeParam'), + expect(referenceLookup(aMethodExtendedQ, 'BTypeParam'), equals(MatchingLinkResult(null))); - expect(newLookup(aMethodExtendedQ, 'CTypeParam'), + expect(referenceLookup(aMethodExtendedQ, 'CTypeParam'), equals(MatchingLinkResult(null))); - expect(newLookup(aMethodExtendedQ, 'QTypeParam'), + expect(referenceLookup(aMethodExtendedQ, 'QTypeParam'), equals(MatchingLinkResult(QTypeParam))); }); test('on classes', () { - expect(bothLookup(TypeParameterThings, 'ATypeParam'), + expect(referenceLookup(TypeParameterThings, 'ATypeParam'), equals(MatchingLinkResult(ATypeParam))); - expect(bothLookup(TypeParameterThings, 'BTypeParam'), + expect(referenceLookup(TypeParameterThings, 'BTypeParam'), equals(MatchingLinkResult(BTypeParam))); - expect(bothLookup(aName, 'ATypeParam'), + expect(referenceLookup(aName, 'ATypeParam'), equals(MatchingLinkResult(ATypeParam))); - expect(bothLookup(aThing, 'BTypeParam'), + expect(referenceLookup(aThing, 'BTypeParam'), equals(MatchingLinkResult(BTypeParam))); - expect(bothLookup(aMethod, 'CTypeParam'), + expect(referenceLookup(aMethod, 'CTypeParam'), equals(MatchingLinkResult(CTypeParam))); - expect(bothLookup(aParam, 'ATypeParam'), + expect(referenceLookup(aParam, 'ATypeParam'), equals(MatchingLinkResult(ATypeParam))); - // Original didn't handle this case properly and popped out to higher - // levels. - expect(newLookup(anotherParam, 'CTypeParam'), + expect(referenceLookup(anotherParam, 'CTypeParam'), equals(MatchingLinkResult(CTypeParam))); }); test('on top level methods', () { - expect(bothLookup(aTopLevelTypeParameterFunction, 'DTypeParam'), + expect(referenceLookup(aTopLevelTypeParameterFunction, 'DTypeParam'), equals(MatchingLinkResult(DTypeParam))); - expect(bothLookup(typedParam, 'DTypeParam'), + expect(referenceLookup(typedParam, 'DTypeParam'), equals(MatchingLinkResult(DTypeParam))); }); }); @@ -2661,32 +2649,36 @@ void main() { .parameters .firstWhere((p) => p.name == 'fParamC'); - expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamA'), + expect( + referenceLookup(aSetterWithFunctionParameter, 'fParam.fParamA'), equals(MatchingLinkResult(fParamA))); - expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamB'), + expect( + referenceLookup(aSetterWithFunctionParameter, 'fParam.fParamB'), equals(MatchingLinkResult(fParamB))); - expect(bothLookup(aSetterWithFunctionParameter, 'fParam.fParamC'), + expect( + referenceLookup(aSetterWithFunctionParameter, 'fParam.fParamC'), equals(MatchingLinkResult(fParamC))); }); test('in class scope overridden by fields', () { - expect(bothLookup(FactoryConstructorThings, 'aName'), + expect(referenceLookup(FactoryConstructorThings, 'aName'), equals(MatchingLinkResult(aNameField))); var anotherNameField = FactoryConstructorThings.allFields .firstWhere((f) => f.name == 'anotherName'); - expect(bothLookup(FactoryConstructorThings, 'anotherName'), + expect(referenceLookup(FactoryConstructorThings, 'anotherName'), equals(MatchingLinkResult(anotherNameField))); - expect(bothLookup(FactoryConstructorThings, 'yetAnotherName'), + expect(referenceLookup(FactoryConstructorThings, 'yetAnotherName'), equals(MatchingLinkResult(yetAnotherNameField))); - expect(bothLookup(FactoryConstructorThings, 'initViaFieldFormal'), + expect( + referenceLookup(FactoryConstructorThings, 'initViaFieldFormal'), equals(MatchingLinkResult(initViaFieldFormal))); - expect(bothLookup(FactoryConstructorThings, 'redHerring'), + expect(referenceLookup(FactoryConstructorThings, 'redHerring'), equals(MatchingLinkResult(redHerring))); }); test('in class scope overridden by constructors when specified', () { expect( - bothLookup(FactoryConstructorThings, + referenceLookup(FactoryConstructorThings, 'new FactoryConstructorThings.anotherName'), equals(MatchingLinkResult(anotherName))); }); @@ -2695,75 +2687,75 @@ void main() { 'in default constructor scope referring to a field formal parameter', () { expect( - newLookup(factoryConstructorThingsDefault, 'initViaFieldFormal'), + referenceLookup( + factoryConstructorThingsDefault, 'initViaFieldFormal'), equals(MatchingLinkResult(initViaFieldFormal))); }); test('in factory constructor scope referring to parameters', () { - expect(newLookup(anotherName, 'aName'), + expect(referenceLookup(anotherName, 'aName'), equals(MatchingLinkResult(aName))); - expect(bothLookup(anotherName, 'anotherName'), + expect(referenceLookup(anotherName, 'anotherName'), equals(MatchingLinkResult(anotherNameParameter))); - expect(bothLookup(anotherName, 'anotherDifferentName'), + expect(referenceLookup(anotherName, 'anotherDifferentName'), equals(MatchingLinkResult(anotherDifferentName))); - expect(bothLookup(anotherName, 'differentName'), + expect(referenceLookup(anotherName, 'differentName'), equals(MatchingLinkResult(differentName))); - expect(bothLookup(anotherName, 'redHerring'), + expect(referenceLookup(anotherName, 'redHerring'), equals(MatchingLinkResult(redHerring))); }); test('in factory constructor scope referring to constructors', () { // A bare constructor reference is OK because there is no conflict. - expect(bothLookup(anotherName, 'anotherConstructor'), + expect(referenceLookup(anotherName, 'anotherConstructor'), equals(MatchingLinkResult(anotherConstructor))); // A conflicting constructor has to be explicit. expect( - bothLookup( + referenceLookup( anotherName, 'new FactoryConstructorThings.anotherName'), equals(MatchingLinkResult(anotherName))); }); test('in method scope referring to parameters and variables', () { - expect(bothLookup(aMethod, 'yetAnotherName'), + expect(referenceLookup(aMethod, 'yetAnotherName'), equals(MatchingLinkResult(yetAnotherName))); - expect(bothLookup(aMethod, 'FactoryConstructorThings.yetAnotherName'), + expect( + referenceLookup( + aMethod, 'FactoryConstructorThings.yetAnotherName'), equals(MatchingLinkResult(yetAnotherNameField))); expect( - bothLookup( + referenceLookup( aMethod, 'FactoryConstructorThings.anotherName.anotherName'), equals(MatchingLinkResult(anotherNameParameter))); - expect(bothLookup(aMethod, 'aName'), + expect(referenceLookup(aMethod, 'aName'), equals(MatchingLinkResult(aNameField))); }); }); test('Referring to a renamed library directly works', () { - // The new code forwards from the prefix, so doesn't quite work the - // same as the old for an equality comparison via [MatchingLinkResult]. expect( - (bothLookup(aFunctionUsingRenamedLib, 'renamedLib').commentReferable - as ModelElement) + (referenceLookup(aFunctionUsingRenamedLib, 'renamedLib') + .commentReferable as ModelElement) .canonicalModelElement, equals(mylibpub)); }); test('Referring to libraries and packages with the same name is fine', () { - expect(bothLookup(Apple, 'Dart'), equals(MatchingLinkResult(Dart))); - // New feature: allow disambiguation if you really want to specify a package. - expect(newLookup(Apple, 'package:Dart'), + expect( + referenceLookup(Apple, 'Dart'), equals(MatchingLinkResult(Dart))); + expect(referenceLookup(Apple, 'package:Dart'), equals(MatchingLinkResult(DartPackage))); }); test('Verify basic linking inside a constructor', () { - // Field formal parameters worked sometimes by accident in the old code, - // but should work reliably now. - expect(newLookup(aNonDefaultConstructor, 'initializeMe'), + expect(referenceLookup(aNonDefaultConstructor, 'initializeMe'), equals(MatchingLinkResult(initializeMe))); - expect(newLookup(aNonDefaultConstructor, 'aNonDefaultConstructor'), + expect( + referenceLookup(aNonDefaultConstructor, 'aNonDefaultConstructor'), equals(MatchingLinkResult(aNonDefaultConstructor))); expect( - bothLookup(aNonDefaultConstructor, + referenceLookup(aNonDefaultConstructor, 'BaseForDocComments.aNonDefaultConstructor'), equals(MatchingLinkResult(aNonDefaultConstructor))); }); @@ -2771,130 +2763,126 @@ void main() { test( 'Verify that constructors do not override member fields unless explicitly specified', () { - expect(bothLookup(baseForDocComments, 'aConstructorShadowed'), + expect(referenceLookup(baseForDocComments, 'aConstructorShadowed'), equals(MatchingLinkResult(aConstructorShadowedField))); expect( - bothLookup( + referenceLookup( baseForDocComments, 'BaseForDocComments.aConstructorShadowed'), equals(MatchingLinkResult(aConstructorShadowedField))); expect( - bothLookup(baseForDocComments, + referenceLookup(baseForDocComments, 'new BaseForDocComments.aConstructorShadowed'), equals(MatchingLinkResult(aConstructorShadowed))); }); test('Deprecated lookup styles still function', () { - // dart-lang/dartdoc#2683 - // We can't check the old code this way as this is one of the few - // cases in the old code where it relies on the analyzer's resolution of - // the doc comment -- the test harness can't do that for the old code. - expect(newLookup(baseForDocComments, 'aPrefix.UseResult'), + expect(referenceLookup(baseForDocComments, 'aPrefix.UseResult'), equals(MatchingLinkResult(metaUseResult))); }); test('Verify basic linking inside class', () { expect( - bothLookup( + referenceLookup( baseForDocComments, 'BaseForDocComments.BaseForDocComments'), equals(MatchingLinkResult(defaultConstructor))); // We don't want the parameter on the default constructor, here. - expect(bothLookup(fakeLibrary, 'BaseForDocComments.somethingShadowy'), + expect( + referenceLookup(fakeLibrary, 'BaseForDocComments.somethingShadowy'), equals(MatchingLinkResult(somethingShadowy))); - expect(bothLookup(baseForDocComments, 'somethingShadowy'), + expect(referenceLookup(baseForDocComments, 'somethingShadowy'), equals(MatchingLinkResult(somethingShadowy))); // Allow specific reference if necessary expect( - newLookup(baseForDocComments, + referenceLookup(baseForDocComments, 'BaseForDocComments.BaseForDocComments.somethingShadowy'), equals(MatchingLinkResult(somethingShadowyParameter))); - expect(bothLookup(doAwesomeStuff, 'aNonDefaultConstructor'), + expect(referenceLookup(doAwesomeStuff, 'aNonDefaultConstructor'), equals(MatchingLinkResult(aNonDefaultConstructor))); expect( - bothLookup( + referenceLookup( doAwesomeStuff, 'BaseForDocComments.aNonDefaultConstructor'), equals(MatchingLinkResult(aNonDefaultConstructor))); - expect(bothLookup(doAwesomeStuff, 'this'), + expect(referenceLookup(doAwesomeStuff, 'this'), equals(MatchingLinkResult(baseForDocComments))); - expect(bothLookup(doAwesomeStuff, 'value'), + expect(referenceLookup(doAwesomeStuff, 'value'), equals(MatchingLinkResult(doAwesomeStuffParam))); // Parent class of [doAwesomeStuff]. - expect(bothLookup(doAwesomeStuff, 'BaseForDocComments'), + expect(referenceLookup(doAwesomeStuff, 'BaseForDocComments'), equals(MatchingLinkResult(baseForDocComments))); // Top level constants in the same library as [doAwesomeStuff]. - expect(bothLookup(doAwesomeStuff, 'NAME_WITH_TWO_UNDERSCORES'), + expect(referenceLookup(doAwesomeStuff, 'NAME_WITH_TWO_UNDERSCORES'), equals(MatchingLinkResult(nameWithTwoUnderscores))); - expect(bothLookup(doAwesomeStuff, 'NAME_SINGLEUNDERSCORE'), + expect(referenceLookup(doAwesomeStuff, 'NAME_SINGLEUNDERSCORE'), equals(MatchingLinkResult(nameWithSingleUnderscore))); // Top level class from [dart:core]. - expect(bothLookup(doAwesomeStuff, 'String'), + expect(referenceLookup(doAwesomeStuff, 'String'), equals(MatchingLinkResult(string))); // Another method in the same class. - expect(bothLookup(doAwesomeStuff, 'anotherMethod'), + expect(referenceLookup(doAwesomeStuff, 'anotherMethod'), equals(MatchingLinkResult(anotherMethod))); // A top level function in this library. - expect(bothLookup(doAwesomeStuff, 'topLevelFunction'), + expect(referenceLookup(doAwesomeStuff, 'topLevelFunction'), equals(MatchingLinkResult(topLevelFunction))); // A top level function in another library imported into this library. - expect(bothLookup(doAwesomeStuff, 'function1'), + expect(referenceLookup(doAwesomeStuff, 'function1'), equals(MatchingLinkResult(function1))); // A class in another library imported into this library. - expect(bothLookup(doAwesomeStuff, 'Apple'), + expect(referenceLookup(doAwesomeStuff, 'Apple'), equals(MatchingLinkResult(Apple))); // A top level constant in this library sharing the same name as a name in another library. - expect(bothLookup(doAwesomeStuff, 'incorrectDocReference'), + expect(referenceLookup(doAwesomeStuff, 'incorrectDocReference'), equals(MatchingLinkResult(incorrectDocReference))); // A top level constant in another library. - expect(bothLookup(doAwesomeStuff, 'incorrectDocReferenceFromEx'), + expect(referenceLookup(doAwesomeStuff, 'incorrectDocReferenceFromEx'), equals(MatchingLinkResult(incorrectDocReferenceFromEx))); // A prefixed constant in another library. - expect(bothLookup(doAwesomeStuff, 'css.theOnlyThingInTheLibrary'), + expect(referenceLookup(doAwesomeStuff, 'css.theOnlyThingInTheLibrary'), equals(MatchingLinkResult(theOnlyThingInTheLibrary))); // A name that exists in this package but is not imported. - expect(bothLookup(doAwesomeStuff, 'doesStuff'), + expect(referenceLookup(doAwesomeStuff, 'doesStuff'), equals(MatchingLinkResult(doesStuff))); // A name of a class from an import of a library that exported that name. - expect(bothLookup(doAwesomeStuff, 'BaseClass'), + expect(referenceLookup(doAwesomeStuff, 'BaseClass'), equals(MatchingLinkResult(BaseClass))); // A bracket operator within this class. - expect(bothLookup(doAwesomeStuff, 'operator []'), + expect(referenceLookup(doAwesomeStuff, 'operator []'), equals(MatchingLinkResult(bracketOperator))); // A bracket operator in another class. - // Doesn't work in older lookup code. - expect(newLookup(doAwesomeStuff, 'SpecialList.operator []'), + expect(referenceLookup(doAwesomeStuff, 'SpecialList.operator []'), equals(MatchingLinkResult(bracketOperatorOtherClass))); // Reference containing a type parameter. - expect(bothLookup(doAwesomeStuff, 'ExtraSpecialList'), + expect(referenceLookup(doAwesomeStuff, 'ExtraSpecialList'), equals(MatchingLinkResult(ExtraSpecialList))); // Reference to an inherited member. expect( - bothLookup( + referenceLookup( doAwesomeStuff, 'ClassWithUnusualProperties.forInheriting'), equals(MatchingLinkResult(forInheriting))); // Reference to an inherited member in another library via class name. - expect(bothLookup(doAwesomeStuff, 'ExtendedBaseReexported.action'), + expect(referenceLookup(doAwesomeStuff, 'ExtendedBaseReexported.action'), equals(MatchingLinkResult(action))); }); diff --git a/test/src/utils.dart b/test/src/utils.dart index 5737a8ec89..0e41a0aa98 100644 --- a/test/src/utils.dart +++ b/test/src/utils.dart @@ -19,7 +19,6 @@ import 'package:dartdoc/src/package_config_provider.dart'; import 'package:dartdoc/src/package_meta.dart'; import 'package:dartdoc/src/warnings.dart'; import 'package:pub_semver/pub_semver.dart'; -import 'package:test/expect.dart'; /// The number of public libraries in testing/test_package, minus 2 for /// the excluded libraries listed in the initializers for _testPackageGraphMemo @@ -212,16 +211,5 @@ MatchingLinkResult definingLinkResult(MatchingLinkResult originalResult) { return originalResult; } -MatchingLinkResult originalLookup(Warnable element, String codeRef) => - definingLinkResult(getMatchingLinkElement(element, codeRef, - enhancedReferenceLookup: false)); -MatchingLinkResult newLookup(Warnable element, String codeRef) => - definingLinkResult(getMatchingLinkElement(element, codeRef, - enhancedReferenceLookup: true)); - -MatchingLinkResult bothLookup(Warnable element, String codeRef) { - var originalLookupResult = originalLookup(element, codeRef); - var newLookupResult = newLookup(element, codeRef); - expect(newLookupResult.isEquivalentTo(originalLookupResult), isTrue); - return newLookupResult; -} +MatchingLinkResult referenceLookup(Warnable element, String codeRef) => + definingLinkResult(getMatchingLinkElement(element, codeRef));