pinterest / arcanist-linters

A collection of custom Arcanist linters
Apache License 2.0
62 stars 45 forks source link

Support autofix with eslint #79

Closed RainNapper closed 3 years ago

RainNapper commented 3 years ago

Pasted from https://github.com/pinterest/arcanist-linters/issues/80

When there are autofixable lint errors detected by eslint, the order of operations is: 1) arc diff 2) if there are uncommited changes, amend or commit 3) arc lint, which autofixes and returns successful 4) autofixed changes are not committed nor staged 5) diff is pushed to phabricator without changes

This means that arc diff fails to properly apply lint changes before creating a diff.

See messages for a file provided by eslint@6.8.0

        "messages": [
            {
                "ruleId": "prettier/prettier",
                "severity": 2,
                "message": "Replace `(flow.component·&&·flow.component.archived)` with `flow.component·&&·flow.component.archived`",
                "line": 61,
                "column": 10,
                "nodeType": null,
                "messageId": "replace",
                "endLine": 61,
                "endColumn": 53,
                "fix": {
                    "range": [
                        1462,
                        1505
                    ],
                    "text": "flow.component && flow.component.archived"
                }
            },
            {
                "ruleId": "prettier/prettier",
                "severity": 2,
                "message": "Insert `(⏎··········`",
                "line": 65,
                "column": 13,
                "nodeType": null,
                "messageId": "insert",
                "endLine": 65,
                "endColumn": 13,
                "fix": {
                    "range": [
                        1675,
                        1675
                    ],
                    "text": "(\n          "
                }
            },
            {
                "ruleId": "prettier/prettier",
                "severity": 2,
                "message": "Replace `⏎⏎⏎········` with `········)`",
                "line": 66,
                "column": 1,
                "nodeType": null,
                "messageId": "replace",
                "endLine": 69,
                "endColumn": 9,
                "fix": {
                    "range": [
                        1685,
                        1696
                    ],
                    "text": "        )"
                }
            }
        ],

Summary of changes: 1) Parse the fix fields in each message, and build ArcanistLintMessage with originalText and replacementText populated, along with a severity of ArcanistLintSeverity::SEVERITY_AUTOFIX. This means that the user will receive a prompt asking if they'd like to apply the fixes. 2) Remove the eslint.fix flag because it likely causes more confusion that it helps. 3) Use the existing eslint.fix flag to toggle this logic for backwards compatibility and as a way of gating these changes.