Improve the placement of added named parameters
Change-Id: I8ed588c14402a3d85d134dc75c324d9e3d96f532
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/181820
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/modify_parameters.dart b/pkg/analysis_server/lib/src/services/correction/fix/data_driven/modify_parameters.dart
index 32d0b8e..3c49589 100644
--- a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/modify_parameters.dart
+++ b/pkg/analysis_server/lib/src/services/correction/fix/data_driven/modify_parameters.dart
@@ -60,7 +60,7 @@
var arguments = argumentList.arguments;
var argumentCount = arguments.length;
var templateContext = TemplateContext(argumentList.parent, fix.utils);
- var newNamed = <AddParameter>[];
+
var indexToNewArgumentMap = <int, AddParameter>{};
var argumentsToInsert = <int>[];
var argumentsToDelete = <int>[];
@@ -69,16 +69,14 @@
if (modification is AddParameter) {
var index = modification.index;
indexToNewArgumentMap[index] = modification;
- if (modification.isPositional) {
+ if (modification.isPositional || modification.isRequired) {
argumentsToInsert.add(index);
- } else if (modification.isRequired) {
- newNamed.add(modification);
} else {
var requiredIfCondition =
modification.argumentValue?.requiredIfCondition;
if (requiredIfCondition != null &&
requiredIfCondition.evaluateIn(templateContext)) {
- newNamed.add(modification);
+ argumentsToInsert.add(index);
}
}
} else if (modification is RemoveParameter) {
@@ -94,7 +92,6 @@
}
}
argumentsToInsert.sort();
- newNamed.sort((first, second) => first.name.compareTo(second.name));
/// Write to the [builder] the argument associated with a single
/// [parameter].
@@ -185,39 +182,21 @@
var insertionRange = insertionRanges[nextInsertionRange];
var lower = insertionRange.lower;
var upper = insertionRange.upper;
- while (upper >= lower && !indexToNewArgumentMap[upper].isRequired) {
+ var parameter = indexToNewArgumentMap[upper];
+ while (upper >= lower &&
+ (parameter.isPositional && !parameter.isRequired)) {
upper--;
}
if (upper >= lower) {
builder.addInsertion(offset, (builder) {
- writeInsertionRange(builder, _IndexRange(lower, upper), true);
+ writeInsertionRange(builder, _IndexRange(lower, upper),
+ nextRemaining > 0 || insertionCount > 0);
});
}
nextInsertionRange++;
}
}
//
- // Insert arguments for required named parameters.
- //
- if (newNamed.isNotEmpty) {
- int offset;
- var needsInitialComma = false;
- if (remainingArguments.isEmpty && argumentsToInsert.isEmpty) {
- offset = argumentList.rightParenthesis.offset;
- } else {
- offset = arguments[arguments.length - 1].end;
- needsInitialComma = true;
- }
- builder.addInsertion(offset, (builder) {
- for (var i = 0; i < newNamed.length; i++) {
- if (i > 0 || needsInitialComma) {
- builder.write(', ');
- }
- writeArgument(builder, newNamed[i]);
- }
- });
- }
- //
// The remaining deletion ranges are now ready to be removed.
//
for (var subRange in deletionRanges) {
diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/flutter_use_case_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/flutter_use_case_test.dart
index f6359cd..ee6c688 100644
--- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/flutter_use_case_test.dart
+++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/flutter_use_case_test.dart
@@ -2526,6 +2526,152 @@
''');
}
+ Future<void> test_widgets_Stack_overflow_clip() async {
+ setPackageContent('''
+class Stack {
+ const Stack({
+ @deprecated Overflow overflow: Overflow.clip,
+ Clip clipBehavior: Clip.hardEdge,
+ List<Widget> children: const <Widget>[]});
+}
+class Overflow {
+ static const Overflow clip = Overflow();
+ static const Overflow visible = Overflow();
+ const Overflow();
+}
+class Clip {
+ static const Clip hardEdge = Clip();
+ static const Clip none = Clip();
+ const Clip();
+}
+class Widget {}
+''');
+ addPackageDataFile('''
+version: 1
+transforms:
+ - title: "Migrate to 'clipBehavior'"
+ date: 2020-09-22
+ element:
+ uris: ['$importUri']
+ constructor: ''
+ inClass: 'Stack'
+ oneOf:
+ - if: "overflow == 'Overflow.clip'"
+ changes:
+ - kind: 'addParameter'
+ index: 0
+ name: 'clipBehavior'
+ style: optional_named
+ argumentValue:
+ expression: 'Clip.hardEdge'
+ requiredIf: "overflow == 'Overflow.clip'"
+ - kind: 'removeParameter'
+ name: 'overflow'
+ - if: "overflow == 'Overflow.visible'"
+ changes:
+ - kind: 'addParameter'
+ index: 0
+ name: 'clipBehavior'
+ style: optional_named
+ argumentValue:
+ expression: 'Clip.none'
+ requiredIf: "overflow == 'Overflow.visible'"
+ - kind: 'removeParameter'
+ name: 'overflow'
+ variables:
+ overflow:
+ kind: 'fragment'
+ value: 'arguments[overflow]'
+''');
+ await resolveTestCode('''
+import '$importUri';
+
+void f() {
+ const Stack(overflow: Overflow.clip, children: []);
+}
+''');
+ await assertHasFix('''
+import '$importUri';
+
+void f() {
+ const Stack(clipBehavior: Clip.hardEdge, children: []);
+}
+''');
+ }
+
+ Future<void> test_widgets_Stack_overflow_visible() async {
+ setPackageContent('''
+class Stack {
+ const Stack({
+ @deprecated Overflow overflow: Overflow.clip,
+ Clip clipBehavior: Clip.hardEdge,
+ List<Widget> children: const <Widget>[]});
+}
+class Overflow {
+ static const Overflow clip = Overflow();
+ static const Overflow visible = Overflow();
+ const Overflow();
+}
+class Clip {
+ static const Clip hardEdge = Clip();
+ static const Clip none = Clip();
+ const Clip();
+}
+class Widget {}
+''');
+ addPackageDataFile('''
+version: 1
+transforms:
+ - title: "Migrate to 'clipBehavior'"
+ date: 2020-09-22
+ element:
+ uris: ['$importUri']
+ constructor: ''
+ inClass: 'Stack'
+ oneOf:
+ - if: "overflow == 'Overflow.clip'"
+ changes:
+ - kind: 'addParameter'
+ index: 0
+ name: 'clipBehavior'
+ style: optional_named
+ argumentValue:
+ expression: 'Clip.hardEdge'
+ requiredIf: "overflow == 'Overflow.clip'"
+ - kind: 'removeParameter'
+ name: 'overflow'
+ - if: "overflow == 'Overflow.visible'"
+ changes:
+ - kind: 'addParameter'
+ index: 0
+ name: 'clipBehavior'
+ style: optional_named
+ argumentValue:
+ expression: 'Clip.none'
+ requiredIf: "overflow == 'Overflow.visible'"
+ - kind: 'removeParameter'
+ name: 'overflow'
+ variables:
+ overflow:
+ kind: 'fragment'
+ value: 'arguments[overflow]'
+''');
+ await resolveTestCode('''
+import '$importUri';
+
+void f() {
+ const Stack(overflow: Overflow.visible, children: []);
+}
+''');
+ await assertHasFix('''
+import '$importUri';
+
+void f() {
+ const Stack(clipBehavior: Clip.none, children: []);
+}
+''');
+ }
+
Future<void>
test_widgets_StatefulElement_inheritFromElement_deprecated() async {
setPackageContent('''
diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/modify_parameters_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/modify_parameters_test.dart
index d67d30e..77e7e1f 100644
--- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/modify_parameters_test.dart
+++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/modify_parameters_test.dart
@@ -101,7 +101,7 @@
import '$importUri';
void f(C c) {
- c.m2(b: 1, a: 0);
+ c.m2(a: 0, b: 1);
}
''');
}
