feat(forms): add migration for AbstractControl.parent accesses (#39009)

As of #32671, the type of `AbstractControl.parent` can be null which can cause
compilation errors in existing apps. These changes add a migration that will append
non-null assertions to existing unsafe accesses.

````
// Before
console.log(control.parent.value);

// After
console.log(control.parent!.value);
```

The migration also tries its best to avoid cases where the non-null assertions aren't
necessary (e.g. if the `parent` was null checked already).

PR Close #39009
This commit is contained in:
Kristiyan Kostadinov 2020-09-26 21:35:25 +03:00 committed by Joey Perrott
parent f2fca6d58e
commit aeec223db1
8 changed files with 560 additions and 0 deletions

View File

@ -10,6 +10,7 @@ pkg_npm(
srcs = ["migrations.json"],
visibility = ["//packages/core:__pkg__"],
deps = [
"//packages/core/schematics/migrations/abstract-control-parent",
"//packages/core/schematics/migrations/dynamic-queries",
"//packages/core/schematics/migrations/missing-injectable",
"//packages/core/schematics/migrations/module-with-providers",

View File

@ -54,6 +54,11 @@
"version": "11.0.0-beta",
"description": "The default value for `relativeLinkResolution` is changing from 'legacy' to 'corrected'.\nThis migration updates `RouterModule` configurations that use the default value to \nnow specifically use 'legacy' to prevent breakages when updating.",
"factory": "./migrations/relative-link-resolution/index"
},
"migration-v11-abstract-control-parent": {
"version": "11.0.0-beta",
"description": "In Angular version 11, the type of `AbstractControl.parent` can be `null` to reflect the runtime value more accurately. This migration automatically adds non-null assertions to existing accesses of the `parent` property on types like `FormControl`, `FormArray` and `FormGroup`.",
"factory": "./migrations/abstract-control-parent/index"
}
}
}

View File

@ -0,0 +1,18 @@
load("//tools:defaults.bzl", "ts_library")
ts_library(
name = "abstract-control-parent",
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",
],
)

View File

@ -0,0 +1,34 @@
## `AbstractControl.parent` migration
As of Angular v11, the type of `AbstractControl.parent` can be null. This migration automatically
identifies usages and adds non-null assertions.
#### Before
```ts
import { Component } from '@angular/core';
import { FormControl } from '@angular/forms';
@Component()
export class MyComponent {
private _control = new FormControl();
getParentValue() {
return this._control.parent.value; // <- Compilation error in v11.
}
}
```
#### After
```ts
import { Component } from '@angular/core';
import { FormControl } from '@angular/forms';
@Component()
export class MyComponent {
private _control = new FormControl();
getParentValue() {
return this._control.parent!.value; // <- Non-null assertion added during the migration.
}
}
```

View File

@ -0,0 +1,57 @@
/**
* @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 {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths';
import {createMigrationProgram} from '../../utils/typescript/compiler_host';
import {findParentAccesses} from './util';
/** Migration that marks accesses of `AbstractControl.parent` 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 AbstractControl.parent accesses.');
}
for (const tsconfigPath of allPaths) {
runNativeAbstractControlParentMigration(tree, tsconfigPath, basePath);
}
};
}
function runNativeAbstractControlParentMigration(
tree: Tree, tsconfigPath: string, basePath: string) {
const {program} = createMigrationProgram(tree, tsconfigPath, basePath);
const typeChecker = program.getTypeChecker();
const sourceFiles = program.getSourceFiles().filter(
f => !f.isDeclarationFile && !program.isSourceFileFromExternalLibrary(f));
sourceFiles.forEach(sourceFile => {
// We sort the nodes based on their position in the file and we offset the positions by one
// for each non-null assertion that we've added. We have to do it this way, rather than
// creating and printing a new AST node like in other migrations, because property access
// expressions can be nested (e.g. `control.parent.parent.value`), but the node positions
// aren't being updated as we're inserting new code. If we were to go through the AST,
// we'd have to update the `SourceFile` and start over after each operation.
findParentAccesses(typeChecker, sourceFile)
.sort((a, b) => a.getStart() - b.getStart())
.forEach((node, index) => {
const update = tree.beginUpdate(relative(basePath, sourceFile.fileName));
update.insertRight(node.getStart() + node.getWidth() + index, '!');
tree.commitUpdate(update);
});
});
}

View File

@ -0,0 +1,155 @@
/**
* @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 {normalize} from 'path';
import * as ts from 'typescript';
/** Names of symbols from `@angular/forms` whose `parent` accesses have to be migrated. */
const abstractControlSymbols = new Set<string>([
'AbstractControl',
'FormArray',
'FormControl',
'FormGroup',
]);
/**
* Finds the `PropertyAccessExpression`-s that are accessing the `parent` property in
* such a way that may result in a compilation error after the v11 type changes.
*/
export function findParentAccesses(
typeChecker: ts.TypeChecker, sourceFile: ts.SourceFile): ts.PropertyAccessExpression[] {
const results: ts.PropertyAccessExpression[] = [];
sourceFile.forEachChild(function walk(node: ts.Node) {
if (ts.isPropertyAccessExpression(node) && node.name.text === 'parent' && !isNullCheck(node) &&
!isSafeAccess(node) && results.indexOf(node) === -1 &&
isAbstractControlReference(typeChecker, node) && isNullableType(typeChecker, node)) {
results.unshift(node);
}
node.forEachChild(walk);
});
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 {
let current: ts.Expression = node;
const formsPattern = /node_modules\/?.*\/@angular\/forms/;
// Walks up the property access chain and tries to find a symbol tied to a `SourceFile`.
// 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 sourceFile = symbol.valueDeclaration?.getSourceFile();
return sourceFile != null &&
formsPattern.test(normalize(sourceFile.fileName).replace(/\\/g, '/')) &&
hasAbstractControlType(typeChecker, type);
}
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;
}

View File

@ -8,6 +8,7 @@ ts_library(
"//packages/core/schematics:migrations.json",
],
deps = [
"//packages/core/schematics/migrations/abstract-control-parent",
"//packages/core/schematics/migrations/dynamic-queries",
"//packages/core/schematics/migrations/missing-injectable",
"//packages/core/schematics/migrations/module-with-providers",

View File

@ -0,0 +1,289 @@
/**
* @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('AbstractControl.parent 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/forms/index.d.ts', `
export declare abstract class AbstractControl {
get dirty(): boolean;
get disabled(): boolean;
get parent(): FormGroup | FormArray | null;
}
export declare class FormArray extends AbstractControl {
getRawValue(): any[];
}
export declare class FormControl extends AbstractControl {
setValue(value: any): void;
}
export declare class FormGroup extends AbstractControl {
getRawValue(): any;
}
`);
// Fake non-Angular package to make sure that we don't migrate packages we don't own.
writeFile('/node_modules/@not-angular/forms/index.d.ts', `
export declare abstract class AbstractControl {
get dirty(): boolean;
get disabled(): boolean;
get parent(): FormGroup | FormArray | null;
}
export declare class FormControl extends AbstractControl {
setValue(value: any): void;
}
`);
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 AbstractControl.parent', async () => {
writeFile('/index.ts', `
import {AbstractControl} from '@angular/forms';
class App {
private _control: AbstractControl;
getParentValue() {
return this._control.parent.value;
}
}
`);
await runMigration();
expect(tree.readContent('/index.ts')).toContain(`return this._control.parent!.value;`);
});
it('should add non-null assertions to accesses of FormArray.parent', async () => {
writeFile('/index.ts', `
import {FormArray} from '@angular/forms';
class App {
getParentValueOf(control: FormArray) {
return control.parent.value;
}
}
`);
await runMigration();
expect(tree.readContent('/index.ts')).toContain(`return control.parent!.value;`);
});
it('should add non-null assertions to accesses of FormControl.parent', async () => {
writeFile('/index.ts', `
import {FormControl} from '@angular/forms';
class App {
getBlankControlParentValue() {
return this._getControl().parent.value;
}
private _getControl() {
return new FormControl();
}
}
`);
await runMigration();
expect(tree.readContent('/index.ts')).toContain(`return this._getControl().parent!.value;`);
});
it('should add non-null assertions to accesses of FormGroup.parent', async () => {
writeFile('/index.ts', `
import {FormGroup} from '@angular/forms';
class App {
getGlobalGroupParentValue() {
const parent = (window.foo as FormGroup).parent;
return parent.value;
}
}
`);
await runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`const parent = (window.foo as FormGroup).parent!;`);
});
it('should add non-null assertions to nested accesses of `AbstractControl.parent`', async () => {
writeFile('/index.ts', `
import {FormControl} from '@angular/forms';
class App {
private _control = new FormControl();
getGreatGrandParentValue() {
return this._control.parent.parent.parent.value;
}
}
`);
await runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`return this._control.parent!.parent!.parent!.value;`);
});
it('should not add non-null assertions if the `parent` has been null checked in an if statement',
async () => {
writeFile('/index.ts', `
import {FormControl} from '@angular/forms';
function getParentValue(control: FormControl) {
if (control.parent) {
return control.parent.value;
}
return null;
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`if (control.parent) {`);
expect(content).toContain(`return control.parent.value;`);
});
it('should not add non-null assertions if the `parent` has been null checked in an else if statement',
async () => {
writeFile('/index.ts', `
import {FormControl} from '@angular/forms';
function getParentValue(foo: boolean, control: FormControl) {
if (foo) {
return foo;
} else if (control.parent) {
return control.parent.value;
}
return null;
}
`);
await runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`} else if (control.parent) {`);
expect(content).toContain(`return control.parent.value;`);
});
it('should not add non-null assertions if the `parent` has been null checked in a ternary expression',
async () => {
writeFile('/index.ts', `
import {FormControl} from '@angular/forms';
function getParentValue(control: FormControl) {
return control.parent ? control.parent.value : null;
}
`);
await runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`return control.parent ? control.parent.value : null;`);
});
it('should not add non-null assertions if a nested `parent` has been null checked', async () => {
writeFile('/index.ts', `
import {FormControl} from '@angular/forms';
function getGreatGrandParentValue(control: FormControl) {
return control.parent && control.parent.parent && control.parent.parent.parent && control.parent.parent.parent.value;
}
`);
await runMigration();
expect(tree.readContent('/index.ts'))
.toContain(
`return control.parent && control.parent.parent && control.parent.parent.parent && control.parent.parent.parent.value;`);
});
it('should not add non-null assertions if there is one already', async () => {
writeFile('/index.ts', `
import {FormControl} from '@angular/forms';
function getParentValue(control: FormControl) {
return control.parent!.value;
}
`);
await runMigration();
expect(tree.readContent('/index.ts')).toContain(`return control.parent!.value;`);
});
it('should not add non-null assertions if there is a safe access', async () => {
writeFile('/index.ts', `
import {FormControl} from '@angular/forms';
function getParentValue(control: FormControl) {
return control.parent?.value;
}
`);
await runMigration();
expect(tree.readContent('/index.ts')).toContain(`return control.parent?.value;`);
});
it('should not add non-null assertions if the symbol does not come from @angular/forms',
async () => {
writeFile('/index.ts', `
import {FormControl} from '@not-angular/forms';
function getParentValue(control: FormControl) {
return control.parent.value;
}
`);
await runMigration();
expect(tree.readContent('/index.ts')).toContain(`return control.parent.value;`);
});
function writeFile(filePath: string, contents: string) {
host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents));
}
function runMigration() {
return runner.runSchematicAsync('migration-v11-abstract-control-parent', {}, tree).toPromise();
}
});