microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.06k stars 12.37k forks source link

Inline function refactoring #27070

Open D0nGiovanni opened 5 years ago

D0nGiovanni commented 5 years ago

Search Terms

inline function method refactoring

Suggestion

I would like a refactoring that inlines a function from

1: function foo() { return 42; }
2: function bar() { const meaningOfLife = foo(); }

to

1: function bar() { const meaningOfLife = 42; }

Use Cases

This is a very common refactoring and thus widely used while cleaning up code.

Examples

In the above code sample, block 1, line 1: selecting foo, the user should be able to inline this function to every occurence and optionally delete the function definition.

In the above code sample, block 1, line 2: selecting foo, the user should be able to inline the function to this occurence, and, if it's the only one, optionally delete the function definition.

I am not sure how the option can be handled in vscode. Eclipse brings up a pop-up with the two options, but afaik vs code tries to be minimalist.

Checklist

My suggestion meets these guidelines:

ghost commented 5 years ago

Related: #18459

Kingwl commented 5 years ago

The boundary of refactor seems to be difficult to determine 🀫

hediet commented 4 years ago

I think this is one of the most important (but underused) refactorings. Many other refactorings (like rename, parameter removal/addition/reordering) can be reduced to this refactoring.

For example, if you want to reorder these parameters:

function apiFunction(arg1: string, arg2: string) {
    // ...
}

function consumerCode() {
    apiFunction("arg1", "arg2");
    apiFunction("foo", "bar");
}

... you can first rename apiFunction to apiFunction_ and delegate to a copy of apiFunction (for n consumers, manual edit complexity is O(1)):

function apiFunction(arg1: string, arg2: string) {
    // ...
}

function apiFunction_(arg1: string, arg2: string) {
    apiFunction(arg1, arg2);
}

function consumerCode() {
    apiFunction_("arg1", "arg2");
    apiFunction_("foo", "bar");
}

Now you can apply the local refactoring to apiFunction_, e.g. swapping parameter arg1 with arg2 (for n consumers, manual edit complexity is O(1)):

function apiFunction(arg2: string, arg1: string) {
    // ...
}

function apiFunction_(arg1: string, arg2: string) {
    apiFunction(arg2, arg1);
}

function consumerCode() {
    apiFunction_("arg1", "arg2");
    apiFunction_("foo", "bar");
}

And now you can inline apiFunction_ (for n consumers, manual edit complexity is O(1)):

function apiFunction(arg2: string, arg1: string) {
    // ...
}

function consumerCode() {
    apiFunction("arg2", "arg1");
    apiFunction("bar", "foo");
}

The inline refactoring is very complicated to implement though. Local variables might need to be renamed to prevent accidental shadowing and pattern destructuring of function arguments must be supported and ideally inlined against supplied object literals.

boyland commented 3 years ago

Yes, I miss this refactoring in VSC. I would also like inline constant expression which is even easier:

const foo = someMethodCall(...);
... foo + 100 ...

becomes

... someMethodCall(...) + 100 ...

In Opdyke's dissertation which gave a rigorous treatment of refactorings, he stressed that every refactoring has an equal and opposite refactoring: extract function has inline function as an inverse. VSC omits lots of the inverses, it seems.

sduzair commented 4 months ago

Looking forward to this! Coming from Uncle Bob's and Primeagen's video. This seems to be super helpful when understanding a function with many nested functions and refactoring towards better abstractions as per clean code.

ukslim commented 3 months ago

Yes, I miss this refactoring in VSC. I would also like inline constant expression which is even easier:

const foo = someMethodCall(...);
... foo + 100 ...

becomes

... someMethodCall(...) + 100 ...

This seems to already be supported as "inline variable". Cursor on the foo of foo + 100, ctrl-cmd-I (in VSC).

I do miss "inline function" though.

ukslim commented 3 months ago

The inline refactoring is very complicated to implement though. Local variables might need to be renamed to prevent accidental shadowing and pattern destructuring of function arguments must be supported and ideally inlined against supplied object literals.

I think it would be acceptable to error and refuse to perform the refactoring if variable names conflict. Refactorings should be mechanical and reversible, so "inline function" followed by "extract function" should get you back where you started. I think it's OK to expect the developer to rename some variables to unblock "inline function" .

Ideally some guidance - "cannot inline function because of conflicting identifier 'foo'"