fix: ignore directives for no-fallthrough#16757
Merged
Merged
Conversation
eslint-github-bot
Bot
added
the
triage
✅ Deploy Preview for docs-eslint ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
gfyoung
commented
Jan 7, 2023
| * @returns {boolean} `true` if the comment string is truly a fallthrough comment. | ||
| */ | ||
| function isFallThroughComment(comment, fallthroughCommentPattern) { | ||
| return fallthroughCommentPattern.test(comment) && !directivesPattern.test(comment.trim()); |
Contributor
Author
There was a problem hiding this comment.
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.
Contributor
Author
|
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 |
snitin315
reviewed
Jan 8, 2023
snitin315
added
rule
gfyoung
force-pushed
the
no-fallthrough-directive-fix
branch
2 times, most recently
from
de3d5aa to
38d404a
Compare
snitin315
approved these changes
Jan 12, 2023
snitin315
left a comment
snitin315
left a comment
Contributor
There was a problem hiding this comment.
LGTM, I'll leave it open for others to review.
Fallthrough comment pattern detection was incorrectly matching to eslint directives. Fixes eslint#16650
gfyoung
force-pushed
the
no-fallthrough-directive-fix
branch
from
38d404a to
8934a7b
Compare
crapStone
pushed a commit
to Calciumdibromid/CaBr2
that referenced
this pull request
Jan 17, 2023
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>
kecrily
added a commit
to eslint/zh-hans.docs.eslint.org
that referenced
this pull request
Feb 20, 2023
kecrily
added a commit
to eslint/zh-hans.docs.eslint.org
that referenced
this pull request
Feb 28, 2023
kecrily
added a commit
to eslint/zh-hans.docs.eslint.org
that referenced
this pull request
Feb 28, 2023
* 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>
eslint-github-bot
Bot
added
the
archived due to age
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
ESLintdirectives.Abstracts logic from main
linter.jsfile 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