nicoespeon / abracadabra

Automated refactorings for VS Code (JS & TS) ✨ It's magic ✨

Hi there, welcome 👋

My name is Nicolas Carlo and I help people build maintainable softwares!

About me

I live in Montréal (Canada 🍁) and I'm a Freelance Web Developer specialized in legacy codebases.

I'm really into community events 🍻 and I organize the Software Crafters, React, and TypeScript meetups in Montréal. I'm also the organizer of The Legacy of SoCraTes conferences. I give talks too!

I frequently share advice to help you deal with Legacy Code on my blog Understand Legacy Code. I even published on book to share the techniques that work best: Legacy Code: First Aid Kit ⛑

I'm the author of Abracadabra, a VS Code extension for intuitive JS/TS refactorings 🔮

The best way to reach me out is through Twitter: @nicoespeon

abracadabra's Issues

New Refactoring: Extract Function

If you want this refactoring to be developed next, please vote on this issue with 👍

VS Code does provide this refactoring. But again, the UX is not the one I expect.

Problem

It's not that bad, for sure. It handles extraction into a method if we're in a class.

But:

• It always take me some time to understand where the extracted function would be
• I need to select the whole selection to get the quick fix. I can't have a partial selection, or simply the cursor on the expression

Inspiration

Webstorm does this well: https://www.jetbrains.com/help/webstorm/specific-javascript-refactorings.html#javascript_extract_method

It's even better because, when you select where to extract the function, it does scroll to the position, making it explicit.

What needs to be done

It's not a trivial one. Check if there is a way to rely on VS Code extraction, but provide a better UX (partial selection & showing "where" the function will be extracted).

Otherwise, the mechanics is:

1. Create a new function
2. Copy the extracted code from the source function into the new one
3. Scan the extracted code for references that won't be in the new function scope. Pass them as parameters
   • This step is not necessary if we create the function in the same scope
   • If a variable is only used inside the extracted code, move the declaration into the extracted code
   • If a variable is assigned in the extracted function, return this variable from the function.
   • If multiple variables are assigned in the extracted function, return a structure containing all variables
4. Replace the extracted code with a call to the function
5. (Find code similar to the extracted code in the same scope and ask user about replacing these too)
6. Trigger "Rename Symbol" on the new function

Expand selection to region with nearest possible refactoring

Is this request related to a problem? Please describe.

Trying to refactor there is a necessity to select or put the cursor on the exact place to see the available refactorings. I know that it is a default for VSC built-in refactoring but it is problematic/ irritating.

Describe the solution you'd like

It would be perfect if there will be a command which will combine two steps:

1. Expand selection to the region with nearest possible refactoring if there is not currently already done
2. Show available refactorings for the selected region/code

The feature I mean is implemented in that way in another extension: Intelli Refactor

Additional context

Thank you for great extension!

Performance improvements

This is a continuation of #77.

Extract a variable on a function with types inferred

Describe what's missing

With the following snippet as example:

import PanelGroup from "react-panelgroup";
import React from "react";

const MyAwesomeComp: React.FC = () => {
  // ...

  return (
  <PanelGroup onUpdate={([sidebar]) => setSidebarWidth(prev => sidebar.size || prev)}>
    {/* Implementation details… */}
  </PanelGroup>)
};

If I try to "Extract variable" the onUpdate function in a typescript context, the refactor will not be as good as expected due to the lack of types.

Indeed, when a function is defined as props, the types inferred, but not when it's extracted.

Expected behavior

In a perfect world, I would like this after the "Extract variable" on this kind of case.

import PanelGroup, {PropsTypes} from "react-panelgroup";
import React from "react";

const MyAwesomeComp: React.FC = () => {
  // ...

  const extracted: PropsType["onUpdate"] = ([sidebar]) => setSidebarWidth(prev => sidebar.size || prev)
  return (
  <PanelGroup onUpdate={extracted}>
    {/* Implementation details… */}
  </PanelGroup>)
};

One step further

In an ever better world, what I really would like is to have a Extract in useCallback!

Additional information

• Version of the extension used: v0.9.0

New Refactoring: Simplify ternary expression

Is this request related to a problem?

I find myself refactoring boolean expressions quite often and sometimes I (i.e. the available refactorings) end up with one of the following expressions:

const one = a ? b : true;
const two = a ? b : false;
const three = a ? true : c;
const four = a ? false : c;
const five = a ? true : false;
const six = a ? false : true;

All of these can be simplified:

const one = !a || b; // = a ? b : true;
const two = Boolean(a) && b; // = a ? b : false;
const three = Boolean(a) || c; // = a ? true : c;
const four = !a && c; // = a ? false : c;
const five = Boolean(a); // = a ? true : false;
const six = !a; // = a ? false : true;

If a is boolean, the casts are redundant and can be omitted.

Describe the solution you'd like

I‘d love a refactoring on any ternary that simplifies these expressions.

Additional context

Additionally, an "Expand to ternary expression" refactoring which is the inverse of the described "Simplify ternary expression" would be awesome.

There are other expressions which can be simplified, e.g.:

const seven = a || !a; // => a
const eight = true || b; // => true
const nine = false || b; // => b
const ten = true || true; // => true
const eleven = false || false; // => false
const twelve = true || false; // => true
const thirteen = false || true; // => true
const fourteen = a && a; // => a
const fifteen = a && !a; // => false
const sixteen = a && true; // => Boolean(a)
const seventeen = a && false; // => false
const eighteen = true && a; // => a
const nineteen = false && a; // => false
const twenty = a && b || a && c; // => a && (b || c)
const twentyOne = !(a && b); // => !a || !b
const twentyTwo = !(!a || !b); // => Boolean(a && b)
...

This feature request covers ternary expressions but my desired solution would be called "Simplify expression“ and it would recursively try to find the shortest /most simple version of a given expression.

Clashing keyboard shortcut for important VS Code feature

Shift + Ctrl + Arrow up/down control the multiple selection cursor which is a top VS Code feature, it seems abracadabra overrides this with the Move Statement shortcut... this can't be right?!

Configure a Travis CI

It would be great to have tests run against PRs and master. I've activated the repo on Travis. Now we only miss:

• Set up a Travis CI config on this project.
• Put the badge in the README

Better contribution guide

Hey!

I'm trying to contribute to this extension, but couldn't figure out how can I make changes in Abracadabra's code and see them live within the Extension Development Host (EDH). I tried running Webpack in watch mode, but then the EDH wouldn't start. I tried in regular build (yarn run build), but then I couldn't see the changes in the EDH until I restart webpack.

Extension runs out of memory while browsing node_modules

Describe the bug

A clear and concise description of what the bug is.

How to reproduce

I was just browsing prettier folder in my node_modules

Expected behavior

does not run out of memory

Screenshots

Additional information

New Refactoring: Extract Generic Type

This is a refactoring we started working on in February, but life came across and we didn't finish it: https://github.com/nicoespeon/abracadabra/tree/extract-generic-type

Creating an issue so we don't forget to finish this.

The main idea is to be able to make a type generic (so it's TS specific):

interface Position {
  x: number;
  y: number;
}

With cursor on x: number would propose "Extract Generic Type" refactoring. If you trigger it, code will become:

interface Position<T = number> {
  x: T;
  y: number;
}

And T symbol should enter rename mode.

Extract Variable should work on selected code fragment

Describe what's missing

The Extract Variable refactoring currently only runs when the cursor is placed in a piece of code. To have more control about what exactly is being extracted, the refactoring should allow the user to select an expression in the code editor. Currently, if you select a piece of code and run the refactoring, you get an error message: "I didn't found a valid code to extract from current selection".

Not sure if this is a bug or just not supported yet.

How would it work

The refactoring takes into account what is being selected. Only the selected expression is extracted to a variable.

Expected behavior

Example:

function test() {
	const myVar = {
		someArray: [
			{
				somethingNested: 42
			}
		]
	};
	console.log(myVar.someArray[0].somethingNested);
}

Select myVar.someArray[0] and run the refactoring ->

...
        const extracted = myVar.someArray[0];
	console.log(extracted.somethingNested);
...

Select myVar.someArray[0].somethingNested and run the refactoring ->

...
        const extracted = myVar.someArray[0].somethingNested;
	console.log(extracted);
...

Additional information

• Version of the extension used: v3.2.1

Shortcut conflict

First of all, thank you for this great extension! ❤️

Describe what's missing

One shortcut built-in to VS Code which I frequently use is command + shift + down/up, to select from the cursor to the start/end of file.

Unfortunately this extension defines a shortcut which conflicts with this. I can probably reconfigure the extension to avoid this conflict, but I thought I would file an issue in case this effects other users. Perhaps we could replace the default shortcut with one that isn't already being used? 😄

Extract Variable from a subset of string or template literal

Describe what's missing

It would be nice to be able to extract selected text subset of a string or template literal.

How would it work

For example, with this code:

const message = "[*] Mailing list worker started (attached to web app)";

If we select Mailing list worker and run Extract Variable, the code should be:

const extracted = "Mailing list worker";
const message = `[*] ${extracted} started (attached to web app)`;

Same for template literals.

Additional information

We should probably handle a bunch of edge cases:

• if we select the whole string, we should extract the whole string and not have `${extracted}`
• if we don't select something and we just have the cursor inside the string, it should still extract the whole string
• if selection is not just a string (e.g. in a template literal, we could select the interpolated variables), then it should extract the whole string too

Re-Introduce "Convert to Template Literal" for non-concatenated, valid string literals

Is this request related to a problem? Please describe.

With release 4.0.0 the Convert to Template Literal was removed from the extension. The explanation for this is that VSCode introduced a similar refactoring (see here). The change log suggests that the refactoring is now obsolete in Abracadabra, so it was removed.

I am not so happy with this change, because the VSCode feature does not cover the same workflow that was possible with the Abracadabra refactoring.
It seems VSCode works like this: you take an already written string concatination like

const a = "I have " + numApples + " apples";

, apply the VSCode refactoring which changes the code to

const a = "I have ${numApples} apples";

This is useful, but it only works if you already have such a concatination.

My workflow with the refactoring often looked like this: I have a string in which I now want to inject a dynamic value. So I could place the cursor into the string and apply the Convert to Template Literal refactoring, e.g.

const a = "I have some apples";

to

const a = "I have ${numApples} apples";

This now longer works now.

Describe the solution you'd like

Would you consider re-introducing the refactoring to the extension?

convert to class based component to function based component is not working

Describe the bug

A clear and concise description of what the bug is.

How to reproduce

Please detail steps to reproduce the behavior. Add code if necessary.

Expected behavior

Clear and concise description of what you expected to happen.

Screenshots

If applicable, add screenshots to help explain your problem.

Additional information

• Version of the extension impacted: vX.X.X

🧙‍ Add any other context about the problem here.

Redo forces cursor to end of file

Describe the bug

Thanks for awesome extension!

When I do a 'split if statement' it works, I undo it, that works, then redo it and the cursor goes to the bottom of the file.

• Version of the extension impacted: v1.2.1

🧙‍ Add any other context about the problem here.

New Refactoring: Add Braces to JSX Attribute

Is this request related to a problem? Please describe.

Sometimes, I need to transform a primitive JSX attribute into an expression. To do so, I need to wrap the attribute with braces. I'd be nice if Abracadabra could do it for me.

Describe the solution you'd like

With following code:

function render() {
  return <Header title="Hello" />; 
}

If my cursor is on the title="Hello" attribute, I'd like to propose a quick fix to add braces around:

function render() {
  return <Header title={"Hello"} />; 
}

Additional context

The difference between these 2 snippets in terms of AST is a JSXExpressionContainer.

First AST is:

Second AST is:

So we need to put the value of the attribute into a JSXExpressionContainer if it's not already.

Provide Quick Fix for Inline Variable/Function

Describe what's missing

A quick fix option in the VSCode Quick Fix menu (CTRL+.) for the Inline Variable / Inline Function refactoring when it is applicable.

How would it work

It would check whether the refactoring can be applied at the currently selected location in the code and would then add the refactoring to the Quick Fix menu just like it's being done with other refactorings

Expected behavior

Consider this code:

const test = "hello";
console.log(test);

Put cursor on the variable declaration ("test" in line 1). VSCode should then offer the Inline Variable Quick Fix.

Additional information

• Version of the extension used: _v4.0.0

New Rafactoring: Create function

A lot of times I would write:

return <button onClick={handleClick}/>

And then, above the return statement I would implement handleClick.

I would be nice if when the cursor is within the identifier handleClick, I could have the option to create a function with the same name, resulting in the following code:

function handleClick() {

}

return <button onClick={handleClick}/>

And just wanted to add that I like this extension a lot! "Extract Variable" especially has been very useful in my day to day coding.

Leverage "Refactoring documentation contribution point"

Describe what's missing

See microsoft/vscode#86788

This is a proposal for extensions to show refactoring documentation to end users.

We could use that to show a link to the docs when refactorings can be made (JS, JSX, TS, TSX files), for Abracadabra users.

How would it work

We'd use that to redirect people to Abracadabra refactoring documentation (which is in the README, as of today).

Convert to template literal shoud not be propose on imports

Describe the bug

Abracadabra propose to transform my import into template literal, even if it's valid, I'm not sure we should propose this refactor here 😁

How to reproduce

Import something and clic on the bulb!

Expected behavior

Nothing! Leave my import alone 😅

Screenshots

Improve tests of the first refactorings

"Extract Variable", "Inline Variable" and "Negate Expression" are not implemented the same way than the more recent refactorings.

Instead of manipulating the AST, we identify the nodes, then ask VS Code to read the code and finally we act on that. We could probably refactor them to have the same implementation than the recent ones though.

The problem is their tests are not super explicit like the others can be.

What we ideally want is:

• what the code looks before refactoring
• where is the selection
• what the code looks after the refactoring

Improve the tests of these refactorings to show that. It will require implementing an in-memory adapter for that.

• Create adapter
• Inline Variable
• Extract Variable
• Negate Expression

Extract variable with empty arrays around

Describe the bug

Abracadabra propose to replace way too many values when I "extract variable" an array with the entire array selected.

How to reproduce

With the following snippet:

const a = []

const b = [{a: "foo", b: "bar"}, {a: "foo", b: "bar"}]

1. Select [{a: "foo", b: "bar"}, {a: "foo", b: "bar"}]
2. Execute the helper "Extract variable"
3. Select "Replace all 2 occurrences"
4. The result is:

const extracted = [{a: "foo", b: "bar"}, {a: "foo", b: "bar"}];
const a = extracted

const b = extracted

5. 😱 My code is broken! This is not a refactor! 😱

Expected behavior

Don't replace a when a != b

Screenshots

Additional information

• Version of the extension impacted: v0.9.0

This bug is only present with empty arrays (a = []), so probably in the comparison algo.

Use selection for "Add braces to if statement"

Describe what's missing

Currently, when you choose "Add braces to if statement" it works with a single line; however, it's often the case that multiple lines need to be added to an if statement.

How would it work

Say I have three lines that are not yet part of an if statement (S1):

let ok
ok = someFunc()
callAnotherFunc(ok)

I've started typing an if statement (S2):

if (condition === 'true')
let ok
ok = someFunc()
callAnotherFunc(ok)

And now I'd like to make all three lines part of an if statement block (S3):

if (condition === 'true') {
  let ok
  ok = someFunc()
  callAnotherFunc(ok)
}

Expected behavior

I'd like to be able to highlight the three lines above (in S2), and then choose "Add braces to if statement" and have it result in S3.

Additional information

Version of the extension used: 4.1.0

Extract Variable refactoring screws up code

Describe the bug

When I try to apply the Extract Variable refactoring, the generated code is messed up, resulting in not compilable code. See below.

How to reproduce

Consider this code:

function test(arg: string[]) {
	console.log(arg.length);
	const a = arg.length + 1;
	const b = arg.length % 2;
	return { a, b };
}

Put the cursor on the length part of arg.length (e.g. on the "n"). Execute Extract Variable refactoring.

The refactoring generates this code:

function test(arg: string[]) {
	conconst { length } = arg;
    sole.log(arglength
	const a = arglength1;
	const b = arglength2;
	return { a, b };
}

Expected behavior

The expression is extracted to a single variable and each occurence is replaced with the created variable. No syntax error is created.
E.g. something like this:

function test(arg: string[]) {
        const length = arg.length;
	console.log(length);
	const a = length + 1;
	const b = length % 2;
	return { a, b };
}

Additional information

• Version of the extension impacted: 3.2.1

Make "Convert for to forEach" handle for…of loops

The for loop to foreach refactoring apparently does not work on for (const entry of entries) style loops. At least the plugin says it did not detect a valid for-loop.
Would be great to make it work for this style of loops.

Abracadabra version: 3.2.3

Unable to start the debugger in "Run Extension" mode

First of all: Thanks for this great extension 👍

I'm experiencing an issue when trying to start the debugger in "Run Extension" mode and i'm not sure if this is a bug or just my somewhat misconfigured VSCode 😅

VSCode shows me an error telling me that the npm: build task does not exist:

The .vscode/tasks.json indeed only contains an npm: watch task and i'm not sure if the build task is actually missing or if VSCode "somehow" should figure it out.

Extension version: latest master commit (a72c6f9)
VSCode version: Version: 1.41.1

New Refactoring: Move to an existing file

Is this request related to a problem?

There already is a built-in move to file refactoring but it only moves functions to a new file with the function name as file name.

Describe the solution you'd like

A "Move to file" refactoring should be able to either move the function to a new file (already built-in) or an existing file. Imports and global constants should be moved (or copied if moving is not possible because they are used elsewhere) as well.

New Refactoring: Remove Braces from If Statement

Is this request related to a problem? Please describe.

It's the reverse of #24

I prefer to use one-liner If Statements to write guard clause.

function doSomething() {
  if (!isValid) return;

  // … rest of the code
}

Sometimes, the guard clause is implemented with braces:

function doSomething() {
  if (!isValid) {
    return;
  }

  // … rest of the code
}

I'd like to simply remove braces with a quick fix, so it will take 2 keystrokes. Without that, I have to:

• put the cursor after the closing brace
• remove the closing brace
• put the cursor after the opening brace
• remove the opening brace
• remove the remaining spaces to inline the return next to the if

Describe the solution you'd like

Given:

function doSomething() {
  if (!isValid) {
    return;
  }

  console.log("Something");
}

When I trigger the refactoring on the If Statement node,

Then I want:

function doSomething() {
  if (!isValid) return;

  console.log("Something");
}

Additional context

It's a similar refactoring than Remove Braces from Arrow Function.

You can use yarn new to help you bootstrap the refactoring.

I'm here to help.

Remove dead code should remove unused import

Describe what's missing

When I run Remove dead code on my imports, the unused one are not cleaned.

Expected behavior

If it's dead / unused, remove the code.

Screenshots

Additional information

• Version of the extension used: v0.9.0

Do you noticed performance issue? Let us know.

I think the way we provide the Quick Fixes might lead to performance issues. So far, it seems to be fine. But I'm not sure how much current implementation will scale.

If you experienced performance issues because of the extension, please let us know here 🙏

Details

Today, each Quick Fix is independent.

To tell VS Code if it should propose a refactoring as a Quick Fix, we traverse the code, check if the refactoring could be made from current selection, and tell VS Code if we can.

Example of such behavior, implemented in action-provider.ts:

abracadabra/src/refactorings/convert-ternary-to-if-else/action-provider.ts

Lines 15 to 35 in 28273cf

	public provideCodeActions(
	document: vscode.TextDocument,
	range: vscode.Range | vscode.Selection
	): vscode.ProviderResult<vscode.CodeAction[]> {
	const code = document.getText();
	const selection = createSelectionFromVSCode(range);
	if (!hasTernaryToConvert(code, selection)) return;
	const action = new vscode.CodeAction(
	"✨ Convert ternary to if/else",
	this.kind
	);
	action.isPreferred = false;
	action.command = {
	command: commandKey,
	title: "Convert Ternary to If/Else"
	};
	return [action];
	}

abracadabra/src/refactorings/convert-ternary-to-if-else/convert-ternary-to-if-else.ts

Lines 27 to 29 in 28273cf

	function hasTernaryToConvert(code: Code, selection: Selection): boolean {
	return updateCode(code, selection).hasCodeChanged;
	}

The performance problem would be due to the fact that each refactoring parses, traverses and transforms the AST. At some point, this could be too much.

If this is confirmed, the solution would be to traverse the AST less. Ideally, we would traverse it once and try to figure out what list of refactorings could be performed from that.

The challenge would be to provide individual action providers to VS Code. That's why current implementation is very naive, so every refactoring is independent—which simplifies development a lot!

---

But until we got actual confirmation that this is a concern, we'll keep current implementation. #makeItWork #makeItRight #makeItFast

Make "Extract Variable" handle multiple occurrences

Current implementation of "Extract Variable" only extract the selected occurrence.

It could detect other similar occurrences. If there is more than one, it can propose to extract all of them to the user.

Webstorm does that quite well:

If the user decides to extract all occurrences, we should replace all of them with the extracted variable.

How to provide the best UX?

The trick is to provide a nice UX when asking the user about that:

• if there is only 1 occurrence detected, don't even ask
• if there are many occurrences detected, then we need something

Few options in mind:

1. The simplest way seems to be a quick input, but I would prefer something that is close to the extraction point.
2. Ideally, we would have a choice selection popup like we have for Quick Fixes (cf. Webstorm screenshot).
3. I thought about not asking and consider this is almost what we always want to do. It might be true, but the problem is that it will be really inconvenient when it's something we don't want to do. I think asking is the best solution here. We should aim to reduce friction as much as possible.

So, if we manage to get the selection popup appear where the cursor is when we ask for extraction, I'd order the choices differently:

1. Extract X occurrences
2. Extract this occurrence only

That's because my personal experience is I want to extract all the occurrences 80% of the time I do this.

After digging into VS Code capabilities, I think there is no other way than using "Quick Pick" to do so. I'd have preferred the popup to be at the cursor position to stay in context, but I don't see how to do that with VS Code.

Thus, the ideal behaviour would be:

• Extract something with only one occurrence => do the extraction (today's behaviour) 1 action
• Extract something with multiple occurrences
  • Trigger a selection popup (Quick Pick)
  • User can select to extract all occurrences by pressing ↵ (default choice) 2 actions
  • User can select to extract only this occurrence (arrow down & ↵) 3 actions

New Refactoring: Remove Braces from JSX Attribute

Is this request related to a problem? Please describe.

This is the reverse of #61.

Sometimes I could simplify a JSX attribute and I don't need the wrapping braces anymore. Having an Abracadabra quick fix would make this change a little bit faster.

Describe the solution you'd like

With following code:

function render() {
  return <Header title={"Hello"} />; 
}

If my cursor is on the title={"Hello"} attribute, I'd like to propose a quick fix to remove braces around:

function render() {
  return <Header title="Hello" />; 
}

Additional context

It's the reverse of #61, so we need to unwrap the content of the JSXExpressionContainer back into the JSX attribute value.

Improve Move Statements UX

Describe what's missing

The current UX of "Move Statements" refactorings is not as good as I'd expect.

Few things that needs to be improved:

• On some patterns, the refactoring would add unexpected blank spaces. That's the case when we move objects / classes attributes or methods.
• The refactoring bubbles up to the parent, and that's sometimes unexpected. For example, if we move down a method of a class it will move the whole class down if we reach the end and the method can't be moved down anymore => the command bubbled up to the whole class, and that was not expected
• Overall, moving statements should feel fluid and the scrolling should follow accordingly

Expected behavior

Webstorm does it very well. What I want is to feel the same comfort in moving statements within VS Code, thanks to Abracadabra.

Let's do some usability tests (e.g. try to move statements on some patterns and see if it doesn't feel perfect) and fix the issues.

I'll close the issue when I feel Move Statements UX is so good that I would only use that to move things up and down.

To Do

After playing with current implementation, here are the things we want to improve:

• block scope (don't move parent!)
• only scroll when it's necessary
• Arrays, just like object properties
• fix cursor position after moving statement (fix the edge cases: comments, blank spaces, methods, functions, etc.)
• refactor duplication between slide up and slide down I didn't find the correct abstraction yet, so I'll keep it like that

Extract Class

Is this request related to a problem? Please describe.

Often when I'm refactoring, I find a hidden class. A collection of functions and properties that would be better in a new class or in another existing class.

Describe the solution you'd like

I'd like to be able to select a function or more, maybe even some variables, and extract it out to a new class.
I'd also like for all existing calls to those functions to now be prepended with the new class.

Additional context

For example, I'd like to turn this:

class Foo () {
  barSize: int = 30;
  fooString: string = "baz";

  doFoo () {
    console.log(fooString);
  }

  doBar() {
    return barSize /2;
  }

  doubleBar() {
    return barSize *2;
  }
}

class Baz() {
  Foo.doBar();
  Foo.doFoo();
}

into this:

class Foo () {
  fooString: string = "baz";

  doFoo () {
    console.log(fooString);
  }
}

class Bar() {
  barSize: int = 30;
  
  doBar() {
    return barSize /2;
  } 

  doubleBar() {
    return barSize *2;
  }
}

class Baz() {
  Bar.doBar();
  Foo.doFoo();
}

Being able to then move it into a new file would be a nice bonus.

New Refactoring: Remove Braces from Arrow Function

As we introduced Add Braces to Arrow Function, we should implement the reverse.

It should work as a Quick Fix.

It should not work if arrow function body doesn't have just a return statement.

Why this refactoring?

Same reason than Add Braces to Arrow Function.

VS Code does provide this refactoring too, but it has the same problem as usual: it only works if you got the correct selection:

If your cursor is inside the arrow function, no cookie for you:

Convert if/else to ternary could be more intelligent with refactoring return statements

I am not sure if this is a bug or a feature request. As soon as I started using this plugin I am finding immediately many cases like this:

function calculateYear(selectedDates) {
  if (
    selectedDates[0].getMonth() === 0 ||
    (selectedDates[0].getMonth() === 1 && selectedDates[0].getDate() < 4)
  ) {
    return selectedDates[0].getFullYear() - 1;
  }
  return selectedDates[0].getFullYear();
}

abracadabra does not seem to recognise this as an ideal chance to convert to a ternary, and there is no lighbulb or option to convert. It would be great if it did see this opportunity and output:

function calculateYear(selectedDates) {
  return selectedDates[0].getMonth() === 0 ||
    (selectedDates[0].getMonth() === 1 && selectedDates[0].getDate() < 4)
    ? selectedDates[0].getFullYear() - 1
    : selectedDates[0].getFullYear();
}

I know this is not a great bit of logic in my code, but this is just an example, I am having a go at refactoring an old app :) Cheers!

Inline variable doesn't work with destructured variable declarations

Describe the scenario

"Inline variable" refactoring doesn't work if the variable is declared via destructuring

How to reproduce

// Case 1.
// Command+Option+N ->
// Throws "I didn't found a valid code to inline from current selection 🤔" error
const { userId } = session
return messages.map(message => ({ userId }))


// Case 2.
// Command+Option+N ->
// Inlines just fine
const playerId = session.playerId
return messages.map(message => ({ playerId }))

Expected behavior

Expecting Case 1. to work

Screenshots

Additional information

• Version of the extension impacted: v0.4.0

Thank you for this amazing project btw ❤️

Progression

• Handle destructured object patterns #34
• Handle destructured array patterns #38

ActionProvider throws if code can't be parsed

Describe the bug

When I write following code:

error message:

Unexpected token (29:4): SyntaxError: Unexpected token (29:4)
	at Object.raise (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:657452)
	at Object.unexpected (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:677107)
	at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:698149)
	at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:575944)
	at Object.parseExprSubscripts (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:690154)
	at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:689746)
	at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:646315)
	at Object.parseExprOps (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:687156)
	at Object.parseMaybeConditional (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:686688)
	at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:685576)
	at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:645779)
	at Object.parseObjectProperty (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:706149)
	at Object.parseObjPropValue (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:706631)
	at Object.parseObjPropValue (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:644824)
	at Object.parseObjectMember (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:704931)
	at Object.parseObj (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:703762)
	at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:697112)
	at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:575944)
	at Object.parseExprSubscripts (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:690154)
	at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:689746)
	at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:646315)
	at Object.parseExprOps (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:687156)
	at Object.parseMaybeConditional (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:686688)
	at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:685576)
	at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:645779)
	at Object.parseObjectProperty (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:706149)
	at Object.parseObjPropValue (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:706631)
	at Object.parseObjPropValue (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:644824)
	at Object.parseObjectMember (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:704931)
	at Object.parseObj (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:703762)
	at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:697112)
	at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:575944)
	at Object.parseExprSubscripts (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:690154)
	at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:689746)
	at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:646315)
	at Object.parseExprOps (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:687156)
	at Object.parseMaybeConditional (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:686688)
	at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:685576)
	at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:645779)
	at Object.parseVar (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:728033)
	at Object.parseVarStatement (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:724611)
	at Object.parseStatementContent (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:717106)
	at Object.parseStatementContent (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:641972)
	at Object.parseStatement (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:715840)
	at Object.parseBlockOrModuleBlockBody (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:726551)
	at Object.parseBlockBody (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:726360)
	at Object.parseTopLevel (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:714491)
	at Object.parse (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:743906)
	at Object.t.parse (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:745123)
	at Object.parse (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:1040107)
	at Object.t.parse (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:1281972)
	at Object.y [as parse] (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:1040079)
	at t.RefactoringActionProvider.provideCodeActions (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:1345737)
	at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:586:864
	at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:54:345
	at new Promise (<anonymous>)
	at Object.t.asPromise (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:54:317)
	at T.provideCodeActions (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:586:835)
	at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:609:631
	at V._withAdapter (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:604:832)
	at V.$provideCodeActions (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:609:609)
	at p._doInvokeHandler (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:688:464)
	at p._invokeHandler (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:688:156)
	at p._receiveRequest (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:686:766)
	at p._receiveOneMessage (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:685:623)
	at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:683:791
	at l.fire (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:48:845)
	at v.fire (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:191:325)
	at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:851:104
	at l.fire (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:48:845)
	at v.fire (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:191:325)
	at t.PersistentProtocol._receiveMessage (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:195:717)
	at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:192:897
	at l.fire (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:48:845)
	at p.acceptChunk (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:188:897)
	at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:188:249
	at Socket.t (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:197:232)
	at Socket.emit (events.js:203:13)
	at addChunk (_stream_readable.js:295:12)
	at readableAddChunk (_stream_readable.js:276:11)
	at Socket.Readable.push (_stream_readable.js:210:10)
	at Pipe.onStreamRead (internal/stream_base_commons.js:166:17)

How to reproduce

write following code:

const obj = {
    a:
}

Expected behavior

no error message

Additional information

• Version of the extension impacted: vX.X.X

Version: 1.43.0 (user setup)
Commit: 78a4c91400152c0f27ba4d363eb56d2835f9903a
Date: 2020-03-09T19:47:57.235Z
Electron: 7.1.11
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Windows_NT x64 10.0.18363_

Move commands / action providers next to their refactorings

Following the principle of proximity: everything that changes together, should be close together.

Current structure for commands & action providers is mostly "a technical choice" as it's "the VS Code part". We're mature enough to move these along refactorings now.

New Refactoring: Implement function

Is this request related to a problem? Please describe.

My workflow often involves writing the call site of a new function first and then to write the actual function. This means I have to declare the function signature so I usually copy and paste the function name, check the infered types of the parameters and manually add the parameters to the signature. This is a lot of repeated manual work and I'd like some automatic solution for that.

Edit: I understand that technically this isn't a refactoring since it does not involve transforming working code to a different form of working code, but rather broken code to working code. But maybe this would also be in the scope of this extension.

Describe the solution you'd like

I suggest a new refactoring which implements the declaration of a new function based on its call site. The new function declaration is put in the enclosing scope of the current function.

For example, say I have this code:

function doSomething(): number {
  const a = 3;
  const b = 4;
  return sum(a, b);
}

The function sum is not declared in this scope. I put my cursor on sum and invoke the refactoring. The refactoring generates code like this:

function doSomething(): number {
  const a = 3;
  const b = 4;
  return sum(a, b);
}

function sum(a: number, b: number): number {

}

This could also work with async code, i.e. invoking the refactoring on return await sum(a, b); would generate async function sum(...).
Ideally, the refactoring is only offered to the user if the function is not declared yet.

Additional context

Here's how Android Studio does it:

This could also be extended to types, class members, etc.

New Refactoring: Inline Function

If you want this refactoring to be developed next, please vote on this issue with 👍

This is the reverse operation of "Extract Function".

Problem

VS Code doesn't seem to propose that.

Inspiration

Webstorm does this refactoring quite well: https://www.jetbrains.com/help/webstorm/specific-javascript-refactorings.html#javascript_inline_refactorings

What needs to be done

If it's a method of a class, we'd need to check if it's overridden in a subclass. It's fine to not handle that case for now and tell the user about it.

The mechanics is the following:

1. Find all callers of the function to inline
2. Replace each call with the function body
3. Remove the function

Extract Variable declaration should be accessible by all occurrences

Describe the bug

When extracting a variable, Abracadabra proposes to replace all occurrences. When doing so, it's possible to generate invalid code because the variable declaration will not be placed in a scope that's accessible by all occurrences.

How to reproduce

function brokenScenario() {
  if (isValid) {
    if (item.name == "sulfuras") {
    }
  }

  console.log("sulfuras");
}

Put your cursor on any "sulfuras" and extract the variable. Select to replace the 2 occurrences.

Here's the resulting code:

function brokenScenario() {
  if (isValid) {
    const sulfuras = "sulfuras";
    if (item.name == sulfuras) {
    }
  }

  console.log(sulfuras);
}

The second occurrence can't access the variable.

The declaration is placed before the top-most occurrence:

abracadabra/src/refactorings/extract-variable/extract-variable.ts

Lines 32 to 41 in 2abc82f

	const topMostOccurrence = extractedOccurrences.sort(topToBottom)[0];
	await editor.readThenWrite(
	selectedOccurrence.selection,
	extractedCode => [
	// Insert new variable declaration.
	{
	code: selectedOccurrence.toVariableDeclaration(extractedCode),
	selection: topMostOccurrence.scopeParentCursor
	},

It doesn't take the scope of all occurrences into account.

Expected behavior

The declaration should be put in the closest common ancestor of all occurrences.

function brokenScenario() {
  const sulfuras = "sulfuras";

  if (isValid) {
    if (item.name == sulfuras) {
    }
  }

  console.log(sulfuras);
}

Inline exported variables

Describe what's missing

Today, Abracadabra limits its scope to the open file. Thus, we don't inline variables that are exported.

Also, unlocking inter-files refactorings will allow for a whole new set of refactorings, including class-related refactorings since classes are often separated in distinct files from my experience.

How would it work

We should be able to index symbols across the editor workspace. Based on that, we should be able to extend refactoring outside of a single file.

That might change the way we handle refactorings too. Today, we parse the AST every time we trigger the refactoring. Tomorrow, we might want to have a language server or something that keeps track of the code structure, so it's faster to update.

Expected behavior

When I inline an exported variable, it is inlined in other files that imports it.

Additional thoughts

It would be awesome if we could rely on existing work. I guess TypeScript or VS Code already does that at some point. It might be the point where it'd be more valuable to contribute to native refactorings that trying to re-do the work in this extension though 🤷‍♂

Replace Jest `it.each` with a custom implementation

Jest it.each seemed great, but they are limited.

Specifically, I want to be able to .only a single test of the list when I'm working on it, which is not doable. I know this can be done at runtime through Jest runner, but I use other tools that are integrated with the editor (so I don't look at the terminal). Thus, I need .only.

New Refactoring: Add Braces to If Statement

Is this request related to a problem? Please describe.

I do use one-liner If Statements to write guard clause.

function doSomething() {
  if (!isValid) return;

  // … rest of the code
}

But sometimes, the guard clause might evolve and do something else. In that scenario, I want to add braces.

function doSomething() {
  if (!isValid) {
    // … now I can add some logic here
    return;
  }

  // … rest of the code
}

Adding braces can be a bit painful because the editor will likely insert the closing one. So I usually have to:

• put the cursor at the right place
• insert the opening brace
• eventually delete the closing one
• put the cursor at the right place again
• insert the closing brace

Describe the solution you'd like

Some extensions might help me wrap selection into braces. I want almost the same thing, but something simpler and specific to If Statements: a quick fix that would trigger whenever the selection is inside the correct node.

Given:

function doSomething() {
  if (!isValid) return;

  console.log("Something");
}

When I trigger the refactoring on the If Statement node,

Then I want:

function doSomething() {
  if (!isValid) {
    return;
  }

  console.log("Something");
}

Additional context

It's a similar refactoring than Add Braces to Arrow Function.

You can use yarn new to help you bootstrap the refactoring.

I'm here to help.

Example on Webstorm:

Update @babel/parser

Deno has top level await enabled, and so does TypeScript 3.8.

At the moment any code with this in stops abracadabra working. I think it could be cured by updating https://www.npmjs.com/package/@babel/parser to the latest, it seems it supports top level await now, but I don't know if you need to enable it? I have not used Babel. Cheers!

Add a Table Of Contents to README and CONTRIBUTING files

Describe what's missing

The documentation is really great and give plenty of information.
As I started to contribute, I unfortunately missed some useful links about AST.
The links are actually provided in the CONTRIBUTING file.
A table of contents, with clickable titles, might help to get a quick look on the file structure and go to the desired section.

How would it work

I suggest to add a basic table of contents.
e.g.: https://github.com/jamiebuilds/babel-handbook/blob/master/translations/en/plugin-handbook.md#table-of-contents

Expected behavior

Provided guidelines have a dynamic table of contents.

Screenshots

Not applicable.

Additional information

Not applicable.

Set up a code generator to create new refactorings

Creating a new refactoring usually involves the same boilerplate code:

• create folder in src/refactorings/
• create refactoring TS file & test file
• create a command file
• update extension.ts to use the command
• if it's exposed through action provider (Quick Fix), create the action provider & update extension.ts accordingly
• update package.json accordingly

We could simplify this step with a code generator.

An example of "new refactoring" is available here: #11

I think we could use https://www.hygen.io/ for that.

It doesn't need to be perfect. A starting point would be good. Hence, this is something that could be done as a first issue (and I can help!) 🧙‍♂️

New Refactoring: Convert Switch to If/Else

This would help me and others a lot. Would it be complex to implement, assuming we already have if/else to switch?

_{Sent with GitHawk}