diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index a53d16c2f3..13891e7fa5 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -19,7 +19,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest] - sdk: [dev] + sdk: [dev, beta] job: [nnbd, flutter, sdk-analyzer, packages, sdk-docs] include: - os: macos-latest @@ -32,9 +32,9 @@ jobs: # Do not try to run flutter against the "stable" sdk, # it is unlikely to work and produces uninteresting # results. - - sdk: stable + - sdk: beta job: flutter - - sdk: stable + - sdk: beta job: sdk-analyzer steps: diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart index c4a8649a33..fc45d2a56c 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -264,7 +264,7 @@ class Dartdoc { return dartdocResults; } finally { // Clear out any cached tool snapshots and temporary directories. - SnapshotCache.instance?.dispose(); + SnapshotCache.instanceFor(config.resourceProvider).dispose(); // ignore: unawaited_futures ToolTempFileTracker.instance?.dispose(); } diff --git a/lib/src/comment_references/model_comment_reference.dart b/lib/src/comment_references/model_comment_reference.dart index 7416994650..dce0a95741 100644 --- a/lib/src/comment_references/model_comment_reference.dart +++ b/lib/src/comment_references/model_comment_reference.dart @@ -11,12 +11,10 @@ import 'package:analyzer/file_system/file_system.dart'; import 'package:dartdoc/src/comment_references/parser.dart'; abstract class ModelCommentReference { - /// Does the structure of the reference itself imply a possible default + /// Does the structure of the reference itself imply a possible unnamed /// constructor? - // TODO(jcollins-g): rewrite/discard this once default constructor tear-off - // design process is complete. - bool get allowDefaultConstructor; - bool get allowDefaultConstructorParameter; + bool get allowUnnamedConstructor; + bool get allowUnnamedConstructorParameter; String get codeRef; bool get hasConstructorHint; bool get hasCallableHint; @@ -37,42 +35,45 @@ abstract class ModelCommentReference { /// information needed for Dartdoc. Drops link to the [CommentReference] /// and [ResourceProvider] after construction. class _ModelCommentReferenceImpl implements ModelCommentReference { - bool _allowDefaultConstructor; + bool _allowUnnamedConstructor; void _initAllowCache() { var referencePieces = parsed.whereType().toList(); - _allowDefaultConstructor = false; - _allowDefaultConstructorParameter = false; + _allowUnnamedConstructor = false; + _allowUnnamedConstructorParameter = false; if (referencePieces.length >= 2) { IdentifierNode nodeLast; for (var f in referencePieces) { if (f.text == nodeLast?.text) { if (identical(referencePieces.last, f)) { - _allowDefaultConstructor = true; + _allowUnnamedConstructor = true; } else { - _allowDefaultConstructorParameter = true; + _allowUnnamedConstructorParameter = true; } } nodeLast = f; } + if (referencePieces.last.text == 'new') { + _allowUnnamedConstructor = true; + } } } @override - bool get allowDefaultConstructor { - if (_allowDefaultConstructor == null) { + bool get allowUnnamedConstructor { + if (_allowUnnamedConstructor == null) { _initAllowCache(); } - return _allowDefaultConstructor; + return _allowUnnamedConstructor; } - bool _allowDefaultConstructorParameter; + bool _allowUnnamedConstructorParameter; @override - bool get allowDefaultConstructorParameter { - if (_allowDefaultConstructorParameter == null) { + bool get allowUnnamedConstructorParameter { + if (_allowUnnamedConstructorParameter == null) { _initAllowCache(); } - return _allowDefaultConstructorParameter; + return _allowUnnamedConstructorParameter; } @override diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart index fe05e6f79c..5d4f402057 100644 --- a/lib/src/dartdoc_options.dart +++ b/lib/src/dartdoc_options.dart @@ -1240,9 +1240,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); @@ -1377,17 +1374,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) { var envFlutterRoot = Platform.environment['FLUTTER_ROOT']; diff --git a/lib/src/element_type.dart b/lib/src/element_type.dart index 95f0429db2..ea82b74286 100644 --- a/lib/src/element_type.dart +++ b/lib/src/element_type.dart @@ -33,7 +33,7 @@ abstract class ElementType extends Privacy with CommentReferable, Nameable { f.element.kind == ElementKind.DYNAMIC || f.element.kind == ElementKind.NEVER) { if (f is FunctionType) { - if (f.aliasElement != null) { + if (f.alias?.element != null) { return AliasedFunctionTypeElementType( f, library, packageGraph, returnedFrom); } @@ -46,14 +46,14 @@ abstract class ElementType extends Privacy with CommentReferable, Nameable { // In that case it is an actual type alias of some kind (generic // or otherwise. Here however aliasElement signals that this is a // type referring to an alias. - if (f is! TypeAliasElement && f.aliasElement != null) { + if (f is! TypeAliasElement && f.alias?.element != null) { return AliasedElementType( f, library, packageGraph, element, returnedFrom); } assert(f is ParameterizedType || f is TypeParameterType); // TODO(jcollins-g): strip out all the cruft that's accumulated // here for non-generic type aliases. - var isGenericTypeAlias = f.aliasElement != null && f is! InterfaceType; + var isGenericTypeAlias = f.alias?.element != null && f is! InterfaceType; if (f is FunctionType) { assert(f is ParameterizedType); // This is an indication we have an extremely out of date analyzer.... @@ -190,8 +190,8 @@ class AliasedFunctionTypeElementType extends FunctionTypeElementType AliasedFunctionTypeElementType(FunctionType f, Library library, PackageGraph packageGraph, ElementType returnedFrom) : super(f, library, packageGraph, returnedFrom) { - assert(type.aliasElement != null); - assert(type.aliasArguments != null); + assert(type.alias?.element != null); + assert(type.alias?.typeArguments != null); } @override @@ -222,18 +222,18 @@ class ParameterizedElementType extends DefinedElementType with Rendered { /// A [ElementType] whose underlying type was referrred to by a type alias. mixin Aliased implements ElementType { @override - String get name => type.aliasElement.name; + String get name => type.alias.element.name; @override bool get isTypedef => true; ModelElement _aliasElement; ModelElement get aliasElement => _aliasElement ??= - ModelElement.fromElement(type.aliasElement, packageGraph); + ModelElement.fromElement(type.alias.element, packageGraph); Iterable _aliasArguments; Iterable get aliasArguments => - _aliasArguments ??= type.aliasArguments + _aliasArguments ??= type.alias.typeArguments .map((f) => ElementType.from(f, library, packageGraph)) .toList(growable: false); } @@ -242,7 +242,7 @@ class AliasedElementType extends ParameterizedElementType with Aliased { AliasedElementType(ParameterizedType type, Library library, PackageGraph packageGraph, ModelElement element, ElementType returnedFrom) : super(type, library, packageGraph, element, returnedFrom) { - assert(type.aliasElement != null); + assert(type.alias?.element != null); } @override @@ -431,7 +431,7 @@ class CallableElementType extends DefinedElementType with Rendered, Callable { Iterable _typeArguments; @override Iterable get typeArguments => - _typeArguments ??= (type.aliasArguments ?? []) + _typeArguments ??= (type.alias?.typeArguments ?? []) .map((f) => ElementType.from(f, library, packageGraph)) .toList(growable: false); } diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart index 578eaae7fc..888cf064fe 100644 --- a/lib/src/generator/templates.runtime_renderers.dart +++ b/lib/src/generator/templates.runtime_renderers.dart @@ -3358,20 +3358,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, @@ -7461,19 +7447,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: @@ -14804,6 +14777,7 @@ const _invisibleGetters = { 'DartType': { 'hashCode', 'runtimeType', + 'alias', 'aliasArguments', 'aliasElement', 'displayName', @@ -14843,7 +14817,6 @@ const _invisibleGetters = { 'dropTextFrom', 'examplePathPrefix', 'excludePackages', - 'enhancedReferenceLookup', 'flutterRoot', 'hideSdkText', 'include', @@ -15113,6 +15086,7 @@ const _invisibleGetters = { 'LibraryElement': { 'hashCode', 'runtimeType', + 'accessibleExtensions', 'definingCompilationUnit', 'entryPoint', 'exportedLibraries', @@ -15240,7 +15214,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 08a02797c9..819c20d5aa 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -10,9 +10,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'; @@ -20,7 +18,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', @@ -121,41 +119,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(), @@ -175,8 +142,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* { @@ -192,25 +159,12 @@ 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], +/// Return false if the passed [referable] is an unnamed [Constructor], /// or if it is shadowing another type of element, or is a parameter of /// one of the above. -bool _rejectDefaultAndShadowingConstructors(CommentReferable referable) { +bool _rejectUnnamedAndShadowingConstructors(CommentReferable referable) { if (referable is Constructor) { - if (referable.name == referable.enclosingElement.name) { - return false; - } + if (referable.isUnnamedConstructor) return false; if (referable.enclosingElement .referenceChildren[referable.name.split('.').last] is! Constructor) { return false; @@ -237,12 +191,12 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable( bool Function(CommentReferable) filter; bool Function(CommentReferable) allowTree; - // Constructor references are pretty ambiguous by nature since they are + // Constructor references are pretty ambiguous by nature since they can be // declared with the same name as the class they are constructing, and even // if they don't use field-formal parameters, sometimes have parameters // named the same as members. // Maybe clean this up with inspiration from constructor tear-off syntax? - if (commentReference.allowDefaultConstructor) { + if (commentReference.allowUnnamedConstructor) { // Neither reject, nor require, a default constructor in the event // the comment reference structure implies one. (We can not require it // in case a library name is the same as a member class name and the class @@ -252,18 +206,18 @@ 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. filter = _requireCallable; } else { - // Without hints, reject default constructors and their parameters to force + // Without hints, reject unnamed constructors and their parameters to force // resolution to the class. - filter = _rejectDefaultAndShadowingConstructors; + filter = _rejectUnnamedAndShadowingConstructors; - if (!commentReference.allowDefaultConstructorParameter) { - allowTree = _rejectDefaultAndShadowingConstructors; + if (!commentReference.allowUnnamedConstructorParameter) { + allowTree = _rejectUnnamedAndShadowingConstructors; } } @@ -274,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) { @@ -942,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; @@ -1033,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); @@ -1092,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; @@ -1135,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; } @@ -1145,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 f19d9b10f8..ad18a8f2d6 100644 --- a/lib/src/matching_link_result.dart +++ b/lib/src/matching_link_result.dart @@ -4,7 +4,6 @@ // @dart=2.9 -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'; @@ -25,56 +24,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'; @@ -84,9 +33,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}%)'; @@ -98,18 +44,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/class.dart b/lib/src/model/class.dart index 63e4c9c278..5524d8ee94 100644 --- a/lib/src/model/class.dart +++ b/lib/src/model/class.dart @@ -72,10 +72,15 @@ class Class extends Container return _unnamedConstructor; } - @Deprecated( - 'Renamed to `unnamedConstructor`; this getter with the old name will be ' - 'removed as early as Dartdoc 1.0.0') - Constructor get defaultConstructor => unnamedConstructor; + Constructor _defaultConstructor; + + /// With constructor tearoffs, this is no longer equivalent to the unnamed + /// constructor and assumptions based on that are incorrect. + Constructor get defaultConstructor { + _defaultConstructor ??= unnamedConstructor ?? + constructors.firstWhere((c) => c.isDefaultConstructor); + return _defaultConstructor; + } @override Iterable get instanceMethods => @@ -615,8 +620,11 @@ class Class extends Container for (var constructor in source) { yield MapEntry(constructor.referenceName, constructor); yield MapEntry( - '${constructor.enclosingElement.name}.${constructor.referenceName}', + '${constructor.enclosingElement.referenceName}.${constructor.referenceName}', constructor); + if (constructor.isDefaultConstructor) { + yield MapEntry('new', constructor); + } } } diff --git a/lib/src/model/constructor.dart b/lib/src/model/constructor.dart index 66b22317bf..8f86a158fb 100644 --- a/lib/src/model/constructor.dart +++ b/lib/src/model/constructor.dart @@ -71,10 +71,8 @@ class Constructor extends ModelElement bool get isUnnamedConstructor => name == enclosingElement.name; - @Deprecated( - 'Renamed to `isUnnamedConstructor`; this getter with the old name will ' - 'be removed as early as Dartdoc 1.0.0') - bool get isDefaultConstructor => isUnnamedConstructor; + bool get isDefaultConstructor => + name == '${enclosingElement.name}.new' || isUnnamedConstructor; bool get isFactory => element.isFactory; @@ -146,5 +144,5 @@ class Constructor extends ModelElement @override String get referenceName => - element.name == '' ? enclosingElement.name : element.name; + isUnnamedConstructor ? enclosingElement.name : element.name; } diff --git a/lib/src/model/container.dart b/lib/src/model/container.dart index 13adf614a5..e5b4202f2f 100644 --- a/lib/src/model/container.dart +++ b/lib/src/model/container.dart @@ -204,21 +204,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 d6e61c0b82..120d447403 100644 --- a/lib/src/model/library.dart +++ b/lib/src/model/library.dart @@ -581,26 +581,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_builder.dart b/lib/src/model/package_builder.dart index ffeb4dfdf1..623c470daf 100644 --- a/lib/src/model/package_builder.dart +++ b/lib/src/model/package_builder.dart @@ -157,7 +157,7 @@ class PubPackageBuilder implements PackageBuilder { var analysisContext = contextCollection.contextFor(config.inputDir); var session = analysisContext.currentSession; // Allow dart source files with inappropriate suffixes (#1897). - final library = await session.getResolvedLibrary2(filePath); + final library = await session.getResolvedLibrary(filePath); if (library is ResolvedLibraryResult) { final libraryElement = library.element; var restoredUri = libraryElement.source.uri.toString(); diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index d0ea3e7b29..c57102afd1 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -200,25 +200,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 @@ -466,20 +447,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/package_meta.dart b/lib/src/package_meta.dart index 1dfc79105e..cb9f419a77 100644 --- a/lib/src/package_meta.dart +++ b/lib/src/package_meta.dart @@ -27,10 +27,11 @@ class PackageMetaFailure extends DartdocFailure { } /// For each list in this list, at least one of the given paths must exist -/// for this to be detected as an SDK. -final List> _sdkDirFilePathsPosix = [ +/// for this to be detected as an SDK. Update [_writeMockSdkBinFiles] in +/// `test/src/utils.dart` if removing any entries here. +const List> _sdkDirFilePathsPosix = [ ['bin/dart.bat', 'bin/dart.exe', 'bin/dart'], - ['bin/pub.bat', 'bin/pub'], + ['include/dart_version.h'], ['lib/core/core.dart'], ]; @@ -330,12 +331,13 @@ class _FilePackageMeta extends PubPackageMeta { List parameters; if (requiresFlutter) { binPath = p.join(flutterRoot, 'bin', 'flutter'); + if (Platform.isWindows) binPath += '.bat'; parameters = ['pub', 'get']; } else { - binPath = p.join(p.dirname(Platform.resolvedExecutable), 'pub'); - parameters = ['get']; + binPath = p.join(p.dirname(Platform.resolvedExecutable), 'dart'); + if (Platform.isWindows) binPath += '.exe'; + parameters = ['pub', 'get']; } - if (Platform.isWindows) binPath += '.bat'; var result = Process.runSync(binPath, parameters, workingDirectory: dir.path); diff --git a/lib/src/tool_definition.dart b/lib/src/tool_definition.dart index 95046d815e..ba39692c45 100644 --- a/lib/src/tool_definition.dart +++ b/lib/src/tool_definition.dart @@ -111,8 +111,8 @@ class DartToolDefinition extends ToolDefinition { assert(args[0] == command.first); // Set up flags to create a new snapshot, if needed, and use the first run // as the training run. - SnapshotCache.createInstance(_resourceProvider); - var snapshot = SnapshotCache.instance.getSnapshot(command.first); + var snapshotCache = SnapshotCache.instanceFor(_resourceProvider); + var snapshot = snapshotCache.getSnapshot(command.first); var snapshotFile = snapshot._snapshotFile; var snapshotPath = _resourceProvider.pathContext.absolute(snapshotFile.path); @@ -214,11 +214,11 @@ class _Snapshot { void _snapshotCompleted() => _snapshotCompleter.complete(); } -/// A singleton that keeps track of cached snapshot files. The [dispose] +/// A class that keeps track of cached snapshot files. The [dispose] /// function must be called before process exit to clean up snapshots in the /// cache. class SnapshotCache { - static SnapshotCache _instance; + static final _instances = {}; final Folder snapshotCache; final ResourceProvider _resourceProvider; @@ -229,10 +229,12 @@ class SnapshotCache { : snapshotCache = _resourceProvider.createSystemTemp('dartdoc_snapshot_cache_'); - static SnapshotCache get instance => _instance; - - static SnapshotCache createInstance(ResourceProvider resourceProvider) => - _instance ??= SnapshotCache._(resourceProvider); + /// Returns a [SnapshotCache] for a given [ResourceProvider], creating one + /// only if one doesn't exist yet for the given [ResourceProvider]. + factory SnapshotCache.instanceFor(ResourceProvider resourceProvider) { + return _instances.putIfAbsent( + resourceProvider, () => SnapshotCache._(resourceProvider)); + } _Snapshot getSnapshot(String toolPath) { if (snapshots.containsKey(toolPath)) { @@ -245,7 +247,7 @@ class SnapshotCache { } void dispose() { - _instance = null; + _instances.remove(_resourceProvider); if (snapshotCache != null && snapshotCache.exists) { return snapshotCache.delete(); } diff --git a/lib/src/tool_runner.dart b/lib/src/tool_runner.dart index bc24e3fad8..527a0eb5bd 100644 --- a/lib/src/tool_runner.dart +++ b/lib/src/tool_runner.dart @@ -177,7 +177,7 @@ class ToolRunner { // find out where their script was coming from as an absolute path on the // filesystem. envWithInput['DART_SNAPSHOT_CACHE'] = pathContext.absolute( - SnapshotCache.createInstance(resourceProvider).snapshotCache.path); + SnapshotCache.instanceFor(resourceProvider).snapshotCache.path); if (toolDefinition.setupCommand != null) { envWithInput['DART_SETUP_COMMAND'] = toolDefinition.setupCommand[0]; } diff --git a/lib/src/warnings.dart b/lib/src/warnings.dart index acd4aa272a..46669104a7 100644 --- a/lib/src/warnings.dart +++ b/lib/src/warnings.dart @@ -277,21 +277,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 @@ -344,9 +329,6 @@ enum PackageWarning { missingConstantConstructor, missingExampleFile, missingCodeBlockLanguage, - referenceLookupFoundWithNew, - referenceLookupMissingWithNew, - referenceLookupDiffersWithNew, } /// Used to declare defaults for a particular package warning. diff --git a/pubspec.yaml b/pubspec.yaml index 62c0cc8331..b5f87f316a 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -7,7 +7,7 @@ environment: sdk: '>=2.12.0 <3.0.0' dependencies: - analyzer: ^2.0.0 + analyzer: ^2.1.0 args: ^2.0.0 charcode: ^1.2.0 collection: ^1.15.0 diff --git a/test/end2end/dartdoc_test.dart b/test/end2end/dartdoc_test.dart index 047dc158d8..21c21d1f6d 100644 --- a/test/end2end/dartdoc_test.dart +++ b/test/end2end/dartdoc_test.dart @@ -22,7 +22,7 @@ import 'package:test/test.dart'; import '../src/utils.dart'; final _experimentPackageAllowed = - VersionRange(min: Version.parse('2.14.0-0'), includeMin: true); + VersionRange(min: Version.parse('2.15.0-0'), includeMin: true); final _resourceProvider = pubPackageMetaProvider.resourceProvider; final _pathContext = _resourceProvider.pathContext; diff --git a/test/end2end/model_special_cases_test.dart b/test/end2end/model_special_cases_test.dart index 2ced723a72..dc644f5448 100644 --- a/test/end2end/model_special_cases_test.dart +++ b/test/end2end/model_special_cases_test.dart @@ -12,8 +12,8 @@ library dartdoc.model_special_cases_test; import 'dart:io'; -import 'package:analyzer/dart/element/type.dart'; import 'package:async/async.dart'; +import 'package:dartdoc/src/matching_link_result.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:dartdoc/src/package_config_provider.dart'; import 'package:dartdoc/src/package_meta.dart'; @@ -22,6 +22,7 @@ import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart'; import '../src/utils.dart' as utils; +import '../src/utils.dart'; final _testPackageGraphExperimentsMemo = AsyncMemoizer(); Future get _testPackageGraphExperiments => @@ -73,132 +74,121 @@ void main() { // ExperimentalFeature.experimentalReleaseVersion as these are set to null // even when partial analyzer implementations are available, and are often // set too high after release. - final _genericMetadataAllowed = - VersionRange(min: Version.parse('2.14.0-0'), includeMin: true); - final _tripleShiftAllowed = - VersionRange(min: Version.parse('2.14.0-0'), includeMin: true); + final _constructorTearoffsAllowed = + VersionRange(min: Version.parse('2.15.0-0'), includeMin: true); // Experimental features not yet enabled by default. Move tests out of this // block when the feature is enabled by default. group('Experiments', () { - group('triple-shift', () { - Library tripleShift; - Class C, E, F; - Extension ShiftIt; - Operator classShift, extensionShift; - Field constantTripleShifted; + group('constructor-tearoffs', () { + Library constructorTearoffs; + Class A, B, C, D, E, F; + Mixin M; + Typedef At, Bt, Ct, Et, Ft, NotAClass; + Constructor Anew, Bnew, Cnew, Dnew, Enew, Fnew; setUpAll(() async { - tripleShift = (await _testPackageGraphExperiments) + constructorTearoffs = (await _testPackageGraphExperiments) .libraries - .firstWhere((l) => l.name == 'triple_shift'); - C = tripleShift.classes.firstWhere((c) => c.name == 'C'); - E = tripleShift.classes.firstWhere((c) => c.name == 'E'); - F = tripleShift.classes.firstWhere((c) => c.name == 'F'); - ShiftIt = tripleShift.extensions.firstWhere((e) => e.name == 'ShiftIt'); - classShift = - C.instanceOperators.firstWhere((o) => o.name.contains('>>>')); - extensionShift = - ShiftIt.instanceOperators.firstWhere((o) => o.name.contains('>>>')); - constantTripleShifted = C.constantFields - .firstWhere((f) => f.name == 'constantTripleShifted'); + .firstWhere((l) => l.name == 'constructor_tearoffs'); + A = constructorTearoffs.classes.firstWhere((c) => c.name == 'A'); + B = constructorTearoffs.classes.firstWhere((c) => c.name == 'B'); + C = constructorTearoffs.classes.firstWhere((c) => c.name == 'C'); + D = constructorTearoffs.classes.firstWhere((c) => c.name == 'D'); + E = constructorTearoffs.classes.firstWhere((c) => c.name == 'E'); + F = constructorTearoffs.classes.firstWhere((c) => c.name == 'F'); + M = constructorTearoffs.mixins.firstWhere((m) => m.name == 'M'); + At = constructorTearoffs.typedefs.firstWhere((t) => t.name == 'At'); + Bt = constructorTearoffs.typedefs.firstWhere((t) => t.name == 'Bt'); + Ct = constructorTearoffs.typedefs.firstWhere((t) => t.name == 'Ct'); + Et = constructorTearoffs.typedefs.firstWhere((t) => t.name == 'Et'); + Ft = constructorTearoffs.typedefs.firstWhere((t) => t.name == 'Ft'); + NotAClass = constructorTearoffs.typedefs + .firstWhere((t) => t.name == 'NotAClass'); + Anew = A.unnamedConstructor; + Bnew = B.unnamedConstructor; + Cnew = C.unnamedConstructor; + Dnew = D.unnamedConstructor; + Enew = E.unnamedConstructor; + Fnew = F.unnamedConstructor; }); - test('constants with triple shift render correctly', () { - expect(constantTripleShifted.constantValue, equals('3 >>> 5')); + test('smoke test', () { + expect(A, isNotNull); + expect(B, isNotNull); + expect(C, isNotNull); + expect(D, isNotNull); + expect(E, isNotNull); + expect(F, isNotNull); + expect(M, isNotNull); + expect(At, isNotNull); + expect(Bt, isNotNull); + expect(Ct, isNotNull); + expect(Et, isNotNull); + expect(Ft, isNotNull); + expect(NotAClass, isNotNull); + expect(Anew, isNotNull); + expect(Bnew, isNotNull); + expect(Cnew, isNotNull); + expect(Dnew, isNotNull); + expect(Enew, isNotNull); + expect(Fnew, isNotNull); }); - test('operators exist and are named correctly', () { - expect(classShift.name, equals('operator >>>')); - expect(extensionShift.name, equals('operator >>>')); + test('reference regression', () { + expect(referenceLookup(constructorTearoffs, 'A.A'), + equals(MatchingLinkResult(Anew))); + expect(referenceLookup(constructorTearoffs, 'new A()'), + equals(MatchingLinkResult(Anew))); + expect(referenceLookup(constructorTearoffs, 'A()'), + equals(MatchingLinkResult(Anew))); + expect(referenceLookup(constructorTearoffs, 'B.B'), + equals(MatchingLinkResult(Bnew))); + expect(referenceLookup(constructorTearoffs, 'new B()'), + equals(MatchingLinkResult(Bnew))); + expect(referenceLookup(constructorTearoffs, 'B()'), + equals(MatchingLinkResult(Bnew))); + expect(referenceLookup(constructorTearoffs, 'C.C'), + equals(MatchingLinkResult(Cnew))); + expect(referenceLookup(constructorTearoffs, 'new C()'), + equals(MatchingLinkResult(Cnew))); + expect(referenceLookup(constructorTearoffs, 'C()'), + equals(MatchingLinkResult(Cnew))); + expect(referenceLookup(constructorTearoffs, 'D.D'), + equals(MatchingLinkResult(Dnew))); + expect(referenceLookup(constructorTearoffs, 'new D()'), + equals(MatchingLinkResult(Dnew))); + expect(referenceLookup(constructorTearoffs, 'D()'), + equals(MatchingLinkResult(Dnew))); + expect(referenceLookup(constructorTearoffs, 'E.E'), + equals(MatchingLinkResult(Enew))); + expect(referenceLookup(constructorTearoffs, 'new E()'), + equals(MatchingLinkResult(Enew))); + expect(referenceLookup(constructorTearoffs, 'E()'), + equals(MatchingLinkResult(Enew))); + expect(referenceLookup(constructorTearoffs, 'F.F'), + equals(MatchingLinkResult(Fnew))); + expect(referenceLookup(constructorTearoffs, 'new F()'), + equals(MatchingLinkResult(Fnew))); + expect(referenceLookup(constructorTearoffs, 'F()'), + equals(MatchingLinkResult(Fnew))); }); - test( - 'inheritance and overriding of triple shift operators works correctly', - () { - var tripleShiftE = - E.instanceOperators.firstWhere((o) => o.name.contains('>>>')); - var tripleShiftF = - F.instanceOperators.firstWhere((o) => o.name.contains('>>>')); - - expect(tripleShiftE.isInherited, isTrue); - expect(tripleShiftE.canonicalModelElement, equals(classShift)); - expect(tripleShiftE.modelType.returnType.name, equals('C')); - expect(tripleShiftF.isInherited, isFalse); - expect(tripleShiftF.modelType.returnType.name, equals('F')); + test('.new works on classes', () { + expect(referenceLookup(constructorTearoffs, 'A.new'), + equals(MatchingLinkResult(Anew))); + expect(referenceLookup(constructorTearoffs, 'B.new'), + equals(MatchingLinkResult(Bnew))); + expect(referenceLookup(constructorTearoffs, 'C.new'), + equals(MatchingLinkResult(Cnew))); + expect(referenceLookup(constructorTearoffs, 'D.new'), + equals(MatchingLinkResult(Dnew))); + expect(referenceLookup(constructorTearoffs, 'E.new'), + equals(MatchingLinkResult(Enew))); + expect(referenceLookup(constructorTearoffs, 'F.new'), + equals(MatchingLinkResult(Fnew))); }); - }, skip: !_tripleShiftAllowed.allows(utils.platformVersion)); - - group('generic metadata', () { - Library genericMetadata; - TopLevelVariable f; - Typedef F; - Class C; - Method mp, mn; - - setUpAll(() async { - genericMetadata = (await _testPackageGraphExperiments) - .libraries - .firstWhere((l) => l.name == 'generic_metadata'); - F = genericMetadata.typedefs.firstWhere((t) => t.name == 'F'); - f = genericMetadata.properties.firstWhere((p) => p.name == 'f'); - C = genericMetadata.classes.firstWhere((c) => c.name == 'C'); - mp = C.instanceMethods.firstWhere((m) => m.name == 'mp'); - mn = C.instanceMethods.firstWhere((m) => m.name == 'mn'); - }); - - test( - 'Verify annotations and their type arguments render on type parameters for typedefs', - () { - expect((F.aliasedType as FunctionType).typeFormals.first.metadata, - isNotEmpty); - expect((F.aliasedType as FunctionType).parameters.first.metadata, - isNotEmpty); - // TODO(jcollins-g): add rendering verification once we have data from - // analyzer. - }, skip: 'dart-lang/sdk#46064'); - - test('Verify type arguments on annotations renders, including parameters', - () { - var ab0 = - '@A<B>(0)'; - - expect(genericMetadata.annotations.first.linkedNameWithParameters, - equals(ab0)); - expect(f.annotations.first.linkedNameWithParameters, equals(ab0)); - expect(C.annotations.first.linkedNameWithParameters, equals(ab0)); - expect( - C.typeParameters.first.annotations.first.linkedNameWithParameters, - equals(ab0)); - expect( - mp.parameters - .map((p) => p.annotations.first.linkedNameWithParameters), - everyElement(equals(ab0))); - expect( - mn.parameters - .map((p) => p.annotations.first.linkedNameWithParameters), - everyElement(equals(ab0))); - - expect(genericMetadata.features.map((f) => f.linkedNameWithParameters), - contains(ab0)); - expect( - f.features.map((f) => f.linkedNameWithParameters), contains(ab0)); - expect( - C.features.map((f) => f.linkedNameWithParameters), contains(ab0)); - expect( - C.typeParameters.first.features - .map((f) => f.linkedNameWithParameters), - contains(ab0)); - expect( - mp.parameters - .map((p) => p.features.map((f) => f.linkedNameWithParameters)), - everyElement(contains(ab0))); - expect( - mn.parameters - .map((p) => p.features.map((f) => f.linkedNameWithParameters)), - everyElement(contains(ab0))); - }); - }, skip: !_genericMetadataAllowed.allows(utils.platformVersion)); + }, skip: !_constructorTearoffsAllowed.allows(utils.platformVersion)); }); group('HTML Injection when allowed', () { diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart index 769ecd2935..b820e6a6cb 100644 --- a/test/end2end/model_test.dart +++ b/test/end2end/model_test.dart @@ -8,10 +8,10 @@ library dartdoc.model_test; import 'dart:io'; +import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/source/line_info.dart'; import 'package:async/async.dart'; import 'package:dartdoc/src/element_type.dart'; -import 'package:dartdoc/src/markdown_processor.dart'; import 'package:dartdoc/src/matching_link_result.dart'; import 'package:dartdoc/src/model/feature.dart'; import 'package:dartdoc/src/model/model.dart'; @@ -26,14 +26,13 @@ import 'package:dartdoc/src/special_elements.dart'; import 'package:dartdoc/src/warnings.dart'; import 'package:test/test.dart'; -import '../src/utils.dart' as utils; +import '../src/utils.dart' + show bootBasicPackage, referenceLookup, kTestPackagePublicLibraries; final _testPackageGraphMemo = AsyncMemoizer(); Future get testPackageGraph async => - _testPackageGraphMemo.runOnce(() => utils.bootBasicPackage( - 'testing/test_package', - pubPackageMetaProvider, - PhysicalPackageConfigProvider(), + _testPackageGraphMemo.runOnce(() => bootBasicPackage('testing/test_package', + pubPackageMetaProvider, PhysicalPackageConfigProvider(), excludeLibraries: ['css', 'code_in_comments'], additionalArguments: ['--no-link-to-remote'])); @@ -96,6 +95,121 @@ void main() { packageGraph.libraries.firstWhere((lib) => lib.name == 'base_class'); }); + group('triple-shift', () { + Library tripleShift; + Class C, E, F; + Extension ShiftIt; + Operator classShift, extensionShift; + Field constantTripleShifted; + + setUpAll(() async { + tripleShift = (await testPackageGraph) + .libraries + .firstWhere((l) => l.name == 'triple_shift'); + C = tripleShift.classes.firstWhere((c) => c.name == 'C'); + E = tripleShift.classes.firstWhere((c) => c.name == 'E'); + F = tripleShift.classes.firstWhere((c) => c.name == 'F'); + ShiftIt = tripleShift.extensions.firstWhere((e) => e.name == 'ShiftIt'); + classShift = + C.instanceOperators.firstWhere((o) => o.name.contains('>>>')); + extensionShift = + ShiftIt.instanceOperators.firstWhere((o) => o.name.contains('>>>')); + constantTripleShifted = + C.constantFields.firstWhere((f) => f.name == 'constantTripleShifted'); + }); + + test('constants with triple shift render correctly', () { + expect(constantTripleShifted.constantValue, equals('3 >>> 5')); + }); + + test('operators exist and are named correctly', () { + expect(classShift.name, equals('operator >>>')); + expect(extensionShift.name, equals('operator >>>')); + }); + + test('inheritance and overriding of triple shift operators works correctly', + () { + var tripleShiftE = + E.instanceOperators.firstWhere((o) => o.name.contains('>>>')); + var tripleShiftF = + F.instanceOperators.firstWhere((o) => o.name.contains('>>>')); + + expect(tripleShiftE.isInherited, isTrue); + expect(tripleShiftE.canonicalModelElement, equals(classShift)); + expect(tripleShiftE.modelType.returnType.name, equals('C')); + expect(tripleShiftF.isInherited, isFalse); + expect(tripleShiftF.modelType.returnType.name, equals('F')); + }); + }); + + group('generic metadata', () { + Library genericMetadata; + TopLevelVariable f; + Typedef F; + Class C; + Method mp, mn; + + setUpAll(() async { + genericMetadata = (await testPackageGraph) + .libraries + .firstWhere((l) => l.name == 'generic_metadata'); + F = genericMetadata.typedefs.firstWhere((t) => t.name == 'F'); + f = genericMetadata.properties.firstWhere((p) => p.name == 'f'); + C = genericMetadata.classes.firstWhere((c) => c.name == 'C'); + mp = C.instanceMethods.firstWhere((m) => m.name == 'mp'); + mn = C.instanceMethods.firstWhere((m) => m.name == 'mn'); + }); + + test( + 'Verify annotations and their type arguments render on type parameters for typedefs', + () { + expect((F.aliasedType as FunctionType).typeFormals.first.metadata, + isNotEmpty); + expect((F.aliasedType as FunctionType).parameters.first.metadata, + isNotEmpty); + // TODO(jcollins-g): add rendering verification once we have data from + // analyzer. + }, skip: 'dart-lang/sdk#46064'); + + test('Verify type arguments on annotations renders, including parameters', + () { + var ab0 = + '@A<B>(0)'; + + expect(genericMetadata.annotations.first.linkedNameWithParameters, + equals(ab0)); + expect(f.annotations.first.linkedNameWithParameters, equals(ab0)); + expect(C.annotations.first.linkedNameWithParameters, equals(ab0)); + expect(C.typeParameters.first.annotations.first.linkedNameWithParameters, + equals(ab0)); + expect( + mp.parameters + .map((p) => p.annotations.first.linkedNameWithParameters), + everyElement(equals(ab0))); + expect( + mn.parameters + .map((p) => p.annotations.first.linkedNameWithParameters), + everyElement(equals(ab0))); + + expect(genericMetadata.features.map((f) => f.linkedNameWithParameters), + contains(ab0)); + expect(f.features.map((f) => f.linkedNameWithParameters), contains(ab0)); + expect(C.features.map((f) => f.linkedNameWithParameters), contains(ab0)); + expect( + C.typeParameters.first.features + .map((f) => f.linkedNameWithParameters), + contains(ab0)); + expect( + mp.parameters + .map((p) => p.features.map((f) => f.linkedNameWithParameters)), + everyElement(contains(ab0))); + expect( + mn.parameters + .map((p) => p.features.map((f) => f.linkedNameWithParameters)), + everyElement(contains(ab0))); + }); + }); + group('generalized typedefs', () { Library generalizedTypedefs; Typedef T0, T1, T2, T3, T4, T5, T6, T7; @@ -673,7 +787,7 @@ void main() { packageGraph .localPackages.first.defaultCategory.publicLibraries.length, // Only 5 libraries have categories, the rest belong in default. - equals(utils.kTestPackagePublicLibraries - 5)); + equals(kTestPackagePublicLibraries - 5)); }); // TODO consider moving these to a separate suite @@ -1741,11 +1855,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 @@ -2126,34 +2235,40 @@ void main() { // Put linkage tests here; rendering tests should go to the appropriate // [Class], [Extension], etc groups. group('Comment References link tests', () { - /// For comparison purposes, return an equivalent [MatchingLinkResult] - /// for the defining element returned. May return [originalResult]. - /// We do this to eliminate canonicalization effects from comparison, - /// as the original lookup code returns canonicalized results and the - /// new lookup code is only guaranteed to return equivalent results. - MatchingLinkResult definingLinkResult(MatchingLinkResult originalResult) { - if (originalResult.commentReferable?.element != null) { - return MatchingLinkResult( - ModelElement.fromElement(originalResult.commentReferable.element, - originalResult.commentReferable.packageGraph), - warn: originalResult.warn); - } - return originalResult; - } + group('Linking for generalized typedef cases works', () { + Library generalizedTypedefs; + Typedef T0, T2, T5, T8; + Class C2; - 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; - } + setUpAll(() { + generalizedTypedefs = packageGraph.libraries + .firstWhere((l) => l.name == 'generalized_typedefs'); + T0 = generalizedTypedefs.typedefs.firstWhere((a) => a.name == 'T0'); + T2 = generalizedTypedefs.typedefs.firstWhere((a) => a.name == 'T2'); + T5 = generalizedTypedefs.typedefs.firstWhere((a) => a.name == 'T5'); + T8 = generalizedTypedefs.typedefs.firstWhere((a) => a.name == 'T8'); + C2 = generalizedTypedefs.classes.firstWhere((c) => c.name == 'C2'); + }); + + test('Verify basic ability to link anything', () { + 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(referenceLookup(T2, 'X'), equals(MatchingLinkResult(T2X))); + var T5X = T5.typeParameters.firstWhere((t) => t.name == 'X'); + expect(referenceLookup(T5, 'X'), equals(MatchingLinkResult(T5X))); + }); + + test('Verify ability to link to parameters', () { + var T5name = T5.parameters.firstWhere((t) => t.name == 'name'); + expect(referenceLookup(T5, 'name'), equals(MatchingLinkResult(T5name))); + }); + }); group('Linking for complex inheritance and reexport cases', () { Library base, extending, local_scope, two_exports; @@ -2208,49 +2323,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))); }); }); @@ -2315,56 +2425,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))); }); }); @@ -2543,32 +2651,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))); }); @@ -2577,75 +2689,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))); }); @@ -2653,130 +2765,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 46b0e60be7..298024db7b 100644 --- a/test/src/utils.dart +++ b/test/src/utils.dart @@ -12,16 +12,20 @@ import 'package:analyzer/file_system/file_system.dart'; import 'package:analyzer/file_system/memory_file_system.dart'; import 'package:analyzer/src/test_utilities/mock_sdk.dart'; import 'package:dartdoc/src/dartdoc_options.dart'; +import 'package:dartdoc/src/markdown_processor.dart'; +import 'package:dartdoc/src/matching_link_result.dart'; +import 'package:dartdoc/src/model/model_element.dart'; import 'package:dartdoc/src/model/package_builder.dart'; import 'package:dartdoc/src/model/package_graph.dart'; 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'; /// The number of public libraries in testing/test_package, minus 2 for /// the excluded libraries listed in the initializers for _testPackageGraphMemo /// and minus 1 for the tag in the 'excluded' library. -const int kTestPackagePublicLibraries = 24; +const int kTestPackagePublicLibraries = 27; final _resourceProvider = pubPackageMetaProvider.resourceProvider; final _pathContext = _resourceProvider.pathContext; @@ -131,7 +135,8 @@ Folder writeMockSdkFiles(MockSdk mockSdk) { void _writeMockSdkBinFiles(Folder root) { var sdkBinFolder = root.getChildAssumingFolder('bin'); sdkBinFolder.getChildAssumingFile('dart').writeAsStringSync(''); - sdkBinFolder.getChildAssumingFile('pub').writeAsStringSync(''); + var sdkIncludeFolder = root.getChildAssumingFolder('include'); + sdkIncludeFolder.getChildAssumingFile('dart_version.h').writeAsStringSync(''); } /// Writes a package named [packageName], with [resourceProvider], to the @@ -192,3 +197,21 @@ two:lib/ return projectFolder; } + +/// For comparison purposes, return an equivalent [MatchingLinkResult] +/// for the defining element returned. May return [originalResult]. +/// We do this to eliminate canonicalization effects from comparison, +/// as the original lookup code returns canonicalized results and the +/// new lookup code is only guaranteed to return equivalent results. +MatchingLinkResult definingLinkResult(MatchingLinkResult originalResult) { + if (originalResult.commentReferable?.element != null) { + return MatchingLinkResult( + ModelElement.fromElement(originalResult.commentReferable.element, + originalResult.commentReferable.packageGraph), + warn: originalResult.warn); + } + return originalResult; +} + +MatchingLinkResult referenceLookup(Warnable element, String codeRef) => + definingLinkResult(getMatchingLinkElement(element, codeRef)); diff --git a/test/tool_runner_test.dart b/test/tool_runner_test.dart index 015b50f0d1..afacb3deba 100644 --- a/test/tool_runner_test.dart +++ b/test/tool_runner_test.dart @@ -97,7 +97,8 @@ echo: tearDownAll(() { tempDir?.deleteSync(recursive: true); tracker?.dispose(); - SnapshotCache.instance?.dispose(); + SnapshotCache.instanceFor(pubPackageMetaProvider.resourceProvider) + .dispose(); setupFile = null; tempDir = null; }); diff --git a/testing/test_package_experiments/lib/generic_functions_as_type_args.dart b/testing/test_package/lib/features/generic_functions_as_type_args.dart similarity index 100% rename from testing/test_package_experiments/lib/generic_functions_as_type_args.dart rename to testing/test_package/lib/features/generic_functions_as_type_args.dart diff --git a/testing/test_package_experiments/lib/generic_metadata.dart b/testing/test_package/lib/features/generic_metadata.dart similarity index 100% rename from testing/test_package_experiments/lib/generic_metadata.dart rename to testing/test_package/lib/features/generic_metadata.dart diff --git a/testing/test_package_experiments/lib/triple_shift.dart b/testing/test_package/lib/features/triple_shift.dart similarity index 100% rename from testing/test_package_experiments/lib/triple_shift.dart rename to testing/test_package/lib/features/triple_shift.dart diff --git a/testing/test_package/pubspec.yaml b/testing/test_package/pubspec.yaml index 0f3b87cfac..0ff1a4cc49 100644 --- a/testing/test_package/pubspec.yaml +++ b/testing/test_package/pubspec.yaml @@ -8,4 +8,4 @@ dependencies: test_package_imported: path: "../test_package_imported" environment: - sdk: '>=2.13.0 <3.0.0' + sdk: '>=2.14.0-0 <3.0.0' diff --git a/testing/test_package_experiments/analysis_options.yaml b/testing/test_package_experiments/analysis_options.yaml index 620d59460b..8f22a4573b 100644 --- a/testing/test_package_experiments/analysis_options.yaml +++ b/testing/test_package_experiments/analysis_options.yaml @@ -5,3 +5,4 @@ analyzer: - nonfunction-type-aliases - triple-shift - generic-metadata + - constructor-tearoffs diff --git a/testing/test_package_experiments/lib/constructor_tearoffs.dart b/testing/test_package_experiments/lib/constructor_tearoffs.dart new file mode 100644 index 0000000000..6c3e87dd34 --- /dev/null +++ b/testing/test_package_experiments/lib/constructor_tearoffs.dart @@ -0,0 +1,61 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// 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. + +/// Exercise a number of constructor-tearoff related features inside dartdoc. +/// +/// References to tearoffs should work at a top level too: +/// [A.new], [B.new], [At.new], [Bt.new], [C], [C.new], [D.new], +/// +library constructor_tearoffs; + +abstract class A { + final int number; + /// Even though this is abstract, dartdoc should still allow referring to + /// [A.new]. + A.new(this.number); +} + +class B extends A { + B.new() : super(5); +} + +class C {} + +typedef At = A; +typedef Bt = B; +typedef Ct = C; + +/// [Dt.new] and [D.new] should be a thing. +abstract class D {} + +typedef Dt = D; + +/// I refer to many things, including typedef constructor tearoffs [At.new], +/// [Bt.new], [Ct.new], [Et.new]. +/// Don't forget about [E.new], [E.E], or [D.new]. +class E extends D { + final String aValue; + E.new(this.aValue) {} +} + +typedef Et = E; + +/// Referring to [F.new] and [F.new] should work fine. +class F { + F() { + print('I too am a valid constructor invocation with this feature.'); + } +} + +typedef Ft = F; + +/// Referring to [Fstring.new] should be fine. +typedef Fstring = F; + +/// Can't refer to `.new` here. +typedef NotAClass = Function; + +/// Mixins don't have constructors either, so disallow `M.new`. +mixin M on C { +} \ No newline at end of file diff --git a/testing/test_package_experiments/pubspec.yaml b/testing/test_package_experiments/pubspec.yaml index d0ee15f855..fc0fc1ed86 100644 --- a/testing/test_package_experiments/pubspec.yaml +++ b/testing/test_package_experiments/pubspec.yaml @@ -1,5 +1,5 @@ name: test_package_experiments version: 0.0.1 environment: - sdk: '>=2.14.0-0 <3.0.0' + sdk: '>=2.15.0-0 <3.0.0' description: Experimental flags are tested here. diff --git a/tool/grind.dart b/tool/grind.dart index d9723f8d91..6aa411cf35 100644 --- a/tool/grind.dart +++ b/tool/grind.dart @@ -234,11 +234,16 @@ void updateThirdParty() async { } @Task('Analyze dartdoc to ensure there are no errors and warnings') +@Depends(analyzeTestPackages) void analyze() async { await SubprocessLauncher('analyze').runStreamed( sdkBin('dartanalyzer'), ['--fatal-infos', '--options', 'analysis_options_presubmit.yaml', '.'], ); +} + +@Task('Analyze the test packages') +void analyzeTestPackages() async { var testPackagePaths = [testPackage.path]; if (Platform.version.contains('dev')) { testPackagePaths.add(testPackageExperiments.path);