diff --git a/packages/core/schematics/BUILD.bazel b/packages/core/schematics/BUILD.bazel index 4934255a82..5887aa17a3 100644 --- a/packages/core/schematics/BUILD.bazel +++ b/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/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", diff --git a/packages/core/schematics/migrations.json b/packages/core/schematics/migrations.json index 9d6d557e64..85f5fe8371 100644 --- a/packages/core/schematics/migrations.json +++ b/packages/core/schematics/migrations.json @@ -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" } } -} \ No newline at end of file +} diff --git a/packages/core/schematics/migrations/abstract-control-parent/util.ts b/packages/core/schematics/migrations/abstract-control-parent/util.ts index f5e47c516f..37d5cbff8f 100644 --- a/packages/core/schematics/migrations/abstract-control-parent/util.ts +++ b/packages/core/schematics/migrations/abstract-control-parent/util.ts @@ -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([ - '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; -} diff --git a/packages/core/schematics/migrations/activated-route-snapshot-fragment/BUILD.bazel b/packages/core/schematics/migrations/activated-route-snapshot-fragment/BUILD.bazel new file mode 100644 index 0000000000..ff42f9dc83 --- /dev/null +++ b/packages/core/schematics/migrations/activated-route-snapshot-fragment/BUILD.bazel @@ -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", + ], +) diff --git a/packages/core/schematics/migrations/activated-route-snapshot-fragment/README.md b/packages/core/schematics/migrations/activated-route-snapshot-fragment/README.md new file mode 100644 index 0000000000..b7bc129091 --- /dev/null +++ b/packages/core/schematics/migrations/activated-route-snapshot-fragment/README.md @@ -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; + } +} +``` diff --git a/packages/core/schematics/migrations/activated-route-snapshot-fragment/index.ts b/packages/core/schematics/migrations/activated-route-snapshot-fragment/index.ts new file mode 100644 index 0000000000..447ceec7cb --- /dev/null +++ b/packages/core/schematics/migrations/activated-route-snapshot-fragment/index.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 {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); + } + }); +} diff --git a/packages/core/schematics/migrations/activated-route-snapshot-fragment/util.ts b/packages/core/schematics/migrations/activated-route-snapshot-fragment/util.ts new file mode 100644 index 0000000000..681882f39d --- /dev/null +++ b/packages/core/schematics/migrations/activated-route-snapshot-fragment/util.ts @@ -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 { + const results = new Set(); + + 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); +} diff --git a/packages/core/schematics/migrations/google3/BUILD.bazel b/packages/core/schematics/migrations/google3/BUILD.bazel index 1fcd8e91f9..bd21e587de 100644 --- a/packages/core/schematics/migrations/google3/BUILD.bazel +++ b/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/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", diff --git a/packages/core/schematics/migrations/google3/activatedRouteSnapshotFragmentRule.ts b/packages/core/schematics/migrations/google3/activatedRouteSnapshotFragmentRule.ts new file mode 100644 index 0000000000..4b55b2bc3c --- /dev/null +++ b/packages/core/schematics/migrations/google3/activatedRouteSnapshotFragmentRule.ts @@ -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; + } +} diff --git a/packages/core/schematics/test/BUILD.bazel b/packages/core/schematics/test/BUILD.bazel index c6d62041e8..93a4fec3f2 100644 --- a/packages/core/schematics/test/BUILD.bazel +++ b/packages/core/schematics/test/BUILD.bazel @@ -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", diff --git a/packages/core/schematics/test/activated_route_snapshot_fragment_migration_spec.ts b/packages/core/schematics/test/activated_route_snapshot_fragment_migration_spec.ts new file mode 100644 index 0000000000..4d352e524f --- /dev/null +++ b/packages/core/schematics/test/activated_route_snapshot_fragment_migration_spec.ts @@ -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; + snapshot: ActivatedRouteSnapshot; + url: Observable; + } + + 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(); + } +}); diff --git a/packages/core/schematics/test/google3/activated_route_snapshot_fragment_spec.ts b/packages/core/schematics/test/google3/activated_route_snapshot_fragment_spec.ts new file mode 100644 index 0000000000..19232cf1a9 --- /dev/null +++ b/packages/core/schematics/test/google3/activated_route_snapshot_fragment_spec.ts @@ -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; + snapshot: ActivatedRouteSnapshot; + url: Observable; + } + + 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;`); + }); +}); diff --git a/packages/core/schematics/utils/typescript/nodes.ts b/packages/core/schematics/utils/typescript/nodes.ts index f6b5a3eb0a..f14f4ef46b 100644 --- a/packages/core/schematics/utils/typescript/nodes.ts +++ b/packages/core/schematics/utils/typescript/nodes.ts @@ -26,3 +26,43 @@ export function closestNode(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; +} diff --git a/packages/core/schematics/utils/typescript/symbol.ts b/packages/core/schematics/utils/typescript/symbol.ts index 9d4bbffd1f..a63cc3a989 100644 --- a/packages/core/schematics/utils/typescript/symbol.ts +++ b/packages/core/schematics/utils/typescript/symbol.ts @@ -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; +}