Enable bulk application of more data-driven fixes

Change-Id: Ieffd7093bb207fe3754cda8d03dde6c781971375
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/165144
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/bulk_fix_processor.dart b/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart
index 003eeb4..bbbf044 100644
--- a/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart
+++ b/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart
@@ -249,17 +249,21 @@
     CompileTimeErrorCode.EXTENDS_NON_CLASS: [
       DataDriven.newInstance,
     ],
-    // TODO(brianwilkerson) Uncomment the entries below as we add tests for
-    //  them.
+    // TODO(brianwilkerson) The following fix fails if an invocation of the
+    //  function is the argument that needs to be removed.
     // CompileTimeErrorCode.EXTRA_POSITIONAL_ARGUMENTS: [
     //   DataDriven.newInstance,
     // ],
+    // TODO(brianwilkerson) The following fix fails if an invocation of the
+    //  function is the argument that needs to be updated.
     // CompileTimeErrorCode.EXTRA_POSITIONAL_ARGUMENTS_COULD_BE_NAMED: [
     //   DataDriven.newInstance,
     // ],
     CompileTimeErrorCode.IMPLEMENTS_NON_CLASS: [
       DataDriven.newInstance,
     ],
+    // TODO(brianwilkerson) Uncomment the entries below as we add tests for
+    //  them.
     // CompileTimeErrorCode.INVALID_OVERRIDE: [
     //   DataDriven.newInstance,
     // ],
@@ -269,27 +273,27 @@
     // CompileTimeErrorCode.NEW_WITH_UNDEFINED_CONSTRUCTOR_DEFAULT: [
     //   DataDriven.newInstance,
     // ],
-    // CompileTimeErrorCode.NOT_ENOUGH_POSITIONAL_ARGUMENTS: [
-    //   DataDriven.newInstance,
-    // ],
-    // CompileTimeErrorCode.UNDEFINED_CLASS: [
-    //   DataDriven.newInstance,
-    // ],
-    // CompileTimeErrorCode.UNDEFINED_FUNCTION: [
-    //   DataDriven.newInstance,
-    // ],
-    // CompileTimeErrorCode.UNDEFINED_GETTER: [
-    //   DataDriven.newInstance,
-    // ],
-    // CompileTimeErrorCode.UNDEFINED_IDENTIFIER: [
-    //   DataDriven.newInstance,
-    // ],
-    // CompileTimeErrorCode.UNDEFINED_METHOD: [
-    //   DataDriven.newInstance,
-    // ],
-    // CompileTimeErrorCode.UNDEFINED_SETTER: [
-    //   DataDriven.newInstance,
-    // ],
+    CompileTimeErrorCode.NOT_ENOUGH_POSITIONAL_ARGUMENTS: [
+      DataDriven.newInstance,
+    ],
+    CompileTimeErrorCode.UNDEFINED_CLASS: [
+      DataDriven.newInstance,
+    ],
+    CompileTimeErrorCode.UNDEFINED_FUNCTION: [
+      DataDriven.newInstance,
+    ],
+    CompileTimeErrorCode.UNDEFINED_GETTER: [
+      DataDriven.newInstance,
+    ],
+    CompileTimeErrorCode.UNDEFINED_IDENTIFIER: [
+      DataDriven.newInstance,
+    ],
+    CompileTimeErrorCode.UNDEFINED_METHOD: [
+      DataDriven.newInstance,
+    ],
+    CompileTimeErrorCode.UNDEFINED_SETTER: [
+      DataDriven.newInstance,
+    ],
     // CompileTimeErrorCode.WRONG_NUMBER_OF_TYPE_ARGUMENTS: [
     //   DataDriven.newInstance,
     // ],
@@ -308,9 +312,9 @@
     // HintCode.DEPRECATED_MEMBER_USE_WITH_MESSAGE: [
     //   DataDriven.newInstance,
     // ],
-    // HintCode.OVERRIDE_ON_NON_OVERRIDING_METHOD: [
-    //   DataDriven.newInstance,
-    // ],
+    HintCode.OVERRIDE_ON_NON_OVERRIDING_METHOD: [
+      DataDriven.newInstance,
+    ],
   };
 
   /// A map from an error code to a generator used to create the correction
diff --git a/pkg/analysis_server/test/src/services/correction/fix/bulk/data_driven_test.dart b/pkg/analysis_server/test/src/services/correction/fix/bulk/data_driven_test.dart
index 6f65772..1d5cf23 100644
--- a/pkg/analysis_server/test/src/services/correction/fix/bulk/data_driven_test.dart
+++ b/pkg/analysis_server/test/src/services/correction/fix/bulk/data_driven_test.dart
@@ -10,8 +10,18 @@
 void main() {
   defineReflectiveSuite(() {
     defineReflectiveTests(ExtendsNonClassTest);
+    defineReflectiveTests(ExtraPositionalArgumentsCouldBeNamedTest);
+    defineReflectiveTests(ExtraPositionalArgumentsTest);
     defineReflectiveTests(ImplementsNonClassTest);
     defineReflectiveTests(MixinOfNonClassTest);
+    defineReflectiveTests(NotEnoughPositionalArgumentsTest);
+    defineReflectiveTests(OverrideOnNonOverridingMethodTest);
+    defineReflectiveTests(UndefinedClassTest);
+    defineReflectiveTests(UndefinedFunctionTest);
+    defineReflectiveTests(UndefinedGetterTest);
+    defineReflectiveTests(UndefinedIdentifierTest);
+    defineReflectiveTests(UndefinedMethodTest);
+    defineReflectiveTests(UndefinedSetterTest);
   });
 }
 
@@ -47,6 +57,90 @@
 }
 
 @reflectiveTest
+class ExtraPositionalArgumentsCouldBeNamedTest extends _DataDrivenTest {
+  @failingTest
+  Future<void> test_replaceParameter() async {
+    // This fails because we grab the argument from the outer invocation before
+    // we modify it, but then we add the edits to modify it, which causes the
+    // wrong code to be put in the wrong places.
+    setPackageContent('''
+int f(int x, {int y = 0}) => x;
+''');
+    addPackageDataFile('''
+version: 1
+transforms:
+- title: 'Replace parameter'
+  date: 2020-09-01
+  element:
+    uris: ['$importUri']
+    function: 'f'
+  changes:
+    - kind: 'addParameter'
+      index: 1
+      name: 'y'
+      style: required_named
+      argumentValue:
+        expression: '{% old %}'
+        variables:
+          old:
+            kind: 'argument'
+            index: 1
+    - kind: 'removeParameter'
+      index: 1
+''');
+    await resolveTestUnit('''
+import '$importUri';
+void g() {
+  f(0, f(1, 2));
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+void g() {
+  f(0, y: f(1, y: 2));
+}
+''');
+  }
+}
+
+@reflectiveTest
+class ExtraPositionalArgumentsTest extends _DataDrivenTest {
+  @failingTest
+  Future<void> test_removeParameter() async {
+    // This fails because we delete the extra argument from the inner invocation
+    // (`, 2`) before deleting the argument from the outer invocation, which
+    // results in three characters too many being deleted.
+    setPackageContent('''
+int f(int x) => x;
+''');
+    addPackageDataFile('''
+version: 1
+transforms:
+- title: 'Remove parameter'
+  date: 2020-09-01
+  element:
+    uris: ['$importUri']
+    function: 'f'
+  changes:
+    - kind: 'removeParameter'
+      index: 1
+''');
+    await resolveTestUnit('''
+import '$importUri';
+void g() {
+  f(0, f(1, 2));
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+void g() {
+  f(0);
+}
+''');
+  }
+}
+
+@reflectiveTest
 class ImplementsNonClassTest extends _DataDrivenTest {
   Future<void> test_rename() async {
     setPackageContent('''
@@ -108,6 +202,292 @@
   }
 }
 
+@reflectiveTest
+class NotEnoughPositionalArgumentsTest extends _DataDrivenTest {
+  Future<void> test_removeParameter() async {
+    setPackageContent('''
+int f(int x, int y) => x + y;
+''');
+    addPackageDataFile('''
+version: 1
+transforms:
+- title: 'Add parameter'
+  date: 2020-09-01
+  element:
+    uris: ['$importUri']
+    function: 'f'
+  changes:
+    - kind: 'addParameter'
+      index: 1
+      name: 'y'
+      style: required_positional
+      argumentValue:
+        expression: '0'
+''');
+    await resolveTestUnit('''
+import '$importUri';
+void g() {
+  f(f(0));
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+void g() {
+  f(f(0, 0), 0);
+}
+''');
+  }
+}
+
+@reflectiveTest
+class OverrideOnNonOverridingMethodTest extends _DataDrivenTest {
+  Future<void> test_rename() async {
+    setPackageContent('''
+class C {
+  int new(int x) => x + 1;
+}
+''');
+    addPackageDataFile('''
+version: 1
+transforms:
+- title: 'Rename to new'
+  date: 2020-09-01
+  element:
+    uris: ['$importUri']
+    method: 'old'
+    inClass: 'C'
+  changes:
+    - kind: 'rename'
+      newName: 'new'
+''');
+    await resolveTestUnit('''
+import '$importUri';
+class D extends C {
+  @override
+  int old(int x) => x + 2;
+}
+class E extends C {
+  @override
+  int old(int x) => x + 3;
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+class D extends C {
+  @override
+  int new(int x) => x + 2;
+}
+class E extends C {
+  @override
+  int new(int x) => x + 3;
+}
+''');
+  }
+}
+
+@reflectiveTest
+class UndefinedClassTest extends _DataDrivenTest {
+  Future<void> test_rename() async {
+    setPackageContent('''
+class New {}
+''');
+    addPackageDataFile('''
+version: 1
+transforms:
+- title: 'Rename to New'
+  date: 2020-09-01
+  element:
+    uris: ['$importUri']
+    class: 'Old'
+  changes:
+    - kind: 'rename'
+      newName: 'New'
+''');
+    await resolveTestUnit('''
+import '$importUri';
+void f(Old a, Old b) {}
+''');
+    await assertHasFix('''
+import '$importUri';
+void f(New a, New b) {}
+''');
+  }
+}
+
+@reflectiveTest
+class UndefinedFunctionTest extends _DataDrivenTest {
+  Future<void> test_rename() async {
+    setPackageContent('''
+int new(int x) => x + 1;
+''');
+    addPackageDataFile('''
+version: 1
+transforms:
+- title: 'Rename to new'
+  date: 2020-09-01
+  element:
+    uris: ['$importUri']
+    function: 'old'
+  changes:
+    - kind: 'rename'
+      newName: 'new'
+''');
+    await resolveTestUnit('''
+import '$importUri';
+void f() {
+  old(old(0));
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+void f() {
+  new(new(0));
+}
+''');
+  }
+}
+
+@reflectiveTest
+class UndefinedGetterTest extends _DataDrivenTest {
+  Future<void> test_rename() async {
+    setPackageContent('''
+class C {
+  int get new => 0;
+}
+''');
+    addPackageDataFile('''
+version: 1
+transforms:
+- title: 'Rename to new'
+  date: 2020-09-01
+  element:
+    uris: ['$importUri']
+    getter: 'old'
+    inClass: 'C'
+  changes:
+    - kind: 'rename'
+      newName: 'new'
+''');
+    await resolveTestUnit('''
+import '$importUri';
+void f(C a, C b) {
+  a.old + b.old;
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+void f(C a, C b) {
+  a.new + b.new;
+}
+''');
+  }
+}
+
+@reflectiveTest
+class UndefinedIdentifierTest extends _DataDrivenTest {
+  Future<void> test_rename_topLevelVariable() async {
+    setPackageContent('''
+int new = 0;
+''');
+    addPackageDataFile('''
+version: 1
+transforms:
+- title: 'Rename to new'
+  date: 2020-09-01
+  element:
+    uris: ['$importUri']
+    function: 'old'
+  changes:
+    - kind: 'rename'
+      newName: 'new'
+''');
+    await resolveTestUnit('''
+import '$importUri';
+void f() {
+  old + old;
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+void f() {
+  new + new;
+}
+''');
+  }
+}
+
+@reflectiveTest
+class UndefinedMethodTest extends _DataDrivenTest {
+  Future<void> test_rename() async {
+    setPackageContent('''
+class C {
+  int new(int x) => x + 1;
+}
+''');
+    addPackageDataFile('''
+version: 1
+transforms:
+- title: 'Rename to new'
+  date: 2020-09-01
+  element:
+    uris: ['$importUri']
+    method: 'old'
+    inClass: 'C'
+  changes:
+    - kind: 'rename'
+      newName: 'new'
+''');
+    await resolveTestUnit('''
+import '$importUri';
+void f(C a, C b) {
+  a.old(b.old(0));
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+void f(C a, C b) {
+  a.new(b.new(0));
+}
+''');
+  }
+}
+
+@reflectiveTest
+class UndefinedSetterTest extends _DataDrivenTest {
+  Future<void> test_rename() async {
+    setPackageContent('''
+class C {
+  set new(int x) {}
+}
+''');
+    addPackageDataFile('''
+version: 1
+transforms:
+- title: 'Rename to new'
+  date: 2020-09-01
+  element:
+    uris: ['$importUri']
+    setter: 'old'
+    inClass: 'C'
+  changes:
+    - kind: 'rename'
+      newName: 'new'
+''');
+    await resolveTestUnit('''
+import '$importUri';
+void f(C a, C b) {
+  a.old = b.old = 1;
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+void f(C a, C b) {
+  a.new = b.new = 1;
+}
+''');
+  }
+}
+
 class _DataDrivenTest extends BulkFixProcessorTest {
   /// Return the URI used to import the library created by [setPackageContent].
   String get importUri => 'package:p/lib.dart';