Address short circuiting in and / or interpreted

Author: TotalTechGeek

compiler.js

@@ -314,7 +314,7 @@ function processBuiltString (method, str, buildState) {
   return Object.assign(
     (typeof globalThis !== 'undefined' ? globalThis : global).eval(final)(values, methods, notTraversed, asyncIterators, engine, above, coerceArray), {
       [Sync]: !buildState.asyncDetected,
-      aboveDetected: str.includes(', above')
+      aboveDetected: typeof str === 'string' && str.includes(', above')
     })
 }
 

defaultMethods.js

@@ -159,17 +159,81 @@ const defaultMethods = {
   '!=': ([a, b]) => a != b,
   '!==': ([a, b]) => a !== b,
   xor: ([a, b]) => a ^ b,
-  or: (arr, _1, _2, engine) => {
-    for (let i = 0; i < arr.length; i++) {
-      if (engine.truthy(arr[i])) return arr[i]
-    }
-    return arr[arr.length - 1]
+  // Why "executeInLoop"? Because if it needs to execute to get an array, I do not want to execute the arguments,
+  // Both for performance and safety reasons.
+  or: {
+    method: (arr, _1, _2, engine) => {
+      // See "executeInLoop" above
+      const executeInLoop = Array.isArray(arr)
+      if (!executeInLoop) arr = engine.run(arr, _1, { above: _2 })
+
+      let item
+      for (let i = 0; i < arr.length; i++) {
+        item = executeInLoop ? engine.run(arr[i], _1, { above: _2 }) : arr[i]
+        if (engine.truthy(item)) return item
+      }
+
+      return item
+    },
+    asyncMethod: async (arr, _1, _2, engine) => {
+      // See "executeInLoop" above
+      const executeInLoop = Array.isArray(arr)
+      if (!executeInLoop) arr = await engine.run(arr, _1, { above: _2 })
+
+      let item
+      for (let i = 0; i < arr.length; i++) {
+        item = executeInLoop ? await engine.run(arr[i], _1, { above: _2 }) : arr[i]
+        if (engine.truthy(item)) return item
+      }
+
+      return item
+    },
+    deterministic: (data, buildState) => isDeterministic(data, buildState.engine, buildState),
+    compile: (data, buildState) => {
+      if (!buildState.engine.truthy.IDENTITY) return false
+      if (Array.isArray(data)) {
+        return `(${data.map((i) => buildString(i, buildState)).join(' || ')})`
+      } else {
+        return `(${buildString(data, buildState)}).reduce((a,b) => a||b, false)`
+      }
+    },
+    traverse: false
   },
-  and: (arr, _1, _2, engine) => {
-    for (let i = 0; i < arr.length; i++) {
-      if (!engine.truthy(arr[i])) return arr[i]
+  and: {
+    method: (arr, _1, _2, engine) => {
+      // See "executeInLoop" above
+      const executeInLoop = Array.isArray(arr)
+      if (!executeInLoop) arr = engine.run(arr, _1, { above: _2 })
+
+      let item
+      for (let i = 0; i < arr.length; i++) {
+        item = executeInLoop ? engine.run(arr[i], _1, { above: _2 }) : arr[i]
+        if (!engine.truthy(item)) return item
+      }
+      return item
+    },
+    asyncMethod: async (arr, _1, _2, engine) => {
+      // See "executeInLoop" above
+      const executeInLoop = Array.isArray(arr)
+      if (!executeInLoop) arr = await engine.run(arr, _1, { above: _2 })
+
+      let item
+      for (let i = 0; i < arr.length; i++) {
+        item = executeInLoop ? await engine.run(arr[i], _1, { above: _2 }) : arr[i]
+        if (!engine.truthy(item)) return item
+      }
+      return item
+    },
+    traverse: false,
+    deterministic: (data, buildState) => isDeterministic(data, buildState.engine, buildState),
+    compile: (data, buildState) => {
+      if (!buildState.engine.truthy.IDENTITY) return false
+      if (Array.isArray(data)) {
+        return `(${data.map((i) => buildString(i, buildState)).join(' && ')})`
+      } else {
+        return `(${buildString(data, buildState)}).reduce((a,b) => a&&b, true)`
+      }
     }
-    return arr[arr.length - 1]
   },
   substr: ([string, from, end]) => {
     if (end < 0) {
@@ -677,32 +741,12 @@ defaultMethods['%'].compile = function (data, buildState) {
   }
 }
 
-// @ts-ignore Allow custom attribute
-defaultMethods.or.compile = function (data, buildState) {
-  if (!buildState.engine.truthy.IDENTITY) return false
-  if (Array.isArray(data)) {
-    return `(${data.map((i) => buildString(i, buildState)).join(' || ')})`
-  } else {
-    return `(${buildString(data, buildState)}).reduce((a,b) => a||b, false)`
-  }
-}
-
 // @ts-ignore Allow custom attribute
 defaultMethods.in.compile = function (data, buildState) {
   if (!Array.isArray(data)) return false
   return buildState.compile`(${data[1]} || []).includes(${data[0]})`
 }
 
-// @ts-ignore Allow custom attribute
-defaultMethods.and.compile = function (data, buildState) {
-  if (!buildState.engine.truthy.IDENTITY) return false
-  if (Array.isArray(data)) {
-    return `(${data.map((i) => buildString(i, buildState)).join(' && ')})`
-  } else {
-    return `(${buildString(data, buildState)}).reduce((a,b) => a&&b, true)`
-  }
-}
-
 // @ts-ignore Allow custom attribute
 defaultMethods['-'].compile = function (data, buildState) {
   if (Array.isArray(data)) {

general.test.js

@@ -198,6 +198,56 @@ describe('Various Test Cases', () => {
     for (const engine of [...normalEngines, ...permissiveEngines]) await testEngine(engine, { map: [[1, 2, 3], { map: [[1], { var: '../../../../name' }] }] }, { name: 'Bob' }, [['Bob'], ['Bob'], ['Bob']])
   })
 
+  it('correctly short circuits (or)', async () => {
+    for (const engine of [...normalEngines, ...permissiveEngines]) {
+      let count = 0
+
+      engine.addMethod('increment', {
+        method: () => {
+          count++
+          return true
+        }
+      })
+      await testEngine(engine, { or: [{ increment: true }, { increment: true }, { increment: true }] }, {}, true)
+      // It's called twice because it runs once for interpreted, and once for compiled.
+      assert.strictEqual(count, 2, 'Should have only called increment twice. (Count)')
+    }
+  })
+
+  it('correctly short circuits (and)', async () => {
+    for (const engine of [...normalEngines, ...permissiveEngines]) {
+      let count = 0
+
+      engine.addMethod('increment', {
+        method: () => {
+          count++
+          return false
+        }
+      })
+      await testEngine(engine, { and: [{ increment: true }, { increment: true }, { increment: true }] }, {}, false)
+      // It's called twice because it runs once for interpreted, and once for compiled.
+      assert.strictEqual(count, 2, 'Should have only called increment twice. (Count)')
+    }
+  })
+
+  it('does not execute any logic from and / or array if the argument is executed', async () => {
+    for (const engine of [...normalEngines, ...permissiveEngines]) {
+      let count = 0
+      engine.addMethod('increment', {
+        method: () => {
+          count++
+          return true
+        }
+      })
+
+      await testEngine(engine, { and: { preserve: [{ increment: true }, { increment: true }] } }, {}, { increment: true })
+      assert.strictEqual(count, 0, 'Should not have called increment on and.')
+
+      await testEngine(engine, { or: { preserve: [{ increment: true }, { increment: true }] } }, {}, { increment: true })
+      assert.strictEqual(count, 0, 'Should not have called increment on or.')
+    }
+  })
+
   it('disables interpreted optimization when it realizes it will not be faster', async () => {
     for (const engine of [...normalEngines, ...permissiveEngines]) {
       const disableInterpretedOptimization = engine.disableInterpretedOptimization

package.json

@@ -1,6 +1,6 @@
 {
   "name": "json-logic-engine",
-  "version": "4.0.1",
+  "version": "4.0.2",
   "description": "Construct complex rules with JSON & process them.",
   "main": "./dist/cjs/index.js",
   "module": "./dist/esm/index.js",