Exp: modify isReadOnly to support more read cases (#29)

* fix: segregate read only to implementations

moves isRead only to manually handle react hook primitives
creates another helper to check if a value is being returned
for a custom hook
also added additional check to `isReadOnly` to avoid
raising flags when working with callExpressions

* additional case

Author: barelyhuman

.gitignore

@@ -24,3 +24,4 @@ yarn-error.log*
 
 # misc
 .DS_Store
+.eslintcache

src/StateSnapshot.js

@@ -1,8 +1,10 @@
 import {
   callExpressions,
   functionTypes,
-  getParentOfNodeType,
-  isInHookDeps,
+  isDepthSameAsRootComponent,
+  isInCustomHookDef,
+  isInReactHookDeps,
+  isInReactHooks,
   isInSomething,
   isReadOnly,
 } from './lib/utils'
@@ -79,8 +81,9 @@ export default {
         if (
           node.parent.type === 'MemberExpression' &&
           node.parent.property === node
-        )
+        ) {
           return
+        }
 
         const kind = which(node.name, scope)
         if (kind === 'state' && isInRender(node)) {
@@ -94,16 +97,35 @@ export default {
           // ignore the error if the snapshot
           // is just being read in the hook and is a part of the dependency array
 
-          if (
-            isReadOnly(node) &&
-            (isInHookDeps(node) ||
+          // Valids
+          // - allowed to read at root level for computation (new hook / render level defs) , basically anything defined at the root of the component definition
+          // [x] allowed to read in a useEffect and useCallback if added in deps
+
+          // Invalids
+          // [x] if being used in a callback that isn't useEffect or useCallback
+
+          if (isReadOnly(node)) {
+            if (isInReactHooks(node) && !isInReactHookDeps(node)) {
+              return context.report({
+                node,
+                message: SNAPSHOT_CALLBACK_MESSAGE,
+              })
+            }
+            if (
+              isDepthSameAsRootComponent(node) ||
               isInJSXContainer(node) ||
-              isInDeclaration(node))
-          ) {
-            return
+              isInCustomHookDef(node)
+            ) {
+              return
+            }
+          } else {
+            return context.report({
+              node,
+              message: SNAPSHOT_CALLBACK_MESSAGE,
+            })
           }
 
-          if (isInCallback(node)) {
+          if (isInCallback(node) && !isInReactHooks(node)) {
             return context.report({
               node,
               message: SNAPSHOT_CALLBACK_MESSAGE,
@@ -218,8 +240,9 @@ function isComputedIdentifier(node, scope) {
     }
   })
 
-  if (!isIt && scope.upper)
+  if (!isIt && scope.upper) {
     return (isIt = isComputedIdentifier(node, scope.upper))
+  }
 
   return isIt
 }
@@ -325,8 +348,9 @@ function isUsedInUseProxy(node, scope) {
       }
     }
   })
-  if (!isUsed && scope.upper)
+  if (!isUsed && scope.upper) {
     return (isUsed = isUsedInUseProxy(node, scope.upper))
+  }
   return isUsed
 }
 
@@ -372,12 +396,12 @@ function isInJSXContainer(node) {
   )
 }
 
-function isInDeclaration(node) {
-  const _parentDeclaration = getParentOfNodeType(node, 'VariableDeclarator')
+// function isInDeclaration(node) {
+//   const _parentDeclaration = getParentOfNodeType(node, 'VariableDeclarator')
 
-  if (_parentDeclaration?.init?.callee?.name === 'useSnapshot') {
-    return true
-  }
+//   if (_parentDeclaration?.init?.callee?.name === 'useSnapshot') {
+//     return true
+//   }
 
-  return false
-}
+//   return false
+// }

src/lib/utils.js

@@ -16,7 +16,6 @@ export const writingOpExpressionTypes = ['UpdateExpression']
  *
  *  isInSomething(thisNode,"FunctionExpression") // true
  *  isInSomething(thisNode,"ExpressionStatement") // false
- *
  */
 export function isInSomething(node, thing) {
   if (node.parent && node.parent.type !== thing) {
@@ -30,18 +29,16 @@ export function isInSomething(node, thing) {
 /**
  * @param {any} node ASTNode
  *
- *
  * @example
  *
- *const stateOne = proxy({
+ * const stateOne = proxy({
  * count: 0,
  * inc: function () {
  *   ++state.count //<== taking node from here
  *  },
  * })
  *
  * nearestCalleeName(node) //=> "proxy" //<= proxy is the nearest callee
- *
  */
 export function nearestCalleeName(node) {
   if (!(node && node.parent)) {
@@ -81,9 +78,12 @@ export function nearestCalleeName(node) {
  *
  *  getParentOfNodeType(thisNode,"FunctionExpression") // ASTNode.type: "FunctionExpression"
  *  getParentOfNodeType(thisNode,"ExpressionStatement") // null
- *
  */
 export function getParentOfNodeType(node, nodeType) {
+  if (!node?.parent) {
+    return null
+  }
+
   if (node.parent && node.parent.type !== nodeType) {
     return getParentOfNodeType(node.parent, nodeType)
   } else if (node.parent && node.parent.type === nodeType) {
@@ -111,11 +111,14 @@ export function isReadOnly(node) {
     return isReadOnly(node.parent)
   }
 
+  if (node.parent.type === 'CallExpression') {
+    return true
+  }
+
   return true
 }
 
 /**
- *
  * @param {*} node
  * @returns {boolean} true if the node is on the left
  * of an assignment expression aka being modified
@@ -152,28 +155,43 @@ export function returnFirstCallback(node) {
  * resulting node or not
  * @returns {* | boolean} - either true/false or the CallExpression node that belongs to the hook
  */
-export function isInHook(node, returnHook = false) {
+export function isInReactHooks(node, returnHook = false) {
   const hookDef = getNearestHook(node)
   if (returnHook) {
     return hookDef
   }
   return hookDef ? true : false
 }
 
+function isReactPrimitive(node) {
+  if (
+    node.type === 'Identifier' &&
+    (node.name === 'useEffect' || node.name === 'useCallback')
+  ) {
+    return true
+  }
+
+  if (node.type === 'MemberExpression') {
+    const flatExpr = flattenMemberExpression(node)
+    return flatExpr.endsWith('useEffect') || flatExpr.endsWith('useCallback')
+  }
+
+  return false
+}
+
 export function getNearestHook(node) {
   if (!node.parent || !node.parent.type) return false
 
-  if (
-    functionTypes.includes(node.type) &&
-    node.parent.type == 'CallExpression' &&
-    node.parent.callee.type === 'Identifier' &&
-    (node.parent.callee.name === 'useEffect' ||
-      node.parent.callee.name === 'useCallback')
-  ) {
-    return node.parent
-  } else {
-    return getNearestHook(node.parent)
+  const parentCaller = getParentOfNodeType(node, 'CallExpression')
+  if (!parentCaller) {
+    return false
+  }
+
+  if (!isReactPrimitive(parentCaller.callee)) {
+    return getNearestHook(parentCaller)
   }
+
+  return parentCaller
 }
 
 /**
@@ -182,8 +200,8 @@ export function getNearestHook(node) {
  * @param {*} node
  * @returns {boolean}
  */
-export function isInHookDeps(node) {
-  const hookNode = isInHook(node, true)
+export function isInReactHookDeps(node) {
+  const hookNode = isInReactHooks(node, true)
   if (!hookNode) {
     return false
   }
@@ -271,3 +289,55 @@ function flattenMemberExpression(expr, key = '') {
     return path
   }
 }
+
+export function isDepthSameAsRootComponent(node) {
+  const varDef = getParentOfNodeType(node, 'VariableDeclaration')
+  const parentNormalFunc = getParentOfNodeType(varDef, 'FunctionDeclaration')
+  const parentArrFunc = getParentOfNodeType(varDef, 'ArrowFunctionExpression')
+
+  const parentFunc = parentNormalFunc || parentArrFunc
+  if (!parentFunc) {
+    return false
+  }
+
+  if (
+    parentFunc?.parent.type === 'VariableDeclarator' &&
+    parentFunc?.parent.parent.type === 'VariableDeclaration' &&
+    parentFunc?.parent.parent.parent.type === 'Program'
+  ) {
+    return true
+  }
+}
+
+export function isInCustomHookDef(node) {
+  const nearestReturn = getParentOfNodeType(node, 'ReturnStatement')
+  const returnInBlock = getParentOfNodeType(nearestReturn, 'BlockStatement')
+  const normalFuncDef = getParentOfNodeType(
+    returnInBlock,
+    'ArrowFunctionExpression'
+  )
+  const arrowFuncDef = getParentOfNodeType(returnInBlock, 'FunctionExpression')
+
+  const nearestFuncDef = normalFuncDef || arrowFuncDef || false
+
+  if (!nearestFuncDef) {
+    return false
+  }
+
+  const varDeclaratorOfFunc = getParentOfNodeType(
+    nearestFuncDef,
+    'VariableDeclarator'
+  )
+  const varDefOfFunc = getParentOfNodeType(
+    varDeclaratorOfFunc,
+    'VariableDeclaration'
+  )
+
+  const varDefOnRoot = varDefOfFunc.parent?.type === 'Program' || false
+
+  return (
+    varDefOnRoot &&
+    varDeclaratorOfFunc.id?.type === 'Identifier' &&
+    varDeclaratorOfFunc.id?.name.startsWith('use')
+  )
+}