Skip to content

Commit 4d50d84

Browse files
atscottKeen Yee Liau
authored andcommitted
feat: Add renaming capability
This commit adds rename capability to the Angular Language Service extension. It's important to note that vscode only uses results from a single rename provider. We cannot currently ensure that Angular takes precedence over TypeScript when renaming from TS files (see microsoft/vscode#115354) so this feature is only available in _external_ template files (it is neither available for inline templates nor other locations in TS files).
1 parent 9d78720 commit 4d50d84

File tree

7 files changed

+231
-10
lines changed

7 files changed

+231
-10
lines changed

integration/lsp/ivy_spec.ts

Lines changed: 143 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {URI} from 'vscode-uri';
1212
import {ProjectLanguageService, ProjectLanguageServiceParams} from '../../common/notifications';
1313
import {NgccProgress, NgccProgressToken, NgccProgressType} from '../../common/progress';
1414

15-
import {APP_COMPONENT, createConnection, FOO_TEMPLATE, initializeServer, openTextDocument} from './test_utils';
15+
import {APP_COMPONENT, createConnection, FOO_COMPONENT, FOO_TEMPLATE, initializeServer, openTextDocument} from './test_utils';
1616

1717
describe('Angular Ivy language server', () => {
1818
jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; /* 10 seconds */
@@ -117,6 +117,148 @@ describe('Angular Ivy language server', () => {
117117
value: 'declare (property) NgIf<boolean>.ngIf: boolean',
118118
});
119119
});
120+
121+
describe('renaming', () => {
122+
describe('from template files', () => {
123+
beforeEach(async () => {
124+
openTextDocument(client, FOO_TEMPLATE);
125+
const languageServiceEnabled = await waitForNgcc(client);
126+
expect(languageServiceEnabled).toBeTrue();
127+
});
128+
129+
it('should handle prepare rename request for property read', async () => {
130+
const response = await client.sendRequest(lsp.PrepareRenameRequest.type, {
131+
textDocument: {
132+
uri: `file://${FOO_TEMPLATE}`,
133+
},
134+
position: {line: 0, character: 3},
135+
}) as {range: lsp.Range, placeholder: string};
136+
expect(response.range).toEqual({
137+
start: {line: 0, character: 2},
138+
end: {line: 0, character: 7},
139+
});
140+
expect(response.placeholder).toEqual('title');
141+
});
142+
143+
const expectedRenameInComponent = {
144+
range: {
145+
start: {line: 6, character: 2},
146+
end: {line: 6, character: 7},
147+
},
148+
newText: 'subtitle',
149+
};
150+
const expectedRenameInTemplate = {
151+
range: {
152+
start: {line: 0, character: 2},
153+
end: {line: 0, character: 7},
154+
},
155+
newText: 'subtitle',
156+
};
157+
158+
it('should handle rename request for property read', async () => {
159+
const response = await client.sendRequest(lsp.RenameRequest.type, {
160+
textDocument: {
161+
uri: `file://${FOO_TEMPLATE}`,
162+
},
163+
position: {line: 0, character: 3},
164+
newName: 'subtitle'
165+
});
166+
expect(response).not.toBeNull();
167+
expect(response?.changes?.[FOO_TEMPLATE].length).toBe(1);
168+
expect(response?.changes?.[FOO_TEMPLATE]).toContain(expectedRenameInTemplate);
169+
expect(response?.changes?.[FOO_COMPONENT].length).toBe(1);
170+
expect(response?.changes?.[FOO_COMPONENT]).toContain(expectedRenameInComponent);
171+
});
172+
});
173+
174+
describe('from typescript files', () => {
175+
beforeEach(async () => {
176+
openTextDocument(client, APP_COMPONENT);
177+
const languageServiceEnabled = await waitForNgcc(client);
178+
expect(languageServiceEnabled).toBeTrue();
179+
});
180+
181+
it('should not be enabled, see https://github.com/microsoft/vscode/issues/115354',
182+
async () => {
183+
const prepareRenameResponse = await client.sendRequest(lsp.PrepareRenameRequest.type, {
184+
textDocument: {
185+
uri: `file://${APP_COMPONENT}`,
186+
},
187+
position: {line: 4, character: 25},
188+
}) as {range: lsp.Range, placeholder: string};
189+
expect(prepareRenameResponse).toBeNull();
190+
const renameResponse = await client.sendRequest(lsp.RenameRequest.type, {
191+
textDocument: {
192+
uri: `file://${APP_COMPONENT}`,
193+
},
194+
position: {line: 4, character: 25},
195+
newName: 'surname'
196+
});
197+
expect(renameResponse).toBeNull();
198+
});
199+
200+
xdescribe('Blocked by https://github.com/microsoft/vscode/issues/115354', () => {
201+
it('should handle prepare rename request for property read', async () => {
202+
const response = await client.sendRequest(lsp.PrepareRenameRequest.type, {
203+
textDocument: {
204+
uri: `file://${APP_COMPONENT}`,
205+
},
206+
position: {line: 4, character: 25},
207+
}) as {range: lsp.Range, placeholder: string};
208+
expect(response.range).toEqual({
209+
start: {line: 4, character: 25},
210+
end: {line: 4, character: 29},
211+
});
212+
expect(response.placeholder).toEqual('name');
213+
});
214+
215+
describe('property rename', () => {
216+
const expectedRenameInComponent = {
217+
range: {
218+
start: {line: 7, character: 2},
219+
end: {line: 7, character: 6},
220+
},
221+
newText: 'surname',
222+
};
223+
const expectedRenameInTemplate = {
224+
range: {
225+
start: {line: 4, character: 25},
226+
end: {line: 4, character: 29},
227+
},
228+
newText: 'surname',
229+
};
230+
231+
it('should handle rename request for property read in a template', async () => {
232+
const response = await client.sendRequest(lsp.RenameRequest.type, {
233+
textDocument: {
234+
uri: `file://${APP_COMPONENT}`,
235+
},
236+
position: {line: 4, character: 25},
237+
newName: 'surname'
238+
});
239+
expect(response).not.toBeNull();
240+
expect(response?.changes?.[APP_COMPONENT].length).toBe(2);
241+
expect(response?.changes?.[APP_COMPONENT]).toContain(expectedRenameInComponent);
242+
expect(response?.changes?.[APP_COMPONENT]).toContain(expectedRenameInTemplate);
243+
});
244+
245+
it('should handle rename request for property in the component', async () => {
246+
const response = await client.sendRequest(lsp.RenameRequest.type, {
247+
textDocument: {
248+
uri: `file://${APP_COMPONENT}`,
249+
},
250+
position: {line: 7, character: 4},
251+
newName: 'surname'
252+
});
253+
expect(response).not.toBeNull();
254+
expect(response?.changes?.[APP_COMPONENT].length).toBe(2);
255+
expect(response?.changes?.[APP_COMPONENT]).toContain(expectedRenameInComponent);
256+
expect(response?.changes?.[APP_COMPONENT]).toContain(expectedRenameInTemplate);
257+
});
258+
});
259+
});
260+
});
261+
});
120262
});
121263

122264
function onNgccProgress(client: MessageConnection): Promise<string> {

integration/lsp/test_utils.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const PACKAGE_ROOT = resolve(__dirname, '../../..');
1717
const PROJECT_PATH = `${PACKAGE_ROOT}/integration/project`;
1818
export const APP_COMPONENT = `${PROJECT_PATH}/app/app.component.ts`;
1919
export const FOO_TEMPLATE = `${PROJECT_PATH}/app/foo.component.html`;
20+
export const FOO_COMPONENT = `${PROJECT_PATH}/app/foo.component.ts`;
2021

2122
export interface ServerOptions {
2223
ivy: boolean;
@@ -71,10 +72,16 @@ export function initializeServer(client: MessageConnection): Promise<lsp.Initial
7172
}
7273

7374
export function openTextDocument(client: MessageConnection, filePath: string) {
75+
let languageId = 'unknown';
76+
if (filePath.endsWith('ts')) {
77+
languageId = 'typescript';
78+
} else if (filePath.endsWith('html')) {
79+
languageId = 'html';
80+
}
7481
client.sendNotification(lsp.DidOpenTextDocumentNotification.type, {
7582
textDocument: {
7683
uri: `file://${filePath}`,
77-
languageId: 'typescript',
84+
languageId,
7885
version: 1,
7986
text: fs.readFileSync(filePath, 'utf-8'),
8087
},

integration/lsp/viewengine_spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ describe('initialization', () => {
127127
typeDefinitionProvider: false,
128128
hoverProvider: true,
129129
referencesProvider: false,
130+
renameProvider: false,
130131
workspace: {
131132
workspaceFolders: {
132133
supported: true,
Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
import { Component } from '@angular/core';
1+
import {Component} from '@angular/core';
22

33
@Component({
44
selector: 'my-app',
55
template: `<h1>Hello {{name}}</h1>`,
66
})
7-
export class AppComponent { name = 'Angular'; }
7+
export class AppComponent {
8+
name = 'Angular';
9+
}

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@
133133
"test:syntaxes": "yarn compile:syntaxes-test && yarn build:syntaxes && jasmine dist/syntaxes/test/driver.js"
134134
},
135135
"dependencies": {
136-
"@angular/language-service": "11.1.1",
136+
"@angular/language-service": "v11.2.0-next.0",
137137
"typescript": "~4.1.0",
138138
"vscode-jsonrpc": "6.0.0",
139139
"vscode-languageclient": "7.0.0",
@@ -159,4 +159,4 @@
159159
"type": "git",
160160
"url": "https://github.com/angular/vscode-ng-language-service"
161161
}
162-
}
162+
}

server/src/session.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ export class Session {
118118
conn.onDefinition(p => this.onDefinition(p));
119119
conn.onTypeDefinition(p => this.onTypeDefinition(p));
120120
conn.onReferences(p => this.onReferences(p));
121+
conn.onRenameRequest(p => this.onRenameRequest(p));
122+
conn.onPrepareRename(p => this.onPrepareRename(p));
121123
conn.onHover(p => this.onHover(p));
122124
conn.onCompletion(p => this.onCompletion(p));
123125
conn.onCompletionResolve(p => this.onCompletionResolve(p));
@@ -331,6 +333,11 @@ export class Session {
331333
definitionProvider: true,
332334
typeDefinitionProvider: this.ivy,
333335
referencesProvider: this.ivy,
336+
renameProvider: this.ivy ? {
337+
// Renames should be checked and tested before being executed.
338+
prepareProvider: true,
339+
} :
340+
false,
334341
hoverProvider: true,
335342
workspace: {
336343
workspaceFolders: {supported: true},
@@ -507,6 +514,68 @@ export class Session {
507514
return this.tsDefinitionsToLspLocationLinks(definitions);
508515
}
509516

517+
private onRenameRequest(params: lsp.RenameParams): lsp.WorkspaceEdit|undefined {
518+
const lsInfo = this.getLSAndScriptInfo(params.textDocument);
519+
if (lsInfo === undefined) {
520+
return;
521+
}
522+
const {languageService, scriptInfo} = lsInfo;
523+
if (scriptInfo.scriptKind === ts.ScriptKind.TS) {
524+
// Because we cannot ensure our extension is prioritized for renames in TS files (see
525+
// https://github.com/microsoft/vscode/issues/115354) we disable renaming completely so we can
526+
// provide consistent expectations.
527+
return;
528+
}
529+
const offset = lspPositionToTsPosition(scriptInfo, params.position);
530+
const renameLocations = languageService.findRenameLocations(
531+
scriptInfo.fileName, offset, /*findInStrings*/ false, /*findInComments*/ false);
532+
if (renameLocations === undefined) {
533+
return;
534+
}
535+
536+
const changes = renameLocations.reduce((changes, location) => {
537+
if (changes[location.fileName] === undefined) {
538+
changes[location.fileName] = [];
539+
}
540+
const fileEdits = changes[location.fileName];
541+
542+
const lsInfo = this.getLSAndScriptInfo(location.fileName);
543+
if (lsInfo === undefined) {
544+
return changes;
545+
}
546+
const range = tsTextSpanToLspRange(lsInfo.scriptInfo, location.textSpan);
547+
fileEdits.push({range, newText: params.newName});
548+
return changes;
549+
}, {} as {[uri: string]: lsp.TextEdit[]});
550+
551+
return {changes};
552+
}
553+
554+
private onPrepareRename(params: lsp.PrepareRenameParams):
555+
{range: lsp.Range, placeholder: string}|undefined {
556+
const lsInfo = this.getLSAndScriptInfo(params.textDocument);
557+
if (lsInfo === undefined) {
558+
return;
559+
}
560+
const {languageService, scriptInfo} = lsInfo;
561+
if (scriptInfo.scriptKind === ts.ScriptKind.TS) {
562+
// Because we cannot ensure our extension is prioritized for renames in TS files (see
563+
// https://github.com/microsoft/vscode/issues/115354) we disable renaming completely so we can
564+
// provide consistent expectations.
565+
return;
566+
}
567+
const offset = lspPositionToTsPosition(scriptInfo, params.position);
568+
const renameInfo = languageService.getRenameInfo(scriptInfo.fileName, offset);
569+
if (!renameInfo.canRename) {
570+
return undefined;
571+
}
572+
const range = tsTextSpanToLspRange(scriptInfo, renameInfo.triggerSpan);
573+
return {
574+
range,
575+
placeholder: renameInfo.displayName,
576+
};
577+
}
578+
510579
private onReferences(params: lsp.TextDocumentPositionParams): lsp.Location[]|undefined {
511580
const lsInfo = this.getLSAndScriptInfo(params.textDocument);
512581
if (lsInfo === undefined) {

yarn.lock

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
# yarn lockfile v1
33

44

5-
"@angular/language-service@11.1.1":
6-
version "11.1.1"
7-
resolved "https://registry.yarnpkg.com/@angular/language-service/-/language-service-11.1.1.tgz#f43e9541933efe5102f9ea5ad372d8d5255f9300"
8-
integrity sha512-87PYlTBBaMr0DYMYxkyeFas1qXIRYM0soNYkXC8yE+hxkGWTN15Zjk19+lx5z43++uNOiZw1mqnKTJoO46kE6A==
5+
"@angular/language-service@v11.2.0-next.0":
6+
version "11.2.0-next.0"
7+
resolved "https://registry.yarnpkg.com/@angular/language-service/-/language-service-11.2.0-next.0.tgz#092281d39b87c74cebcc3506ae8bd61fcf8dbec9"
8+
integrity sha512-+NMO2WAk932zqho1mPK4L5WLDMWukPBLe47uVQ1PPkVmAIo26pQ3y9oGxOlOYTQtWO9HDV72NazNnV5kIBX7rQ==
99

1010
"@babel/code-frame@^7.0.0":
1111
version "7.10.4"

0 commit comments

Comments
 (0)