feat(router): add migration to update calls to navigateByUrl and createUrlTree with invalid parameters (#38825)

In #38227 the signatures of `navigateByUrl` and `createUrlTree` were updated to exclude unsupported
properties from their `extras` parameter. This migration looks for the relevant method calls that
pass in an `extras` parameter and drops the unsupported properties.

**Before:**
```
this._router.navigateByUrl('/', {skipLocationChange: false, fragment: 'foo'});
```

**After:**
```
this._router.navigateByUrl('/', {
  /* Removed unsupported properties by Angular migration: fragment. */
  skipLocationChange: false
});
```

These changes also move the method call detection logic out of the `Renderer2` migration and into
a common place so that it can be reused in other migrations.

PR Close #38825
This commit is contained in:
Kristiyan Kostadinov 2020-09-12 14:33:06 +02:00 committed by Andrew Kushnir
parent 97adc27207
commit 7849fdde09
19 changed files with 1053 additions and 73 deletions

View File

@ -14,6 +14,7 @@ pkg_npm(
"//packages/core/schematics/migrations/missing-injectable",
"//packages/core/schematics/migrations/module-with-providers",
"//packages/core/schematics/migrations/move-document",
"//packages/core/schematics/migrations/navigation-extras-omissions",
"//packages/core/schematics/migrations/renderer-to-renderer2",
"//packages/core/schematics/migrations/static-queries",
"//packages/core/schematics/migrations/template-var-assignment",

View File

@ -44,6 +44,11 @@
"version": "10.0.0-beta",
"description": "Undecorated classes with Angular features migration. In version 10, classes that use Angular features and do not have an Angular decorator are no longer supported. Read more about this here: https://v10.angular.io/guide/migration-undecorated-classes",
"factory": "./migrations/undecorated-classes-with-decorated-fields/index"
},
"migration-v11-navigation-extras-omissions": {
"version": "11.0.0-beta",
"description": "NavigationExtras omissions migration. In version 11, some unsupported properties were omitted from the `extras` parameter of the `Router.navigateByUrl` and `Router.createUrlTree` methods.",
"factory": "./migrations/navigation-extras-omissions/index"
}
}
}

View File

@ -9,6 +9,7 @@ ts_library(
"//packages/core/schematics/migrations/dynamic-queries",
"//packages/core/schematics/migrations/missing-injectable",
"//packages/core/schematics/migrations/missing-injectable/google3",
"//packages/core/schematics/migrations/navigation-extras-omissions",
"//packages/core/schematics/migrations/renderer-to-renderer2",
"//packages/core/schematics/migrations/static-queries",
"//packages/core/schematics/migrations/template-var-assignment",

View File

@ -0,0 +1,38 @@
/**
* @license
* Copyright Google LLC 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 {Replacement, RuleFailure, Rules} from 'tslint';
import * as ts from 'typescript';
import {findLiteralsToMigrate, migrateLiteral} from '../../migrations/navigation-extras-omissions/util';
/** TSLint rule that migrates `navigateByUrl` and `createUrlTree` calls to an updated signature. */
export class Rule extends Rules.TypedRule {
applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] {
const failures: RuleFailure[] = [];
const typeChecker = program.getTypeChecker();
const printer = ts.createPrinter();
const literalsToMigrate = findLiteralsToMigrate(sourceFile, typeChecker);
literalsToMigrate.forEach((instances, methodName) => instances.forEach(instance => {
const migratedNode = migrateLiteral(methodName, instance);
if (migratedNode !== instance) {
failures.push(new RuleFailure(
sourceFile, instance.getStart(), instance.getEnd(),
'Object used in navigateByUrl or createUrlTree call contains unsupported properties.',
this.ruleName,
new Replacement(
instance.getStart(), instance.getWidth(),
printer.printNode(ts.EmitHint.Unspecified, migratedNode, sourceFile))));
}
}));
return failures;
}
}

View File

@ -8,10 +8,11 @@
import {Replacement, RuleFailure, Rules} from 'tslint';
import * as ts from 'typescript';
import {getImportSpecifier} from '../../utils/typescript/imports';
import {getHelper, HelperFunction} from '../renderer-to-renderer2/helpers';
import {migrateExpression, replaceImport} from '../renderer-to-renderer2/migration';
import {findCoreImport, findRendererReferences} from '../renderer-to-renderer2/util';
import {findRendererReferences, getNamedImports} from '../renderer-to-renderer2/util';
/**
* TSLint rule that migrates from `Renderer` to `Renderer2`. More information on how it works:
@ -22,18 +23,21 @@ export class Rule extends Rules.TypedRule {
const typeChecker = program.getTypeChecker();
const printer = ts.createPrinter();
const failures: RuleFailure[] = [];
const rendererImport = findCoreImport(sourceFile, 'Renderer');
const rendererImportSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'Renderer');
const rendererImport =
rendererImportSpecifier ? getNamedImports(rendererImportSpecifier) : null;
// If there are no imports for the `Renderer`, we can exit early.
if (!rendererImport) {
if (!rendererImportSpecifier || !rendererImport) {
return failures;
}
const {typedNodes, methodCalls, forwardRefs} =
findRendererReferences(sourceFile, typeChecker, rendererImport);
findRendererReferences(sourceFile, typeChecker, rendererImportSpecifier);
const helpersToAdd = new Set<HelperFunction>();
failures.push(this._getNamedImportsFailure(rendererImport, sourceFile, printer));
failures.push(
this._getNamedImportsFailure(rendererImport, rendererImportSpecifier, sourceFile, printer));
typedNodes.forEach(node => failures.push(this._getTypedNodeFailure(node, sourceFile)));
forwardRefs.forEach(node => failures.push(this._getIdentifierNodeFailure(node, sourceFile)));
@ -61,9 +65,10 @@ export class Rule extends Rules.TypedRule {
/** Gets a failure for an import of the Renderer. */
private _getNamedImportsFailure(
node: ts.NamedImports, sourceFile: ts.SourceFile, printer: ts.Printer): RuleFailure {
node: ts.NamedImports, importSpecifier: ts.ImportSpecifier, sourceFile: ts.SourceFile,
printer: ts.Printer): RuleFailure {
const replacementText = printer.printNode(
ts.EmitHint.Unspecified, replaceImport(node, 'Renderer', 'Renderer2'), sourceFile);
ts.EmitHint.Unspecified, replaceImport(node, importSpecifier, 'Renderer2'), sourceFile);
return new RuleFailure(
sourceFile, node.getStart(), node.getEnd(),

View File

@ -0,0 +1,18 @@
load("//tools:defaults.bzl", "ts_library")
ts_library(
name = "navigation-extras-omissions",
srcs = glob(["**/*.ts"]),
tsconfig = "//packages/core/schematics:tsconfig.json",
visibility = [
"//packages/core/schematics:__pkg__",
"//packages/core/schematics/migrations/google3:__pkg__",
"//packages/core/schematics/test:__pkg__",
],
deps = [
"//packages/core/schematics/utils",
"@npm//@angular-devkit/schematics",
"@npm//@types/node",
"@npm//typescript",
],
)

View File

@ -0,0 +1,35 @@
## Router.navigateByUrl and Router.createUrlTree extras migration
Previously the `extras` parameter of `Router.navigateByUrl` and `Router.createUrlTree` accepted the
full `NavigationExtras` object, even though only a subset of properties was supported. This
migration removes the unsupported properties from the relevant method call sites.
#### Before
```ts
import { Component } from '@angular/core';
import { Router } from '@angular/router';
@Component({})
export class MyComponent {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', {skipLocationChange: false, fragment: 'foo'});
}
}
```
#### After
```ts
import { Component } from '@angular/core';
import { Router } from '@angular/router';
@Component({})
export class MyComponent {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', { /* Removed unsupported properties by Angular migration: fragment. */ skipLocationChange: false });
}
}
```

View File

@ -0,0 +1,61 @@
/**
* @license
* Copyright Google LLC 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 {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 {findLiteralsToMigrate, migrateLiteral} from './util';
/** Migration that switches `Router.navigateByUrl` and `Router.createUrlTree` to a new signature. */
export default function(): Rule {
return (tree: Tree) => {
const {buildPaths, testPaths} = getProjectTsConfigPaths(tree);
const basePath = process.cwd();
const allPaths = [...buildPaths, ...testPaths];
if (!allPaths.length) {
throw new SchematicsException(
'Could not find any tsconfig file. Cannot migrate ' +
'Router.navigateByUrl and Router.createUrlTree calls.');
}
for (const tsconfigPath of allPaths) {
runNavigationExtrasOmissionsMigration(tree, tsconfigPath, basePath);
}
};
}
function runNavigationExtrasOmissionsMigration(tree: Tree, tsconfigPath: string, basePath: 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));
sourceFiles.forEach(sourceFile => {
const literalsToMigrate = findLiteralsToMigrate(sourceFile, typeChecker);
const update = tree.beginUpdate(relative(basePath, sourceFile.fileName));
literalsToMigrate.forEach((instances, methodName) => instances.forEach(instance => {
const migratedNode = migrateLiteral(methodName, instance);
if (migratedNode !== instance) {
update.remove(instance.getStart(), instance.getWidth());
update.insertRight(
instance.getStart(),
printer.printNode(ts.EmitHint.Unspecified, migratedNode, sourceFile));
}
}));
tree.commitUpdate(update);
});
}

View File

@ -0,0 +1,125 @@
/**
* @license
* Copyright Google LLC 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 * as ts from 'typescript';
import {getImportSpecifier} from '../../utils/typescript/imports';
import {isReferenceToImport} from '../../utils/typescript/symbol';
/**
* Configures the methods that the migration should be looking for
* and the properties from `NavigationExtras` that should be preserved.
*/
const methodConfig = new Map<string, Set<string>>([
['navigateByUrl', new Set<string>(['skipLocationChange', 'replaceUrl', 'state'])],
[
'createUrlTree', new Set<string>([
'relativeTo', 'queryParams', 'fragment', 'preserveQueryParams', 'queryParamsHandling',
'preserveFragment'
])
]
]);
export function migrateLiteral(
methodName: string, node: ts.ObjectLiteralExpression): ts.ObjectLiteralExpression {
const allowedProperties = methodConfig.get(methodName);
if (!allowedProperties) {
throw Error(`Attempting to migrate unconfigured method called ${methodName}.`);
}
const propertiesToKeep: ts.ObjectLiteralElementLike[] = [];
const removedPropertyNames: string[] = [];
node.properties.forEach(property => {
// Only look for regular and shorthand property assignments since resolving things
// like spread operators becomes too complicated for this migration.
if ((ts.isPropertyAssignment(property) || ts.isShorthandPropertyAssignment(property)) &&
(ts.isStringLiteralLike(property.name) || ts.isNumericLiteral(property.name) ||
ts.isIdentifier(property.name))) {
if (allowedProperties.has(property.name.text)) {
propertiesToKeep.push(property);
} else {
removedPropertyNames.push(property.name.text);
}
} else {
propertiesToKeep.push(property);
}
});
// Don't modify the node if there's nothing to remove.
if (removedPropertyNames.length === 0) {
return node;
}
// Note that the trailing/leading spaces are necessary so the comment looks good.
const removalComment =
` Removed unsupported properties by Angular migration: ${removedPropertyNames.join(', ')}. `;
if (propertiesToKeep.length > 0) {
propertiesToKeep[0] = addUniqueLeadingComment(propertiesToKeep[0], removalComment);
return ts.createObjectLiteral(propertiesToKeep);
} else {
return addUniqueLeadingComment(ts.createObjectLiteral(propertiesToKeep), removalComment);
}
}
export function findLiteralsToMigrate(sourceFile: ts.SourceFile, typeChecker: ts.TypeChecker) {
const results = new Map<string, Set<ts.ObjectLiteralExpression>>(
Array.from(methodConfig.keys(), key => [key, new Set()]));
const routerImport = getImportSpecifier(sourceFile, '@angular/router', 'Router');
const seenLiterals = new Map<ts.ObjectLiteralExpression, string>();
if (routerImport) {
sourceFile.forEachChild(function visitNode(node: ts.Node) {
// Look for calls that look like `foo.<method to migrate>` with more than one parameter.
if (ts.isCallExpression(node) && node.arguments.length > 1 &&
ts.isPropertyAccessExpression(node.expression) && ts.isIdentifier(node.expression.name) &&
methodConfig.has(node.expression.name.text)) {
// Check whether the type of the object on which the
// function is called refers to the Router import.
if (isReferenceToImport(typeChecker, node.expression.expression, routerImport)) {
const methodName = node.expression.name.text;
const parameterDeclaration =
typeChecker.getTypeAtLocation(node.arguments[1]).getSymbol()?.valueDeclaration;
// Find the source of the object literal.
if (parameterDeclaration && ts.isObjectLiteralExpression(parameterDeclaration)) {
if (!seenLiterals.has(parameterDeclaration)) {
results.get(methodName)!.add(parameterDeclaration);
seenLiterals.set(parameterDeclaration, methodName);
// If the same literal has been passed into multiple different methods, we can't
// migrate it, because the supported properties are different. When we detect such
// a case, we drop it from the results so that it gets ignored. If it's used multiple
// times for the same method, it can still be migrated.
} else if (seenLiterals.get(parameterDeclaration) !== methodName) {
results.forEach(literals => literals.delete(parameterDeclaration));
}
}
}
} else {
node.forEachChild(visitNode);
}
});
}
return results;
}
/** Adds a leading comment to a node, if the node doesn't have such a comment already. */
function addUniqueLeadingComment<T extends ts.Node>(node: T, comment: string): T {
const existingComments = ts.getSyntheticLeadingComments(node);
// This logic is primarily to ensure that we don't add the same comment multiple
// times when tslint runs over the same file again with outdated information.
if (!existingComments || existingComments.every(c => c.text !== comment)) {
return ts.addSyntheticLeadingComment(node, ts.SyntaxKind.MultiLineCommentTrivia, comment);
}
return node;
}

View File

@ -12,10 +12,11 @@ import * as ts from 'typescript';
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {getImportSpecifier} from '../../utils/typescript/imports';
import {getHelper, HelperFunction} from './helpers';
import {migrateExpression, replaceImport} from './migration';
import {findCoreImport, findRendererReferences} from './util';
import {findRendererReferences, getNamedImports} from './util';
const MODULE_AUGMENTATION_FILENAME = 'ɵɵRENDERER_MIGRATION_CORE_AUGMENTATION.d.ts';
@ -61,15 +62,17 @@ function runRendererToRenderer2Migration(tree: Tree, tsconfigPath: string, baseP
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
sourceFiles.forEach(sourceFile => {
const rendererImport = findCoreImport(sourceFile, 'Renderer');
const rendererImportSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'Renderer');
const rendererImport =
rendererImportSpecifier ? getNamedImports(rendererImportSpecifier) : null;
// If there are no imports for the `Renderer`, we can exit early.
if (!rendererImport) {
if (!rendererImportSpecifier || !rendererImport) {
return;
}
const {typedNodes, methodCalls, forwardRefs} =
findRendererReferences(sourceFile, typeChecker, rendererImport);
findRendererReferences(sourceFile, typeChecker, rendererImportSpecifier);
const update = tree.beginUpdate(relative(basePath, sourceFile.fileName));
const helpersToAdd = new Set<HelperFunction>();
@ -78,8 +81,8 @@ function runRendererToRenderer2Migration(tree: Tree, tsconfigPath: string, baseP
update.insertRight(
rendererImport.getStart(),
printer.printNode(
ts.EmitHint.Unspecified, replaceImport(rendererImport, 'Renderer', 'Renderer2'),
sourceFile));
ts.EmitHint.Unspecified,
replaceImport(rendererImport, rendererImportSpecifier, 'Renderer2'), sourceFile));
// Change the method parameter and property types to `Renderer2`.
typedNodes.forEach(node => {

View File

@ -9,31 +9,28 @@
import * as ts from 'typescript';
import {HelperFunction} from './helpers';
import {findImportSpecifier} from './util';
/** A call expression that is based on a property access. */
type PropertyAccessCallExpression = ts.CallExpression&{expression: ts.PropertyAccessExpression};
/** Replaces an import inside an import statement with a different one. */
export function replaceImport(node: ts.NamedImports, oldImport: string, newImport: string) {
const isAlreadyImported = findImportSpecifier(node.elements, newImport);
export function replaceImport(
node: ts.NamedImports, existingImport: ts.ImportSpecifier, newImportName: string) {
const isAlreadyImported = node.elements.find(element => {
const {name, propertyName} = element;
return propertyName ? propertyName.text === newImportName : name.text === newImportName;
});
if (isAlreadyImported) {
return node;
}
const existingImport = findImportSpecifier(node.elements, oldImport);
if (!existingImport) {
throw new Error(`Could not find an import to replace using ${oldImport}.`);
}
return ts.updateNamedImports(node, [
...node.elements.filter(current => current !== existingImport),
// Create a new import while trying to preserve the alias of the old one.
ts.createImportSpecifier(
existingImport.propertyName ? ts.createIdentifier(newImport) : undefined,
existingImport.propertyName ? existingImport.name : ts.createIdentifier(newImport))
existingImport.propertyName ? ts.createIdentifier(newImportName) : undefined,
existingImport.propertyName ? existingImport.name : ts.createIdentifier(newImportName))
]);
}

View File

@ -8,30 +8,32 @@
import * as ts from 'typescript';
import {getImportSpecifier} from '../../utils/typescript/imports';
import {isReferenceToImport} from '../../utils/typescript/symbol';
/**
* Finds typed nodes (e.g. function parameters or class properties) that are referencing the old
* `Renderer`, as well as calls to the `Renderer` methods.
*/
export function findRendererReferences(
sourceFile: ts.SourceFile, typeChecker: ts.TypeChecker, rendererImport: ts.NamedImports) {
sourceFile: ts.SourceFile, typeChecker: ts.TypeChecker,
rendererImportSpecifier: ts.ImportSpecifier) {
const typedNodes = new Set<ts.ParameterDeclaration|ts.PropertyDeclaration|ts.AsExpression>();
const methodCalls = new Set<ts.CallExpression>();
const forwardRefs = new Set<ts.Identifier>();
const importSpecifier = findImportSpecifier(rendererImport.elements, 'Renderer');
const forwardRefImport = findCoreImport(sourceFile, 'forwardRef');
const forwardRefSpecifier =
forwardRefImport ? findImportSpecifier(forwardRefImport.elements, 'forwardRef') : null;
const forwardRefSpecifier = getImportSpecifier(sourceFile, '@angular/core', 'forwardRef');
ts.forEachChild(sourceFile, function visitNode(node: ts.Node) {
if ((ts.isParameter(node) || ts.isPropertyDeclaration(node)) &&
isReferenceToImport(typeChecker, node.name, importSpecifier)) {
isReferenceToImport(typeChecker, node.name, rendererImportSpecifier)) {
typedNodes.add(node);
} else if (
ts.isAsExpression(node) && isReferenceToImport(typeChecker, node.type, importSpecifier)) {
ts.isAsExpression(node) &&
isReferenceToImport(typeChecker, node.type, rendererImportSpecifier)) {
typedNodes.add(node);
} else if (ts.isCallExpression(node)) {
if (ts.isPropertyAccessExpression(node.expression) &&
isReferenceToImport(typeChecker, node.expression.expression, importSpecifier)) {
isReferenceToImport(typeChecker, node.expression.expression, rendererImportSpecifier)) {
methodCalls.add(node);
} else if (
// If we're dealing with a forwardRef that's returning a Renderer.
@ -39,7 +41,7 @@ export function findRendererReferences(
isReferenceToImport(typeChecker, node.expression, forwardRefSpecifier) &&
node.arguments.length) {
const rendererIdentifier =
findRendererIdentifierInForwardRef(typeChecker, node, importSpecifier);
findRendererIdentifierInForwardRef(typeChecker, node, rendererImportSpecifier);
if (rendererIdentifier) {
forwardRefs.add(rendererIdentifier);
}
@ -52,59 +54,27 @@ export function findRendererReferences(
return {typedNodes, methodCalls, forwardRefs};
}
/** Finds the import from @angular/core that has a symbol with a particular name. */
export function findCoreImport(sourceFile: ts.SourceFile, symbolName: string): ts.NamedImports|
null {
// Only look through the top-level imports.
for (const node of sourceFile.statements) {
if (!ts.isImportDeclaration(node) || !ts.isStringLiteral(node.moduleSpecifier) ||
node.moduleSpecifier.text !== '@angular/core') {
continue;
}
/** Gets the closest `NamedImports` to an `ImportSpecifier`. */
export function getNamedImports(specifier: ts.ImportSpecifier): ts.NamedImports|null {
let current: ts.Node = specifier;
const namedBindings = node.importClause && node.importClause.namedBindings;
if (!namedBindings || !ts.isNamedImports(namedBindings)) {
continue;
}
if (findImportSpecifier(namedBindings.elements, symbolName)) {
return namedBindings;
while (current && !ts.isSourceFile(current)) {
if (ts.isNamedImports(current)) {
return current;
}
current = current.parent;
}
return null;
}
/** Finds an import specifier with a particular name, accounting for aliases. */
export function findImportSpecifier(
elements: ts.NodeArray<ts.ImportSpecifier>, importName: string) {
return elements.find(element => {
const {name, propertyName} = element;
return propertyName ? propertyName.text === importName : name.text === importName;
}) ||
null;
}
/** Checks whether a node is referring to an import spcifier. */
function isReferenceToImport(
typeChecker: ts.TypeChecker, node: ts.Node, importSpecifier: ts.ImportSpecifier|null): boolean {
if (importSpecifier) {
const nodeSymbol = typeChecker.getTypeAtLocation(node).getSymbol();
const importSymbol = typeChecker.getTypeAtLocation(importSpecifier).getSymbol();
return !!(nodeSymbol && importSymbol) &&
nodeSymbol.valueDeclaration === importSymbol.valueDeclaration;
}
return false;
}
/** Finds the identifier referring to the `Renderer` inside a `forwardRef` call expression. */
function findRendererIdentifierInForwardRef(
typeChecker: ts.TypeChecker, node: ts.CallExpression,
rendererImport: ts.ImportSpecifier|null): ts.Identifier|null {
const firstArg = node.arguments[0];
if (ts.isArrowFunction(firstArg)) {
if (ts.isArrowFunction(firstArg) && rendererImport) {
// Check if the function is `forwardRef(() => Renderer)`.
if (ts.isIdentifier(firstArg.body) &&
isReferenceToImport(typeChecker, firstArg.body, rendererImport)) {

View File

@ -12,6 +12,7 @@ ts_library(
"//packages/core/schematics/migrations/missing-injectable",
"//packages/core/schematics/migrations/module-with-providers",
"//packages/core/schematics/migrations/move-document",
"//packages/core/schematics/migrations/navigation-extras-omissions",
"//packages/core/schematics/migrations/renderer-to-renderer2",
"//packages/core/schematics/migrations/static-queries",
"//packages/core/schematics/migrations/template-var-assignment",

View File

@ -0,0 +1,334 @@
/**
* @license
* Copyright Google LLC 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 {readFileSync, writeFileSync} from 'fs';
import {dirname, join} from 'path';
import * as shx from 'shelljs';
import {Configuration, Linter} from 'tslint';
describe('Google3 NavigationExtras omissions TSLint rule', () => {
const rulesDirectory =
dirname(require.resolve('../../migrations/google3/navigationExtrasOmissionsRule'));
let tmpDir: string;
beforeEach(() => {
tmpDir = join(process.env['TEST_TMPDIR']!, 'google3-test');
shx.mkdir('-p', tmpDir);
// We need to declare the Angular symbols we're testing for, otherwise type checking won't work.
writeFile('router.d.ts', `
export declare class Router {
navigateByUrl(url: string, extras?: any);
createUrlTree(commands: any[], extras?: any);
}
`);
writeFile('tsconfig.json', JSON.stringify({
compilerOptions: {
module: 'es2015',
baseUrl: './',
paths: {
'@angular/router': ['router.d.ts'],
}
},
}));
});
afterEach(() => shx.rm('-r', tmpDir));
function runTSLint(fix: boolean) {
const program = Linter.createProgram(join(tmpDir, 'tsconfig.json'));
const linter = new Linter({fix, rulesDirectory: [rulesDirectory]}, program);
const config = Configuration.parseConfigFile({rules: {'navigation-extras-omissions': true}});
program.getRootFileNames().forEach(fileName => {
linter.lint(fileName, program.getSourceFile(fileName)!.getFullText(), config);
});
return linter;
}
function writeFile(fileName: string, content: string) {
writeFileSync(join(tmpDir, fileName), content);
}
function getFile(fileName: string) {
return readFileSync(join(tmpDir, fileName), 'utf8');
}
it('should flag objects with invalid properties used inside the relevant method calls', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', {fragment: 'foo'});
}
createTree() {
return this._router.createUrlTree(['/'], {state: {}});
}
goAway() {
this._router.navigateByUrl('/away');
}
}
`);
const linter = runTSLint(false);
const failures = linter.getResult().failures.map(failure => failure.getFailure());
expect(failures.length).toBe(2);
expect(failures[0])
.toMatch(
/Object used in navigateByUrl or createUrlTree call contains unsupported properties/);
expect(failures[1])
.toMatch(
/Object used in navigateByUrl or createUrlTree call contains unsupported properties/);
});
it('should not change calls with a single argument', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/');
}
}
function createTree(router: Router) {
return router.createUrlTree(['/']);
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(`this._router.navigateByUrl('/');`);
expect(content).toContain(`return router.createUrlTree(['/']);`);
});
it('should not change calls with an empty object literal', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', {});
}
}
function createTree(router: Router) {
return router.createUrlTree(['/'], {});
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(`this._router.navigateByUrl('/', {});`);
expect(content).toContain(`return router.createUrlTree(['/'], {});`);
});
it('should not change objects that are used in multiple different methods', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
const config = {replaceUrl: true, fragment: 'foo', state: {}};
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', config);
return this._router.createUrlTree(['/'], config);
}
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(`const config = {replaceUrl: true, fragment: 'foo', state: {}};`);
});
it('should preserve calls if the router does not come from @angular/router', () => {
writeFile('/index.ts', `
import {Router} from '@custom/router';
function createTree(router: Router) {
return router.createUrlTree(['/'], {foo: 1, bar: 2});
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(`return router.createUrlTree(['/'], {foo: 1, bar: 2});`);
});
it('should change invalid navigateByUrl calls', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', {preserveFragment: false, skipLocationChange: false, fragment: 'foo'});
}
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(
`this._router.navigateByUrl('/', { /* Removed unsupported properties by Angular migration: preserveFragment, fragment. */ skipLocationChange: false });`);
});
it('should change invalid navigateByUrl calls', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
function createTree(router: Router) {
return router.createUrlTree(['/'], {replaceUrl: true, preserveFragment: true, state: {}});
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(
`return router.createUrlTree(['/'], { /* Removed unsupported properties by Angular migration: replaceUrl, state. */ preserveFragment: true });`);
});
it('should set the comment outside the object if all properties were removed', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
function navigate(router: Router) {
router.navigateByUrl('/', {fragment: 'foo'});
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(
`router.navigateByUrl('/', /* Removed unsupported properties by Angular migration: fragment. */ {});`);
});
it('should migrate object literals defined as variables', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
const config = {skipLocationChange: false, fragment: 'foo'};
const proxy = config;
function navigate(router: Router) {
router.navigateByUrl('/', proxy);
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(
`const config = { /* Removed unsupported properties by Angular migration: fragment. */ skipLocationChange: false };`);
expect(content).toContain(`const proxy = config;`);
expect(content).toContain(`router.navigateByUrl('/', proxy);`);
});
it('should pick up calls where the router is returned by a function', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
function navigate(router: Router) {
getRouter().navigateByUrl('/', {fragment: 'foo'});
}
function getRouter() {
return {} as Router;
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(
`getRouter().navigateByUrl('/', /* Removed unsupported properties by Angular migration: fragment. */ {});`);
});
it('should pick up calls where the router is aliased', () => {
writeFile('/index.ts', `
import {Router as AliasedRouter} from '@angular/router';
function navigate(router: AliasedRouter) {
router.navigateByUrl('/', {fragment: 'foo'});
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(
`router.navigateByUrl('/', /* Removed unsupported properties by Angular migration: fragment. */ {});`);
});
it('should preserve object spread assignments', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
function navigate(router: Router) {
const overrides = {foo: 1};
router.navigateByUrl('/', {replaceUrl: true, fragment: 'foo', ...overrides});
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(
`router.navigateByUrl('/', { /* Removed unsupported properties by Angular migration: fragment. */ replaceUrl: true, ...overrides });`);
});
it('should migrate objects that are used in multiple calls of the same method', () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
const config = {skipLocationChange: false, fragment: 'foo'};
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', config);
}
goFish() {
this._router.navigateByUrl('/fish', config);
}
}
`);
runTSLint(true);
const content = getFile('/index.ts');
expect(content).toContain(
`const config = { /* Removed unsupported properties by Angular migration: fragment. */ skipLocationChange: false };`);
});
});

View File

@ -105,6 +105,26 @@ describe('Google3 Renderer to Renderer2 TSLint rule', () => {
expect(content).toContain('(renderer: Renderer2)');
});
it('should not change Renderer imports if Renderer2 is already imported', () => {
writeFile('/index.ts', `
import { Renderer, Component, Renderer2 } from '@angular/core';
@Component({template: ''})
export class MyComp {
public renderer: Renderer;
constructor(renderer: Renderer) {
this.renderer = renderer;
}
}
`);
runTSLint(true);
const content = getFile('index.ts');
expect(content).toContain(`import { Renderer, Component, Renderer2 } from '@angular/core';`);
});
it('should change Renderer inside single-line forwardRefs to Renderer2', () => {
writeFile('/index.ts', `
import { Renderer, Component, forwardRef, Inject } from '@angular/core';

View File

@ -0,0 +1,302 @@
/**
* @license
* Copyright Google LLC 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('NavigationExtras omissions migration', () => {
let runner: SchematicTestRunner;
let host: TempScopedNodeJsSyncHost;
let tree: UnitTestTree;
let tmpDirPath: string;
let previousWorkingDir: string;
beforeEach(() => {
runner = new SchematicTestRunner('test', require.resolve('../migrations.json'));
host = new TempScopedNodeJsSyncHost();
tree = new UnitTestTree(new HostTree(host));
writeFile('/tsconfig.json', JSON.stringify({
compilerOptions: {
lib: ['es2015'],
strictNullChecks: true,
},
}));
writeFile('/angular.json', JSON.stringify({
projects: {t: {architect: {build: {options: {tsConfig: './tsconfig.json'}}}}}
}));
// We need to declare the Angular symbols we're testing for, otherwise type checking won't work.
writeFile('/node_modules/@angular/router/index.d.ts', `
export declare class Router {
navigateByUrl(url: string, extras?: any);
createUrlTree(commands: any[], extras?: any);
}
`);
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);
});
it('should not change calls with a single argument', async () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/');
}
}
function createTree(router: Router) {
return router.createUrlTree(['/']);
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`this._router.navigateByUrl('/');`);
expect(content).toContain(`return router.createUrlTree(['/']);`);
});
it('should not change calls with an empty object literal', async () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', {});
}
}
function createTree(router: Router) {
return router.createUrlTree(['/'], {});
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`this._router.navigateByUrl('/', {});`);
expect(content).toContain(`return router.createUrlTree(['/'], {});`);
});
it('should not change objects that are used in multiple different methods', async () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
const config = {replaceUrl: true, fragment: 'foo', state: {}};
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', config);
return this._router.createUrlTree(['/'], config);
}
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`const config = {replaceUrl: true, fragment: 'foo', state: {}};`);
});
it('should preserve calls if the router does not come from @angular/router', async () => {
writeFile('/index.ts', `
import {Router} from '@custom/router';
function createTree(router: Router) {
return router.createUrlTree(['/'], {foo: 1, bar: 2});
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`return router.createUrlTree(['/'], {foo: 1, bar: 2});`);
});
it('should change invalid navigateByUrl calls', async () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', {preserveFragment: false, skipLocationChange: false, fragment: 'foo'});
}
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(
`this._router.navigateByUrl('/', { /* Removed unsupported properties by Angular migration: preserveFragment, fragment. */ skipLocationChange: false });`);
});
it('should change invalid navigateByUrl calls', async () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
function createTree(router: Router) {
return router.createUrlTree(['/'], {replaceUrl: true, preserveFragment: true, state: {}});
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(
`return router.createUrlTree(['/'], { /* Removed unsupported properties by Angular migration: replaceUrl, state. */ preserveFragment: true });`);
});
it('should set the comment outside the object if all properties were removed', async () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
function navigate(router: Router) {
router.navigateByUrl('/', {fragment: 'foo'});
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(
`router.navigateByUrl('/', /* Removed unsupported properties by Angular migration: fragment. */ {});`);
});
it('should migrate object literals defined as variables', async () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
const config = {skipLocationChange: false, fragment: 'foo'};
const proxy = config;
function navigate(router: Router) {
router.navigateByUrl('/', proxy);
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(
`const config = { /* Removed unsupported properties by Angular migration: fragment. */ skipLocationChange: false };`);
expect(content).toContain(`const proxy = config;`);
expect(content).toContain(`router.navigateByUrl('/', proxy);`);
});
it('should pick up calls where the router is returned by a function', async () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
function navigate(router: Router) {
getRouter().navigateByUrl('/', {fragment: 'foo'});
}
function getRouter() {
return {} as Router;
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(
`getRouter().navigateByUrl('/', /* Removed unsupported properties by Angular migration: fragment. */ {});`);
});
it('should pick up calls where the router is aliased', async () => {
writeFile('/index.ts', `
import {Router as AliasedRouter} from '@angular/router';
function navigate(router: AliasedRouter) {
router.navigateByUrl('/', {fragment: 'foo'});
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(
`router.navigateByUrl('/', /* Removed unsupported properties by Angular migration: fragment. */ {});`);
});
it('should preserve object spread assignments', async () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
function navigate(router: Router) {
const overrides = {foo: 1};
router.navigateByUrl('/', {replaceUrl: true, fragment: 'foo', ...overrides});
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(
`router.navigateByUrl('/', { /* Removed unsupported properties by Angular migration: fragment. */ replaceUrl: true, ...overrides });`);
});
it('should migrate objects that are used in multiple calls of the same method', async () => {
writeFile('/index.ts', `
import {Router} from '@angular/router';
const config = {skipLocationChange: false, fragment: 'foo'};
class Navigator {
constructor(private _router: Router) {}
goHome() {
this._router.navigateByUrl('/', config);
}
goFish() {
this._router.navigateByUrl('/fish', config);
}
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(
`const config = { /* Removed unsupported properties by Angular migration: fragment. */ skipLocationChange: false };`);
});
function writeFile(filePath: string, contents: string) {
host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents));
}
function runMigration() {
return runner.runSchematicAsync('migration-v11-navigation-extras-omissions', {}, tree)
.toPromise();
}
});

View File

@ -99,6 +99,21 @@ describe('Renderer to Renderer2 migration', () => {
expect(content).toContain(`import { Component } from '@angular/core';`);
expect(content).toContain(`import { Renderer } from './my-renderer';`);
});
it('should not change imports if Renderer2 was already imported', async () => {
writeFile('/index.ts', `
import { Renderer, Component, Renderer2 } from '@angular/core';
@Component({template: ''})
export class MyComp {
constructor(renderer: Renderer) {}
}
`);
await runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`import { Renderer, Component, Renderer2 } from '@angular/core';`);
});
});
describe('type renaming', () => {

View File

@ -42,3 +42,43 @@ export function getImportOfIdentifier(typeChecker: ts.TypeChecker, node: ts.Iden
node: importDecl
};
}
/**
* Gets a top-level import specifier with a specific name that is imported from a particular module.
* E.g. given a file that looks like:
*
* ```
* import { Component, Directive } from '@angular/core';
* import { Foo } from './foo';
* ```
*
* Calling `getImportSpecifier(sourceFile, '@angular/core', 'Directive')` will yield the node
* referring to `Directive` in the top import.
*
* @param sourceFile File in which to look for imports.
* @param moduleName Name of the import's module.
* @param specifierName Original name of the specifier to look for. Aliases will be resolved to
* their original name.
*/
export function getImportSpecifier(
sourceFile: ts.SourceFile, moduleName: string, specifierName: string): ts.ImportSpecifier|null {
for (const node of sourceFile.statements) {
if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier) &&
node.moduleSpecifier.text === moduleName) {
const namedBindings = node.importClause && node.importClause.namedBindings;
if (namedBindings && ts.isNamedImports(namedBindings)) {
const match = namedBindings.elements.find(element => {
const {name, propertyName} = element;
return propertyName ? propertyName.text === specifierName : name.text === specifierName;
});
if (match) {
return match;
}
}
}
}
return null;
}

View File

@ -18,3 +18,12 @@ export function getValueSymbolOfDeclaration(node: ts.Node, typeChecker: ts.TypeC
return symbol;
}
/** Checks whether a node is referring to a specific import specifier. */
export function isReferenceToImport(
typeChecker: ts.TypeChecker, node: ts.Node, importSpecifier: ts.ImportSpecifier): boolean {
const nodeSymbol = typeChecker.getTypeAtLocation(node).getSymbol();
const importSymbol = typeChecker.getTypeAtLocation(importSpecifier).getSymbol();
return !!(nodeSymbol && importSymbol) &&
nodeSymbol.valueDeclaration === importSymbol.valueDeclaration;
}