search

BenoitZugmeyer / eslint-plugin-html

An ESLint plugin to extract and lint scripts from HTML files.
ISC License
430 stars 51 forks source link

Provided ESLint Auto fix information have incorrect positions #38

Closed dbaeumer closed 7 years ago

dbaeumer commented 7 years ago

.eslintrc.json

{
    "plugins": [
        "html"
    ],
    "env": {
        "browser": true,
        "commonjs": true,
        "es6": true,
        "node": true
    },
    "parserOptions": {
        "ecmaFeatures": {
            "jsx": true
        },
        "sourceType": "module"
    },
    "rules": {
        "no-const-assign": "warn",
        "no-this-before-super": "warn",
        "no-undef": "warn",
        "no-unreachable": "warn",
        "no-unused-vars": "warn",
        "constructor-super": "warn",
        "valid-typeof": "warn",
        "no-extra-semi": "warn"
    }
}

test.html

<!DOCTYPE html>
<html lang="en">
    <script>
        function foo() {
        }
        foo();;
    </script>
</html>

This correct reports the following error: 'Unnecessary semicolon. (no-extra-semi)'

However the auto fix information attached to the reported problem when using ESLints node API has the following information: { range: [75, 76] text: "" }

which will delete the second 'o' of foo();;

The content of the html file passed to ESLint as a string looks like:

capture

I guess that the html plugin does some offset magic and might not apply it to the fixes attached to a problem.

BenoitZugmeyer commented 7 years ago

Thank you for bringing this up. The plugin automatically dedent the JS code, so rules like indent are working nicely. To report correct columns, messages columns are adjusted. But I never had the need to use the fix.range property of messages, so I didn't bother remaping those offsets.

This is quite complicated to fix though: I can easily adjust columns but adjusting offsets would require an in-depth rewrite of the plugin. But you are in luck: I have a work in progress to do just that. It's still a bit rough though, so not ready for prime time, but I'll try to commit something this week-end.

BenoitZugmeyer commented 7 years ago

So, as promised, I commited a bunch of stuff I made locally some time ago, few steps toward the v2 of this plugin. Could you check the v2 branch and tell me if it fits your needs?

dbaeumer commented 7 years ago

@BenoitZugmeyer thanks. Is there a npm module available for the v2 branch. Makes it easier for me to test.

I also released 1.2.2 version of the ESLint extension for VS Code. In that version you can selectively enable autoFix support for html which you could use as a test bed. Simple configure eslint to use the html plugin and then add the following to the workspace setting in VS Code.

"eslint.validate": [ "javascript", "javascriptreact", { "language": "html", "autoFix": true } ]

This will validate JS in HTML and will offer code action (the light bulb). You can click the light bulb an execute fixes.

BenoitZugmeyer commented 7 years ago

Thank you, I'll try to reproduce this with VS Code.

In the meantime, you can install the v2 branch of this plugin via npm install 'BenoitZugmeyer/eslint-plugin-html#v2' (see https://docs.npmjs.com/cli/install)

dbaeumer commented 7 years ago

Thanks. I tested it with the latest ESLint 1.2.2 plugin and it looks good to me:

cast

dbaeumer commented 7 years ago

One additional question: could you be affected by https://github.com/Microsoft/vscode-eslint/issues/185#issuecomment-267744550 as well ?

BenoitZugmeyer commented 7 years ago

I don't think I'm affected by this, I don't use parse5 and I have tests on \r\n support.

Closing this issue since it's resolved in v2.