refactor(compiler-cli): provide a host to readConfiguration (#39619)

Currently `readConfiguration` relies on the file system to perform disk
utilities needed to read determine a project configuration file and read
it. This poses a challenge for the language service, which would like to
use `readConfiguration` to watch and read configurations dependent on
extended tsconfigs (#39134). Challenges are at least twofold:

1. To test this, the langauge service would need to provide to the
   compiler a mock file system.
2. The language service uses file system utilities primarily through
   TypeScript's `Project` abstraction. In general this should correspond
   to the underlying file system, but it may differ and it is better to
   go through one channel when possible.

This patch alleviates the concern by directly providing to the compiler
a "ParseConfigurationHost" with read-only "file system"-like utilties.
For the language service, this host is derived from the project owned by
the language service.

For more discussion see
https://docs.google.com/document/d/1TrbT-m7bqyYZICmZYHjnJ7NG9Vzt5Rd967h43Qx8jw0/edit?usp=sharing

PR Close #39619

packages/compiler-cli/src/perform_compile.ts

@@ -9,7 +9,7 @@
 import {isSyntaxError, Position} from '@angular/compiler';
 import * as ts from 'typescript';
 
-import {absoluteFrom, AbsoluteFsPath, getFileSystem, relative, resolve} from '../src/ngtsc/file_system';
+import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, relative, resolve} from '../src/ngtsc/file_system';
 
 import {replaceTsWithNgInErrors} from './ngtsc/diagnostics';
 import * as api from './transformers/api';
@@ -108,6 +108,11 @@ export function formatDiagnostics(
   }
 }
 
+// TODO(ayazhafiz): split FileSystem into a ReadonlyFileSystem and make this a
+// subset of that.
+export type ParseConfigurationHost =
+    Pick<FileSystem, 'readFile'|'exists'|'lstat'|'resolve'|'join'|'dirname'|'extname'|'pwd'>;
+
 export interface ParsedConfiguration {
   project: string;
   options: api.CompilerOptions;
@@ -117,14 +122,14 @@ export interface ParsedConfiguration {
   errors: Diagnostics;
 }
 
-export function calcProjectFileAndBasePath(project: string):
+export function calcProjectFileAndBasePath(
+    project: string, host: ParseConfigurationHost = getFileSystem()):
     {projectFile: AbsoluteFsPath, basePath: AbsoluteFsPath} {
-  const fs = getFileSystem();
-  const absProject = fs.resolve(project);
-  const projectIsDir = fs.lstat(absProject).isDirectory();
-  const projectFile = projectIsDir ? fs.join(absProject, 'tsconfig.json') : absProject;
-  const projectDir = projectIsDir ? absProject : fs.dirname(absProject);
-  const basePath = fs.resolve(projectDir);
+  const absProject = host.resolve(project);
+  const projectIsDir = host.lstat(absProject).isDirectory();
+  const projectFile = projectIsDir ? host.join(absProject, 'tsconfig.json') : absProject;
+  const projectDir = projectIsDir ? absProject : host.dirname(absProject);
+  const basePath = host.resolve(projectDir);
   return {projectFile, basePath};
 }
 
@@ -139,14 +144,15 @@ export function createNgCompilerOptions(
 }
 
 export function readConfiguration(
-    project: string, existingOptions?: ts.CompilerOptions): ParsedConfiguration {
+    project: string, existingOptions?: ts.CompilerOptions,
+    host: ParseConfigurationHost = getFileSystem()): ParsedConfiguration {
   try {
-    const fs = getFileSystem();
-    const {projectFile, basePath} = calcProjectFileAndBasePath(project);
+    const {projectFile, basePath} = calcProjectFileAndBasePath(project, host);
 
     const readExtendedConfigFile =
         (configFile: string, existingConfig?: any): {config?: any, error?: ts.Diagnostic} => {
-          const {config, error} = ts.readConfigFile(configFile, ts.sys.readFile);
+          const {config, error} =
+              ts.readConfigFile(configFile, file => host.readFile(host.resolve(file)));
 
           if (error) {
             return {error};
@@ -163,12 +169,12 @@ export function readConfiguration(
           }
 
           if (config.extends) {
-            let extendedConfigPath = fs.resolve(fs.dirname(configFile), config.extends);
-            extendedConfigPath = fs.extname(extendedConfigPath) ?
+            let extendedConfigPath = host.resolve(host.dirname(configFile), config.extends);
+            extendedConfigPath = host.extname(extendedConfigPath) ?
                 extendedConfigPath :
                 absoluteFrom(`${extendedConfigPath}.json`);
 
-            if (fs.exists(extendedConfigPath)) {
+            if (host.exists(extendedConfigPath)) {
               // Call read config recursively as TypeScript only merges CompilerOptions
               return readExtendedConfigFile(extendedConfigPath, baseConfig);
             }
@@ -190,11 +196,11 @@ export function readConfiguration(
     }
     const parseConfigHost = {
       useCaseSensitiveFileNames: true,
-      fileExists: fs.exists.bind(fs),
+      fileExists: host.exists.bind(host),
       readDirectory: ts.sys.readDirectory,
       readFile: ts.sys.readFile
     };
-    const configFileName = fs.resolve(fs.pwd(), projectFile);
+    const configFileName = host.resolve(host.pwd(), projectFile);
     const parsed = ts.parseJsonConfigFileContent(
         config, parseConfigHost, basePath, existingOptions, configFileName);
     const rootNames = parsed.fileNames;

packages/language-service/ivy/BUILD.bazel

@@ -17,6 +17,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/shims",
         "//packages/compiler-cli/src/ngtsc/typecheck",
         "//packages/compiler-cli/src/ngtsc/typecheck/api",
+        "@npm//@types/node",
         "@npm//typescript",
     ],
 )

packages/language-service/ivy/language_service.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {CompilerOptions, createNgCompilerOptions} from '@angular/compiler-cli';
+import {CompilerOptions, formatDiagnostics, ParseConfigurationHost, readConfiguration} from '@angular/compiler-cli';
 import {absoluteFromSourceFile, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {TypeCheckShimGenerator} from '@angular/compiler-cli/src/ngtsc/typecheck';
 import {OptimizeFor, TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
@@ -14,7 +14,7 @@ import * as ts from 'typescript/lib/tsserverlibrary';
 
 import {CompilerFactory} from './compiler_factory';
 import {DefinitionBuilder} from './definitions';
-import {LanguageServiceAdapter} from './language_service_adapter';
+import {LanguageServiceAdapter, LSParseConfigHost} from './language_service_adapter';
 import {QuickInfoBuilder} from './quick_info';
 import {getTargetAtPosition} from './template_target';
 import {getTemplateInfoAtPosition, isTypeScriptFile} from './utils';
@@ -24,15 +24,21 @@ export class LanguageService {
   readonly compilerFactory: CompilerFactory;
   private readonly strategy: TypeCheckingProgramStrategy;
   private readonly adapter: LanguageServiceAdapter;
+  private readonly parseConfigHost: LSParseConfigHost;
 
   constructor(project: ts.server.Project, private readonly tsLS: ts.LanguageService) {
-    this.options = parseNgCompilerOptions(project);
+    this.parseConfigHost = new LSParseConfigHost(project);
+    this.options = parseNgCompilerOptions(project, this.parseConfigHost);
     this.strategy = createTypeCheckingProgramStrategy(project);
     this.adapter = new LanguageServiceAdapter(project);
     this.compilerFactory = new CompilerFactory(this.adapter, this.strategy, this.options);
     this.watchConfigFile(project);
   }
 
+  getCompilerOptions(): CompilerOptions {
+    return this.options;
+  }
+
   getSemanticDiagnostics(fileName: string): ts.Diagnostic[] {
     const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName);
     const ttc = compiler.getTemplateTypeChecker();
@@ -104,24 +110,24 @@ export class LanguageService {
         project.getConfigFilePath(), (fileName: string, eventKind: ts.FileWatcherEventKind) => {
           project.log(`Config file changed: ${fileName}`);
           if (eventKind === ts.FileWatcherEventKind.Changed) {
-            this.options = parseNgCompilerOptions(project);
+            this.options = parseNgCompilerOptions(project, this.parseConfigHost);
           }
         });
   }
 }
 
-export function parseNgCompilerOptions(project: ts.server.Project): CompilerOptions {
-  let config = {};
-  if (project instanceof ts.server.ConfiguredProject) {
-    const configPath = project.getConfigFilePath();
-    const result = ts.readConfigFile(configPath, path => project.readFile(path));
-    if (result.error) {
-      project.error(ts.flattenDiagnosticMessageText(result.error.messageText, '\n'));
-    }
-    config = result.config || config;
+export function parseNgCompilerOptions(
+    project: ts.server.Project, host: ParseConfigurationHost): CompilerOptions {
+  if (!(project instanceof ts.server.ConfiguredProject)) {
+    return {};
   }
-  const basePath = project.getCurrentDirectory();
-  return createNgCompilerOptions(basePath, config, project.getCompilationSettings());
+  const {options, errors} =
+      readConfiguration(project.getConfigFilePath(), /* existingOptions */ undefined, host);
+  if (errors.length > 0) {
+    project.error(formatDiagnostics(errors));
+  }
+
+  return options;
 }
 
 function createTypeCheckingProgramStrategy(project: ts.server.Project):

packages/language-service/ivy/language_service_adapter.ts

@@ -6,9 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {ParseConfigurationHost} from '@angular/compiler-cli';
 import {NgCompilerAdapter} from '@angular/compiler-cli/src/ngtsc/core/api';
-import {absoluteFrom, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
+import {absoluteFrom, AbsoluteFsPath, FileStats, PathSegment, PathString} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {isShim} from '@angular/compiler-cli/src/ngtsc/shims';
+import * as p from 'path';
 import * as ts from 'typescript/lib/tsserverlibrary';
 
 import {isTypeScriptFile} from './utils';
@@ -78,3 +80,53 @@ export class LanguageServiceAdapter implements NgCompilerAdapter {
     return lastVersion !== latestVersion;
   }
 }
+
+/**
+ * Used to read configuration files.
+ *
+ * A language service parse configuration host is independent of the adapter
+ * because signatures of calls like `FileSystem#readFile` are a bit stricter
+ * than those on the adapter.
+ */
+export class LSParseConfigHost implements ParseConfigurationHost {
+  private readonly host: ts.server.ServerHost = this.project.projectService.host;
+  constructor(private readonly project: ts.server.Project) {}
+  exists(path: AbsoluteFsPath): boolean {
+    return this.project.fileExists(path) || this.project.directoryExists(path);
+  }
+  readFile(path: AbsoluteFsPath): string {
+    const content = this.project.readFile(path);
+    if (content === undefined) {
+      throw new Error(`LanguageServiceFS#readFile called on unavailable file ${path}`);
+    }
+    return content;
+  }
+  lstat(path: AbsoluteFsPath): FileStats {
+    return {
+      isFile: () => {
+        return this.project.fileExists(path);
+      },
+      isDirectory: () => {
+        return this.project.directoryExists(path);
+      },
+      isSymbolicLink: () => {
+        throw new Error(`LanguageServiceFS#lstat#isSymbolicLink not implemented`);
+      },
+    };
+  }
+  pwd(): AbsoluteFsPath {
+    return this.project.getCurrentDirectory() as AbsoluteFsPath;
+  }
+  extname(path: AbsoluteFsPath|PathSegment): string {
+    return p.extname(path);
+  }
+  resolve(...paths: string[]): AbsoluteFsPath {
+    return this.host.resolvePath(this.join(paths[0], ...paths.slice(1))) as AbsoluteFsPath;
+  }
+  dirname<T extends PathString>(file: T): T {
+    return p.dirname(file) as T;
+  }
+  join<T extends PathString>(basePath: T, ...paths: string[]): T {
+    return p.join(basePath, ...paths) as T;
+  }
+}

packages/language-service/ivy/test/legacy/language_service_spec.ts

@@ -8,33 +8,66 @@
 
 import * as ts from 'typescript/lib/tsserverlibrary';
 
-import {LanguageService, parseNgCompilerOptions} from '../../language_service';
+import {LanguageService} from '../../language_service';
 
-import {MockService, setup, TEST_TEMPLATE} from './mock_host';
+import {MockConfigFileFs, MockService, setup, TEST_TEMPLATE, TSCONFIG} from './mock_host';
 
 describe('language service adapter', () => {
   let project: ts.server.Project;
   let service: MockService;
   let ngLS: LanguageService;
+  let configFileFs: MockConfigFileFs;
 
   beforeAll(() => {
-    const {project: _project, tsLS, service: _service} = setup();
+    const {project: _project, tsLS, service: _service, configFileFs: _configFileFs} = setup();
     project = _project;
     service = _service;
     ngLS = new LanguageService(project, tsLS);
+    configFileFs = _configFileFs;
   });
 
-  describe('parseNgCompilerOptions', () => {
-    it('should read angularCompilerOptions in tsconfig.json', () => {
-      const options = parseNgCompilerOptions(project);
-      expect(options).toEqual(jasmine.objectContaining({
+  afterEach(() => {
+    configFileFs.clear();
+  });
+
+  describe('parse compiler options', () => {
+    beforeEach(() => {
+      // Need to reset project on each test to reinitialize file watchers.
+      const {project: _project, tsLS, service: _service, configFileFs: _configFileFs} = setup();
+      project = _project;
+      service = _service;
+      configFileFs = _configFileFs;
+      ngLS = new LanguageService(project, tsLS);
+    });
+
+    it('should initialize with angularCompilerOptions from tsconfig.json', () => {
+      expect(ngLS.getCompilerOptions()).toEqual(jasmine.objectContaining({
         enableIvy: true,  // default for ivy is true
         strictTemplates: true,
         strictInjectionParameters: true,
       }));
     });
+
+    it('should reparse angularCompilerOptions on tsconfig.json change', () => {
+      expect(ngLS.getCompilerOptions()).toEqual(jasmine.objectContaining({
+        enableIvy: true,  // default for ivy is true
+        strictTemplates: true,
+        strictInjectionParameters: true,
+      }));
+
+      configFileFs.overwriteConfigFile(TSCONFIG, `{
+         "angularCompilerOptions": {
+           "strictTemplates": false
+         }
+       }`);
+
+      expect(ngLS.getCompilerOptions()).toEqual(jasmine.objectContaining({
+        strictTemplates: false,
+      }));
+    });
   });
 
+
   describe('last known program', () => {
     beforeEach(() => {
       service.reset();

packages/language-service/ivy/test/legacy/mock_host.ts

@@ -43,31 +43,97 @@ const NOOP_FILE_WATCHER: ts.FileWatcher = {
   close() {}
 };
 
-export const host: ts.server.ServerHost = {
-  ...ts.sys,
-  readFile(absPath: string, encoding?: string): string |
-      undefined {
-        return ts.sys.readFile(absPath, encoding);
-      },
-  watchFile(path: string, callback: ts.FileWatcherCallback): ts.FileWatcher {
-    return NOOP_FILE_WATCHER;
-  },
-  watchDirectory(path: string, callback: ts.DirectoryWatcherCallback): ts.FileWatcher {
-    return NOOP_FILE_WATCHER;
-  },
-  setTimeout() {
-    throw new Error('setTimeout is not implemented');
-  },
-  clearTimeout() {
-    throw new Error('clearTimeout is not implemented');
-  },
-  setImmediate() {
-    throw new Error('setImmediate is not implemented');
-  },
-  clearImmediate() {
-    throw new Error('clearImmediate is not implemented');
-  },
-};
+class MockWatcher implements ts.FileWatcher {
+  constructor(
+      private readonly fileName: string,
+      private readonly cb: ts.FileWatcherCallback,
+      readonly close: () => void,
+  ) {}
+
+  changed() {
+    this.cb(this.fileName, ts.FileWatcherEventKind.Changed);
+  }
+
+  deleted() {
+    this.cb(this.fileName, ts.FileWatcherEventKind.Deleted);
+  }
+}
+
+/**
+ * A mock file system impacting configuration files.
+ * Queries for all other files are deferred to the underlying filesystem.
+ */
+export class MockConfigFileFs implements
+    Pick<ts.server.ServerHost, 'readFile'|'fileExists'|'watchFile'> {
+  private configOverwrites = new Map<string, string>();
+  private configFileWatchers = new Map<string, MockWatcher>();
+
+  overwriteConfigFile(configFile: string, contents: string) {
+    if (!configFile.endsWith('.json')) {
+      throw new Error(`${configFile} is not a configuration file.`);
+    }
+    this.configOverwrites.set(configFile, contents);
+    this.configFileWatchers.get(configFile)?.changed();
+  }
+
+  readFile(file: string, encoding?: string): string|undefined {
+    const read = this.configOverwrites.get(file) ?? ts.sys.readFile(file, encoding);
+    return read;
+  }
+
+  fileExists(file: string): boolean {
+    return this.configOverwrites.has(file) || ts.sys.fileExists(file);
+  }
+
+  watchFile(path: string, callback: ts.FileWatcherCallback) {
+    if (!path.endsWith('.json')) {
+      // We only care about watching config files.
+      return NOOP_FILE_WATCHER;
+    }
+    const watcher = new MockWatcher(path, callback, () => {
+      this.configFileWatchers.delete(path);
+    });
+    this.configFileWatchers.set(path, watcher);
+    return watcher;
+  }
+
+  clear() {
+    this.configOverwrites.clear();
+    this.configFileWatchers.clear();
+  }
+}
+
+function createHost(configFileFs: MockConfigFileFs): ts.server.ServerHost {
+  return {
+    ...ts.sys,
+    fileExists(absPath: string): boolean {
+      return configFileFs.fileExists(absPath);
+    },
+    readFile(absPath: string, encoding?: string): string |
+        undefined {
+          return configFileFs.readFile(absPath, encoding);
+        },
+    watchFile(path: string, callback: ts.FileWatcherCallback): ts.FileWatcher {
+      return configFileFs.watchFile(path, callback);
+    },
+    watchDirectory(path: string, callback: ts.DirectoryWatcherCallback): ts.FileWatcher {
+      return NOOP_FILE_WATCHER;
+    },
+    setTimeout() {
+      throw new Error('setTimeout is not implemented');
+    },
+    clearTimeout() {
+      throw new Error('clearTimeout is not implemented');
+    },
+    setImmediate() {
+      throw new Error('setImmediate is not implemented');
+    },
+    clearImmediate() {
+      throw new Error('clearImmediate is not implemented');
+    },
+  };
+}
+
 
 /**
  * Create a ConfiguredProject and an actual program for the test project located
@@ -76,8 +142,9 @@ export const host: ts.server.ServerHost = {
  * and modify test files.
  */
 export function setup() {
+  const configFileFs = new MockConfigFileFs();
   const projectService = new ts.server.ProjectService({
-    host,
+    host: createHost(configFileFs),
     logger,
     cancellationToken: ts.server.nullCancellationToken,
     useSingleInferredProject: true,
@@ -97,6 +164,7 @@ export function setup() {
     service: new MockService(project, projectService),
     project,
     tsLS,
+    configFileFs,
   };
 }