From 3290d7a137031205ca475ead56b62323dcb4d066 Mon Sep 17 00:00:00 2001 From: pq Date: Mon, 6 Jun 2022 16:53:38 +0000 Subject: [PATCH] [data driven] support moving symbols across packages See: https://github.com/dart-lang/sdk/issues/48997 Change-Id: Iad16b9eae0523bc4bc14537af642b05efa75b6f7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246663 Commit-Queue: Phil Quitslund Reviewed-by: Brian Wilkerson --- .../fix/data_driven/replaced_by.dart | 5 + .../fix/data_driven/replaced_by_test.dart | 78 ++++++++++++++ .../correction/fix/data_driven/test_all.dart | 2 + .../fix/data_driven/test_use_case_test.dart | 102 ++++++++++++++++++ .../change_builder/change_builder_dart.dart | 20 ++++ .../change_builder/change_builder_dart.dart | 4 + 6 files changed, 211 insertions(+) create mode 100644 pkg/analysis_server/test/src/services/correction/fix/data_driven/test_use_case_test.dart diff --git a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/replaced_by.dart b/pkg/analysis_server/lib/src/services/correction/fix/data_driven/replaced_by.dart index d690c0b23ffb..a8d459b6ef4e 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix/data_driven/replaced_by.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix/data_driven/replaced_by.dart @@ -27,6 +27,11 @@ class ReplacedBy extends Change<_Data> { void apply(DartFileEditBuilder builder, DataDrivenFix fix, _Data data) { var referenceRange = data.referenceRange; builder.addSimpleReplacement(referenceRange, _referenceTo(newElement)); + var libraryUris = newElement.libraryUris; + if (libraryUris.isEmpty) return; + if (!libraryUris.any((uri) => builder.importsLibrary(uri))) { + builder.importLibraryElement(libraryUris.first); + } } @override diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/replaced_by_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/replaced_by_test.dart index a7b70a4caae1..b4643f24abf3 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/replaced_by_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/replaced_by_test.dart @@ -14,6 +14,7 @@ import 'data_driven_test_support.dart'; void main() { defineReflectiveSuite(() { defineReflectiveTests(ReplacedByTest); + defineReflectiveTests(ReplacedByUriSemanticsTest); }); } @@ -862,6 +863,83 @@ var x = $prefixReference${newElement.reference}$invocation; } } +@reflectiveTest +class ReplacedByUriSemanticsTest extends DataDrivenFixProcessorTest { + Future test_new_element_uris_multiple() async { + setPackageContent(''); + newFile('$workspaceRootPath/p/lib/expect.dart', ''' +void expect(actual, expected) {} +'''); + newFile('$workspaceRootPath/p/lib/export.dart', ''' +export 'expect.dart'; +'''); + addPackageDataFile(''' +version: 1 +transforms: + - title: 'Replace expect' + date: 2022-05-12 + bulkApply: false + element: + uris: ['$importUri'] + function: 'expect' + changes: + - kind: 'replacedBy' + newElement: + uris: ['package:p/expect.dart', 'package:p/export.dart'] + function: 'expect' +'''); + await resolveTestCode(''' +import '$importUri'; + +f() { + expect(true, true); +} +'''); + await assertHasFix(''' +import 'package:p/expect.dart'; +import '$importUri'; + +f() { + expect(true, true); +} +''', errorFilter: ignoreUnusedImport); + } + + Future test_new_element_uris_single() async { + setPackageContent(''); + addPackageDataFile(''' +version: 1 +transforms: + - title: 'Replace expect' + date: 2022-05-12 + bulkApply: false + element: + uris: ['$importUri'] + function: 'expect' + changes: + - kind: 'replacedBy' + newElement: + uris: ['package:matcher/expect.dart'] + function: 'expect' +'''); + await resolveTestCode(''' +import '$importUri'; + +main() { + expect(true, true); +} +'''); + await assertHasFix(''' +import 'package:matcher/expect.dart'; +import '$importUri'; + +main() { + expect(true, true); +} +''', errorFilter: ignoreUnusedImport); + } +} + class _Element { final ElementKind kind; final List components; diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_all.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_all.dart index 544f27c7cf65..f9623c1744f5 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_all.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_all.dart @@ -17,6 +17,7 @@ import 'rename_parameter_test.dart' as rename_parameter; import 'rename_test.dart' as rename; import 'replaced_by_test.dart' as replaced_by; import 'sdk_fix_test.dart' as sdk_fix; +import 'test_use_case_test.dart' as test_use_case; import 'transform_override_set_parser_test.dart' as transform_override_set_parser; import 'transform_set_manager_test.dart' as transform_set_manager; @@ -37,6 +38,7 @@ void main() { rename.main(); replaced_by.main(); sdk_fix.main(); + test_use_case.main(); transform_override_set_parser.main(); transform_set_manager.main(); transform_set_parser.main(); diff --git a/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_use_case_test.dart b/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_use_case_test.dart new file mode 100644 index 000000000000..aa0386bdad77 --- /dev/null +++ b/pkg/analysis_server/test/src/services/correction/fix/data_driven/test_use_case_test.dart @@ -0,0 +1,102 @@ +// Copyright (c) 2022, 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. + +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../fix_processor.dart'; +import 'data_driven_test_support.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(TestUseCaseTest); + }); +} + +@reflectiveTest +class TestUseCaseTest extends DataDrivenFixProcessorTest { + @FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/23067') + Future test_expect_export_deprecated() async { + newFile('$workspaceRootPath/p/lib/lib.dart', ''' +library p; +@deprecated +export 'package:matcher/expect.dart' show expect; +'''); + newFile('$workspaceRootPath/matcher/lib/expect.dart', ''' +void expect(actual, matcher) {} +'''); + + writeTestPackageConfig( + config: PackageConfigFileBuilder() + ..add(name: 'matcher', rootPath: '$workspaceRootPath/matcher') + ..add(name: 'p', rootPath: '$workspaceRootPath/p'), + ); + + addPackageDataFile(''' +version: 1 +transforms: + - title: 'Replace expect' + date: 2022-05-12 + bulkApply: false + element: + uris: ['$importUri'] + function: 'expect' + changes: + - kind: 'replacedBy' + newElement: + uris: ['package:matcher/expect.dart'] + function: 'expect' +'''); + await resolveTestCode(''' +import '$importUri'; + +main() { + expect(true, true); +} +'''); + await assertHasFix(''' +import 'package:matcher/expect.dart'; +import '$importUri'; + +main() { + expect(true, true); +} +'''); + } + + Future test_expect_removed() async { + setPackageContent(''' +'''); + + addPackageDataFile(''' +version: 1 +transforms: + - title: 'Replace expect' + date: 2022-05-12 + bulkApply: false + element: + uris: ['$importUri'] + function: 'expect' + changes: + - kind: 'replacedBy' + newElement: + uris: ['package:matcher/expect.dart'] + function: 'expect' +'''); + await resolveTestCode(''' +import '$importUri'; + +main() { + expect(true, true); +} +'''); + await assertHasFix(''' +import 'package:matcher/expect.dart'; +import '$importUri'; + +main() { + expect(true, true); +} +''', errorFilter: ignoreUnusedImport); + } +} diff --git a/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart b/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart index 660832746ead..9ad3e64d9dfd 100644 --- a/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart +++ b/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart @@ -1483,6 +1483,26 @@ class DartFileEditBuilderImpl extends FileEditBuilderImpl return _importLibrary(uri, prefix: prefix, forceRelative: true).uriText; } + @override + bool importsLibrary(Uri uri) { + // Self-reference. + if (resolvedUnit.libraryElement.source.uri == uri) return false; + + // Existing import. + for (var import in resolvedUnit.libraryElement.imports) { + var importedLibrary = import.importedLibrary; + if (importedLibrary != null && importedLibrary.source.uri == uri) { + return true; + } + } + + // Queued change. + var importChange = (libraryChangeBuilder ?? this).librariesToImport[uri]; + if (importChange != null) return true; + + return false; + } + @override void replaceTypeWithFuture( TypeAnnotation? typeAnnotation, TypeProvider typeProvider) { diff --git a/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_dart.dart b/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_dart.dart index 4eababeb2118..2082af034028 100644 --- a/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_dart.dart +++ b/pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_dart.dart @@ -359,6 +359,10 @@ abstract class DartFileEditBuilder implements FileEditBuilder { /// If there is no existing import, a new import is added. ImportLibraryElementResult importLibraryElement(Uri uri); + /// Return `true` if the given library [uri] is already imported or will be + /// imported by a scheduled edit. + bool importsLibrary(Uri uri); + /// Optionally create an edit to replace the given [typeAnnotation] with the /// type `Future` (with the given type annotation as the type argument). The /// [typeProvider] is used to check the current type, because if it is already