nicoespeon / abracadabra

Automated refactorings for VS Code (JS & TS) ✨ It's magic ✨
https://marketplace.visualstudio.com/items?itemName=nicoespeon.abracadabra
MIT License
809 stars 48 forks source link

Refactorings don't work on a specific .ts file #1058

Closed montoner0 closed 2 months ago

montoner0 commented 3 months ago

Describe the bug

When I try to call Abracadabra's refactoring on a specific file, I'm getting the error I'm sorry, something went wrong: I can't build the AST from the source code. This may be due to a syntax error that you can fix. Here's what went wrong: Unexpected token. (27:4)

Although the code is perfectly well. The part "what went wrong" specifies some arbitrary error and changes when I change the code.

How to reproduce

Try to call refactoring on line 28 (e. g. convert if/else to ternary) in the attached .ts file. It won't work. If press Ctrl-. then no such refactoring option available. If invoke the routine through command palette then the mentioned error appears.

Expected behavior

The refactoring works.

Additional information

foo-bar.component.ts.zip

nicoespeon commented 3 months ago

Thanks for reporting @montoner0. Indeed, it seems to struggle to parse this code specifically: <FooType>'bar'

I'll dig into that on the parser. This may just be some options to enable 🤔

It looks like some JSX. Is it a syntax specific to some framework like Angular, Vue, or Svelte? If you have pointers to some online docs about it, that may help. Thanks!

j4k0xb commented 3 months ago

its a type assertion, same thing as 'bar' as FooType only valid in ts, not tsx/jsx files

nicoespeon commented 3 months ago

Thanks! I wasn't familiar with this syntax.

Something is definitely off with the @babel/parser. I can't get it to parse that syntax, even with most options enabled: dig

I'll dig into that.

EDIT: actually, if I turn off jsx it works. I think the fix is to not use this option for a regular JS/TS file to avoid the confusion. I'll come up with a fix—need some refactoring because the options are currently a constant, not dynamic yet.

nicoespeon commented 2 months ago

Got it to work:

https://github.com/user-attachments/assets/7a556d08-93df-403d-a785-2e4705482e71

The implementation is not pretty (I'm basically naively retrying to parse if it fails), but that was the shortest path to make it work in this scenario. A proper implementation would require extensive refactoring so the parse() method gets passed the language as a parameter.

However, it's covered with a test so at least it works 😄

I'll ship it with a new 9.4.6 release.

nicoespeon commented 2 months ago

@all-contributors please add montoner0 for bugs

allcontributors[bot] commented 2 months ago

@nicoespeon

I've put up a pull request to add @montoner0! :tada: