diff --git a/lib/src/comment_references/model_comment_reference.dart b/lib/src/comment_references/model_comment_reference.dart index 7d6e0fcbfb..8696006e1c 100644 --- a/lib/src/comment_references/model_comment_reference.dart +++ b/lib/src/comment_references/model_comment_reference.dart @@ -8,12 +8,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; @@ -34,42 +32,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/markdown_processor.dart b/lib/src/markdown_processor.dart index 07a4ca7562..5fd12cfa8b 100644 --- a/lib/src/markdown_processor.dart +++ b/lib/src/markdown_processor.dart @@ -157,14 +157,12 @@ class _IterableBlockParser extends md.BlockParser { } } -/// 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; @@ -191,12 +189,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 @@ -212,12 +210,12 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable( // 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; } } diff --git a/lib/src/model/class.dart b/lib/src/model/class.dart index db3f826765..85090ca9b4 100644 --- a/lib/src/model/class.dart +++ b/lib/src/model/class.dart @@ -70,10 +70,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 => @@ -613,8 +618,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 155fdf27eb..0db067279e 100644 --- a/lib/src/model/constructor.dart +++ b/lib/src/model/constructor.dart @@ -67,16 +67,10 @@ class Constructor extends ModelElement @override bool get isConst => element.isConst; - bool get isUnnamedConstructor => - name == enclosingElement.name || name == '${enclosingElement.name}.new'; - - @Deprecated( - // TODO(jcollins-g): This, in retrospect, seems like a bad idea. - // We should disambiguate the two concepts (default and unnamed) and - // allow both if useful. - '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 isUnnamedConstructor => name == enclosingElement.name; + + bool get isDefaultConstructor => + name == '${enclosingElement.name}.new' || isUnnamedConstructor; bool get isFactory => element.isFactory; diff --git a/test/end2end/model_special_cases_test.dart b/test/end2end/model_special_cases_test.dart index 947d76043e..0e2af4366b 100644 --- a/test/end2end/model_special_cases_test.dart +++ b/test/end2end/model_special_cases_test.dart @@ -171,6 +171,21 @@ void main() { expect(referenceLookup(constructorTearoffs, 'F()'), equals(MatchingLinkResult(Fnew))); }); + + 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: !_constructorTearoffsAllowed.allows(utils.platformVersion)); }); diff --git a/tool/grind.dart b/tool/grind.dart index 365730ab22..8fce2ae6c8 100644 --- a/tool/grind.dart +++ b/tool/grind.dart @@ -232,11 +232,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);