Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

refactor(core): Make the typed forms migration apply to all usages of the symbols. (#45311)

Browse source 
Previously, the migration only migrated constructor calls. Now, the migration will rewrite every usage, in all contexts. Both ways are technically correct, but migrating all symbols is likely to produce clearer and more readable results.

PR Close #45311
This commit is contained in:
Dylan Hunn 2022-03-09 18:48:52 -08:00 committed by Alex Rickabaugh
parent 79d334b138
commit c26915c8cf
3 changed files with 147 additions and 67 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

56
packages/core/schematics/migrations/typed-forms/util.ts
View file

@ -9,14 +9,14 @@
import ts from 'typescript';
import {getImportSpecifier} from '../../utils/typescript/imports';
import {isReferenceToImport} from '../../utils/typescript/symbol';
export const classes = new Set(['FormArray', 'FormBuilder', 'FormControl', 'FormGroup']);
export const formControl = 'FormControl';
export const untypedPrefix = 'Untyped';
export const forms = '@angular/forms';
export interface MigratableNode {
node: ts.Expression;
node: ts.Node;
importName: string;
}
@ -50,7 +50,7 @@ export function migrateFile(
}
}
// For each imported control class, insert the corresponding uptyped import.
// For each imported control class, migrate to the corresponding uptyped import.
for (const imp of imports) {
const untypedClass = getUntypedVersionOfImportOrName(imp.getText());
if (untypedClass === null) {
@ -64,7 +64,7 @@ export function migrateFile(
// class is already present. If present, immediately continue.
continue;
}
rewrite(imp.getEnd(), 0, `, ${untypedClass}`);
rewrite(imp.getStart(), imp.getWidth(), untypedClass);
}
}
@ -89,18 +89,52 @@ function getUntypedVersionOfImportOrName(name: string): string|null {
function getUsages(
sourceFile: ts.SourceFile, typeChecker: ts.TypeChecker,
importSpecifier: ts.ImportSpecifier|null): MigratableNode[] {
if (importSpecifier === null) return [];
importSpecifier: ts.ImportSpecifier): MigratableNode[] {
const usages: MigratableNode[] = [];
const visitNode = (node: ts.Node) => {
// Look for a `new` expression with no type arguments which references an import we care about:
// `new FormControl()`
if (ts.isNewExpression(node) && !node.typeArguments &&
isReferenceToImport(typeChecker, node.expression, importSpecifier)) {
usages.push({node: node.expression, importName: importSpecifier.getText()});
if (ts.isImportSpecifier(node)) {
// Skip this node and all of its children; imports are a special case.
return;
}
if (ts.isIdentifier(node) && isUsageOfFormsImport(typeChecker, node, importSpecifier)) {
usages.push({node, importName: importSpecifier.getText()});
}
ts.forEachChild(node, visitNode);
};
ts.forEachChild(sourceFile, visitNode);
return usages;
}
function isUsageOfFormsImport(
typeChecker: ts.TypeChecker, node: ts.Identifier,
importSpecifier: ts.ImportSpecifier): boolean {
const symbol = typeChecker.getSymbolAtLocation(node);
// We check symbol.declarations because we actually care about the name at the declaration site,
// not the usage site. These could be different in the case of overriden constructors.
if (!symbol || symbol.declarations === undefined || !symbol.declarations.length) return false;
const decl = symbol.declarations[0];
if (!ts.isImportSpecifier(decl)) return false;
// As per `typescript/imports.ts`, we must walk up the tree to find the enclosing import
// declaration. For reasons specific to the TS AST, this is always 3 levels up from an import
// specifier node.
const importDecl = decl.parent.parent.parent;
if (!ts.isStringLiteral(importDecl.moduleSpecifier)) return false;
const importName = typeChecker.getTypeAtLocation(importSpecifier)?.getSymbol()?.getName();
if (!importName) return false;
// Handles aliased imports: e.g. "import {Component as myComp} from ...";
const declName = decl.propertyName ? decl.propertyName.text : decl.name.text;
if (importName === declName) return true;
// In the case of FormControl's overridden exported constructor, the value name and declaration
// name are not exactly the same. For our purposes, it's enough to check whether the latter is a
// substring of the former.
if (declName === formControl && importName.includes(declName)) return true;
return false;
}

31
packages/core/schematics/test/google3/typed_forms_spec.ts
View file

@ -66,43 +66,26 @@ describe('Google3 typedForms TSLint rule', () => {
return readFileSync(join(tmpDir, fileName), 'utf8');
}
it('should migrate a complete example', () => {
// This is just a sanity check for the TSLint configuration; see test/typed_forms_spec.ts for the
// full test suite.
it('should migrate a simple example', () => {
writeFile('/index.ts', `
import { Component } from '@angular/core';
import { AbstractControl, FormArray, FormBuilder, FormControl as FC, FormGroup, UntypedFormGroup } from '@angular/forms';
@Component({template: ''})
export class MyComponent {
private _control = new FC(42);
private _group = new FormGroup({});
private _control: FC = new FC(42);
private _group: FormGroup = new FormGroup({});
private _array = new FormArray([]);
private _ungroup = new FormGroup({});
private fb = new FormBuilder();
build() {
const c = this.fb.control(42);
const g = this.fb.group({one: this.fb.control('')});
const a = this.fb.array([42]);
const fc2 = new FC(0);
}
}
`);
const linter = runTSLint(true);
const cases = [
// All the imports should be paired with an new untyped version,
// except UntypedFormGroup (which is already present).
`import { AbstractControl, FormArray, UntypedFormArray, FormBuilder, UntypedFormBuilder, FormControl as FC, UntypedFormControl, FormGroup, UntypedFormGroup } from '@angular/forms';`,
// Existing constructor calls should be rewritten, in various positions, including qualified
// imports.
`private _control = new UntypedFormControl(42);`,
`private _group = new UntypedFormGroup({});`,
`private _control: UntypedFormControl = new UntypedFormControl(42);`,
`private _group: UntypedFormGroup = new UntypedFormGroup({});`,
`private _array = new UntypedFormArray([]);`,
`private fb = new UntypedFormBuilder();`,
`const fc2 = new UntypedFormControl(0);`,
// Except UntypedFormGroup, which is already migrated.
`private _ungroup = new UntypedFormGroup({});`,
];
cases.forEach(t => expect(getFile(`/index.ts`)).toContain(t));
});

127
packages/core/schematics/test/typed_forms_spec.ts
View file

@ -38,15 +38,42 @@ describe('Typed Forms migration', () => {
// 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 class FormControl {}
export declare class FormGroup {}
export declare class FormArray {}
export declare class AbstractControl {}
export declare class FormBuilder {}
export declare class UntypedFormControl {}
export declare class UntypedFormGroup {}
export declare class UntypedFormArray {}
export declare class UntypedFormBuilder {}
export interface FormControl<T = any> {}
type FormControlInterface<T = any> = FormControl<T>;
export interface ɵFormControlCtor {
new<T = any>(value?: any): FormControl<T>;
}
export const FormControl: ɵFormControlCtor =
(class FormControl<T = any> implements FormControlInterface<T> {
constructor(value?: any) {}
});
export declare class FormGroup<T = any> {
constructor(controls?: any)
}
export declare class FormArray {
constructor(controls?: any)
}
export declare class AbstractControl {}
export declare class FormBuilder {
control(v: any): void;
group(v: any): void;
array(v: any): void;
}
export declare class UntypedFormControl {
constructor(value?: any)
}
export declare class UntypedFormGroup {
constructor(controls?: any)
}
export declare class UntypedFormArray {
constructor(controls?: any)
}
export declare class UntypedFormBuilder {
control(v: any): void;
group(v: any): void;
array(v: any): void;
}
export declare class Form {}
`);
previousWorkingDir = shx.pwd();
@ -69,58 +96,94 @@ describe('Typed Forms migration', () => {
@Component({template: ''})
export class MyComponent {
private _control = new FC(42);
private _control: FC = new FC(42);
private _group = new FormGroup({});
private _array = new FormArray([]);
private _ungroup = new FormGroup({});
private _array: FormArray = new FormArray([]);
private _ungroup: UntypedFormGroup = new UntypedFormGroup({});
private FormC: Form = new Form();
private nested = new FormGroup({a: new FC(1)});
private nested2 = new FormGroup<{a: FormGroup<{b: FC<number>}>}>({a: new FormGroup<{b: FC<number>}>(new FC(1))});
private fb = new FormBuilder();
private fb2!: FormBuilder;
private someSet = new Set([1]);
private FCSet = new Set<FC>(new FC(1));
build() {
foo(fc: FC) {}
bar<T extends FormGroup>(baz: T) {}
baz(T: FormGroup&string) {}
build(fg: FormGroup) {
let tg: UntypedFormGroup;
const c = this.fb.control(42);
const g = this.fb.group({one: this.fb.control('')});
const a = this.fb.array([42]);
const fc2 = new FC(0);
}
}
class TypedFormGroup extends FormGroup {}
let a!: TypedFormGroup;
class ormGroup extends FormGroup {}
`);
await runMigration();
// There are a huge number of positions in which identifiers can show up. This tests an
// assortment of them, but is not exhaustive.
const cases = [
// All the imports should be paired with an new untyped version,
// except UntypedFormGroup (which is already present).
`import { AbstractControl, FormArray, UntypedFormArray, FormBuilder, UntypedFormBuilder, FormControl as FC, UntypedFormControl, FormGroup, UntypedFormGroup } from '@angular/forms';`,
// Existing constructor calls should be rewritten, in various positions, including qualified
// imports.
`private _control = new UntypedFormControl(42);`,
// Imports, excluding already migrated imports
`import { AbstractControl, UntypedFormArray, UntypedFormBuilder, UntypedFormControl, FormGroup, UntypedFormGroup } from '@angular/forms';`,
// Constructor calls, in various positions and qualifications
`private _control: UntypedFormControl = new UntypedFormControl(42);`,
`private _group = new UntypedFormGroup({});`,
`private _array = new UntypedFormArray([]);`,
`private _array: UntypedFormArray = new UntypedFormArray([]);`,
`private fb = new UntypedFormBuilder();`,
`const fc2 = new UntypedFormControl(0);`,
// Except UntypedFormGroup, which is already migrated.
`private _ungroup = new UntypedFormGroup({});`,
// Declarations
`let tg: UntypedFormGroup;`,
`private fb2!: UntypedFormBuilder;`,
// Function parameters
`foo(fc: UntypedFormControl) {}`,
`build(fg: UntypedFormGroup) {`,
// Generic arguments
`private FCSet = new Set<UntypedFormControl>(new UntypedFormControl(1));`,
// Generic functions
`bar<T extends UntypedFormGroup>(baz: T) {}`,
// Intersection types
`baz(T: UntypedFormGroup&string) {}`,
// Nested types
`private nested = new UntypedFormGroup({a: new UntypedFormControl(1)});`,
`private nested2 = new UntypedFormGroup<{a: UntypedFormGroup<{b: UntypedFormControl<number>}>}>({a: new UntypedFormGroup<{b: UntypedFormControl<number>}>(new UntypedFormControl(1))});`,
// Skip UntypedFormGroup, which is already migrated (idempotent migration)
`private _ungroup: UntypedFormGroup = new UntypedFormGroup({});`,
// Form class should not be changed.
`private FormC: Form = new Form();`,
// Unrelated constructors should not be changed.
`private someSet = new Set([1]);`,
// Unrelated classes with similar names should not be changed.
`private someSet = new Set([1]);`,
`class TypedFormGroup extends UntypedFormGroup {}`,
`let a!: TypedFormGroup;`,
`class ormGroup extends UntypedFormGroup {}`,
];
cases.forEach(t => expect(tree.readContent('/index.ts')).toContain(t));
});
it('skip adding imports that would be unused', async () => {
writeFile('/index.ts', `
import { Component } from '@angular/core';
import { FormControl, FormGroup } from '@angular/forms';
import {Component} from '@angular/core';
import {FormControl, FormGroup} from '@angular/forms';
@Component({template: ''})
export class MyComponent {
private _group!: FormGroup;
private _control = new FormControl(42);
}
`);
@Component({template: ''}) export class MyComponent {
private fc: FormControl;
} `);
await runMigration();
const cases = [
// Because FormGroup is never directly constructed, the import should not be added.
`import { FormControl, UntypedFormControl, FormGroup } from '@angular/forms';`,
// Because FormGroup is never used, the import should not be updated.
`import {UntypedFormControl, FormGroup} from '@angular/forms';`,
];
cases.forEach(t => expect(tree.readContent('/index.ts')).toContain(t));
});