Skip to content

Commit 190fa07

Browse files
crisbetojosephperrott
authored andcommitted
feat(router): add migration for ActivatedRouteSnapshot.fragment (angular#41092)
Adds a migration that casts the value of `ActivatedRouteSnapshot.fragment` to be non-nullable. Also moves some code from the `AbstractControl.parent` migration so that it can be reused. Relates to angular#37336. PR Close angular#41092
1 parent 1eba57e commit 190fa07

File tree

14 files changed

+698
-105
lines changed

14 files changed

+698
-105
lines changed

packages/core/schematics/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ pkg_npm(
1111
visibility = ["//packages/core:__pkg__"],
1212
deps = [
1313
"//packages/core/schematics/migrations/abstract-control-parent",
14+
"//packages/core/schematics/migrations/activated-route-snapshot-fragment",
1415
"//packages/core/schematics/migrations/can-activate-with-redirect-to",
1516
"//packages/core/schematics/migrations/dynamic-queries",
1617
"//packages/core/schematics/migrations/initial-navigation",

packages/core/schematics/migrations.json

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@
8484
"version": "11.1.0-beta",
8585
"description": "Removes `canActivate` from a `Route` config when `redirectTo` is also present",
8686
"factory": "./migrations/can-activate-with-redirect-to/index"
87+
},
88+
"migration-v12-activated-route-snapshot-fragment": {
89+
"version": "12.0.0-beta",
90+
"description": "In Angular version 12, the type of ActivatedRouteSnapshot.fragment is nullable. This migration automatically adds non-null assertions to it.",
91+
"factory": "./migrations/activated-route-snapshot-fragment/index"
8792
}
8893
}
89-
}
94+
}

packages/core/schematics/migrations/abstract-control-parent/util.ts

Lines changed: 6 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,11 @@
88

99
import {normalize} from 'path';
1010
import * as ts from 'typescript';
11+
import {isNullCheck, isSafeAccess} from '../../utils/typescript/nodes';
12+
import {hasOneOfTypes, isNullableType} from '../../utils/typescript/symbol';
1113

1214
/** Names of symbols from `@angular/forms` whose `parent` accesses have to be migrated. */
13-
const abstractControlSymbols = new Set<string>([
14-
'AbstractControl',
15-
'FormArray',
16-
'FormControl',
17-
'FormGroup',
18-
]);
15+
const abstractControlSymbols = ['AbstractControl', 'FormArray', 'FormControl', 'FormGroup'];
1916

2017
/**
2118
* Finds the `PropertyAccessExpression`-s that are accessing the `parent` property in
@@ -38,78 +35,6 @@ export function findParentAccesses(
3835
return results;
3936
}
4037

41-
/** Checks whether a node's type is nullable (`null`, `undefined` or `void`). */
42-
function isNullableType(typeChecker: ts.TypeChecker, node: ts.Node) {
43-
// Skip expressions in the form of `foo.bar!.baz` since the `TypeChecker` seems
44-
// to identify them as null, even though the user indicated that it won't be.
45-
if (node.parent && ts.isNonNullExpression(node.parent)) {
46-
return false;
47-
}
48-
49-
const type = typeChecker.getTypeAtLocation(node);
50-
const typeNode = typeChecker.typeToTypeNode(type, undefined, ts.NodeBuilderFlags.None);
51-
let hasSeenNullableType = false;
52-
53-
// Trace the type of the node back to a type node, walk
54-
// through all of its sub-nodes and look for nullable tyes.
55-
if (typeNode) {
56-
(function walk(current: ts.Node) {
57-
if (current.kind === ts.SyntaxKind.NullKeyword ||
58-
current.kind === ts.SyntaxKind.UndefinedKeyword ||
59-
current.kind === ts.SyntaxKind.VoidKeyword) {
60-
hasSeenNullableType = true;
61-
// Note that we don't descend into type literals, because it may cause
62-
// us to mis-identify the root type as nullable, because it has a nullable
63-
// property (e.g. `{ foo: string | null }`).
64-
} else if (!hasSeenNullableType && !ts.isTypeLiteralNode(current)) {
65-
current.forEachChild(walk);
66-
}
67-
})(typeNode);
68-
}
69-
70-
return hasSeenNullableType;
71-
}
72-
73-
/**
74-
* Checks whether a particular node is part of a null check. E.g. given:
75-
* `control.parent ? control.parent.value : null` the null check would be `control.parent`.
76-
*/
77-
function isNullCheck(node: ts.PropertyAccessExpression): boolean {
78-
if (!node.parent) {
79-
return false;
80-
}
81-
82-
// `control.parent && control.parent.value` where `node` is `control.parent`.
83-
if (ts.isBinaryExpression(node.parent) && node.parent.left === node) {
84-
return true;
85-
}
86-
87-
// `control.parent && control.parent.parent && control.parent.parent.value`
88-
// where `node` is `control.parent`.
89-
if (node.parent.parent && ts.isBinaryExpression(node.parent.parent) &&
90-
node.parent.parent.left === node.parent) {
91-
return true;
92-
}
93-
94-
// `if (control.parent) {...}` where `node` is `control.parent`.
95-
if (ts.isIfStatement(node.parent) && node.parent.expression === node) {
96-
return true;
97-
}
98-
99-
// `control.parent ? control.parent.value : null` where `node` is `control.parent`.
100-
if (ts.isConditionalExpression(node.parent) && node.parent.condition === node) {
101-
return true;
102-
}
103-
104-
return false;
105-
}
106-
107-
/** Checks whether a property access is safe (e.g. `foo.parent?.value`). */
108-
function isSafeAccess(node: ts.PropertyAccessExpression): boolean {
109-
return node.parent != null && ts.isPropertyAccessExpression(node.parent) &&
110-
node.parent.expression === node && node.parent.questionDotToken != null;
111-
}
112-
11338
/** Checks whether a property access is on an `AbstractControl` coming from `@angular/forms`. */
11439
function isAbstractControlReference(
11540
typeChecker: ts.TypeChecker, node: ts.PropertyAccessExpression): boolean {
@@ -119,37 +44,14 @@ function isAbstractControlReference(
11944
// If such a node is found, we check whether the type is one of the `AbstractControl` symbols
12045
// and whether it comes from the `@angular/forms` directory in the `node_modules`.
12146
while (ts.isPropertyAccessExpression(current)) {
122-
const type = typeChecker.getTypeAtLocation(current.expression);
123-
const symbol = type.getSymbol();
124-
if (symbol && type) {
47+
const symbol = typeChecker.getTypeAtLocation(current.expression)?.getSymbol();
48+
if (symbol) {
12549
const sourceFile = symbol.valueDeclaration?.getSourceFile();
12650
return sourceFile != null &&
12751
formsPattern.test(normalize(sourceFile.fileName).replace(/\\/g, '/')) &&
128-
hasAbstractControlType(typeChecker, type);
52+
hasOneOfTypes(typeChecker, current.expression, abstractControlSymbols);
12953
}
13054
current = current.expression;
13155
}
13256
return false;
13357
}
134-
135-
/**
136-
* Walks through the sub-types of a type, looking for a type that
137-
* has the same name as one of the `AbstractControl` types.
138-
*/
139-
function hasAbstractControlType(typeChecker: ts.TypeChecker, type: ts.Type): boolean {
140-
const typeNode = typeChecker.typeToTypeNode(type, undefined, ts.NodeBuilderFlags.None);
141-
let hasMatch = false;
142-
if (typeNode) {
143-
(function walk(current: ts.Node) {
144-
if (ts.isIdentifier(current) && abstractControlSymbols.has(current.text)) {
145-
hasMatch = true;
146-
// Note that we don't descend into type literals, because it may cause
147-
// us to mis-identify the root type as nullable, because it has a nullable
148-
// property (e.g. `{ foo: FormControl }`).
149-
} else if (!hasMatch && !ts.isTypeLiteralNode(current)) {
150-
current.forEachChild(walk);
151-
}
152-
})(typeNode);
153-
}
154-
return hasMatch;
155-
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
load("//tools:defaults.bzl", "ts_library")
2+
3+
ts_library(
4+
name = "activated-route-snapshot-fragment",
5+
srcs = glob(["**/*.ts"]),
6+
tsconfig = "//packages/core/schematics:tsconfig.json",
7+
visibility = [
8+
"//packages/core/schematics:__pkg__",
9+
"//packages/core/schematics/migrations/google3:__pkg__",
10+
"//packages/core/schematics/test:__pkg__",
11+
],
12+
deps = [
13+
"//packages/core/schematics/utils",
14+
"@npm//@angular-devkit/schematics",
15+
"@npm//@types/node",
16+
"@npm//typescript",
17+
],
18+
)
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
## `ActivatedRouteSnapshot.fragment` migration
2+
3+
The value if `ActivatedRouteSnapshot.fragment` is becoming nullable. This migration adds non-null
4+
assertions to it.
5+
6+
#### Before
7+
```ts
8+
import { Component } from '@angular/core';
9+
import { ActivatedRouteSnapshot } from '@angular/router';
10+
11+
@Component({})
12+
export class YourComponent {
13+
private _activatedRouteSnapshot: ActivatedRouteSnapshot;
14+
15+
getFragmentValue() {
16+
return this._activatedRouteSnapshot.fragment.value;
17+
}
18+
}
19+
```
20+
21+
#### After
22+
```ts
23+
import { Component } from '@angular/core';
24+
import { ActivatedRoute } from '@angular/router';
25+
26+
@Component({})
27+
export class YourComponent {
28+
private _activatedRouteSnapshot: ActivatedRouteSnapshot;
29+
30+
getFragmentValue() {
31+
return this._activatedRouteSnapshot.fragment!.value;
32+
}
33+
}
34+
```
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {Rule, SchematicsException, Tree} from '@angular-devkit/schematics';
10+
import {relative} from 'path';
11+
import * as ts from 'typescript';
12+
13+
import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
14+
import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host';
15+
import {findFragmentAccesses, migrateActivatedRouteSnapshotFragment} from './util';
16+
17+
18+
/**
19+
* Migration that marks accesses of `ActivatedRouteSnapshot.fragment` as non-null.
20+
*/
21+
export default function(): Rule {
22+
return (tree: Tree) => {
23+
const {buildPaths, testPaths} = getProjectTsConfigPaths(tree);
24+
const basePath = process.cwd();
25+
const allPaths = [...buildPaths, ...testPaths];
26+
27+
if (!allPaths.length) {
28+
throw new SchematicsException(
29+
'Could not find any tsconfig file. Cannot migrate ' +
30+
'`ActivatedRouteSnapshot.fragment` accesses.');
31+
}
32+
33+
for (const tsconfigPath of allPaths) {
34+
runActivatedRouteSnapshotFragmentMigration(tree, tsconfigPath, basePath);
35+
}
36+
};
37+
}
38+
39+
function runActivatedRouteSnapshotFragmentMigration(
40+
tree: Tree, tsconfigPath: string, basePath: string) {
41+
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
42+
const typeChecker = program.getTypeChecker();
43+
const sourceFiles =
44+
program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program));
45+
const printer = ts.createPrinter();
46+
47+
sourceFiles.forEach(sourceFile => {
48+
const nodesToMigrate = findFragmentAccesses(typeChecker, sourceFile);
49+
50+
if (nodesToMigrate.size > 0) {
51+
const update = tree.beginUpdate(relative(basePath, sourceFile.fileName));
52+
nodesToMigrate.forEach(node => {
53+
update.remove(node.getStart(), node.getWidth());
54+
update.insertRight(
55+
node.getStart(),
56+
printer.printNode(
57+
ts.EmitHint.Unspecified, migrateActivatedRouteSnapshotFragment(node), sourceFile));
58+
});
59+
tree.commitUpdate(update);
60+
}
61+
});
62+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import * as ts from 'typescript';
10+
import {isNullCheck, isSafeAccess} from '../../utils/typescript/nodes';
11+
import {hasOneOfTypes, isNullableType} from '../../utils/typescript/symbol';
12+
13+
/**
14+
* Finds all the accesses of `ActivatedRouteSnapshot.fragment`
15+
* that need to be migrated within a particular file.
16+
*/
17+
export function findFragmentAccesses(
18+
typeChecker: ts.TypeChecker, sourceFile: ts.SourceFile): Set<ts.PropertyAccessExpression> {
19+
const results = new Set<ts.PropertyAccessExpression>();
20+
21+
sourceFile.forEachChild(function walk(node: ts.Node) {
22+
if (ts.isPropertyAccessExpression(node) && node.name.text === 'fragment' &&
23+
!results.has(node) && !isNullCheck(node) && !isSafeAccess(node) &&
24+
hasOneOfTypes(typeChecker, node.expression, ['ActivatedRouteSnapshot']) &&
25+
isNullableType(typeChecker, node)) {
26+
results.add(node);
27+
}
28+
29+
node.forEachChild(walk);
30+
});
31+
32+
return results;
33+
}
34+
35+
/** Migrates an `ActivatedRouteSnapshot.fragment` access. */
36+
export function migrateActivatedRouteSnapshotFragment(node: ts.PropertyAccessExpression): ts.Node {
37+
// Turns `foo.fragment` into `foo.fragment!`.
38+
return ts.createNonNullExpression(node);
39+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ ts_library(
66
tsconfig = "//packages/core/schematics:tsconfig.json",
77
visibility = ["//packages/core/schematics/test/google3:__pkg__"],
88
deps = [
9+
"//packages/core/schematics/migrations/activated-route-snapshot-fragment",
910
"//packages/core/schematics/migrations/can-activate-with-redirect-to",
1011
"//packages/core/schematics/migrations/dynamic-queries",
1112
"//packages/core/schematics/migrations/initial-navigation",
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {Replacement, RuleFailure, Rules} from 'tslint';
10+
import * as ts from 'typescript';
11+
12+
import {findFragmentAccesses, migrateActivatedRouteSnapshotFragment} from '../activated-route-snapshot-fragment/util';
13+
14+
export class Rule extends Rules.TypedRule {
15+
applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] {
16+
if (sourceFile.isDeclarationFile || program.isSourceFileFromExternalLibrary(sourceFile)) {
17+
return [];
18+
}
19+
20+
const failures: RuleFailure[] = [];
21+
const typeChecker = program.getTypeChecker();
22+
const nodesToMigrate = findFragmentAccesses(typeChecker, sourceFile);
23+
24+
if (nodesToMigrate.size > 0) {
25+
const printer = ts.createPrinter();
26+
nodesToMigrate.forEach(node => {
27+
const sourceFile = node.getSourceFile();
28+
const migratedNode = migrateActivatedRouteSnapshotFragment(node);
29+
const replacement = new Replacement(
30+
node.getStart(), node.getWidth(),
31+
printer.printNode(ts.EmitHint.Unspecified, migratedNode, sourceFile));
32+
failures.push(new RuleFailure(
33+
sourceFile, node.getStart(), node.getEnd(),
34+
'`ActivatedRouteSnapshot.fragment` is nullable.', this.ruleName, replacement));
35+
});
36+
}
37+
38+
return failures;
39+
}
40+
}

packages/core/schematics/test/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ ts_library(
99
],
1010
deps = [
1111
"//packages/core/schematics/migrations/abstract-control-parent",
12+
"//packages/core/schematics/migrations/activated-route-snapshot-fragment",
1213
"//packages/core/schematics/migrations/can-activate-with-redirect-to",
1314
"//packages/core/schematics/migrations/dynamic-queries",
1415
"//packages/core/schematics/migrations/initial-navigation",

0 commit comments

Comments
 (0)