diff --git a/checker/src/main/java/dev/cel/checker/ExprChecker.java b/checker/src/main/java/dev/cel/checker/ExprChecker.java index 12f725f09..642577e49 100644 --- a/checker/src/main/java/dev/cel/checker/ExprChecker.java +++ b/checker/src/main/java/dev/cel/checker/ExprChecker.java @@ -652,10 +652,11 @@ private OverloadResolution resolveOverload( } } if (resultType == null) { - // Non-bool operands of the logical operators are reported individually, as this is more - // precise than reporting that no overload matches. - boolean reportedLogicalOperandsErrors = reportLogicalOperandsErrors(function.name(), args); - if (!reportedLogicalOperandsErrors) { + // Non-bool operands of the logical operators and non-bool conditions of the conditional + // operator are reported individually, as this is more precise than reporting that no + // overload matches. + boolean reportedBoolOperandsErrors = reportBoolOperandsErrors(function.name(), args); + if (!reportedBoolOperandsErrors) { env.reportError( callExprId, position, @@ -672,19 +673,25 @@ private OverloadResolution resolveOverload( } /** - * Reports a type mismatch for each operand of a logical operator ({@code _&&_}, {@code _||_}) - * which is not a {@code bool}. + * Reports a type mismatch for each operand of a logical operator ({@code _&&_}, {@code _||_}), + * and for the condition of the conditional operator ({@code _?_:_}), which is not a {@code bool}. * * @return whether any error was reported. */ - private boolean reportLogicalOperandsErrors(String functionName, List args) { - if (!functionName.equals(Operator.LOGICAL_AND.getFunction()) - && !functionName.equals(Operator.LOGICAL_OR.getFunction())) { + private boolean reportBoolOperandsErrors(String functionName, List args) { + List boolArgs; + if (functionName.equals(Operator.LOGICAL_AND.getFunction()) + || functionName.equals(Operator.LOGICAL_OR.getFunction())) { + boolArgs = args; + } else if (functionName.equals(Operator.CONDITIONAL.getFunction())) { + // Only the condition must be a bool: the branches can have any type. + boolArgs = args.subList(0, 1); + } else { return false; } boolean reported = false; - for (CelMutableExpr arg : args) { + for (CelMutableExpr arg : boolArgs) { CelType argType = env.getType(arg); if (!inferenceContext.isAssignable(argType, SimpleType.BOOL)) { env.reportError( diff --git a/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java b/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java index d29fc599c..824ea57ca 100644 --- a/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java +++ b/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java @@ -183,6 +183,17 @@ public void operatorsBoolErrors() throws Exception { source = "'a' || 1"; runTest(); + source = "1 ? 'a' : 'b'"; + runTest(); + + // The condition is a bool, but the branches don't agree on a type. + source = "true ? 'a' : 1"; + runTest(); + + // The filter macro expands its predicate into the condition of a conditional. + source = "[1, 2].filter(x, x + 1)"; + runTest(); + // Bool operands still match no overload if _&&_ has no bool overload. declareFunction( "_&&_", diff --git a/checker/src/test/resources/operatorsBoolErrors.baseline b/checker/src/test/resources/operatorsBoolErrors.baseline index 9f357b3bf..50ae4493f 100644 --- a/checker/src/test/resources/operatorsBoolErrors.baseline +++ b/checker/src/test/resources/operatorsBoolErrors.baseline @@ -13,6 +13,24 @@ ERROR: test_location:1:8: expected type 'bool' but found 'int' | 'a' || 1 | .......^ +Source: 1 ? 'a' : 'b' +=====> +ERROR: test_location:1:1: expected type 'bool' but found 'int' + | 1 ? 'a' : 'b' + | ^ + +Source: true ? 'a' : 1 +=====> +ERROR: test_location:1:6: found no matching overload for '_?_:_' applied to '(bool, string, int)' (candidates: (bool, %A0, %A0)) + | true ? 'a' : 1 + | .....^ + +Source: [1, 2].filter(x, x + 1) +=====> +ERROR: test_location:1:20: expected type 'bool' but found 'int' + | [1, 2].filter(x, x + 1) + | ...................^ + Source: true && false declare _&&_ { function logical_and_int (int, int) -> int diff --git a/extensions/src/test/java/dev/cel/extensions/CelComprehensionsExtensionsTest.java b/extensions/src/test/java/dev/cel/extensions/CelComprehensionsExtensionsTest.java index 9d57f6fbe..b263e7603 100644 --- a/extensions/src/test/java/dev/cel/extensions/CelComprehensionsExtensionsTest.java +++ b/extensions/src/test/java/dev/cel/extensions/CelComprehensionsExtensionsTest.java @@ -307,7 +307,8 @@ public void unparseAST_twoVarComprehension( @TestParameters( "{expr: '{}.transformMapEntry(i.j, k, {k: i.j})', err: 'argument must be a simple name'}") @TestParameters( - "{expr: '{}.transformMapEntry(j, k, \"bad filter\", {k: j})', err: 'no matching overload'}") + "{expr: '{}.transformMapEntry(j, k, \"bad filter\", {k: j})', err: \"expected type 'bool'" + + " but found 'string'\"}") @TestParameters( "{expr: '[1, 2].transformList(i, v, v % 2 == 0 ? [v] : v)', err: 'no matching overload'}") @TestParameters(