Change 'Extract Widget' to generate named constructor parameters, with 'key'.
R=devoncarew@google.com, paulberry@google.com
Bug: https://github.com/flutter/flutter-intellij/issues/2166
Change-Id: Ie698dc27a1328b7e18e92f46583f84c22c84de57
Reviewed-on: https://dart-review.googlesource.com/53060
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/services/refactoring/extract_widget.dart b/pkg/analysis_server/lib/src/services/refactoring/extract_widget.dart
index 76d0115..9d52193 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/extract_widget.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/extract_widget.dart
@@ -37,6 +37,7 @@
CorrectionUtils utils;
ClassElement classBuildContext;
+ ClassElement classKey;
ClassElement classStatefulWidget;
ClassElement classStatelessWidget;
ClassElement classWidget;
@@ -96,8 +97,8 @@
_enclosingUnitMember = (_expression ?? _method).getAncestor(
(n) => n is CompilationUnitMember && n.parent is CompilationUnit);
- result.addStatus(await _initializeParameters());
result.addStatus(await _initializeClasses());
+ result.addStatus(await _initializeParameters());
return result;
}
@@ -200,6 +201,7 @@
}
classBuildContext = await getClass('BuildContext');
+ classKey = await getClass('Key');
classStatelessWidget = await getClass('StatelessWidget');
classStatefulWidget = await getClass('StatefulWidget');
classWidget = await getClass('Widget');
@@ -229,6 +231,10 @@
// We added fields, now add the method parameters.
if (_method != null) {
for (var parameter in _method.parameters.parameters) {
+ if (parameter is DefaultFormalParameter) {
+ DefaultFormalParameter defaultFormalParameter = parameter;
+ parameter = defaultFormalParameter.parameter;
+ }
if (parameter is NormalFormalParameter) {
_parameters.add(new _Parameter(
parameter.identifier.name, parameter.element.type,
@@ -237,6 +243,18 @@
}
}
+ RefactoringStatus status = collector.status;
+
+ // If there is an existing parameter "key" warn the user.
+ // We could rename it, but that would require renaming references to it.
+ // It is probably pretty rare, and the user can always rename before.
+ for (var parameter in _parameters) {
+ if (parameter.name == 'key') {
+ status.addError(
+ "The parameter 'key' will conflict with the widget 'key'.");
+ }
+ }
+
return collector.status;
}
@@ -254,29 +272,31 @@
var collector = new _MethodInvocationsCollector(_method.element);
_enclosingClassNode.accept(collector);
for (var invocation in collector.invocations) {
- builder.addReplacement(
- range.startEnd(invocation, invocation.argumentList.leftParenthesis),
- (builder) {
- builder.write('new $name(');
+ List<Expression> arguments = invocation.argumentList.arguments;
+ builder.addReplacement(range.node(invocation), (builder) {
+ builder.write('new $name(');
- // Insert field references.
- for (var parameter in _parameters) {
- if (parameter.isMethodParameter) {
- break;
- }
- if (parameter != _parameters.first) {
- builder.write(', ');
- }
- builder.write(parameter.name);
- }
-
- // Separate references to fields and method arguments.
- if (_parameters.isNotEmpty &&
- invocation.argumentList.arguments.isNotEmpty) {
+ // Insert field references (as named arguments).
+ // Ensure that invocation arguments are named.
+ int argumentIndex = 0;
+ for (var parameter in _parameters) {
+ if (parameter != _parameters.first) {
builder.write(', ');
}
- },
- );
+ builder.write(parameter.name);
+ builder.write(': ');
+ if (parameter.isMethodParameter) {
+ Expression argument = arguments[argumentIndex++];
+ if (argument is NamedExpression) {
+ argument = (argument as NamedExpression).expression;
+ }
+ builder.write(utils.getNodeText(argument));
+ } else {
+ builder.write(parameter.name);
+ }
+ }
+ builder.write(')');
+ });
}
}
@@ -289,8 +309,8 @@
name,
superclass: classStatelessWidget.type,
membersWriter: () {
+ // Add the fields for the parameters.
if (_parameters.isNotEmpty) {
- // Add the fields for the parameters.
for (var parameter in _parameters) {
builder.write(' ');
builder.writeFieldDeclaration(parameter.name,
@@ -298,15 +318,34 @@
builder.writeln();
}
builder.writeln();
-
- // Add the constructor.
- builder.write(' ');
- builder.writeConstructorDeclaration(name,
- fieldNames: _parameters.map((e) => e.name).toList());
- builder.writeln();
- builder.writeln();
}
+ // Add the constructor.
+ builder.write(' ');
+ builder.writeConstructorDeclaration(
+ name,
+ parameterWriter: () {
+ builder.write('{');
+
+ // Add the required `key` parameter.
+ builder.writeParameter('key', type: classKey.type);
+
+ // Add parameters for fields, local, and method parameters.
+ for (int i = 0; i < _parameters.length; i++) {
+ builder.write(', ');
+ builder.write('this.');
+ builder.write(_parameters[i].name);
+ }
+
+ builder.write('}');
+ },
+ initializerWriter: () {
+ builder.write('super(key: key)');
+ },
+ );
+ builder.writeln();
+ builder.writeln();
+
// Widget build(BuildContext context) { ... }
builder.writeln(' @override');
builder.write(' ');
@@ -352,6 +391,8 @@
builder.write(', ');
}
builder.write(parameter.name);
+ builder.write(': ');
+ builder.write(parameter.name);
}
builder.write(')');
@@ -381,7 +422,7 @@
/// Whether the parameter is a parameter of the method being extracted.
final bool isMethodParameter;
- _Parameter(this.name, this.type, {this.isMethodParameter = false});
+ _Parameter(this.name, this.type, {this.isMethodParameter: false});
}
class _ParametersCollector extends RecursiveAstVisitor<void> {
diff --git a/pkg/analysis_server/test/services/refactoring/extract_widget_test.dart b/pkg/analysis_server/test/services/refactoring/extract_widget_test.dart
index 3355620..4e8323b 100644
--- a/pkg/analysis_server/test/services/refactoring/extract_widget_test.dart
+++ b/pkg/analysis_server/test/services/refactoring/extract_widget_test.dart
@@ -130,6 +130,8 @@
}
class Test extends StatelessWidget {
+ Test({Key key}) : super(key: key);
+
@override
Widget build(BuildContext context) {
return new Column(
@@ -178,6 +180,8 @@
}
class Test extends StatelessWidget {
+ Test({Key key}) : super(key: key);
+
@override
Widget build(BuildContext context) {
return new Text('AAA');
@@ -211,6 +215,8 @@
}
class Test extends StatelessWidget {
+ Test({Key key}) : super(key: key);
+
@override
Widget build(BuildContext context) {
return new Container();
@@ -248,6 +254,8 @@
}
class Test extends StatelessWidget {
+ Test({Key key}) : super(key: key);
+
@override
Widget build(BuildContext context) {
return new Text('AAA');
@@ -345,14 +353,14 @@
@override
Widget build(BuildContext context) {
- return new Test(c);
+ return new Test(c: c);
}
}
class Test extends StatelessWidget {
final C c;
- Test(this.c);
+ Test({Key key, this.c}) : super(key: key);
@override
Widget build(BuildContext context) {
@@ -400,6 +408,8 @@
}
class Test extends StatelessWidget {
+ Test({Key key}) : super(key: key);
+
@override
Widget build(BuildContext context) {
var a = new Text('AAA');
@@ -453,8 +463,8 @@
int bar = 1;
return new Row(
children: <Widget>[
- new Test(foo, 'aaa', bar),
- new Test(foo, 'bbb', 2),
+ new Test(foo: foo, p1: 'aaa', p2: bar),
+ new Test(foo: foo, p1: 'bbb', p2: 2),
],
);
}
@@ -465,7 +475,74 @@
final String p1;
final int p2;
- Test(this.foo, this.p1, this.p2);
+ Test({Key key, this.foo, this.p1, this.p2}) : super(key: key);
+
+ @override
+ Widget build(BuildContext context) {
+ var a = new Text('$foo $p1');
+ var b = new Text('$p2');
+ return new Column(
+ children: <Widget>[a, b],
+ );
+ }
+}
+''');
+ }
+
+ test_method_parameters_named() async {
+ addFlutterPackage();
+ await indexTestUnit(r'''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+ String foo;
+
+ @override
+ Widget build(BuildContext context) {
+ int bar = 1;
+ return new Row(
+ children: <Widget>[
+ createColumn(p1: 'aaa', p2: bar),
+ createColumn(p1: 'bbb', p2: 2),
+ ],
+ );
+ }
+
+ Widget createColumn({String p1, int p2}) {
+ var a = new Text('$foo $p1');
+ var b = new Text('$p2');
+ return new Column(
+ children: <Widget>[a, b],
+ );
+ }
+}
+''');
+ _createRefactoringForStringOffset('createColumn({String');
+
+ await _assertSuccessfulRefactoring(r'''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+ String foo;
+
+ @override
+ Widget build(BuildContext context) {
+ int bar = 1;
+ return new Row(
+ children: <Widget>[
+ new Test(foo: foo, p1: 'aaa', p2: bar),
+ new Test(foo: foo, p1: 'bbb', p2: 2),
+ ],
+ );
+ }
+}
+
+class Test extends StatelessWidget {
+ final String foo;
+ final String p1;
+ final int p2;
+
+ Test({Key key, this.foo, this.p1, this.p2}) : super(key: key);
@override
Widget build(BuildContext context) {
@@ -503,14 +580,14 @@
@override
Widget build(BuildContext context) {
- return new Test(field);
+ return new Test(field: field);
}
}
class Test extends StatelessWidget {
final String field;
- Test(this.field);
+ Test({Key key, this.field}) : super(key: key);
@override
Widget build(BuildContext context) {
@@ -552,14 +629,14 @@
@override
Widget build(BuildContext context) {
- return new Test(c);
+ return new Test(c: c);
}
}
class Test extends StatelessWidget {
final C c;
- Test(this.c);
+ Test({Key key, this.c}) : super(key: key);
@override
Widget build(BuildContext context) {
@@ -598,6 +675,8 @@
}
class Test extends StatelessWidget {
+ Test({Key key}) : super(key: key);
+
@override
Widget build(BuildContext context) {
return new Text(field);
@@ -695,14 +774,14 @@
@override
Widget build(BuildContext context) {
- return new Test(c);
+ return new Test(c: c);
}
}
class Test extends StatelessWidget {
final C c;
- Test(this.c);
+ Test({Key key, this.c}) : super(key: key);
@override
Widget build(BuildContext context) {
@@ -717,6 +796,25 @@
''');
}
+ test_parameters_key() async {
+ addFlutterPackage();
+ await indexTestUnit(r'''
+import 'package:flutter/material.dart';
+
+class MyWidget extends StatelessWidget {
+ @override
+ Widget build(BuildContext context) {
+ String key;
+ return new Text('$key $key');
+ }
+}
+''');
+ _createRefactoringForStringOffset('new Text');
+
+ RefactoringStatus status = await refactoring.checkAllConditions();
+ assertRefactoringStatus(status, RefactoringProblemSeverity.ERROR);
+ }
+
test_parameters_local_read_enclosingScope() async {
addFlutterPackage();
await indexTestUnit(r'''
@@ -739,14 +837,14 @@
@override
Widget build(BuildContext context) {
String local;
- return new Test(local);
+ return new Test(local: local);
}
}
class Test extends StatelessWidget {
final String local;
- Test(this.local);
+ Test({Key key, this.local}) : super(key: key);
@override
Widget build(BuildContext context) {
@@ -811,7 +909,7 @@
@override
Widget build(BuildContext context) {
String local;
- return new Test(field, local);
+ return new Test(field: field, local: local);
}
}
@@ -819,7 +917,7 @@
final String field;
final String local;
- Test(this.field, this.local);
+ Test({Key key, this.field, this.local}) : super(key: key);
@override
Widget build(BuildContext context) {
