Support renaming constructors from named to unnamed or vice versa

Change-Id: Id447d6b6d6bb75d88a68b1c9f3ca865290e5cf70
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/162600
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>

pkg/analysis_server/lib/src/services/correction/dart/data_driven.dart

@@ -4,6 +4,7 @@
 
 import 'package:analysis_server/src/services/correction/dart/abstract_producer.dart';
 import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analysis_server/src/services/correction/fix/data_driven/element_descriptor.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/transform.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/transform_set.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/transform_set_manager.dart';
@@ -83,6 +84,9 @@ class DataDrivenFix extends CorrectionProducer {
 
   DataDrivenFix(this._transform);
 
+  /// Return a description of the element that was changed.
+  ElementDescriptor get element => _transform.element;
+
   @override
   List<Object> get fixArguments => [_transform.title];
 

pkg/analysis_server/lib/src/services/correction/fix/data_driven/element_descriptor.dart

@@ -9,18 +9,30 @@ class ElementDescriptor {
   /// The URIs of the library in which the element is defined.
   final List<String> libraryUris;
 
+  /// The kind of element that was changed.
+  final String kind;
+
   /// The components that uniquely identify the element within its library.
   final List<String> components;
 
   /// Initialize a newly created element descriptor to describe an element
   /// accessible via any of the [libraryUris] where the path to the element
-  /// within the library is given by the list of [components].
-  ElementDescriptor({@required this.libraryUris, @required this.components});
+  /// within the library is given by the list of [components]. The [kind] of the
+  /// element is represented by the key used in the data file.
+  ElementDescriptor(
+      {@required this.libraryUris,
+      @required this.kind,
+      @required this.components});
 
   /// Return `true` if this descriptor matches an element with the given [name]
   /// in a library that imports the [importedUris].
   bool matches(String name, List<String> importedUris) {
-    if (components.last != name) {
+    var lastComponent = components.last;
+    if (lastComponent.isEmpty) {
+      if (components[components.length - 2] != name) {
+        return false;
+      }
+    } else if (lastComponent != name) {
       return false;
     }
     for (var importedUri in importedUris) {

pkg/analysis_server/lib/src/services/correction/fix/data_driven/rename.dart

@@ -21,7 +21,45 @@ class Rename extends Change<SimpleIdentifier> {
   @override
   void apply(DartFileEditBuilder builder, DataDrivenFix fix,
       SimpleIdentifier nameNode) {
-    builder.addSimpleReplacement(range.node(nameNode), newName);
+    if (fix.element.kind == 'constructor') {
+      var parent = nameNode.parent;
+      if (parent is ConstructorName) {
+        if (nameNode != null && newName.isEmpty) {
+          // The constructor was renamed from a named constructor to an unnamed
+          // constructor.
+          builder.addDeletion(range.startEnd(parent.period, parent));
+        } else if (nameNode == null && newName.isNotEmpty) {
+          // The constructor was renamed from an unnamed constructor to a named
+          // constructor.
+          builder.addSimpleInsertion(parent.end, '.$newName');
+        } else {
+          // The constructor was renamed from a named constructor to another
+          // named constructor.
+          builder.addSimpleReplacement(range.node(nameNode), newName);
+        }
+      } else if (parent is MethodInvocation) {
+        if (newName.isEmpty) {
+          // The constructor was renamed from a named constructor to an unnamed
+          // constructor.
+          builder.addDeletion(range.startEnd(parent.operator, nameNode));
+        } else {
+          // The constructor was renamed from a named constructor to another
+          // named constructor.
+          builder.addSimpleReplacement(range.node(nameNode), newName);
+        }
+      } else if (parent is TypeName && parent.parent is ConstructorName) {
+        // The constructor was renamed from an unnamed constructor to a named
+        // constructor.
+        builder.addSimpleInsertion(parent.end, '.$newName');
+      } else {
+        // The constructor was renamed from a named constructor to another named
+        // constructor.
+        builder.addSimpleReplacement(range.node(nameNode), newName);
+      }
+    } else {
+      // The name is a simple identifier.
+      builder.addSimpleReplacement(range.node(nameNode), newName);
+    }
   }
 
   @override

pkg/analysis_server/lib/src/services/correction/fix/data_driven/transform_set_parser.dart

@@ -373,7 +373,8 @@ class TransformSetParser {
       } else {
         components.insert(0, containerName);
       }
-      return ElementDescriptor(libraryUris: uris, components: components);
+      return ElementDescriptor(
+          libraryUris: uris, kind: elementKey, components: components);
     } else if (node == null) {
       // TODO(brianwilkerson) Report the missing YAML.
       return null;

pkg/analysis_server/lib/src/services/correction/fix_internal.dart

@@ -528,6 +528,9 @@ class FixProcessor extends BaseProcessor {
     CompileTimeErrorCode.NEW_WITH_NON_TYPE: [
       ImportLibrary.forType,
     ],
+    CompileTimeErrorCode.NEW_WITH_UNDEFINED_CONSTRUCTOR_DEFAULT: [
+      DataDriven.newInstance,
+    ],
     CompileTimeErrorCode.NO_DEFAULT_SUPER_CONSTRUCTOR_EXPLICIT: [
       AddSuperConstructorInvocation.newInstance,
     ],

pkg/analysis_server/test/src/services/correction/fix/data_driven/add_type_parameter_test.dart

@@ -459,7 +459,10 @@ abstract class _AddTypeParameterChange extends DataDrivenFixProcessorTest {
       Transform(
           title: 'title',
           element: ElementDescriptor(
-              libraryUris: [importUri], components: components ?? ['C', 'm']),
+              libraryUris: [importUri],
+              // The kind isn't important to these tests.
+              kind: '',
+              components: components ?? ['C', 'm']),
           changes: [
             AddTypeParameter(
                 extendedType: extendedType,

pkg/analysis_server/test/src/services/correction/fix/data_driven/modify_parameters_test.dart

@@ -873,11 +873,15 @@ void h() {
 
 abstract class _ModifyParameters extends DataDrivenFixProcessorTest {
   Transform _modify(List<String> originalComponents,
-          List<ParameterModification> modifications, {String newName}) =>
+          List<ParameterModification> modifications,
+          {String newName}) =>
       Transform(
           title: 'title',
           element: ElementDescriptor(
-              libraryUris: [importUri], components: originalComponents),
+              libraryUris: [importUri],
+              // The kind isn't important to these tests.
+              kind: '',
+              components: originalComponents),
           changes: [
             ModifyParameters(modifications: modifications),
             if (newName != null) Rename(newName: newName),

pkg/analysis_server/test/src/services/correction/fix/data_driven/rename_test.dart

@@ -24,6 +24,109 @@ void main() {
 
 @reflectiveTest
 class RenameClassTest extends _AbstractRenameTest {
+  @override
+  String get _kind => 'class';
+
+  Future<void> test_constructor_named_deprecated() async {
+    setPackageContent('''
+@deprecated
+class Old {
+  Old.c();
+}
+class New {
+  New.c();
+}
+''');
+    setPackageData(_rename(['Old'], 'New'));
+    await resolveTestUnit('''
+import '$importUri';
+
+void f() {
+  Old.c();
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+
+void f() {
+  New.c();
+}
+''');
+  }
+
+  Future<void> test_constructor_named_removed() async {
+    setPackageContent('''
+class New {
+  New.c();
+}
+''');
+    setPackageData(_rename(['Old'], 'New'));
+    await resolveTestUnit('''
+import '$importUri';
+
+void f() {
+  Old.c();
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+
+void f() {
+  New.c();
+}
+''', errorFilter: ignoreUnusedImport);
+  }
+
+  Future<void> test_constructor_unnamed_deprecated() async {
+    setPackageContent('''
+@deprecated
+class Old {
+  Old();
+}
+class New {
+  New();
+}
+''');
+    setPackageData(_rename(['Old'], 'New'));
+    await resolveTestUnit('''
+import '$importUri';
+
+void f() {
+  Old();
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+
+void f() {
+  New();
+}
+''');
+  }
+
+  Future<void> test_constructor_unnamed_removed() async {
+    setPackageContent('''
+class New {
+  New();
+}
+''');
+    setPackageData(_rename(['Old'], 'New'));
+    await resolveTestUnit('''
+import '$importUri';
+
+void f() {
+  Old();
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+
+void f() {
+  New();
+}
+''', errorFilter: ignoreUnusedImport);
+  }
+
   Future<void> test_inExtends_deprecated() async {
     setPackageContent('''
 @deprecated
@@ -249,7 +352,10 @@ var s = New.empty;
 
 @reflectiveTest
 class RenameConstructorTest extends _AbstractRenameTest {
-  Future<void> test_named_deprecated() async {
+  @override
+  String get _kind => 'constructor';
+
+  Future<void> test_named_named_deprecated() async {
     setPackageContent('''
 class C {
   @deprecated
@@ -274,7 +380,7 @@ void f() {
 ''');
   }
 
-  Future<void> test_named_removed() async {
+  Future<void> test_named_named_removed() async {
     setPackageContent('''
 class C {
   C.new();
@@ -297,59 +403,108 @@ void f() {
 ''');
   }
 
-  Future<void> test_unnamed_deprecated() async {
+  Future<void> test_named_unnamed_deprecated() async {
     setPackageContent('''
-@deprecated
-class Old {
-  Old();
-}
-class New {
-  New();
+class C {
+  @deprecated
+  C.old();
+  C();
 }
 ''');
-    setPackageData(_rename(['Old'], 'New'));
+    setPackageData(_rename(['C', 'old'], ''));
     await resolveTestUnit('''
 import '$importUri';
 
 void f() {
-  Old();
+  C.old();
 }
 ''');
     await assertHasFix('''
 import '$importUri';
 
 void f() {
-  New();
+  C();
 }
 ''');
   }
 
-  Future<void> test_unnamed_removed() async {
+  Future<void> test_named_unnamed_removed() async {
     setPackageContent('''
-class New {
-  New();
+class C {
+  C();
 }
 ''');
-    setPackageData(_rename(['Old'], 'New'));
+    setPackageData(_rename(['C', 'old'], ''));
     await resolveTestUnit('''
 import '$importUri';
 
 void f() {
-  Old();
+  C.old();
 }
 ''');
     await assertHasFix('''
 import '$importUri';
 
 void f() {
-  New();
+  C();
 }
-''', errorFilter: ignoreUnusedImport);
+''');
+  }
+
+  Future<void> test_unnamed_named_deprecated() async {
+    setPackageContent('''
+class C {
+  @deprecated
+  C();
+  C.new();
+}
+''');
+    setPackageData(_rename(['C', ''], 'new'));
+    await resolveTestUnit('''
+import '$importUri';
+
+void f() {
+  C();
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+
+void f() {
+  C.new();
+}
+''');
+  }
+
+  Future<void> test_unnamed_named_removed() async {
+    setPackageContent('''
+class C {
+  C.new();
+}
+''');
+    setPackageData(_rename(['C', ''], 'new'));
+    await resolveTestUnit('''
+import '$importUri';
+
+void f() {
+  C();
+}
+''');
+    await assertHasFix('''
+import '$importUri';
+
+void f() {
+  C.new();
+}
+''');
   }
 }
 
 @reflectiveTest
 class RenameExtensionTest extends _AbstractRenameTest {
+  @override
+  String get _kind => 'extension';
+
   Future<void> test_override_deprecated() async {
     setPackageContent('''
 @deprecated
@@ -437,6 +592,9 @@ var s = New.empty;
 
 @reflectiveTest
 class RenameFieldTest extends _AbstractRenameTest {
+  @override
+  String get _kind => 'field';
+
   Future<void> test_instance_reference_deprecated() async {
     setPackageContent('''
 class C {
@@ -584,6 +742,9 @@ void f() {
 
 @reflectiveTest
 class RenameMethodTest extends _AbstractRenameTest {
+  @override
+  String get _kind => 'method';
+
   @failingTest
   Future<void> test_instance_override_deprecated() async {
     setPackageContent('''
@@ -736,6 +897,9 @@ void f() {
 
 @reflectiveTest
 class RenameMixinTest extends _AbstractRenameTest {
+  @override
+  String get _kind => 'mixin';
+
   Future<void> test_inWith_deprecated() async {
     setPackageContent('''
 @deprecated
@@ -775,6 +939,9 @@ class C with New {}
 
 @reflectiveTest
 class RenameTopLevelFunctionTest extends _AbstractRenameTest {
+  @override
+  String get _kind => 'function';
+
   Future<void> test_deprecated() async {
     setPackageContent('''
 @deprecated
@@ -822,6 +989,9 @@ void f() {
 
 @reflectiveTest
 class RenameTypedefTest extends _AbstractRenameTest {
+  @override
+  String get _kind => 'typedef';
+
   Future<void> test_deprecated() async {
     setPackageContent('''
 @deprecated
@@ -859,11 +1029,14 @@ void f(New o) {}
   }
 }
 
-class _AbstractRenameTest extends DataDrivenFixProcessorTest {
+abstract class _AbstractRenameTest extends DataDrivenFixProcessorTest {
+  /// Return the kind of element being renamed.
+  String get _kind;
+
   Transform _rename(List<String> components, String newName) => Transform(
           title: 'title',
           element: ElementDescriptor(
-              libraryUris: [importUri], components: components),
+              libraryUris: [importUri], kind: _kind, components: components),
           changes: [
             Rename(newName: newName),
           ]);