p42ai / js-assistant Goto Github PK

View Code? Open in Web Editor NEW

119.0 6.0 5.0 55.5 MB

120+ refactorings and code-assists for Visual Studio Code

Home Page: https://marketplace.visualstudio.com/items?itemName=p42ai.refactor

License: MIT License

JavaScript 2.43% TypeScript 95.33% CSS 2.24% Shell 0.01%

vscode typescript vscode-extension javascript legacy-code technical-debt lint refactor codemod visual-studio-code

js-assistant's Issues

Provide a good story for string literal <-> template strings conversion

I usually start with a string literal ("hello world") and later want to convert it to `hello ${name}`.

While the code actions are nice, I think the story can be improved.

I'm thinking of automatic conversion from a normal string to a template string once the user types the first ${}, and the opposite direction once the users removes the last ${}.

Also, the conversion code action should keep the cursor at its position in the string (use two edits for the prefix and suffix instead a single one for the entire string).

inline refactor bug in jsx

export function App() {
  const handleClick = () => console.log(test);
  return (
    <>
      <div onClick={handleClick}>test</div>
    </>
  );
}

If the handleClick function is refactored inline, the output is as follows. (<,t is repeated twice )

export function App() {
  return (
    <>
      <<div onClick={() => console.log(test)}>ttest</div>
    </>
  );
}

Feature: global/user configuration

While I really enjoy this extension, the inability to disable something like Add Numeric Separator without polluting upstream repositories with my personal preferences, kinda annoy me.

Being able to configure stuff like that in VS Code User settings would help.

On a side note: As per p42 configuration

Adding a p42.toml to .gitignore and a p42.toml file to repository root, containing:

[refactoring.add-numeric-separator]
enable = false

Have zero effect ???

v1.29.0 causes the Extension Host to crash

Hi,

VSCode: 1.60.2
Extension: 1.29.0
OS: Windows 10.0.19043 x64

After starting VSCode, the error message "Extension host terminated unexpectedly" appears, with an option to restart it. After restarting it, it terminates again after a few seconds.

From the window log:
[2021-09-30 11:26:03.284] [renderer1] [error] Extension host terminated unexpectedly. The following extensions were running: aaron-bond.better-comments, mechatroner.rainbow-csv, vscode.extension-editing, vscode.typescript-language-features, christian-kohler.npm-intellisense, p42ai.refactor, VisualStudioExptTeam.vscodeintellicode, vscode.microsoft-authentication, ms-vscode.js-debug, vscode.debug-auto-launch, vscode.git, acarreiro.calculate, mhutchie.git-graph, PKief.material-icon-theme, rafamel.subtle-brackets, ritwickdey.LiveServer, vincaslt.highlight-matching-tag, vscode-icons-team.vscode-icons, wayou.vscode-todo-highlight, vscode.configuration-editing, vscode.json-language-features, vscode.npm

Checking the "Running Extensions" list, the P42 refactor extension is marked as unresponsive. Disabling the extension solves the problem (extension host does not terminate anymore).

Could you please look into this? TIA

push-operator-into-assignment error

I got this message “Can push + operator into += assignment.” for this line of code:

content = excerpt + content;

The proposed replacement is (which is marked as safe):

content += excerpt;

Which is not the same as I which to put excerpt before content in resulting content variable.

Code Action Idea: Reorder If/Else Branches

if (cond1) {
    body1
} el|se if (cond2) {
    body2
} else {
    body3
}

(| cursor)

-- Move Item Up ->

|if (cond2) {
    body2
} else if (cond1) {
    body1
} else {
    body3
}

Could use the same shortcut for reordering parameters/members etc.

Move Up/Down Code Actions

Personally, I think generic Up/Down Move Code Actions are extremely useful.
There should be two clever commands: Move Up and Move Down. Dependening on the selection and context, they do "the right". Because there are only two, they should have good keybindings.

Here is an (incomplete) collection of what they would enable:

Sometimes a move is safe, sometimes not.

A key property of move commands should be that "Move up" and "Move down" are inverse.

Refactor Idea: Factor out common substring

return name ? `From Event: ${name}` : 'From Event';

return 'From Event' + (name ? `: ${name}` + '');

Idea: Support Renaming Unresolved Variables

After copy&pasting, I ended up with this code:

public extendInputRange(extendedInputRange: LineRange): LineRangeMapping {

    const startDelta = extendedInputRange.startLineNumber - l.inputRange.startLineNumber;
    const endDelta = extendedInputRange.endLineNumberExclusive - l.inputRange.endLineNumberExclusive;
    return new LineRangeMapping(
        inputRange,
        new LineRange(
            l.outputRange.startLineNumber + startDelta,
            l.outputRange.lineCount - startDelta + endDelta
        )
    );
}

Now I would like to replace l with this.
This is not very easy to do.

It would be awesome if I could rename l to this.

Refactoring Idea: Early Return and Back

this._store.add(
    this.inputResultView.editor.onDidScrollChange(
        reentrancyBarrier.makeExclusive((c) => {
            if (c.scrollTopChanged) {
                synchronizeScrolling(this.inputResultView.editor, this.input1View.editor, input1ResultMapping.get(), 2);
                synchronizeScrolling(
                    this.inputResultView.editor,
                    this.input2View.editor,
                    input2ResultMapping.get(),
                    2
                );
            }
        })
    )
);

<=>

this._store.add(
    this.inputResultView.editor.onDidScrollChange(
        reentrancyBarrier.makeExclusive((c) => {
            if (!c.scrollTopChanged) {
                return;
            }
            
            synchronizeScrolling(this.inputResultView.editor, this.input1View.editor, input1ResultMapping.get(), 2);
            synchronizeScrolling(
                this.inputResultView.editor,
                this.input2View.editor,
                input2ResultMapping.get(),
                2
            );
        })
    )
);

convert-to-for-of-loop / convert-to-foreach-loop linting and refactoring not working.

Hi @lgrammel,
Linting for convert-to-for-of-loop / convert-to-foreach-loop is not showing. Also, no refactoring option is available for to convert simple for loop to for-of loop.

Inline occurence issue

Previous Code
const temp3 = []; temp3[0] = temp1; temp3[1] = temp2;

After refactoring
[][0] = temp1; [][1] = temp2;

Smart extend keyboard short cut conflicts with default Move Line Up/Move Line Down

I am not sure if you noticed, but by default, Move Line Up is bound to Alt + UpArrow and Move Line Down is bound to Alt + DownArrow.

Given how incredibly useful Move Line Up/Down are, I would expect that everyone would be complaining that p42 just installed a conflicting set of keyboard bindings.

I am removing the bindings that p42 added. I am sure there is a better set of keys to bind the smart expand to.

Inline Const Not Available

Convert to template string issue

Hey,
I noticed a error when refactoring this code:

fetch(
    process.env.API_URL +
      `/foo/bar?id=${id}&token=${token}`
  );

Result:

fetch(
    `${process.env.API_URL}/foo/bar?id=${id}&token=`token}`
  );

Best,
Tom

Refactoring Idea: To/From Ternary

let originalRange: LineRange;
if (c.originalEndLineNumber === 0) {
    // Insertion
    originalRange = new LineRange(c.originalStartLineNumber + 1, c.originalStartLineNumber + 1);
} else {
    originalRange = new LineRange(c.originalStartLineNumber, c.originalEndLineNumber + 1);
}

let originalRange: LineRange = c.originalEndLineNumber === 0
    // Insertion
    ? new LineRange(c.originalStartLineNumber + 1, c.originalStartLineNumber + 1)
    : new LineRange(c.originalStartLineNumber, c.originalEndLineNumber + 1);

Context:

export class SmartLinesDiffComputer implements ILinesDiffComputer {
	computeDiff(originalLines: string[], modifiedLines: string[], options: ILinesDiffComputerOptions): ILinesDiff {
		const diffComputer = new DiffComputer(originalLines, modifiedLines, {
			maxComputationTime: options.maxComputationTime,
			shouldIgnoreTrimWhitespace: options.ignoreTrimWhitespace,
			shouldComputeCharChanges: true,
			shouldMakePrettyDiff: true,
			shouldPostProcessCharChanges: true,
		});
		const result = diffComputer.computeDiff();
		return {
			quitEarly: result.quitEarly,
			changes: result.changes.map(c => {
				let originalRange: LineRange;
				if (c.originalEndLineNumber === 0) {
					// Insertion
					originalRange = new LineRange(c.originalStartLineNumber + 1, c.originalStartLineNumber + 1);
				} else {
					originalRange = new LineRange(c.originalStartLineNumber, c.originalEndLineNumber + 1);
				}

				let modifiedRange: LineRange;
				if (c.modifiedEndLineNumber === 0) {
					// Deletion
					modifiedRange = new LineRange(c.modifiedStartLineNumber + 1, c.modifiedStartLineNumber + 1);
				} else {
					modifiedRange = new LineRange(c.modifiedStartLineNumber, c.modifiedEndLineNumber + 1);
				}

				return {
					originalRange,
					modifiedRange,
					innerChanges: c.charChanges?.map(c => new RangeMapping(
						new Range(c.originalStartLineNumber, c.originalStartColumn, c.originalEndLineNumber, c.originalEndColumn),
						new Range(c.modifiedStartLineNumber, c.modifiedStartColumn, c.modifiedEndLineNumber, c.modifiedEndColumn),
					))
				};
			})
		};
	}
}

Move Up / Down Is Not Inverse

Refactor Idea: Convert React Class Component to/from Function Component

function EditorDemo() {
	return <div className="p-5 mb-4">
		<h2>Editor</h2>

		<div className="mt-4 row row-cols-4">
			<div className="col">
				<label className="control-label">Language</label>
				<select className="language-picker"></select>
			</div>
			<div className="col">
				<label className="control-label">Theme</label>
				<select className="theme-picker">
					<option>Visual Studio</option>
					<option>Visual Studio Dark</option>
					<option>High Contrast Dark</option>
				</select>
			</div>
		</div>

		<div className="mt-2" style={{ height: 500, border: "1px solid gray" }}>
			<MonacoEditor
				model={undefined}
			/>
		</div>
	</div>;
}

<->

class EditorDemo {
	render() {
        return (
            <div className="p-5 mb-4">
                <h2>Editor</h2>

                <div className="mt-4 row row-cols-4">
                    <div className="col">
                        <label className="control-label">Language</label>
                        <select className="language-picker"></select>
                    </div>
                    <div className="col">
                        <label className="control-label">Theme</label>
                        <select className="theme-picker">
                            <option>Visual Studio</option>
                            <option>Visual Studio Dark</option>
                            <option>High Contrast Dark</option>
                        </select>
                    </div>
                </div>

                <div className="mt-2" style={{ height: 500, border: "1px solid gray" }}>
                    <MonacoEditor
                        model={undefined}
                    />
                </div>
            </div>;
        );
    }
}

Issue with Push-Parameter_Into_IIFE

Hi @lgrammel ,

Please refer the below snaps for the issue.

Before refactoring

After refactoring

check for the index variable i in forEach loop after refactoring.

Feature: Exclude directories from mass refactorings

Hello

Is it possible to exclude some directories from mass refactorings? I only found a way to exclude files by adding the annotation but ignore entire folders does not seem to be possible?

Thanks

Convert To Ternary

let diff = 0;
if (last) {
    diff = last.outputRange.endLineNumberExclusive - last.inputRange.endLineNumberExclusive;
}

const diff = last ? last.outputRange.endLineNumberExclusive - last.inputRange.endLineNumberExclusive : 0;

Convert to map has issues with the ... operator

const combinedDiffs: { diff: RangeMapping; input: 1 | 2 }[] = [];

	for (const diffs of baseRange.input1Diffs) {
		combinedDiffs.push(...diffs.innerRangeMappings.map(diff => ({ diff, input: 1 as const })));
	}
	for (const diffs of baseRange.input2Diffs) {
		combinedDiffs.push(...diffs.innerRangeMappings.map(diff => ({ diff, input: 2 as const })));
	}

Convert to map ->

const combinedDiffs: { diff: RangeMapping; input: 1 | 2 }[] = baseRange.input1Diffs.map((diffs) => {
		return ...diffs.innerRangeMappings.map(diff => ({ diff, input: 1 as const }));
	});

	for (const diffs of baseRange.input2Diffs) {
		combinedDiffs.push(...diffs.innerRangeMappings.map(diff => ({ diff, input: 2 as const })));
	}

🐛

There is also flatMap!

Bug with return statement suggestions

I found an example in the Arquero codebase where P42 makes buggy suggestions marked as "safe". The simplified snippet below involves a series of if-statement fallthroughs that can update the ast variable. P42 (correctly) notes that the assignment in the if(obj.window) branch can be replaced with a return statement. But afterward it continues suggesting similar replacements for other assignments which actually break the logic.

function test(obj) {
  if (obj.expr) {
    let ast;
    if (obj.field === true) {
      ast = obj.field;
    } else if (obj.func === true) {
      ast = obj.func;
    }
    if (ast) {
      if (obj.desc) {
        ast = { type: Descending, expr: ast };
      }
      if (obj.window) {
        ast = { type: Window, expr: ast, ...obj.window };
      }
      return ast;
    }
  }
}

Refactor Idea: Move Up / Down Braces to Extend Scope

Move Brace:

if (!modifiedBaseRange || modifiedBaseRange.isConflicting) {
⟦}⟧

for (const diff of m.rights) {
    const range = diff.outputRange.toInclusiveRange();
    if (range) {
        result.push({
            range,
            options: {
                className: `merge-editor-diff result`,
                description: 'Merge Editor',
                isWholeLine: true,
            }
        });
    }

    if (diff.rangeMappings) {
        for (const d of diff.rangeMappings) {
            result.push({
                range: d.outputRange,
                options: {
                    className: `merge-editor-diff-word result`,
                    description: 'Merge Editor'
                }
            });
        }
    }
}

⟦}⟧ being selected.

--- Move down --->

if (!modifiedBaseRange || modifiedBaseRange.isConflicting) {

    for (const diff of m.rights) {
        const range = diff.outputRange.toInclusiveRange();
        if (range) {
            result.push({
                range,
                options: {
                    className: `merge-editor-diff result`,
                    description: 'Merge Editor',
                    isWholeLine: true,
                }
            });
        }

        if (diff.rangeMappings) {
            for (const d of diff.rangeMappings) {
                result.push({
                    range: d.outputRange,
                    options: {
                        className: `merge-editor-diff-word result`,
                        description: 'Merge Editor'
                    }
                });
            }
        }
    }
⟦}⟧

This should not be confused with move statement up/down:

if (!modifiedBaseRange || modifiedBaseRange.isConflicting) {
⟦⟧}

for (const diff of m.rights) {
    const range = diff.outputRange.toInclusiveRange();
    if (range) {
        result.push({
            range,
            options: {
                className: `merge-editor-diff result`,
                description: 'Merge Editor',
                isWholeLine: true,
            }
        });
    }

    if (diff.rangeMappings) {
        for (const d of diff.rangeMappings) {
            result.push({
                range: d.outputRange,
                options: {
                    className: `merge-editor-diff-word result`,
                    description: 'Merge Editor'
                }
            });
        }
    }
}

--- Move Down --->

for (const diff of m.rights) {
    const range = diff.outputRange.toInclusiveRange();
    if (range) {
        result.push({
            range,
            options: {
                className: `merge-editor-diff result`,
                description: 'Merge Editor',
                isWholeLine: true,
            }
        });
    }

    if (diff.rangeMappings) {
        for (const d of diff.rangeMappings) {
            result.push({
                range: d.outputRange,
                options: {
                    className: `merge-editor-diff-word result`,
                    description: 'Merge Editor'
                }
            });
        }
    }
}

if (!modifiedBaseRange || modifiedBaseRange.isConflicting) {
⟦⟧}

Optional chaining conversion issue

P42 tooltip (v1.101.1)

You can shorten 'this.control && this.control.invalid && this.control.touched' expression with optional chaining.p42 use-optional-chaining

  return this.control && this.control.invalid && this.control.touched;

P42 suggests incorrectly to convert into

return this.control?.invalid?.touched;

Do not suggest destruction on optional chaining.

const baz = window.foo?.bar.baz;

should not change to

const { baz } = window.foo?.bar;

Swapping if/else does not work

I thought this used to work:

But it does not swap the branches.

If this is a who-wins thing, I would definitely expect the if branch to move when the cursor is on the if or else.

Refactoring Idea: new Array<T> to :T[] = [];

const arr = new Array<T>();

<->

const arr: T[] = [];

The second one is faster (by 5-10%).

Support Moving Up / Down Multiple Items At Once

E.g. when all this is selected and I press control+down:

Code Action Idea: Parallelize Awaits

await writeFile(ancestorUri, 'some content');
await writeFile(input1Uri, 'some content');
await writeFile(input2Uri, 'some content');
await writeFile(resultUri, 'some content');

<->

Promise.all([
    writeFile(ancestorUri, 'some content'),
    writeFile(input1Uri, 'some content'),
    writeFile(input2Uri, 'some content'),
    writeFile(resultUri, 'some content'),
])

Ideally with detection if there are any dependencies

Bug report: Comment node is removed after "Extract to function in module scope"

Hi. When I refactor a callback in my JSX, which does something rather complicated, one of my comments are removed.

Code example:

//@ts-nocheck
/* eslint-disable */

export default function Something() {
  return (
    <SomeComponent
      aCallbackThatProducesAReactNode={() => {
        // this comment will be kept intact
        const someProps = {
          prop1: props.props1, // BUT THIS COMMENT WILL BE REMOVED :(

          prop2: props.props2 // this comment will be kept intact too
        };

        return <SomeOtherComponent {...someProps} />;
      }}
    />
  );
}

Try refactoring the whole callback into a function in module scope.

One:

Two:

Three:

As you see one of my comments are removed.

Thanks.

Too Agressive Add {...} to arrow function

Cursor jumps after refactoring

Consider diffing for more precise edits.

Move In

this.debouncer.run(() => {
    
});
f|or (const handler of this._previewHandlers) {
    handler.handlePreview(state);
}

this.debouncer.run(() => {
    f|or (const handler of this._previewHandlers) {
        handler.handlePreview(state);
    }
});

Refactor idea: Refactor function to module scope, but preserve the function itself (with/out arguments)

Hi. Sometimes I use P42 for refactoring callbacks, so the code would be cleaner, but I always have to make a manual refactor to make it more performant. I want my callbacks to be preserved (instead of me

Initial:

Result:

Expected:

I'm not too familiar with this whole parsing and automatic refactoring thing and I don't know the thing I expect is possible or not. And I'm aware with arguments it will be more and more complicated But I believe the simple cases can be detected and put as an option.

Refactor Idea: Replace usages with const declaration

Replace usages with const declaration, when the cursor is at the const declaration.

Support to convert if-else if to switch case.

Can we have an option to convert if else if to switch case?

I am using tslint but it only shows the linting but does not refactors it.

Code Action Idea: Fix Missing Argument From Context

class MyService {}

class Foo {
    constructor(
        private readonly myService: MyService
    ) {}

    blah() {
        bar('hello'); // Cursor is here
    }
}

function bar(baz: string, service: MyService) {
    // ...
}

Cursor is on the squiggles.

- Use missing argument from context ->

class MyService {}

class Foo {
    constructor(
        private readonly myService: MyService
    ) {}

    blah() {
        bar('hello', this.myService);
    }
}

function bar(baz: string, service: MyService) {
    // ...
}

- Use missing argument from parameter ->

class MyService {}

class Foo {
    constructor(
        private readonly myService: MyService
    ) {}

    blah(myService: MyServic) {
        bar('hello', myService);
    }
}

function bar(baz: string, service: MyService) {
    // ...
}

If you could record my code changes, you would see that parameter "wiring" is a task I do very often.

Code Action: Initialize field from constructor

This should generate a constructor if there is none and add a parameter.
Alternatively, what I would actually prefer, it uses constructor(public editorWorkerService: ...).

To Arrow Anchor Point

Wasn't the "Remove {...} from arrow function" action suggested here once?

Showing wrong hint for inline-return

Original code:

    public replaceVariables(path: Path, variables: { [key: string]: string }) {
        const me = this;
        let newPath;

        newPath = me.resolvePath(path);

        Object.keys(variables).forEach((variable: string) => {
            newPath.replace('{' + variable + '}', variables[variable]);
        });

        return newPath;
    }

code after the fix:

    public replaceVariables(path: Path, variables: { [key: string]: string }) {
        const me = this;
        let newPath;

        return me.resolvePath(path);

        //unreachable code after fix
        Object.keys(variables).forEach((variable: string) => {
            newPath.replace('{' + variable + '}', variables[variable]);
        });

    }

Refactoring Idea: Use variable for expressions

It would be awesome to have a refactoring that can replace expressions with a variable.

In this screenshot, I would like to replace all occurences of flip(sourceNmbr) with targetNmbr:

const targetNmbr = flip(sourceNmbr);

if (firstBefore && firstBefore.getInputRange(sourceNmbr).contains(topLineNumber)) {
    sourceRange = firstBefore.getInputRange(sourceNmbr);
    targetRange = firstBefore.getInputRange(flip(sourceNmbr));
} else if (firstBefore && firstBefore.getInputRange(sourceNmbr).isEmpty && firstBefore.getInputRange(sourceNmbr).startLineNumber === topLineNumber) {
    sourceRange = firstBefore.getInputRange(sourceNmbr).deltaEnd(1);
    targetRange = firstBefore.getInputRange(flip(sourceNmbr)).deltaEnd(1);
} else {
    const delta = firstBefore ? firstBefore.getInputRange(flip(sourceNmbr)).endLineNumberExclusive - firstBefore.getInputRange(sourceNmbr).endLineNumberExclusive : 0;
    sourceRange = new LineRange(topLineNumber, 1);
    targetRange = new LineRange(topLineNumber + delta, 1);
}

This would basically be the second step after extracting a constant.

Extension doesn't show any hints

Suggestions don't appear to be working in my vscode for some reason. I've put in some code that I know should trigger a refactor suggestion, but no suggestions appear on the code or the suggestions sidebar. I tried uninstalling and reinstalled and all of the suggestions settings are set to hint.

Does anyone have any suggestions on how I can fix this?

P42
v1.86.0

VScode
Version: 1.64.2 (user setup)
Commit: f80445acd5a3dadef24aa209168452a3d97cc326
Date: 2022-02-09T22:02:28.252Z
Electron: 13.5.2
Chromium: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Windows_NT x64 10.0.22000