From 2856f0338e2948af086ef755fa9d249de241282b Mon Sep 17 00:00:00 2001
From: Panos Vekris <pvekris@meta.com>
Date: Tue, 2 Apr 2024 18:37:07 -0700
Subject: [PATCH] [flow-dev-tools] Do not remove suppressions that include
 "eslint-disable"

Summary:
While not common it is possible that a single comment is used to suppress both a Flow error and an eslint issue. E.g.
```
// $FlowFixMe eslint-disable-next-line no-fallthrough
```
This diff prevents `flow update-suppressions` from removing a Flow suppression comment if in included in the text associated with it is the string `eslint-disable`.

Changelog: [internal]

Reviewed By: SamChou19815

Differential Revision: D55656739

fbshipit-source-id: 9768276f4df1895377347883dfef9a976f172815
---
 .../flow-dev-tools/src/__tests__/all-tests.js | 50 +++++++++++++++++++
 .../src/comment/commentMutator.js             |  6 +++
 .../update-suppressionsRunner.js              |  7 +++
 3 files changed, 63 insertions(+)

diff --git a/packages/flow-dev-tools/src/__tests__/all-tests.js b/packages/flow-dev-tools/src/__tests__/all-tests.js
index c7f0f26233e..248e31f8d4d 100644
--- a/packages/flow-dev-tools/src/__tests__/all-tests.js
+++ b/packages/flow-dev-tools/src/__tests__/all-tests.js
@@ -59,6 +59,56 @@ function test(name, fn) {
   collectedTests.push({name, fn});
 }
 
+test('updateSuppressionsInText does not remove eslint-disable comment', async () => {
+  const testInput = `
+// $FlowFixMe eslint-disable-next-line no-fallthrough
+const x = 4;
+`.trimLeft();
+
+  const loc = makeLoc(testInput, 1, 1, 1, 54);
+
+  const testOutput = `
+// $FlowFixMe eslint-disable-next-line no-fallthrough
+const x = 4;
+`.trimLeft();
+
+  const errorsByLine = addErrorByLine(
+    new Map(),
+    testInput,
+    loc,
+    [],
+    ['foo', 'bar'],
+  );
+  await expectUpdatedComments(testInput, testOutput, errorsByLine, flowBinPath);
+});
+
+test('updateSuppressionsInText does not remove eslint-disable multiline comment', async () => {
+  const testInput = `
+/* $FlowFixMe
+ * eslint-disable no-fallthrough
+ */
+const x = 4;
+`.trimLeft();
+
+  const loc = makeLoc(testInput, 1, 1, 3, 4);
+
+  const testOutput = `
+/* $FlowFixMe
+ * eslint-disable no-fallthrough
+ */
+const x = 4;
+`.trimLeft();
+
+  const errorsByLine = addErrorByLine(
+    new Map(),
+    testInput,
+    loc,
+    [],
+    ['foo', 'bar'],
+  );
+  await expectUpdatedComments(testInput, testOutput, errorsByLine, flowBinPath);
+});
+
 test('exec', async () => {
   expect(await exec('echo foo')).toBe('foo\n');
   expect(await exec('cat', {stdin: 'bar'})).toBe('bar');
diff --git a/packages/flow-dev-tools/src/comment/commentMutator.js b/packages/flow-dev-tools/src/comment/commentMutator.js
index 4b7572467d9..b56595102a3 100644
--- a/packages/flow-dev-tools/src/comment/commentMutator.js
+++ b/packages/flow-dev-tools/src/comment/commentMutator.js
@@ -24,6 +24,11 @@ function isLintSuppression(commentAST: {value: string}): boolean {
   return flowlintRegex.test(commentAST.value);
 }
 
+const eslintRegex = /eslint-disable/m;
+function isEsLintSuppression(value: string): boolean {
+  return eslintRegex.test(value);
+}
+
 const newlineRegex = /[\u2029\u2028\r\n]/;
 const edible = /[\t ]/;
 /* This is the most confusing part of this command. A simple version of this
@@ -428,6 +433,7 @@ function formatComment(
 
 module.exports = {
   isLintSuppression,
+  isEsLintSuppression,
   findStartOfLine,
   insertCommentToText,
   addCommentToText,
diff --git a/packages/flow-dev-tools/src/update-suppressions/update-suppressionsRunner.js b/packages/flow-dev-tools/src/update-suppressions/update-suppressionsRunner.js
index e5d043cd526..5c2c99c4306 100644
--- a/packages/flow-dev-tools/src/update-suppressions/update-suppressionsRunner.js
+++ b/packages/flow-dev-tools/src/update-suppressions/update-suppressionsRunner.js
@@ -25,6 +25,7 @@ const {readFile, writeFile} = require('fs').promises;
 const {
   removeUnusedErrorSuppressionFromText,
   isLintSuppression,
+  isEsLintSuppression,
   addCommentToText,
   findStartOfLine,
 } = require('../comment/commentMutator');
@@ -219,6 +220,12 @@ function updateErrorSuppression(
   let innerOffset = commentStartOffset + 2; // `/*` and `//` are both 2 chars
   let text = contents.slice(innerOffset, endOffset).toString('utf8');
 
+  // do not remove the comment if it's a eslint suppression
+  // TODO update the logging provided in the end of the command
+  if (isEsLintSuppression(text)) {
+    return contents;
+  }
+
   const existingRoots = getSites(text); // may include unknown roots
   const roots = new Set([...existingRoots, ...knownRoots]);
   unusedRoots.forEach(root => roots.delete(root));