From 6ab10514864e12299d795dcd3328b4e03de0a666 Mon Sep 17 00:00:00 2001 From: Stephen Cyron <63068182+scyron6@users.noreply.github.com> Date: Thu, 7 Apr 2022 19:22:47 -0400 Subject: [PATCH] Fix false positive lint error with large number of branches (#24287) * Switched RulesOfHooks.js to use BigInt. Added test and updated .eslintrc.js to use es2020. * Added BigInt as readonly global in eslintrc.cjs.js and eslintrc.cjs2015.js * Added comment to RulesOfHooks.js that gets rid of BigInt eslint error * Got rid of changes in .eslintrc.js and yarn.lock * Move global down Co-authored-by: stephen cyron Co-authored-by: dan --- .../__tests__/ESLintRulesOfHooks-test.js | 69 +++++++++++++++++++ .../src/RulesOfHooks.js | 19 ++--- scripts/rollup/validate/eslintrc.cjs.js | 1 + scripts/rollup/validate/eslintrc.cjs2015.js | 1 + 4 files changed, 81 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index b6b0c8d4a9674..044cc58f40c4c 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -325,6 +325,75 @@ const tests = { useHook(); } `, + ` + // Valid because the neither the conditions before or after the hook affect the hook call + // Failed prior to implementing BigInt because pathsFromStartToEnd and allPathsFromStartToEnd were too big and had rounding errors + const useSomeHook = () => {}; + + const SomeName = () => { + const filler = FILLER ?? FILLER ?? FILLER; + const filler2 = FILLER ?? FILLER ?? FILLER; + const filler3 = FILLER ?? FILLER ?? FILLER; + const filler4 = FILLER ?? FILLER ?? FILLER; + const filler5 = FILLER ?? FILLER ?? FILLER; + const filler6 = FILLER ?? FILLER ?? FILLER; + const filler7 = FILLER ?? FILLER ?? FILLER; + const filler8 = FILLER ?? FILLER ?? FILLER; + + useSomeHook(); + + if (anyConditionCanEvenBeFalse) { + return null; + } + + return ( + + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + {FILLER ? FILLER : FILLER} + + ); + }; + `, ` // Valid because the neither the condition nor the loop affect the hook call. function App(props) { diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index 1273bf1d0c712..31d54e813ae65 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +/* global BigInt */ /* eslint-disable no-for-of-loops/no-for-of-loops */ 'use strict'; @@ -175,7 +176,7 @@ export default { cyclic.add(cyclicSegment); } - return 0; + return BigInt('0'); } // add the current segment to pathList @@ -187,11 +188,11 @@ export default { } if (codePath.thrownSegments.includes(segment)) { - paths = 0; + paths = BigInt('0'); } else if (segment.prevSegments.length === 0) { - paths = 1; + paths = BigInt('1'); } else { - paths = 0; + paths = BigInt('0'); for (const prevSegment of segment.prevSegments) { paths += countPathsFromStart(prevSegment, pathList); } @@ -199,7 +200,7 @@ export default { // If our segment is reachable then there should be at least one path // to it from the start of our code path. - if (segment.reachable && paths === 0) { + if (segment.reachable && paths === BigInt('0')) { cache.delete(segment.id); } else { cache.set(segment.id, paths); @@ -246,7 +247,7 @@ export default { cyclic.add(cyclicSegment); } - return 0; + return BigInt('0'); } // add the current segment to pathList @@ -258,11 +259,11 @@ export default { } if (codePath.thrownSegments.includes(segment)) { - paths = 0; + paths = BigInt('0'); } else if (segment.nextSegments.length === 0) { - paths = 1; + paths = BigInt('1'); } else { - paths = 0; + paths = BigInt('0'); for (const nextSegment of segment.nextSegments) { paths += countPathsToEnd(nextSegment, pathList); } diff --git a/scripts/rollup/validate/eslintrc.cjs.js b/scripts/rollup/validate/eslintrc.cjs.js index 802141d6bc101..a76aa67155e22 100644 --- a/scripts/rollup/validate/eslintrc.cjs.js +++ b/scripts/rollup/validate/eslintrc.cjs.js @@ -7,6 +7,7 @@ module.exports = { }, globals: { // ES 6 + BigInt: 'readonly', Map: 'readonly', Set: 'readonly', Proxy: 'readonly', diff --git a/scripts/rollup/validate/eslintrc.cjs2015.js b/scripts/rollup/validate/eslintrc.cjs2015.js index b579566145778..32bb3e83aa3a8 100644 --- a/scripts/rollup/validate/eslintrc.cjs2015.js +++ b/scripts/rollup/validate/eslintrc.cjs2015.js @@ -7,6 +7,7 @@ module.exports = { }, globals: { // ES 6 + BigInt: 'readonly', Map: 'readonly', Set: 'readonly', Proxy: 'readonly',