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 <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/data_driven.dart b/pkg/analysis_server/lib/src/services/correction/dart/data_driven.dart
index 837da74..176d2c6 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/data_driven.dart
+++ b/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,7 +35,8 @@
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);
@@ -42,6 +44,50 @@
}
}
+ 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) {
diff --git a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/element_descriptor.dart b/pkg/analysis_server/lib/src/services/correction/fix/data_driven/element_descriptor.dart
index 84b5dd0..7fb93d9 100644
--- a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/element_descriptor.dart
+++ b/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 @@
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 @@
/// 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;
}
diff --git a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/element_kind.dart b/pkg/analysis_server/lib/src/services/correction/fix/data_driven/element_kind.dart
new file mode 100644
index 0000000..c0233f7
--- /dev/null
+++ b/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;
+ }
+}
diff --git a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/element_matcher.dart b/pkg/analysis_server/lib/src/services/correction/fix/data_driven/element_matcher.dart
index f18c300..aac5dff 100644
--- a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/element_matcher.dart
+++ b/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 @@
/// 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 @@
} 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)) {
diff --git a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/transform_set_parser.dart b/pkg/analysis_server/lib/src/services/correction/fix/data_driven/transform_set_parser.dart
index e36d5e5..30d05a2 100644
--- a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/transform_set_parser.dart
+++ b/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/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 @@
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 {
diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/add_type_parameter_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/add_type_parameter_test.dart
index c36b226..e90178e 100644
--- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/add_type_parameter_test.dart
+++ b/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 @@
@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 @@
@reflectiveTest
class AddTypeParameterToExtensionTest extends _AddTypeParameterChange {
+ @override
+ String get _kind => 'extension';
+
Future<void> test_override_removed() async {
setPackageContent('''
class C {}
@@ -160,6 +167,9 @@
@reflectiveTest
class AddTypeParameterToMethodTest extends _AddTypeParameterChange {
+ @override
+ String get _kind => 'method';
+
Future<void> test_first_deprecated() async {
setPackageContent('''
class C {
@@ -332,6 +342,9 @@
@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 @@
@reflectiveTest
class AddTypeParameterToTopLevelFunctionTest extends _AddTypeParameterChange {
+ @override
+ String get _kind => 'function';
+
Future<void> test_only_deprecated() async {
setPackageContent('''
@deprecated
@@ -377,6 +393,9 @@
@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 @@
}
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(
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 6d5e0aa..dd56d2c 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
@@ -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 @@
@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 @@
/// 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 @@
}
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),
diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/rename_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/rename_test.dart
index c335a76..39fd964 100644
--- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/rename_test.dart
+++ b/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 @@
@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 @@
title: 'title',
element: ElementDescriptor(
libraryUris: [Uri.parse(importUri)],
- kind: _kind,
+ kind: ElementKindUtilities.fromName(_kind),
components: components),
changes: [
Rename(newName: newName),
