feat(core): Add schematic to fix invalid Route configs (#40067)

`Route` configs with `redirectTo` as well as `canActivate` are not valid
because the `canActivate` guards will never execute. Redirects are
applied before activation. There is no error currently for these
configs, but another commit will change this so that an error does
appear in dev mode. This migration fixes the configs by removing the
`canActivate` property.

PR Close #40067

packages/core/schematics/BUILD.bazel

@@ -11,6 +11,7 @@ pkg_npm(
     visibility = ["//packages/core:__pkg__"],
     deps = [
         "//packages/core/schematics/migrations/abstract-control-parent",
+        "//packages/core/schematics/migrations/can-activate-with-redirect-to",
         "//packages/core/schematics/migrations/dynamic-queries",
         "//packages/core/schematics/migrations/initial-navigation",
         "//packages/core/schematics/migrations/missing-injectable",

packages/core/schematics/migrations.json

@@ -79,6 +79,11 @@
       "version": "11.0.0-beta",
       "description": "Updates the `initialNavigation` property for `RouterModule.forRoot`.",
       "factory": "./migrations/initial-navigation/index"
+    },
+    "migration-v11.1-can-activate-with-redirect-to": {
+      "version": "11.1.0-beta",
+      "description": "Removes `canActivate` from a `Route` config when `redirectTo` is also present",
+      "factory": "./migrations/can-activate-with-redirect-to/index"
     }
   }
-}
+}

packages/core/schematics/migrations/can-activate-with-redirect-to/BUILD.bazel

@@ -0,0 +1,18 @@
+load("//tools:defaults.bzl", "ts_library")
+
+ts_library(
+    name = "can-activate-with-redirect-to",
+    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",
+    ],
+)

packages/core/schematics/migrations/can-activate-with-redirect-to/README.md

@@ -0,0 +1,23 @@
+## Router migration to remove canActivate property from Routes that also have redirectTo
+
+The activation stage of the router happens after redirects so any `canActivate` guards
+will not be executed. This invalid configuration is now an error. This migration 
+removes `canActivate` from the `Route` to fix pre-existing invalid configurations.
+
+#### Before
+```ts
+import { Routes } from '@angular/router';
+
+const routes: Routes = [
+  {path: '', redirectTo: 'other', canActivate: [MyGuard]}
+];
+```
+
+#### After
+```ts
+import { Routes } from '@angular/router';
+
+const routes: Routes = [
+  {path: '', redirectTo: 'other'}
+];
+```

packages/core/schematics/migrations/can-activate-with-redirect-to/index.ts

@@ -0,0 +1,56 @@
+/**
+ * @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 {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
+import {findLiteralsToMigrate, migrateLiteral} from './util';
+
+
+/** Migration that removes `canActivate` property from routes that also have `redirectTo`. */
+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) {
+      runCanActivateWithRedirectToMigration(tree, tsconfigPath, basePath);
+    }
+  };
+}
+
+function runCanActivateWithRedirectToMigration(tree: Tree, tsconfigPath: string, basePath: string) {
+  const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
+  const printer = ts.createPrinter();
+  const sourceFiles =
+      program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));
+
+  sourceFiles.forEach(sourceFile => {
+    const literalsToMigrate = findLiteralsToMigrate(sourceFile);
+    const update = tree.beginUpdate(relative(basePath, sourceFile.fileName));
+
+    for (const literal of Array.from(literalsToMigrate)) {
+      const migratedNode = migrateLiteral(literal);
+      update.remove(literal.getStart(), literal.getWidth());
+      update.insertRight(
+          literal.getStart(), printer.printNode(ts.EmitHint.Unspecified, migratedNode, sourceFile));
+    }
+
+    tree.commitUpdate(update);
+  });
+}

packages/core/schematics/migrations/can-activate-with-redirect-to/util.ts

@@ -0,0 +1,62 @@
+/**
+ * @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';
+
+const CAN_ACTIVATE = 'canActivate';
+const REDIRECT_TO = 'redirectTo';
+
+export function migrateLiteral(node: ts.ObjectLiteralExpression): ts.ObjectLiteralExpression {
+  const propertiesToKeep: ts.ObjectLiteralElementLike[] = [];
+  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 (property.name.text !== CAN_ACTIVATE) {
+        propertiesToKeep.push(property);
+      }
+    } else {
+      propertiesToKeep.push(property);
+    }
+  });
+
+  return ts.createObjectLiteral(propertiesToKeep);
+}
+
+
+export function findLiteralsToMigrate(sourceFile: ts.SourceFile) {
+  const results = new Set<ts.ObjectLiteralExpression>();
+
+  sourceFile.forEachChild(function visitNode(node: ts.Node) {
+    if (!ts.isObjectLiteralExpression(node)) {
+      node.forEachChild(visitNode);
+      return;
+    }
+    if (hasProperty(node, REDIRECT_TO) && hasProperty(node, CAN_ACTIVATE)) {
+      results.add(node);
+    }
+  });
+
+  return results;
+}
+
+function hasProperty(node: ts.ObjectLiteralExpression, propertyName: string): boolean {
+  for (const property of node.properties) {
+    // 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)) &&
+        property.name.text === propertyName) {
+      return true;
+    }
+  }
+  return false;
+}

packages/core/schematics/migrations/google3/BUILD.bazel

@@ -6,6 +6,7 @@ ts_library(
     tsconfig = "//packages/core/schematics:tsconfig.json",
     visibility = ["//packages/core/schematics/test/google3:__pkg__"],
     deps = [
+        "//packages/core/schematics/migrations/can-activate-with-redirect-to",
         "//packages/core/schematics/migrations/dynamic-queries",
         "//packages/core/schematics/migrations/initial-navigation",
         "//packages/core/schematics/migrations/initial-navigation/google3",

packages/core/schematics/migrations/google3/canActivateWithRedirectToRule.ts

@@ -0,0 +1,33 @@
+/**
+ * @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 '../can-activate-with-redirect-to/util';
+
+
+/** TSLint rule that removes canActivate from Route configs that also have redirectTo. */
+export class Rule extends Rules.TypedRule {
+  applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] {
+    const failures: RuleFailure[] = [];
+    const printer = ts.createPrinter();
+    const literalsToMigrate = findLiteralsToMigrate(sourceFile);
+
+    for (const literal of Array.from(literalsToMigrate)) {
+      const migratedNode = migrateLiteral(literal);
+      failures.push(new RuleFailure(
+          sourceFile, literal.getStart(), literal.getEnd(),
+          'canActivate cannot be used with redirectTo.', this.ruleName,
+          new Replacement(
+              literal.getStart(), literal.getWidth(),
+              printer.printNode(ts.EmitHint.Unspecified, migratedNode, sourceFile))));
+    }
+
+    return failures;
+  }
+}

packages/core/schematics/test/BUILD.bazel

@@ -9,6 +9,7 @@ ts_library(
     ],
     deps = [
         "//packages/core/schematics/migrations/abstract-control-parent",
+        "//packages/core/schematics/migrations/can-activate-with-redirect-to",
         "//packages/core/schematics/migrations/dynamic-queries",
         "//packages/core/schematics/migrations/initial-navigation",
         "//packages/core/schematics/migrations/missing-injectable",

packages/core/schematics/test/can_activate_with_redirect_migration_spec.ts

@@ -0,0 +1,78 @@
+/**
+ * @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('canActivate removal with redirectTo', () => {
+  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'}}}}}
+    }));
+
+    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 remove canActivate when redirectTo is not present', async () => {
+    writeFile('/index.ts', `const route = {path: '', canActivate: ['my_guard_token']}`);
+
+    await runMigration();
+
+    const content = tree.readContent('/index.ts');
+    expect(content).toEqual(`const route = {path: '', canActivate: ['my_guard_token']}`);
+  });
+
+  it('removes canActivate when redirectTo is present', async () => {
+    writeFile(
+        '/index.ts',
+        `const route = {path: '', redirectTo: 'other', canActivate: ['my_guard_token']}`);
+
+    await runMigration();
+
+    const content = tree.readContent('/index.ts');
+    expect(content).toEqual(`const route = { path: '', redirectTo: 'other' }`);
+  });
+
+  function writeFile(filePath: string, contents: string) {
+    host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents));
+  }
+
+  function runMigration() {
+    return runner.runSchematicAsync('migration-v11.1-can-activate-with-redirect-to', {}, tree)
+        .toPromise();
+  }
+});

packages/core/schematics/test/google3/can_activate_with_redirect_rule_spec.ts

@@ -0,0 +1,83 @@
+/**
+ * @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 canActivate with redirectTo', () => {
+  const rulesDirectory =
+      dirname(require.resolve('../../migrations/google3/canActivateWithRedirectToRule'));
+
+  let tmpDir: string;
+
+  beforeEach(() => {
+    tmpDir = join(process.env['TEST_TMPDIR']!, 'google3-test');
+    shx.mkdir('-p', tmpDir);
+
+    writeFile('tsconfig.json', JSON.stringify({
+      compilerOptions: {
+        module: 'es2015',
+        baseUrl: './',
+      },
+    }));
+  });
+
+  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: {'can-activate-with-redirect-to': 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 not flag canActivate when redirectTo is not present', async () => {
+    writeFile('/index.ts', `const route = {path: '', canActivate: ['my_guard_token']}`);
+
+    const linter = runTSLint(false);
+    const failures = linter.getResult().failures.map(failure => failure.getFailure());
+
+    expect(failures.length).toBe(0);
+  });
+
+  it('should flag when canActivate when redirectTo is present', async () => {
+    writeFile(
+        '/index.ts',
+        `const route = {path: '', redirectTo: 'other', canActivate: ['my_guard_token']}`);
+
+    const linter = runTSLint(false);
+    const failures = linter.getResult().failures.map(failure => failure.getFailure());
+    expect(failures.length).toBe(1);
+    expect(failures[0]).toMatch(/canActivate cannot be used with redirectTo./);
+  });
+
+  it('should fix when canActivate when redirectTo is present', async () => {
+    writeFile(
+        '/index.ts',
+        `const route = {path: '', redirectTo: 'other', canActivate: ['my_guard_token']}`);
+
+    runTSLint(true);
+    const content = getFile('/index.ts');
+    expect(content).toContain(`const route = { path: '', redirectTo: 'other' }`);
+  });
+});