feat(rome_js_analyze): add an `inlineVariable` code action
Summary
This PR introduces a simple inlineVariable code assist, available on all variable declarations that only have have read-only references. The rule inlines the initializer expression for that variable into all the places it's used and removes the original variable declaration node, using logic that's largely shared with the existing noShoutyConstants rule
Test Plan
I've added a test file for the rule verifying that "read-only variables" can be inlined while writable variables are not
Deploying with
Cloudflare Pages
| Latest commit: |
cc4202d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0bde8ee3.tools-8rn.pages.dev |
| Branch Preview URL: | https://feature-inline-variable.tools-8rn.pages.dev |
Should we limit this to only one read reference? Because if the "initializer" has side effects, we will be changing the code semantic.
Should we limit this to only one read reference? Because if the "initializer" has side effects, we will be changing the code semantic.
I think for refactors we can be less strict about semantics and side-effects compared to quick fixes: they never get applied automatically, and don't have the same implied "urgency" since they're not attached to an outstanding diagnostic. The code action only gets executed in response to a very clear action intent from the user (select the variable, click on "Refactor", click on "Inline Variable") so at this point we can trust that this is really what they want to do, and they know what they're doing
Should we limit this to only one read reference? Because if the "initializer" has side effects, we will be changing the code semantic.
I think for refactors we can be less strict about semantics and side-effects compared to quick fixes: they never get applied automatically, and don't have the same implied "urgency" since they're not attached to an outstanding diagnostic. The code action only gets executed in response to a very clear action intent from the user (select the variable, click on "Refactor", click on "Inline Variable") so at this point we can trust that this is really what they want to do, and they know what they're doing
While this is true, there's also the fact that we can't preview a refactor. It's my understanding that, as for today, these refactors are available only via LSP?
I share @xunilrj 's concerns here, and I think we should add more test cases, to make sure that we emit the correct code without breaking semantics.
While this is true, there's also the fact that we can't preview a refactor. It's my understanding that, as for today, these refactors are available only via LSP?
Yes refactor actions can only be used through the language server integration, since they're emitted contextually for the code the user has currently selected. How these actions are presented to the user depends on the actual editor being used, but VSCode has a "Refactor Preview" panel that shows the changes about to be applied for instance:

I share @xunilrj 's concerns here, and I think we should add more test cases, to make sure that we emit the correct code without breaking semantics.
I added a few more test cases (and ended up exposing a bug in the way lint rules are filtered in the code actions query)