Skip to content

Commit

Permalink
fix(core): migration error if program contains files outside of the p…
Browse files Browse the repository at this point in the history
…roject (#39790)

Currently all of our migrations are set up to find the tsconfig paths within a project,
create a `Program` out of each and migrate the files inside of the `Program`. The
problem is that the `Program` can include files outside of the project and the CLI
APIs that we use to interact with the file system assume that all files are within
the project.

These changes consolidate the logic, that determines whether a file can be migrated,
in a single place and add an extra check to exclude files outside of the root.

Fixes #39778.

PR Close #39790
  • Loading branch information
crisbeto authored and AndrewKushnir committed Nov 20, 2020
1 parent b1ebfb1 commit 1a26f6d
Show file tree
Hide file tree
Showing 19 changed files with 72 additions and 64 deletions.
Expand Up @@ -10,7 +10,7 @@ import {Rule, SchematicsException, Tree} from '@angular-devkit/schematics';
import {relative} from 'path';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {findParentAccesses} from './util';


Expand All @@ -36,8 +36,8 @@ function runNativeAbstractControlParentMigration(
tree: Tree, tsconfigPath: string, basePath: string) {
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

sourceFiles.forEach(sourceFile => {
// We sort the nodes based on their position in the file and we offset the positions by one
Expand Down
6 changes: 3 additions & 3 deletions packages/core/schematics/migrations/dynamic-queries/index.ts
Expand Up @@ -11,7 +11,7 @@ import {relative} from 'path';
import * as ts from 'typescript';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';

import {identifyDynamicQueryNodes, removeOptionsParameter, removeStaticFlag} from './util';

Expand Down Expand Up @@ -39,8 +39,8 @@ export default function(): Rule {
function runDynamicQueryMigration(tree: Tree, tsconfigPath: string, basePath: string) {
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));
const printer = ts.createPrinter();

sourceFiles.forEach(sourceFile => {
Expand Down
Expand Up @@ -31,7 +31,7 @@ export class Rule extends Rules.TypedRule {
sourceFiles.forEach(sourceFile => initialNavigationCollector.visitNode(sourceFile));

const {assignments} = initialNavigationCollector;
const transformer = new InitialNavigationTransform(typeChecker, getUpdateRecorder);
const transformer = new InitialNavigationTransform(getUpdateRecorder);
const updateRecorders = new Map<ts.SourceFile, TslintUpdateRecorder>();

transformer.migrateInitialNavigationAssignments(Array.from(assignments));
Expand Down
Expand Up @@ -10,7 +10,7 @@ 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 {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {InitialNavigationCollector} from './collector';
import {InitialNavigationTransform} from './transform';
import {UpdateRecorder} from './update_recorder';
Expand All @@ -36,14 +36,14 @@ function runInitialNavigationMigration(tree: Tree, tsconfigPath: string, basePat
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const initialNavigationCollector = new InitialNavigationCollector(typeChecker);
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

// Analyze source files by detecting all modules.
sourceFiles.forEach(sourceFile => initialNavigationCollector.visitNode(sourceFile));

const {assignments} = initialNavigationCollector;
const transformer = new InitialNavigationTransform(typeChecker, getUpdateRecorder);
const transformer = new InitialNavigationTransform(getUpdateRecorder);
const updateRecorders = new Map<ts.SourceFile, UpdateRecorder>();
transformer.migrateInitialNavigationAssignments(Array.from(assignments));

Expand Down
Expand Up @@ -13,9 +13,7 @@ import {UpdateRecorder} from './update_recorder';
export class InitialNavigationTransform {
private printer = ts.createPrinter();

constructor(
private typeChecker: ts.TypeChecker,
private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder) {}
constructor(private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder) {}

/** Migrate the ExtraOptions#InitialNavigation property assignments. */
migrateInitialNavigationAssignments(literals: ts.PropertyAssignment[]) {
Expand Down Expand Up @@ -62,14 +60,3 @@ function getUpdatedInitialNavigationValue(initializer: ts.Expression): ts.Expres

return !!newText ? ts.createIdentifier(`'${newText}'`) : null;
}

/**
* Check whether the value assigned to an `initialNavigation` assignment
* conforms to the expected types for ExtraOptions#InitialNavigation
* @param node the property assignment to check
*/
function isValidInitialNavigationValue(node: ts.PropertyAssignment): boolean {
return ts.isStringLiteralLike(node.initializer) ||
node.initializer.kind === ts.SyntaxKind.FalseKeyword ||
node.initializer.kind === ts.SyntaxKind.TrueKeyword;
}
Expand Up @@ -10,7 +10,7 @@ import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit
import {relative} from 'path';
import * as ts from 'typescript';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {NgDefinitionCollector} from './definition_collector';
import {MissingInjectableTransform} from './transform';
import {UpdateRecorder} from './update_recorder';
Expand Down Expand Up @@ -46,8 +46,8 @@ function runMissingInjectableMigration(
const failures: string[] = [];
const typeChecker = program.getTypeChecker();
const definitionCollector = new NgDefinitionCollector(typeChecker);
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

// Analyze source files by detecting all modules, directives and components.
sourceFiles.forEach(sourceFile => definitionCollector.visitNode(sourceFile));
Expand Down
Expand Up @@ -11,7 +11,7 @@ import {relative} from 'path';
import * as ts from 'typescript';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';

import {Collector} from './collector';
import {AnalysisFailure, ModuleWithProvidersTransform} from './transform';
Expand Down Expand Up @@ -49,8 +49,8 @@ function runModuleWithProvidersMigration(tree: Tree, tsconfigPath: string, baseP
const failures: string[] = [];
const typeChecker = program.getTypeChecker();
const collector = new Collector(typeChecker);
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

// Analyze source files by detecting all modules.
sourceFiles.forEach(sourceFile => collector.visitNode(sourceFile));
Expand Down
6 changes: 3 additions & 3 deletions packages/core/schematics/migrations/move-document/index.ts
Expand Up @@ -11,7 +11,7 @@ import {relative} from 'path';
import * as ts from 'typescript';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, 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';
Expand Down Expand Up @@ -43,8 +43,8 @@ function runMoveDocumentMigration(tree: Tree, tsconfigPath: string, basePath: st
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const visitor = new DocumentImportVisitor(typeChecker);
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

// Analyze source files by finding imports.
sourceFiles.forEach(sourceFile => visitor.visitNode(sourceFile));
Expand Down
Expand Up @@ -10,7 +10,7 @@ import {Rule, SchematicsException, Tree} from '@angular-devkit/schematics';
import {relative} from 'path';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {findNativeEncapsulationNodes} from './util';


Expand All @@ -35,8 +35,8 @@ export default function(): Rule {
function runNativeViewEncapsulationMigration(tree: Tree, tsconfigPath: string, basePath: string) {
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

sourceFiles.forEach(sourceFile => {
const update = tree.beginUpdate(relative(basePath, sourceFile.fileName));
Expand Down
Expand Up @@ -11,7 +11,7 @@ import {relative} from 'path';
import * as ts from 'typescript';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {findLiteralsToMigrate, migrateLiteral} from './util';


Expand All @@ -38,8 +38,8 @@ function runNavigationExtrasOmissionsMigration(tree: Tree, tsconfigPath: string,
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const printer = ts.createPrinter();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

sourceFiles.forEach(sourceFile => {
const literalsToMigrate = findLiteralsToMigrate(sourceFile, typeChecker);
Expand Down
Expand Up @@ -10,7 +10,7 @@ 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 {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {RelativeLinkResolutionCollector} from './collector';
import {RelativeLinkResolutionTransform} from './transform';
import {UpdateRecorder} from './update_recorder';
Expand All @@ -36,8 +36,8 @@ function runRelativeLinkResolutionMigration(tree: Tree, tsconfigPath: string, ba
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const relativeLinkResolutionCollector = new RelativeLinkResolutionCollector(typeChecker);
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

// Analyze source files by detecting all modules.
sourceFiles.forEach(sourceFile => relativeLinkResolutionCollector.visitNode(sourceFile));
Expand Down
Expand Up @@ -11,7 +11,7 @@ import {relative} from 'path';
import * as ts from 'typescript';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {getImportSpecifier, replaceImport} from '../../utils/typescript/imports';
import {closestNode} from '../../utils/typescript/nodes';

Expand Down Expand Up @@ -59,8 +59,8 @@ function runRendererToRenderer2Migration(tree: Tree, tsconfigPath: string, baseP
}, [MODULE_AUGMENTATION_FILENAME]);
const typeChecker = program.getTypeChecker();
const printer = ts.createPrinter();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

sourceFiles.forEach(sourceFile => {
const rendererImportSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'Renderer');
Expand Down
Expand Up @@ -11,7 +11,7 @@ import {relative} from 'path';
import * as ts from 'typescript';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {findLiteralsToMigrate, migrateLiteral} from './util';


Expand Down Expand Up @@ -41,8 +41,8 @@ function runPreserveQueryParamsMigration(tree: Tree, tsconfigPath: string, baseP
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const printer = ts.createPrinter();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

sourceFiles.forEach(sourceFile => {
const literalsToMigrate = findLiteralsToMigrate(sourceFile, typeChecker);
Expand Down
6 changes: 3 additions & 3 deletions packages/core/schematics/migrations/static-queries/index.ts
Expand Up @@ -14,7 +14,7 @@ import * as ts from 'typescript';

import {NgComponentTemplateVisitor} from '../../utils/ng_component_template';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';

import {NgQueryResolveVisitor} from './angular/ng_query_visitor';
import {QueryTemplateStrategy} from './strategies/template_strategy/template_strategy';
Expand Down Expand Up @@ -123,8 +123,8 @@ function analyzeProject(
}

const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));
const queryVisitor = new NgQueryResolveVisitor(typeChecker);

// Analyze all project source-files and collect all queries that
Expand Down
Expand Up @@ -12,7 +12,7 @@ import {relative} from 'path';

import {NgComponentTemplateVisitor} from '../../utils/ng_component_template';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';

import {analyzeResolvedTemplate} from './analyze_template';

Expand Down Expand Up @@ -48,8 +48,8 @@ function runTemplateVariableAssignmentCheck(
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const templateVisitor = new NgComponentTemplateVisitor(typeChecker);
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

// Analyze source files by detecting HTML templates.
sourceFiles.forEach(sourceFile => templateVisitor.visitNode(sourceFile));
Expand Down
Expand Up @@ -11,7 +11,7 @@ import {relative} from 'path';
import * as ts from 'typescript';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';

import {UndecoratedClassesWithDecoratedFieldsTransform} from './transform';
import {UpdateRecorder} from './update_recorder';
Expand Down Expand Up @@ -49,8 +49,8 @@ function runUndecoratedClassesMigration(
const failures: string[] = [];
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
file => !file.isDeclarationFile && !program.isSourceFileFromExternalLibrary(file));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));
const updateRecorders = new Map<ts.SourceFile, UpdateRecorder>();
const transform =
new UndecoratedClassesWithDecoratedFieldsTransform(typeChecker, getUpdateRecorder);
Expand Down
Expand Up @@ -16,7 +16,7 @@ import {relative} from 'path';
import * as ts from 'typescript';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationCompilerHost} from '../../utils/typescript/compiler_host';

import {createNgcProgram} from './create_ngc_program';
import {NgDeclarationCollector} from './ng_declaration_collector';
Expand Down Expand Up @@ -85,8 +85,8 @@ function runUndecoratedClassesMigration(
const partialEvaluator = new PartialEvaluator(
new TypeScriptReflectionHost(typeChecker), typeChecker, /* dependencyTracker */ null);
const declarationCollector = new NgDeclarationCollector(typeChecker, partialEvaluator);
const sourceFiles = program.getSourceFiles().filter(
s => !s.isDeclarationFile && !program.isSourceFileFromExternalLibrary(s));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));

// Analyze source files by detecting all directives, components and providers.
sourceFiles.forEach(sourceFile => declarationCollector.visitNode(sourceFile));
Expand Down
6 changes: 3 additions & 3 deletions packages/core/schematics/migrations/wait-for-async/index.ts
Expand Up @@ -11,7 +11,7 @@ import {relative} from 'path';
import * as ts from 'typescript';

import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
import {getImportSpecifier, replaceImport} from '../../utils/typescript/imports';
import {closestNode} from '../../utils/typescript/nodes';

Expand Down Expand Up @@ -54,8 +54,8 @@ function runWaitForAsyncMigration(tree: Tree, tsconfigPath: string, basePath: st
}, [MODULE_AUGMENTATION_FILENAME]);
const typeChecker = program.getTypeChecker();
const printer = ts.createPrinter();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
const sourceFiles =
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));
const deprecatedFunction = 'async';
const newFunction = 'waitForAsync';

Expand Down

0 comments on commit 1a26f6d

Please sign in to comment.