fix: ignore messages without a `ruleId` in `getRulesMetaForResults` #16409

fasttime · 2022-10-11T19:10:31Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[X] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

Fixes #16402

What changes did you make? (Give an overview)

FlatESLint.prototype.getRulesMetaForResults will now ignore messages without a ruleId instead of crashing with a TypeError.
Unit tests for getRulesMetaForResults added for both FlatESLint and ESLint.
The unit test "should return one rule meta when there is a suppressed linting error" was ported to FlatESLint. This was originally introduced for ESLint in feat: implement rfc 2021-suppression-support #15459.

Is there anything you'd like reviewers to focus on?

* `FlatESLint.prototype.getRulesMetaForResults` will now ignore messages without a `ruleId` instead of crashing with a `TypeError`. * Unit tests for `getRulesMetaForResults` added for both `FlatESLint` and `ESLint`. * The unit test "should return one rule meta when there is a suppressed linting error" was ported to `FlatESLint`. This was originally introduced for `ESLint` in #15459. Fixes #16402

netlify · 2022-10-11T19:10:39Z

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	`1d6060c`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/634c68dc2094c1000857115d

tests/lib/eslint/flat-eslint.js

nzakas · 2022-10-13T19:52:02Z

tests/lib/eslint/flat-eslint.js

+            const results = await engine.lintText("a // eslint-disable-line semi");
+            const rulesMeta = engine.getRulesMetaForResults(results);
+
+            assert.strictEqual(rulesMeta.semi, coreRules.get("semi").meta);


Would it also make sense to ensure that there is only one key in rulesMeta? (Based on the name of this test.)

I've added an assertion to make sure that there is only one key to the two unit tests that mention "one rule meta" in the title. Thanks @nzakas!

mdjermanovic · 2022-10-15T20:11:54Z

tests/lib/eslint/flat-eslint.js

+                assert.deepStrictEqual(rulesMeta, {});
+            }
+            {
+                const results = await engine.lintText("", { filePath: "/.ignored.js", warnIgnored: true });


Can we use some explicitly ignored file path instead of /.ignored.js? /.ignored.js will indeed most likely be treated as an ignored file due to being outside of cwd, but that still depends on how the environment where the tests are run is set up. It may also be ignored because it's a dotfile, but it is not yet certain how dotfiles will be treated. For the purposes of this test, I think it would be better to avoid these assumptions. We can, for example, add ignorePatterns: ["ignored.js"] to new FlatESLint({ ... }) and use filePath: "ignored.js".

(the same for the eslint.js test)

Updated as suggested, thanks!

…lts`

mdjermanovic

LGTM, thanks!

Just waiting for @nzakas, and we should also fix CI (the errors are not related to the changes from this PR).

mdjermanovic · 2022-10-18T11:59:18Z

CI has been fixed and all checks on this PR are now passing.

nzakas

LGTM. Thanks!

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.25.0` -> `8.26.0`](https://renovatebot.com/diffs/npm/eslint/8.25.0/8.26.0) | --- ### Release Notes <details> <summary>eslint/eslint</summary> ### [`v8.26.0`](https://github.com/eslint/eslint/releases/tag/v8.26.0) [Compare Source](eslint/eslint@v8.25.0...v8.26.0) #### Features - [`4715787`](eslint/eslint@4715787) feat: check `Object.create()` in getter-return ([#16420](eslint/eslint#16420)) (Yuki Hirasawa) - [`28d1902`](eslint/eslint@28d1902) feat: `no-implicit-globals` supports `exported` block comment ([#16343](eslint/eslint#16343)) (Sosuke Suzuki) - [`e940be7`](eslint/eslint@e940be7) feat: Use ESLINT_USE_FLAT_CONFIG environment variable for flat config ([#16356](eslint/eslint#16356)) (Tomer Aberbach) - [`dd0c58f`](eslint/eslint@dd0c58f) feat: Swap out Globby for custom globbing solution. ([#16369](eslint/eslint#16369)) (Nicholas C. Zakas) #### Bug Fixes - [`df77409`](eslint/eslint@df77409) fix: use `baseConfig` constructor option in FlatESLint ([#16432](eslint/eslint#16432)) (Milos Djermanovic) - [`33668ee`](eslint/eslint@33668ee) fix: Ensure that glob patterns are matched correctly. ([#16449](eslint/eslint#16449)) (Nicholas C. Zakas) - [`740b208`](eslint/eslint@740b208) fix: ignore messages without a `ruleId` in `getRulesMetaForResults` ([#16409](eslint/eslint#16409)) (Francesco Trotta) - [`8f9759e`](eslint/eslint@8f9759e) fix: `--ignore-pattern` in flat config mode should be relative to `cwd` ([#16425](eslint/eslint#16425)) (Milos Djermanovic) - [`325ad37`](eslint/eslint@325ad37) fix: make `getRulesMetaForResults` return a plain object in trivial case ([#16438](eslint/eslint#16438)) (Francesco Trotta) - [`a2810bc`](eslint/eslint@a2810bc) fix: Ensure that directories can be unignored. ([#16436](eslint/eslint#16436)) (Nicholas C. Zakas) - [`35916ad`](eslint/eslint@35916ad) fix: Ensure unignore and reignore work correctly in flat config. ([#16422](eslint/eslint#16422)) (Nicholas C. Zakas) #### Documentation - [`651649b`](eslint/eslint@651649b) docs: Core concepts page ([#16399](eslint/eslint#16399)) (Ben Perlmutter) - [`631cf72`](eslint/eslint@631cf72) docs: note --ignore-path not supported with flat config ([#16434](eslint/eslint#16434)) (Andy Edwards) - [`1692840`](eslint/eslint@1692840) docs: fix syntax in examples for new config files ([#16427](eslint/eslint#16427)) (Milos Djermanovic) - [`d336cfc`](eslint/eslint@d336cfc) docs: Document extending plugin with new config ([#16394](eslint/eslint#16394)) (Ben Perlmutter) #### Chores - [`e917a9a`](eslint/eslint@e917a9a) ci: add node v19 ([#16443](eslint/eslint#16443)) (Koichi ITO) - [`4b70b91`](eslint/eslint@4b70b91) chore: Add VS Code issues link ([#16423](eslint/eslint#16423)) (Nicholas C. Zakas) - [`232d291`](eslint/eslint@232d291) chore: suppress a Node.js deprecation warning ([#16398](eslint/eslint#16398)) (Koichi ITO) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - 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 this update again. --- - [ ] 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/1599 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

eslint-github-bot bot added triage An ESLint team member will look at this issue soon bug ESLint is working incorrectly labels Oct 11, 2022

fasttime marked this pull request as ready for review October 11, 2022 19:18

mdjermanovic added core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Oct 11, 2022

mdjermanovic reviewed Oct 11, 2022

View reviewed changes

tests/lib/eslint/flat-eslint.js Outdated Show resolved Hide resolved

nzakas reviewed Oct 13, 2022

View reviewed changes

fasttime mentioned this pull request Oct 14, 2022

Bug: FlatESLint getRulesMetaForResults fails on anonymous files when option cwd is set #16410

Closed

1 task

Update unit tests as suggested

61768c8

mdjermanovic reviewed Oct 15, 2022

View reviewed changes

Explicitly ignore a file path in a unit test for `getRulesMetaForResu…

1d6060c

…lts`

mdjermanovic approved these changes Oct 17, 2022

View reviewed changes

nzakas approved these changes Oct 19, 2022

View reviewed changes

nzakas merged commit 740b208 into eslint:main Oct 19, 2022

nzakas added the contributor pool label Oct 19, 2022

eslint-github-bot bot locked and limited conversation to collaborators Apr 18, 2023

eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Apr 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: ignore messages without a `ruleId` in `getRulesMetaForResults` #16409

fix: ignore messages without a `ruleId` in `getRulesMetaForResults` #16409

fasttime commented Oct 11, 2022

netlify bot commented Oct 11, 2022 •

edited

Loading

nzakas Oct 13, 2022

fasttime Oct 14, 2022

mdjermanovic Oct 15, 2022

fasttime Oct 16, 2022

mdjermanovic left a comment

mdjermanovic commented Oct 18, 2022

nzakas left a comment

fix: ignore messages without a ruleId in getRulesMetaForResults #16409

fix: ignore messages without a ruleId in getRulesMetaForResults #16409

Conversation

fasttime commented Oct 11, 2022

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

netlify bot commented Oct 11, 2022 • edited Loading

✅ Deploy Preview for docs-eslint canceled.

nzakas Oct 13, 2022

Choose a reason for hiding this comment

fasttime Oct 14, 2022

Choose a reason for hiding this comment

mdjermanovic Oct 15, 2022

Choose a reason for hiding this comment

fasttime Oct 16, 2022

Choose a reason for hiding this comment

mdjermanovic left a comment

Choose a reason for hiding this comment

mdjermanovic commented Oct 18, 2022

nzakas left a comment

Choose a reason for hiding this comment

fix: ignore messages without a `ruleId` in `getRulesMetaForResults` #16409

fix: ignore messages without a `ruleId` in `getRulesMetaForResults` #16409

netlify bot commented Oct 11, 2022 •

edited

Loading