Skip to content

Commit 86d5040

Browse files
gabrittogabritto
authored
Fix renaming of node_modules (microsoft#49568)
* add bug repro test * add test and start fix implementation * adjust for useAlias preference * fix existing renaming test * refactor to get rid of options * fix named bindings & other imports cases * fix eslint error * address cr comments * hopefully actually fix eslint * clean up stale baseline * make API change non-breaking * add/fix comments
1 parent 01ba3a4 commit 86d5040

20 files changed

+352
-36
lines changed

‎src/compiler/diagnosticMessages.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6312,6 +6312,15 @@
63126312
"category": "Error",
63136313
"code": 8034
63146314
},
6315+
"You cannot rename elements that are defined in a 'node_modules' folder.": {
6316+
"category": "Error",
6317+
"code": 8035
6318+
},
6319+
"You cannot rename elements that are defined in another 'node_modules' folder.": {
6320+
"category": "Error",
6321+
"code": 8036
6322+
},
6323+
63156324
"Declaration emit for this file requires using private name '{0}'. An explicit type annotation may unblock declaration emit.": {
63166325
"category": "Error",
63176326
"code": 9005

‎src/compiler/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8989,6 +8989,7 @@ namespace ts {
89898989
readonly includeInlayPropertyDeclarationTypeHints?: boolean;
89908990
readonly includeInlayFunctionLikeReturnTypeHints?: boolean;
89918991
readonly includeInlayEnumMemberValueHints?: boolean;
8992+
readonly allowRenameOfImportPath?: boolean;
89928993
}
89938994

89948995
/** Represents a bigint literal value without requiring bigint support */

‎src/harness/client.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ namespace ts.server {
440440
return notImplemented();
441441
}
442442

443-
getRenameInfo(fileName: string, position: number, _options?: RenameInfoOptions, findInStrings?: boolean, findInComments?: boolean): RenameInfo {
443+
getRenameInfo(fileName: string, position: number, _preferences: UserPreferences, findInStrings?: boolean, findInComments?: boolean): RenameInfo {
444444
// Not passing along 'options' because server should already have those from the 'configure' command
445445
const args: protocol.RenameRequestArgs = { ...this.createFileLocationRequestArgs(fileName, position), findInStrings, findInComments };
446446

@@ -500,14 +500,14 @@ namespace ts.server {
500500
// User preferences have to be set through the `Configure` command
501501
this.configure({ providePrefixAndSuffixTextForRename });
502502
// Options argument is not used, so don't pass in options
503-
this.getRenameInfo(fileName, position, /*options*/{}, findInStrings, findInComments);
503+
this.getRenameInfo(fileName, position, /*preferences*/{}, findInStrings, findInComments);
504504
// Restore previous user preferences
505505
if (this.preferences) {
506506
this.configure(this.preferences);
507507
}
508508
}
509509
else {
510-
this.getRenameInfo(fileName, position, /*options*/{}, findInStrings, findInComments);
510+
this.getRenameInfo(fileName, position, /*preferences*/{}, findInStrings, findInComments);
511511
}
512512
}
513513

‎src/harness/fourslashImpl.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,14 +1430,17 @@ namespace FourSlash {
14301430
contextRangeDelta?: number
14311431
contextRangeId?: string;
14321432
}
1433-
const { findInStrings = false, findInComments = false, ranges = this.getRanges(), providePrefixAndSuffixTextForRename = true } = ts.isArray(options) ? { findInStrings: false, findInComments: false, ranges: options, providePrefixAndSuffixTextForRename: true } : options;
1433+
const { findInStrings = false, findInComments = false, ranges = this.getRanges(), providePrefixAndSuffixTextForRename = true } =
1434+
ts.isArray(options)
1435+
? { findInStrings: false, findInComments: false, ranges: options, providePrefixAndSuffixTextForRename: true }
1436+
: options;
14341437

14351438
const _startRanges = toArray(startRanges);
14361439
assert(_startRanges.length);
14371440
for (const startRange of _startRanges) {
14381441
this.goToRangeStart(startRange);
14391442

1440-
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition);
1443+
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, { providePrefixAndSuffixTextForRename });
14411444
if (!renameInfo.canRename) {
14421445
this.raiseError("Expected rename to succeed, but it actually failed.");
14431446
break;
@@ -1630,8 +1633,15 @@ namespace FourSlash {
16301633
}
16311634
}
16321635

1633-
public verifyRenameInfoSucceeded(displayName: string | undefined, fullDisplayName: string | undefined, kind: string | undefined, kindModifiers: string | undefined, fileToRename: string | undefined, expectedRange: Range | undefined, renameInfoOptions: ts.RenameInfoOptions | undefined): void {
1634-
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, renameInfoOptions || { allowRenameOfImportPath: true });
1636+
public verifyRenameInfoSucceeded(
1637+
displayName: string | undefined,
1638+
fullDisplayName: string | undefined,
1639+
kind: string | undefined,
1640+
kindModifiers: string | undefined,
1641+
fileToRename: string | undefined,
1642+
expectedRange: Range | undefined,
1643+
preferences: ts.UserPreferences | undefined): void {
1644+
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, preferences || { allowRenameOfImportPath: true });
16351645
if (!renameInfo.canRename) {
16361646
throw this.raiseError("Rename did not succeed");
16371647
}
@@ -1656,9 +1666,9 @@ namespace FourSlash {
16561666
}
16571667
}
16581668

1659-
public verifyRenameInfoFailed(message?: string, allowRenameOfImportPath?: boolean) {
1660-
allowRenameOfImportPath = allowRenameOfImportPath === undefined ? true : allowRenameOfImportPath;
1661-
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, { allowRenameOfImportPath });
1669+
public verifyRenameInfoFailed(message?: string, preferences?: ts.UserPreferences) {
1670+
const allowRenameOfImportPath = preferences?.allowRenameOfImportPath === undefined ? true : preferences.allowRenameOfImportPath;
1671+
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, { ...preferences, allowRenameOfImportPath });
16621672
if (renameInfo.canRename) {
16631673
throw this.raiseError("Rename was expected to fail");
16641674
}

‎src/harness/fourslashInterfaceImpl.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -556,12 +556,19 @@ namespace FourSlashInterface {
556556
this.state.replaceWithSemanticClassifications(format);
557557
}
558558

559-
public renameInfoSucceeded(displayName?: string, fullDisplayName?: string, kind?: string, kindModifiers?: string, fileToRename?: string, expectedRange?: FourSlash.Range, options?: ts.RenameInfoOptions) {
560-
this.state.verifyRenameInfoSucceeded(displayName, fullDisplayName, kind, kindModifiers, fileToRename, expectedRange, options);
561-
}
562-
563-
public renameInfoFailed(message?: string, allowRenameOfImportPath?: boolean) {
564-
this.state.verifyRenameInfoFailed(message, allowRenameOfImportPath);
559+
public renameInfoSucceeded(
560+
displayName?: string,
561+
fullDisplayName?: string,
562+
kind?: string,
563+
kindModifiers?: string,
564+
fileToRename?: string,
565+
expectedRange?: FourSlash.Range,
566+
preferences?: ts.UserPreferences) {
567+
this.state.verifyRenameInfoSucceeded(displayName, fullDisplayName, kind, kindModifiers, fileToRename, expectedRange, preferences);
568+
}
569+
570+
public renameInfoFailed(message?: string, preferences?: ts.UserPreferences) {
571+
this.state.verifyRenameInfoFailed(message, preferences);
565572
}
566573

567574
public renameLocations(startRanges: ArrayOrSingle<FourSlash.Range>, options: RenameLocationsOptions) {

‎src/harness/harnessLanguageService.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,8 +511,8 @@ namespace Harness.LanguageService {
511511
getSignatureHelpItems(fileName: string, position: number, options: ts.SignatureHelpItemsOptions | undefined): ts.SignatureHelpItems {
512512
return unwrapJSONCallResult(this.shim.getSignatureHelpItems(fileName, position, options));
513513
}
514-
getRenameInfo(fileName: string, position: number, options?: ts.RenameInfoOptions): ts.RenameInfo {
515-
return unwrapJSONCallResult(this.shim.getRenameInfo(fileName, position, options));
514+
getRenameInfo(fileName: string, position: number, preferences: ts.UserPreferences): ts.RenameInfo {
515+
return unwrapJSONCallResult(this.shim.getRenameInfo(fileName, position, preferences));
516516
}
517517
getSmartSelectionRange(fileName: string, position: number): ts.SelectionRange {
518518
return unwrapJSONCallResult(this.shim.getSmartSelectionRange(fileName, position));

‎src/server/session.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1707,7 +1707,8 @@ namespace ts.server {
17071707
private getRenameInfo(args: protocol.FileLocationRequestArgs): RenameInfo {
17081708
const { file, project } = this.getFileAndProject(args);
17091709
const position = this.getPositionInFile(args, file);
1710-
return project.getLanguageService().getRenameInfo(file, position, { allowRenameOfImportPath: this.getPreferences(file).allowRenameOfImportPath });
1710+
const preferences = this.getPreferences(file);
1711+
return project.getLanguageService().getRenameInfo(file, position, preferences);
17111712
}
17121713

17131714
private getProjects(args: protocol.FileRequestArgs, getScriptInfoEnsuringProjectsUptoDate?: boolean, ignoreNoProjectError?: boolean): Projects {
@@ -1759,8 +1760,9 @@ namespace ts.server {
17591760
const position = this.getPositionInFile(args, file);
17601761
const projects = this.getProjects(args);
17611762
const defaultProject = this.getDefaultProject(args);
1763+
const preferences = this.getPreferences(file);
17621764
const renameInfo: protocol.RenameInfo = this.mapRenameInfo(
1763-
defaultProject.getLanguageService().getRenameInfo(file, position, { allowRenameOfImportPath: this.getPreferences(file).allowRenameOfImportPath }), Debug.checkDefined(this.projectService.getScriptInfo(file)));
1765+
defaultProject.getLanguageService().getRenameInfo(file, position, preferences), Debug.checkDefined(this.projectService.getScriptInfo(file)));
17641766

17651767
if (!renameInfo.canRename) return simplifiedResult ? { info: renameInfo, locs: [] } : [];
17661768

@@ -1770,7 +1772,7 @@ namespace ts.server {
17701772
{ fileName: args.file, pos: position },
17711773
!!args.findInStrings,
17721774
!!args.findInComments,
1773-
this.getPreferences(file)
1775+
preferences,
17741776
);
17751777
if (!simplifiedResult) return locations;
17761778
return { info: renameInfo, locs: this.toSpanGroups(locations) };

‎src/services/rename.ts

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
/* @internal */
22
namespace ts.Rename {
3-
export function getRenameInfo(program: Program, sourceFile: SourceFile, position: number, options?: RenameInfoOptions): RenameInfo {
3+
export function getRenameInfo(program: Program, sourceFile: SourceFile, position: number, preferences: UserPreferences): RenameInfo {
44
const node = getAdjustedRenameLocation(getTouchingPropertyName(sourceFile, position));
55
if (nodeIsEligibleForRename(node)) {
6-
const renameInfo = getRenameInfoForNode(node, program.getTypeChecker(), sourceFile, program, options);
6+
const renameInfo = getRenameInfoForNode(node, program.getTypeChecker(), sourceFile, program, preferences);
77
if (renameInfo) {
88
return renameInfo;
99
}
1010
}
1111
return getRenameInfoError(Diagnostics.You_cannot_rename_this_element);
1212
}
1313

14-
function getRenameInfoForNode(node: Node, typeChecker: TypeChecker, sourceFile: SourceFile, program: Program, options?: RenameInfoOptions): RenameInfo | undefined {
14+
function getRenameInfoForNode(
15+
node: Node,
16+
typeChecker: TypeChecker,
17+
sourceFile: SourceFile,
18+
program: Program,
19+
preferences: UserPreferences): RenameInfo | undefined {
1520
const symbol = typeChecker.getSymbolAtLocation(node);
1621
if (!symbol) {
1722
if (isStringLiteralLike(node)) {
@@ -43,7 +48,13 @@ namespace ts.Rename {
4348
}
4449

4550
if (isStringLiteralLike(node) && tryGetImportFromModuleSpecifier(node)) {
46-
return options && options.allowRenameOfImportPath ? getRenameInfoForModule(node, sourceFile, symbol) : undefined;
51+
return preferences.allowRenameOfImportPath ? getRenameInfoForModule(node, sourceFile, symbol) : undefined;
52+
}
53+
54+
// Disallow rename for elements that would rename across `*/node_modules/*` packages.
55+
const wouldRenameNodeModules = wouldRenameInOtherNodeModules(sourceFile, symbol, typeChecker, preferences);
56+
if (wouldRenameNodeModules) {
57+
return getRenameInfoError(wouldRenameNodeModules);
4758
}
4859

4960
const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, node);
@@ -60,6 +71,55 @@ namespace ts.Rename {
6071
return program.isSourceFileDefaultLibrary(sourceFile) && fileExtensionIs(sourceFile.fileName, Extension.Dts);
6172
}
6273

74+
function wouldRenameInOtherNodeModules(
75+
originalFile: SourceFile,
76+
symbol: Symbol,
77+
checker: TypeChecker,
78+
preferences: UserPreferences
79+
): DiagnosticMessage | undefined {
80+
if (!preferences.providePrefixAndSuffixTextForRename && symbol.flags & SymbolFlags.Alias) {
81+
const importSpecifier = symbol.declarations && find(symbol.declarations, decl => isImportSpecifier(decl));
82+
if (importSpecifier && !(importSpecifier as ImportSpecifier).propertyName) {
83+
symbol = checker.getAliasedSymbol(symbol);
84+
}
85+
}
86+
const { declarations } = symbol;
87+
if (!declarations) {
88+
return undefined;
89+
}
90+
const originalPackage = getPackagePathComponents(originalFile.path);
91+
if (originalPackage === undefined) { // original source file is not in node_modules
92+
if (some(declarations, declaration => isInsideNodeModules(declaration.getSourceFile().path))) {
93+
return Diagnostics.You_cannot_rename_elements_that_are_defined_in_a_node_modules_folder;
94+
}
95+
else {
96+
return undefined;
97+
}
98+
}
99+
// original source file is in node_modules
100+
for (const declaration of declarations) {
101+
const declPackage = getPackagePathComponents(declaration.getSourceFile().path);
102+
if (declPackage) {
103+
const length = Math.min(originalPackage.length, declPackage.length);
104+
for (let i = 0; i <= length; i++) {
105+
if (compareStringsCaseSensitive(originalPackage[i], declPackage[i]) !== Comparison.EqualTo) {
106+
return Diagnostics.You_cannot_rename_elements_that_are_defined_in_another_node_modules_folder;
107+
}
108+
}
109+
}
110+
}
111+
return undefined;
112+
}
113+
114+
function getPackagePathComponents(filePath: Path): string[] | undefined {
115+
const components = getPathComponents(filePath);
116+
const nodeModulesIdx = components.lastIndexOf("node_modules");
117+
if (nodeModulesIdx === -1) {
118+
return undefined;
119+
}
120+
return components.slice(0, nodeModulesIdx + 2);
121+
}
122+
63123
function getRenameInfoForModule(node: StringLiteralLike, sourceFile: SourceFile, moduleSymbol: Symbol): RenameInfo | undefined {
64124
if (!isExternalModuleNameRelative(node.text)) {
65125
return getRenameInfoError(Diagnostics.You_cannot_rename_a_module_via_a_global_import);

‎src/services/services.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2537,9 +2537,9 @@ namespace ts {
25372537
}
25382538
}
25392539

2540-
function getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): RenameInfo {
2540+
function getRenameInfo(fileName: string, position: number, preferences: UserPreferences | RenameInfoOptions | undefined): RenameInfo {
25412541
synchronizeHostData();
2542-
return Rename.getRenameInfo(program, getValidSourceFile(fileName), position, options);
2542+
return Rename.getRenameInfo(program, getValidSourceFile(fileName), position, preferences || {});
25432543
}
25442544

25452545
function getRefactorContext(file: SourceFile, positionOrRange: number | TextRange, preferences: UserPreferences, formatOptions?: FormatCodeSettings, triggerReason?: RefactorTriggerReason, kind?: string): RefactorContext {

‎src/services/shims.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ namespace ts {
165165
* Returns a JSON-encoded value of the type:
166166
* { canRename: boolean, localizedErrorMessage: string, displayName: string, fullDisplayName: string, kind: string, kindModifiers: string, triggerSpan: { start; length } }
167167
*/
168-
getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): string;
168+
getRenameInfo(fileName: string, position: number, preferences: UserPreferences): string;
169169
getSmartSelectionRange(fileName: string, position: number): string;
170170

171171
/**
@@ -855,10 +855,10 @@ namespace ts {
855855
);
856856
}
857857

858-
public getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): string {
858+
public getRenameInfo(fileName: string, position: number, preferences: UserPreferences): string {
859859
return this.forwardJSONCall(
860860
`getRenameInfo('${fileName}', ${position})`,
861-
() => this.languageService.getRenameInfo(fileName, position, options)
861+
() => this.languageService.getRenameInfo(fileName, position, preferences)
862862
);
863863
}
864864

‎src/services/types.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,10 @@ namespace ts {
467467

468468
getSignatureHelpItems(fileName: string, position: number, options: SignatureHelpItemsOptions | undefined): SignatureHelpItems | undefined;
469469

470+
getRenameInfo(fileName: string, position: number, preferences: UserPreferences): RenameInfo;
471+
/** @deprecated Use the signature with `UserPreferences` instead. */
470472
getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): RenameInfo;
473+
471474
findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): readonly RenameLocation[] | undefined;
472475

473476
getSmartSelectionRange(fileName: string, position: number): SelectionRange;
@@ -944,7 +947,7 @@ namespace ts {
944947
Remove = "remove",
945948
}
946949

947-
/* @deprecated - consider using EditorSettings instead */
950+
/** @deprecated - consider using EditorSettings instead */
948951
export interface EditorOptions {
949952
BaseIndentSize?: number;
950953
IndentSize: number;
@@ -965,7 +968,7 @@ namespace ts {
965968
trimTrailingWhitespace?: boolean;
966969
}
967970

968-
/* @deprecated - consider using FormatCodeSettings instead */
971+
/** @deprecated - consider using FormatCodeSettings instead */
969972
export interface FormatCodeOptions extends EditorOptions {
970973
InsertSpaceAfterCommaDelimiter: boolean;
971974
InsertSpaceAfterSemicolonInForStatements: boolean;
@@ -1135,6 +1138,9 @@ namespace ts {
11351138
localizedErrorMessage: string;
11361139
}
11371140

1141+
/**
1142+
* @deprecated Use `UserPreferences` instead.
1143+
*/
11381144
export interface RenameInfoOptions {
11391145
readonly allowRenameOfImportPath?: boolean;
11401146
}

0 commit comments

Comments
 (0)