Skip to content

Allow for anonymous typed functions #1506

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 18 commits into from
Oct 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ install:
- set PATH=%PATH%;C:\tools\dart-sdk\bin
- set PATH=%PATH%;%APPDATA%\Pub\Cache\bin
- pub get
- cmd: cd testing
Copy link
Member

@devoncarew devoncarew Sep 28, 2017

Choose a reason for hiding this comment

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

(cd testing/test_package; pub get)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is Windows, so that won't work. I deliberately wrote this in the most generic way possible for clarity.

- cmd: cd test_package
- cmd: pub get
- cmd: cd ..
- cmd: cd ..

build: off

Expand Down
52 changes: 24 additions & 28 deletions lib/src/element_type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,42 +21,38 @@ class ElementType {

bool get isFunctionType => (_type is FunctionType);

bool get isParameterizedType {
if (_type is FunctionType) {
return typeArguments.isNotEmpty;
} else if (_type is ParameterizedType) {
return (_type as ParameterizedType).typeArguments.isNotEmpty;
}
return false;
}
bool get isParameterizedType => (_type is ParameterizedType);
Copy link
Member

Choose a reason for hiding this comment

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

This matches the above, but the parans could be dropped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dart style guide has no opinion, so leaving to match style.


bool get isParameterType => (_type is TypeParameterType);

String get linkedName {
if (_linkedName != null) return _linkedName;

StringBuffer buf = new StringBuffer();

if (isParameterType) {
buf.write(name);
} else {
buf.write(element.linkedName);
}
if (_linkedName == null) {
StringBuffer buf = new StringBuffer();

// not TypeParameterType or Void or Union type
if (isParameterizedType) {
if (typeArguments.every((t) => t.linkedName == 'dynamic')) {
_linkedName = buf.toString();
return _linkedName;
if (isParameterType) {
Copy link
Member

Choose a reason for hiding this comment

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

buf.write(isParameterType ? name : element.linkedName)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is all code from before that was indented. Leaving.

buf.write(name);
} else {
buf.write(element.linkedName);
}
if (typeArguments.isNotEmpty) {
buf.write('<');
buf.writeAll(typeArguments.map((t) => t.linkedName), ', ');
buf.write('>');

// not TypeParameterType or Void or Union type
if (isParameterizedType) {
if (!typeArguments.every((t) => t.linkedName == 'dynamic') &&
typeArguments.isNotEmpty) {
buf.write('<');
buf.writeAll(typeArguments.map((t) => t.linkedName), ', ');
buf.write('>');
}
// Hide parameters if there's a an explicit typedef behind this
// element, but if there is no typedef, be explicit.
if (element is ModelFunctionAnonymous) {
buf.write('(');
Copy link
Member

Choose a reason for hiding this comment

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

I guess you're avoiding string interpolation here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to maintain a consistent style with what was previously there.

buf.write(element.linkedParams());
buf.write(')');
}
}
_linkedName = buf.toString();
}
_linkedName = buf.toString();

return _linkedName;
}

Expand Down
67 changes: 54 additions & 13 deletions lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ class Class extends ModelElement implements EnclosedElement {

@override
String get nameWithGenerics {
if (!modelType.isParameterizedType) return name;
if (!modelType.isParameterizedType || _typeParameters.isEmpty) return name;
return '$name<${_typeParameters.map((t) => t.name).join(', ')}>';
}

Expand Down Expand Up @@ -2216,6 +2216,14 @@ abstract class ModelElement extends Nameable
}
if (e is FunctionElement) {
newModelElement = new ModelFunction(e, library);
} else if (e is GenericFunctionTypeElement) {
if (e is FunctionTypeAliasElement) {
assert(e.name != '');
newModelElement = new ModelFunctionTypedef(e, library);
} else {
assert(e.name == '');
newModelElement = new ModelFunctionAnonymous(e, library);
}
}
if (e is FunctionTypeAliasElement) {
newModelElement = new Typedef(e, library);
Expand Down Expand Up @@ -2285,10 +2293,7 @@ abstract class ModelElement extends Nameable
}
}
}
// TODO(jcollins-g): Consider subclass for ModelFunctionTyped.
if (e is GenericFunctionTypeElement) {
newModelElement = new ModelFunctionTyped(e, library);
}


if (newModelElement == null) throw "Unknown type ${e.runtimeType}";
if (enclosingClass != null) assert(newModelElement is Inheritable);
Expand Down Expand Up @@ -3203,6 +3208,7 @@ abstract class ModelElement extends Nameable
}
}

/// A [ModelElement] for a [FunctionElement] that isn't part of a type definition.
class ModelFunction extends ModelFunctionTyped {
ModelFunction(FunctionElement element, Library library)
: super(element, library);
Expand All @@ -3212,10 +3218,52 @@ class ModelFunction extends ModelFunctionTyped {
return _func.isStatic;
}

@override
String get name {
if (element.enclosingElement is ParameterElement && super.name.isEmpty)
return element.enclosingElement.name;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure of the context of this code, so it might not matter, but parameters are no longer required to have a name (when inside a generic function type).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's OK here. With this CL, it's now more OK than it used to be, in fact.

return super.name;
}

@override
FunctionElement get _func => (element as FunctionElement);
}

/// A [ModelElement] for a [GenericModelFunctionElement] that is an
/// explicit typedef.
///
/// Distinct from ModelFunctionTypedef in that it doesn't
/// have a name, but we document it as "Function" to match how these are
/// written in declarations.
class ModelFunctionAnonymous extends ModelFunctionTyped {
ModelFunctionAnonymous(FunctionTypedElement element, Library library)
: super(element, library) {}

@override
String get name => 'Function';

@override
bool get isPublic => false;
}

/// A [ModelElement] for a [GenericModelFunctionElement] that is part of an
/// explicit typedef.
class ModelFunctionTypedef extends ModelFunctionTyped {
ModelFunctionTypedef(FunctionTypedElement element, Library library)
: super(element, library) {}

@override
String get name {
Element e = element;
while (e != null) {
if (e is FunctionTypeAliasElement) return e.name;
e = e.enclosingElement;
}
assert(false);
return super.name;
}
}

class ModelFunctionTyped extends ModelElement
with SourceCodeMixin
implements EnclosedElement {
Expand All @@ -3238,13 +3286,6 @@ class ModelFunctionTyped extends ModelElement

String get fileName => "$name.html";

@override
String get name {
if (element.enclosingElement is ParameterElement && super.name.isEmpty)
return element.enclosingElement.name;
return super.name;
}

@override
String get href {
if (canonicalLibrary == null) return null;
Expand Down Expand Up @@ -4590,7 +4631,7 @@ class Typedef extends ModelElement

@override
String get nameWithGenerics {
if (!modelType.isParameterizedType) return name;
if (!modelType.isParameterizedType || _typeParameters.isEmpty) return name;
return '$name<${_typeParameters.map((t) => t.name).join(', ')}>';
}

Expand Down
10 changes: 6 additions & 4 deletions lib/src/model_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,14 @@ bool isInExportedLibraries(

final RegExp slashes = new RegExp('[\/]');
bool hasPrivateName(Element e) {
if (e.name.startsWith('_') ||
(e is LibraryElement &&
(e.identifier == 'dart:_internal' ||
e.identifier == 'dart:nativewrappers'))) {
if (e.name.startsWith('_')) {
return true;
}
if (e is LibraryElement &&
(e.identifier.startsWith('dart:_') ||
['dart:nativewrappers'].contains(e.identifier))) {
return true;
}
if (e is LibraryElement) {
List<String> locationParts = e.location.components[0].split(slashes);
// TODO(jcollins-g): Implement real cross package detection
Expand Down
16 changes: 14 additions & 2 deletions pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ packages:
url: "https://pub.dartlang.org"
source: hosted
version: "1.0.0"
js:
description:
name: js
url: "https://pub.dartlang.org"
source: hosted
version: "0.6.1"
kernel:
description:
name: kernel
Expand Down Expand Up @@ -169,6 +175,12 @@ packages:
url: "https://pub.dartlang.org"
source: hosted
version: "1.1.0"
node_preamble:
description:
name: node_preamble
url: "https://pub.dartlang.org"
source: hosted
version: "1.4.0"
package_config:
description:
name: package_config
Expand Down Expand Up @@ -294,7 +306,7 @@ packages:
name: test
url: "https://pub.dartlang.org"
source: hosted
version: "0.12.20+13"
version: "0.12.24+6"
tuple:
description:
name: tuple
Expand Down Expand Up @@ -350,4 +362,4 @@ packages:
source: hosted
version: "2.1.12"
sdks:
dart: ">=1.23.0-dev.11.5 <2.0.0-dev.infinity"
dart: ">=1.23.0 <=2.0.0-dev.2.0"
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ dev_dependencies:
http: ^0.11.0
meta: ^1.0.0
pub_semver: ^1.0.0
test: ^0.12.0
test: '^0.12.20+24'
executables:
dartdoc: null
29 changes: 27 additions & 2 deletions test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ void main() {
});

test('correctly finds all the classes', () {
expect(classes, hasLength(21));
expect(classes, hasLength(22));
});

test('abstract', () {
Expand Down Expand Up @@ -1027,9 +1027,10 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
});

group('Method', () {
Class classB, klass, HasGenerics, Cat, CatString;
Class classB, klass, HasGenerics, Cat, CatString, TypedFunctionsWithoutTypedefs;
Method m1, isGreaterThan, m4, m5, m6, m7, convertToMap, abstractMethod;
Method inheritedClear, testGeneric, testGenericMethod;
Method getAFunctionReturningVoid;

setUp(() {
klass = exLibrary.classes.singleWhere((c) => c.name == 'Klass');
Expand Down Expand Up @@ -1061,6 +1062,8 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
.singleWhere((m) => m.name == 'testGenericMethod');
convertToMap = HasGenerics.instanceMethods
.singleWhere((m) => m.name == 'convertToMap');
TypedFunctionsWithoutTypedefs = exLibrary.classes.singleWhere((c) => c.name == 'TypedFunctionsWithoutTypedefs');
getAFunctionReturningVoid = TypedFunctionsWithoutTypedefs.instanceMethods.singleWhere((m) => m.name == 'getAFunctionReturningVoid');
});

tearDown(() {
Expand All @@ -1070,6 +1073,15 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
}
});

test('verify parameters to types are displayed', () {
var matcher = new RegExp('Function\\(<span class="parameter" id="getAFunctionReturningVoid-param-"><span class="type-annotation">T.</span></span> <span class="parameter" id="getAFunctionReturningVoid-param-"><span class="type-annotation">T.</span></span>\\)');
expect(matcher.hasMatch(getAFunctionReturningVoid.linkedReturnType), isTrue);
});

test('verify parameter types are correctly displayed', () {
expect(getAFunctionReturningVoid.linkedReturnType, equals('Function(<span class="parameter" id="getAFunctionReturningVoid-param-"><span class="type-annotation">T1</span></span> <span class="parameter" id="getAFunctionReturningVoid-param-"><span class="type-annotation">T2</span></span>)'));
}, skip: 'blocked on https://github.com/dart-lang/sdk/issues/30146');

test('has a fully qualified name', () {
expect(m1.fullyQualifiedName, 'ex.B.m1');
});
Expand Down Expand Up @@ -1757,13 +1769,26 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
group('Typedef', () {
Typedef t;
Typedef generic;
Typedef aComplexTypedef;

setUp(() {
t = exLibrary.typedefs.firstWhere((t) => t.name == 'processMessage');
generic =
fakeLibrary.typedefs.firstWhere((t) => t.name == 'NewGenericTypedef');
aComplexTypedef = exLibrary.typedefs.firstWhere((t) => t.name == 'aComplexTypedef');
});

test('anonymous nested functions inside typedefs are handled', () {
expect(aComplexTypedef, isNotNull);
expect(aComplexTypedef.linkedReturnType, startsWith('Function'));
expect(aComplexTypedef.nameWithGenerics, equals('aComplexTypedef&lt;A1, A2, A3&gt;'));
});

test('anonymous nested functions inside typedefs are handled correctly', () {
expect(aComplexTypedef.linkedReturnType, equals('Function(<span class="parameter" id="-param-"><span class="type-annotation">A1</span></span> <span class="parameter" id="-param-"><span class="type-annotation">A2</span></span> <span class="parameter" id="-param-"><span class="type-annotation">A3</span></span>)'));
expect(aComplexTypedef.linkedParamsLines, equals('<span class="parameter" id="aComplexTypedef-param-"><span class="type-annotation">A3</span></span> <span class="parameter" id="aComplexTypedef-param-"><span class="type-annotation">String</span></span>'));
}, skip: 'blocked on https://github.com/dart-lang/sdk/issues/30146');

test('has a fully qualified name', () {
expect(t.fullyQualifiedName, 'ex.processMessage');
expect(generic.fullyQualifiedName, 'fake.NewGenericTypedef');
Expand Down
13 changes: 13 additions & 0 deletions testing/test_package/lib/example.dart
Original file line number Diff line number Diff line change
Expand Up @@ -424,3 +424,16 @@ class _RetainedEnum {
@override
String toString() => name;
}

/// Someone might do this some day.
typedef aComplexTypedef<A1, A2, A3> = void Function(A1, A2, A3) Function(A3, String);

/// This class has a complicated type situation.
abstract class TypedFunctionsWithoutTypedefs {
/// Returns a function that returns a void with some generic types sprinkled in.
void Function(T1, T2) getAFunctionReturningVoid<T1, T2>(
void callback(T1 argument1, T2 argument2));

/// Returns a complex typedef that includes some anonymous typed functions.
aComplexTypedef getAComplexTypedef<A4, A5, A6>();
}
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/Animal-class.html
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
<li><a href="ex/WithGenericSub-class.html">WithGenericSub</a></li>

Expand Down Expand Up @@ -87,6 +88,7 @@ <h5>library ex</h5>
<li><a href="ex/Animal-class.html">Animal</a></li>

<li class="section-title"><a href="ex/ex-library.html#typedefs">Typedefs</a></li>
<li><a href="ex/aComplexTypedef.html">aComplexTypedef</a></li>
<li><a href="ex/ParameterizedTypedef.html">ParameterizedTypedef</a></li>
<li><a href="ex/processMessage.html">processMessage</a></li>

Expand Down
2 changes: 2 additions & 0 deletions testing/test_package_docs/ex/Apple-class.html
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ <h5>library ex</h5>
<li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
<li><a href="ex/ShapeType-class.html">ShapeType</a></li>
<li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
<li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
<li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
<li><a href="ex/WithGenericSub-class.html">WithGenericSub</a></li>

Expand Down Expand Up @@ -87,6 +88,7 @@ <h5>library ex</h5>
<li><a href="ex/Animal-class.html">Animal</a></li>

<li class="section-title"><a href="ex/ex-library.html#typedefs">Typedefs</a></li>
<li><a href="ex/aComplexTypedef.html">aComplexTypedef</a></li>
<li><a href="ex/ParameterizedTypedef.html">ParameterizedTypedef</a></li>
<li><a href="ex/processMessage.html">processMessage</a></li>

Expand Down
Loading