diff --git a/docs/src/rules/no-fallthrough.md b/docs/src/rules/no-fallthrough.md index be07ffbe70d..c49293cad4a 100644 --- a/docs/src/rules/no-fallthrough.md +++ b/docs/src/rules/no-fallthrough.md @@ -32,7 +32,7 @@ switch(foo) { } ``` -That works fine when you don't want a fallthrough, but what if the fallthrough is intentional, there is no way to indicate that in the language. It's considered a best practice to always indicate when a fallthrough is intentional using a comment which matches the `/falls?\s?through/i` regular expression: +That works fine when you don't want a fallthrough, but what if the fallthrough is intentional, there is no way to indicate that in the language. It's considered a best practice to always indicate when a fallthrough is intentional using a comment which matches the `/falls?\s?through/i` regular expression but isn't a directive: ```js switch(foo) { @@ -169,7 +169,7 @@ Note that the last `case` statement in these examples does not cause a warning b This rule has an object option: -* Set the `commentPattern` option to a regular expression string to change the test for intentional fallthrough comment. +* Set the `commentPattern` option to a regular expression string to change the test for intentional fallthrough comment. If the fallthrough comment matches a directive, that takes precedence over `commentPattern`. * Set the `allowEmptyCase` option to `true` to allow empty cases regardless of the layout. By default, this rule does not require a fallthrough comment after an empty `case` only if the empty `case` and the next `case` are on the same line or on consecutive lines. diff --git a/lib/linter/linter.js b/lib/linter/linter.js index 764ea5f5795..0f1bd4f7761 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -18,6 +18,9 @@ const merge = require("lodash.merge"), pkg = require("../../package.json"), astUtils = require("../shared/ast-utils"), + { + directivesPattern + } = require("../shared/directives"), { Legacy: { ConfigOps, @@ -377,7 +380,7 @@ function getDirectiveComments(ast, ruleMapper, warnInlineConfig) { ast.comments.filter(token => token.type !== "Shebang").forEach(comment => { const { directivePart, justificationPart } = extractDirectiveComment(comment.value); - const match = /^(eslint(?:-env|-enable|-disable(?:(?:-next)?-line)?)?|exported|globals?)(?:\s|$)/u.exec(directivePart); + const match = directivesPattern.exec(directivePart); if (!match) { return; diff --git a/lib/rules/no-fallthrough.js b/lib/rules/no-fallthrough.js index 536aa213f8a..4c911eaf409 100644 --- a/lib/rules/no-fallthrough.js +++ b/lib/rules/no-fallthrough.js @@ -4,12 +4,28 @@ */ "use strict"; +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const { directivesPattern } = require("../shared/directives"); + //------------------------------------------------------------------------------ // Helpers //------------------------------------------------------------------------------ const DEFAULT_FALLTHROUGH_COMMENT = /falls?\s?through/iu; +/** + * Checks whether or not a given comment string is really a fallthrough comment and not an ESLint directive. + * @param {string} comment The comment string to check. + * @param {RegExp} fallthroughCommentPattern The regular expression used for checking for fallthrough comments. + * @returns {boolean} `true` if the comment string is truly a fallthrough comment. + */ +function isFallThroughComment(comment, fallthroughCommentPattern) { + return fallthroughCommentPattern.test(comment) && !directivesPattern.test(comment.trim()); +} + /** * Checks whether or not a given case has a fallthrough comment. * @param {ASTNode} caseWhichFallsThrough SwitchCase node which falls through. @@ -25,14 +41,14 @@ function hasFallthroughComment(caseWhichFallsThrough, subsequentCase, context, f const trailingCloseBrace = sourceCode.getLastToken(caseWhichFallsThrough.consequent[0]); const commentInBlock = sourceCode.getCommentsBefore(trailingCloseBrace).pop(); - if (commentInBlock && fallthroughCommentPattern.test(commentInBlock.value)) { + if (commentInBlock && isFallThroughComment(commentInBlock.value, fallthroughCommentPattern)) { return true; } } const comment = sourceCode.getCommentsBefore(subsequentCase).pop(); - return Boolean(comment && fallthroughCommentPattern.test(comment.value)); + return Boolean(comment && isFallThroughComment(comment.value, fallthroughCommentPattern)); } /** diff --git a/lib/shared/directives.js b/lib/shared/directives.js new file mode 100644 index 00000000000..ff67b00a553 --- /dev/null +++ b/lib/shared/directives.js @@ -0,0 +1,15 @@ +/** + * @fileoverview Common utils for directives. + * + * This file contains only shared items for directives. + * If you make a utility for rules, please see `../rules/utils/ast-utils.js`. + * + * @author gfyoung + */ +"use strict"; + +const directivesPattern = /^(eslint(?:-env|-enable|-disable(?:(?:-next)?-line)?)?|exported|globals?)(?:\s|$)/u; + +module.exports = { + directivesPattern +}; diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 549969cc165..7a1ba3779c0 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -4189,6 +4189,27 @@ var a = "test2"; assert.strictEqual(suppressedMessages[0].ruleId, "no-alert"); }); + it("reports no problems for no-fallthrough despite comment pattern match", () => { + const code = "switch (foo) { case 0: a(); \n// eslint-disable-next-line no-fallthrough\n case 1: }"; + const config = { + reportUnusedDisableDirectives: true, + rules: { + "no-fallthrough": 2 + } + }; + + const messages = linter.verify(code, config, { + filename, + allowInlineConfig: true + }); + const suppressedMessages = linter.getSuppressedMessages(); + + assert.strictEqual(messages.length, 0); + + assert.strictEqual(suppressedMessages.length, 1); + assert.strictEqual(suppressedMessages[0].ruleId, "no-fallthrough"); + }); + describe("autofix", () => { const alwaysReportsRule = { create(context) { diff --git a/tests/lib/rules/no-fallthrough.js b/tests/lib/rules/no-fallthrough.js index d6f6c6f93f2..80fa61733f3 100644 --- a/tests/lib/rules/no-fallthrough.js +++ b/tests/lib/rules/no-fallthrough.js @@ -63,6 +63,7 @@ ruleTester.run("no-fallthrough", rule, { "switch (foo) { case 0: try {} finally { break; } default: b(); }", "switch (foo) { case 0: try { throw 0; } catch (err) { break; } default: b(); }", "switch (foo) { case 0: do { throw 0; } while(a); default: b(); }", + "switch (foo) { case 0: a(); \n// eslint-disable-next-line no-fallthrough\n case 1: }", { code: "switch(foo) { case 0: a(); /* no break */ case 1: b(); }", options: [{ @@ -297,6 +298,18 @@ ruleTester.run("no-fallthrough", rule, { column: 34 } ] + }, + { + code: "switch (foo) { case 0: a(); \n// eslint-enable no-fallthrough\n case 1: }", + options: [{}], + errors: [ + { + messageId: "case", + type: "SwitchCase", + line: 3, + column: 2 + } + ] } ] });