feat(core): Add migration to update empty routerLinks in templates (#…

…43176)

The previous behavior of `RouterLink` for `null` and `undefined` inputs was to treat
the input the same as `[]` or `''`. This creates several unresolvable issues with
correctly disabling the links because `commands = []` does not behave the same
as disabling a link. Instead, it navigates to the current page, but will also
clear any fragment and/or query params.

The new behavior of the `routerLink` input will be to completely disable navigation
for `null` and `undefined` inputs. For HTML Anchor elements, this will also mean
removing the `href` attribute.

Migration for change in #43087

PR Close #43176

packages/core/schematics/BUILD.bazel

@@ -23,6 +23,7 @@ pkg_npm(
         "//packages/core/schematics/migrations/navigation-extras-omissions",
         "//packages/core/schematics/migrations/relative-link-resolution",
         "//packages/core/schematics/migrations/renderer-to-renderer2",
+        "//packages/core/schematics/migrations/router-link-empty-expression",
         "//packages/core/schematics/migrations/router-preserve-query-params",
         "//packages/core/schematics/migrations/static-queries",
         "//packages/core/schematics/migrations/template-var-assignment",

packages/core/schematics/migrations.json

@@ -99,6 +99,11 @@
       "version": "12.0.2",
       "description": "Automatically migrates shadow-piercing selector from `/deep/` to the recommended alternative `::ng-deep`.",
       "factory": "./migrations/deep-shadow-piercing-selector/index"
+    },
+    "migration-v13-router-link-empty-expression": {
+      "version": "13.0.0-beta",
+      "description": "Migrates `[routerLink]=\"\"` in templates to `[routerLink]=\"[]\"` because these links are likely intented to route to the current page with updated fragment/query params.",
+      "factory": "./migrations/router-link-empty-expression/index"
     }
   }
-}
+}

packages/core/schematics/migrations/router-link-empty-expression/BUILD.bazel

@@ -0,0 +1,20 @@
+load("//tools:defaults.bzl", "ts_library")
+
+ts_library(
+    name = "router-link-empty-expression",
+    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/compiler",
+        "//packages/core/schematics/utils",
+        "@npm//@angular-devkit/core",
+        "@npm//@angular-devkit/schematics",
+        "@npm//@types/node",
+        "@npm//typescript",
+    ],
+)

packages/core/schematics/migrations/router-link-empty-expression/README.md

@@ -0,0 +1,28 @@
+## RouterLink `null` and `undefined` inputs
+
+The previous behavior of `RouterLink` for `null` and `undefined` inputs was to treat
+the input the same as `[]` or `''`. This creates several unresolvable issues with
+correctly disabling the links because `commands = []` does not behave the same
+as disabling a link. Instead, it navigates to the current page, but will also
+clear any fragment and/or query params.
+
+The new behavior of the `routerLink` input will be to completely disable navigation
+for `null` and `undefined` inputs. For HTML Anchor elements, this will also mean
+removing the `href` attribute.
+
+```html
+<button [routerLink]="" fragment="section_2">section 2</button>
+```
+
+In the example from above, there is no value provided to the `routerLink` input.
+This button would previously navigate to the current page and update the fragment to "section_2".
+The updated behavio is to disable this link because the input
+for `routerLink` is `undefined`.
+
+If the intent for the link is to link to the current page rather than disable navigation,
+the template should be updated to one of the following options:
+```html
+<button [routerLink]="[]" fragment="section_2">section 2</button>
+<button [routerLink]="''" fragment="section_2">section 2</button>
+<button routerLink fragment="section_2">section 2</button>
+```

packages/core/schematics/migrations/router-link-empty-expression/analyze_template.ts

@@ -0,0 +1,29 @@
+/**
+ * @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 {BoundAttribute, visitAll} from '@angular/compiler/src/render3/r3_ast';
+
+import {ResolvedTemplate} from '../../utils/ng_component_template';
+import {parseHtmlGracefully} from '../../utils/parse_html';
+
+import {RouterLinkEmptyExprVisitor} from './angular/html_routerlink_empty_expr_visitor';
+
+export function analyzeResolvedTemplate(template: ResolvedTemplate): BoundAttribute[]|null {
+  const templateNodes = parseHtmlGracefully(template.content, template.filePath);
+
+  if (!templateNodes) {
+    return null;
+  }
+
+  const visitor = new RouterLinkEmptyExprVisitor();
+
+  // Analyze the Angular Render3 HTML AST and collect all template variable assignments.
+  visitAll(visitor, templateNodes);
+
+  return visitor.emptyRouterLinkExpressions;
+}

packages/core/schematics/migrations/router-link-empty-expression/angular/html_routerlink_empty_expr_visitor.ts

@@ -0,0 +1,35 @@
+/**
+ * @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 {ASTWithSource, EmptyExpr} from '@angular/compiler';
+import {BoundAttribute, Element, NullVisitor, Template, visitAll} from '@angular/compiler/src/render3/r3_ast';
+
+/**
+ * HTML AST visitor that traverses the Render3 HTML AST in order to find all
+ * undefined routerLink asssignment ([routerLink]="").
+ */
+export class RouterLinkEmptyExprVisitor extends NullVisitor {
+  readonly emptyRouterLinkExpressions: BoundAttribute[] = [];
+
+  override visitElement(element: Element): void {
+    visitAll(this, element.inputs);
+    visitAll(this, element.children);
+  }
+
+  override visitTemplate(t: Template): void {
+    visitAll(this, t.inputs);
+    visitAll(this, t.children);
+  }
+
+  override visitBoundAttribute(node: BoundAttribute) {
+    if (node.name === 'routerLink' && node.value instanceof ASTWithSource &&
+        node.value.ast instanceof EmptyExpr) {
+      this.emptyRouterLinkExpressions.push(node);
+    }
+  }
+}

packages/core/schematics/migrations/router-link-empty-expression/index.ts

@@ -0,0 +1,157 @@
+/**
+ * @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 {logging, normalize} from '@angular-devkit/core';
+import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
+import {EmptyExpr, TmplAstBoundAttribute} from '@angular/compiler';
+import {relative} from 'path';
+
+import {NgComponentTemplateVisitor, ResolvedTemplate} from '../../utils/ng_component_template';
+import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
+import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
+
+import {analyzeResolvedTemplate} from './analyze_template';
+
+type Logger = logging.LoggerApi;
+
+const README_URL =
+    'https://github.com/angular/angular/blob/master/packages/core/schematics/migrations/router-link-empty-expression/README.md';
+
+interface FixedTemplate {
+  originalTemplate: ResolvedTemplate;
+  newContent: string;
+  emptyRouterlinkExpressions: TmplAstBoundAttribute[];
+}
+
+/** Entry point for the RouterLink empty expression migration. */
+export default function(): Rule {
+  return (tree: Tree, context: SchematicContext) => {
+    const {buildPaths, testPaths} = getProjectTsConfigPaths(tree);
+    const basePath = process.cwd();
+
+    if (!buildPaths.length && !testPaths.length) {
+      throw new SchematicsException(
+          'Could not find any tsconfig file. Cannot check templates for empty routerLinks.');
+    }
+
+    for (const tsconfigPath of [...buildPaths, ...testPaths]) {
+      runEmptyRouterLinkExpressionMigration(tree, tsconfigPath, basePath, context.logger);
+    }
+  };
+}
+
+/**
+ * Runs the routerLink migration, changing routerLink="" to routerLink="[]" and notifying developers
+ * which templates received updates.
+ */
+function runEmptyRouterLinkExpressionMigration(
+    tree: Tree, tsconfigPath: string, basePath: string, logger: Logger) {
+  const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
+  const typeChecker = program.getTypeChecker();
+  const templateVisitor = new NgComponentTemplateVisitor(typeChecker);
+  const sourceFiles =
+      program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));
+
+  // Analyze source files by detecting HTML templates.
+  sourceFiles.forEach(sourceFile => templateVisitor.visitNode(sourceFile));
+
+  const {resolvedTemplates} = templateVisitor;
+  fixEmptyRouterlinks(resolvedTemplates, tree, logger);
+}
+
+function fixEmptyRouterlinks(resolvedTemplates: ResolvedTemplate[], tree: Tree, logger: Logger) {
+  const basePath = process.cwd();
+  const collectedFixes: string[] = [];
+  const fixesByFile = getFixesByFile(resolvedTemplates);
+
+  for (const [absFilePath, fixes] of fixesByFile) {
+    const treeFilePath = relative(normalize(basePath), normalize(absFilePath));
+    const originalFileContent = tree.read(treeFilePath)?.toString();
+    if (originalFileContent === undefined) {
+      logger.error(
+          `Failed to read file containing template; cannot apply fixes for empty routerLink expressions in ${
+              treeFilePath}.`);
+      continue;
+    }
+
+    const updater = tree.beginUpdate(treeFilePath);
+    for (const fix of fixes) {
+      const displayFilePath = normalize(relative(basePath, fix.originalTemplate.filePath));
+      updater.remove(fix.originalTemplate.start, fix.originalTemplate.content.length);
+      updater.insertLeft(fix.originalTemplate.start, fix.newContent);
+
+      for (const n of fix.emptyRouterlinkExpressions) {
+        const {line, character} =
+            fix.originalTemplate.getCharacterAndLineOfPosition(n.sourceSpan.start.offset);
+        collectedFixes.push(`${displayFilePath}@${line + 1}:${character + 1}`);
+      }
+      tree.commitUpdate(updater);
+    }
+  }
+
+  if (collectedFixes.length > 0) {
+    logger.info('---- RouterLink empty assignment schematic ----');
+    logger.info('The behavior of empty/`undefined` inputs for `routerLink` has changed');
+    logger.info('from linking to the current page to instead completely disable the link.');
+    logger.info(`Read more about this change here: ${README_URL}`);
+    logger.info('');
+    logger.info('The following empty `routerLink` inputs were found and fixed:');
+    collectedFixes.forEach(fix => logger.warn(`⮑   ${fix}`));
+  }
+}
+
+/**
+ * Returns fixes for nodes in templates which contain empty routerLink assignments, grouped by file.
+ */
+function getFixesByFile(templates: ResolvedTemplate[]): Map<string, FixedTemplate[]> {
+  const fixesByFile = new Map<string, FixedTemplate[]>();
+  for (const template of templates) {
+    const templateFix = fixEmptyRouterlinksInTemplate(template);
+    if (templateFix === null) {
+      continue;
+    }
+
+    const file = template.filePath;
+    if (fixesByFile.has(file)) {
+      if (template.inline) {
+        // External templates may be referenced multiple times in the project
+        // (e.g. if shared between components), but we only want to record them
+        // once. On the other hand, an inline template resides in a TS file that
+        // may contain multiple inline templates.
+        fixesByFile.get(file)!.push(templateFix);
+      }
+    } else {
+      fixesByFile.set(file, [templateFix]);
+    }
+  }
+
+  return fixesByFile;
+}
+
+function fixEmptyRouterlinksInTemplate(template: ResolvedTemplate): FixedTemplate|null {
+  const emptyRouterlinkExpressions = analyzeResolvedTemplate(template);
+
+  if (!emptyRouterlinkExpressions) {
+    return null;
+  }
+
+  // Sort backwards so string replacements do not conflict
+  emptyRouterlinkExpressions.sort((a, b) => b.value.sourceSpan.start - a.value.sourceSpan.start);
+  let newContent = template.content;
+  for (const expr of emptyRouterlinkExpressions) {
+    if (expr.valueSpan) {
+      newContent = newContent.substr(0, expr.value.sourceSpan.start) + '[]' +
+          newContent.substr(expr.value.sourceSpan.start);
+    } else {
+      newContent = newContent.substr(0, expr.sourceSpan.end.offset) + '="[]"' +
+          newContent.substr(expr.sourceSpan.end.offset);
+    }
+  }
+
+  return {originalTemplate: template, newContent, emptyRouterlinkExpressions};
+}

packages/core/schematics/test/BUILD.bazel

@@ -21,6 +21,7 @@ ts_library(
         "//packages/core/schematics/migrations/navigation-extras-omissions",
         "//packages/core/schematics/migrations/relative-link-resolution",
         "//packages/core/schematics/migrations/renderer-to-renderer2",
+        "//packages/core/schematics/migrations/router-link-empty-expression",
         "//packages/core/schematics/migrations/router-preserve-query-params",
         "//packages/core/schematics/migrations/static-queries",
         "//packages/core/schematics/migrations/template-var-assignment",