[eslint-plugin-react-hooks] Fix cyclic caching for loops containing a… (#16853)

* [eslint-plugin-react-hooks] Fix cyclic caching for loops containing a condition
* [eslint-plugin-react-hooks] prettier write
* [eslint-plugin-react-hooks] Fix set for tests
* Update packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
Co-Authored-By: Luke Kang <kidkkr@icloud.com>
Co-authored-by: Luke Kang <kidkkr@icloud.com>

Author: M-Izadmehr

packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js

@@ -405,6 +405,18 @@ const tests = {
  useHook();
  }
  `,
+`
+ // Valid because the neither the condition nor the loop affect the hook call.
+ function App(props) {
+ const someObject = {propA: true};
+ for (const propName in someObject) {
+ if (propName === true) {
+ } else {
+ }
+ }
+ const [myState, setMyState] = useState(null);
+ }
+ `,
 ],
 invalid: [
 {
@@ -640,14 +652,7 @@ const tests = {
  }
  }
  `,
-errors: [
-loopError('useHook1'),
-
-// NOTE: Small imprecision in error reporting due to caching means we
-// have a conditional error here instead of a loop error. However,
-// we will always get an error so this is acceptable.
-conditionalError('useHook2',true),
-],
+errors: [loopError('useHook1'),loopError('useHook2',true)],
 },
 {
 code: `

packages/eslint-plugin-react-hooks/src/RulesOfHooks.js

@@ -152,39 +152,41 @@ export default {
  * Populates `cyclic` with cyclic segments.
  */
 
-functioncountPathsFromStart(segment){
+functioncountPathsFromStart(segment,pathHistory){
 const{cache}=countPathsFromStart;
 letpaths=cache.get(segment.id);
-
-// If `paths` is null then we've found a cycle! Add it to `cyclic` and
-// any other segments which are a part of this cycle.
-if(paths===null){
-if(cyclic.has(segment.id)){
-return0;
-}else{
-cyclic.add(segment.id);
-for(constprevSegmentofsegment.prevSegments){
-countPathsFromStart(prevSegment);
-}
-return0;
+constpathList=newSet(pathHistory);
+
+// If `pathList` includes the current segment then we've found a cycle!
+// We need to fill `cyclic` with all segments inside cycle
+if(pathList.has(segment.id)){
+constpathArray=[...pathList];
+constcyclicSegments=pathArray.slice(
+pathArray.indexOf(segment.id)+1,
+);
+for(constcyclicSegmentofcyclicSegments){
+cyclic.add(cyclicSegment);
 }
+
+return0;
 }
 
+// add the current segment to pathList
+pathList.add(segment.id);
+
 // We have a cached `paths`. Return it.
 if(paths!==undefined){
 returnpaths;
 }
 
-// Compute `paths` and cache it. Guarding against cycles.
-cache.set(segment.id,null);
 if(codePath.thrownSegments.includes(segment)){
 paths=0;
 }elseif(segment.prevSegments.length===0){
 paths=1;
 }else{
 paths=0;
 for(constprevSegmentofsegment.prevSegments){
-paths+=countPathsFromStart(prevSegment);
+paths+=countPathsFromStart(prevSegment,pathList);
 }
 }
 
@@ -221,43 +223,45 @@ export default {
  * Populates `cyclic` with cyclic segments.
  */
 
-functioncountPathsToEnd(segment){
+functioncountPathsToEnd(segment,pathHistory){
 const{cache}=countPathsToEnd;
 letpaths=cache.get(segment.id);
-
-// If `paths` is null then we've found a cycle! Add it to `cyclic` and
-// any other segments which are a part of this cycle.
-if(paths===null){
-if(cyclic.has(segment.id)){
-return0;
-}else{
-cyclic.add(segment.id);
-for(constnextSegmentofsegment.nextSegments){
-countPathsToEnd(nextSegment);
-}
-return0;
+letpathList=newSet(pathHistory);
+
+// If `pathList` includes the current segment then we've found a cycle!
+// We need to fill `cyclic` with all segments inside cycle
+if(pathList.has(segment.id)){
+constpathArray=Array.from(pathList);
+constcyclicSegments=pathArray.slice(
+pathArray.indexOf(segment.id)+1,
+);
+for(constcyclicSegmentofcyclicSegments){
+cyclic.add(cyclicSegment);
 }
+
+return0;
 }
 
+// add the current segment to pathList
+pathList.add(segment.id);
+
 // We have a cached `paths`. Return it.
 if(paths!==undefined){
 returnpaths;
 }
 
-// Compute `paths` and cache it. Guarding against cycles.
-cache.set(segment.id,null);
 if(codePath.thrownSegments.includes(segment)){
 paths=0;
 }elseif(segment.nextSegments.length===0){
 paths=1;
 }else{
 paths=0;
 for(constnextSegmentofsegment.nextSegments){
-paths+=countPathsToEnd(nextSegment);
+paths+=countPathsToEnd(nextSegment,pathList);
 }
 }
-cache.set(segment.id,paths);
 
+cache.set(segment.id,paths);
 returnpaths;
 }