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
This commit is contained in:
Pete Bacon Darwin 2020-05-06 21:58:45 +01:00 committed by Alex Rickabaugh
parent 0ec0ff3bce
commit 53a8459d5f
7 changed files with 46 additions and 25 deletions

View File

@ -75,7 +75,8 @@ export class DecorationAnalyzer {
// TODO(alxhub): there's no reason why ngcc needs the "logical file system" logic here, as ngcc // 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 // projects only ever have one rootDir. Instead, ngcc should just switch its emitted import
// based on whether a bestGuessOwningModule is present in the Reference. // 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 ? aliasingHost = this.bundle.entryPoint.generateDeepReexports ?
new PrivateExportAliasingHost(this.reflectionHost) : new PrivateExportAliasingHost(this.reflectionHost) :

View File

@ -612,8 +612,8 @@ export class NgCompiler {
(this.options.rootDirs !== undefined && this.options.rootDirs.length > 0)) { (this.options.rootDirs !== undefined && this.options.rootDirs.length > 0)) {
// rootDirs logic is in effect - use the `LogicalProjectStrategy` for in-project relative // rootDirs logic is in effect - use the `LogicalProjectStrategy` for in-project relative
// imports. // imports.
localImportStrategy = localImportStrategy = new LogicalProjectStrategy(
new LogicalProjectStrategy(reflector, new LogicalFileSystem([...this.host.rootDirs])); reflector, new LogicalFileSystem([...this.host.rootDirs], this.host));
} else { } else {
// Plain relative imports are all that's needed. // Plain relative imports are all that's needed.
localImportStrategy = new RelativePathStrategy(reflector); localImportStrategy = new RelativePathStrategy(reflector);

View File

@ -53,10 +53,13 @@ export class LogicalFileSystem {
*/ */
private cache: Map<AbsoluteFsPath, LogicalProjectPath|null> = new Map(); private cache: Map<AbsoluteFsPath, LogicalProjectPath|null> = 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, // 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. // 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. * of the TS project's root directories.
*/ */
logicalPathOfFile(physicalFile: AbsoluteFsPath): LogicalProjectPath|null { 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; let logicalFile: LogicalProjectPath|null = null;
for (const rootDir of this.rootDirs) { for (const rootDir of this.rootDirs) {
if (physicalFile.startsWith(rootDir)) { if (isWithinBasePath(rootDir, canonicalFilePath)) {
logicalFile = this.createLogicalProjectPath(physicalFile, rootDir); logicalFile = this.createLogicalProjectPath(canonicalFilePath, rootDir);
// The logical project does not include any special "node_modules" nested directories. // The logical project does not include any special "node_modules" nested directories.
if (logicalFile.indexOf('/node_modules/') !== -1) { if (logicalFile.indexOf('/node_modules/') !== -1) {
logicalFile = null; 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): private createLogicalProjectPath(file: AbsoluteFsPath, rootDir: AbsoluteFsPath):
@ -100,3 +105,11 @@ export class LogicalFileSystem {
return (logicalPath.startsWith('/') ? logicalPath : '/' + logicalPath) as LogicalProjectPath; 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('..');
}

View File

@ -6,18 +6,24 @@
* found in the LICENSE file at https://angular.io/license * 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 {LogicalFileSystem, LogicalProjectPath} from '../src/logical';
import {runInEachFileSystem} from '../testing'; import {runInEachFileSystem} from '../testing';
runInEachFileSystem(() => { runInEachFileSystem(() => {
describe('logical paths', () => { describe('logical paths', () => {
let _: typeof absoluteFrom; let _: typeof absoluteFrom;
beforeEach(() => _ = absoluteFrom); let host: NgtscCompilerHost;
beforeEach(() => {
_ = absoluteFrom;
host = new NgtscCompilerHost(getFileSystem());
});
describe('LogicalFileSystem', () => { describe('LogicalFileSystem', () => {
it('should determine logical paths in a single root file system', () => { 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'))) expect(fs.logicalPathOfFile(_('/test/foo/foo.ts')))
.toEqual('/foo/foo' as LogicalProjectPath); .toEqual('/foo/foo' as LogicalProjectPath);
expect(fs.logicalPathOfFile(_('/test/bar/bar.ts'))) expect(fs.logicalPathOfFile(_('/test/bar/bar.ts')))
@ -26,23 +32,23 @@ runInEachFileSystem(() => {
}); });
it('should determine logical paths in a multi-root file system', () => { 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/foo/foo.ts'))).toEqual('/foo' as LogicalProjectPath);
expect(fs.logicalPathOfFile(_('/test/bar/bar.ts'))).toEqual('/bar' 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', () => { 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/foo.ts'))).toEqual('/foo' as LogicalProjectPath);
expect(fs.logicalPathOfFile(_('/test/dist/foo.ts'))).toEqual('/foo' as LogicalProjectPath); expect(fs.logicalPathOfFile(_('/test/dist/foo.ts'))).toEqual('/foo' as LogicalProjectPath);
}); });
it('should always return `/` prefixed logical paths', () => { it('should always return `/` prefixed logical paths', () => {
const rootFs = new LogicalFileSystem([_('/')]); const rootFs = new LogicalFileSystem([_('/')], host);
expect(rootFs.logicalPathOfFile(_('/foo/foo.ts'))) expect(rootFs.logicalPathOfFile(_('/foo/foo.ts')))
.toEqual('/foo/foo' as LogicalProjectPath); .toEqual('/foo/foo' as LogicalProjectPath);
const nonRootFs = new LogicalFileSystem([_('/test/')]); const nonRootFs = new LogicalFileSystem([_('/test/')], host);
expect(nonRootFs.logicalPathOfFile(_('/test/foo/foo.ts'))) expect(nonRootFs.logicalPathOfFile(_('/test/foo/foo.ts')))
.toEqual('/foo/foo' as LogicalProjectPath); .toEqual('/foo/foo' as LogicalProjectPath);
}); });

View File

@ -146,7 +146,7 @@ runInEachFileSystem(() => {
} }
} }
const {program} = makeProgram([ const {program, host} = makeProgram([
{ {
name: _('/index.ts'), name: _('/index.ts'),
contents: `export class Foo {}`, contents: `export class Foo {}`,
@ -157,7 +157,7 @@ runInEachFileSystem(() => {
} }
]); ]);
const checker = program.getTypeChecker(); const checker = program.getTypeChecker();
const logicalFs = new LogicalFileSystem([_('/')]); const logicalFs = new LogicalFileSystem([_('/')], host);
const strategy = new LogicalProjectStrategy(new TestHost(checker), logicalFs); const strategy = new LogicalProjectStrategy(new TestHost(checker), logicalFs);
const decl = getDeclaration(program, _('/index.ts'), 'Foo', ts.isClassDeclaration); const decl = getDeclaration(program, _('/index.ts'), 'Foo', ts.isClassDeclaration);
const context = program.getSourceFile(_('/context.ts'))!; const context = program.getSourceFile(_('/context.ts'))!;

View File

@ -95,7 +95,7 @@ export function angularCoreDts(): TestFile {
export declare class EventEmitter<T> { export declare class EventEmitter<T> {
subscribe(generatorOrNext?: any, error?: any, complete?: any): unknown; subscribe(generatorOrNext?: any, error?: any, complete?: any): unknown;
} }
export declare type NgIterable<T> = Array<T> | Iterable<T>; export declare type NgIterable<T> = Array<T> | Iterable<T>;
` `
}; };
@ -254,7 +254,7 @@ export function typecheck(
makeProgram(files, {strictNullChecks: true, noImplicitAny: true, ...opts}, undefined, false); makeProgram(files, {strictNullChecks: true, noImplicitAny: true, ...opts}, undefined, false);
const sf = program.getSourceFile(absoluteFrom('/main.ts'))!; const sf = program.getSourceFile(absoluteFrom('/main.ts'))!;
const checker = program.getTypeChecker(); 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 reflectionHost = new TypeScriptReflectionHost(checker);
const moduleResolver = const moduleResolver =
new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); new ModuleResolver(program, options, host, /* moduleResolutionCache */ null);

View File

@ -7,7 +7,7 @@
*/ */
import * as ts from 'typescript'; 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 {runInEachFileSystem, TestFile} from '../../file_system/testing';
import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
import {isNamedClassDeclaration, ReflectionHost, TypeScriptReflectionHost} from '../../reflection'; import {isNamedClassDeclaration, ReflectionHost, TypeScriptReflectionHost} from '../../reflection';
@ -40,6 +40,7 @@ runInEachFileSystem(() => {
}); });
it('should not produce an empty SourceFile when there is nothing to typecheck', () => { it('should not produce an empty SourceFile when there is nothing to typecheck', () => {
const host = new NgtscCompilerHost(getFileSystem());
const file = new TypeCheckFile( const file = new TypeCheckFile(
_('/_typecheck_.ts'), ALL_ENABLED_CONFIG, new ReferenceEmitter([]), _('/_typecheck_.ts'), ALL_ENABLED_CONFIG, new ReferenceEmitter([]),
/* reflector */ null!); /* reflector */ null!);
@ -64,7 +65,7 @@ TestClass.ngTypeCtor({value: 'test'});
const {program, host, options} = makeProgram(files, undefined, undefined, false); const {program, host, options} = makeProgram(files, undefined, undefined, false);
const checker = program.getTypeChecker(); const checker = program.getTypeChecker();
const reflectionHost = new TypeScriptReflectionHost(checker); const reflectionHost = new TypeScriptReflectionHost(checker);
const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); const logicalFs = new LogicalFileSystem(getRootDirs(host, options), host);
const moduleResolver = const moduleResolver =
new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); new ModuleResolver(program, options, host, /* moduleResolutionCache */ null);
const emitter = new ReferenceEmitter([ const emitter = new ReferenceEmitter([
@ -100,7 +101,7 @@ TestClass.ngTypeCtor({value: 'test'});
const {program, host, options} = makeProgram(files, undefined, undefined, false); const {program, host, options} = makeProgram(files, undefined, undefined, false);
const checker = program.getTypeChecker(); const checker = program.getTypeChecker();
const reflectionHost = new TypeScriptReflectionHost(checker); const reflectionHost = new TypeScriptReflectionHost(checker);
const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); const logicalFs = new LogicalFileSystem(getRootDirs(host, options), host);
const moduleResolver = const moduleResolver =
new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); new ModuleResolver(program, options, host, /* moduleResolutionCache */ null);
const emitter = new ReferenceEmitter([ const emitter = new ReferenceEmitter([
@ -143,7 +144,7 @@ TestClass.ngTypeCtor({value: 'test'});
const {program, host, options} = makeProgram(files, undefined, undefined, false); const {program, host, options} = makeProgram(files, undefined, undefined, false);
const checker = program.getTypeChecker(); const checker = program.getTypeChecker();
const reflectionHost = new TypeScriptReflectionHost(checker); const reflectionHost = new TypeScriptReflectionHost(checker);
const logicalFs = new LogicalFileSystem(getRootDirs(host, options)); const logicalFs = new LogicalFileSystem(getRootDirs(host, options), host);
const moduleResolver = const moduleResolver =
new ModuleResolver(program, options, host, /* moduleResolutionCache */ null); new ModuleResolver(program, options, host, /* moduleResolutionCache */ null);
const emitter = new ReferenceEmitter([ const emitter = new ReferenceEmitter([