Update imports in files other than the source and destination when moving declarations

Change-Id: I3675bddc9479744a4642bdbb66b8de013f0f01df
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/273524
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Brian Wilkerson 2022-12-02 23:22:16 +00:00 committed by Commit Queue
parent 95a390cfd9
commit 6346e7fcd1
7 changed files with 219 additions and 4 deletions

View file

@ -5,6 +5,7 @@
import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
import 'package:analysis_server/src/services/correction/change_workspace.dart';
import 'package:analysis_server/src/services/correction/util.dart';
import 'package:analysis_server/src/services/search/search_engine.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/analysis/session.dart';
import 'package:analyzer/dart/ast/ast.dart';
@ -53,6 +54,9 @@ class RefactoringContext {
required this.selectionLength,
});
/// Return the search engine used to search outside the resolved library.
SearchEngine get searchEngine => server.searchEngine;
/// Return the analysis session in which additional resolution can occur.
AnalysisSession get session => resolvedUnitResult.session;
}

View file

@ -6,6 +6,7 @@ import 'package:analysis_server/lsp_protocol/protocol_custom_generated.dart';
import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
import 'package:analysis_server/src/services/correction/util.dart';
import 'package:analysis_server/src/services/refactoring/framework/refactoring_context.dart';
import 'package:analysis_server/src/services/search/search_engine.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/token.dart';
@ -34,6 +35,9 @@ abstract class RefactoringProducer {
/// Return a list of the parameters to send to the client.
List<CommandParameter> get parameters;
/// Return the search engine used to search outside the resolved library.
SearchEngine get searchEngine => _context.searchEngine;
/// Return the node that was selected, or `null` if the selection is not
/// valid.
AstNode? get selectedNode => _context.selectedNode;

View file

@ -2,11 +2,13 @@
// 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/lsp_protocol/protocol_custom_generated.dart';
import 'package:analysis_server/lsp_protocol/protocol_custom_generated.dart'
show CommandParameter, SaveUriCommandParameter;
import 'package:analysis_server/src/services/refactoring/framework/refactoring_producer.dart';
import 'package:analysis_server/src/utilities/extensions/ast.dart';
import 'package:analysis_server/src/utilities/import_analyzer.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
import 'package:analyzer_plugin/utilities/range_factory.dart';
@ -138,8 +140,47 @@ class MoveTopLevelToFile extends RefactoringProducer {
}
builder.addDeletion(range.deletionRange(member.node));
});
// TODO(brianwilkerson) Find references to the moved declaration(s) outside
// the source library and update the imports in those files.
// TODO(brianwilkerson) This doesn't correctly handle prefixes. In order to
// use the correct prefix when adding the import we need to either
// - enhance SearchMatch to know the prefix used for a reference match, or
// - look at the imports for the library to find the imports that might
// have been used to import the `element` and create a corresponding
// import of the new library for each of them.
//
// The latter has the disadvantage that we might end up adding more imports
// than are actually required because we can't know whether they're all
// needed.
var libraries = <LibraryElement, Set<Element>>{};
for (var element in analyzer.movingDeclarations) {
var matches = await searchEngine.searchReferences(element);
for (var match in matches) {
if (match.isResolved) {
libraries.putIfAbsent(match.libraryElement, () => {}).add(element);
}
}
}
/// Don't update the library from which the code is being moved because
/// that's already been done.
libraries.remove(libraryResult.element);
for (var entry in libraries.entries) {
var library = entry.key;
var prefixes = <String>{};
for (var element in entry.value) {
var prefixList =
await searchEngine.searchPrefixesUsedInLibrary(library, element);
prefixes.addAll(prefixList);
}
await builder.addDartFileEdit(library.source.fullName, (builder) {
for (var prefix in prefixes) {
if (prefix.isEmpty) {
builder.importLibrary(destinationImportUri);
} else {
builder.importLibrary(destinationImportUri, prefix: prefix);
}
}
});
}
}
@override

View file

@ -92,6 +92,12 @@ abstract class SearchEngine {
/// [name] - the name being referenced by the found matches.
Future<List<SearchMatch>> searchMemberReferences(String name);
/// Return the prefixes used to reference the [element] in any of the
/// compilation units in the [library]. The returned set will include an empty
/// string if the element is referenced without a prefix.
Future<Set<String>> searchPrefixesUsedInLibrary(
LibraryElement library, Element element);
/// Returns references to the given [Element].
///
/// [element] - the [Element] being referenced by the found matches.

View file

@ -5,7 +5,9 @@
import 'package:analysis_server/src/services/search/search_engine.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/dart/analysis/driver.dart';
import 'package:analyzer/src/dart/analysis/driver_based_analysis_context.dart';
import 'package:analyzer/src/dart/analysis/search.dart';
import 'package:analyzer/src/dart/element/element.dart';
import 'package:analyzer/src/generated/source.dart' show Source, SourceRange;
import 'package:analyzer/src/util/performance/operation_performance.dart';
@ -103,6 +105,14 @@ class SearchEngineImpl implements SearchEngine {
return allResults.map(SearchMatchImpl.forSearchResult).toList();
}
@override
Future<Set<String>> searchPrefixesUsedInLibrary(
covariant LibraryElementImpl library, Element element) async {
var driver =
(library.session.analysisContext as DriverBasedAnalysisContext).driver;
return await driver.search.prefixesUsedInLibrary(library, element);
}
@override
Future<List<SearchMatch>> searchReferences(Element element) async {
var allResults = <SearchResult>[];

View file

@ -299,6 +299,129 @@ class B {}
newFileContent: newFileContent);
}
Future<void> test_imports_referenceInThirdFile_noPrefix() async {
var originalSource = '''
class A {}
class B^ {}
''';
var modifiedSource = '''
class A {}
''';
var declarationName = 'B';
var newFileName = 'b.dart';
var newFileContent = '''
class B {}
''';
var otherFilePath = '$projectFolderPath/lib/c.dart';
var otherFileContent = '''
import 'package:test/main.dart';
B? b;
''';
var modifiedOtherFileContent = '''
import 'package:test/b.dart';
import 'package:test/main.dart';
B? b;
''';
await _singleDeclaration(
originalSource: originalSource,
modifiedSource: modifiedSource,
declarationName: declarationName,
newFileName: newFileName,
newFileContent: newFileContent,
otherFilePath: otherFilePath,
otherFileContent: otherFileContent,
modifiedOtherFileContent: modifiedOtherFileContent);
}
@failingTest
Future<void> test_imports_referenceInThirdFile_withMultiplePrefixes() async {
// This fails for two reasons:
// 1. The indexer isn't recording when a top-level element is referenced
// without a prefix.
// 2. The method `DartFileEditBuilderImpl._importLibrary` doesn't support
// importing the same URI with multiple prefixes.
var originalSource = '''
class A {}
class B^ {}
''';
var modifiedSource = '''
class A {}
''';
var declarationName = 'B';
var newFileName = 'b.dart';
var newFileContent = '''
class B {}
''';
var otherFilePath = '$projectFolderPath/lib/c.dart';
var otherFileContent = '''
import 'package:test/main.dart';
import 'package:test/main.dart' as p;
import 'package:test/main.dart' as q;
void f(p.B b, q.B b, B b) {}
''';
var modifiedOtherFileContent = '''
import 'package:test/b.dart';
import 'package:test/b.dart' as p;
import 'package:test/b.dart' as q;
import 'package:test/main.dart';
import 'package:test/main.dart' as p;
import 'package:test/main.dart' as q;
void f(p.B b, q.B b, B b) {}
''';
await _singleDeclaration(
originalSource: originalSource,
modifiedSource: modifiedSource,
declarationName: declarationName,
newFileName: newFileName,
newFileContent: newFileContent,
otherFilePath: otherFilePath,
otherFileContent: otherFileContent,
modifiedOtherFileContent: modifiedOtherFileContent);
}
Future<void> test_imports_referenceInThirdFile_withSinglePrefix() async {
var originalSource = '''
class A {}
class B^ {}
''';
var modifiedSource = '''
class A {}
''';
var declarationName = 'B';
var newFileName = 'b.dart';
var newFileContent = '''
class B {}
''';
var otherFilePath = '$projectFolderPath/lib/c.dart';
var otherFileContent = '''
import 'package:test/main.dart' as p;
p.B? b;
''';
var modifiedOtherFileContent = '''
import 'package:test/b.dart' as p;
import 'package:test/main.dart' as p;
p.B? b;
''';
await _singleDeclaration(
originalSource: originalSource,
modifiedSource: modifiedSource,
declarationName: declarationName,
newFileName: newFileName,
newFileContent: newFileContent,
otherFilePath: otherFilePath,
otherFileContent: otherFileContent,
modifiedOtherFileContent: modifiedOtherFileContent);
}
Future<void> test_mixin() async {
var originalSource = '''
class A {}
@ -394,8 +517,14 @@ int variableToMove = 3;
required String modifiedSource,
required String declarationName,
required String newFileName,
required String newFileContent}) async {
required String newFileContent,
String? otherFilePath,
String? otherFileContent,
String? modifiedOtherFileContent}) async {
addTestSource(originalSource);
if (otherFilePath != null) {
addSource(otherFilePath, otherFileContent!);
}
/// Expected new file path/content.
final expectedNewFilePath = join(projectFolderPath, 'lib', newFileName);
@ -409,5 +538,8 @@ int variableToMove = 3;
// was sent, the executeRefactor helper would've thrown when trying to
// apply the changes.
expect(content[expectedNewFilePath], newFileContent);
if (modifiedOtherFileContent != null) {
expect(content[otherFilePath!], modifiedOtherFileContent);
}
}
}

View file

@ -264,6 +264,24 @@ class Search {
return elements;
}
/// Return the prefixes used to reference the [element] in any of the
/// compilation units in the [library]. The returned set will include an empty
/// string if the element is referenced without a prefix.
Future<Set<String>> prefixesUsedInLibrary(
LibraryElementImpl library, Element element) async {
var prefixes = <String>{};
for (var unit in library.units) {
var index = await _driver.getIndex(unit.source.fullName);
if (index != null) {
_IndexRequest request = _IndexRequest(index);
int elementId = request.findElementId(element);
var prefixList = index.elementImportPrefixes[elementId].split(',');
prefixes.addAll(prefixList);
}
}
return prefixes;
}
/// Returns references to the [element].
Future<List<SearchResult>> references(
Element? element, SearchedFiles searchedFiles) async {