Convert to named parameters by gabritto · Pull Request #30089 · microsoft/TypeScript (original) (raw)

Fixes #23552

Convert to "named parameters" refactor

A function-like declaration with parameters is refactored to receive a single parameter object with properties corresponding to its old parameters. Thus a function's arguments can be named when calling it. This applies to functions, methods, constructors, arrow functions and function expressions.

Example:

function add(a: number, b: number, c: number): number { return a + b + c; }

would be refactored to

function add({ a, b, c }: { a: number, b: number, c: number }): number { return a + b + c; }

Refactor availability

The refactor will be listed as available on:

class Foo { constructor(private a: string) {} }

References

This refactor changes a function's signature, so it affects the way a function should be called either directly or indirectly. The refactor also changes assignability to and from a function's type, since its type will change.
Because of that, there are several situations where performing the refactor could cause a breaking change that would in the best scenario result in type errors or, in the worst scenario, runtime errors and wrong behavior.
The current solution to this problem is to adopt a conservative approach: the refactor will perform changes only if those changes are guaranteed not to break the code. Otherwise, it will not perform any change.
The refactor has to look at all the references of a function (through findAllReferences) to determine if changing it is safe. If a constructor is being refactored, the refactor also looks at all the references of the parent class declaration or class expression.
However, finding all references is too slow to be performed when the user is checking what are the available refactors. So the refactor will only look at all references if the user tries to apply it.
Therefore, the refactor can fail to perform changes even if it was listed as available and the user tried to applied it. Currently there is no way to inform a user why a refactor has failed.

Good cases

Direct calls

The refactor can change direct calls to functions to conform to the new function signature. This is done by taking the old argument list and replacing it with an argument object whose properties have values corresponding to the old arguments.
Example:
1.

function foo(a: number, b: number) { } foo(1, 2);

becomes

function foo({ a, b }: { a: number; b: number; }) { } foo({ a: 1, b: 2 });

const foo = function(a: number, b: number) { }; foo(1, 2);

becomes

const foo = function({ a, b }: { a: number; b: number; }) { }; foo({ a: 1, b: 2 });

class Foo { bar(t: string, s: string): string { } } var foo = new Foo(); foo"bar"; foo.bar("a", "b");

becomes

class Foo { bar({ t, s }: { t: string; s: string; }): string { return s + t; } } var foo = new Foo(); foo["bar"]({ t: "a", s: "b" }); foo.bar({ t: "a", s: "b" });

Imports & Exports

Since findAllReferences can track imports/exports across a project, importing/exporting the refactored function should be allowed.

Other good cases

Good class cases

If a constructor is being refactored, the refactor will also check if all the references to the corresponding class are valid usages.
Currently, for a class declaration (class C { ... }), the following usages are recognized:

  1. New calls:

class C { }

const c = new C();

  1. Property access and element access:

class C { static a: number = 2; }

const b = C.a; C["a"] = 3;

  1. Type references and heritage clauses

class C { } let c: C; function f(c: C) { } class B extends C { } class A implements C { }

For a class expression (const c = class { ... }), the following usages are recognized:

  1. New calls:

const c = class C { }

const a = new c();

  1. Property access and element access:

const c = class C { static a: number = 2; }

const b = c.a; c["a"] = 3;

JSDoc

Currently the refactor will not do anything with JSDoc, it will treat it as regular comments.

  1. Non-inline JSDoc

Example:

/**

should become something like

/**

  1. Inline JSDoc

Example:

function getPerson(/** person's name*/ name: string, /** person's address*/ address: string) { }

Since the parameter names above will become binding elements in a destructuring pattern, we cannot inline the existing JSDoc,
so the refactor will break inline JSDoc.

Rest parameters

The refactor works for rest parameters as illustrated below. The rest parameter will be converted to an optional property of the new parameter object, and the corresponding variable will be initialized to an empty array. This ensures that the variable is not undefined in case no rest arguments are provided, which reflects how rest parameters behave.

Example:

function buildName(firstName: string, middleName: string, ...restOfName: string[]) { return firstName + " " + middleName + " " + restOfName.join(" "); }

let employeeName = buildName("Joseph", "Samuel", "Lucas", "MacKinzie");

becomes

function buildName({ firstName, middleName, restOfName = [] }: { firstName: string, middleName: string, restOfName?: string[] }) { return firstName + " " + middleName + " " + restOfName.join(" "); }

let employeeName = buildName({ firstName: "Joseph", middleName: "Samuel", restOfName: ["Lucas", "MacKinzie"]);

This parameter

A function can have a this parameter as its first parameter. In this case, the refactor should ignore this parameter and only modify the other parameters.

Example:

function f(this: void, a: string, b: string) { } f("a", "b");

becomes

function f(this: void, { a, b }: { a: string, b: string }) { } f({ a: "a", b: "b" });

Parameters with initializers

A parameter with an initializer is considered an optional member of the new parameter type. If a parameter has an initializer but has no type annotation, its type is inferred (using checker.getTypeAtLocation(node)).

Example:

function add(a: number, b = 1) { return a + b; }

should be

function add({ a, b = 1 }: { a: number, b?: number }) { return a + b; }

Optional parameters

When a parameter is optional (when it is followed by a ? or has an initializer), the corresponding property in the generated parameter object type should also be optional.
In addition, if all parameters are optional, the new parameter object should be optional. This is achieved by adding an empty object initializer.

Example:

function foo(a: number, b?: number) { return a; } foo(1);

becomes

function foo({ a, b }: { a: number; b?: number; }) { return a; } foo({ a: 1 });

and

function bar(a?: string, b?: string) { } bar();

becomes

function bar({ a, b }: { a?: string; b?: string; } = {}) { } bar();

JavaScript

Currently the refactor is not available on a JavaScript file, but we would like the refactor to work on JavaScript as well. Essentially the difference in the refactor would be the omission of type annotations.

Example:

function foo(a, b) { return a + b; }

becomes

function foo({ a, b }) { return a + b; }

JS code can be harder to analyze and to validate the refactor's changes.
So, if the refactor does something wrong, it would be harder for the user to notice.
If the refactored function has annotations, it becomes easy to apply the refactor.

Some bad cases

Aliasing

As shown above, aliasing makes it not possible to track down all calls and usages of the refactored function.

Example:

const foo = (a: number, b: number) => a + b; foo(1, 2); var otherFoo = foo; otherFoo(1, 2);

after the refactor:

const foo = ({ a, b }: { a: number; b: number; }) => a + b; foo({ a: 1, b: 2 }); var otherFoo = foo; otherFoo(1, 2); // Error: Expected 1 arguments, but got 2.

Other example:

class Foo { constructor(s: string, t: string) { } } var c = Foo; var f = new c("a", "b");

after refactoring the constructor:

class Foo { constructor({ s, t }: { s: string; t: string; }) { } } var c = Foo; var f = new c("a", "b"); // Error: Expected 1 arguments, but got 2.

Class expression example:

const foo = class { constructor(a: number, b: number) { } } class Bar extends foo { constructor() { super(1, 2); } }

after refactoring the constructor:

const foo = class { constructor({ a, b }: { a: number; b: number; }) { } } class Bar extends foo { constructor() { super(1, 2); // Error: Expected 1 arguments, but got 2. } }

Types

Since the refactor changes the type of the function, there might be type errors.
Example:

function add(a: number, b: number) { return a + b; } function sum(fn: (a: number, b: number) => number, ...numbers: number[]) { let total = 0; for (let n of numbers) { total += n; } return total; } var x: (a: number, b: number) => number = add; var y = sum(add, 1, 2, 3);

becomes:

function add({ a, b }: { a: number, b: number }) { return a + b; } function sum(fn: (a: number, b: number) => number, ...numbers: number[]) { let total = 0; for (let n of numbers) { total += n; } return total; } var x: (a: number, b: number) => number = add; // error on x: type ... is not assignable to type ... var y = sum(add, 1, 2, 3); // error on add: argument of type ... is not assignable to parameter of type ...

Nameless declarations (anonymous functions/classes)

Anonymous functions and classes are tricky when looking for references.
Example:

var foo = (a: number, b: number) => a + b; foo(1, 2); // ... more code ... foo = (a: number, b: number) => a - b; foo(1, 2);

In the example above, if we refactor the arrow function assigned to foo on line 1, then we should refactor the call on line 2. But later on foo is reassigned, so this will cause a type error. The same goes for constructors of class expressions.

bind/call/apply calls

Calling bind, call or apply on a function that has been refactored might go wrong.
Example:

function add(a: number, b: number, c: number) { return a + b + c; } let add1 = add.bind(null, 1); let z = add1(2, 3); let x = add.call(null, 1, 2, 3); let y = add.apply(null, [1, 2, 3]);

becomes

function add({ a, b, c }: { a: number, b: number, c: number }) { return a + b + c; } let add1 = add.bind(null, 1); let z = add1(2, 3); // NaN or Argument not assignable to parameter let x = add.call(null, 1, 2, 3); // NaN or Expected 2 arguments, but got 4 let y = add.apply(null, [1, 2, 3]); // NaN or Type 'number' is not assignable to type '{ a: ... }'

The compiler will emit the errors above only if strictBindCallApply is on.

Overloads

A function which has overloads is tricky to refactor. Changing a function overload or a function implementation which has overloads can be a breaking change. The overloads and the implementation should be considered together when refactoring. It is not clear what the refactor should do with overloads, so the refactor will not be available in that case.

Example:

function foo(a: number, b: number): number; function foo(a: string, b: number): string; function foo(a: string | number, b: number): string | number { // implementation return 0; }

might become

function foo(a: number, b: number): number; // Overload signature is not compatible with function implementation function foo(a: string, b: number): string; function foo({ a, b }: { a: string | number, b: number }): string | number { // implementation return 0; }

or

function foo(a: number, b: number): number; function foo({ a, b }: { a: string, b: number }): string; // Overload signature is not compatible with function implementation function foo(a: string | number, b: number): string | number { // implementation return 0; }

Overrides

As with overloads, refactoring overrided methods can also be a breaking change. Refactoring a method which overrides another can mean that the refactored method's type is no longer assignable to the type of the method it overrides. Similarly, changing a method which is overriden by another can mean that the overriding methods' types are no longer assignable to the refactored method.

Example:

class A { foo(a: string, b: string) { } } class B extends A { foo(c: string, d: string) { } }

would become

class A { foo({ a, b }: { a: string, b: string }) { } } class B extends A { foo(c: string, d: string) { } // error: property 'foo' in type B is not assignable to the same property in base type 'A' }

That scenario is currently avoided by the refactor. Once it checks all the references and finds a declaration different than the one where the refactor should be applied, it does not perform any changes.

Inference

When a function is refactored to have an object parameter instead of a list of parameters, inference can no longer proceed left-to-right on the parameter list, so the compiler no longer reuses inferences from previous parameters for the next parameter.
Because of that, inference can become worse and cause breaks in rare cases (see #29791).

Known issues

Formatting

In some cases involving comments and line breaks, the refactor generates an argument object that is ill-formatted.
The refactorConverToNamedParameters_callComments2.ts test is currently wrong because of that.

Inherited constructors

Currently findAllReferences will not track usage of an inherited constructor.

Example:

class Foo { constructor(t: string, s: string) { } } class Bar extends Foo { } var bar = new Bar("a", "b");

refactoring Foo's constructor, becomes:

class Foo { constructor({ t, s }: { t: string; s: string; }) { } } class Bar extends Foo { } var bar = new Bar("a", "b"); // Error: Expected 1 arguments, but got 2.

The refactorConvertToNamedParameters_inheritedConstructor.ts test is currently wrong because of that.
However, findAllReferences tracks inherited methods, which causes the refactor to behave differently in similar situations when it should behave the same. This leads me to believe that findAllReferences should be able to find references to inherited constructors.
Update: solved in #30514.

Type errors when refactoring a method

When a class/object literal's method is refactored, its type changes.
Because of that, we can get type errors where the new class/object literal type is not compatible with the expected type.
Example:

class Bad { fn(a: number, b: number) { return a + b; } }

interface Expected { fn(a: number, b: number): number; }

declare var e: Expected; declare var b: Bad;

e = b; b = e;

After refactoring fn on Bad:

class Bad { fn({ a, b }: { a: number; b: number; }) { return a + b; } }

interface Expected { fn(a: number, b: number): number; }

declare var e: Expected; declare var b: Bad;

e = b; // Error: Type 'Bad' is not assignable to type 'Expected'. b = e; // Error: Type 'Expected' is not assignable to type 'Bad'.

Open questions

Type Literal or Interface

Should the refactor generate a type literal for the new parameter object or an interface?
In the case where this refactor generates a type literal,
this refactor should be composed with a refactor for extracting a type literal into an interface.

Example:

function add(a: number, b: number, c: number): number { return a + b + c; }

could be refactored to

function add({ a, b, c }: { a: number, b: number, c: number }): number { return a + b + c; }

or

interface AddOptions { a: number; b: number; c: number; }

function add({ a, b, c }: AddOptions) { return a + b + c; }

Name

What should the refactor be called?
Currently it is 'named parameters', but 'parameter object' was suggested.
Update: done in #30401.

Dealing with bad cases

The refactor's current approach is to not apply changes when it finds a reference it does not recognize.
This approach comes from the idea that a refactor should not break user's code, i.e. it should 'do the right thing'.
So, when the refactor does not know the right thing to do, it does nothing.
Some cons:

Good cases

For the current approach, what are other usages of the refactored function that the refactor should recognize?

JavaScript

If the JavaScript code is not type annotated, what should we do?