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);
    // ...
  }
```
9fca9c6651/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
This commit is contained in:
Keen Yee Liau 2021-01-19 22:09:59 -08:00 committed by Andrew Kushnir
parent 402e2e6189
commit e1e1db3c01
2 changed files with 28 additions and 7 deletions

View File

@ -62,7 +62,12 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
private readonly staticSymbolResolver: StaticSymbolResolver;
private readonly staticSymbolCache = new StaticSymbolCache();
private readonly fileToComponent = new Map<string, StaticSymbol>();
/**
* 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<ts.server.NormalizedPath, StaticSymbol>();
private readonly collectedErrors = new Map<string, any[]>();
private readonly fileVersions = new Map<string, string>();
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;
}

View File

@ -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');
});
});