Skip to content

Initial implementation of constructor tearoff support #2763

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.

Sign up for GitHub

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 4 commits into from
Aug 25, 2021
Merged

Initial implementation of constructor tearoff support #2763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9c36806
Initial implementation of constructor tearoff support
jcollins-g Aug 24, 2021
622db50
Merge branch 'master' into constructor-tearoffs-smoke
jcollins-g Aug 24, 2021
aec4729
Move older experiments out now that the beta branch exists
jcollins-g Aug 24, 2021
69556c5
initial review comments
jcollins-g Aug 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/src/model/constructor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,13 @@ class Constructor extends ModelElement
@override
bool get isConst => element.isConst;

bool get isUnnamedConstructor => name == enclosingElement.name;
bool get isUnnamedConstructor =>
name == enclosingElement.name || name == '${enclosingElement.name}.new';

@Deprecated(
// TODO(jcollins-g): This, in retrospect, seems like a bad idea.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, why is that? Unnamed constructors and "default" constructors are different things (the latter being a constructor which is implicitly created when a class features no constructors).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly because it is confusing and the two ideas are used interchangeably here. I'll try and clarify.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah interesting. Yeah we'd have to see if it is important to distinguish. For example:

class C {
  C(int i);
}

has a constructor whose isUnnamedConstructor is true, but it is not a default constructor (it cannot be implicitly called by any subclass's constructor).

// 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;
Expand Down Expand Up @@ -144,5 +148,5 @@ class Constructor extends ModelElement

@override
String get referenceName =>
element.name == '' ? enclosingElement.name : element.name;
isUnnamedConstructor ? enclosingElement.name : element.name;
}
2 changes: 1 addition & 1 deletion test/end2end/dartdoc_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,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;
Expand Down
205 changes: 90 additions & 115 deletions test/end2end/model_special_cases_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,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';
Expand All @@ -20,6 +20,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<PackageGraph>();
Future<PackageGraph> get _testPackageGraphExperiments =>
Expand Down Expand Up @@ -71,132 +72,106 @@ 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 &gt;&gt;&gt; 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(newLookup(constructorTearoffs, 'A.A'),
equals(MatchingLinkResult(Anew)));
expect(newLookup(constructorTearoffs, 'new A()'),
equals(MatchingLinkResult(Anew)));
expect(newLookup(constructorTearoffs, 'A()'),
equals(MatchingLinkResult(Anew)));
expect(newLookup(constructorTearoffs, 'B.B'),
equals(MatchingLinkResult(Bnew)));
expect(newLookup(constructorTearoffs, 'new B()'),
equals(MatchingLinkResult(Bnew)));
expect(newLookup(constructorTearoffs, 'B()'),
equals(MatchingLinkResult(Bnew)));
expect(newLookup(constructorTearoffs, 'C.C'),
equals(MatchingLinkResult(Cnew)));
expect(newLookup(constructorTearoffs, 'new C()'),
equals(MatchingLinkResult(Cnew)));
expect(newLookup(constructorTearoffs, 'C()'),
equals(MatchingLinkResult(Cnew)));
expect(newLookup(constructorTearoffs, 'D.D'),
equals(MatchingLinkResult(Dnew)));
expect(newLookup(constructorTearoffs, 'new D()'),
equals(MatchingLinkResult(Dnew)));
expect(newLookup(constructorTearoffs, 'D()'),
equals(MatchingLinkResult(Dnew)));
expect(newLookup(constructorTearoffs, 'E.E'),
equals(MatchingLinkResult(Enew)));
expect(newLookup(constructorTearoffs, 'new E()'),
equals(MatchingLinkResult(Enew)));
expect(newLookup(constructorTearoffs, 'E()'),
equals(MatchingLinkResult(Enew)));
expect(newLookup(constructorTearoffs, 'F.F'),
equals(MatchingLinkResult(Fnew)));
expect(newLookup(constructorTearoffs, 'new F()'),
equals(MatchingLinkResult(Fnew)));
expect(newLookup(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'));
});
}, 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 href="%%__HTMLBASE_dartdoc_internal__%%generic_metadata/A-class.html">A</a><span class="signature">&lt;<wbr><span class="type-parameter"><a href="%%__HTMLBASE_dartdoc_internal__%%generic_metadata/B.html">B</a></span>&gt;</span>(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', () {
Expand Down
Loading