Skip to content

Basic .new support for constructors #2766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Aug 30, 2021
Merged
35 changes: 18 additions & 17 deletions lib/src/comment_references/model_comment_reference.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<IdentifierNode>().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
Expand Down
20 changes: 9 additions & 11 deletions lib/src/markdown_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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;
}
}

Expand Down
18 changes: 13 additions & 5 deletions lib/src/model/class.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<Method> get instanceMethods =>
Expand Down Expand Up @@ -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);
}
}
}

Expand Down
14 changes: 4 additions & 10 deletions lib/src/model/constructor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
15 changes: 15 additions & 0 deletions test/end2end/model_special_cases_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});

Expand Down
5 changes: 5 additions & 0 deletions tool/grind.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down