Skip to content

Commit fc2d325

Browse files
Cristina Borzacopybara-github
authored andcommitted
Report a non-bool condition of the conditional operator as a type mismatch.
The `filter`, `exists_one` and `map` with a filter macros expand their predicate into the condition of `_?_:_`. When the predicate isn't a bool the type-checker reports `found no matching overload for '_?_:_' applied to ...'`, which points at an operator that doesn't appear in the expression. Report `expected type 'bool' but found '<type>'` at the condition instead, as is already done for the operands of `&&` and `||`. PiperOrigin-RevId: 1003153382
1 parent 9f35821 commit fc2d325

4 files changed

Lines changed: 48 additions & 11 deletions

File tree

‎checker/src/main/java/dev/cel/checker/ExprChecker.java‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -652,10 +652,11 @@ private OverloadResolution resolveOverload(
652652
}
653653
}
654654
if (resultType == null) {
655-
// Non-bool operands of the logical operators are reported individually, as this is more
656-
// precise than reporting that no overload matches.
657-
boolean reportedLogicalOperandsErrors = reportLogicalOperandsErrors(function.name(), args);
658-
if (!reportedLogicalOperandsErrors) {
655+
// Non-bool operands of the logical operators and non-bool conditions of the conditional
656+
// operator are reported individually, as this is more precise than reporting that no
657+
// overload matches.
658+
boolean reportedBoolOperandsErrors = reportBoolOperandsErrors(function.name(), args);
659+
if (!reportedBoolOperandsErrors) {
659660
env.reportError(
660661
callExprId,
661662
position,
@@ -672,19 +673,25 @@ private OverloadResolution resolveOverload(
672673
}
673674

674675
/**
675-
* Reports a type mismatch for each operand of a logical operator ({@code _&&_}, {@code _||_})
676-
* which is not a {@code bool}.
676+
* Reports a type mismatch for each operand of a logical operator ({@code _&&_}, {@code _||_}),
677+
* and for the condition of the conditional operator ({@code _?_:_}), which is not a {@code bool}.
677678
*
678679
* @return whether any error was reported.
679680
*/
680-
private boolean reportLogicalOperandsErrors(String functionName, List<CelMutableExpr> args) {
681-
if (!functionName.equals(Operator.LOGICAL_AND.getFunction())
682-
&& !functionName.equals(Operator.LOGICAL_OR.getFunction())) {
681+
private boolean reportBoolOperandsErrors(String functionName, List<CelMutableExpr> args) {
682+
List<CelMutableExpr> boolArgs;
683+
if (functionName.equals(Operator.LOGICAL_AND.getFunction())
684+
|| functionName.equals(Operator.LOGICAL_OR.getFunction())) {
685+
boolArgs = args;
686+
} else if (functionName.equals(Operator.CONDITIONAL.getFunction())) {
687+
// Only the condition must be a bool: the branches can have any type.
688+
boolArgs = args.subList(0, 1);
689+
} else {
683690
return false;
684691
}
685692

686693
boolean reported = false;
687-
for (CelMutableExpr arg : args) {
694+
for (CelMutableExpr arg : boolArgs) {
688695
CelType argType = env.getType(arg);
689696
if (!inferenceContext.isAssignable(argType, SimpleType.BOOL)) {
690697
env.reportError(

‎checker/src/test/java/dev/cel/checker/ExprCheckerTest.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,17 @@ public void operatorsBoolErrors() throws Exception {
183183
source = "'a' || 1";
184184
runTest();
185185

186+
source = "1 ? 'a' : 'b'";
187+
runTest();
188+
189+
// The condition is a bool, but the branches don't agree on a type.
190+
source = "true ? 'a' : 1";
191+
runTest();
192+
193+
// The filter macro expands its predicate into the condition of a conditional.
194+
source = "[1, 2].filter(x, x + 1)";
195+
runTest();
196+
186197
// Bool operands still match no overload if _&&_ has no bool overload.
187198
declareFunction(
188199
"_&&_",

‎checker/src/test/resources/operatorsBoolErrors.baseline‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,24 @@ ERROR: test_location:1:8: expected type 'bool' but found 'int'
1313
| 'a' || 1
1414
| .......^
1515

16+
Source: 1 ? 'a' : 'b'
17+
=====>
18+
ERROR: test_location:1:1: expected type 'bool' but found 'int'
19+
| 1 ? 'a' : 'b'
20+
| ^
21+
22+
Source: true ? 'a' : 1
23+
=====>
24+
ERROR: test_location:1:6: found no matching overload for '_?_:_' applied to '(bool, string, int)' (candidates: (bool, %A0, %A0))
25+
| true ? 'a' : 1
26+
| .....^
27+
28+
Source: [1, 2].filter(x, x + 1)
29+
=====>
30+
ERROR: test_location:1:20: expected type 'bool' but found 'int'
31+
| [1, 2].filter(x, x + 1)
32+
| ...................^
33+
1634
Source: true && false
1735
declare _&&_ {
1836
function logical_and_int (int, int) -> int

‎extensions/src/test/java/dev/cel/extensions/CelComprehensionsExtensionsTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,8 @@ public void unparseAST_twoVarComprehension(
307307
@TestParameters(
308308
"{expr: '{}.transformMapEntry(i.j, k, {k: i.j})', err: 'argument must be a simple name'}")
309309
@TestParameters(
310-
"{expr: '{}.transformMapEntry(j, k, \"bad filter\", {k: j})', err: 'no matching overload'}")
310+
"{expr: '{}.transformMapEntry(j, k, \"bad filter\", {k: j})', err: \"expected type 'bool'"
311+
+ " but found 'string'\"}")
311312
@TestParameters(
312313
"{expr: '[1, 2].transformList(i, v, v % 2 == 0 ? [v] : v)', err: 'no matching overload'}")
313314
@TestParameters(

0 commit comments

Comments
 (0)