From 00b9de56f52fab9f80a56147f4281f309978f710 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 8 May 2020 13:34:03 +0100 Subject: [PATCH] Revert "fix(compiler-cli): ensure LogicalFileSystem handles case-sensitivity (#36968)" (#37003) This reverts commit 65337fb8b8d9d16407c182af935dd737beaea7dd. The changes to the case-sensitivity handling in #36968 caused multiple projects to fail to build. See #36992, #36993 and #37000. The issue is related to the logical path handling. But it is felt that it is safer to revert the entire PR and then to investigate further. PR Close #37003 --- .../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, 25 insertions(+), 46 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index 1558865e68..2b66c5662d 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -75,8 +75,7 @@ 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, this.host)), + new LogicalProjectStrategy(this.reflectionHost, new LogicalFileSystem(this.rootDirs)), ]); 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 433c70f387..f932927066 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -576,8 +576,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], this.host)); + localImportStrategy = + new LogicalProjectStrategy(reflector, new LogicalFileSystem([...this.host.rootDirs])); } 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 2c1be6ba8b..15b5c24ae7 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/src/logical.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/src/logical.ts @@ -53,13 +53,10 @@ export class LogicalFileSystem { */ private cache: Map = new Map(); - constructor(rootDirs: AbsoluteFsPath[], private compilerHost: ts.CompilerHost) { + constructor(rootDirs: AbsoluteFsPath[]) { // 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.map(dir => this.compilerHost.getCanonicalFileName(dir) as AbsoluteFsPath) - .concat([]) - .sort((a, b) => b.length - a.length); + this.rootDirs = rootDirs.concat([]).sort((a, b) => b.length - a.length); } /** @@ -79,13 +76,11 @@ export class LogicalFileSystem { * of the TS project's root directories. */ logicalPathOfFile(physicalFile: AbsoluteFsPath): LogicalProjectPath|null { - const canonicalFilePath = - this.compilerHost.getCanonicalFileName(physicalFile) as AbsoluteFsPath; - if (!this.cache.has(canonicalFilePath)) { + if (!this.cache.has(physicalFile)) { let logicalFile: LogicalProjectPath|null = null; for (const rootDir of this.rootDirs) { - if (isWithinBasePath(rootDir, canonicalFilePath)) { - logicalFile = this.createLogicalProjectPath(canonicalFilePath, rootDir); + if (physicalFile.startsWith(rootDir)) { + logicalFile = this.createLogicalProjectPath(physicalFile, rootDir); // The logical project does not include any special "node_modules" nested directories. if (logicalFile.indexOf('/node_modules/') !== -1) { logicalFile = null; @@ -94,9 +89,9 @@ export class LogicalFileSystem { } } } - this.cache.set(canonicalFilePath, logicalFile); + this.cache.set(physicalFile, logicalFile); } - return this.cache.get(canonicalFilePath)!; + return this.cache.get(physicalFile)!; } private createLogicalProjectPath(file: AbsoluteFsPath, rootDir: AbsoluteFsPath): @@ -105,11 +100,3 @@ 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 1bf4fc26b2..35d89aa107 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,24 +6,18 @@ * found in the LICENSE file at https://angular.io/license */ -import {NgtscCompilerHost} from '../src/compiler_host'; -import {absoluteFrom, getFileSystem} from '../src/helpers'; +import {absoluteFrom} from '../src/helpers'; import {LogicalFileSystem, LogicalProjectPath} from '../src/logical'; import {runInEachFileSystem} from '../testing'; runInEachFileSystem(() => { describe('logical paths', () => { let _: typeof absoluteFrom; - let host: NgtscCompilerHost; - - beforeEach(() => { - _ = absoluteFrom; - host = new NgtscCompilerHost(getFileSystem()); - }); + beforeEach(() => _ = absoluteFrom); describe('LogicalFileSystem', () => { it('should determine logical paths in a single root file system', () => { - const fs = new LogicalFileSystem([_('/test')], host); + const fs = new LogicalFileSystem([_('/test')]); expect(fs.logicalPathOfFile(_('/test/foo/foo.ts'))) .toEqual('/foo/foo' as LogicalProjectPath); expect(fs.logicalPathOfFile(_('/test/bar/bar.ts'))) @@ -32,23 +26,23 @@ runInEachFileSystem(() => { }); it('should determine logical paths in a multi-root file system', () => { - const fs = new LogicalFileSystem([_('/test/foo'), _('/test/bar')], host); + const fs = new LogicalFileSystem([_('/test/foo'), _('/test/bar')]); 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')], host); + const fs = new LogicalFileSystem([_('/test'), _('/test/dist')]); 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([_('/')], host); + const rootFs = new LogicalFileSystem([_('/')]); expect(rootFs.logicalPathOfFile(_('/foo/foo.ts'))) .toEqual('/foo/foo' as LogicalProjectPath); - const nonRootFs = new LogicalFileSystem([_('/test/')], host); + const nonRootFs = new LogicalFileSystem([_('/test/')]); 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 9305e0c2b8..eb6bc7ac53 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, host} = makeProgram([ + const {program} = makeProgram([ { name: _('/index.ts'), contents: `export class Foo {}`, @@ -157,7 +157,7 @@ runInEachFileSystem(() => { } ]); const checker = program.getTypeChecker(); - const logicalFs = new LogicalFileSystem([_('/')], host); + const logicalFs = new LogicalFileSystem([_('/')]); 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 1c3e22192f..89d7d5c0b2 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -92,7 +92,7 @@ export function angularCoreDts(): TestFile { export declare class EventEmitter { subscribe(generatorOrNext?: any, error?: any, complete?: any): unknown; } - + export declare type NgIterable = Array | Iterable; ` }; @@ -251,7 +251,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), host); + const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); 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 16c405531e..9f9e1e141c 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, getFileSystem, getSourceFileOrError, LogicalFileSystem, NgtscCompilerHost} from '../../file_system'; +import {absoluteFrom, getSourceFileOrError, LogicalFileSystem} from '../../file_system'; import {runInEachFileSystem, TestFile} from '../../file_system/testing'; import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection'; @@ -42,7 +42,6 @@ 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!); @@ -67,7 +66,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), host); + const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); const moduleResolver = new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); const emitter = new ReferenceEmitter([ @@ -103,7 +102,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), host); + const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); const moduleResolver = new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); const emitter = new ReferenceEmitter([ @@ -145,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), host); + const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); const moduleResolver = new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); const emitter = new ReferenceEmitter([