From 64c3135be7c5d1007945074478c894b71b229cea Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Mon, 9 Nov 2020 18:20:02 -0600 Subject: [PATCH] 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 | 40 +++--- packages/language-service/ivy/BUILD.bazel | 1 + .../language-service/ivy/language_service.ts | 36 +++--- .../ivy/language_service_adapter.ts | 54 +++++++- .../ivy/test/legacy/language_service_spec.ts | 47 ++++++- .../ivy/test/legacy/mock_host.ts | 120 ++++++++++++++---- 6 files changed, 232 insertions(+), 66 deletions(-) diff --git a/packages/compiler-cli/src/perform_compile.ts b/packages/compiler-cli/src/perform_compile.ts index 1fc1a42c97..3ec3ca0783 100644 --- a/packages/compiler-cli/src/perform_compile.ts +++ b/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; + 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; diff --git a/packages/language-service/ivy/BUILD.bazel b/packages/language-service/ivy/BUILD.bazel index 8b584217c6..ce37cd7631 100644 --- a/packages/language-service/ivy/BUILD.bazel +++ b/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", ], ) diff --git a/packages/language-service/ivy/language_service.ts b/packages/language-service/ivy/language_service.ts index 62d1e3dbb3..db23b16588 100644 --- a/packages/language-service/ivy/language_service.ts +++ b/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): diff --git a/packages/language-service/ivy/language_service_adapter.ts b/packages/language-service/ivy/language_service_adapter.ts index 2aeb1e76a0..928c3b3e70 100644 --- a/packages/language-service/ivy/language_service_adapter.ts +++ b/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(file: T): T { + return p.dirname(file) as T; + } + join(basePath: T, ...paths: string[]): T { + return p.join(basePath, ...paths) as T; + } +} diff --git a/packages/language-service/ivy/test/legacy/language_service_spec.ts b/packages/language-service/ivy/test/legacy/language_service_spec.ts index 1831138563..71ff969447 100644 --- a/packages/language-service/ivy/test/legacy/language_service_spec.ts +++ b/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(); diff --git a/packages/language-service/ivy/test/legacy/mock_host.ts b/packages/language-service/ivy/test/legacy/mock_host.ts index bf86c266ac..f1b6f3f4e0 100644 --- a/packages/language-service/ivy/test/legacy/mock_host.ts +++ b/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 { + private configOverwrites = new Map(); + private configFileWatchers = new Map(); + + 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, }; }