Skip to content

Commit b8e47e2

Browse files
kyliauAndrewKushnir
authored andcommitted
fix(language-service): Paths on Windows should be normalized (#40492)
Many `ts.LanguageService` APIs accept a filename, for example ```ts getQuickInfoAtPosition(fileName: string, position: number) ``` The requirement is that `fileName` is agnostic to the platform (Linux, Mac, Windows, etc), and is always normalized to TypeScript's internal `NormalizedPath`. This is evident from the way these APIs are called from the language server: ```ts private onHover(params: lsp.TextDocumentPositionParams) { const lsInfo = this.getLSAndScriptInfo(params.textDocument); if (lsInfo === undefined) { return; } const {languageService, scriptInfo} = lsInfo; const offset = lspPositionToTsPosition(scriptInfo, params.position); const info = languageService.getQuickInfoAtPosition(scriptInfo.fileName, offset); // ... } ``` https://github.com/angular/vscode-ng-language-service/blob/9fca9c66510974c26d5c21b31adb9fa39ac0a38a/server/src/session.ts#L594 Here `scriptInfo.fileName` is always a `ts.server.NormalizedPath`. However, #39917 accidentally leaked the platform-specific paths, and caused a mismatch between the incoming paths and the paths stored in the internal data structure `fileToComponent`. This PR fixes the bug by always normalizing the paths, and updating the type to reflect the format of the underlying data. Fixes angular/vscode-ng-language-service#1063 PR Close #40492
1 parent 2731a4b commit b8e47e2

File tree

2 files changed

+28
-7
lines changed

2 files changed

+28
-7
lines changed

packages/language-service/src/typescript_host.ts

+9-4
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,12 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
6262
private readonly staticSymbolResolver: StaticSymbolResolver;
6363

6464
private readonly staticSymbolCache = new StaticSymbolCache();
65-
private readonly fileToComponent = new Map<string, StaticSymbol>();
65+
/**
66+
* Key of the `fileToComponent` map must be TS internal normalized path (path
67+
* separator must be `/`), value of the map is the StaticSymbol for the
68+
* Component class declaration.
69+
*/
70+
private readonly fileToComponent = new Map<ts.server.NormalizedPath, StaticSymbol>();
6671
private readonly collectedErrors = new Map<string, any[]>();
6772
private readonly fileVersions = new Map<string, string>();
6873
private readonly urlResolver: UrlResolver;
@@ -165,7 +170,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
165170
/**
166171
* Return all known external templates.
167172
*/
168-
getExternalTemplates(): string[] {
173+
getExternalTemplates(): ts.server.NormalizedPath[] {
169174
return [...this.fileToComponent.keys()];
170175
}
171176

@@ -210,7 +215,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
210215
const templateName = this.urlResolver.resolve(
211216
this.reflector.componentModuleUrl(directive.reference),
212217
metadata.template.templateUrl);
213-
this.fileToComponent.set(templateName, directive.reference);
218+
this.fileToComponent.set(tss.server.toNormalizedPath(templateName), directive.reference);
214219
}
215220
}
216221
}
@@ -417,7 +422,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
417422
}
418423
const source = snapshot.getText(0, snapshot.getLength());
419424
// Next find the component class symbol
420-
const classSymbol = this.fileToComponent.get(fileName);
425+
const classSymbol = this.fileToComponent.get(tss.server.toNormalizedPath(fileName));
421426
if (!classSymbol) {
422427
return;
423428
}

packages/language-service/test/typescript_host_spec.ts

+19-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import * as path from 'path';
910
import * as ts from 'typescript';
1011

1112
import {TypeScriptServiceHost} from '../src/typescript_host';
@@ -114,7 +115,7 @@ describe('TypeScriptServiceHost', () => {
114115
const tsLS = ts.createLanguageService(tsLSHost);
115116
const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS);
116117
ngLSHost.getAnalyzedModules();
117-
expect(ngLSHost.getExternalTemplates()).toContain('/app/#inner/inner.html');
118+
expect(ngLSHost.getExternalTemplates() as string[]).toContain('/app/#inner/inner.html');
118119
});
119120

120121
// https://github.com/angular/angular/issues/32301
@@ -238,7 +239,7 @@ describe('TypeScriptServiceHost', () => {
238239
let content = `
239240
import {CommonModule} from '@angular/common';
240241
import {NgModule} from '@angular/core';
241-
242+
242243
@NgModule({
243244
entryComponents: [CommonModule],
244245
})
@@ -256,7 +257,7 @@ describe('TypeScriptServiceHost', () => {
256257
content = `
257258
import {CommonModule} from '@angular/common';
258259
import {NgModule} from '@angular/core';
259-
260+
260261
@NgModule({})
261262
export class AppModule {}
262263
`;
@@ -265,4 +266,19 @@ describe('TypeScriptServiceHost', () => {
265266
newModules = ngLSHost.getAnalyzedModules();
266267
expect(newModules.ngModules.length).toBeGreaterThan(0);
267268
});
269+
270+
it('should normalize path on Windows', () => {
271+
// Spy on the `path.resolve()` method called by the URL resolver and mimic
272+
// behavior on Windows.
273+
spyOn(path, 'resolve').and.callFake((...pathSegments: string[]) => {
274+
return path.win32.resolve(...pathSegments);
275+
});
276+
const tsLSHost = new MockTypescriptHost(['/app/main.ts']);
277+
const tsLS = ts.createLanguageService(tsLSHost);
278+
const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS);
279+
ngLSHost.getAnalyzedModules();
280+
const externalTemplates: string[] = ngLSHost.getExternalTemplates();
281+
// External templates should be normalized.
282+
expect(externalTemplates).toContain('/app/test.ng');
283+
});
268284
});

0 commit comments

Comments
 (0)