From 55e873e230d5f443bcf9ba69fceae4bbed34762d Mon Sep 17 00:00:00 2001
From: Simon Fan <simon@orioro.design>
Date: Mon, 15 Feb 2021 22:36:36 -0300
Subject: [PATCH] feat: add logging to possible unknown expression evaluation

---
 src/expression.spec.ts          |  9 +++++++
 src/expression.ts               | 45 ++++++++++++++++++++++++++++-----
 src/expressions/logical.spec.ts | 42 ++++++++++++++++++++----------
 src/expressions/logical.ts      |  4 ---
 todo.md                         |  4 +--
 5 files changed, 76 insertions(+), 28 deletions(-)

diff --git a/src/expression.spec.ts b/src/expression.spec.ts
index 04dbe0c..6a9c31e 100644
--- a/src/expression.spec.ts
+++ b/src/expression.spec.ts
@@ -15,6 +15,9 @@ describe('evaluateTyped(expectedTypes, context, value)', () => {
       )
     }).toThrow(TypeError)
 
+    const _warn = console.warn
+    console.warn = jest.fn()
+
     expect(() => {
       console.log(
         evaluateTyped(
@@ -27,6 +30,12 @@ describe('evaluateTyped(expectedTypes, context, value)', () => {
         )
       )
     }).toThrow(TypeError)
+
+    expect(console.warn).toHaveBeenCalledWith(
+      'Possible missing expression error: ["$someUnknownExpression"]. ' +
+        "No interpreter was found for '$someUnknownExpression'"
+    )
+    console.warn = _warn
   })
 
   test('array object type', () => {
diff --git a/src/expression.ts b/src/expression.ts
index 35ce241..8dd3f25 100644
--- a/src/expression.ts
+++ b/src/expression.ts
@@ -18,13 +18,12 @@ export const isExpression = (
   Array.isArray(candidateExpression) &&
   typeof interpreters[candidateExpression[0]] === 'function'
 
-/**
- * @function evaluate
- * @param {EvaluationContext} context
- * @param {Expression | *} expOrValue
- * @returns {*}
- */
-export const evaluate = (
+const _maybeExpression = (value) =>
+  Array.isArray(value) &&
+  typeof value[0] === 'string' &&
+  value[0].startsWith('$')
+
+const _evaluateProd = (
   context: EvaluationContext,
   expOrValue: Expression | any
 ): any => {
@@ -38,6 +37,38 @@ export const evaluate = (
   return interpreter(context, ...interpreterArgs)
 }
 
+const _ellipsis = (str, maxlen = 50) =>
+  str.length > maxlen ? str.substr(0, maxlen - 1).concat('...') : str
+
+const _evaluateDev = (
+  context: EvaluationContext,
+  expOrValue: Expression | any
+): any => {
+  if (
+    !isExpression(context.interpreters, expOrValue) &&
+    _maybeExpression(expOrValue)
+  ) {
+    console.warn(
+      `Possible missing expression error: ${_ellipsis(
+        JSON.stringify(expOrValue)
+      )}. No interpreter was found for '${expOrValue[0]}'`
+    )
+  }
+
+  return _evaluateProd(context, expOrValue)
+}
+
+/**
+ * @function evaluate
+ * @param {EvaluationContext} context
+ * @param {Expression | *} expOrValue
+ * @returns {*}
+ */
+export const evaluate =
+  process && process.env && process.env.NODE_ENV !== 'production'
+    ? _evaluateDev
+    : _evaluateProd
+
 /**
  * @function evaluateTyped
  * @param {String | string[]} expectedTypes
diff --git a/src/expressions/logical.spec.ts b/src/expressions/logical.spec.ts
index 09a8307..4db673f 100644
--- a/src/expressions/logical.spec.ts
+++ b/src/expressions/logical.spec.ts
@@ -155,21 +155,35 @@ describe('$and', () => {
   })
 
   // eslint-disable-next-line jest/no-disabled-tests
-  test.skip('value coercion on unknown expressions', () => {
-    expect(() => {
-      evaluate(
-        {
-          interpreters,
-          scope: {
-            $$VALUE: [
-              ['$unknownExpression', 1, 2],
-              ['$anotherUnknownExpression', null],
-            ],
-          },
+  test('value coercion on unknown expressions / expression-like values should trigger warning', () => {
+    const warn_ = console.warn
+    console.warn = jest.fn()
+
+    evaluate(
+      {
+        interpreters,
+        scope: {
+          $$VALUE: [
+            ['$unknownExpression', 1, 2],
+            ['$anotherUnknownExpression', null],
+          ],
         },
-        ['$and']
-      )
-    }).toThrow(TypeError)
+      },
+      ['$and']
+    )
+
+    expect(console.warn).toHaveBeenNthCalledWith(
+      1,
+      'Possible missing expression error: ["$unknownExpression",1,2]. ' +
+        "No interpreter was found for '$unknownExpression'"
+    )
+    expect(console.warn).toHaveBeenNthCalledWith(
+      2,
+      'Possible missing expression error: ["$anotherUnknownExpression",null]. ' +
+        "No interpreter was found for '$anotherUnknownExpression'"
+    )
+
+    console.warn = warn_
   })
 
   test('w/ comparison', () => {
diff --git a/src/expressions/logical.ts b/src/expressions/logical.ts
index e1b49e8..a3a0868 100644
--- a/src/expressions/logical.ts
+++ b/src/expressions/logical.ts
@@ -2,10 +2,6 @@ import { evaluate, interpreter } from '../expression'
 import { Expression, EvaluationContext, PlainObject } from '../types'
 
 /**
- * @todo logical Better handle unknown expressions for boolean logical operators
- *               Uninterpreted expressions are returned as simple arrays, which
- *               in turn return mistaken true results
- *
  * @function $and
  * @param {Array} expressionsExp
  * @returns {Boolean}
diff --git a/todo.md b/todo.md
index 914bb85..8b13789 100644
--- a/todo.md
+++ b/todo.md
@@ -1,3 +1 @@
-- logical | Better handle unknown expressions for boolean logical operators
-              Uninterpreted expressions are returned as simple arrays, which
-              in turn return mistaken true results (src/expressions/logical.ts)
+