Skip to content
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

Add user preference to opt-in to renaming import paths #29385

Merged
merged 3 commits into from
Jan 15, 2019
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
5 changes: 3 additions & 2 deletions src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,8 @@ namespace ts.server {
return notImplemented();
}

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

const request = this.processRequest<protocol.RenameRequest>(CommandNames.Rename, args);
Expand Down Expand Up @@ -428,7 +429,7 @@ namespace ts.server {
this.lastRenameEntry.inputs.position !== position ||
this.lastRenameEntry.inputs.findInStrings !== findInStrings ||
this.lastRenameEntry.inputs.findInComments !== findInComments) {
this.getRenameInfo(fileName, position, findInStrings, findInComments);
this.getRenameInfo(fileName, position, { allowRenameOfImportPath: true }, findInStrings, findInComments);
}

return this.lastRenameEntry!.locations;
Expand Down
17 changes: 9 additions & 8 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1308,8 +1308,8 @@ Actual: ${stringify(fullActual)}`);
}
}

public verifyRenameInfoSucceeded(displayName: string | undefined, fullDisplayName: string | undefined, kind: string | undefined, kindModifiers: string | undefined, fileToRename: string | undefined, expectedRange: Range | undefined): void {
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition);
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 });
if (!renameInfo.canRename) {
throw this.raiseError("Rename did not succeed");
}
Expand All @@ -1334,8 +1334,9 @@ Actual: ${stringify(fullActual)}`);
}
}

public verifyRenameInfoFailed(message?: string) {
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition);
public verifyRenameInfoFailed(message?: string, allowRenameOfImportPath?: boolean) {
allowRenameOfImportPath = allowRenameOfImportPath === undefined ? true : allowRenameOfImportPath;
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition, { allowRenameOfImportPath });
if (renameInfo.canRename) {
throw this.raiseError("Rename was expected to fail");
}
Expand Down Expand Up @@ -4091,12 +4092,12 @@ namespace FourSlashInterface {
this.state.verifySemanticClassifications(classifications);
}

public renameInfoSucceeded(displayName?: string, fullDisplayName?: string, kind?: string, kindModifiers?: string, fileToRename?: string, expectedRange?: FourSlash.Range) {
this.state.verifyRenameInfoSucceeded(displayName, fullDisplayName, kind, kindModifiers, fileToRename, expectedRange);
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) {
this.state.verifyRenameInfoFailed(message);
public renameInfoFailed(message?: string, allowRenameOfImportPath?: boolean) {
this.state.verifyRenameInfoFailed(message, allowRenameOfImportPath);
}

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 @@ -469,8 +469,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): ts.RenameInfo {
return unwrapJSONCallResult(this.shim.getRenameInfo(fileName, position));
getRenameInfo(fileName: string, position: number, options?: ts.RenameInfoOptions): ts.RenameInfo {
return unwrapJSONCallResult(this.shim.getRenameInfo(fileName, position, options));
}
findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean): ts.RenameLocation[] {
return unwrapJSONCallResult(this.shim.findRenameLocations(fileName, position, findInStrings, findInComments));
Expand Down
1 change: 1 addition & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2905,6 +2905,7 @@ namespace ts.server.protocol {
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
readonly allowTextChangesInNewFiles?: boolean;
readonly lazyConfiguredProjectsFromExternalProject?: boolean;
readonly allowRenameOfImportPath?: boolean;
uniqueiniquity marked this conversation as resolved.
Show resolved Hide resolved
}

export interface CompilerOptions {
Expand Down
5 changes: 3 additions & 2 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,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);
const preferences = this.getHostPreferences();
return project.getLanguageService().getRenameInfo(file, position, { allowRenameOfImportPath: preferences.allowRenameOfImportPath });
}

private getProjects(args: protocol.FileRequestArgs, getScriptInfoEnsuringProjectsUptoDate?: boolean, ignoreNoProjectError?: boolean): Projects {
Expand Down Expand Up @@ -1236,7 +1237,7 @@ namespace ts.server {
if (!simplifiedResult) return locations;

const defaultProject = this.getDefaultProject(args);
const renameInfo: protocol.RenameInfo = this.mapRenameInfo(defaultProject.getLanguageService().getRenameInfo(file, position), Debug.assertDefined(this.projectService.getScriptInfo(file)));
const renameInfo: protocol.RenameInfo = this.mapRenameInfo(defaultProject.getLanguageService().getRenameInfo(file, position, { allowRenameOfImportPath: this.getHostPreferences().allowRenameOfImportPath }), Debug.assertDefined(this.projectService.getScriptInfo(file)));
return { info: renameInfo, locs: this.toSpanGroups(locations) };
}

Expand Down
8 changes: 4 additions & 4 deletions src/services/rename.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
/* @internal */
namespace ts.Rename {
export function getRenameInfo(program: Program, sourceFile: SourceFile, position: number): RenameInfo {
export function getRenameInfo(program: Program, sourceFile: SourceFile, position: number, options?: RenameInfoOptions): RenameInfo {
const node = getTouchingPropertyName(sourceFile, position);
const renameInfo = node && nodeIsEligibleForRename(node)
? getRenameInfoForNode(node, program.getTypeChecker(), sourceFile, declaration => program.isSourceFileDefaultLibrary(declaration.getSourceFile()))
? getRenameInfoForNode(node, program.getTypeChecker(), sourceFile, declaration => program.isSourceFileDefaultLibrary(declaration.getSourceFile()), options)
: undefined;
return renameInfo || getRenameInfoError(Diagnostics.You_cannot_rename_this_element);
}

function getRenameInfoForNode(node: Node, typeChecker: TypeChecker, sourceFile: SourceFile, isDefinedInLibraryFile: (declaration: Node) => boolean): RenameInfo | undefined {
function getRenameInfoForNode(node: Node, typeChecker: TypeChecker, sourceFile: SourceFile, isDefinedInLibraryFile: (declaration: Node) => boolean, options?: RenameInfoOptions): RenameInfo | undefined {
const symbol = typeChecker.getSymbolAtLocation(node);
if (!symbol) return;
// Only allow a symbol to be renamed if it actually has at least one declaration.
Expand All @@ -26,7 +26,7 @@ namespace ts.Rename {
}

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

const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, node);
Expand Down
4 changes: 2 additions & 2 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2062,9 +2062,9 @@ namespace ts {
}
}

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

function getRefactorContext(file: SourceFile, positionOrRange: number | TextRange, preferences: UserPreferences, formatOptions?: FormatCodeSettings): 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 @@ -164,7 +164,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): string;
getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): string;

/**
* Returns a JSON-encoded value of the type:
Expand Down Expand Up @@ -831,10 +831,10 @@ namespace ts {
);
}

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

Expand Down
6 changes: 5 additions & 1 deletion src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ namespace ts {

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

getRenameInfo(fileName: string, position: number): RenameInfo;
getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): RenameInfo;
findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean): ReadonlyArray<RenameLocation> | undefined;

getDefinitionAtPosition(fileName: string, position: number): ReadonlyArray<DefinitionInfo> | undefined;
Expand Down Expand Up @@ -848,6 +848,10 @@ namespace ts {
localizedErrorMessage: string;
}

export interface RenameInfoOptions {
readonly allowRenameOfImportPath?: boolean;
}

export interface SignatureHelpParameter {
name: string;
documentation: SymbolDisplayPart[];
Expand Down
14 changes: 12 additions & 2 deletions src/testRunner/unittests/tsserver/rename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,18 @@ namespace ts.projectSystem {
const session = createSession(createServerHost([aTs, bTs]));
openFilesForSession([bTs], session);

const response = executeSessionRequest<protocol.RenameRequest, protocol.RenameResponse>(session, protocol.CommandTypes.Rename, protocolFileLocationFromSubstring(bTs, 'a";'));
assert.deepEqual<protocol.RenameResponseBody | undefined>(response, {
const response1 = executeSessionRequest<protocol.RenameRequest, protocol.RenameResponse>(session, protocol.CommandTypes.Rename, protocolFileLocationFromSubstring(bTs, 'a";'));
assert.deepEqual<protocol.RenameResponseBody | undefined>(response1, {
info: {
canRename: false,
localizedErrorMessage: "You cannot rename this element."
},
locs: [{ file: bTs.path, locs: [protocolRenameSpanFromSubstring(bTs.content, "./a")] }],
});

session.getProjectService().setHostConfiguration({ preferences: { allowRenameOfImportPath: true } });
const response2 = executeSessionRequest<protocol.RenameRequest, protocol.RenameResponse>(session, protocol.CommandTypes.Rename, protocolFileLocationFromSubstring(bTs, 'a";'));
assert.deepEqual<protocol.RenameResponseBody | undefined>(response2, {
info: {
canRename: true,
fileToRename: aTs.path,
Expand Down
6 changes: 5 additions & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4706,7 +4706,7 @@ declare namespace ts {
getNameOrDottedNameSpan(fileName: string, startPos: number, endPos: number): TextSpan | undefined;
getBreakpointStatementAtPosition(fileName: string, position: number): TextSpan | undefined;
getSignatureHelpItems(fileName: string, position: number, options: SignatureHelpItemsOptions | undefined): SignatureHelpItems | undefined;
getRenameInfo(fileName: string, position: number): RenameInfo;
getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): RenameInfo;
findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean): ReadonlyArray<RenameLocation> | undefined;
getDefinitionAtPosition(fileName: string, position: number): ReadonlyArray<DefinitionInfo> | undefined;
getDefinitionAndBoundSpan(fileName: string, position: number): DefinitionInfoAndBoundSpan | undefined;
Expand Down Expand Up @@ -5150,6 +5150,9 @@ declare namespace ts {
canRename: false;
localizedErrorMessage: string;
}
interface RenameInfoOptions {
readonly allowRenameOfImportPath?: boolean;
}
interface SignatureHelpParameter {
name: string;
documentation: SymbolDisplayPart[];
Expand Down Expand Up @@ -7923,6 +7926,7 @@ declare namespace ts.server.protocol {
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
readonly allowTextChangesInNewFiles?: boolean;
readonly lazyConfiguredProjectsFromExternalProject?: boolean;
readonly allowRenameOfImportPath?: boolean;
}
interface CompilerOptions {
allowJs?: boolean;
Expand Down
5 changes: 4 additions & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4706,7 +4706,7 @@ declare namespace ts {
getNameOrDottedNameSpan(fileName: string, startPos: number, endPos: number): TextSpan | undefined;
getBreakpointStatementAtPosition(fileName: string, position: number): TextSpan | undefined;
getSignatureHelpItems(fileName: string, position: number, options: SignatureHelpItemsOptions | undefined): SignatureHelpItems | undefined;
getRenameInfo(fileName: string, position: number): RenameInfo;
getRenameInfo(fileName: string, position: number, options?: RenameInfoOptions): RenameInfo;
findRenameLocations(fileName: string, position: number, findInStrings: boolean, findInComments: boolean): ReadonlyArray<RenameLocation> | undefined;
getDefinitionAtPosition(fileName: string, position: number): ReadonlyArray<DefinitionInfo> | undefined;
getDefinitionAndBoundSpan(fileName: string, position: number): DefinitionInfoAndBoundSpan | undefined;
Expand Down Expand Up @@ -5150,6 +5150,9 @@ declare namespace ts {
canRename: false;
localizedErrorMessage: string;
}
interface RenameInfoOptions {
readonly allowRenameOfImportPath?: boolean;
}
interface SignatureHelpParameter {
name: string;
documentation: SymbolDisplayPart[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,5 @@ verify.renameLocations(r2, [r0, r1, r2]);
for (const range of [r3b, r4b]) {
goTo.rangeStart(range);
verify.renameInfoSucceeded(/*displayName*/ "/a.ts", /*fullDisplayName*/ "/a.ts", /*kind*/ "module", /*kindModifiers*/ "", /*fileToRename*/ "/a.ts", range);
verify.renameInfoFailed("You cannot rename this element.", /*allowRenameOfImportPath*/ false);
}
4 changes: 2 additions & 2 deletions tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,8 @@ declare namespace FourSlashInterface {
text: string;
textSpan?: TextSpan;
}[]): void;
renameInfoSucceeded(displayName?: string, fullDisplayName?: string, kind?: string, kindModifiers?: string, fileToRename?: string, range?: Range): void;
renameInfoFailed(message?: string): void;
renameInfoSucceeded(displayName?: string, fullDisplayName?: string, kind?: string, kindModifiers?: string, fileToRename?: string, range?: Range, allowRenameOfImportPath?: boolean): void;
renameInfoFailed(message?: string, allowRenameOfImportPath?: boolean): void;
renameLocations(startRanges: ArrayOrSingle<Range>, options: RenameLocationsOptions): void;

/** Verify the quick info available at the current marker. */
Expand Down
1 change: 1 addition & 0 deletions tests/cases/fourslash/renameImport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ goTo.eachRange(range => {
const name = target === "dir" ? "/dir" : target === "dir/index" ? "/dir/index.ts" : "/a.ts";
const kind = target === "dir" ? "directory" : "module";
verify.renameInfoSucceeded(/*displayName*/ name, /*fullDisplayName*/ name, /*kind*/ kind, /*kindModifiers*/ "", /*fileToRename*/ name, range);
verify.renameInfoFailed("You cannot rename this element.", /*allowRenameOfImportPath*/ false);
});

goTo.marker("global");
Expand Down