ataylorme / eslint-annotate-action

A GitHub action that takes ESLint results from a JSON file and adds them as annotated pull request comments
MIT License
98 stars 33 forks source link

Ignore messages without a rule ID #41

Closed pmcelhaney closed 2 years ago

pmcelhaney commented 2 years ago

fixes #40

ataylorme commented 2 years ago

@pmcelhaney I am making updates in a v2 refactor #44. I added this update there. Can you test ataylorme/eslint-annotate-action@v2

Also if you provide an ESLint JSON file with an example of a missing ruleId I will add a unit test for it.

pmcelhaney commented 2 years ago

Here's an example of a message that does not have a rule ID.

[
  {
    "filePath": "/path/to/ignored-me.js",
    "messages": [
      {
        "fatal": false,
        "severity": 1,
        "message": "File ignored because of a matching ignore pattern. Use \"--no-ignore\" to override."
      }
    ],
    "suppressedMessages": [],
    "errorCount": 0,
    "fatalErrorCount": 0,
    "warningCount": 1,
    "fixableErrorCount": 0,
    "fixableWarningCount": 0,
    "usedDeprecatedRules": [
      { "ruleId": "no-buffer-constructor", "replacedBy": [] }
    ]
  }
]

To recreate:

  1. Create a file called ignore-me.js
  2. Add ignore-me.js to your .eslintignore
  3. Run eslint ignore-me.js --format json

I would test your branch, but I don't have any repos that are using the action right now. I ended up creating my own custom formatter and running ESLint directly as part of a larger workflow.

ataylorme commented 2 years ago

Thank you for following up @pmcelhaney. I'm going to close this PR since v2 of the Action incorporates this change.

I will say though that I think there may be some exploring to do with how you are triggering ESLInt to generate the report in the first place.

Ignored files shouldn't be making it into the report in general with eslint .. However, if you specify a path to lint that includes ignored files they will be added to the report. eslint **/*.js as an example. See this comment

pmcelhaney commented 2 years ago

I’m using the changed files action to get a list of changed .js and .ts files and pass the list to ESLint.

ataylorme commented 2 years ago

@pmcelhaney I think that is an issue as you are explicitly passing files to the longer that are normally ignored.

The v2 version of this Action has an option to only check files changes in the PR. Please test that instead