From e1e1db3c011cd162f32eb6b3f10730cef2e8f7c6 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Tue, 19 Jan 2021 22:09:59 -0800 Subject: [PATCH] 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, https://github.com/angular/angular/pull/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 https://github.com/angular/vscode-ng-language-service/issues/1063 PR Close #40492 --- .../language-service/src/typescript_host.ts | 13 +++++++---- .../test/typescript_host_spec.ts | 22 ++++++++++++++++--- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts index 5bc657e0cc..934501daf9 100644 --- a/packages/language-service/src/typescript_host.ts +++ b/packages/language-service/src/typescript_host.ts @@ -62,7 +62,12 @@ export class TypeScriptServiceHost implements LanguageServiceHost { private readonly staticSymbolResolver: StaticSymbolResolver; private readonly staticSymbolCache = new StaticSymbolCache(); - private readonly fileToComponent = new Map(); + /** + * Key of the `fileToComponent` map must be TS internal normalized path (path + * separator must be `/`), value of the map is the StaticSymbol for the + * Component class declaration. + */ + private readonly fileToComponent = new Map(); private readonly collectedErrors = new Map(); private readonly fileVersions = new Map(); private readonly urlResolver: UrlResolver; @@ -165,7 +170,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { /** * Return all known external templates. */ - getExternalTemplates(): string[] { + getExternalTemplates(): ts.server.NormalizedPath[] { return [...this.fileToComponent.keys()]; } @@ -210,7 +215,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { const templateName = this.urlResolver.resolve( this.reflector.componentModuleUrl(directive.reference), metadata.template.templateUrl); - this.fileToComponent.set(templateName, directive.reference); + this.fileToComponent.set(tss.server.toNormalizedPath(templateName), directive.reference); } } } @@ -417,7 +422,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { } const source = snapshot.getText(0, snapshot.getLength()); // Next find the component class symbol - const classSymbol = this.fileToComponent.get(fileName); + const classSymbol = this.fileToComponent.get(tss.server.toNormalizedPath(fileName)); if (!classSymbol) { return; } diff --git a/packages/language-service/test/typescript_host_spec.ts b/packages/language-service/test/typescript_host_spec.ts index f1bab4d1bc..ee4b7ed69c 100644 --- a/packages/language-service/test/typescript_host_spec.ts +++ b/packages/language-service/test/typescript_host_spec.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import * as path from 'path'; import * as ts from 'typescript'; import {TypeScriptServiceHost} from '../src/typescript_host'; @@ -114,7 +115,7 @@ describe('TypeScriptServiceHost', () => { const tsLS = ts.createLanguageService(tsLSHost); const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS); ngLSHost.getAnalyzedModules(); - expect(ngLSHost.getExternalTemplates()).toContain('/app/#inner/inner.html'); + expect(ngLSHost.getExternalTemplates() as string[]).toContain('/app/#inner/inner.html'); }); // https://github.com/angular/angular/issues/32301 @@ -238,7 +239,7 @@ describe('TypeScriptServiceHost', () => { let content = ` import {CommonModule} from '@angular/common'; import {NgModule} from '@angular/core'; - + @NgModule({ entryComponents: [CommonModule], }) @@ -256,7 +257,7 @@ describe('TypeScriptServiceHost', () => { content = ` import {CommonModule} from '@angular/common'; import {NgModule} from '@angular/core'; - + @NgModule({}) export class AppModule {} `; @@ -265,4 +266,19 @@ describe('TypeScriptServiceHost', () => { newModules = ngLSHost.getAnalyzedModules(); expect(newModules.ngModules.length).toBeGreaterThan(0); }); + + it('should normalize path on Windows', () => { + // Spy on the `path.resolve()` method called by the URL resolver and mimic + // behavior on Windows. + spyOn(path, 'resolve').and.callFake((...pathSegments: string[]) => { + return path.win32.resolve(...pathSegments); + }); + const tsLSHost = new MockTypescriptHost(['/app/main.ts']); + const tsLS = ts.createLanguageService(tsLSHost); + const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS); + ngLSHost.getAnalyzedModules(); + const externalTemplates: string[] = ngLSHost.getExternalTemplates(); + // External templates should be normalized. + expect(externalTemplates).toContain('/app/test.ng'); + }); });