Use the information we have about the kind of element when computing fixes

This puts in place support for using the element kind when determining
whether a transform applies. It is not as fully tested as it should be
but I plan to add more tests in future CLs.

Change-Id: Ife89c49384d1ceba9de5b18b7ef197018747ee25
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/167000
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: bwilkerson

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

@@ -5,12 +5,13 @@
 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/element_kind.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/element_matcher.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';
 import 'package:analyzer/dart/ast/ast.dart';
-import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/dart/element/element.dart' show LibraryElement;
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 import 'package:meta/meta.dart';
@@ -34,14 +35,59 @@ class DataDriven extends MultiCorrectionProducer {
         importedUris.add(Uri.parse(uri));
       }
     }
-    var matcher = ElementMatcher(importedUris: importedUris, name: name);
+    var matcher =
+        ElementMatcher(importedUris: importedUris, name: name, kinds: _kinds);
     for (var set in _availableTransformSetsForLibrary(library)) {
       for (var transform in set.transformsFor(matcher)) {
         yield DataDrivenFix(transform);
       }
     }
   }
 
+  List<ElementKind> get _kinds {
+    AstNode child;
+    var node = this.node;
+    while (node != null) {
+      if (node is ConstructorName) {
+        return const [ElementKind.constructorKind];
+      } else if (node is ExtensionOverride) {
+        return const [ElementKind.extensionKind];
+      } else if (node is InstanceCreationExpression) {
+        return const [ElementKind.constructorKind];
+      } else if (node is MethodInvocation) {
+        if (node.target == child) {
+          return const [
+            ElementKind.classKind,
+            ElementKind.enumKind,
+            ElementKind.mixinKind
+          ];
+        } else if (node.realTarget != null) {
+          return const [ElementKind.constructorKind, ElementKind.methodKind];
+        }
+        return const [
+          ElementKind.classKind,
+          ElementKind.extensionKind,
+          ElementKind.functionKind,
+          ElementKind.methodKind
+        ];
+      } else if (node is NamedType) {
+        var parent = node.parent;
+        if (parent is ConstructorName && parent.name == null) {
+          return const [ElementKind.classKind, ElementKind.constructorKind];
+        }
+        return const [
+          ElementKind.classKind,
+          ElementKind.enumKind,
+          ElementKind.mixinKind,
+          ElementKind.typedefKind
+        ];
+      }
+      child = node;
+      node = node.parent;
+    }
+    return null;
+  }
+
   /// Return the name of the element that was changed.
   String get _name {
     String nameFromParent(AstNode node) {

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

@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'package:analysis_server/src/services/correction/fix/data_driven/element_kind.dart';
 import 'package:meta/meta.dart';
 
 /// The path to an element.
@@ -10,7 +11,7 @@ class ElementDescriptor {
   final List<Uri> libraryUris;
 
   /// The kind of element that was changed.
-  final String _kind;
+  final ElementKind kind;
 
   /// The components that uniquely identify the element within its library.
   final List<String> components;
@@ -21,10 +22,9 @@ class ElementDescriptor {
   /// element is represented by the key used in the data file.
   ElementDescriptor(
       {@required this.libraryUris,
-      @required String kind,
-      @required this.components})
-      : _kind = kind;
+      @required this.kind,
+      @required this.components});
 
   /// Return `true` if the described element is a constructor.
-  bool get isConstructor => _kind == 'constructor';
+  bool get isConstructor => kind == ElementKind.constructorKind;
 }

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

@@ -0,0 +1,32 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+/// An indication of the kind of an element.
+enum ElementKind {
+  classKind,
+  constantKind,
+  constructorKind,
+  enumKind,
+  extensionKind,
+  fieldKind,
+  functionKind,
+  getterKind,
+  methodKind,
+  mixinKind,
+  setterKind,
+  typedefKind,
+  variableKind
+}
+
+extension ElementKindUtilities on ElementKind {
+  /// Return the element kind corresponding to the given [name].
+  static ElementKind fromName(String name) {
+    for (var kind in ElementKind.values) {
+      if (kind.toString() == 'ElementKind.${name}Kind') {
+        return kind;
+      }
+    }
+    return null;
+  }
+}

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

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analysis_server/src/services/correction/fix/data_driven/element_descriptor.dart';
+import 'package:analysis_server/src/services/correction/fix/data_driven/element_kind.dart';
 import 'package:meta/meta.dart';
 
 /// An object that can be used to determine whether an element is appropriate
@@ -15,9 +16,18 @@ class ElementMatcher {
   /// The name of the element being referenced.
   final String name;
 
+  /// A list of the kinds of elements that are appropriate for some given
+  /// location in the code An empty list represents all kinds rather than no
+  /// kinds.
+  List<ElementKind> validKinds;
+
   /// Initialize a newly created matcher representing a reference to an element
   /// with the given [name] in a library that imports the [importedUris].
-  ElementMatcher({@required this.importedUris, @required this.name});
+  ElementMatcher(
+      {@required this.importedUris,
+      @required this.name,
+      List<ElementKind> kinds})
+      : validKinds = kinds ?? const [];
 
   /// Return `true` if this matcher matches the given [element].
   bool matches(ElementDescriptor element) {
@@ -30,6 +40,11 @@ class ElementMatcher {
     } else if (lastComponent != name) {
       return false;
     }
+
+    if (validKinds.isNotEmpty && !validKinds.contains(element.kind)) {
+      return false;
+    }
+
     var libraryUris = element.libraryUris;
     for (var importedUri in importedUris) {
       if (libraryUris.contains(importedUri)) {

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

@@ -6,6 +6,7 @@ import 'package:analysis_server/src/services/correction/fix/data_driven/add_type
 import 'package:analysis_server/src/services/correction/fix/data_driven/change.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/code_template.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/element_descriptor.dart';
+import 'package:analysis_server/src/services/correction/fix/data_driven/element_kind.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/modify_parameters.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/parameter_reference.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/rename.dart';
@@ -534,7 +535,9 @@ class TransformSetParser {
         return null;
       }
       return ElementDescriptor(
-          libraryUris: uris, kind: elementKey, components: components);
+          libraryUris: uris,
+          kind: ElementKindUtilities.fromName(elementKey),
+          components: components);
     } else if (node == null) {
       return _reportMissingKey(context);
     } else {

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

@@ -4,6 +4,7 @@
 
 import 'package:analysis_server/src/services/correction/fix/data_driven/add_type_parameter.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/element_descriptor.dart';
+import 'package:analysis_server/src/services/correction/fix/data_driven/element_kind.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/transform.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
@@ -22,6 +23,9 @@ void main() {
 
 @reflectiveTest
 class AddTypeParameterToClassTest extends _AddTypeParameterChange {
+  @override
+  String get _kind => 'class';
+
   Future<void> test_constructorInvocation_removed() async {
     setPackageContent('''
 class C<S, T> {
@@ -133,6 +137,9 @@ class B with A<String, int> {}
 
 @reflectiveTest
 class AddTypeParameterToExtensionTest extends _AddTypeParameterChange {
+  @override
+  String get _kind => 'extension';
+
   Future<void> test_override_removed() async {
     setPackageContent('''
 class C {}
@@ -160,6 +167,9 @@ void f(C c) {
 
 @reflectiveTest
 class AddTypeParameterToMethodTest extends _AddTypeParameterChange {
+  @override
+  String get _kind => 'method';
+
   Future<void> test_first_deprecated() async {
     setPackageContent('''
 class C {
@@ -332,6 +342,9 @@ class D extends C {
 
 @reflectiveTest
 class AddTypeParameterToMixinTest extends _AddTypeParameterChange {
+  @override
+  String get _kind => 'mixin';
+
   Future<void> test_inWith_removed() async {
     setPackageContent('''
 mixin M<S, T> {}
@@ -352,6 +365,9 @@ class B with M<String, int> {}
 
 @reflectiveTest
 class AddTypeParameterToTopLevelFunctionTest extends _AddTypeParameterChange {
+  @override
+  String get _kind => 'function';
+
   Future<void> test_only_deprecated() async {
     setPackageContent('''
 @deprecated
@@ -377,6 +393,9 @@ void g() {
 
 @reflectiveTest
 class AddTypeParameterToTypedefTest extends _AddTypeParameterChange {
+  @override
+  String get _kind => 'typedef';
+
   @failingTest
   Future<void> test_functionType_removed() async {
     // The test fails because the change is to the typedef `F`, not to the
@@ -454,13 +473,15 @@ void g(F<String> f) {
 }
 
 abstract class _AddTypeParameterChange extends DataDrivenFixProcessorTest {
+  /// Return the kind of element whose parameters are being modified.
+  String get _kind;
+
   Transform _add(int index, {List<String> components, String extendedType}) =>
       Transform(
           title: 'title',
           element: ElementDescriptor(
               libraryUris: [Uri.parse(importUri)],
-              // The kind isn't important to these tests.
-              kind: '',
+              kind: ElementKindUtilities.fromName(_kind),
               components: components ?? ['C', 'm']),
           changes: [
             AddTypeParameter(

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

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analysis_server/src/services/correction/fix/data_driven/element_descriptor.dart';
+import 'package:analysis_server/src/services/correction/fix/data_driven/element_kind.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/modify_parameters.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/parameter_reference.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/rename.dart';
@@ -20,6 +21,9 @@ void main() {
 
 @reflectiveTest
 class ModifyParametersOfMethodTest extends _ModifyParameters {
+  @override
+  String get _kind => 'method';
+
   Future<void> test_add_first_optionalNamed_deprecated() async {
     setPackageContent('''
 class C {
@@ -817,6 +821,9 @@ void f(C c) {
 /// applied to top-level functions, but are not intended to be exhaustive.
 @reflectiveTest
 class ModifyParametersOfTopLevelFunctionTest extends _ModifyParameters {
+  @override
+  String get _kind => 'function';
+
   Future<void> test_add_first_requiredNamed_deprecated() async {
     setPackageContent('''
 @deprecated
@@ -871,15 +878,16 @@ void h() {
 }
 
 abstract class _ModifyParameters extends DataDrivenFixProcessorTest {
+  /// Return the kind of element whose parameters are being modified.
+  String get _kind;
+
   Transform _modify(List<String> originalComponents,
-          List<ParameterModification> modifications,
-          {String newName}) =>
+          List<ParameterModification> modifications, {String newName}) =>
       Transform(
           title: 'title',
           element: ElementDescriptor(
               libraryUris: [Uri.parse(importUri)],
-              // The kind isn't important to these tests.
-              kind: '',
+              kind: ElementKindUtilities.fromName(_kind),
               components: originalComponents),
           changes: [
             ModifyParameters(modifications: modifications),

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

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analysis_server/src/services/correction/fix/data_driven/element_descriptor.dart';
+import 'package:analysis_server/src/services/correction/fix/data_driven/element_kind.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/rename.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/transform.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -746,7 +747,30 @@ class RenameGetterTest extends _AbstractRenameTest {
   @override
   String get _kind => 'getter';
 
-  Future<void> test_instance_nonReference_deprecated() async {
+  Future<void> test_instance_nonReference_method_deprecated() async {
+    setPackageContent('''
+class C {
+  @deprecated
+  int get a => 0;
+  int get b => 1;
+}
+class D {
+  @deprecated
+  void a(int b) {}
+}
+''');
+    setPackageData(_rename(['C', 'a'], 'b'));
+    await resolveTestUnit('''
+import '$importUri';
+
+void f(D d) {
+  d.a(2);
+}
+''');
+    await assertNoFix();
+  }
+
+  Future<void> test_instance_nonReference_parameter_deprecated() async {
     setPackageContent('''
 class C {
   @deprecated
@@ -1156,7 +1180,7 @@ abstract class _AbstractRenameTest extends DataDrivenFixProcessorTest {
           title: 'title',
           element: ElementDescriptor(
               libraryUris: [Uri.parse(importUri)],
-              kind: _kind,
+              kind: ElementKindUtilities.fromName(_kind),
               components: components),
           changes: [
             Rename(newName: newName),