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>
This commit is contained in:
Konstantin Shcheglov 2018-04-30 16:49:00 +00:00 committed by commit-bot@chromium.org
parent b4a2259b5f
commit b7d8ae30e2
2 changed files with 184 additions and 45 deletions

View file

@ -37,6 +37,7 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
CorrectionUtils utils;
ClassElement classBuildContext;
ClassElement classKey;
ClassElement classStatefulWidget;
ClassElement classStatelessWidget;
ClassElement classWidget;
@ -96,8 +97,8 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
_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 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
}
classBuildContext = await getClass('BuildContext');
classKey = await getClass('Key');
classStatelessWidget = await getClass('StatelessWidget');
classStatefulWidget = await getClass('StatefulWidget');
classWidget = await getClass('Widget');
@ -229,6 +231,10 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
// 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 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
}
}
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 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
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 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
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 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
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 @@ class ExtractWidgetRefactoringImpl extends RefactoringImpl
builder.write(', ');
}
builder.write(parameter.name);
builder.write(': ');
builder.write(parameter.name);
}
builder.write(')');
@ -381,7 +422,7 @@ class _Parameter {
/// 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> {

View file

@ -130,6 +130,8 @@ class MyWidget extends StatelessWidget {
}
class Test extends StatelessWidget {
Test({Key key}) : super(key: key);
@override
Widget build(BuildContext context) {
return new Column(
@ -178,6 +180,8 @@ Widget main() {
}
class Test extends StatelessWidget {
Test({Key key}) : super(key: key);
@override
Widget build(BuildContext context) {
return new Text('AAA');
@ -211,6 +215,8 @@ class MyWidget extends StatelessWidget {
}
class Test extends StatelessWidget {
Test({Key key}) : super(key: key);
@override
Widget build(BuildContext context) {
return new Container();
@ -248,6 +254,8 @@ Widget main() {
}
class Test extends StatelessWidget {
Test({Key key}) : super(key: key);
@override
Widget build(BuildContext context) {
return new Text('AAA');
@ -345,14 +353,14 @@ class MyWidget extends StatelessWidget {
@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 MyWidget extends StatelessWidget {
}
class Test extends StatelessWidget {
Test({Key key}) : super(key: key);
@override
Widget build(BuildContext context) {
var a = new Text('AAA');
@ -453,8 +463,8 @@ class MyWidget extends StatelessWidget {
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 @@ class Test extends StatelessWidget {
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 @@ class MyWidget extends StatelessWidget {
@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 @@ class MyWidget extends StatelessWidget {
@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 MyWidget extends StatelessWidget {
}
class Test extends StatelessWidget {
Test({Key key}) : super(key: key);
@override
Widget build(BuildContext context) {
return new Text(field);
@ -695,14 +774,14 @@ class MyWidget extends StatelessWidget {
@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 @@ class Test extends StatelessWidget {
''');
}
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 @@ class MyWidget extends StatelessWidget {
@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 @@ class MyWidget extends StatelessWidget {
@override
Widget build(BuildContext context) {
String local;
return new Test(field, local);
return new Test(field: field, local: local);
}
}
@ -819,7 +917,7 @@ class Test extends StatelessWidget {
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) {