Skip to content

Commit ebf5662

Browse files
atscottKeen Yee Liau
authored andcommitted
fix: disable rename feature when strictTemplates is disabled
When `strictTemplates` is disabled, we know that the compiler's view of a template is not completely "accurate". That is, the compiler's generated Type Check Blocks will not have sufficient type information to properly perform a rename operation.
1 parent 0676697 commit ebf5662

File tree

2 files changed

+56
-10
lines changed

2 files changed

+56
-10
lines changed

integration/lsp/ivy_spec.ts

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -274,16 +274,42 @@ describe('Angular Ivy language server', () => {
274274
fs.writeFileSync(TSCONFIG, originalConfig);
275275
});
276276

277-
it('should suggest strict mode', async () => {
278-
const config = JSON.parse(originalConfig);
279-
config.angularCompilerOptions.strictTemplates = false;
280-
fs.writeFileSync(TSCONFIG, JSON.stringify(config, null, 2));
281-
282-
openTextDocument(client, APP_COMPONENT);
283-
const languageServiceEnabled = await waitForNgcc(client);
284-
expect(languageServiceEnabled).toBeTrue();
285-
const configFilePath = await onSuggestStrictMode(client);
286-
expect(configFilePath.endsWith('integration/project/tsconfig.json')).toBeTrue();
277+
describe('strictTemplates: false', () => {
278+
beforeEach(async () => {
279+
const config = JSON.parse(originalConfig);
280+
config.angularCompilerOptions.strictTemplates = false;
281+
fs.writeFileSync(TSCONFIG, JSON.stringify(config, null, 2));
282+
283+
openTextDocument(client, APP_COMPONENT);
284+
const languageServiceEnabled = await waitForNgcc(client);
285+
expect(languageServiceEnabled).toBeTrue();
286+
});
287+
288+
it('should suggest strict mode', async () => {
289+
const configFilePath = await onSuggestStrictMode(client);
290+
expect(configFilePath.endsWith('integration/project/tsconfig.json')).toBeTrue();
291+
});
292+
293+
it('should disable renaming when strict mode is disabled', async () => {
294+
await onSuggestStrictMode(client);
295+
296+
const prepareRenameResponse = await client.sendRequest(lsp.PrepareRenameRequest.type, {
297+
textDocument: {
298+
uri: `file://${APP_COMPONENT}`,
299+
},
300+
position: {line: 4, character: 25},
301+
}) as {range: lsp.Range, placeholder: string};
302+
expect(prepareRenameResponse).toBeNull();
303+
304+
const renameResponse = await client.sendRequest(lsp.RenameRequest.type, {
305+
textDocument: {
306+
uri: `file://${APP_COMPONENT}`,
307+
},
308+
position: {line: 4, character: 25},
309+
newName: 'surname'
310+
});
311+
expect(renameResponse).toBeNull();
312+
});
287313
});
288314
});
289315
});

server/src/session.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ export class Session {
4949
private readonly logToConsole: boolean;
5050
private diagnosticsTimeout: NodeJS.Timeout|null = null;
5151
private isProjectLoading = false;
52+
/**
53+
* Tracks which `ts.server.Project`s have the renaming capability disabled.
54+
*
55+
* If we detect the compiler options diagnostic that suggests enabling strict mode, we want to
56+
* disable renaming because we know that there are many cases where it will not work correctly.
57+
*/
58+
private renameDisabledProjects: WeakSet<ts.server.Project> = new WeakSet();
5259

5360
constructor(options: SessionOptions) {
5461
this.logger = options.logger;
@@ -217,6 +224,9 @@ export class Session {
217224
configFilePath,
218225
message: suggestStrictModeDiag.messageText,
219226
});
227+
this.renameDisabledProjects.add(project);
228+
} else {
229+
this.renameDisabledProjects.delete(project);
220230
}
221231
}
222232

@@ -543,6 +553,11 @@ export class Session {
543553
// provide consistent expectations.
544554
return;
545555
}
556+
const project = this.getDefaultProjectForScriptInfo(scriptInfo);
557+
if (project === undefined || this.renameDisabledProjects.has(project)) {
558+
return;
559+
}
560+
546561
const offset = lspPositionToTsPosition(scriptInfo, params.position);
547562
const renameLocations = languageService.findRenameLocations(
548563
scriptInfo.fileName, offset, /*findInStrings*/ false, /*findInComments*/ false);
@@ -581,6 +596,11 @@ export class Session {
581596
// provide consistent expectations.
582597
return;
583598
}
599+
const project = this.getDefaultProjectForScriptInfo(scriptInfo);
600+
if (project === undefined || this.renameDisabledProjects.has(project)) {
601+
return;
602+
}
603+
584604
const offset = lspPositionToTsPosition(scriptInfo, params.position);
585605
const renameInfo = languageService.getRenameInfo(scriptInfo.fileName, offset);
586606
if (!renameInfo.canRename) {

0 commit comments

Comments
 (0)