fix: ignore directives for no-fallthrough (#16757)

Fallthrough comment pattern detection was incorrectly matching to eslint directives. Fixes #16650

Author: gfyoung

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.
 

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;

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));
 }
 
 /**

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 <https://github.com/gfyoung>
+ */
+"use strict";
+
+const directivesPattern = /^(eslint(?:-env|-enable|-disable(?:(?:-next)?-line)?)?|exported|globals?)(?:\s|$)/u;
+
+module.exports = {
+ directivesPattern
+};

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) {

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
+ }
+ ]
  }
  ]
 });