feat(router): add migration for ActivatedRouteSnapshot.fragment (#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 #37336. PR Close #41092
This commit is contained in:
		
							parent
							
								
									1eba57eb00
								
							
						
					
					
						commit
						190fa07b9a
					
				| @ -11,6 +11,7 @@ pkg_npm( | ||||
|     visibility = ["//packages/core:__pkg__"], | ||||
|     deps = [ | ||||
|         "//packages/core/schematics/migrations/abstract-control-parent", | ||||
|         "//packages/core/schematics/migrations/activated-route-snapshot-fragment", | ||||
|         "//packages/core/schematics/migrations/can-activate-with-redirect-to", | ||||
|         "//packages/core/schematics/migrations/dynamic-queries", | ||||
|         "//packages/core/schematics/migrations/initial-navigation", | ||||
|  | ||||
| @ -84,6 +84,11 @@ | ||||
|       "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" | ||||
|     }, | ||||
|     "migration-v12-activated-route-snapshot-fragment": { | ||||
|       "version": "12.0.0-beta", | ||||
|       "description": "In Angular version 12, the type of ActivatedRouteSnapshot.fragment is nullable. This migration automatically adds non-null assertions to it.", | ||||
|       "factory": "./migrations/activated-route-snapshot-fragment/index" | ||||
|     } | ||||
|   } | ||||
| } | ||||
| } | ||||
|  | ||||
| @ -8,14 +8,11 @@ | ||||
| 
 | ||||
| import {normalize} from 'path'; | ||||
| import * as ts from 'typescript'; | ||||
| import {isNullCheck, isSafeAccess} from '../../utils/typescript/nodes'; | ||||
| import {hasOneOfTypes, isNullableType} from '../../utils/typescript/symbol'; | ||||
| 
 | ||||
| /** Names of symbols from `@angular/forms` whose `parent` accesses have to be migrated. */ | ||||
| const abstractControlSymbols = new Set<string>([ | ||||
|   'AbstractControl', | ||||
|   'FormArray', | ||||
|   'FormControl', | ||||
|   'FormGroup', | ||||
| ]); | ||||
| const abstractControlSymbols = ['AbstractControl', 'FormArray', 'FormControl', 'FormGroup']; | ||||
| 
 | ||||
| /** | ||||
|  * Finds the `PropertyAccessExpression`-s that are accessing the `parent` property in | ||||
| @ -38,78 +35,6 @@ export function findParentAccesses( | ||||
|   return results; | ||||
| } | ||||
| 
 | ||||
| /** Checks whether a node's type is nullable (`null`, `undefined` or `void`). */ | ||||
| function isNullableType(typeChecker: ts.TypeChecker, node: ts.Node) { | ||||
|   // Skip expressions in the form of `foo.bar!.baz` since the `TypeChecker` seems
 | ||||
|   // to identify them as null, even though the user indicated that it won't be.
 | ||||
|   if (node.parent && ts.isNonNullExpression(node.parent)) { | ||||
|     return false; | ||||
|   } | ||||
| 
 | ||||
|   const type = typeChecker.getTypeAtLocation(node); | ||||
|   const typeNode = typeChecker.typeToTypeNode(type, undefined, ts.NodeBuilderFlags.None); | ||||
|   let hasSeenNullableType = false; | ||||
| 
 | ||||
|   // Trace the type of the node back to a type node, walk
 | ||||
|   // through all of its sub-nodes and look for nullable tyes.
 | ||||
|   if (typeNode) { | ||||
|     (function walk(current: ts.Node) { | ||||
|       if (current.kind === ts.SyntaxKind.NullKeyword || | ||||
|           current.kind === ts.SyntaxKind.UndefinedKeyword || | ||||
|           current.kind === ts.SyntaxKind.VoidKeyword) { | ||||
|         hasSeenNullableType = true; | ||||
|         // Note that we don't descend into type literals, because it may cause
 | ||||
|         // us to mis-identify the root type as nullable, because it has a nullable
 | ||||
|         // property (e.g. `{ foo: string | null }`).
 | ||||
|       } else if (!hasSeenNullableType && !ts.isTypeLiteralNode(current)) { | ||||
|         current.forEachChild(walk); | ||||
|       } | ||||
|     })(typeNode); | ||||
|   } | ||||
| 
 | ||||
|   return hasSeenNullableType; | ||||
| } | ||||
| 
 | ||||
| /** | ||||
|  * Checks whether a particular node is part of a null check. E.g. given: | ||||
|  * `control.parent ? control.parent.value : null` the null check would be `control.parent`. | ||||
|  */ | ||||
| function isNullCheck(node: ts.PropertyAccessExpression): boolean { | ||||
|   if (!node.parent) { | ||||
|     return false; | ||||
|   } | ||||
| 
 | ||||
|   // `control.parent && control.parent.value` where `node` is `control.parent`.
 | ||||
|   if (ts.isBinaryExpression(node.parent) && node.parent.left === node) { | ||||
|     return true; | ||||
|   } | ||||
| 
 | ||||
|   // `control.parent && control.parent.parent && control.parent.parent.value`
 | ||||
|   // where `node` is `control.parent`.
 | ||||
|   if (node.parent.parent && ts.isBinaryExpression(node.parent.parent) && | ||||
|       node.parent.parent.left === node.parent) { | ||||
|     return true; | ||||
|   } | ||||
| 
 | ||||
|   // `if (control.parent) {...}` where `node` is `control.parent`.
 | ||||
|   if (ts.isIfStatement(node.parent) && node.parent.expression === node) { | ||||
|     return true; | ||||
|   } | ||||
| 
 | ||||
|   // `control.parent ? control.parent.value : null` where `node` is `control.parent`.
 | ||||
|   if (ts.isConditionalExpression(node.parent) && node.parent.condition === node) { | ||||
|     return true; | ||||
|   } | ||||
| 
 | ||||
|   return false; | ||||
| } | ||||
| 
 | ||||
| /** Checks whether a property access is safe (e.g. `foo.parent?.value`). */ | ||||
| function isSafeAccess(node: ts.PropertyAccessExpression): boolean { | ||||
|   return node.parent != null && ts.isPropertyAccessExpression(node.parent) && | ||||
|       node.parent.expression === node && node.parent.questionDotToken != null; | ||||
| } | ||||
| 
 | ||||
| /** Checks whether a property access is on an `AbstractControl` coming from `@angular/forms`. */ | ||||
| function isAbstractControlReference( | ||||
|     typeChecker: ts.TypeChecker, node: ts.PropertyAccessExpression): boolean { | ||||
| @ -119,37 +44,14 @@ function isAbstractControlReference( | ||||
|   // If such a node is found, we check whether the type is one of the `AbstractControl` symbols
 | ||||
|   // and whether it comes from the `@angular/forms` directory in the `node_modules`.
 | ||||
|   while (ts.isPropertyAccessExpression(current)) { | ||||
|     const type = typeChecker.getTypeAtLocation(current.expression); | ||||
|     const symbol = type.getSymbol(); | ||||
|     if (symbol && type) { | ||||
|     const symbol = typeChecker.getTypeAtLocation(current.expression)?.getSymbol(); | ||||
|     if (symbol) { | ||||
|       const sourceFile = symbol.valueDeclaration?.getSourceFile(); | ||||
|       return sourceFile != null && | ||||
|           formsPattern.test(normalize(sourceFile.fileName).replace(/\\/g, '/')) && | ||||
|           hasAbstractControlType(typeChecker, type); | ||||
|           hasOneOfTypes(typeChecker, current.expression, abstractControlSymbols); | ||||
|     } | ||||
|     current = current.expression; | ||||
|   } | ||||
|   return false; | ||||
| } | ||||
| 
 | ||||
| /** | ||||
|  * Walks through the sub-types of a type, looking for a type that | ||||
|  * has the same name as one of the `AbstractControl` types. | ||||
|  */ | ||||
| function hasAbstractControlType(typeChecker: ts.TypeChecker, type: ts.Type): boolean { | ||||
|   const typeNode = typeChecker.typeToTypeNode(type, undefined, ts.NodeBuilderFlags.None); | ||||
|   let hasMatch = false; | ||||
|   if (typeNode) { | ||||
|     (function walk(current: ts.Node) { | ||||
|       if (ts.isIdentifier(current) && abstractControlSymbols.has(current.text)) { | ||||
|         hasMatch = true; | ||||
|         // Note that we don't descend into type literals, because it may cause
 | ||||
|         // us to mis-identify the root type as nullable, because it has a nullable
 | ||||
|         // property (e.g. `{ foo: FormControl }`).
 | ||||
|       } else if (!hasMatch && !ts.isTypeLiteralNode(current)) { | ||||
|         current.forEachChild(walk); | ||||
|       } | ||||
|     })(typeNode); | ||||
|   } | ||||
|   return hasMatch; | ||||
| } | ||||
|  | ||||
| @ -0,0 +1,18 @@ | ||||
| load("//tools:defaults.bzl", "ts_library") | ||||
| 
 | ||||
| ts_library( | ||||
|     name = "activated-route-snapshot-fragment", | ||||
|     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", | ||||
|     ], | ||||
| ) | ||||
| @ -0,0 +1,34 @@ | ||||
| ## `ActivatedRouteSnapshot.fragment` migration | ||||
| 
 | ||||
| The value if `ActivatedRouteSnapshot.fragment` is becoming nullable. This migration adds non-null | ||||
| assertions to it. | ||||
| 
 | ||||
| #### Before | ||||
| ```ts | ||||
| import { Component } from '@angular/core'; | ||||
| import { ActivatedRouteSnapshot } from '@angular/router'; | ||||
| 
 | ||||
| @Component({}) | ||||
| export class YourComponent { | ||||
|   private _activatedRouteSnapshot: ActivatedRouteSnapshot; | ||||
| 
 | ||||
|   getFragmentValue() { | ||||
|     return this._activatedRouteSnapshot.fragment.value; | ||||
|   } | ||||
| } | ||||
| ``` | ||||
| 
 | ||||
| #### After | ||||
| ```ts | ||||
| import { Component } from '@angular/core'; | ||||
| import { ActivatedRoute } from '@angular/router'; | ||||
| 
 | ||||
| @Component({}) | ||||
| export class YourComponent { | ||||
|   private _activatedRouteSnapshot: ActivatedRouteSnapshot; | ||||
| 
 | ||||
|   getFragmentValue() { | ||||
|     return this._activatedRouteSnapshot.fragment!.value; | ||||
|   } | ||||
| } | ||||
| ``` | ||||
| @ -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 {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 {findFragmentAccesses, migrateActivatedRouteSnapshotFragment} from './util'; | ||||
| 
 | ||||
| 
 | ||||
| /** | ||||
|  * Migration that marks accesses of `ActivatedRouteSnapshot.fragment` as non-null. | ||||
|  */ | ||||
| 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 ' + | ||||
|           '`ActivatedRouteSnapshot.fragment` accesses.'); | ||||
|     } | ||||
| 
 | ||||
|     for (const tsconfigPath of allPaths) { | ||||
|       runActivatedRouteSnapshotFragmentMigration(tree, tsconfigPath, basePath); | ||||
|     } | ||||
|   }; | ||||
| } | ||||
| 
 | ||||
| function runActivatedRouteSnapshotFragmentMigration( | ||||
|     tree: Tree, tsconfigPath: string, basePath: string) { | ||||
|   const {program} = createMigrationProgram(tree, tsconfigPath, basePath); | ||||
|   const typeChecker = program.getTypeChecker(); | ||||
|   const sourceFiles = | ||||
|       program.getSourceFiles().filter(sourceFile => canMigrateFile(basePath, sourceFile, program)); | ||||
|   const printer = ts.createPrinter(); | ||||
| 
 | ||||
|   sourceFiles.forEach(sourceFile => { | ||||
|     const nodesToMigrate = findFragmentAccesses(typeChecker, sourceFile); | ||||
| 
 | ||||
|     if (nodesToMigrate.size > 0) { | ||||
|       const update = tree.beginUpdate(relative(basePath, sourceFile.fileName)); | ||||
|       nodesToMigrate.forEach(node => { | ||||
|         update.remove(node.getStart(), node.getWidth()); | ||||
|         update.insertRight( | ||||
|             node.getStart(), | ||||
|             printer.printNode( | ||||
|                 ts.EmitHint.Unspecified, migrateActivatedRouteSnapshotFragment(node), sourceFile)); | ||||
|       }); | ||||
|       tree.commitUpdate(update); | ||||
|     } | ||||
|   }); | ||||
| } | ||||
| @ -0,0 +1,39 @@ | ||||
| /** | ||||
|  * @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 {isNullCheck, isSafeAccess} from '../../utils/typescript/nodes'; | ||||
| import {hasOneOfTypes, isNullableType} from '../../utils/typescript/symbol'; | ||||
| 
 | ||||
| /** | ||||
|  * Finds all the accesses of `ActivatedRouteSnapshot.fragment` | ||||
|  * that need to be migrated within a particular file. | ||||
|  */ | ||||
| export function findFragmentAccesses( | ||||
|     typeChecker: ts.TypeChecker, sourceFile: ts.SourceFile): Set<ts.PropertyAccessExpression> { | ||||
|   const results = new Set<ts.PropertyAccessExpression>(); | ||||
| 
 | ||||
|   sourceFile.forEachChild(function walk(node: ts.Node) { | ||||
|     if (ts.isPropertyAccessExpression(node) && node.name.text === 'fragment' && | ||||
|         !results.has(node) && !isNullCheck(node) && !isSafeAccess(node) && | ||||
|         hasOneOfTypes(typeChecker, node.expression, ['ActivatedRouteSnapshot']) && | ||||
|         isNullableType(typeChecker, node)) { | ||||
|       results.add(node); | ||||
|     } | ||||
| 
 | ||||
|     node.forEachChild(walk); | ||||
|   }); | ||||
| 
 | ||||
|   return results; | ||||
| } | ||||
| 
 | ||||
| /** Migrates an `ActivatedRouteSnapshot.fragment` access. */ | ||||
| export function migrateActivatedRouteSnapshotFragment(node: ts.PropertyAccessExpression): ts.Node { | ||||
|   // Turns `foo.fragment` into `foo.fragment!`.
 | ||||
|   return ts.createNonNullExpression(node); | ||||
| } | ||||
| @ -6,6 +6,7 @@ ts_library( | ||||
|     tsconfig = "//packages/core/schematics:tsconfig.json", | ||||
|     visibility = ["//packages/core/schematics/test/google3:__pkg__"], | ||||
|     deps = [ | ||||
|         "//packages/core/schematics/migrations/activated-route-snapshot-fragment", | ||||
|         "//packages/core/schematics/migrations/can-activate-with-redirect-to", | ||||
|         "//packages/core/schematics/migrations/dynamic-queries", | ||||
|         "//packages/core/schematics/migrations/initial-navigation", | ||||
|  | ||||
| @ -0,0 +1,40 @@ | ||||
| /** | ||||
|  * @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 {findFragmentAccesses, migrateActivatedRouteSnapshotFragment} from '../activated-route-snapshot-fragment/util'; | ||||
| 
 | ||||
| export class Rule extends Rules.TypedRule { | ||||
|   applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] { | ||||
|     if (sourceFile.isDeclarationFile || program.isSourceFileFromExternalLibrary(sourceFile)) { | ||||
|       return []; | ||||
|     } | ||||
| 
 | ||||
|     const failures: RuleFailure[] = []; | ||||
|     const typeChecker = program.getTypeChecker(); | ||||
|     const nodesToMigrate = findFragmentAccesses(typeChecker, sourceFile); | ||||
| 
 | ||||
|     if (nodesToMigrate.size > 0) { | ||||
|       const printer = ts.createPrinter(); | ||||
|       nodesToMigrate.forEach(node => { | ||||
|         const sourceFile = node.getSourceFile(); | ||||
|         const migratedNode = migrateActivatedRouteSnapshotFragment(node); | ||||
|         const replacement = new Replacement( | ||||
|             node.getStart(), node.getWidth(), | ||||
|             printer.printNode(ts.EmitHint.Unspecified, migratedNode, sourceFile)); | ||||
|         failures.push(new RuleFailure( | ||||
|             sourceFile, node.getStart(), node.getEnd(), | ||||
|             '`ActivatedRouteSnapshot.fragment` is nullable.', this.ruleName, replacement)); | ||||
|       }); | ||||
|     } | ||||
| 
 | ||||
|     return failures; | ||||
|   } | ||||
| } | ||||
| @ -9,6 +9,7 @@ ts_library( | ||||
|     ], | ||||
|     deps = [ | ||||
|         "//packages/core/schematics/migrations/abstract-control-parent", | ||||
|         "//packages/core/schematics/migrations/activated-route-snapshot-fragment", | ||||
|         "//packages/core/schematics/migrations/can-activate-with-redirect-to", | ||||
|         "//packages/core/schematics/migrations/dynamic-queries", | ||||
|         "//packages/core/schematics/migrations/initial-navigation", | ||||
|  | ||||
| @ -0,0 +1,186 @@ | ||||
| /** | ||||
|  * @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('ActivatedRouteSnapshot.fragment 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.d.ts', ` | ||||
|       export declare class ActivatedRoute { | ||||
|         get children(): ActivatedRoute[]; | ||||
|         fragment: Observable<string | null>; | ||||
|         snapshot: ActivatedRouteSnapshot; | ||||
|         url: Observable<unknown[]>; | ||||
|       } | ||||
| 
 | ||||
|       export declare class ActivatedRouteSnapshot { | ||||
|         fragment: string | null; | ||||
|         url: unknown[]; | ||||
|       } | ||||
|     `);
 | ||||
| 
 | ||||
|     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 add non-null assertions to accesses of `ActivatedRouteSnapshot.fragment`', | ||||
|      async () => { | ||||
|        writeFile('/index.ts', ` | ||||
|           import {ActivatedRoute} from '@angular/router'; | ||||
| 
 | ||||
|           class App { | ||||
|             private _route: ActivatedRoute; | ||||
| 
 | ||||
|             getFragment() { | ||||
|               return this._getSnapshot().fragment.foo; | ||||
|             } | ||||
| 
 | ||||
|             private _getSnapshot() { | ||||
|               return this._route.snapshot; | ||||
|             } | ||||
|           } | ||||
|         `);
 | ||||
| 
 | ||||
|        await runMigration(); | ||||
| 
 | ||||
|        expect(tree.readContent('/index.ts')).toContain('return this._getSnapshot().fragment!.foo'); | ||||
|      }); | ||||
| 
 | ||||
|   it('should not add non-null assertions to accesses of `ActivatedRouteSnapshot.fragment` if there is one already', | ||||
|      async () => { | ||||
|        writeFile('/index.ts', ` | ||||
|         import {ActivatedRoute} from '@angular/router'; | ||||
| 
 | ||||
|         class App { | ||||
|           private _route: ActivatedRoute; | ||||
| 
 | ||||
|           getFragment() { | ||||
|             return this._route.snapshot.fragment!.foo; | ||||
|           } | ||||
|         } | ||||
|       `);
 | ||||
| 
 | ||||
|        await runMigration(); | ||||
| 
 | ||||
|        expect(tree.readContent('/index.ts')) | ||||
|            .toContain('return this._route.snapshot.fragment!.foo;'); | ||||
|      }); | ||||
| 
 | ||||
|   it('should not add non-null assertions if the `ActivatedRouteSnapshot.fragment` has been null checked in an if statement', | ||||
|      async () => { | ||||
|        writeFile('/index.ts', ` | ||||
|         import {ActivatedRouteSnapshot} from '@angular/router'; | ||||
| 
 | ||||
|         function getFragmentValue(snapshot: ActivatedRouteSnapshot) { | ||||
|           if (snapshot.fragment) { | ||||
|             return snapshot.fragment.value; | ||||
|           } | ||||
| 
 | ||||
|           return null; | ||||
|         } | ||||
|       `);
 | ||||
| 
 | ||||
|        await runMigration(); | ||||
| 
 | ||||
|        const content = tree.readContent('/index.ts'); | ||||
|        expect(content).toContain(`if (snapshot.fragment) {`); | ||||
|        expect(content).toContain(`return snapshot.fragment.value;`); | ||||
|      }); | ||||
| 
 | ||||
|   it('should not add non-null assertions if the `ActivatedRouteSnapshot.fragment` has been null checked in an else if statement', | ||||
|      async () => { | ||||
|        writeFile('/index.ts', ` | ||||
|         import {ActivatedRouteSnapshot} from '@angular/router'; | ||||
| 
 | ||||
|         function getSnapshotValue(foo: boolean, snapshot: ActivatedRouteSnapshot) { | ||||
|           if (foo) { | ||||
|             return foo; | ||||
|           } else if (snapshot.fragment) { | ||||
|             return snapshot.fragment.value; | ||||
|           } | ||||
| 
 | ||||
|           return null; | ||||
|         } | ||||
|       `);
 | ||||
| 
 | ||||
|        await runMigration(); | ||||
| 
 | ||||
|        const content = tree.readContent('/index.ts'); | ||||
|        expect(content).toContain(`} else if (snapshot.fragment) {`); | ||||
|        expect(content).toContain(`return snapshot.fragment.value;`); | ||||
|      }); | ||||
| 
 | ||||
|   it('should not add non-null assertions if the `ActivatedRouteSnapshot.fragment` has been null checked in a ternary expression', | ||||
|      async () => { | ||||
|        writeFile('/index.ts', ` | ||||
|         import {ActivatedRouteSnapshot} from '@angular/router'; | ||||
| 
 | ||||
|         function getSnapshotValue(snapshot: ActivatedRouteSnapshot) { | ||||
|           return snapshot.fragment ? snapshot.fragment.value : null; | ||||
|         } | ||||
|       `);
 | ||||
| 
 | ||||
|        await runMigration(); | ||||
| 
 | ||||
|        expect(tree.readContent('/index.ts')) | ||||
|            .toContain(`return snapshot.fragment ? snapshot.fragment.value : null;`); | ||||
|      }); | ||||
| 
 | ||||
|   it('should not add non-null assertion to `ActivatedRouteSnapshot.fragment` if there is a safe access', | ||||
|      async () => { | ||||
|        writeFile('/index.ts', ` | ||||
|         import {ActivatedRouteSnapshot} from '@angular/router'; | ||||
| 
 | ||||
|         function getSnapshotValue(snapshot: ActivatedRouteSnapshot) { | ||||
|           return snapshot.fragment?.value; | ||||
|         } | ||||
|       `);
 | ||||
| 
 | ||||
|        await runMigration(); | ||||
|        expect(tree.readContent('/index.ts')).toContain(`return snapshot.fragment?.value;`); | ||||
|      }); | ||||
| 
 | ||||
|   function writeFile(filePath: string, contents: string) { | ||||
|     host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents)); | ||||
|   } | ||||
| 
 | ||||
|   function runMigration() { | ||||
|     return runner.runSchematicAsync('migration-v12-activated-route-snapshot-fragment', {}, tree) | ||||
|         .toPromise(); | ||||
|   } | ||||
| }); | ||||
| @ -0,0 +1,210 @@ | ||||
| /** | ||||
|  * @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 ActivatedRouteSnapshot.fragment TSLint rule', () => { | ||||
|   const rulesDirectory = | ||||
|       dirname(require.resolve('../../migrations/google3/activatedRouteSnapshotFragmentRule')); | ||||
| 
 | ||||
|   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 ActivatedRoute { | ||||
|         get children(): ActivatedRoute[]; | ||||
|         fragment: Observable<string | null>; | ||||
|         snapshot: ActivatedRouteSnapshot; | ||||
|         url: Observable<UrlSegment[]>; | ||||
|       } | ||||
| 
 | ||||
|       export declare class ActivatedRouteSnapshot { | ||||
|         fragment: string | null; | ||||
|         url: UrlSegment[]; | ||||
|       } | ||||
|     `);
 | ||||
| 
 | ||||
|     writeFile('tsconfig.json', JSON.stringify({ | ||||
|       compilerOptions: { | ||||
|         module: 'es2015', | ||||
|         baseUrl: './', | ||||
|         strictNullChecks: true, | ||||
|         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: {'activated-route-snapshot-fragment': 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 accesses to `ActivatedRouteSnapshot.fragment`', () => { | ||||
|     writeFile('/index.ts', ` | ||||
|       import {ActivatedRoute} from '@angular/router'; | ||||
| 
 | ||||
|       class App { | ||||
|         private _route: ActivatedRoute; | ||||
| 
 | ||||
|         ngOnInit() { | ||||
|           this._route.fragment.subscribe(); | ||||
|         } | ||||
| 
 | ||||
|         getFragment() { | ||||
|           return this._route.snapshot.fragment.foo; | ||||
|         } | ||||
|       } | ||||
|   `);
 | ||||
| 
 | ||||
|     const linter = runTSLint(false); | ||||
|     const failures = linter.getResult().failures.map(failure => failure.getFailure()); | ||||
|     expect(failures).toEqual(['`ActivatedRouteSnapshot.fragment` is nullable.']); | ||||
|   }); | ||||
| 
 | ||||
|   it('should add non-null assertions to accesses of `ActivatedRouteSnapshot.fragment`', () => { | ||||
|     writeFile('/index.ts', ` | ||||
|       import {ActivatedRoute} from '@angular/router'; | ||||
| 
 | ||||
|       class App { | ||||
|         private _route: ActivatedRoute; | ||||
| 
 | ||||
|         getFragment() { | ||||
|           return this._getSnapshot().fragment.foo; | ||||
|         } | ||||
| 
 | ||||
|         private _getSnapshot() { | ||||
|           return this._route.snapshot; | ||||
|         } | ||||
|       } | ||||
|     `);
 | ||||
| 
 | ||||
|     runTSLint(true); | ||||
| 
 | ||||
|     expect(getFile('/index.ts')).toContain('return this._getSnapshot().fragment!.foo'); | ||||
|   }); | ||||
| 
 | ||||
|   it('should not add non-null assertions to accesses of `ActivatedRouteSnapshot.fragment` if there is one already', | ||||
|      () => { | ||||
|        writeFile('/index.ts', ` | ||||
|         import {ActivatedRoute} from '@angular/router'; | ||||
| 
 | ||||
|         class App { | ||||
|           private _route: ActivatedRoute; | ||||
| 
 | ||||
|           getFragment() { | ||||
|             return this._route.snapshot.fragment!.foo; | ||||
|           } | ||||
|         } | ||||
|       `);
 | ||||
| 
 | ||||
|        runTSLint(true); | ||||
| 
 | ||||
|        expect(getFile('/index.ts')).toContain('return this._route.snapshot.fragment!.foo;'); | ||||
|      }); | ||||
| 
 | ||||
|   it('should not add non-null assertions if the `ActivatedRouteSnapshot.fragment` has been null checked in an if statement', | ||||
|      () => { | ||||
|        writeFile('/index.ts', ` | ||||
|         import {ActivatedRouteSnapshot} from '@angular/router'; | ||||
| 
 | ||||
|         function getFragmentValue(snapshot: ActivatedRouteSnapshot) { | ||||
|           if (snapshot.fragment) { | ||||
|             return snapshot.fragment.value; | ||||
|           } | ||||
| 
 | ||||
|           return null; | ||||
|         } | ||||
|       `);
 | ||||
| 
 | ||||
|        runTSLint(true); | ||||
| 
 | ||||
|        const content = getFile('/index.ts'); | ||||
|        expect(content).toContain(`if (snapshot.fragment) {`); | ||||
|        expect(content).toContain(`return snapshot.fragment.value;`); | ||||
|      }); | ||||
| 
 | ||||
|   it('should not add non-null assertions if the `ActivatedRouteSnapshot.fragment` has been null checked in an else if statement', | ||||
|      () => { | ||||
|        writeFile('/index.ts', ` | ||||
|         import {ActivatedRouteSnapshot} from '@angular/router'; | ||||
| 
 | ||||
|         function getSnapshotValue(foo: boolean, snapshot: ActivatedRouteSnapshot) { | ||||
|           if (foo) { | ||||
|             return foo; | ||||
|           } else if (snapshot.fragment) { | ||||
|             return snapshot.fragment.value; | ||||
|           } | ||||
| 
 | ||||
|           return null; | ||||
|         } | ||||
|       `);
 | ||||
| 
 | ||||
|        runTSLint(true); | ||||
| 
 | ||||
|        const content = getFile('/index.ts'); | ||||
|        expect(content).toContain(`} else if (snapshot.fragment) {`); | ||||
|        expect(content).toContain(`return snapshot.fragment.value;`); | ||||
|      }); | ||||
| 
 | ||||
|   it('should not add non-null assertions if the `ActivatedRouteSnapshot.fragment` has been null checked in a ternary expression', | ||||
|      () => { | ||||
|        writeFile('/index.ts', ` | ||||
|         import {ActivatedRouteSnapshot} from '@angular/router'; | ||||
| 
 | ||||
|         function getSnapshotValue(snapshot: ActivatedRouteSnapshot) { | ||||
|           return snapshot.fragment ? snapshot.fragment.value : null; | ||||
|         } | ||||
|       `);
 | ||||
| 
 | ||||
|        runTSLint(true); | ||||
| 
 | ||||
|        expect(getFile('/index.ts')) | ||||
|            .toContain(`return snapshot.fragment ? snapshot.fragment.value : null;`); | ||||
|      }); | ||||
| 
 | ||||
|   it('should not add non-null assertion to `ActivatedRouteSnapshot.fragment` if there is a safe access', | ||||
|      () => { | ||||
|        writeFile('/index.ts', ` | ||||
|         import {ActivatedRouteSnapshot} from '@angular/router'; | ||||
| 
 | ||||
|         function getSnapshotValue(snapshot: ActivatedRouteSnapshot) { | ||||
|           return snapshot.fragment?.value; | ||||
|         } | ||||
|       `);
 | ||||
| 
 | ||||
|        runTSLint(true); | ||||
|        expect(getFile('/index.ts')).toContain(`return snapshot.fragment?.value;`); | ||||
|      }); | ||||
| }); | ||||
| @ -26,3 +26,43 @@ export function closestNode<T extends ts.Node>(node: ts.Node, kind: ts.SyntaxKin | ||||
| 
 | ||||
|   return null; | ||||
| } | ||||
| 
 | ||||
| /** | ||||
|  * Checks whether a particular node is part of a null check. E.g. given: | ||||
|  * `foo.bar ? foo.bar.value : null` the null check would be `foo.bar`. | ||||
|  */ | ||||
| export function isNullCheck(node: ts.Node): boolean { | ||||
|   if (!node.parent) { | ||||
|     return false; | ||||
|   } | ||||
| 
 | ||||
|   // `foo.bar && foo.bar.value` where `node` is `foo.bar`.
 | ||||
|   if (ts.isBinaryExpression(node.parent) && node.parent.left === node) { | ||||
|     return true; | ||||
|   } | ||||
| 
 | ||||
|   // `foo.bar && foo.bar.parent && foo.bar.parent.value`
 | ||||
|   // where `node` is `foo.bar`.
 | ||||
|   if (node.parent.parent && ts.isBinaryExpression(node.parent.parent) && | ||||
|       node.parent.parent.left === node.parent) { | ||||
|     return true; | ||||
|   } | ||||
| 
 | ||||
|   // `if (foo.bar) {...}` where `node` is `foo.bar`.
 | ||||
|   if (ts.isIfStatement(node.parent) && node.parent.expression === node) { | ||||
|     return true; | ||||
|   } | ||||
| 
 | ||||
|   // `foo.bar ? foo.bar.value : null` where `node` is `foo.bar`.
 | ||||
|   if (ts.isConditionalExpression(node.parent) && node.parent.condition === node) { | ||||
|     return true; | ||||
|   } | ||||
| 
 | ||||
|   return false; | ||||
| } | ||||
| 
 | ||||
| /** Checks whether a property access is safe (e.g. `foo.parent?.value`). */ | ||||
| export function isSafeAccess(node: ts.Node): boolean { | ||||
|   return node.parent != null && ts.isPropertyAccessExpression(node.parent) && | ||||
|       node.parent.expression === node && node.parent.questionDotToken != null; | ||||
| } | ||||
|  | ||||
| @ -27,3 +27,57 @@ export function isReferenceToImport( | ||||
|   return !!(nodeSymbol && importSymbol) && | ||||
|       nodeSymbol.valueDeclaration === importSymbol.valueDeclaration; | ||||
| } | ||||
| 
 | ||||
| /** Checks whether a node's type is nullable (`null`, `undefined` or `void`). */ | ||||
| export function isNullableType(typeChecker: ts.TypeChecker, node: ts.Node) { | ||||
|   // Skip expressions in the form of `foo.bar!.baz` since the `TypeChecker` seems
 | ||||
|   // to identify them as null, even though the user indicated that it won't be.
 | ||||
|   if (node.parent && ts.isNonNullExpression(node.parent)) { | ||||
|     return false; | ||||
|   } | ||||
| 
 | ||||
|   const type = typeChecker.getTypeAtLocation(node); | ||||
|   const typeNode = typeChecker.typeToTypeNode(type, undefined, ts.NodeBuilderFlags.None); | ||||
|   let hasSeenNullableType = false; | ||||
| 
 | ||||
|   // Trace the type of the node back to a type node, walk
 | ||||
|   // through all of its sub-nodes and look for nullable tyes.
 | ||||
|   if (typeNode) { | ||||
|     (function walk(current: ts.Node) { | ||||
|       if (current.kind === ts.SyntaxKind.NullKeyword || | ||||
|           current.kind === ts.SyntaxKind.UndefinedKeyword || | ||||
|           current.kind === ts.SyntaxKind.VoidKeyword) { | ||||
|         hasSeenNullableType = true; | ||||
|         // Note that we don't descend into type literals, because it may cause
 | ||||
|         // us to mis-identify the root type as nullable, because it has a nullable
 | ||||
|         // property (e.g. `{ foo: string | null }`).
 | ||||
|       } else if (!hasSeenNullableType && !ts.isTypeLiteralNode(current)) { | ||||
|         current.forEachChild(walk); | ||||
|       } | ||||
|     })(typeNode); | ||||
|   } | ||||
| 
 | ||||
|   return hasSeenNullableType; | ||||
| } | ||||
| 
 | ||||
| /** | ||||
|  * Walks through the types and sub-types of a node, looking for a | ||||
|  * type that has the same name as one of the passed-in ones. | ||||
|  */ | ||||
| export function hasOneOfTypes( | ||||
|     typeChecker: ts.TypeChecker, node: ts.Node, types: string[]): boolean { | ||||
|   const type = typeChecker.getTypeAtLocation(node); | ||||
|   const typeNode = | ||||
|       type ? typeChecker.typeToTypeNode(type, undefined, ts.NodeBuilderFlags.None) : undefined; | ||||
|   let hasMatch = false; | ||||
|   if (typeNode) { | ||||
|     (function walk(current: ts.Node) { | ||||
|       if (ts.isIdentifier(current) && types.includes(current.text)) { | ||||
|         hasMatch = true; | ||||
|       } else if (!hasMatch && !ts.isTypeLiteralNode(current)) { | ||||
|         current.forEachChild(walk); | ||||
|       } | ||||
|     })(typeNode); | ||||
|   } | ||||
|   return hasMatch; | ||||
| } | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user