From 61a01d800b24dfb79bc7035119166d2822f3ff1b Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Wed, 22 Apr 2026 15:44:18 -0700 Subject: [PATCH] Change function failures (dispatch / overload match) to always include the function name PiperOrigin-RevId: 904088792 --- .../extensions/CelListsExtensionsTest.java | 3 +- .../src/main/java/dev/cel/runtime/BUILD.bazel | 4 +- .../dev/cel/runtime/CelFunctionBinding.java | 1 + .../cel/runtime/CelLateFunctionBindings.java | 5 +++ .../dev/cel/runtime/CelResolvedOverload.java | 26 ++++++++--- .../java/dev/cel/runtime/CelRuntimeImpl.java | 10 +++++ .../dev/cel/runtime/CelRuntimeLegacyImpl.java | 10 +++++ .../dev/cel/runtime/DefaultDispatcher.java | 23 +++++++--- .../dev/cel/runtime/FunctionBindingImpl.java | 35 +++++++++++++-- .../runtime/InternalCelFunctionBinding.java | 29 ++++++++++++ .../java/dev/cel/runtime/LiteRuntimeImpl.java | 15 +++++-- .../dev/cel/runtime/planner/EvalBinary.java | 9 +++- .../dev/cel/runtime/planner/EvalHelpers.java | 19 ++++++-- .../runtime/planner/EvalLateBoundCall.java | 2 +- .../dev/cel/runtime/planner/EvalUnary.java | 8 +++- .../cel/runtime/planner/EvalVarArgsCall.java | 8 +++- .../cel/runtime/planner/EvalZeroArity.java | 16 +++++-- .../cel/runtime/planner/ProgramPlanner.java | 14 ++++-- .../src/test/java/dev/cel/runtime/BUILD.bazel | 1 + .../cel/runtime/CelResolvedOverloadTest.java | 45 ++++++++++++------- .../cel/runtime/DefaultDispatcherTest.java | 12 ++++- .../cel/runtime/DefaultInterpreterTest.java | 10 ++++- .../cel/runtime/PlannerInterpreterTest.java | 15 ++++--- .../runtime/planner/ProgramPlannerTest.java | 18 +++----- .../planner_optional_errors.baseline | 5 +++ 25 files changed, 266 insertions(+), 77 deletions(-) create mode 100644 runtime/src/main/java/dev/cel/runtime/InternalCelFunctionBinding.java create mode 100644 runtime/src/test/resources/planner_optional_errors.baseline diff --git a/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java b/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java index f5536da4e..4520f81ba 100644 --- a/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java +++ b/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java @@ -29,6 +29,7 @@ import dev.cel.expr.conformance.test.SimpleTest; import dev.cel.parser.CelStandardMacro; import dev.cel.runtime.CelEvaluationException; +import dev.cel.testing.CelRuntimeFlavor; import org.junit.Assume; import org.junit.Test; import org.junit.runner.RunWith; @@ -143,7 +144,7 @@ public void flatten_negativeDepth_throws() { CelEvaluationException e = assertThrows(CelEvaluationException.class, () -> eval(cel, "[1,2,3,4].flatten(-1)")); - if (isParseOnly) { + if (runtimeFlavor.equals(CelRuntimeFlavor.PLANNER)) { assertThat(e) .hasMessageThat() .contains("evaluation error at :17: Function 'flatten' failed"); diff --git a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel index 6f0607de4..ef0ac71d4 100644 --- a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel +++ b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel @@ -53,6 +53,7 @@ LITE_PROGRAM_IMPL_SOURCES = [ FUNCTION_BINDING_SOURCES = [ "CelFunctionBinding.java", "FunctionBindingImpl.java", + "InternalCelFunctionBinding.java", ] # keep sorted @@ -740,6 +741,7 @@ java_library( deps = [ ":evaluation_exception", ":function_overload", + "//common/annotations", "//common/exceptions:overload_not_found", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", @@ -754,6 +756,7 @@ cel_android_library( deps = [ ":evaluation_exception", ":function_overload_android", + "//common/annotations", "//common/exceptions:overload_not_found", "@maven//:com_google_errorprone_error_prone_annotations", "@maven_android//:com_google_guava_guava", @@ -890,7 +893,6 @@ java_library( "//common/types:type_providers", "//common/values:cel_value_provider", "//common/values:proto_message_value_provider", - "//runtime/standard:add", "//runtime/standard:int", "//runtime/standard:timestamp", "@maven//:com_google_code_findbugs_annotations", diff --git a/runtime/src/main/java/dev/cel/runtime/CelFunctionBinding.java b/runtime/src/main/java/dev/cel/runtime/CelFunctionBinding.java index 88be0d3c3..98991d383 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelFunctionBinding.java +++ b/runtime/src/main/java/dev/cel/runtime/CelFunctionBinding.java @@ -100,6 +100,7 @@ static CelFunctionBinding from( overloadId, ImmutableList.copyOf(argTypes), impl, /* isStrict= */ true); } + /** See {@link #fromOverloads(String, Collection)}. */ static ImmutableSet fromOverloads( String functionName, CelFunctionBinding... overloadBindings) { diff --git a/runtime/src/main/java/dev/cel/runtime/CelLateFunctionBindings.java b/runtime/src/main/java/dev/cel/runtime/CelLateFunctionBindings.java index 3d75845cf..2da08120c 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelLateFunctionBindings.java +++ b/runtime/src/main/java/dev/cel/runtime/CelLateFunctionBindings.java @@ -63,7 +63,12 @@ public static CelLateFunctionBindings from(Collection functi } private static CelResolvedOverload createResolvedOverload(CelFunctionBinding binding) { + String functionName = binding.getOverloadId(); + if (binding instanceof InternalCelFunctionBinding) { + functionName = ((InternalCelFunctionBinding) binding).getFunctionName(); + } return CelResolvedOverload.of( + functionName, binding.getOverloadId(), binding.getDefinition(), binding.isStrict(), diff --git a/runtime/src/main/java/dev/cel/runtime/CelResolvedOverload.java b/runtime/src/main/java/dev/cel/runtime/CelResolvedOverload.java index 7063720a1..fbe9a3289 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelResolvedOverload.java +++ b/runtime/src/main/java/dev/cel/runtime/CelResolvedOverload.java @@ -30,6 +30,9 @@ @Internal public abstract class CelResolvedOverload { + /** The base function name. */ + public abstract String getFunctionName(); + /** The overload id of the function. */ public abstract String getOverloadId(); @@ -61,7 +64,7 @@ public Object invoke(Object[] args) throws CelEvaluationException { || CelFunctionOverload.canHandle(args, getParameterTypes(), isStrict())) { return getDefinition().apply(args); } - throw new CelOverloadNotFoundException(getOverloadId()); + throw new CelOverloadNotFoundException(getFunctionName(), ImmutableList.of(getOverloadId())); } public Object invoke(Object arg) throws CelEvaluationException { @@ -69,7 +72,7 @@ public Object invoke(Object arg) throws CelEvaluationException { || CelFunctionOverload.canHandle(arg, getParameterTypes(), isStrict())) { return getOptimizedDefinition().apply(arg); } - throw new CelOverloadNotFoundException(getOverloadId()); + throw new CelOverloadNotFoundException(getFunctionName(), ImmutableList.of(getOverloadId())); } public Object invoke(Object arg1, Object arg2) throws CelEvaluationException { @@ -77,24 +80,28 @@ public Object invoke(Object arg1, Object arg2) throws CelEvaluationException { || CelFunctionOverload.canHandle(arg1, arg2, getParameterTypes(), isStrict())) { return getOptimizedDefinition().apply(arg1, arg2); } - throw new CelOverloadNotFoundException(getOverloadId()); + throw new CelOverloadNotFoundException(getFunctionName(), ImmutableList.of(getOverloadId())); } /** - * Creates a new resolved overload from the given overload id, parameter types, and definition. + * Creates a new resolved overload from the given function name, overload id, parameter types, and + * definition. */ public static CelResolvedOverload of( + String functionName, String overloadId, CelFunctionOverload definition, boolean isStrict, Class... parameterTypes) { - return of(overloadId, definition, isStrict, ImmutableList.copyOf(parameterTypes)); + return of(functionName, overloadId, definition, isStrict, ImmutableList.copyOf(parameterTypes)); } /** - * Creates a new resolved overload from the given overload id, parameter types, and definition. + * Creates a new resolved overload from the given function name, overload id, parameter types, and + * definition. */ public static CelResolvedOverload of( + String functionName, String overloadId, CelFunctionOverload definition, boolean isStrict, @@ -104,7 +111,12 @@ public static CelResolvedOverload of( ? (OptimizedFunctionOverload) definition : definition::apply; return new AutoValue_CelResolvedOverload( - overloadId, ImmutableList.copyOf(parameterTypes), isStrict, definition, optimizedDef); + functionName, + overloadId, + ImmutableList.copyOf(parameterTypes), + isStrict, + definition, + optimizedDef); } /** diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java index cab2c666e..43b223fa0 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntimeImpl.java @@ -381,7 +381,12 @@ private static DefaultDispatcher newDispatcher( DefaultDispatcher.Builder builder = DefaultDispatcher.newBuilder(); for (CelFunctionBinding binding : standardFunctions.newFunctionBindings(runtimeEquality, options)) { + String functionName = binding.getOverloadId(); + if (binding instanceof InternalCelFunctionBinding) { + functionName = ((InternalCelFunctionBinding) binding).getFunctionName(); + } builder.addOverload( + functionName, binding.getOverloadId(), binding.getArgTypes(), binding.isStrict(), @@ -389,7 +394,12 @@ private static DefaultDispatcher newDispatcher( } for (CelFunctionBinding binding : customFunctionBindings) { + String functionName = binding.getOverloadId(); + if (binding instanceof InternalCelFunctionBinding) { + functionName = ((InternalCelFunctionBinding) binding).getFunctionName(); + } builder.addOverload( + functionName, binding.getOverloadId(), binding.getArgTypes(), binding.isStrict(), diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java index 8ae4a9e3e..33702b2c6 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java @@ -305,7 +305,12 @@ public CelRuntimeLegacyImpl build() { DefaultDispatcher.Builder dispatcherBuilder = DefaultDispatcher.newBuilder(); for (CelFunctionBinding standardFunctionBinding : newStandardFunctionBindings(runtimeEquality)) { + String functionName = standardFunctionBinding.getOverloadId(); + if (standardFunctionBinding instanceof InternalCelFunctionBinding) { + functionName = ((InternalCelFunctionBinding) standardFunctionBinding).getFunctionName(); + } dispatcherBuilder.addOverload( + functionName, standardFunctionBinding.getOverloadId(), standardFunctionBinding.getArgTypes(), standardFunctionBinding.isStrict(), @@ -313,7 +318,12 @@ public CelRuntimeLegacyImpl build() { } for (CelFunctionBinding customBinding : customFunctionBindings.values()) { + String functionName = customBinding.getOverloadId(); + if (customBinding instanceof InternalCelFunctionBinding) { + functionName = ((InternalCelFunctionBinding) customBinding).getFunctionName(); + } dispatcherBuilder.addOverload( + functionName, customBinding.getOverloadId(), customBinding.getArgTypes(), customBinding.isStrict(), diff --git a/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java b/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java index d6ddf3965..0a467db81 100644 --- a/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java +++ b/runtime/src/main/java/dev/cel/runtime/DefaultDispatcher.java @@ -134,6 +134,8 @@ public static class Builder { @AutoValue @Immutable abstract static class OverloadEntry { + abstract String functionName(); + abstract ImmutableList> argTypes(); abstract boolean isStrict(); @@ -141,8 +143,12 @@ abstract static class OverloadEntry { abstract CelFunctionOverload overload(); private static OverloadEntry of( - ImmutableList> argTypes, boolean isStrict, CelFunctionOverload overload) { - return new AutoValue_DefaultDispatcher_Builder_OverloadEntry(argTypes, isStrict, overload); + String functionName, + ImmutableList> argTypes, + boolean isStrict, + CelFunctionOverload overload) { + return new AutoValue_DefaultDispatcher_Builder_OverloadEntry( + functionName, argTypes, isStrict, overload); } } @@ -150,16 +156,19 @@ private static OverloadEntry of( @CanIgnoreReturnValue public Builder addOverload( + String functionName, String overloadId, ImmutableList> argTypes, boolean isStrict, CelFunctionOverload overload) { + checkNotNull(functionName); + checkArgument(!functionName.isEmpty(), "Function name cannot be empty."); checkNotNull(overloadId); checkArgument(!overloadId.isEmpty(), "Overload ID cannot be empty."); checkNotNull(argTypes); checkNotNull(overload); - OverloadEntry newEntry = OverloadEntry.of(argTypes, isStrict, overload); + OverloadEntry newEntry = OverloadEntry.of(functionName, argTypes, isStrict, overload); overloads.merge( overloadId, @@ -188,7 +197,7 @@ private OverloadEntry mergeDynamicDispatchesOrThrow( boolean isStrict = mergedOverload.getOverloadBindings().stream().allMatch(CelFunctionBinding::isStrict); - return OverloadEntry.of(incoming.argTypes(), isStrict, mergedOverload); + return OverloadEntry.of(overloadId, incoming.argTypes(), isStrict, mergedOverload); } throw new IllegalArgumentException("Duplicate overload ID binding: " + overloadId); @@ -204,7 +213,11 @@ public DefaultDispatcher build() { resolvedOverloads.put( overloadId, CelResolvedOverload.of( - overloadId, overloadImpl, overloadEntry.isStrict(), overloadEntry.argTypes())); + overloadEntry.functionName(), + overloadId, + overloadImpl, + overloadEntry.isStrict(), + overloadEntry.argTypes())); } return new DefaultDispatcher(resolvedOverloads.buildOrThrow()); diff --git a/runtime/src/main/java/dev/cel/runtime/FunctionBindingImpl.java b/runtime/src/main/java/dev/cel/runtime/FunctionBindingImpl.java index c1306ce19..7b8efe8fd 100644 --- a/runtime/src/main/java/dev/cel/runtime/FunctionBindingImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/FunctionBindingImpl.java @@ -23,7 +23,9 @@ import dev.cel.common.exceptions.CelOverloadNotFoundException; @Immutable -final class FunctionBindingImpl implements CelFunctionBinding { +final class FunctionBindingImpl implements InternalCelFunctionBinding { + + private final String functionName; private final String overloadId; @@ -33,6 +35,11 @@ final class FunctionBindingImpl implements CelFunctionBinding { private final boolean isStrict; + @Override + public String getFunctionName() { + return functionName; + } + @Override public String getOverloadId() { return overloadId; @@ -54,20 +61,34 @@ public boolean isStrict() { } FunctionBindingImpl( + String functionName, String overloadId, ImmutableList> argTypes, CelFunctionOverload definition, boolean isStrict) { + this.functionName = functionName; this.overloadId = overloadId; this.argTypes = argTypes; this.definition = definition; this.isStrict = isStrict; } + FunctionBindingImpl( + String overloadId, + ImmutableList> argTypes, + CelFunctionOverload definition, + boolean isStrict) { + this(overloadId, overloadId, argTypes, definition, isStrict); + } + static ImmutableSet groupOverloadsToFunction( String functionName, ImmutableSet overloadBindings) { ImmutableSet.Builder builder = ImmutableSet.builder(); - builder.addAll(overloadBindings); + for (CelFunctionBinding b : overloadBindings) { + builder.add( + new FunctionBindingImpl( + functionName, b.getOverloadId(), b.getArgTypes(), b.getDefinition(), b.isStrict())); + } // If there is already a binding with the same name as the function, we treat it as a // "Singleton" binding and do not create a dynamic dispatch wrapper for it. @@ -80,11 +101,12 @@ static ImmutableSet groupOverloadsToFunction( CelFunctionBinding singleBinding = Iterables.getOnlyElement(overloadBindings); builder.add( new FunctionBindingImpl( + functionName, functionName, singleBinding.getArgTypes(), singleBinding.getDefinition(), singleBinding.isStrict())); - } else { + } else if (overloadBindings.size() > 1) { builder.add(new DynamicDispatchBinding(functionName, overloadBindings)); } } @@ -93,7 +115,7 @@ static ImmutableSet groupOverloadsToFunction( } @Immutable - static final class DynamicDispatchBinding implements CelFunctionBinding { + static final class DynamicDispatchBinding implements InternalCelFunctionBinding { private final boolean isStrict; private final DynamicDispatchOverload dynamicDispatchOverload; @@ -103,6 +125,11 @@ public String getOverloadId() { return dynamicDispatchOverload.functionName; } + @Override + public String getFunctionName() { + return dynamicDispatchOverload.functionName; + } + @Override public ImmutableList> getArgTypes() { return ImmutableList.of(); diff --git a/runtime/src/main/java/dev/cel/runtime/InternalCelFunctionBinding.java b/runtime/src/main/java/dev/cel/runtime/InternalCelFunctionBinding.java new file mode 100644 index 000000000..48a0f36d1 --- /dev/null +++ b/runtime/src/main/java/dev/cel/runtime/InternalCelFunctionBinding.java @@ -0,0 +1,29 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dev.cel.runtime; + +import com.google.errorprone.annotations.Immutable; +import dev.cel.common.annotations.Internal; + +/** + * Internal interface to expose the function name associated with a binding. + * + *

CEL Library Internals. Do Not Use. + */ +@Internal +@Immutable +public interface InternalCelFunctionBinding extends CelFunctionBinding { + String getFunctionName(); +} diff --git a/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java b/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java index 0e5c5cf30..d58eb3be4 100644 --- a/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java @@ -162,9 +162,18 @@ public CelLiteRuntime build() { functionBindingsBuilder .buildOrThrow() .forEach( - (String overloadId, CelFunctionBinding func) -> - dispatcherBuilder.addOverload( - overloadId, func.getArgTypes(), func.isStrict(), func.getDefinition())); + (String overloadId, CelFunctionBinding func) -> { + String functionName = func.getOverloadId(); + if (func instanceof InternalCelFunctionBinding) { + functionName = ((InternalCelFunctionBinding) func).getFunctionName(); + } + dispatcherBuilder.addOverload( + functionName, + overloadId, + func.getArgTypes(), + func.isStrict(), + func.getDefinition()); + }); Interpreter interpreter = new DefaultInterpreter( diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalBinary.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalBinary.java index 7771da3e6..16eba3cce 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalBinary.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalBinary.java @@ -25,6 +25,7 @@ final class EvalBinary extends PlannedInterpretable { + private final String functionName; private final CelResolvedOverload resolvedOverload; private final PlannedInterpretable arg1; private final PlannedInterpretable arg2; @@ -48,25 +49,29 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEval return unknowns; } - return EvalHelpers.dispatch(resolvedOverload, celValueConverter, argVal1, argVal2); + return EvalHelpers.dispatch( + functionName, resolvedOverload, celValueConverter, argVal1, argVal2); } static EvalBinary create( long exprId, + String functionName, CelResolvedOverload resolvedOverload, PlannedInterpretable arg1, PlannedInterpretable arg2, CelValueConverter celValueConverter) { - return new EvalBinary(exprId, resolvedOverload, arg1, arg2, celValueConverter); + return new EvalBinary(exprId, functionName, resolvedOverload, arg1, arg2, celValueConverter); } private EvalBinary( long exprId, + String functionName, CelResolvedOverload resolvedOverload, PlannedInterpretable arg1, PlannedInterpretable arg2, CelValueConverter celValueConverter) { super(exprId); + this.functionName = functionName; this.resolvedOverload = resolvedOverload; this.arg1 = arg1; this.arg2 = arg2; diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java index a30f91880..220642f4a 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalHelpers.java @@ -56,7 +56,10 @@ static Object evalStrictly( } static Object dispatch( - CelResolvedOverload overload, CelValueConverter valueConverter, Object[] args) + String functionName, + CelResolvedOverload overload, + CelValueConverter valueConverter, + Object[] args) throws CelEvaluationException { try { Object result = overload.invoke(args); @@ -66,7 +69,11 @@ static Object dispatch( } } - static Object dispatch(CelResolvedOverload overload, CelValueConverter valueConverter, Object arg) + static Object dispatch( + String functionName, + CelResolvedOverload overload, + CelValueConverter valueConverter, + Object arg) throws CelEvaluationException { try { Object result = overload.invoke(arg); @@ -77,7 +84,11 @@ static Object dispatch(CelResolvedOverload overload, CelValueConverter valueConv } static Object dispatch( - CelResolvedOverload overload, CelValueConverter valueConverter, Object arg1, Object arg2) + String functionName, + CelResolvedOverload overload, + CelValueConverter valueConverter, + Object arg1, + Object arg2) throws CelEvaluationException { try { Object result = overload.invoke(arg1, arg2); @@ -97,7 +108,7 @@ private static RuntimeException handleDispatchException( return new IllegalArgumentException( String.format( "Function '%s' failed with arg(s) '%s'", - overload.getOverloadId(), Joiner.on(", ").join(args)), + overload.getFunctionName(), Joiner.on(", ").join(args)), e); } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalLateBoundCall.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalLateBoundCall.java index cdee878ee..0bd251185 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalLateBoundCall.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalLateBoundCall.java @@ -55,7 +55,7 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEval .findOverload(functionName, overloadIds, argVals) .orElseThrow(() -> new CelOverloadNotFoundException(functionName, overloadIds)); - return EvalHelpers.dispatch(resolvedOverload, celValueConverter, argVals); + return EvalHelpers.dispatch(functionName, resolvedOverload, celValueConverter, argVals); } static EvalLateBoundCall create( diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalUnary.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalUnary.java index 322648ee3..57834161f 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalUnary.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalUnary.java @@ -24,6 +24,7 @@ final class EvalUnary extends PlannedInterpretable { + private final String functionName; private final CelResolvedOverload resolvedOverload; private final PlannedInterpretable arg; private final CelValueConverter celValueConverter; @@ -34,23 +35,26 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEval resolvedOverload.isStrict() ? evalStrictly(arg, resolver, frame) : evalNonstrictly(arg, resolver, frame); - return EvalHelpers.dispatch(resolvedOverload, celValueConverter, argVal); + return EvalHelpers.dispatch(functionName, resolvedOverload, celValueConverter, argVal); } static EvalUnary create( long exprId, + String functionName, CelResolvedOverload resolvedOverload, PlannedInterpretable arg, CelValueConverter celValueConverter) { - return new EvalUnary(exprId, resolvedOverload, arg, celValueConverter); + return new EvalUnary(exprId, functionName, resolvedOverload, arg, celValueConverter); } private EvalUnary( long exprId, + String functionName, CelResolvedOverload resolvedOverload, PlannedInterpretable arg, CelValueConverter celValueConverter) { super(exprId); + this.functionName = functionName; this.resolvedOverload = resolvedOverload; this.arg = arg; this.celValueConverter = celValueConverter; diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalVarArgsCall.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalVarArgsCall.java index eb8745632..fe7c6c430 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalVarArgsCall.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalVarArgsCall.java @@ -25,6 +25,7 @@ final class EvalVarArgsCall extends PlannedInterpretable { + private final String functionName; private final CelResolvedOverload resolvedOverload; @SuppressWarnings("Immutable") @@ -50,23 +51,26 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEval return unknowns; } - return EvalHelpers.dispatch(resolvedOverload, celValueConverter, argVals); + return EvalHelpers.dispatch(functionName, resolvedOverload, celValueConverter, argVals); } static EvalVarArgsCall create( long exprId, + String functionName, CelResolvedOverload resolvedOverload, PlannedInterpretable[] args, CelValueConverter celValueConverter) { - return new EvalVarArgsCall(exprId, resolvedOverload, args, celValueConverter); + return new EvalVarArgsCall(exprId, functionName, resolvedOverload, args, celValueConverter); } private EvalVarArgsCall( long exprId, + String functionName, CelResolvedOverload resolvedOverload, PlannedInterpretable[] args, CelValueConverter celValueConverter) { super(exprId); + this.functionName = functionName; this.resolvedOverload = resolvedOverload; this.args = args; this.celValueConverter = celValueConverter; diff --git a/runtime/src/main/java/dev/cel/runtime/planner/EvalZeroArity.java b/runtime/src/main/java/dev/cel/runtime/planner/EvalZeroArity.java index 5b3138207..7798c8253 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/EvalZeroArity.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/EvalZeroArity.java @@ -22,22 +22,30 @@ final class EvalZeroArity extends PlannedInterpretable { private static final Object[] EMPTY_ARRAY = new Object[0]; + private final String functionName; private final CelResolvedOverload resolvedOverload; private final CelValueConverter celValueConverter; @Override public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEvaluationException { - return EvalHelpers.dispatch(resolvedOverload, celValueConverter, EMPTY_ARRAY); + return EvalHelpers.dispatch(functionName, resolvedOverload, celValueConverter, EMPTY_ARRAY); } static EvalZeroArity create( - long exprId, CelResolvedOverload resolvedOverload, CelValueConverter celValueConverter) { - return new EvalZeroArity(exprId, resolvedOverload, celValueConverter); + long exprId, + String functionName, + CelResolvedOverload resolvedOverload, + CelValueConverter celValueConverter) { + return new EvalZeroArity(exprId, functionName, resolvedOverload, celValueConverter); } private EvalZeroArity( - long exprId, CelResolvedOverload resolvedOverload, CelValueConverter celValueConverter) { + long exprId, + String functionName, + CelResolvedOverload resolvedOverload, + CelValueConverter celValueConverter) { super(exprId); + this.functionName = functionName; this.resolvedOverload = resolvedOverload; this.celValueConverter = celValueConverter; } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java index 9bd5f3ecd..a0b74fc99 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java @@ -308,15 +308,21 @@ private PlannedInterpretable planCall(CelExpr expr, PlannerContext ctx) { switch (argCount) { case 0: - return EvalZeroArity.create(expr.id(), resolvedOverload, celValueConverter); + return EvalZeroArity.create(expr.id(), functionName, resolvedOverload, celValueConverter); case 1: - return EvalUnary.create(expr.id(), resolvedOverload, evaluatedArgs[0], celValueConverter); + return EvalUnary.create( + expr.id(), functionName, resolvedOverload, evaluatedArgs[0], celValueConverter); case 2: return EvalBinary.create( - expr.id(), resolvedOverload, evaluatedArgs[0], evaluatedArgs[1], celValueConverter); + expr.id(), + functionName, + resolvedOverload, + evaluatedArgs[0], + evaluatedArgs[1], + celValueConverter); default: return EvalVarArgsCall.create( - expr.id(), resolvedOverload, evaluatedArgs, celValueConverter); + expr.id(), functionName, resolvedOverload, evaluatedArgs, celValueConverter); } } diff --git a/runtime/src/test/java/dev/cel/runtime/BUILD.bazel b/runtime/src/test/java/dev/cel/runtime/BUILD.bazel index 577010971..7cd24f040 100644 --- a/runtime/src/test/java/dev/cel/runtime/BUILD.bazel +++ b/runtime/src/test/java/dev/cel/runtime/BUILD.bazel @@ -2,6 +2,7 @@ load("@rules_java//java:defs.bzl", "java_library") load("//:cel_android_rules.bzl", "cel_android_local_test") load("//:testing.bzl", "junit4_test_suites") +# Invalidate cache after file removal package( default_applicable_licenses = ["//:license"], default_testonly = True, diff --git a/runtime/src/test/java/dev/cel/runtime/CelResolvedOverloadTest.java b/runtime/src/test/java/dev/cel/runtime/CelResolvedOverloadTest.java index c1210c1ba..471282117 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelResolvedOverloadTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelResolvedOverloadTest.java @@ -27,11 +27,13 @@ public final class CelResolvedOverloadTest { CelResolvedOverload getIncrementIntOverload() { return CelResolvedOverload.of( - "increment_int", - (args) -> { - Long arg = (Long) args[0]; - return arg + 1; - }, + /* functionName= */ "increment_int", + /* overloadId= */ "increment_int_overload", + (CelFunctionOverload) + (args) -> { + Long arg = (Long) args[0]; + return arg + 1; + }, /* isStrict= */ true, Long.class); } @@ -45,14 +47,23 @@ public void canHandle_matchingTypes_returnsTrue() { public void canHandle_nullMessageType_returnsFalse() { CelResolvedOverload overload = CelResolvedOverload.of( - "identity", (args) -> args[0], /* isStrict= */ true, TestAllTypes.class); + /* functionName= */ "identity", + /* overloadId= */ "identity_overload", + (CelFunctionOverload) (args) -> args[0], + /* isStrict= */ true, + TestAllTypes.class); assertThat(overload.canHandle(new Object[] {null})).isFalse(); } @Test public void canHandle_nullPrimitive_returnsFalse() { CelResolvedOverload overload = - CelResolvedOverload.of("identity", (args) -> args[0], /* isStrict= */ true, Long.class); + CelResolvedOverload.of( + /* functionName= */ "identity", + /* overloadId= */ "identity_overload", + (CelFunctionOverload) (args) -> args[0], + /* isStrict= */ true, + Long.class); assertThat(overload.canHandle(new Object[] {null})).isFalse(); } @@ -70,10 +81,12 @@ public void canHandle_nonMatchingArgCount_returnsFalse() { public void canHandle_nonStrictOverload_returnsTrue() { CelResolvedOverload nonStrictOverload = CelResolvedOverload.of( - "non_strict", - (args) -> { - return false; - }, + /* functionName= */ "non_strict", + /* overloadId= */ "non_strict_overload", + (CelFunctionOverload) + (args) -> { + return false; + }, /* isStrict= */ false, Long.class, Long.class); @@ -87,10 +100,12 @@ public void canHandle_nonStrictOverload_returnsTrue() { public void canHandle_nonStrictOverload_returnsFalse() { CelResolvedOverload nonStrictOverload = CelResolvedOverload.of( - "non_strict", - (args) -> { - return false; - }, + /* functionName= */ "non_strict", + /* overloadId= */ "non_strict_overload", + (CelFunctionOverload) + (args) -> { + return false; + }, /* isStrict= */ false, Long.class, Long.class); diff --git a/runtime/src/test/java/dev/cel/runtime/DefaultDispatcherTest.java b/runtime/src/test/java/dev/cel/runtime/DefaultDispatcherTest.java index 255360ee1..d862ddb33 100644 --- a/runtime/src/test/java/dev/cel/runtime/DefaultDispatcherTest.java +++ b/runtime/src/test/java/dev/cel/runtime/DefaultDispatcherTest.java @@ -37,11 +37,19 @@ public void setup() { overloads.put( "overload_1", CelResolvedOverload.of( - "overload_1", args -> (Long) args[0] + 1, /* isStrict= */ true, Long.class)); + /* functionName= */ "overload_1", + /* overloadId= */ "overload_1", + args -> (Long) args[0] + 1, + /* isStrict= */ true, + Long.class)); overloads.put( "overload_2", CelResolvedOverload.of( - "overload_2", args -> (Long) args[0] + 2, /* isStrict= */ true, Long.class)); + /* functionName= */ "overload_2", + /* overloadId= */ "overload_2", + args -> (Long) args[0] + 2, + /* isStrict= */ true, + Long.class)); } @Test diff --git a/runtime/src/test/java/dev/cel/runtime/DefaultInterpreterTest.java b/runtime/src/test/java/dev/cel/runtime/DefaultInterpreterTest.java index 1a8f45161..bd0e96856 100644 --- a/runtime/src/test/java/dev/cel/runtime/DefaultInterpreterTest.java +++ b/runtime/src/test/java/dev/cel/runtime/DefaultInterpreterTest.java @@ -77,15 +77,21 @@ public Object adapt(String messageName, Object message) { CelAbstractSyntaxTree ast = celCompiler.compile("[1].all(x, [2].all(y, error()))").getAst(); DefaultDispatcher.Builder dispatcherBuilder = DefaultDispatcher.newBuilder(); dispatcherBuilder.addOverload( - "error", - ImmutableList.of(long.class), + /* functionName= */ "error", + /* overloadId= */ "error_overload", + ImmutableList.>of(long.class), /* isStrict= */ true, (args) -> new IllegalArgumentException("Always throws")); CelFunctionBinding notStrictlyFalseBinding = NotStrictlyFalseOverload.NOT_STRICTLY_FALSE.newFunctionBinding( CelOptions.DEFAULT, RuntimeEquality.create(RuntimeHelpers.create(), CelOptions.DEFAULT)); + String functionName = notStrictlyFalseBinding.getOverloadId(); + if (notStrictlyFalseBinding instanceof InternalCelFunctionBinding) { + functionName = ((InternalCelFunctionBinding) notStrictlyFalseBinding).getFunctionName(); + } dispatcherBuilder.addOverload( + functionName, notStrictlyFalseBinding.getOverloadId(), notStrictlyFalseBinding.getArgTypes(), notStrictlyFalseBinding.isStrict(), diff --git a/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java b/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java index 2b0e53298..c0b0f76c4 100644 --- a/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java +++ b/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java @@ -82,13 +82,14 @@ protected CelAbstractSyntaxTree prepareTest(CelTypeProvider typeProvider) { @Override public void optional_errors() { - if (isParseOnly) { - // Parsed-only evaluation contains function name in the - // error message instead of the function overload. - skipBaselineVerification(); - } else { - super.optional_errors(); - } + // Exercised in planner_optional_errors instead + skipBaselineVerification(); + } + + @Test + public void planner_optional_errors() { + source = "optional.unwrap([dyn(1)])"; + runTest(ImmutableMap.of()); } @Override diff --git a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java index de30902d3..c58ae782b 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java @@ -75,6 +75,7 @@ import dev.cel.runtime.CelUnknownSet; import dev.cel.runtime.DefaultDispatcher; import dev.cel.runtime.DescriptorTypeResolver; +import dev.cel.runtime.InternalCelFunctionBinding; import dev.cel.runtime.PartialVars; import dev.cel.runtime.Program; import dev.cel.runtime.RuntimeEquality; @@ -244,6 +245,7 @@ private static void addBindingsToDispatcher( overloadBindings.forEach( overload -> builder.addOverload( + ((InternalCelFunctionBinding) overload).getFunctionName(), overload.getOverloadId(), overload.getArgTypes(), overload.isStrict(), @@ -494,15 +496,11 @@ public void plan_call_zeroArgs() throws Exception { public void plan_call_throws() throws Exception { CelAbstractSyntaxTree ast = compile("error()"); Program program = PLANNER.plan(ast); - String expectedOverloadId = isParseOnly ? "error" : "error_overload"; CelEvaluationException e = assertThrows(CelEvaluationException.class, program::eval); assertThat(e) .hasMessageThat() - .contains( - "evaluation error at :5: Function '" - + expectedOverloadId - + "' failed with arg(s) ''"); + .contains("evaluation error at :5: Function 'error' failed with arg(s) ''"); assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class); assertThat(e.getCause()).hasMessageThat().contains("Intentional error"); } @@ -562,13 +560,11 @@ public void plan_call_mapIndex() throws Exception { public void plan_call_noMatchingOverload_throws() throws Exception { CelAbstractSyntaxTree ast = compile("concat(b'abc', dyn_var)"); Program program = PLANNER.plan(ast); - String errorMsg; + String errorMsg = + "No matching overload for function 'concat'. Overload candidates: concat_bytes_bytes"; if (isParseOnly) { - errorMsg = - "No matching overload for function 'concat'. Overload candidates: concat_bytes_bytes," - + " bytes_concat_bytes"; - } else { - errorMsg = "No matching overload for function 'concat_bytes_bytes'"; + // Parsed-only evaluation includes both overloads as candidates due to dynamic dispatch + errorMsg += ", bytes_concat_bytes"; } CelEvaluationException e = diff --git a/runtime/src/test/resources/planner_optional_errors.baseline b/runtime/src/test/resources/planner_optional_errors.baseline new file mode 100644 index 000000000..3d59fefca --- /dev/null +++ b/runtime/src/test/resources/planner_optional_errors.baseline @@ -0,0 +1,5 @@ +Source: optional.unwrap([dyn(1)]) +=====> +bindings: {} +error: evaluation error at test_location:15: Function 'optional.unwrap' failed with arg(s) '[1]' +error_code: INTERNAL_ERROR