fix(language-service): do not treat file URIs as general URLs

In the past, the legacy (VE-based) language service would use a
`UrlResolver` instance to resolve file paths, primarily for compiler
resources like external templates. The problem with this is that the
UrlResolver is designed to resolve URLs in general, and so for a path
like `/a/b/#c`, `#c` is treated as hash/fragment rather than as part
of the path, which can lead to unexpected path resolution (f.x.,
`resolve('a/b/#c/d.ts', './d.html')` would produce `'a/b/d.html'` rather
than the expected `'a/b/#c/d.html'`).

This commit resolves the issue by using Node's `path` module to resolve
file paths directly, which aligns more with how resources are resolved
in the Ivy compiler.

The testing story here is not great, and the API for validating a file
path could be a little bit prettier/robust. However, since the VE-based
language service is going into more of a "maintenance mode" now that
there is a clear path for the Ivy-based LS moving forward, I think it is
okay not to spend too much time here.

Closes angular/vscode-ng-language-service#892
Closes angular/vscode-ng-language-service#1001

packages/language-service/src/ts_plugin.ts

@@ -34,7 +34,7 @@ export function getExternalFiles(project: tss.server.Project): string[] {
 }
 
 export function create(info: tss.server.PluginCreateInfo): tss.LanguageService {
-  const {languageService: tsLS, languageServiceHost: tsLSHost, config, project} = info;
+  const {languageService: tsLS, languageServiceHost: tsLSHost, serverHost, config, project} = info;
   // This plugin could operate under two different modes:
   // 1. TS + Angular
   //    Plugin augments TS language service to provide additional Angular
@@ -45,7 +45,7 @@ export function create(info: tss.server.PluginCreateInfo): tss.LanguageService {
   //    This effectively disables native TS features and is meant for internal
   //    use only.
   const angularOnly = config ? config.angularOnly === true : false;
-  const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS);
+  const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS, serverHost);
   const ngLS = createLanguageService(ngLSHost);
   PROJECT_MAP.set(project, ngLSHost);
 

packages/language-service/src/typescript_host.ts

@@ -6,8 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {analyzeNgModules, AotSummaryResolver, CompileDirectiveSummary, CompileMetadataResolver, CompileNgModuleMetadata, CompilePipeSummary, CompilerConfig, createOfflineCompileUrlResolver, DirectiveNormalizer, DirectiveResolver, DomElementSchemaRegistry, FormattedError, FormattedMessageChain, HtmlParser, isFormattedError, JitSummaryResolver, Lexer, NgAnalyzedModules, NgModuleResolver, Parser, ParseTreeResult, PipeResolver, ResourceLoader, StaticReflector, StaticSymbol, StaticSymbolCache, StaticSymbolResolver, TemplateParser} from '@angular/compiler';
+import {analyzeNgModules, AotSummaryResolver, CompileDirectiveSummary, CompileMetadataResolver, CompileNgModuleMetadata, CompilePipeSummary, CompilerConfig, DirectiveNormalizer, DirectiveResolver, DomElementSchemaRegistry, FormattedError, FormattedMessageChain, HtmlParser, isFormattedError, JitSummaryResolver, Lexer, NgAnalyzedModules, NgModuleResolver, Parser, ParseTreeResult, PipeResolver, ResourceLoader, StaticReflector, StaticSymbol, StaticSymbolCache, StaticSymbolResolver, TemplateParser, UrlResolver} from '@angular/compiler';
 import {SchemaMetadata, ViewEncapsulation, ɵConsole as Console} from '@angular/core';
+import * as path from 'path';
 import * as tss from 'typescript/lib/tsserverlibrary';
 
 import {createLanguageService} from './language_service';
@@ -16,12 +17,15 @@ import {ExternalTemplate, InlineTemplate} from './template';
 import {findTightestNode, getClassDeclFromDecoratorProp, getDirectiveClassLike, getPropertyAssignmentFromValue} from './ts_utils';
 import {AstResult, Declaration, DeclarationError, DiagnosticMessageChain, LanguageService, LanguageServiceHost, Span, TemplateSource} from './types';
 
+export type FileResolutionHost = Required<Pick<tss.ModuleResolutionHost, 'directoryExists'>>;
+
 /**
  * Create a `LanguageServiceHost`
  */
 export function createLanguageServiceFromTypescript(
-    host: tss.LanguageServiceHost, service: tss.LanguageService): LanguageService {
-  const ngHost = new TypeScriptServiceHost(host, service);
+    lsHost: tss.LanguageServiceHost, service: tss.LanguageService,
+    fileResolutionHost: FileResolutionHost): LanguageService {
+  const ngHost = new TypeScriptServiceHost(lsHost, service, fileResolutionHost);
   const ngServer = createLanguageService(ngHost);
   return ngServer;
 }
@@ -64,6 +68,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
   private readonly fileToComponent = new Map<string, StaticSymbol>();
   private readonly collectedErrors = new Map<string, any[]>();
   private readonly fileVersions = new Map<string, string>();
+  private readonly urlResolver: UrlResolver;
 
   private lastProgram: tss.Program|undefined = undefined;
   private analyzedModules: NgAnalyzedModules = {
@@ -72,7 +77,11 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     ngModules: [],
   };
 
-  constructor(readonly tsLsHost: tss.LanguageServiceHost, readonly tsLS: tss.LanguageService) {
+  constructor(
+      readonly tsLsHost: tss.LanguageServiceHost,
+      readonly tsLS: tss.LanguageService,
+      fileResolutionHost: FileResolutionHost,
+  ) {
     this.summaryResolver = new AotSummaryResolver(
         {
           loadSummary(_filePath: string) {
@@ -93,6 +102,14 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     this.staticSymbolResolver = new StaticSymbolResolver(
         this.reflectorHost, this.staticSymbolCache, this.summaryResolver,
         (e, filePath) => this.collectError(e, filePath));
+    this.urlResolver = {
+      resolve: (baseUrl: string, url: string) => {
+        if (fileResolutionHost.directoryExists(baseUrl)) {
+          return path.resolve(baseUrl, url);
+        }
+        return path.resolve(path.dirname(baseUrl), url);
+      }
+    };
   }
 
   // The resolver is instantiated lazily and should not be accessed directly.
@@ -125,7 +142,6 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     const pipeResolver = new PipeResolver(staticReflector);
     const elementSchemaRegistry = new DomElementSchemaRegistry();
     const resourceLoader = new DummyResourceLoader();
-    const urlResolver = createOfflineCompileUrlResolver();
     const htmlParser = new DummyHtmlParser();
     // This tracks the CompileConfig in codegen.ts. Currently these options
     // are hard-coded.
@@ -134,7 +150,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
       useJit: false,
     });
     const directiveNormalizer =
-        new DirectiveNormalizer(resourceLoader, urlResolver, htmlParser, config);
+        new DirectiveNormalizer(resourceLoader, this.urlResolver, htmlParser, config);
     this._resolver = new CompileMetadataResolver(
         config, htmlParser, moduleResolver, directiveResolver, pipeResolver,
         new JitSummaryResolver(), elementSchemaRegistry, directiveNormalizer, new Console(),
@@ -192,12 +208,11 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     }
 
     // update template references and fileToComponent
-    const urlResolver = createOfflineCompileUrlResolver();
     for (const ngModule of this.analyzedModules.ngModules) {
       for (const directive of ngModule.declaredDirectives) {
         const {metadata} = this.resolver.getNonNormalizedDirectiveMetadata(directive.reference)!;
         if (metadata.isComponent && metadata.template && metadata.template.templateUrl) {
-          const templateName = urlResolver.resolve(
+          const templateName = this.urlResolver.resolve(
               this.reflector.componentModuleUrl(directive.reference),
               metadata.template.templateUrl);
           this.fileToComponent.set(templateName, directive.reference);

packages/language-service/test/completions_spec.ts

@@ -20,7 +20,7 @@ const TEST_TEMPLATE = '/app/test.ng';
 describe('completions', () => {
   const mockHost = new MockTypescriptHost(['/app/main.ts']);
   const tsLS = ts.createLanguageService(mockHost);
-  const ngHost = new TypeScriptServiceHost(mockHost, tsLS);
+  const ngHost = new TypeScriptServiceHost(mockHost, tsLS, mockHost);
   const ngLS = createLanguageService(ngHost);
 
   beforeEach(() => {

packages/language-service/test/definitions_spec.ts

@@ -19,7 +19,7 @@ const PARSING_CASES = '/app/parsing-cases.ts';
 describe('definitions', () => {
   const mockHost = new MockTypescriptHost(['/app/main.ts']);
   const service = ts.createLanguageService(mockHost);
-  const ngHost = new TypeScriptServiceHost(mockHost, service);
+  const ngHost = new TypeScriptServiceHost(mockHost, service, mockHost);
   const ngService = createLanguageService(ngHost);
 
   beforeEach(() => {

packages/language-service/test/diagnostics_spec.ts

@@ -29,7 +29,7 @@ const APP_COMPONENT = '/app/app.component.ts';
 describe('diagnostics', () => {
   const mockHost = new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts']);
   const tsLS = ts.createLanguageService(mockHost);
-  const ngHost = new TypeScriptServiceHost(mockHost, tsLS);
+  const ngHost = new TypeScriptServiceHost(mockHost, tsLS, mockHost);
   const ngLS = createLanguageService(ngHost);
 
   beforeEach(() => {

packages/language-service/test/hover_spec.ts

@@ -19,7 +19,7 @@ const PARSING_CASES = '/app/parsing-cases.ts';
 describe('hover', () => {
   const mockHost = new MockTypescriptHost(['/app/main.ts']);
   const tsLS = ts.createLanguageService(mockHost);
-  const ngLSHost = new TypeScriptServiceHost(mockHost, tsLS);
+  const ngLSHost = new TypeScriptServiceHost(mockHost, tsLS, mockHost);
   const ngLS = createLanguageService(ngLSHost);
 
   beforeEach(() => {

packages/language-service/test/language_service_spec.ts

@@ -16,7 +16,7 @@ import {MockTypescriptHost} from './test_utils';
 describe('service without angular', () => {
   const mockHost = new MockTypescriptHost(['/app/main.ts']);
   const service = ts.createLanguageService(mockHost);
-  const ngHost = new TypeScriptServiceHost(mockHost, service);
+  const ngHost = new TypeScriptServiceHost(mockHost, service, mockHost);
   const ngService = createLanguageService(ngHost);
   const TEST_TEMPLATE = '/app/test.ng';
   mockHost.override(TEST_TEMPLATE, '<h1> ~{cursor} </h1>');

packages/language-service/test/project/app/#inner/component.ts

@@ -0,0 +1,16 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {Component} from '@angular/core';
+
+@Component({
+  selector: 'inner',
+  templateUrl: './inner.html',
+})
+export class InnerComponent {
+}

packages/language-service/test/project/app/#inner/inner.html

@@ -0,0 +1 @@
+<div>Hello</div>

packages/language-service/test/project/app/main.ts

@@ -9,13 +9,15 @@
 import {CommonModule} from '@angular/common';
 import {NgModule} from '@angular/core';
 import {FormsModule} from '@angular/forms';
+import {InnerComponent} from './#inner/component';
 import {AppComponent} from './app.component';
 import * as ParsingCases from './parsing-cases';
 
 @NgModule({
   imports: [CommonModule, FormsModule],
   declarations: [
     AppComponent,
+    InnerComponent,
     ParsingCases.CounterDirective,
     ParsingCases.HintModel,
     ParsingCases.NumberModel,

packages/language-service/test/references_spec.ts

@@ -19,7 +19,7 @@ const TEST_TEMPLATE = '/app/test.ng';
 describe('references', () => {
   const mockHost = new MockTypescriptHost(['/app/main.ts']);
   const tsLS = ts.createLanguageService(mockHost);
-  const ngHost = new TypeScriptServiceHost(mockHost, tsLS);
+  const ngHost = new TypeScriptServiceHost(mockHost, tsLS, mockHost);
   const ngLS = createLanguageService(ngHost);
 
   beforeEach(() => {

packages/language-service/test/reflector_host_spec.ts

@@ -50,7 +50,7 @@ describe('reflector_host_spec', () => {
 
     // First count is zero due to lazy instantiation of the StaticReflector
     // and MetadataResolver.
-    const ngLSHost = new TypeScriptServiceHost(mockHost, tsLS);
+    const ngLSHost = new TypeScriptServiceHost(mockHost, tsLS, mockHost);
     const firstCount = spy.calls.count();
     expect(firstCount).toBe(0);
     spy.calls.reset();

packages/language-service/test/test_utils.ts

@@ -69,7 +69,7 @@ function loadTourOfHeroes(): ReadonlyMap<string, string> {
         const value = fs.readFileSync(absPath, 'utf8');
         files.set(key, value);
       } else {
-        const key = path.join('/', filePath);
+        const key = path.join('/', path.relative(root, absPath));
         files.set(key, '[[directory]]');
         dirs.push(absPath);
       }
@@ -189,7 +189,7 @@ export class MockTypescriptHost implements ts.LanguageServiceHost {
     if (this.overrideDirectory.has(directoryName)) return true;
     const effectiveName = this.getEffectiveName(directoryName);
     if (effectiveName === directoryName) {
-      return TOH.has(directoryName);
+      return TOH.get(directoryName) === '[[directory]]';
     }
     if (effectiveName === '/' + this.node_modules) {
       return true;

packages/language-service/test/ts_plugin_spec.ts

@@ -31,7 +31,11 @@ describe('plugin', () => {
     languageService: tsLS,
     languageServiceHost: mockHost,
     project: mockProject,
-    serverHost: {} as any,
+    serverHost: {
+      directoryExists(file: string) {
+        return mockHost.directoryExists(file);
+      },
+    } as any,
     config: {},
   });
 
@@ -57,8 +61,8 @@ describe('plugin', () => {
     const compilerDiags = tsLS.getCompilerOptionsDiagnostics();
     expect(compilerDiags).toEqual([]);
     const sourceFiles = program.getSourceFiles().filter(f => !f.fileName.endsWith('.d.ts'));
-    // there are three .ts files in the test project
-    expect(sourceFiles.length).toBe(3);
+    // there are four .ts files in the test project
+    expect(sourceFiles.length).toBe(4);
     for (const {fileName} of sourceFiles) {
       const syntacticDiags = tsLS.getSyntacticDiagnostics(fileName);
       expect(syntacticDiags).toEqual([]);
@@ -132,9 +136,10 @@ describe('plugin', () => {
 
   it('should return external templates when getExternalFiles() is called', () => {
     const externalTemplates = getExternalFiles(mockProject);
-    expect(externalTemplates).toEqual([
+    expect(new Set(externalTemplates)).toEqual(new Set([
       '/app/test.ng',
-    ]);
+      '/app/#inner/inner.html',
+    ]));
   });
 });
 
@@ -145,7 +150,11 @@ describe(`with config 'angularOnly = true`, () => {
     languageService: tsLS,
     languageServiceHost: mockHost,
     project: mockProject,
-    serverHost: {} as any,
+    serverHost: {
+      directoryExists(file: string) {
+        return mockHost.directoryExists(file);
+      },
+    } as any,
     config: {
       angularOnly: true,
     },