Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linter: no-useless-spread fixer clobbers code before span in Vue script block #5913

Closed
axetroy opened this issue Sep 20, 2024 · 3 comments · Fixed by #6197
Closed

linter: no-useless-spread fixer clobbers code before span in Vue script block #5913

axetroy opened this issue Sep 20, 2024 · 3 comments · Fixed by #6197
Assignees
Labels
A-linter Area - Linter C-bug Category - Bug

Comments

@axetroy
Copy link

axetroy commented Sep 20, 2024

<template>
    <div>Hello world</div>
</template>

<script lang="js">
const isCondition = true

const bar = {
    ...(isCondition ? { a: 1 } : { b: 2 })
}

export default {
    data() {
        bar
    }
}
</script>
  ! �]8;;https://oxc.rs/docs/guide/usage/linter/rules/unicorn/no-useless-spread.html�\eslint-plugin-unicorn(no-useless-spread)�]8;;�\: Using a spread operator here creates a new object unnecessarily.
   ,-[src\renderer\App.vue:5:5]
 4 | const bar = {
 5 |     ...(isCondition ? { a: 1 } : { b: 2 })
   :     ^^^
 6 | }
   `----
  help: `isCondition ? { a: 1 } : { b: 2 }` returns a new object. Spreading it into an object expression to create a new object is redundant.

And here is the fix:

2024-09-20.15.53.53.mov
@axetroy axetroy added the C-bug Category - Bug label Sep 20, 2024
@oxc-project oxc-project deleted a comment from SAMBILI Sep 20, 2024
@DonIsaac
Copy link
Collaborator

@axetroy what happens when you run oxlint --fix?

@DonIsaac DonIsaac added the A-linter Area - Linter label Sep 24, 2024
@DonIsaac DonIsaac changed the title Wrong fix for Vue script block linter: no-useless-spread fixer clobbers code before span in Vue script block Sep 24, 2024
@shulaoda
Copy link
Contributor

@axetroy what happens when you run oxlint --fix?

I tried to reproduce it and found that the file was not fixed through oxlint --fix.

This seems to be because the --fix does not work on .vue files, which is normal for .js and .ts.

@Boshen
Copy link
Member

Boshen commented Sep 25, 2024

We need to ignore it in IDE as well, or actually fix the root cause.

The root cause is the fixer need to be offset by the <script> tag position.

@camchenry camchenry self-assigned this Sep 30, 2024
Boshen pushed a commit that referenced this issue Oct 3, 2024
- fixes #5913

This PR fixes the calculation of spans when dealing with files that have multiple sources and non-zero source start offsets. This is almost always the case for `.vue`, `.astro`, and `.svelte` files. This enables us to correctly apply fixes for these file types both in the CLI with `oxlint` and also in editors like VS Code.

https://github.com/user-attachments/assets/2836c8bd-09be-4e59-801d-7c95f8c2491f

I'm open to ideas on how to improve testing in this area, as I don't think that we currently have any tests for fixing files end-to-end (beyond what the linter rules check). I did run this locally on the gitlab repository (which is written in Vue) and all of the fixes appeared to be applied correctly.
Boshen pushed a commit that referenced this issue Oct 3, 2024
- fixes #5913

This PR fixes the calculation of spans when dealing with files that have multiple sources and non-zero source start offsets. This is almost always the case for `.vue`, `.astro`, and `.svelte` files. This enables us to correctly apply fixes for these file types both in the CLI with `oxlint` and also in editors like VS Code.

https://github.com/user-attachments/assets/2836c8bd-09be-4e59-801d-7c95f8c2491f

I'm open to ideas on how to improve testing in this area, as I don't think that we currently have any tests for fixing files end-to-end (beyond what the linter rules check). I did run this locally on the gitlab repository (which is written in Vue) and all of the fixes appeared to be applied correctly.
@Boshen Boshen closed this as completed Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter C-bug Category - Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants