jose-elias-alvarez / nvim-lsp-ts-utils

Utilities to improve the TypeScript development experience for Neovim's built-in LSP client.
The Unlicense
438 stars 18 forks source link

Eslint Code Action error: Expected lua number #38

Closed mraxime closed 3 years ago

mraxime commented 3 years ago

👋

The issue:

When I'm trying to use the autofix: image I get this error: image and nothing happen.

To reproduce:

I'm using the autofix codeAction using eslint plugin simple-import-sort with these configs:

null-ls:

require("null-ls").setup {}

ts-utils:

ts_utils.setup {
    debug = false,
    disable_commands = false,
    enable_import_on_completion = true,

    -- eslint
    eslint_enable_code_actions = true,
    eslint_enable_disable_comments = true,
    eslint_bin = "eslint",
    eslint_config_fallback = nil,
    eslint_enable_diagnostics = true,

    -- formatting
    enable_formatting = true,
    formatter = "prettier",
    formatter_config_fallback = nil,

    -- parentheses completion
    complete_parens = true,
    signature_help_in_parens = true,

    -- update imports on file move
    update_imports_on_move = false,
    require_confirmation_on_move = false,
    watch_dir = nil,
}

eslintrc:

{
    "plugins": ["simple-import-sort"],
    "rules": {
        "simple-import-sort/exports": "error",
        "simple-import-sort/imports": "error"
    }
}
jose-elias-alvarez commented 3 years ago

Thanks for reporting this! The plugin wasn't handling multiline fixes correctly. I pushed a fix which should improve handling, so could you try updating the plugin and see if that fixes it on your end?

mraxime commented 3 years ago

Thanks for reporting this! The plugin wasn't handling multiline fixes correctly. I pushed a fix which should improve handling, so could you try updating the plugin and see if that fixes it on your end?

No more issues regarding multiline fixes on my end! Nice job! :rocket:

Except no idea if it's related to this, but yesterday I tried other eslint plugins and came across the same error with some eslint-plugin-unicorn autofixes.

For Example - On Their catch-error-name Rule:

Expected:

https://user-images.githubusercontent.com/51096534/122386598-0abbcf80-cf3c-11eb-844a-f09c2fd29d88.mp4

Current behavior:

https://user-images.githubusercontent.com/51096534/122387121-89187180-cf3c-11eb-86a7-8266abfe0c6c.mp4

And sometimes it "works", but makes weird automatic corrections:

https://user-images.githubusercontent.com/51096534/122387329-c2e97800-cf3c-11eb-90f2-e39a1ebd2b94.mp4

Updated eslintrc:

{
    "extends": ["plugin:unicorn/recommended"],
    "plugins": ["simple-import-sort"],
    "rules": {
        "simple-import-sort/exports": "error",
        "simple-import-sort/imports": "error"
    }
}
jose-elias-alvarez commented 3 years ago

Judging from the examples, it's almost definitely the same issue where the structure of the fix doesn't match what the plugin expects. I'll have a chance to take a look at this new set of issues early next week and hopefully get them fixed up.

jose-elias-alvarez commented 3 years ago

I had time to sit down and take a look at this today, but I can't get that specific Unicorn rule to trigger an error. I'm using this code block, based on the example in the documentation:

try {
  console.log("didn't throw");
} catch (badName) {
  console.log(badName);
}

Applying the suggested fix works as expected and results in the following:

try {
  console.log("didn't throw");
} catch (error) {
  console.log(error);
}

Does this example work on your end? If so, could you try to provide a code sample that produces the Lua error and / or fails to format correctly?

mraxime commented 3 years ago

@jose-elias-alvarez Thank you for this - sorry for the delay, I was a bit busy last week. I looked into this problem with my own code and after a long time of investigation, I finally found (I think) what causes this weird bug. It only happens when I just put a special character like é or è somewhere before in my my code 🤔

https://user-images.githubusercontent.com/51096534/124375473-5d9dc280-dc70-11eb-845d-4feccc20b66a.mp4

Edit: I just found out that this also causes other autofix code actions to do similar things, as long as I have that kind of character right before the lint error. For example, if I put a comment like // é right before my imports, it gives me the same "Expected lua number" error

jose-elias-alvarez commented 3 years ago

Thanks to your information, I see that the problem is caused by multibyte characters. The current mechanism uses byte offsets, but ESLint fixes actually use character offsets, and the two go out of sync when multibyte characters are used in the document.

I hoped this would let us implement a more elegant solution, but as it turns out, converting a character offset to a cursor position is way harder than it looks. I'll continue to investigate, but in the meantime, 14eaa814c25005ec2225631d17fec7162b96abab seems to take care of the issue on my end (tested with Latin characters with accents and Japanese characters, both of which didn't work previously). Could you let me know if that works for you?

mraxime commented 3 years ago

Ok I see. Everything works great on my end with the fix! Closing this issue for now. Thanks again for eveything