diff --git a/checker/src/main/java/dev/cel/checker/ExprChecker.java b/checker/src/main/java/dev/cel/checker/ExprChecker.java index 34d0faf81..12f725f09 100644 --- a/checker/src/main/java/dev/cel/checker/ExprChecker.java +++ b/checker/src/main/java/dev/cel/checker/ExprChecker.java @@ -652,20 +652,53 @@ private OverloadResolution resolveOverload( } } if (resultType == null) { - env.reportError( - callExprId, - position, - "found no matching overload for '%s' applied to '%s'%s", - function.name(), - expectedString, - excludedCands.isEmpty() - ? "" - : " (candidates: " + Joiner.on(',').join(excludedCands) + ")"); + // 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) { + env.reportError( + callExprId, + position, + "found no matching overload for '%s' applied to '%s'%s", + function.name(), + expectedString, + excludedCands.isEmpty() + ? "" + : " (candidates: " + Joiner.on(',').join(excludedCands) + ")"); + } resultType = SimpleType.ERROR; } return OverloadResolution.of(refBuilder.build(), resultType); } + /** + * Reports a type mismatch for each operand of a logical operator ({@code _&&_}, {@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())) { + return false; + } + + boolean reported = false; + for (CelMutableExpr arg : args) { + CelType argType = env.getType(arg); + if (!inferenceContext.isAssignable(argType, SimpleType.BOOL)) { + env.reportError( + arg.id(), + getPosition(arg), + "expected type '%s' but found '%s'", + CelTypes.format(SimpleType.BOOL), + CelTypes.format(inferenceContext.specialize(argType))); + reported = true; + } + } + return reported; + } + // Return value from visit is not needed as the subtree is not rewritten here. @SuppressWarnings("CheckReturnValue") private CelType visitSelectField( diff --git a/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java b/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java index a86eaa008..d29fc599c 100644 --- a/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java +++ b/checker/src/test/java/dev/cel/checker/ExprCheckerTest.java @@ -175,6 +175,29 @@ public void operatorsConditional() throws Exception { runTest(); } + @Test + public void operatorsBoolErrors() throws Exception { + source = "1 && true"; + runTest(); + + source = "'a' || 1"; + runTest(); + + // Bool operands still match no overload if _&&_ has no bool overload. + declareFunction( + "_&&_", + globalOverload( + "logical_and_int", ImmutableList.of(SimpleType.INT, SimpleType.INT), SimpleType.INT)); + source = "true && false"; + prepareCompiler(ProtoMessageTypeProvider.newBuilder().build()); + celCompiler = + celCompiler + .toCompilerBuilder() + .setStandardDeclarations(CelStandardDeclarations.EMPTY) + .build(); + testOutput().println(celCompiler.compile(source, testSourceDescription()).getErrorString()); + } + // Name References // =============== diff --git a/checker/src/test/resources/operatorsBoolErrors.baseline b/checker/src/test/resources/operatorsBoolErrors.baseline new file mode 100644 index 000000000..9f357b3bf --- /dev/null +++ b/checker/src/test/resources/operatorsBoolErrors.baseline @@ -0,0 +1,23 @@ +Source: 1 && true +=====> +ERROR: test_location:1:1: expected type 'bool' but found 'int' + | 1 && true + | ^ + +Source: 'a' || 1 +=====> +ERROR: test_location:1:1: expected type 'bool' but found 'string' + | 'a' || 1 + | ^ +ERROR: test_location:1:8: expected type 'bool' but found 'int' + | 'a' || 1 + | .......^ + +Source: true && false +declare _&&_ { + function logical_and_int (int, int) -> int +} +=====> +ERROR: test_location:1:6: found no matching overload for '_&&_' applied to '(bool, bool)' (candidates: (int, int)) + | true && false + | .....^ diff --git a/checker/src/test/resources/quantifiersErrors.baseline b/checker/src/test/resources/quantifiersErrors.baseline index 17c1736c5..b6272cd30 100644 --- a/checker/src/test/resources/quantifiersErrors.baseline +++ b/checker/src/test/resources/quantifiersErrors.baseline @@ -6,6 +6,6 @@ declare x { ERROR: test_location:1:1: expression of type 'cel.expr.conformance.proto3.TestAllTypes' cannot be range of a comprehension (must be list, map, or dynamic) | x.all(e, 0) | ^ -ERROR: test_location:1:6: found no matching overload for '_&&_' applied to '(bool, int)' (candidates: (bool, bool)) +ERROR: test_location:1:10: expected type 'bool' but found 'int' | x.all(e, 0) - | .....^ + | .........^