Skip to content

Commit c06ad0d

Browse files
committed
Issue 24420. Don't allow renaming in pub packages.
There are no ideal solution for this. But all our users want it somehow. [email protected] BUG= #24420 Review URL: https://codereview.chromium.org/1380253003 .
1 parent 93bf72b commit c06ad0d

File tree

4 files changed

+121
-0
lines changed

4 files changed

+121
-0
lines changed

pkg/analysis_server/lib/src/services/refactoring/rename.dart

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import 'package:analyzer/src/generated/element.dart';
1717
import 'package:analyzer/src/generated/engine.dart';
1818
import 'package:analyzer/src/generated/java_core.dart';
1919
import 'package:analyzer/src/generated/source.dart';
20+
import 'package:path/path.dart' as pathos;
2021

2122
/**
2223
* Returns `true` if two given [Element]s are [LocalElement]s and have
@@ -52,13 +53,42 @@ bool isDefinedInLibrary(
5253
return librarySourcesOfSource.contains(librarySourceOfElement);
5354
}
5455

56+
bool isElementInPubCache(Element element) {
57+
Source source = element.source;
58+
String path = source.fullName;
59+
return isPathInPubCache(path);
60+
}
61+
62+
bool isElementInSdkOrPubCache(Element element) {
63+
Source source = element.source;
64+
String path = source.fullName;
65+
return source.isInSystemLibrary || isPathInPubCache(path);
66+
}
67+
5568
/**
5669
* Checks if the given [Element] is in the given [AnalysisContext].
5770
*/
5871
bool isInContext(Element element, AnalysisContext context) {
5972
return element.context == context;
6073
}
6174

75+
bool isPathInPubCache(String path) {
76+
List<String> parts = pathos.split(path);
77+
if (parts.contains('.pub-cache')) {
78+
return true;
79+
}
80+
for (int i = 0; i < parts.length - 1; i++) {
81+
if (parts[i] == 'Pub' && parts[i + 1] == 'Cache') {
82+
return true;
83+
}
84+
if (parts[i] == 'third_party' &&
85+
(parts[i + 1] == 'pkg' || parts[i + 1] == 'pkg_tested')) {
86+
return true;
87+
}
88+
}
89+
return false;
90+
}
91+
6292
/**
6393
* Checks if the given unqualified [SearchMatch] intersects with visibility
6494
* range of [localElement].
@@ -145,6 +175,13 @@ abstract class RenameRefactoringImpl extends RefactoringImpl
145175
getElementQualifiedName(element));
146176
result.addFatalError(message);
147177
}
178+
if (isElementInPubCache(element)) {
179+
String message = format(
180+
"The {0} '{1}' is defined in a pub package, so cannot be renamed.",
181+
getElementKindName(element),
182+
getElementQualifiedName(element));
183+
result.addFatalError(message);
184+
}
148185
return new Future.value(result);
149186
}
150187

pkg/analysis_server/lib/src/services/refactoring/rename_class_member.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ class RenameClassMemberRefactoringImpl extends RenameRefactoringImpl {
100100
if (reference.isResolved) {
101101
continue;
102102
}
103+
// ignore references from SDK and pub cache
104+
if (isElementInSdkOrPubCache(reference.element)) {
105+
print('ignore: $reference');
106+
continue;
107+
}
103108
// check the element being renamed is accessible
104109
{
105110
LibraryElement whereLibrary = reference.element.library;

pkg/analysis_server/test/services/refactoring/rename_class_member_test.dart

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,43 @@ main(var a) {
582582
assertPotentialEdits(['test(); // 1', 'test(); // 2']);
583583
}
584584

585+
test_createChange_MethodElement_potential_inPubCache() async {
586+
String pkgLib = '/.pub-cache/lib.dart';
587+
indexUnit(
588+
pkgLib,
589+
r'''
590+
processObj(p) {
591+
p.test();
592+
}
593+
''');
594+
indexTestUnit('''
595+
import '$pkgLib';
596+
class A {
597+
test() {}
598+
}
599+
main(var a) {
600+
a.test();
601+
}
602+
''');
603+
// configure refactoring
604+
createRenameRefactoringAtString('test() {}');
605+
expect(refactoring.refactoringName, 'Rename Method');
606+
expect(refactoring.oldName, 'test');
607+
refactoring.newName = 'newName';
608+
// validate change
609+
await assertSuccessfulRefactoring('''
610+
import '/.pub-cache/lib.dart';
611+
class A {
612+
newName() {}
613+
}
614+
main(var a) {
615+
a.newName();
616+
}
617+
''');
618+
SourceFileEdit fileEdit = refactoringChange.getFileEdit(pkgLib);
619+
expect(fileEdit, isNull);
620+
}
621+
585622
test_createChange_MethodElement_potential_private_otherLibrary() async {
586623
indexUnit(
587624
'/lib.dart',

pkg/analysis_server/test/services/refactoring/rename_unit_member_test.dart

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,48 @@ class B {
224224
assertRefactoringStatusOK(status);
225225
}
226226

227+
test_checkInitialConditions_inPubCache_posix() async {
228+
addSource(
229+
'/.pub-cache/lib.dart',
230+
r'''
231+
class A {}
232+
''');
233+
indexTestUnit('''
234+
import '/.pub-cache/lib.dart';
235+
main() {
236+
A a;
237+
}
238+
''');
239+
createRenameRefactoringAtString('A a');
240+
// check status
241+
refactoring.newName = 'NewName';
242+
RefactoringStatus status = await refactoring.checkInitialConditions();
243+
assertRefactoringStatus(status, RefactoringProblemSeverity.FATAL,
244+
expectedMessage:
245+
"The class 'A' is defined in a pub package, so cannot be renamed.");
246+
}
247+
248+
test_checkInitialConditions_inPubCache_windows() async {
249+
addSource(
250+
'/Pub/Cache/lib.dart',
251+
r'''
252+
class A {}
253+
''');
254+
indexTestUnit('''
255+
import '/Pub/Cache/lib.dart';
256+
main() {
257+
A a;
258+
}
259+
''');
260+
createRenameRefactoringAtString('A a');
261+
// check status
262+
refactoring.newName = 'NewName';
263+
RefactoringStatus status = await refactoring.checkInitialConditions();
264+
assertRefactoringStatus(status, RefactoringProblemSeverity.FATAL,
265+
expectedMessage:
266+
"The class 'A' is defined in a pub package, so cannot be renamed.");
267+
}
268+
227269
test_checkInitialConditions_inSDK() async {
228270
indexTestUnit('''
229271
main() {

0 commit comments

Comments
 (0)