Skip to content

Fix renaming of node_modules #49568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -6276,6 +6276,15 @@
"category": "Error",
"code": 8034
},
"You cannot rename elements that are defined in a 'node_modules' folder.": {
"category": "Error",
"code": 8035
},
"You cannot rename elements that are defined in another 'node_modules' folder.": {
"category": "Error",
"code": 8036
},

"Declaration emit for this file requires using private name '{0}'. An explicit type annotation may unblock declaration emit.": {
"category": "Error",
"code": 9005
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8822,6 +8822,7 @@ namespace ts {
readonly includeInlayPropertyDeclarationTypeHints?: boolean;
readonly includeInlayFunctionLikeReturnTypeHints?: boolean;
readonly includeInlayEnumMemberValueHints?: boolean;
readonly allowRenameOfImportPath?: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ironically, I think this always belonged in UserPreferences, and providePrefixAndSuffixTextForRename always should have been a per-request option. 🙃 Baby steps in the right direction...

}

/** Represents a bigint literal value without requiring bigint support */
Expand Down
6 changes: 3 additions & 3 deletions src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ namespace ts.server {
return notImplemented();
}

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

Expand Down Expand Up @@ -500,14 +500,14 @@ namespace ts.server {
// User preferences have to be set through the `Configure` command
this.configure({ providePrefixAndSuffixTextForRename });
// Options argument is not used, so don't pass in options
this.getRenameInfo(fileName, position, /*options*/{}, findInStrings, findInComments);
this.getRenameInfo(fileName, position, /*preferences*/{}, findInStrings, findInComments);
// Restore previous user preferences
if (this.preferences) {
this.configure(this.preferences);
}
}
else {
this.getRenameInfo(fileName, position, /*options*/{}, findInStrings, findInComments);
this.getRenameInfo(fileName, position, /*preferences*/{}, findInStrings, findInComments);
}
}

Expand Down
24 changes: 17 additions & 7 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1430,14 +1430,17 @@ namespace FourSlash {
contextRangeDelta?: number
contextRangeId?: string;
}
const { findInStrings = false, findInComments = false, ranges = this.getRanges(), providePrefixAndSuffixTextForRename = true } = ts.isArray(options) ? { findInStrings: false, findInComments: false, ranges: options, providePrefixAndSuffixTextForRename: true } : options;
const { findInStrings = false, findInComments = false, ranges = this.getRanges(), providePrefixAndSuffixTextForRename = true } =
ts.isArray(options)
? { findInStrings: false, findInComments: false, ranges: options, providePrefixAndSuffixTextForRename: true }
: options;

const _startRanges = toArray(startRanges);
assert(_startRanges.length);
for (const startRange of _startRanges) {
this.goToRangeStart(startRange);

const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition);
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, { providePrefixAndSuffixTextForRename });
if (!renameInfo.canRename) {
this.raiseError("Expected rename to succeed, but it actually failed.");
break;
Expand Down Expand Up @@ -1630,8 +1633,15 @@ namespace FourSlash {
}
}

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 {
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, renameInfoOptions || { allowRenameOfImportPath: true });
public verifyRenameInfoSucceeded(
displayName: string | undefined,
fullDisplayName: string | undefined,
kind: string | undefined,
kindModifiers: string | undefined,
fileToRename: string | undefined,
expectedRange: Range | undefined,
preferences: ts.UserPreferences | undefined): void {
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, preferences || { allowRenameOfImportPath: true });
if (!renameInfo.canRename) {
throw this.raiseError("Rename did not succeed");
}
Expand All @@ -1656,9 +1666,9 @@ namespace FourSlash {
}
}

public verifyRenameInfoFailed(message?: string, allowRenameOfImportPath?: boolean) {
allowRenameOfImportPath = allowRenameOfImportPath === undefined ? true : allowRenameOfImportPath;
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, { allowRenameOfImportPath });
public verifyRenameInfoFailed(message?: string, preferences?: ts.UserPreferences) {
const allowRenameOfImportPath = preferences?.allowRenameOfImportPath === undefined ? true : preferences.allowRenameOfImportPath;
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, { ...preferences, allowRenameOfImportPath });
if (renameInfo.canRename) {
throw this.raiseError("Rename was expected to fail");
}
Expand Down
19 changes: 13 additions & 6 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -556,12 +556,19 @@ namespace FourSlashInterface {
this.state.replaceWithSemanticClassifications(format);
}

public renameInfoSucceeded(displayName?: string, fullDisplayName?: string, kind?: string, kindModifiers?: string, fileToRename?: string, expectedRange?: FourSlash.Range, options?: ts.RenameInfoOptions) {
this.state.verifyRenameInfoSucceeded(displayName, fullDisplayName, kind, kindModifiers, fileToRename, expectedRange, options);
}

public renameInfoFailed(message?: string, allowRenameOfImportPath?: boolean) {
this.state.verifyRenameInfoFailed(message, allowRenameOfImportPath);
public renameInfoSucceeded(
displayName?: string,
fullDisplayName?: string,
kind?: string,
kindModifiers?: string,
fileToRename?: string,
expectedRange?: FourSlash.Range,
preferences?: ts.UserPreferences) {
this.state.verifyRenameInfoSucceeded(displayName, fullDisplayName, kind, kindModifiers, fileToRename, expectedRange, preferences);
}

public renameInfoFailed(message?: string, preferences?: ts.UserPreferences) {
this.state.verifyRenameInfoFailed(message, preferences);
}

public renameLocations(startRanges: ArrayOrSingle<FourSlash.Range>, options: RenameLocationsOptions) {
Expand Down
4 changes: 2 additions & 2 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,8 +511,8 @@ namespace Harness.LanguageService {
getSignatureHelpItems(fileName: string, position: number, options: ts.SignatureHelpItemsOptions | undefined): ts.SignatureHelpItems {
return unwrapJSONCallResult(this.shim.getSignatureHelpItems(fileName, position, options));
}
getRenameInfo(fileName: string, position: number, options?: ts.RenameInfoOptions): ts.RenameInfo {
return unwrapJSONCallResult(this.shim.getRenameInfo(fileName, position, options));
getRenameInfo(fileName: string, position: number, preferences: ts.UserPreferences): ts.RenameInfo {
return unwrapJSONCallResult(this.shim.getRenameInfo(fileName, position, preferences));
}
getSmartSelectionRange(fileName: string, position: number): ts.SelectionRange {
return unwrapJSONCallResult(this.shim.getSmartSelectionRange(fileName, position));
Expand Down
8 changes: 5 additions & 3 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1698,7 +1698,8 @@ namespace ts.server {
private getRenameInfo(args: protocol.FileLocationRequestArgs): RenameInfo {
const { file, project } = this.getFileAndProject(args);
const position = this.getPositionInFile(args, file);
return project.getLanguageService().getRenameInfo(file, position, { allowRenameOfImportPath: this.getPreferences(file).allowRenameOfImportPath });
const preferences = this.getPreferences(file);
return project.getLanguageService().getRenameInfo(file, position, preferences);
}

private getProjects(args: protocol.FileRequestArgs, getScriptInfoEnsuringProjectsUptoDate?: boolean, ignoreNoProjectError?: boolean): Projects {
Expand Down Expand Up @@ -1750,8 +1751,9 @@ namespace ts.server {
const position = this.getPositionInFile(args, file);
const projects = this.getProjects(args);
const defaultProject = this.getDefaultProject(args);
const preferences = this.getPreferences(file);
const renameInfo: protocol.RenameInfo = this.mapRenameInfo(
defaultProject.getLanguageService().getRenameInfo(file, position, { allowRenameOfImportPath: this.getPreferences(file).allowRenameOfImportPath }), Debug.checkDefined(this.projectService.getScriptInfo(file)));
defaultProject.getLanguageService().getRenameInfo(file, position, preferences), Debug.checkDefined(this.projectService.getScriptInfo(file)));

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

Expand All @@ -1761,7 +1763,7 @@ namespace ts.server {
{ fileName: args.file, pos: position },
!!args.findInStrings,
!!args.findInComments,
this.getPreferences(file)
preferences,
);
if (!simplifiedResult) return locations;
return { info: renameInfo, locs: this.toSpanGroups(locations) };
Expand Down
68 changes: 64 additions & 4 deletions src/services/rename.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
/* @internal */
namespace ts.Rename {
export function getRenameInfo(program: Program, sourceFile: SourceFile, position: number, options?: RenameInfoOptions): RenameInfo {
export function getRenameInfo(program: Program, sourceFile: SourceFile, position: number, preferences: UserPreferences): RenameInfo {
const node = getAdjustedRenameLocation(getTouchingPropertyName(sourceFile, position));
if (nodeIsEligibleForRename(node)) {
const renameInfo = getRenameInfoForNode(node, program.getTypeChecker(), sourceFile, program, options);
const renameInfo = getRenameInfoForNode(node, program.getTypeChecker(), sourceFile, program, preferences);
if (renameInfo) {
return renameInfo;
}
}
return getRenameInfoError(Diagnostics.You_cannot_rename_this_element);
}

function getRenameInfoForNode(node: Node, typeChecker: TypeChecker, sourceFile: SourceFile, program: Program, options?: RenameInfoOptions): RenameInfo | undefined {
function getRenameInfoForNode(
node: Node,
typeChecker: TypeChecker,
sourceFile: SourceFile,
program: Program,
preferences: UserPreferences): RenameInfo | undefined {
const symbol = typeChecker.getSymbolAtLocation(node);
if (!symbol) {
if (isStringLiteralLike(node)) {
Expand Down Expand Up @@ -43,7 +48,13 @@ namespace ts.Rename {
}

if (isStringLiteralLike(node) && tryGetImportFromModuleSpecifier(node)) {
return options && options.allowRenameOfImportPath ? getRenameInfoForModule(node, sourceFile, symbol) : undefined;
return preferences.allowRenameOfImportPath ? getRenameInfoForModule(node, sourceFile, symbol) : undefined;
}

// Disallow rename for elements that would rename across `*/node_modules/*` packages.
const wouldRenameNodeModules = wouldRenameInOtherNodeModules(sourceFile, symbol, typeChecker, preferences);
if (wouldRenameNodeModules) {
return getRenameInfoError(wouldRenameNodeModules);
}

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

function wouldRenameInOtherNodeModules(
originalFile: SourceFile,
symbol: Symbol,
checker: TypeChecker,
preferences: UserPreferences
): DiagnosticMessage | undefined {
if (!preferences.providePrefixAndSuffixTextForRename && symbol.flags & SymbolFlags.Alias) {
const importSpecifier = symbol.declarations && find(symbol.declarations, decl => isImportSpecifier(decl));
if (importSpecifier && !(importSpecifier as ImportSpecifier).propertyName) {
symbol = checker.getAliasedSymbol(symbol);
}
}
const { declarations } = symbol;
if (!declarations) {
return undefined;
}
const originalPackage = getPackagePathComponents(originalFile.path);
if (originalPackage === undefined) { // original source file is not in node_modules
if (some(declarations, declaration => isInsideNodeModules(declaration.getSourceFile().path))) {
return Diagnostics.You_cannot_rename_elements_that_are_defined_in_a_node_modules_folder;
}
else {
return undefined;
}
}
// original source file is in node_modules
for (const declaration of declarations) {
const declPackage = getPackagePathComponents(declaration.getSourceFile().path);
if (declPackage) {
const length = Math.min(originalPackage.length, declPackage.length);
for (let i = 0; i <= length; i++) {
if (compareStringsCaseSensitive(originalPackage[i], declPackage[i]) !== Comparison.EqualTo) {
return Diagnostics.You_cannot_rename_elements_that_are_defined_in_another_node_modules_folder;
}
}
}
}
return undefined;
}

function getPackagePathComponents(filePath: Path): string[] | undefined {
const components = getPathComponents(filePath);
const nodeModulesIdx = components.lastIndexOf("node_modules");
if (nodeModulesIdx === -1) {
return undefined;
}
return components.slice(0, nodeModulesIdx + 2);
}

function getRenameInfoForModule(node: StringLiteralLike, sourceFile: SourceFile, moduleSymbol: Symbol): RenameInfo | undefined {
if (!isExternalModuleNameRelative(node.text)) {
return getRenameInfoError(Diagnostics.You_cannot_rename_a_module_via_a_global_import);
Expand Down
4 changes: 2 additions & 2 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2537,9 +2537,9 @@ namespace ts {
}
}

function getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): RenameInfo {
function getRenameInfo(fileName: string, position: number, preferences: UserPreferences | RenameInfoOptions | undefined): RenameInfo {
synchronizeHostData();
return Rename.getRenameInfo(program, getValidSourceFile(fileName), position, options);
return Rename.getRenameInfo(program, getValidSourceFile(fileName), position, preferences || {});
}

function getRefactorContext(file: SourceFile, positionOrRange: number | TextRange, preferences: UserPreferences, formatOptions?: FormatCodeSettings, triggerReason?: RefactorTriggerReason, kind?: string): RefactorContext {
Expand Down
6 changes: 3 additions & 3 deletions src/services/shims.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ namespace ts {
* Returns a JSON-encoded value of the type:
* { canRename: boolean, localizedErrorMessage: string, displayName: string, fullDisplayName: string, kind: string, kindModifiers: string, triggerSpan: { start; length } }
*/
getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): string;
getRenameInfo(fileName: string, position: number, preferences: UserPreferences): string;
getSmartSelectionRange(fileName: string, position: number): string;

/**
Expand Down Expand Up @@ -855,10 +855,10 @@ namespace ts {
);
}

public getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): string {
public getRenameInfo(fileName: string, position: number, preferences: UserPreferences): string {
return this.forwardJSONCall(
`getRenameInfo('${fileName}', ${position})`,
() => this.languageService.getRenameInfo(fileName, position, options)
() => this.languageService.getRenameInfo(fileName, position, preferences)
);
}

Expand Down
10 changes: 8 additions & 2 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,10 @@ namespace ts {

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

getRenameInfo(fileName: string, position: number, preferences: UserPreferences): RenameInfo;
/** @deprecated Use the signature with `UserPreferences` instead. */
getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): RenameInfo;

findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): readonly RenameLocation[] | undefined;

getSmartSelectionRange(fileName: string, position: number): SelectionRange;
Expand Down Expand Up @@ -944,7 +947,7 @@ namespace ts {
Remove = "remove",
}

/* @deprecated - consider using EditorSettings instead */
/** @deprecated - consider using EditorSettings instead */
export interface EditorOptions {
BaseIndentSize?: number;
IndentSize: number;
Expand All @@ -965,7 +968,7 @@ namespace ts {
trimTrailingWhitespace?: boolean;
}

/* @deprecated - consider using FormatCodeSettings instead */
/** @deprecated - consider using FormatCodeSettings instead */
export interface FormatCodeOptions extends EditorOptions {
InsertSpaceAfterCommaDelimiter: boolean;
InsertSpaceAfterSemicolonInForStatements: boolean;
Expand Down Expand Up @@ -1135,6 +1138,9 @@ namespace ts {
localizedErrorMessage: string;
}

/**
* @deprecated Use `UserPreferences` instead.
*/
export interface RenameInfoOptions {
readonly allowRenameOfImportPath?: boolean;
}
Expand Down
Loading