fix early type checks for comparison/logical ops in binaryOpNode

src/strands/ir_builders.js

@@ -135,6 +135,9 @@ export function binaryOpNode(
     );
     rightType = DAG.extractNodeTypeInfo(dag, rightStrandsNode.id);
   }
+  // Snapshot the pre-cast types for error messages.
+  const preCastLeftType = { ...leftType };
+  const preCastRightType = { ...rightType };
   const cast = { node: null, toType: leftType };
   const bothDeferred =
     leftType.baseType === rightType.baseType &&
@@ -214,6 +217,65 @@ export function binaryOpNode(
     }
   }
 
+  const leftDim = dag.dimensions[finalLeftNodeID];
+  const rightDim = dag.dimensions[finalRightNodeID];
+  const leftBase = dag.baseTypes[finalLeftNodeID];
+  const rightBase = dag.baseTypes[finalRightNodeID];
+
+  // DEFER carries no user-meaningful type, so fall back to the
+  // resolved post-cast types there.
+  const displayLeftBase = preCastLeftType.baseType === BaseType.DEFER ? leftBase : preCastLeftType.baseType;
+  const displayLeftDim = preCastLeftType.baseType === BaseType.DEFER ? leftDim : preCastLeftType.dimension;
+  const displayRightBase = preCastRightType.baseType === BaseType.DEFER ? rightBase : preCastRightType.baseType;
+  const displayRightDim = preCastRightType.baseType === BaseType.DEFER ? rightDim : preCastRightType.dimension;
+
+  const isOrdering = [
+    OpCode.Binary.LESS_THAN,
+    OpCode.Binary.LESS_EQUAL,
+    OpCode.Binary.GREATER_THAN,
+    OpCode.Binary.GREATER_EQUAL,
+  ].includes(opCode);
+  const isEquality = opCode === OpCode.Binary.EQUAL || opCode === OpCode.Binary.NOT_EQUAL;
+  const isLogical  = opCode === OpCode.Binary.LOGICAL_AND || opCode === OpCode.Binary.LOGICAL_OR;
+
+  if (isOrdering) {
+    if (leftDim > 1 || rightDim > 1) {
+      FES.userError(
+        'type error',
+        `${OpCodeToSymbol[opCode]} is only defined for scalars. ` +
+        `Got ${displayLeftBase}${displayLeftDim} ${OpCodeToSymbol[opCode]} ${displayRightBase}${displayRightDim}.`
+      );
+    } else if (leftBase === BaseType.BOOL || rightBase === BaseType.BOOL) {
+      FES.userError(
+        'type error',
+        `${OpCodeToSymbol[opCode]} is not defined for boolean values. ` +
+        `Got ${displayLeftBase}${displayLeftDim} ${OpCodeToSymbol[opCode]} ${displayRightBase}${displayRightDim}.`
+      );
+    }
+  } else if (isEquality) {
+    if ((leftDim > 1 || rightDim > 1) && (leftDim !== rightDim || leftBase !== rightBase)) {
+      FES.userError(
+        'type error',
+        `Equality comparisons between vectors require matching dimensions and base types. ` +
+        `Got ${displayLeftBase}${displayLeftDim} ${OpCodeToSymbol[opCode]} ${displayRightBase}${displayRightDim}.`
+      );
+    } else if ((leftBase === BaseType.BOOL) !== (rightBase === BaseType.BOOL)) {
+      FES.userError(
+        'type error',
+        `Equality comparisons between boolean and numeric types are not allowed. ` +
+        `Got ${displayLeftBase}${displayLeftDim} ${OpCodeToSymbol[opCode]} ${displayRightBase}${displayRightDim}.`
+      );
+    }
+  } else if (isLogical) {
+    if (leftBase !== BaseType.BOOL || rightBase !== BaseType.BOOL || leftDim !== 1 || rightDim !== 1) {
+      FES.userError(
+        'type error',
+        `${OpCodeToSymbol[opCode]} requires two bool scalars. ` +
+        `Got ${displayLeftBase}${displayLeftDim} ${OpCodeToSymbol[opCode]} ${displayRightBase}${displayRightDim}.`
+      );
+    }
+  }
+
   if (booleanOpCode[opCode]) {
     cast.toType.baseType = BaseType.BOOL;
     cast.toType.dimension = 1;

test/unit/webgl/p5.Shader.js

@@ -3816,6 +3816,254 @@ suite('p5.Shader', function () {
       assert.include(errMsg, 'float4');
     });
 
+    test('ordering comparison with a vector operand throws a clear strands type error', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      try {
+        myp5.baseMaterialShader().modify(
+          () => {
+            myp5.getFinalColor(color => {
+              if (color < 2) {
+                color = [1, 1, 1, 1];
+              }
+              return color;
+            });
+          },
+          { myp5 }
+        );
+      } catch {
+        /* expected */
+      }
+
+      assert.isAbove(
+        mockUserError.mock.calls.length,
+        0,
+        'FES.userError should have been called'
+      );
+      const errMsg = mockUserError.mock.calls[0][1];
+      assert.include(errMsg, '<');
+      assert.include(errMsg, 'only defined for scalars');
+      assert.include(errMsg, 'float4');
+      assert.include(errMsg, 'float1');
+    });
+
+    test('ordering comparison between scalars is allowed', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      myp5.baseMaterialShader().modify(
+        () => {
+          myp5.getFinalColor(color => {
+            if (color.r < 0.5) {
+              color = [1, 1, 1, 1];
+            }
+            return color;
+          });
+        },
+        { myp5 }
+      );
+
+      assert.equal(mockUserError.mock.calls.length, 0);
+    });
+
+    test('equality comparison between matching vectors is allowed', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      myp5.baseMaterialShader().modify(
+        () => {
+          myp5.getFinalColor(color => {
+            if (color.equalTo([1, 1, 1, 1])) {
+              color = [1, 1, 1, 1];
+            }
+            return color;
+          });
+        },
+        { myp5 }
+      );
+
+      assert.equal(mockUserError.mock.calls.length, 0);
+    });
+
+    test('logical and with non-boolean operands throws a clear strands type error', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      try {
+        myp5.baseMaterialShader().modify(
+          () => {
+            myp5.getFinalColor(color => {
+              if (color.r && color.g) {
+                color = [1, 1, 1, 1];
+              }
+              return color;
+            });
+          },
+          { myp5 }
+        );
+      } catch {
+        /* expected */
+      }
+
+      assert.isAbove(
+        mockUserError.mock.calls.length,
+        0,
+        'FES.userError should have been called'
+      );
+      const errMsg = mockUserError.mock.calls[0][1];
+      assert.include(errMsg, '&&');
+      assert.include(errMsg, 'requires two bool scalars');
+    });
+
+    test('logical and between two boolean scalars is allowed', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      myp5.baseMaterialShader().modify(
+        () => {
+          myp5.getFinalColor(color => {
+            if (color.r < 0.5 && color.g > 0.5) {
+              color = [1, 1, 1, 1];
+            }
+            return color;
+          });
+        },
+        { myp5 }
+      );
+
+      assert.equal(mockUserError.mock.calls.length, 0);
+    });
+
+    test('ordering comparison with a boolean operand throws a clear strands type error', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      try {
+        myp5.baseMaterialShader().modify(
+          () => {
+            myp5.getFinalColor(color => {
+              if (myp5.bool(true) < 0.5) {
+                color = [1, 1, 1, 1];
+              }
+              return color;
+            });
+          },
+          { myp5 }
+        );
+      } catch {
+        /* expected */
+      }
+
+      assert.isAbove(
+        mockUserError.mock.calls.length,
+        0,
+        'FES.userError should have been called'
+      );
+      const errMsg = mockUserError.mock.calls[0][1];
+      assert.include(errMsg, '<');
+      assert.include(errMsg, 'not defined for boolean values');
+    });
+
+    test('ordering comparison between two booleans throws a clear strands type error', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      try {
+        myp5.baseMaterialShader().modify(
+          () => {
+            myp5.getFinalColor(color => {
+              if (myp5.bool(true) > myp5.bool(false)) {
+                color = [1, 1, 1, 1];
+              }
+              return color;
+            });
+          },
+          { myp5 }
+        );
+      } catch {
+        /* expected */
+      }
+
+      assert.isAbove(
+        mockUserError.mock.calls.length,
+        0,
+        'FES.userError should have been called'
+      );
+      const errMsg = mockUserError.mock.calls[0][1];
+      assert.include(errMsg, '>');
+      assert.include(errMsg, 'not defined for boolean values');
+    });
+
+    test('equality comparison between boolean and numeric types throws a clear strands type error', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      try {
+        myp5.baseMaterialShader().modify(
+          () => {
+            myp5.getFinalColor(color => {
+              if (myp5.bool(true).equalTo(1.0)) {
+                color = [1, 1, 1, 1];
+              }
+              return color;
+            });
+          },
+          { myp5 }
+        );
+      } catch {
+        /* expected */
+      }
+
+      assert.isAbove(
+        mockUserError.mock.calls.length,
+        0,
+        'FES.userError should have been called'
+      );
+      const errMsg = mockUserError.mock.calls[0][1];
+      assert.include(errMsg, '==');
+      assert.include(errMsg, 'between boolean and numeric');
+    });
+
+    test('inequality comparison between numeric and boolean types throws a clear strands type error', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      try {
+        myp5.baseMaterialShader().modify(
+          () => {
+            myp5.getFinalColor(color => {
+              if (myp5.float(1.0).notEqual(myp5.bool(false))) {
+                color = [1, 1, 1, 1];
+              }
+              return color;
+            });
+          },
+          { myp5 }
+        );
+      } catch {
+        /* expected */
+      }
+
+      assert.isAbove(
+        mockUserError.mock.calls.length,
+        0,
+        'FES.userError should have been called'
+      );
+      const errMsg = mockUserError.mock.calls[0][1];
+      assert.include(errMsg, '!=');
+      assert.include(errMsg, 'between boolean and numeric');
+    });
+
+    test('equality comparison between two booleans is allowed', () => {
+      myp5.createCanvas(50, 50, myp5.WEBGL);
+
+      myp5.baseMaterialShader().modify(
+        () => {
+          myp5.getFinalColor(color => {
+            if (myp5.bool(true).equalTo(myp5.bool(false))) {
+              color = [1, 1, 1, 1];
+            }
+            return color;
+          });
+        },
+        { myp5 }
+      );
+
+      assert.equal(mockUserError.mock.calls.length, 0);
+    });
+
     test('shows a helpful error for web editor loop protection', () => {
       myp5.createCanvas(50, 50, myp5.WEBGL);