From 53a8459d5f94f5c6601dd46daae2c605261e7868 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 6 May 2020 21:58:45 +0100 Subject: [PATCH] fix(compiler-cli): ensure LogicalFileSystem handles case-sensitivity (#36859) The `LogicalFileSystem` was not taking into account the case-sensitivity of the file-system when caching logical file paths. PR Close #36859 --- .../ngcc/src/analysis/decoration_analyzer.ts | 3 ++- .../src/ngtsc/core/src/compiler.ts | 4 +-- .../src/ngtsc/file_system/src/logical.ts | 27 ++++++++++++++----- .../ngtsc/file_system/test/logical_spec.ts | 20 +++++++++----- .../src/ngtsc/imports/test/emitter_spec.ts | 4 +-- .../src/ngtsc/typecheck/test/test_utils.ts | 4 +-- .../typecheck/test/type_constructor_spec.ts | 9 ++++--- 7 files changed, 46 insertions(+), 25 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index b4ec66fde8ca6..4ea614fb34994 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -75,7 +75,8 @@ export class DecorationAnalyzer { // TODO(alxhub): there's no reason why ngcc needs the "logical file system" logic here, as ngcc // projects only ever have one rootDir. Instead, ngcc should just switch its emitted import // based on whether a bestGuessOwningModule is present in the Reference. - new LogicalProjectStrategy(this.reflectionHost, new LogicalFileSystem(this.rootDirs)), + new LogicalProjectStrategy( + this.reflectionHost, new LogicalFileSystem(this.rootDirs, this.host)), ]); aliasingHost = this.bundle.entryPoint.generateDeepReexports ? new PrivateExportAliasingHost(this.reflectionHost) : diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 868b51ab6744a..7d7c1d22c78a3 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -612,8 +612,8 @@ export class NgCompiler { (this.options.rootDirs !== undefined && this.options.rootDirs.length > 0)) { // rootDirs logic is in effect - use the `LogicalProjectStrategy` for in-project relative // imports. - localImportStrategy = - new LogicalProjectStrategy(reflector, new LogicalFileSystem([...this.host.rootDirs])); + localImportStrategy = new LogicalProjectStrategy( + reflector, new LogicalFileSystem([...this.host.rootDirs], this.host)); } else { // Plain relative imports are all that's needed. localImportStrategy = new RelativePathStrategy(reflector); diff --git a/packages/compiler-cli/src/ngtsc/file_system/src/logical.ts b/packages/compiler-cli/src/ngtsc/file_system/src/logical.ts index 15b5c24ae7df1..2c1be6ba8ba96 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/src/logical.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/src/logical.ts @@ -53,10 +53,13 @@ export class LogicalFileSystem { */ private cache: Map = new Map(); - constructor(rootDirs: AbsoluteFsPath[]) { + constructor(rootDirs: AbsoluteFsPath[], private compilerHost: ts.CompilerHost) { // Make a copy and sort it by length in reverse order (longest first). This speeds up lookups, // since there's no need to keep going through the array once a match is found. - this.rootDirs = rootDirs.concat([]).sort((a, b) => b.length - a.length); + this.rootDirs = + rootDirs.map(dir => this.compilerHost.getCanonicalFileName(dir) as AbsoluteFsPath) + .concat([]) + .sort((a, b) => b.length - a.length); } /** @@ -76,11 +79,13 @@ export class LogicalFileSystem { * of the TS project's root directories. */ logicalPathOfFile(physicalFile: AbsoluteFsPath): LogicalProjectPath|null { - if (!this.cache.has(physicalFile)) { + const canonicalFilePath = + this.compilerHost.getCanonicalFileName(physicalFile) as AbsoluteFsPath; + if (!this.cache.has(canonicalFilePath)) { let logicalFile: LogicalProjectPath|null = null; for (const rootDir of this.rootDirs) { - if (physicalFile.startsWith(rootDir)) { - logicalFile = this.createLogicalProjectPath(physicalFile, rootDir); + if (isWithinBasePath(rootDir, canonicalFilePath)) { + logicalFile = this.createLogicalProjectPath(canonicalFilePath, rootDir); // The logical project does not include any special "node_modules" nested directories. if (logicalFile.indexOf('/node_modules/') !== -1) { logicalFile = null; @@ -89,9 +94,9 @@ export class LogicalFileSystem { } } } - this.cache.set(physicalFile, logicalFile); + this.cache.set(canonicalFilePath, logicalFile); } - return this.cache.get(physicalFile)!; + return this.cache.get(canonicalFilePath)!; } private createLogicalProjectPath(file: AbsoluteFsPath, rootDir: AbsoluteFsPath): @@ -100,3 +105,11 @@ export class LogicalFileSystem { return (logicalPath.startsWith('/') ? logicalPath : '/' + logicalPath) as LogicalProjectPath; } } + +/** + * Is the `path` a descendant of the `base`? + * E.g. `foo/bar/zee` is within `foo/bar` but not within `foo/car`. + */ +function isWithinBasePath(base: AbsoluteFsPath, path: AbsoluteFsPath): boolean { + return !relative(base, path).startsWith('..'); +} diff --git a/packages/compiler-cli/src/ngtsc/file_system/test/logical_spec.ts b/packages/compiler-cli/src/ngtsc/file_system/test/logical_spec.ts index 35d89aa107f95..1bf4fc26b2181 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/test/logical_spec.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/test/logical_spec.ts @@ -6,18 +6,24 @@ * found in the LICENSE file at https://angular.io/license */ -import {absoluteFrom} from '../src/helpers'; +import {NgtscCompilerHost} from '../src/compiler_host'; +import {absoluteFrom, getFileSystem} from '../src/helpers'; import {LogicalFileSystem, LogicalProjectPath} from '../src/logical'; import {runInEachFileSystem} from '../testing'; runInEachFileSystem(() => { describe('logical paths', () => { let _: typeof absoluteFrom; - beforeEach(() => _ = absoluteFrom); + let host: NgtscCompilerHost; + + beforeEach(() => { + _ = absoluteFrom; + host = new NgtscCompilerHost(getFileSystem()); + }); describe('LogicalFileSystem', () => { it('should determine logical paths in a single root file system', () => { - const fs = new LogicalFileSystem([_('/test')]); + const fs = new LogicalFileSystem([_('/test')], host); expect(fs.logicalPathOfFile(_('/test/foo/foo.ts'))) .toEqual('/foo/foo' as LogicalProjectPath); expect(fs.logicalPathOfFile(_('/test/bar/bar.ts'))) @@ -26,23 +32,23 @@ runInEachFileSystem(() => { }); it('should determine logical paths in a multi-root file system', () => { - const fs = new LogicalFileSystem([_('/test/foo'), _('/test/bar')]); + const fs = new LogicalFileSystem([_('/test/foo'), _('/test/bar')], host); expect(fs.logicalPathOfFile(_('/test/foo/foo.ts'))).toEqual('/foo' as LogicalProjectPath); expect(fs.logicalPathOfFile(_('/test/bar/bar.ts'))).toEqual('/bar' as LogicalProjectPath); }); it('should continue to work when one root is a child of another', () => { - const fs = new LogicalFileSystem([_('/test'), _('/test/dist')]); + const fs = new LogicalFileSystem([_('/test'), _('/test/dist')], host); expect(fs.logicalPathOfFile(_('/test/foo.ts'))).toEqual('/foo' as LogicalProjectPath); expect(fs.logicalPathOfFile(_('/test/dist/foo.ts'))).toEqual('/foo' as LogicalProjectPath); }); it('should always return `/` prefixed logical paths', () => { - const rootFs = new LogicalFileSystem([_('/')]); + const rootFs = new LogicalFileSystem([_('/')], host); expect(rootFs.logicalPathOfFile(_('/foo/foo.ts'))) .toEqual('/foo/foo' as LogicalProjectPath); - const nonRootFs = new LogicalFileSystem([_('/test/')]); + const nonRootFs = new LogicalFileSystem([_('/test/')], host); expect(nonRootFs.logicalPathOfFile(_('/test/foo/foo.ts'))) .toEqual('/foo/foo' as LogicalProjectPath); }); diff --git a/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts b/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts index eb6bc7ac53b1a..9305e0c2b8e23 100644 --- a/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts +++ b/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts @@ -146,7 +146,7 @@ runInEachFileSystem(() => { } } - const {program} = makeProgram([ + const {program, host} = makeProgram([ { name: _('/index.ts'), contents: `export class Foo {}`, @@ -157,7 +157,7 @@ runInEachFileSystem(() => { } ]); const checker = program.getTypeChecker(); - const logicalFs = new LogicalFileSystem([_('/')]); + const logicalFs = new LogicalFileSystem([_('/')], host); const strategy = new LogicalProjectStrategy(new TestHost(checker), logicalFs); const decl = getDeclaration(program, _('/index.ts'), 'Foo', ts.isClassDeclaration); const context = program.getSourceFile(_('/context.ts'))!; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index 1908f3f4c7cc6..feec2875c1285 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -95,7 +95,7 @@ export function angularCoreDts(): TestFile { export declare class EventEmitter { subscribe(generatorOrNext?: any, error?: any, complete?: any): unknown; } - + export declare type NgIterable = Array | Iterable; ` }; @@ -254,7 +254,7 @@ export function typecheck( makeProgram(files, {strictNullChecks: true, noImplicitAny: true, ...opts}, undefined, false); const sf = program.getSourceFile(absoluteFrom('/main.ts'))!; const checker = program.getTypeChecker(); - const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); + const logicalFs = new LogicalFileSystem(getRootDirs(host, options), host); const reflectionHost = new TypeScriptReflectionHost(checker); const moduleResolver = new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts index 985f33c5bc4b6..e843a1fc7f821 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts @@ -7,7 +7,7 @@ */ import * as ts from 'typescript'; -import {absoluteFrom, getSourceFileOrError, LogicalFileSystem} from '../../file_system'; +import {absoluteFrom, getFileSystem, getSourceFileOrError, LogicalFileSystem, NgtscCompilerHost} from '../../file_system'; import {runInEachFileSystem, TestFile} from '../../file_system/testing'; import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; import {isNamedClassDeclaration, ReflectionHost, TypeScriptReflectionHost} from '../../reflection'; @@ -40,6 +40,7 @@ runInEachFileSystem(() => { }); it('should not produce an empty SourceFile when there is nothing to typecheck', () => { + const host = new NgtscCompilerHost(getFileSystem()); const file = new TypeCheckFile( _('/_typecheck_.ts'), ALL_ENABLED_CONFIG, new ReferenceEmitter([]), /* reflector */ null!); @@ -64,7 +65,7 @@ TestClass.ngTypeCtor({value: 'test'}); const {program, host, options} = makeProgram(files, undefined, undefined, false); const checker = program.getTypeChecker(); const reflectionHost = new TypeScriptReflectionHost(checker); - const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); + const logicalFs = new LogicalFileSystem(getRootDirs(host, options), host); const moduleResolver = new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); const emitter = new ReferenceEmitter([ @@ -100,7 +101,7 @@ TestClass.ngTypeCtor({value: 'test'}); const {program, host, options} = makeProgram(files, undefined, undefined, false); const checker = program.getTypeChecker(); const reflectionHost = new TypeScriptReflectionHost(checker); - const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); + const logicalFs = new LogicalFileSystem(getRootDirs(host, options), host); const moduleResolver = new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); const emitter = new ReferenceEmitter([ @@ -143,7 +144,7 @@ TestClass.ngTypeCtor({value: 'test'}); const {program, host, options} = makeProgram(files, undefined, undefined, false); const checker = program.getTypeChecker(); const reflectionHost = new TypeScriptReflectionHost(checker); - const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); + const logicalFs = new LogicalFileSystem(getRootDirs(host, options), host); const moduleResolver = new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); const emitter = new ReferenceEmitter([