Issue 33181. Use RefactoringWorkspace in rename refactoring to determine when element being renamed is outside the workspace.
R=brianwilkerson@google.com
Bug: https://github.com/dart-lang/sdk/issues/33181
Change-Id: I01c9b275579cd961a581491d8cd25893b3bcc094
Reviewed-on: https://dart-review.googlesource.com/56039
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/edit/edit_domain.dart b/pkg/analysis_server/lib/src/edit/edit_domain.dart
index 77fd41b..1af301c 100644
--- a/pkg/analysis_server/lib/src/edit/edit_domain.dart
+++ b/pkg/analysis_server/lib/src/edit/edit_domain.dart
@@ -65,6 +65,11 @@
SearchEngine searchEngine;
/**
+ * The workspace for rename refactorings.
+ */
+ RefactoringWorkspace refactoringWorkspace;
+
+ /**
* The object used to manage uncompleted refactorings.
*/
_RefactoringManager refactoringManager;
@@ -74,6 +79,8 @@
*/
EditDomainHandler(AnalysisServer server) : super(server) {
searchEngine = server.searchEngine;
+ refactoringWorkspace =
+ new RefactoringWorkspace(server.driverMap.values, searchEngine);
_newRefactoringManager();
}
@@ -589,7 +596,7 @@
// try RENAME
{
RenameRefactoring renameRefactoring = new RenameRefactoring(
- searchEngine, server.getAstProvider(file), element);
+ refactoringWorkspace, server.getAstProvider(file), element);
if (renameRefactoring != null) {
kinds.add(RefactoringKind.RENAME);
}
@@ -614,7 +621,7 @@
* Initializes [refactoringManager] with a new instance.
*/
void _newRefactoringManager() {
- refactoringManager = new _RefactoringManager(server, searchEngine);
+ refactoringManager = new _RefactoringManager(server, refactoringWorkspace);
}
static int _getNumberOfScanParseErrors(List<engine.AnalysisError> errors) {
@@ -695,6 +702,7 @@
const <RefactoringProblem>[];
final AnalysisServer server;
+ final RefactoringWorkspace refactoringWorkspace;
final SearchEngine searchEngine;
StreamSubscription subscriptionToReset;
@@ -711,7 +719,8 @@
Request request;
EditGetRefactoringResult result;
- _RefactoringManager(this.server, this.searchEngine) {
+ _RefactoringManager(this.server, this.refactoringWorkspace)
+ : searchEngine = refactoringWorkspace.searchEngine {
_reset();
}
@@ -978,7 +987,7 @@
// do create the refactoring
refactoring = new RenameRefactoring(
- searchEngine, server.getAstProvider(file), element);
+ refactoringWorkspace, server.getAstProvider(file), element);
feedback = new RenameFeedback(
feedbackOffset, feedbackLength, 'kind', 'oldName');
}
diff --git a/pkg/analysis_server/lib/src/services/refactoring/refactoring.dart b/pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
index 96243dd..74f5d98 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
@@ -23,6 +23,7 @@
import 'package:analyzer/dart/analysis/session.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/src/dart/analysis/driver.dart';
import 'package:analyzer/src/dart/element/ast_provider.dart';
import 'package:analyzer_plugin/protocol/protocol_common.dart'
show RefactoringMethodParameter, SourceChange;
@@ -378,6 +379,25 @@
}
/**
+ * Information about the workspace refactorings operate it.
+ */
+class RefactoringWorkspace {
+ final Iterable<AnalysisDriver> drivers;
+ final SearchEngine searchEngine;
+
+ RefactoringWorkspace(this.drivers, this.searchEngine);
+
+ /**
+ * Whether the file with the given [path] is in a context root.
+ */
+ bool containsFile(String path) {
+ return drivers.any((driver) {
+ return driver.contextRoot.containsFile(path);
+ });
+ }
+}
+
+/**
* Abstract [Refactoring] for renaming some [Element].
*/
abstract class RenameRefactoring implements Refactoring {
@@ -386,8 +406,8 @@
* maybe `null` if there is no support for renaming [Element]s of the given
* type.
*/
- factory RenameRefactoring(
- SearchEngine searchEngine, AstProvider astProvider, Element element) {
+ factory RenameRefactoring(RefactoringWorkspace workspace,
+ AstProvider astProvider, Element element) {
if (element == null) {
return null;
}
@@ -395,28 +415,27 @@
element = (element as PropertyAccessorElement).variable;
}
if (element.enclosingElement is CompilationUnitElement) {
- return new RenameUnitMemberRefactoringImpl(searchEngine, element);
+ return new RenameUnitMemberRefactoringImpl(workspace, element);
}
if (element is ConstructorElement) {
return new RenameConstructorRefactoringImpl(
- searchEngine, astProvider, element);
+ workspace, astProvider, element);
}
if (element is ImportElement) {
- return new RenameImportRefactoringImpl(
- searchEngine, astProvider, element);
+ return new RenameImportRefactoringImpl(workspace, astProvider, element);
}
if (element is LabelElement) {
- return new RenameLabelRefactoringImpl(searchEngine, element);
+ return new RenameLabelRefactoringImpl(workspace, element);
}
if (element is LibraryElement) {
- return new RenameLibraryRefactoringImpl(searchEngine, element);
+ return new RenameLibraryRefactoringImpl(workspace, element);
}
if (element is LocalElement) {
- return new RenameLocalRefactoringImpl(searchEngine, astProvider, element);
+ return new RenameLocalRefactoringImpl(workspace, astProvider, element);
}
if (element.enclosingElement is ClassElement) {
return new RenameClassMemberRefactoringImpl(
- searchEngine, astProvider, element);
+ workspace, astProvider, element);
}
return null;
}
diff --git a/pkg/analysis_server/lib/src/services/refactoring/rename.dart b/pkg/analysis_server/lib/src/services/refactoring/rename.dart
index db79f49..716b2de13 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/rename.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/rename.dart
@@ -12,44 +12,14 @@
import 'package:analysis_server/src/services/search/search_engine.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/generated/java_core.dart';
-import 'package:analyzer/src/generated/source.dart';
import 'package:analyzer_plugin/utilities/range_factory.dart';
-import 'package:path/path.dart' as pathos;
-
-bool isElementInPubCache(Element element) {
- Source source = element.source;
- String path = source.fullName;
- return isPathInPubCache(path);
-}
-
-bool isElementInSdkOrPubCache(Element element) {
- Source source = element.source;
- String path = source.fullName;
- return source.isInSystemLibrary || isPathInPubCache(path);
-}
-
-bool isPathInPubCache(String path) {
- List<String> parts = pathos.split(path);
- if (parts.contains('.pub-cache')) {
- return true;
- }
- for (int i = 0; i < parts.length - 1; i++) {
- if (parts[i] == 'Pub' && parts[i + 1] == 'Cache') {
- return true;
- }
- if (parts[i] == 'third_party' &&
- (parts[i + 1] == 'pkg' || parts[i + 1] == 'pkg_tested')) {
- return true;
- }
- }
- return false;
-}
/**
* An abstract implementation of [RenameRefactoring].
*/
abstract class RenameRefactoringImpl extends RefactoringImpl
implements RenameRefactoring {
+ final RefactoringWorkspace workspace;
final SearchEngine searchEngine;
final Element _element;
final String elementKindName;
@@ -58,8 +28,8 @@
String newName;
- RenameRefactoringImpl(SearchEngine searchEngine, Element element)
- : searchEngine = searchEngine,
+ RenameRefactoringImpl(this.workspace, Element element)
+ : searchEngine = workspace.searchEngine,
_element = element,
elementKindName = element.kind.displayName,
oldName = _getDisplayName(element);
@@ -97,9 +67,9 @@
getElementQualifiedName(element));
result.addFatalError(message);
}
- if (isElementInPubCache(element)) {
+ if (!workspace.containsFile(element.source.fullName)) {
String message = format(
- "The {0} '{1}' is defined in a pub package, so cannot be renamed.",
+ "The {0} '{1}' is defined outside of the project, so cannot be renamed.",
getElementKindName(element),
getElementQualifiedName(element));
result.addFatalError(message);
diff --git a/pkg/analysis_server/lib/src/services/refactoring/rename_class_member.dart b/pkg/analysis_server/lib/src/services/refactoring/rename_class_member.dart
index 9c10cdb..7c58a69 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/rename_class_member.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/rename_class_member.dart
@@ -41,8 +41,8 @@
_ClassMemberValidator _validator;
RenameClassMemberRefactoringImpl(
- SearchEngine searchEngine, this.astProvider, Element element)
- : super(searchEngine, element);
+ RefactoringWorkspace workspace, this.astProvider, Element element)
+ : super(workspace, element);
@override
String get refactoringName {
@@ -102,7 +102,7 @@
List<SourceReference> nameRefs = getSourceReferences(nameMatches);
for (SourceReference reference in nameRefs) {
// ignore references from SDK and pub cache
- if (isElementInSdkOrPubCache(reference.element)) {
+ if (!workspace.containsFile(reference.element.source.fullName)) {
continue;
}
// check the element being renamed is accessible
diff --git a/pkg/analysis_server/lib/src/services/refactoring/rename_constructor.dart b/pkg/analysis_server/lib/src/services/refactoring/rename_constructor.dart
index e079875..ccdb578 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/rename_constructor.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/rename_constructor.dart
@@ -27,9 +27,9 @@
class RenameConstructorRefactoringImpl extends RenameRefactoringImpl {
final AstProvider astProvider;
- RenameConstructorRefactoringImpl(
- SearchEngine searchEngine, this.astProvider, ConstructorElement element)
- : super(searchEngine, element);
+ RenameConstructorRefactoringImpl(RefactoringWorkspace workspace,
+ this.astProvider, ConstructorElement element)
+ : super(workspace, element);
@override
ConstructorElement get element => super.element as ConstructorElement;
diff --git a/pkg/analysis_server/lib/src/services/refactoring/rename_import.dart b/pkg/analysis_server/lib/src/services/refactoring/rename_import.dart
index 2adca4d..826fcf0 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/rename_import.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/rename_import.dart
@@ -27,8 +27,8 @@
final AstProvider astProvider;
RenameImportRefactoringImpl(
- SearchEngine searchEngine, this.astProvider, ImportElement element)
- : super(searchEngine, element);
+ RefactoringWorkspace workspace, this.astProvider, ImportElement element)
+ : super(workspace, element);
@override
ImportElement get element => super.element as ImportElement;
diff --git a/pkg/analysis_server/lib/src/services/refactoring/rename_label.dart b/pkg/analysis_server/lib/src/services/refactoring/rename_label.dart
index 651fc7ac..e8a62c9 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/rename_label.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/rename_label.dart
@@ -8,15 +8,15 @@
import 'package:analysis_server/src/services/refactoring/naming_conventions.dart';
import 'package:analysis_server/src/services/refactoring/refactoring.dart';
import 'package:analysis_server/src/services/refactoring/rename.dart';
-import 'package:analysis_server/src/services/search/search_engine.dart';
import 'package:analyzer/dart/element/element.dart';
/**
* A [Refactoring] for renaming [LabelElement]s.
*/
class RenameLabelRefactoringImpl extends RenameRefactoringImpl {
- RenameLabelRefactoringImpl(SearchEngine searchEngine, LabelElement element)
- : super(searchEngine, element);
+ RenameLabelRefactoringImpl(
+ RefactoringWorkspace workspace, LabelElement element)
+ : super(workspace, element);
@override
LabelElement get element => super.element as LabelElement;
diff --git a/pkg/analysis_server/lib/src/services/refactoring/rename_library.dart b/pkg/analysis_server/lib/src/services/refactoring/rename_library.dart
index 1c72c89..ac8326b 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/rename_library.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/rename_library.dart
@@ -8,7 +8,6 @@
import 'package:analysis_server/src/services/refactoring/naming_conventions.dart';
import 'package:analysis_server/src/services/refactoring/refactoring.dart';
import 'package:analysis_server/src/services/refactoring/rename.dart';
-import 'package:analysis_server/src/services/search/search_engine.dart';
import 'package:analyzer/dart/element/element.dart';
/**
@@ -16,8 +15,8 @@
*/
class RenameLibraryRefactoringImpl extends RenameRefactoringImpl {
RenameLibraryRefactoringImpl(
- SearchEngine searchEngine, LibraryElement element)
- : super(searchEngine, element);
+ RefactoringWorkspace workspace, LibraryElement element)
+ : super(workspace, element);
@override
LibraryElement get element => super.element as LibraryElement;
diff --git a/pkg/analysis_server/lib/src/services/refactoring/rename_local.dart b/pkg/analysis_server/lib/src/services/refactoring/rename_local.dart
index eca6220..d452e7d 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/rename_local.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/rename_local.dart
@@ -12,7 +12,6 @@
import 'package:analysis_server/src/services/refactoring/refactoring.dart';
import 'package:analysis_server/src/services/refactoring/rename.dart';
import 'package:analysis_server/src/services/search/hierarchy.dart';
-import 'package:analysis_server/src/services/search/search_engine.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element.dart';
@@ -29,9 +28,9 @@
List<LocalElement> elements = [];
RenameLocalRefactoringImpl(
- SearchEngine searchEngine, this.astProvider, LocalElement element)
+ RefactoringWorkspace workspace, this.astProvider, LocalElement element)
: unitCache = new ResolvedUnitCache(astProvider),
- super(searchEngine, element);
+ super(workspace, element);
@override
LocalElement get element => super.element as LocalElement;
diff --git a/pkg/analysis_server/lib/src/services/refactoring/rename_unit_member.dart b/pkg/analysis_server/lib/src/services/refactoring/rename_unit_member.dart
index fea2831..5f483a9 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/rename_unit_member.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/rename_unit_member.dart
@@ -42,8 +42,9 @@
* A [Refactoring] for renaming compilation unit member [Element]s.
*/
class RenameUnitMemberRefactoringImpl extends RenameRefactoringImpl {
- RenameUnitMemberRefactoringImpl(SearchEngine searchEngine, Element element)
- : super(searchEngine, element);
+ RenameUnitMemberRefactoringImpl(
+ RefactoringWorkspace workspace, Element element)
+ : super(workspace, element);
@override
String get refactoringName {
diff --git a/pkg/analysis_server/test/services/refactoring/abstract_rename.dart b/pkg/analysis_server/test/services/refactoring/abstract_rename.dart
index 3d8633c..3bb9141 100644
--- a/pkg/analysis_server/test/services/refactoring/abstract_rename.dart
+++ b/pkg/analysis_server/test/services/refactoring/abstract_rename.dart
@@ -55,7 +55,8 @@
* Fails if no [RenameRefactoring] can be created.
*/
void createRenameRefactoringForElement(Element element) {
- refactoring = new RenameRefactoring(searchEngine, astProvider, element);
+ var workspace = new RefactoringWorkspace([driver], searchEngine);
+ refactoring = new RenameRefactoring(workspace, astProvider, element);
expect(refactoring, isNotNull, reason: "No refactoring for '$element'.");
}
diff --git a/pkg/analysis_server/test/services/refactoring/rename_constructor_test.dart b/pkg/analysis_server/test/services/refactoring/rename_constructor_test.dart
index 1434a05..2f0f4a3 100644
--- a/pkg/analysis_server/test/services/refactoring/rename_constructor_test.dart
+++ b/pkg/analysis_server/test/services/refactoring/rename_constructor_test.dart
@@ -230,8 +230,8 @@
}
test_newInstance_nullElement() async {
- RenameRefactoring refactoring =
- new RenameRefactoring(searchEngine, null, null);
+ var workspace = new RefactoringWorkspace([driver], searchEngine);
+ var refactoring = new RenameRefactoring(workspace, null, null);
expect(refactoring, isNull);
}
diff --git a/pkg/analysis_server/test/services/refactoring/rename_unit_member_test.dart b/pkg/analysis_server/test/services/refactoring/rename_unit_member_test.dart
index 6fbd113..e5c22bf 100644
--- a/pkg/analysis_server/test/services/refactoring/rename_unit_member_test.dart
+++ b/pkg/analysis_server/test/services/refactoring/rename_unit_member_test.dart
@@ -214,44 +214,6 @@
assertRefactoringStatusOK(status);
}
- test_checkInitialConditions_inPubCache_posix() async {
- addSource('/.pub-cache/lib.dart', r'''
-class A {}
-''');
- await indexTestUnit('''
-import "${convertPathForImport('/.pub-cache/lib.dart')}";
-main() {
- A a;
-}
-''');
- createRenameRefactoringAtString('A a');
- // check status
- refactoring.newName = 'NewName';
- RefactoringStatus status = await refactoring.checkInitialConditions();
- assertRefactoringStatus(status, RefactoringProblemSeverity.FATAL,
- expectedMessage:
- "The class 'A' is defined in a pub package, so cannot be renamed.");
- }
-
- test_checkInitialConditions_inPubCache_windows() async {
- addSource('/Pub/Cache/lib.dart', r'''
-class A {}
-''');
- await indexTestUnit('''
-import "${convertPathForImport('/Pub/Cache/lib.dart')}";
-main() {
- A a;
-}
-''');
- createRenameRefactoringAtString('A a');
- // check status
- refactoring.newName = 'NewName';
- RefactoringStatus status = await refactoring.checkInitialConditions();
- assertRefactoringStatus(status, RefactoringProblemSeverity.FATAL,
- expectedMessage:
- "The class 'A' is defined in a pub package, so cannot be renamed.");
- }
-
test_checkInitialConditions_inSDK() async {
await indexTestUnit('''
main() {
@@ -267,6 +229,25 @@
"The class 'String' is defined in the SDK, so cannot be renamed.");
}
+ test_checkInitialConditions_outsideOfProject() async {
+ addSource('/other/lib.dart', r'''
+class A {}
+''');
+ await indexTestUnit('''
+import "${convertPathForImport('/other/lib.dart')}";
+main() {
+ A a;
+}
+''');
+ createRenameRefactoringAtString('A a');
+ // check status
+ refactoring.newName = 'NewName';
+ RefactoringStatus status = await refactoring.checkInitialConditions();
+ assertRefactoringStatus(status, RefactoringProblemSeverity.FATAL,
+ expectedMessage:
+ "The class 'A' is defined outside of the project, so cannot be renamed.");
+ }
+
test_checkNewName_ClassElement() async {
await indexTestUnit('''
class Test {}
