-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[@typescript-eslint/no-unnecessary-type-assertion] Fixing Vue single file components applies changes to incorrect character location #4723
Comments
Thanks for posting the issue @jaa134! None of us on the maintenance team here are particularly common Vue users, so just seeing the source file, |
@JoshuaKGoldberg Sure thing, I can set up a repo for you all. I do want to clarify that building the project isn't necessary. You should just need the ESLint config, a |
@bradzacher @JoshuaKGoldberg Here is the repo! Sorry for the delay. Let me know if there is anything else I can help out with. https://github.com/jaa134/ESLint-Vue-Test |
@JoshuaKGoldberg @jaa134 actual reason for this issue is related to what location data are we using when we are dealing with fixers, in case of this rule data used comes from typescript instance and we should always use node range, source of typescript-eslint/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts Lines 174 to 175 in de68e5e
this issue is going to "appear" in every place when we try to use location data from typescript, and its not related to vue, same issue can be reproduced with any other parser eg, mdx |
@armano2 Thank you very much for the quick turn-around time! Do you know of any other rules that might be affected by the same problem? Obviously, I would prefer to turn these off. Searching the code base like |
@JoshuaKGoldberg @armano2 Thank you to everyone involved! Impressed by the quick turnaround time on this. Y'all rock 🔥 |
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.18.0` -> `5.19.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.18.0/5.19.0) | | [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.18.0` -> `5.19.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.18.0/5.19.0) | --- ### Release Notes <details> <summary>typescript-eslint/typescript-eslint (@​typescript-eslint/eslint-plugin)</summary> ### [`v5.19.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#​5190-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5180v5190-2022-04-11) [Compare Source](typescript-eslint/typescript-eslint@v5.18.0...v5.19.0) ##### Bug Fixes - **eslint-plugin:** update code to use estree range instead of ts pos/end [#​4723](typescript-eslint/typescript-eslint#4723) ([#​4790](typescript-eslint/typescript-eslint#4790)) ([a1e9fc4](typescript-eslint/typescript-eslint@a1e9fc4)) ##### Features - **eslint-plugin:** \[unified-signatures] add `ignoreDifferentlyNamedParameters` option ([#​4659](typescript-eslint/typescript-eslint#4659)) ([fdf95e0](typescript-eslint/typescript-eslint@fdf95e0)) - **eslint-plugin:** add support for valid number and bigint intersections in restrict-plus-operands rule ([#​4795](typescript-eslint/typescript-eslint#4795)) ([19c600a](typescript-eslint/typescript-eslint@19c600a)) </details> <details> <summary>typescript-eslint/typescript-eslint (@​typescript-eslint/parser)</summary> ### [`v5.19.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#​5190-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5180v5190-2022-04-11) [Compare Source](typescript-eslint/typescript-eslint@v5.18.0...v5.19.0) **Note:** Version bump only for package [@​typescript-eslint/parser](https://github.com/typescript-eslint/parser) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1299 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Applying the automatic fix for non-null assertions removes a character from the wrong place. It seems related to #2591 and #2680 where
vue-eslint-parser
alters the locations information for every node it produces within a script tag, but it doesn't know about the typescript nodes, so their range information is wrong. The fix is being applied as if there was no script tag. You can see that by running the lint command without the--fix
flag, the eslint command output reports the error at an incorrect line or character index.Repro
Steps:
--fix
flag and with the rule enabled. Seepackage.json
/.eslintrc
examples below. I am using latest versions ofvue-eslint-plugin
, this TS parser, this TS plugin, and Eslint.const
and not the unnecessary non-null assertion.Expected Result
See that the non-null assertion was removed.
Actual Result
See that the letter 't' was removed from
const
.Additional Info
It looks like a similar issue was identified in #2591. And a fix was applied in #2680.
Versions
@typescript-eslint/eslint-plugin
5.16.0
@typescript-eslint/parser
5.16.0
TypeScript
4.4.3
ESLint
8.11.0
node
17.3.1
The text was updated successfully, but these errors were encountered: