Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions checker/src/main/java/dev/cel/checker/ExprChecker.java
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<CelMutableExpr> args) {
if (!functionName.equals(Operator.LOGICAL_AND.getFunction())
&& !functionName.equals(Operator.LOGICAL_OR.getFunction())) {
private boolean reportBoolOperandsErrors(String functionName, List<CelMutableExpr> args) {
List<CelMutableExpr> 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(
Expand Down
11 changes: 11 additions & 0 deletions checker/src/test/java/dev/cel/checker/ExprCheckerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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(
"_&&_",
Expand Down
18 changes: 18 additions & 0 deletions checker/src/test/resources/operatorsBoolErrors.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading