New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ignore directives for no-fallthrough #16757
fix: ignore directives for no-fallthrough #16757
Conversation
|
Name | Link |
---|---|
8934a7b | |
https://app.netlify.com/sites/docs-eslint/deploys/63c2e6fd8d27f000088214a4 | |
https://deploy-preview-16757--docs-eslint.netlify.app/rules/no-fallthrough | |
To edit notification comments on pull requests, go to your Netlify site settings.
* @returns {boolean} `true` if the comment string is truly a fallthrough comment. | ||
*/ | ||
function isFallThroughComment(comment, fallthroughCommentPattern) { | ||
return fallthroughCommentPattern.test(comment) && !directivesPattern.test(comment.trim()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to use .trim()
because directivesPattern
is whitespace-sensitive.
However, comment
could easily be something like " eslint-disable-next-line no-fallthrough"
based on the way that the current fallthrough comment detection logic works, which would match DEFAULT_FALLTHROUGH_COMMENT
.
https://github.com/eslint/eslint/actions/runs/3861076597/jobs/6581823868#step:5:50 This failure looks flaky for reasons unrelated to this PR. Seeing same failure in another PR: https://github.com/eslint/eslint/actions/runs/3843668330/jobs/6546117108#step:5:50 |
de3d5aa
to
38d404a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I'll leave it open for others to review.
Fallthrough comment pattern detection was incorrectly matching to eslint directives. Fixes eslint#16650
38d404a
to
8934a7b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.31.0` -> `8.32.0`](https://renovatebot.com/diffs/npm/eslint/8.31.0/8.32.0) | --- ### Release Notes <details> <summary>eslint/eslint</summary> ### [`v8.32.0`](https://github.com/eslint/eslint/releases/tag/v8.32.0) [Compare Source](eslint/eslint@v8.31.0...v8.32.0) #### Features - [`fc20f24`](eslint/eslint@fc20f24) feat: add suggestions for redundant wrapping in prefer-regex-literals ([#​16658](eslint/eslint#16658)) (YeonJuan) #### Bug Fixes - [`b4f8329`](eslint/eslint@b4f8329) fix: ignore directives for no-fallthrough ([#​16757](eslint/eslint#16757)) (gfyoung) #### Documentation - [`17b65ad`](eslint/eslint@17b65ad) docs: IA Update page URL move ([#​16665](eslint/eslint#16665)) (Ben Perlmutter) - [`5981296`](eslint/eslint@5981296) docs: fix theme switcher button ([#​16752](eslint/eslint#16752)) (Sam Chen) - [`6669413`](eslint/eslint@6669413) docs: deploy prerelease docs under the `/docs/next/` path ([#​16541](eslint/eslint#16541)) (Nitin Kumar) - [`78ecfe0`](eslint/eslint@78ecfe0) docs: use inline code for rule options name ([#​16768](eslint/eslint#16768)) (Percy Ma) - [`fc2ea59`](eslint/eslint@fc2ea59) docs: Update README (GitHub Actions Bot) - [`762a872`](eslint/eslint@762a872) docs: Update README (GitHub Actions Bot) #### Chores - [`2952d6e`](eslint/eslint@2952d6e) chore: sync templates/\*.md files with issue templates ([#​16758](eslint/eslint#16758)) (gfyoung) - [`3e34418`](eslint/eslint@3e34418) chore: Add new issues to triage project ([#​16740](eslint/eslint#16740)) (Nicholas C. Zakas) </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. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMDIuNCIsInVwZGF0ZWRJblZlciI6IjM0LjEwMi43In0=--> Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1734 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>
* docs: add options to check destructuring in no-underscore-dangle eslint/eslint#16006 * docs: check assignment patterns in no-underscore-dangle eslint/eslint#16693 * docs: use inline code for rule options name eslint/eslint#16768 * docs: ignore directives for no-fallthrough eslint/eslint#16757 * docs: IA Update page URL move eslint/eslint#16665 * update * feat: sync v8.31.0 (#94) * docs: User Guide Getting Started expansion * docs: add options to check destructuring in no-underscore-dangle eslint/eslint#16006 * docs: adjust some words * docs: Add function call example for no-undefined eslint/eslint#16712 * docs: check assignment patterns in no-underscore-dangle eslint/eslint#16693 * update formatters * Apply suggestions from code review Co-authored-by: Strek <ssharishkumar@gmail.com> --------- Co-authored-by: Strek <ssharishkumar@gmail.com> * docs: add options to check destructuring in no-underscore-dangle eslint/eslint#16006 * docs: IA Update page URL move eslint/eslint#16665 --------- Co-authored-by: Strek <ssharishkumar@gmail.com>
Prerequisites checklist
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 #16650
What changes did you make? (Give an overview)
Fallthrough comment pattern detection was incorrectly matching to
ESLint
directives.Abstracts logic from main
linter.js
file to detect directives. That detection now takes precedence over fallthrough comment pattern detection, regardless of whether the default pattern or user-provided pattern is used.Is there anything you'd like reviewers to focus on?
N/A