-
Notifications
You must be signed in to change notification settings - Fork 125
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
Conversation
Last patch works around an appveyor problem with newer Dart Windows SDKs where .packages files no longer allow for Unix relative path semantics. |
@bwilkerson Adding you if you have time to check my assumptions about analyzer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm (I'm assuming that Brian will review the element changes)
@@ -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 |
There was a problem hiding this comment.
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)
?
There was a problem hiding this comment.
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.
} | ||
return false; | ||
} | ||
bool get isParameterizedType => (_type is ParameterizedType); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
if (typeArguments.every((t) => t.linkedName == 'dynamic')) { | ||
_linkedName = buf.toString(); | ||
return _linkedName; | ||
if (isParameterType) { |
There was a problem hiding this comment.
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)
?
There was a problem hiding this comment.
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.
// 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('('); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Look at that green travis! |
pubspec.yaml
Outdated
@@ -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 <0.13.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as test: ^0.12.20+24
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -3213,9 +3219,51 @@ class ModelFunction extends ModelFunctionTyped { | |||
} | |||
|
|||
@override | |||
String get name { | |||
if (element.enclosingElement is ParameterElement && super.name.isEmpty) | |||
return element.enclosingElement.name; |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
Merging this, but there's a bug in pub that prevents me from making appveyor and flutter bots work at the same time without hacks. Will file separately, in the meantime I've left the appveyor hack in. |
Fix #1505. Implement the TODO for subclassing ModelFunctionTyped and add support for anonymous typed functions (declared with 'Function()') , which are allowed in the new generic function type specification.