fix(core): avoid migration error when non-existent symbol is imported (#36367)

In rare cases a project with configured `rootDirs` that has imports to
non-existent identifiers could fail in the migration.

This happens because based on the application code, the migration could
end up trying to resolve the `ts.Symbol` of such non-existent
identifiers. This isn't a problem usually, but due to a upstream bug
in the TypeScript compiler, a runtime error is thrown.

This is because TypeScript is unable to compute a relative path from the
originating source file to the imported source file which _should_
provide the non-existent identifier. An issue for this has been reported
upstream: https://github.com/microsoft/TypeScript/issues/37731. The
issue only surfaces since our migrations don't provide an absolute base
path that is used for resolving the root directories.

To fix this, we ensure that we never use relative paths when parsing
tsconfig files. More details can be found in the TS issue.

Fixes #36346.

PR Close #36367
This commit is contained in:
Paul Gschwendtner 2020-04-02 11:01:43 +02:00 committed by Kara Erickson
parent ee70a18a75
commit d43c30688a
11 changed files with 209 additions and 106 deletions

View File

@ -6,23 +6,21 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
import {dirname, relative} from 'path';
import {Rule, SchematicsException, Tree} from '@angular-devkit/schematics';
import {relative} from 'path';
import * as ts from 'typescript';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {identifyDynamicQueryNodes, removeOptionsParameter, removeStaticFlag} from './util';
/**
* Runs the dynamic queries migration for all TypeScript projects in the current CLI workspace.
*/
export default function(): Rule {
return (tree: Tree, ctx: SchematicContext) => {
return (tree: Tree) => {
const {buildPaths, testPaths} = getProjectTsConfigPaths(tree);
const basePath = process.cwd();
const allPaths = [...buildPaths, ...testPaths];
@ -39,9 +37,7 @@ export default function(): Rule {
}
function runDynamicQueryMigration(tree: Tree, tsconfigPath: string, basePath: string) {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = createMigrationCompilerHost(tree, parsed.options, basePath);
const program = ts.createProgram(parsed.fileNames, parsed.options, host);
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));

View File

@ -7,11 +7,10 @@
*/
import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
import {dirname, relative} from 'path';
import {relative} from 'path';
import * as ts from 'typescript';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {NgDefinitionCollector} from './definition_collector';
import {MissingInjectableTransform} from './transform';
import {UpdateRecorder} from './update_recorder';
@ -43,11 +42,8 @@ export default function(): Rule {
function runMissingInjectableMigration(
tree: Tree, tsconfigPath: string, basePath: string): string[] {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = createMigrationCompilerHost(tree, parsed.options, basePath);
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const failures: string[] = [];
const program = ts.createProgram(parsed.fileNames, parsed.options, host);
const typeChecker = program.getTypeChecker();
const definitionCollector = new NgDefinitionCollector(typeChecker);
const sourceFiles = program.getSourceFiles().filter(
@ -83,7 +79,7 @@ function runMissingInjectableMigration(
/** Gets the update recorder for the specified source file. */
function getUpdateRecorder(sourceFile: ts.SourceFile): UpdateRecorder {
if (updateRecorders.has(sourceFile)) {
return updateRecorders.get(sourceFile) !;
return updateRecorders.get(sourceFile)!;
}
const treeRecorder = tree.beginUpdate(relative(basePath, sourceFile.fileName));
const recorder: UpdateRecorder = {
@ -111,7 +107,9 @@ function runMissingInjectableMigration(
treeRecorder.remove(node.getStart(), node.getWidth());
treeRecorder.insertRight(node.getStart(), newText);
},
commitUpdate() { tree.commitUpdate(treeRecorder); }
commitUpdate() {
tree.commitUpdate(treeRecorder);
}
};
updateRecorders.set(sourceFile, recorder);
return recorder;

View File

@ -7,18 +7,16 @@
*/
import {Rule, SchematicContext, SchematicsException, Tree, UpdateRecorder} from '@angular-devkit/schematics';
import {dirname, relative} from 'path';
import {relative} from 'path';
import * as ts from 'typescript';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {Collector} from './collector';
import {AnalysisFailure, ModuleWithProvidersTransform} from './transform';
/**
* Runs the ModuleWithProviders migration for all TypeScript projects in the current CLI workspace.
*/
@ -47,11 +45,8 @@ export default function(): Rule {
}
function runModuleWithProvidersMigration(tree: Tree, tsconfigPath: string, basePath: string) {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = createMigrationCompilerHost(tree, parsed.options, basePath);
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const failures: string[] = [];
const program = ts.createProgram(parsed.fileNames, parsed.options, host);
const typeChecker = program.getTypeChecker();
const collector = new Collector(typeChecker);
const sourceFiles = program.getSourceFiles().filter(
@ -86,7 +81,7 @@ function runModuleWithProvidersMigration(tree: Tree, tsconfigPath: string, baseP
/** Gets the update recorder for the specified source file. */
function getUpdateRecorder(sourceFile: ts.SourceFile): UpdateRecorder {
if (updateRecorders.has(sourceFile)) {
return updateRecorders.get(sourceFile) !;
return updateRecorders.get(sourceFile)!;
}
const recorder = tree.beginUpdate(relative(basePath, sourceFile.fileName));
updateRecorders.set(sourceFile, recorder);

View File

@ -7,18 +7,16 @@
*/
import {Rule, SchematicsException, Tree} from '@angular-devkit/schematics';
import {dirname, relative} from 'path';
import {relative} from 'path';
import * as ts from 'typescript';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {COMMON_IMPORT, DOCUMENT_TOKEN_NAME, DocumentImportVisitor, ResolvedDocumentImport} from './document_import_visitor';
import {addToImport, createImport, removeFromImport} from './move-import';
/** Entry point for the V8 move-document migration. */
export default function(): Rule {
return (tree: Tree) => {
@ -42,10 +40,7 @@ export default function(): Rule {
* new import source.
*/
function runMoveDocumentMigration(tree: Tree, tsconfigPath: string, basePath: string) {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = createMigrationCompilerHost(tree, parsed.options, basePath);
const program = ts.createProgram(parsed.fileNames, parsed.options, host);
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const visitor = new DocumentImportVisitor(typeChecker);
const sourceFiles = program.getSourceFiles().filter(

View File

@ -6,15 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
import {dirname, relative} from 'path';
import {Rule, SchematicsException, Tree} from '@angular-devkit/schematics';
import {relative} from 'path';
import * as ts from 'typescript';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {HelperFunction, getHelper} from './helpers';
import {getHelper, HelperFunction} from './helpers';
import {migrateExpression, replaceImport} from './migration';
import {findCoreImport, findRendererReferences} from './util';
@ -42,8 +41,7 @@ export default function(): Rule {
}
function runRendererToRenderer2Migration(tree: Tree, tsconfigPath: string, basePath: string) {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = createMigrationCompilerHost(tree, parsed.options, basePath, fileName => {
const {program} = createMigrationProgram(tree, tsconfigPath, basePath, fileName => {
// In case the module augmentation file has been requested, we return a source file that
// augments "@angular/core" to include a named export called "Renderer". This ensures that
// we can rely on the type checker for this migration in v9 where "Renderer" has been removed.
@ -56,10 +54,7 @@ function runRendererToRenderer2Migration(tree: Tree, tsconfigPath: string, baseP
`;
}
return null;
});
const program =
ts.createProgram(parsed.fileNames.concat(MODULE_AUGMENTATION_FILENAME), parsed.options, host);
}, [MODULE_AUGMENTATION_FILENAME]);
const typeChecker = program.getTypeChecker();
const printer = ts.createPrinter();
const sourceFiles = program.getSourceFiles().filter(

View File

@ -8,14 +8,13 @@
import {logging} from '@angular-devkit/core';
import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
import {dirname, relative} from 'path';
import {relative} from 'path';
import {from} from 'rxjs';
import * as ts from 'typescript';
import {NgComponentTemplateVisitor} from '../../utils/ng_component_template';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {NgQueryResolveVisitor} from './angular/ng_query_visitor';
import {QueryTemplateStrategy} from './strategies/template_strategy/template_strategy';
@ -107,49 +106,46 @@ async function runMigration(tree: Tree, context: SchematicContext) {
*/
function analyzeProject(
tree: Tree, tsconfigPath: string, basePath: string, analyzedFiles: Set<string>,
logger: logging.LoggerApi):
AnalyzedProject|null {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = createMigrationCompilerHost(tree, parsed.options, basePath);
const program = ts.createProgram(parsed.fileNames, parsed.options, host);
const syntacticDiagnostics = program.getSyntacticDiagnostics();
logger: logging.LoggerApi): AnalyzedProject|null {
const {program, host} = createMigrationProgram(tree, tsconfigPath, basePath);
const syntacticDiagnostics = program.getSyntacticDiagnostics();
// Syntactic TypeScript errors can throw off the query analysis and therefore we want
// to notify the developer that we couldn't analyze parts of the project. Developers
// can just re-run the migration after fixing these failures.
if (syntacticDiagnostics.length) {
logger.warn(
`\nTypeScript project "${tsconfigPath}" has syntactical errors which could cause ` +
`an incomplete migration. Please fix the following failures and rerun the migration:`);
logger.error(ts.formatDiagnostics(syntacticDiagnostics, host));
logger.info(
'Migration can be rerun with: "ng update @angular/core --from 7 --to 8 --migrate-only"\n');
}
// Syntactic TypeScript errors can throw off the query analysis and therefore we want
// to notify the developer that we couldn't analyze parts of the project. Developers
// can just re-run the migration after fixing these failures.
if (syntacticDiagnostics.length) {
logger.warn(
`\nTypeScript project "${tsconfigPath}" has syntactical errors which could cause ` +
`an incomplete migration. Please fix the following failures and rerun the migration:`);
logger.error(ts.formatDiagnostics(syntacticDiagnostics, host));
logger.info(
'Migration can be rerun with: "ng update @angular/core --from 7 --to 8 --migrate-only"\n');
}
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const queryVisitor = new NgQueryResolveVisitor(typeChecker);
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const queryVisitor = new NgQueryResolveVisitor(typeChecker);
// Analyze all project source-files and collect all queries that
// need to be migrated.
sourceFiles.forEach(sourceFile => {
const relativePath = relative(basePath, sourceFile.fileName);
// Analyze all project source-files and collect all queries that
// need to be migrated.
sourceFiles.forEach(sourceFile => {
const relativePath = relative(basePath, sourceFile.fileName);
// Only look for queries within the current source files if the
// file has not been analyzed before.
if (!analyzedFiles.has(relativePath)) {
analyzedFiles.add(relativePath);
queryVisitor.visitNode(sourceFile);
}
});
if (queryVisitor.resolvedQueries.size === 0) {
return null;
}
return {program, host, tsconfigPath, typeChecker, basePath, queryVisitor, sourceFiles};
// Only look for queries within the current source files if the
// file has not been analyzed before.
if (!analyzedFiles.has(relativePath)) {
analyzedFiles.add(relativePath);
queryVisitor.visitNode(sourceFile);
}
});
if (queryVisitor.resolvedQueries.size === 0) {
return null;
}
return {program, host, tsconfigPath, typeChecker, basePath, queryVisitor, sourceFiles};
}
/**
* Runs the static query migration for the given project. The schematic analyzes all
@ -179,7 +175,7 @@ async function runStaticQueryMigration(
// is necessary in order to be able to check component templates for static query usage.
resolvedTemplates.forEach(template => {
if (classMetadata.has(template.container)) {
classMetadata.get(template.container) !.template = template;
classMetadata.get(template.container)!.template = template;
}
});
}

View File

@ -8,13 +8,11 @@
import {logging, normalize} from '@angular-devkit/core';
import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
import {dirname, relative} from 'path';
import * as ts from 'typescript';
import {relative} from 'path';
import {NgComponentTemplateVisitor} from '../../utils/ng_component_template';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {analyzeResolvedTemplate} from './analyze_template';
@ -47,9 +45,7 @@ export default function(): Rule {
*/
function runTemplateVariableAssignmentCheck(
tree: Tree, tsconfigPath: string, basePath: string, logger: Logger) {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = createMigrationCompilerHost(tree, parsed.options, basePath);
const program = ts.createProgram(parsed.fileNames, parsed.options, host);
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const templateVisitor = new NgComponentTemplateVisitor(typeChecker);
const sourceFiles = program.getSourceFiles().filter(

View File

@ -7,13 +7,14 @@
*/
import {Rule, SchematicsException, Tree,} from '@angular-devkit/schematics';
import {dirname, relative} from 'path';
import {relative} from 'path';
import * as ts from 'typescript';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host';
import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig';
import {UpdateRecorder} from './update_recorder';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {UndecoratedClassesWithDecoratedFieldsTransform} from './transform';
import {UpdateRecorder} from './update_recorder';
/**
* Migration that adds an Angular decorator to classes that have Angular field decorators.
@ -37,9 +38,7 @@ export default function(): Rule {
}
function runUndecoratedClassesMigration(tree: Tree, tsconfigPath: string, basePath: string) {
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = createMigrationCompilerHost(tree, parsed.options, basePath);
const program = ts.createProgram(parsed.fileNames, parsed.options, host);
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
file => !file.isDeclarationFile && !program.isSourceFileFromExternalLibrary(file));
@ -61,7 +60,7 @@ function runUndecoratedClassesMigration(tree: Tree, tsconfigPath: string, basePa
/** Gets the update recorder for the specified source file. */
function getUpdateRecorder(sourceFile: ts.SourceFile): UpdateRecorder {
if (updateRecorders.has(sourceFile)) {
return updateRecorders.get(sourceFile) !;
return updateRecorders.get(sourceFile)!;
}
const treeRecorder = tree.beginUpdate(relative(basePath, sourceFile.fileName));
const recorder: UpdateRecorder = {
@ -81,7 +80,9 @@ function runUndecoratedClassesMigration(tree: Tree, tsconfigPath: string, basePa
treeRecorder.remove(namedBindings.getStart(), namedBindings.getWidth());
treeRecorder.insertRight(namedBindings.getStart(), newNamedBindings);
},
commitUpdate() { tree.commitUpdate(treeRecorder); }
commitUpdate() {
tree.commitUpdate(treeRecorder);
}
};
updateRecorders.set(sourceFile, recorder);
return recorder;

View File

@ -0,0 +1,96 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {getSystemPath, normalize, virtualFs} from '@angular-devkit/core';
import {TempScopedNodeJsSyncHost} from '@angular-devkit/core/node/testing';
import {HostTree} from '@angular-devkit/schematics';
import {SchematicTestRunner, UnitTestTree} from '@angular-devkit/schematics/testing';
import * as shx from 'shelljs';
describe('all migrations', () => {
let runner: SchematicTestRunner;
let host: TempScopedNodeJsSyncHost;
let tree: UnitTestTree;
let tmpDirPath: string;
let previousWorkingDir: string;
const migrationCollectionPath = require.resolve('../migrations.json');
const allMigrationSchematics = Object.keys(require(migrationCollectionPath).schematics);
beforeEach(() => {
runner = new SchematicTestRunner('test', migrationCollectionPath);
host = new TempScopedNodeJsSyncHost();
tree = new UnitTestTree(new HostTree(host));
writeFile('/node_modules/@angular/core/index.d.ts', `export const MODULE: any;`);
writeFile('/angular.json', JSON.stringify({
projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}}
}));
writeFile('/tsconfig.json', `{}`);
previousWorkingDir = shx.pwd();
tmpDirPath = getSystemPath(host.root);
// Switch into the temporary directory path. This allows us to run
// the schematic against our custom unit test tree.
shx.cd(tmpDirPath);
});
afterEach(() => {
shx.cd(previousWorkingDir);
shx.rm('-r', tmpDirPath);
});
function writeFile(filePath: string, contents: string) {
host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents));
}
async function runMigration(migrationName: string) {
await runner.runSchematicAsync(migrationName, undefined, tree).toPromise();
}
if (!allMigrationSchematics.length) {
throw Error('No migration schematics found.');
}
allMigrationSchematics.forEach(name => {
describe(name, () => createTests(name));
});
function createTests(migrationName: string) {
// Regression test for: https://github.com/angular/angular/issues/36346.
it('should not throw if non-existent symbols are imported with rootDirs', async () => {
writeFile(`/tsconfig.json`, JSON.stringify({
compilerOptions: {
rootDirs: [
'./generated',
]
}
}));
writeFile('/index.ts', `
import {Renderer} from '@angular/core';
const variableDecl: Renderer = null;
export class Test {
constructor(renderer: Renderer) {}
}
`);
let error: any = null;
try {
await runMigration(migrationName);
} catch (e) {
error = e;
}
expect(error).toBe(null);
});
}
});

View File

@ -6,12 +6,39 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Tree} from '@angular-devkit/schematics';
import {relative} from 'path';
import {dirname, relative, resolve} from 'path';
import * as ts from 'typescript';
import {parseTsconfigFile} from './parse_tsconfig';
export type FakeReadFileFn = (fileName: string) => string|null;
/**
* Creates a TypeScript program instance for a TypeScript project within
* the virtual file system tree.
* @param tree Virtual file system tree that contains the source files.
* @param tsconfigPath Virtual file system path that resolves to the TypeScript project.
* @param basePath Base path for the virtual file system tree.
* @param fakeFileRead Optional file reader function. Can be used to overwrite files in
* the TypeScript program, or to add in-memory files (e.g. to add global types).
* @param additionalFiles Additional file paths that should be added to the program.
*/
export function createMigrationProgram(
tree: Tree, tsconfigPath: string, basePath: string, fakeFileRead?: FakeReadFileFn,
additionalFiles?: string[]) {
// Resolve the tsconfig path to an absolute path. This is needed as TypeScript otherwise
// is not able to resolve root directories in the given tsconfig. More details can be found
// in the following issue: https://github.com/microsoft/TypeScript/issues/37731.
tsconfigPath = resolve(basePath, tsconfigPath);
const parsed = parseTsconfigFile(tsconfigPath, dirname(tsconfigPath));
const host = createMigrationCompilerHost(tree, parsed.options, basePath, fakeFileRead);
const program =
ts.createProgram(parsed.fileNames.concat(additionalFiles || []), parsed.options, host);
return {parsed, host, program};
}
export function createMigrationCompilerHost(
tree: Tree, options: ts.CompilerOptions, basePath: string,
fakeRead?: (fileName: string) => string | null): ts.CompilerHost {
fakeRead?: FakeReadFileFn): ts.CompilerHost {
const host = ts.createCompilerHost(options, true);
// We need to overwrite the host "readFile" method, as we want the TypeScript

View File

@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import * as path from 'path';
import * as ts from 'typescript';
export function parseTsconfigFile(tsconfigPath: string, basePath: string): ts.ParsedCommandLine {
@ -17,5 +18,12 @@ export function parseTsconfigFile(tsconfigPath: string, basePath: string): ts.Pa
readFile: ts.sys.readFile,
};
// Throw if incorrect arguments are passed to this function. Passing relative base paths
// results in root directories not being resolved and in later type checking runtime errors.
// More details can be found here: https://github.com/microsoft/TypeScript/issues/37731.
if (!path.isAbsolute(basePath)) {
throw Error('Unexpected relative base path has been specified.');
}
return ts.parseJsonConfigFileContent(config, parseConfigHost, basePath, {});
}