Skip to content

Commit

Permalink
lambda generation for positional args
Browse files Browse the repository at this point in the history
Fixes: #40202

(See issue for animation of UX.)

Change-Id: I029a7768b887fedef4873f3fa23171d21df8808f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/132380
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
  • Loading branch information
pq authored and commit-bot@chromium.org committed Jan 17, 2020
1 parent 53134fc commit 8fda644
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class ArgListContributor extends DartCompletionContributor {
) {
if (type is FunctionType) {
var indent = getRequestLineIndent(request);
var parametersString = _buildClosureParameters(type);
var parametersString = buildClosureParameters(type);

var blockBuffer = StringBuffer(parametersString);
blockBuffer.writeln(' {');
Expand Down Expand Up @@ -340,38 +340,6 @@ class ArgListContributor extends DartCompletionContributor {
return newExpr != null && flutter.isWidgetCreation(newExpr);
}

static String _buildClosureParameters(FunctionType type) {
var buffer = StringBuffer();
buffer.write('(');

var hasNamed = false;
var hasOptionalPositional = false;
var parameters = type.parameters;
for (var i = 0; i < parameters.length; ++i) {
var parameter = parameters[i];
if (i != 0) {
buffer.write(', ');
}
if (parameter.isNamed && !hasNamed) {
hasNamed = true;
buffer.write('{');
} else if (parameter.isOptionalPositional && !hasOptionalPositional) {
hasOptionalPositional = true;
buffer.write('[');
}
buffer.write(parameter.name);
}

if (hasNamed) {
buffer.write('}');
} else if (hasOptionalPositional) {
buffer.write(']');
}

buffer.write(')');
return buffer.toString();
}

/**
* If the given [comment] is not `null`, fill the [suggestion] documentation
* fields.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,43 @@ void addDefaultArgDetails(
sb.write(', ');
}
offset = sb.length;
String name = param.name;
sb.write(name);
ranges.addAll([offset, name.length]);

if (param.type is FunctionType) {
FunctionType type = param.type;

var rangeStart = offset;
var rangeLength;

// todo (pq): consider adding ranges for params
// pending: https://github.com/dart-lang/sdk/issues/40207
// (types in closure param completions make this UX awkward)
final parametersString = buildClosureParameters(type);
final blockBuffer = StringBuffer(parametersString);

blockBuffer.write(' ');

// todo (pq): consider refactoring to share common logic w/ ArgListContributor.buildClosureSuggestions
final returnType = type.returnType;
if (returnType.isVoid) {
blockBuffer.write('{');
rangeStart = sb.length + blockBuffer.length;
blockBuffer.write(' }');
rangeLength = 1;
} else {
final returnValue = returnType.isDartCoreBool ? 'false' : 'null';
blockBuffer.write('=> ');
rangeStart = sb.length + blockBuffer.length;
blockBuffer.write(returnValue);
rangeLength = returnValue.length;
}

sb.write(blockBuffer);
ranges.addAll([rangeStart, rangeLength]);
} else {
String name = param.name;
sb.write(name);
ranges.addAll([offset, name.length]);
}
}

for (ParameterElement param in namedParams) {
Expand All @@ -83,6 +117,39 @@ void addDefaultArgDetails(
suggestion.defaultArgumentListTextRanges = ranges.isNotEmpty ? ranges : null;
}

String buildClosureParameters(FunctionType type) {
var buffer = StringBuffer();
buffer.write('(');

var hasNamed = false;
var hasOptionalPositional = false;
var parameters = type.parameters;
for (var i = 0; i < parameters.length; ++i) {
var parameter = parameters[i];
if (i != 0) {
buffer.write(', ');
}
if (parameter.isNamed && !hasNamed) {
hasNamed = true;
buffer.write('{');
} else if (parameter.isOptionalPositional && !hasOptionalPositional) {
hasOptionalPositional = true;
buffer.write('[');
}
// todo (pq): consider abbreviating names
buffer.write(parameter.name);
}

if (hasNamed) {
buffer.write('}');
} else if (hasOptionalPositional) {
buffer.write(']');
}

buffer.write(')');
return buffer.toString();
}

/**
* Create a new protocol Element for inclusion in a completion suggestion.
*/
Expand Down Expand Up @@ -268,7 +335,7 @@ String nameForType(SimpleIdentifier identifier, TypeAnnotation declaredType) {
return type.toString();
}

// TODO(pq): fix to use getDefaultStringParameterValue()
/// TODO(pq): fix to use getDefaultStringParameterValue()
String _getDefaultValue(ParameterElement param) => 'null';

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2578,6 +2578,86 @@ g(F.^
assertNotSuggested('T2');
}

test_method_parameter_function_in_param_list() async {
addTestSource('''
class C {
void f(int x, void Function(int a, int b) closure, int y) {}
}
void main() {
new C().^
}
''');
await computeSuggestions();

final suggestion = assertSuggestMethod('f', 'C', 'void');
expect(suggestion.defaultArgumentListString, 'x, (a, b) { }, y');

// Select 'x', ' ', 'y'
expect(suggestion.defaultArgumentListTextRanges,
containsAllInOrder([0, 1, 11, 1, 15, 1]));
}

test_method_parameter_function_return_bool() async {
addTestSource('''
class C {
void f(bool Function(int a, int b) closure) {}
}
void main() {
new C().^
}
''');
await computeSuggestions();

final suggestion = assertSuggestMethod('f', 'C', 'void');
expect(suggestion.defaultArgumentListString, '(a, b) => false');

// Select 'false'
expect(
suggestion.defaultArgumentListTextRanges, containsAllInOrder([10, 5]));
}

test_method_parameter_function_return_object() async {
addTestSource('''
class C {
void f(Object Function(int a, int b) closure) {}
}
void main() {
new C().^
}
''');
await computeSuggestions();

final suggestion = assertSuggestMethod('f', 'C', 'void');
expect(suggestion.defaultArgumentListString, '(a, b) => null');

// Select 'null'
expect(
suggestion.defaultArgumentListTextRanges, containsAllInOrder([10, 4]));
}

test_method_parameter_function_return_void() async {
addTestSource('''
class C {
void f(void Function(int a, int b) closure) {}
}
void main() {
new C().^
}
''');
await computeSuggestions();

final suggestion = assertSuggestMethod('f', 'C', 'void');
expect(suggestion.defaultArgumentListString, '(a, b) { }');

// Select ' '
expect(
suggestion.defaultArgumentListTextRanges, containsAllInOrder([8, 1]));
}

test_method_parameters_mixed_required_and_named() async {
addTestSource('''
class C {
Expand Down

0 comments on commit 8fda644

Please sign in to comment.