From d6428981193d82d40ef7fd690be2b8f7c7389dcf Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Fri, 17 Jul 2026 19:38:39 +0200 Subject: [PATCH 1/4] Implement CEL optional conformance support Add runtime optional values and wire optional constructors, receiver methods, lazy optMap/optFlatMap macros, and optional access operators. Parse optional select, index, list, map, and message syntax with CongoCC, then preserve the protobuf optional-entry AST fields through checking and interpretation. Enable optionals conformance coverage and remove the optional-dependent block-extension skips. --- .../conformance/SimpleConformanceTest.java | 20 +- core/src/main/congocc/cel/cel.ccc | 14 +- .../projectnessie/cel/checker/Checker.java | 49 +++- .../cel/common/operators/Operator.java | 2 + .../cel/common/types/OptionalT.java | 274 ++++++++++++++++++ .../cel/extension/OptionalLib.java | 153 +++++++++- .../cel/interpreter/Interpretable.java | 66 ++++- .../cel/interpreter/InterpretablePlanner.java | 14 +- .../org/projectnessie/cel/parser/Helper.java | 29 +- .../org/projectnessie/cel/parser/Parser.java | 76 ++++- .../cel/extension/OptionalLibTest.java | 116 ++++++++ 11 files changed, 777 insertions(+), 36 deletions(-) create mode 100644 core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java diff --git a/conformance/src/test/java/org/projectnessie/cel/conformance/SimpleConformanceTest.java b/conformance/src/test/java/org/projectnessie/cel/conformance/SimpleConformanceTest.java index 7d3c2e607..15d7bfe54 100644 --- a/conformance/src/test/java/org/projectnessie/cel/conformance/SimpleConformanceTest.java +++ b/conformance/src/test/java/org/projectnessie/cel/conformance/SimpleConformanceTest.java @@ -135,6 +135,7 @@ class SimpleConformanceTest { "math_ext.textproto", "namespace.textproto", "network_ext.textproto", + "optionals.textproto", "parse.textproto", "plumbing.textproto", "proto2.textproto", @@ -185,12 +186,7 @@ class SimpleConformanceTest { "enums/strong_proto3/convert_int_too_big", "enums/strong_proto3/convert_int_too_neg", "enums/strong_proto3/convert_string", - "enums/strong_proto3/convert_string_bad", - // Optional list/map/message syntax and runtime support is not implemented yet. - "block_ext/basic/optional_list", - "block_ext/basic/optional_map", - "block_ext/basic/optional_map_chained", - "block_ext/basic/optional_message"); + "enums/strong_proto3/convert_string_bad"); private static final Set matchedSkips = new LinkedHashSet<>(); private static final AtomicInteger total = new AtomicInteger(); @@ -353,6 +349,9 @@ private static ParsedExpr parse(SimpleTest test) { if (usesTestOnlyBlockMacros(test.getExpr())) { parseOptions.add(macros(Macro.TestOnlyBlockMacros)); } + if (usesOptionals(test.getExpr())) { + parseOptions.add(optionals()); + } Env env = newEnv(parseOptions.toArray(new EnvOption[0])); AstIssuesTuple astIss = env.parse(sourceText); @@ -439,7 +438,7 @@ private static List conformanceEnvOptions(SimpleTest test, EnvOption. if (usesNetworkExtensions(test.getExpr())) { envOptions.add(network()); } - if (test.getExpr().contains("optional.")) { + if (usesOptionals(test.getExpr())) { envOptions.add(optionals()); } envOptions.addAll(List.of(options)); @@ -462,6 +461,13 @@ private static boolean usesStringExtensions(String expression) { || expression.contains(".reverse("); } + private static boolean usesOptionals(String expression) { + return expression.contains("optional.") + || expression.contains(".?") + || expression.contains("[?") + || expression.contains("{?"); + } + private static boolean usesNetworkExtensions(String expression) { return expression.contains("ip(") || expression.contains("cidr(") diff --git a/core/src/main/congocc/cel/cel.ccc b/core/src/main/congocc/cel/cel.ccc index a6acc1458..fbd02f893 100644 --- a/core/src/main/congocc/cel/cel.ccc +++ b/core/src/main/congocc/cel/cel.ccc @@ -50,8 +50,8 @@ Unary : Member : Primary ( - Field [ ( | ExprList )] - | Expr + [] Field [ ( | ExprList )] + | [] Expr | [FieldInitializerList] [] )*! ; @@ -59,7 +59,7 @@ Member : Primary : [] [ ( | ExprList )] | Expr - | ( | ExprList [] ) + | ( | ListInitializerList [] ) | ( | MapInitializerList [] ) | ConstantLiteral ; @@ -68,8 +68,12 @@ ExprList : Expr ( Expr =>||)*! ; +ListInitializerList : + [] Expr ( [] Expr =>||)*! + ; + FieldInitializerList : - Field Expr ( Field Expr =>||)*! + [] Field Expr ( [] Field Expr =>||)*! ; Field : @@ -78,7 +82,7 @@ Field : ; MapInitializerList : - Expr Expr ( Expr Expr =>||)*! + [] Expr Expr ( [] Expr Expr =>||)*! ; ConstantLiteral : diff --git a/core/src/main/java/org/projectnessie/cel/checker/Checker.java b/core/src/main/java/org/projectnessie/cel/checker/Checker.java index ef02447d3..5fdc2cc29 100644 --- a/core/src/main/java/org/projectnessie/cel/checker/Checker.java +++ b/core/src/main/java/org/projectnessie/cel/checker/Checker.java @@ -285,6 +285,13 @@ void checkSelect(Expr.Builder e) { resultType = fieldType.type; } break; + case kindAbstract: + if (isOptionalType(targetType)) { + resultType = Decls.newAbstractType("optional_type", Collections.singletonList(Decls.Dyn)); + } else { + errors.typeDoesNotSupportFieldSelection(location(e), targetType); + } + break; case kindTypeParam: // Set the operand type to DYN to prevent assignment to a potentionally incorrect type // at a later point in type-checking. The isAssignable call will update the type @@ -309,6 +316,10 @@ void checkSelect(Expr.Builder e) { setType(e, resultType); } + private static boolean isOptionalType(Type type) { + return type.hasAbstractType() && "optional_type".equals(type.getAbstractType().getName()); + } + private boolean isQualifiedLocalVariableSelection(Expr.Builder e) { if (e.getExprKindCase() == Expr.ExprKindCase.IDENT_EXPR) { return env.hasLocalIdent(e.getIdentExpr().getName()); @@ -469,10 +480,21 @@ OverloadResolution resolveOverload( void checkCreateList(Expr.Builder e) { CreateList.Builder create = e.getListExprBuilder(); Type elemType = null; + boolean[] optionalIndices = new boolean[create.getElementsCount()]; + for (int index : create.getOptionalIndicesList()) { + optionalIndices[index] = true; + } for (int i = 0; i < create.getElementsBuilderList().size(); i++) { Expr.Builder el = create.getElementsBuilderList().get(i); check(el); - elemType = joinTypes(location(el), elemType, getType(el)); + Type type = getType(el); + if (optionalIndices[i]) { + Type unwrapped = optionalValueType(type); + if (unwrapped != null) { + type = unwrapped; + } + } + elemType = joinTypes(location(el), elemType, type); } if (elemType == null) { // If the list is empty, assign free type var to elem type. @@ -501,7 +523,14 @@ void checkCreateMap(Expr.Builder e) { Expr.Builder val = ent.getValueBuilder(); check(val); - valueType = joinTypes(location(val), valueType, getType(val)); + Type type = getType(val); + if (ent.getOptionalEntry()) { + Type unwrapped = optionalValueType(type); + if (unwrapped != null) { + type = unwrapped; + } + } + valueType = joinTypes(location(val), valueType, type); } if (keyType == null) { // If the map is empty, assign free type variables to typeKey and value type. @@ -553,12 +582,26 @@ void checkCreateMessage(Expr.Builder e) { if (t != null) { fieldType = t.type; } - if (!isAssignable(fieldType, getType(value))) { + Type valueType = getType(value); + if (ent.getOptionalEntry()) { + Type unwrapped = optionalValueType(valueType); + if (unwrapped != null) { + valueType = unwrapped; + } + } + if (!isAssignable(fieldType, valueType)) { errors.fieldTypeMismatch(locationByID(ent.getId()), field, fieldType, getType(value)); } } } + private static Type optionalValueType(Type type) { + if (!isOptionalType(type) || type.getAbstractType().getParameterTypesCount() == 0) { + return null; + } + return type.getAbstractType().getParameterTypes(0); + } + void checkComprehension(Expr.Builder e) { Comprehension.Builder comp = e.getComprehensionExprBuilder(); check(comp.getIterRangeBuilder()); diff --git a/core/src/main/java/org/projectnessie/cel/common/operators/Operator.java b/core/src/main/java/org/projectnessie/cel/common/operators/Operator.java index bd601a42e..73f7744b2 100644 --- a/core/src/main/java/org/projectnessie/cel/common/operators/Operator.java +++ b/core/src/main/java/org/projectnessie/cel/common/operators/Operator.java @@ -42,6 +42,8 @@ public enum Operator { Modulo("_%_", 3, "%"), Negate("-_", 2, "-"), Index("_[_]", 1, null), + OptionalSelect("@optional_select"), + OptionalIndex("@optional_index"), // Macros, must have a valid identifier. Has("has"), All("all"), diff --git a/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java b/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java new file mode 100644 index 000000000..18d67086b --- /dev/null +++ b/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java @@ -0,0 +1,274 @@ +/* + * Copyright (C) 2026 The Authors of CEL-Java + * + * 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 + * + * http://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 org.projectnessie.cel.common.types; + +import static org.projectnessie.cel.common.types.BoolT.False; +import static org.projectnessie.cel.common.types.BoolT.True; +import static org.projectnessie.cel.common.types.Err.newErr; +import static org.projectnessie.cel.common.types.Err.newTypeConversionError; +import static org.projectnessie.cel.common.types.Err.noSuchOverload; +import static org.projectnessie.cel.common.types.IntT.IntZero; +import static org.projectnessie.cel.common.types.TypeT.TypeType; +import static org.projectnessie.cel.common.types.TypeT.newObjectTypeValue; + +import com.google.protobuf.Message; +import java.util.Objects; +import org.projectnessie.cel.common.types.ref.BaseVal; +import org.projectnessie.cel.common.types.ref.Type; +import org.projectnessie.cel.common.types.ref.TypeEnum; +import org.projectnessie.cel.common.types.ref.Val; +import org.projectnessie.cel.common.types.traits.Container; +import org.projectnessie.cel.common.types.traits.FieldTester; +import org.projectnessie.cel.common.types.traits.Indexer; +import org.projectnessie.cel.common.types.traits.Mapper; +import org.projectnessie.cel.common.types.traits.Receiver; +import org.projectnessie.cel.common.types.traits.Sizer; +import org.projectnessie.cel.common.types.traits.Trait; + +/** Runtime value for CEL optional_type values. */ +public final class OptionalT extends BaseVal implements FieldTester, Indexer, Receiver { + public static final String OptionalTypeName = "optional_type"; + public static final Type OptionalType = + newObjectTypeValue( + OptionalTypeName, Trait.FieldTesterType, Trait.IndexerType, Trait.ReceiverType); + + private static final OptionalT None = new OptionalT(null, false); + + private final Val value; + private final boolean present; + + private OptionalT(Val value, boolean present) { + this.value = value; + this.present = present; + } + + public static OptionalT none() { + return None; + } + + public static OptionalT of(Val value) { + return new OptionalT(Objects.requireNonNull(value, "value"), true); + } + + public static OptionalT ofNonZeroValue(Val value) { + return isZeroValue(value) ? none() : of(value); + } + + public static Val optionalSelect(Val operand, Val field) { + return optionalAccess(operand, field); + } + + public static Val optionalIndex(Val operand, Val index) { + return optionalAccess(operand, index); + } + + public boolean hasValue() { + return present; + } + + public Val getValue() { + return value; + } + + @Override + public T convertToNative(Class typeDesc) { + if (typeDesc == Val.class || typeDesc == OptionalT.class) { + return typeDesc.cast(this); + } + if (typeDesc == Object.class) { + return typeDesc.cast(value()); + } + throw new RuntimeException( + String.format( + "native type conversion error from '%s' to '%s'", OptionalType, typeDesc.getName())); + } + + @Override + public Val convertToType(Type typeValue) { + if (typeValue.equals(OptionalType)) { + return this; + } + if (typeValue == TypeType) { + return OptionalType; + } + return newTypeConversionError(OptionalType, typeValue); + } + + @Override + public Val equal(Val other) { + if (!(other instanceof OptionalT)) { + return False; + } + OptionalT optional = (OptionalT) other; + if (!present || !optional.present) { + return present == optional.present ? True : False; + } + return value.equal(optional.value); + } + + @Override + public Type type() { + return OptionalType; + } + + @Override + public Object value() { + return present ? value.value() : null; + } + + @Override + public Val isSet(Val field) { + if (!present) { + return False; + } + if (value instanceof OptionalT) { + return ((OptionalT) value).isSet(field); + } + if (value instanceof FieldTester) { + Val present = ((FieldTester) value).isSet(field); + return isMissingAccess(present) ? False : present; + } + if (value instanceof Container) { + return ((Container) value).contains(field); + } + return noSuchOverload(value, "has", field); + } + + @Override + public Val get(Val index) { + return present ? optionalAccess(value, index) : none(); + } + + @Override + public Val receive(String function, String overload, Val... args) { + switch (function) { + case "hasValue": + return args.length == 0 + ? (present ? True : False) + : noSuchOverload(this, function, overload, args); + case "value": + return value(args, function, overload); + case "or": + return or(args, function, overload); + case "orValue": + return orValue(args, function, overload); + default: + return noSuchOverload(this, function, overload, args); + } + } + + private Val value(Val[] args, String function, String overload) { + if (args.length != 0) { + return noSuchOverload(this, function, overload, args); + } + return present ? value : newErr("optional.none() has no value"); + } + + private Val or(Val[] args, String function, String overload) { + if (args.length != 1 || !(args[0] instanceof OptionalT)) { + return noSuchOverload(this, function, overload, args); + } + return present ? this : args[0]; + } + + private Val orValue(Val[] args, String function, String overload) { + if (args.length != 1) { + return noSuchOverload(this, function, overload, args); + } + return present ? value : args[0]; + } + + private static boolean isZeroValue(Val value) { + switch (value.type().typeEnum()) { + case Null: + return true; + case Bool: + return value == False || !value.booleanValue(); + case Int: + case Uint: + return value.intValue() == 0L; + case Double: + return value.doubleValue() == 0.0d; + case String: + case Bytes: + case List: + case Map: + return value.type().hasTrait(Trait.SizerType) + && ((Sizer) value).size().equal(IntZero) == True; + case Object: + return value.value() instanceof Message + && ((Message) value.value()).getAllFields().isEmpty(); + default: + return false; + } + } + + private static Val optionalAccess(Val operand, Val index) { + if (operand instanceof OptionalT) { + return ((OptionalT) operand).get(index); + } + if (operand instanceof FieldTester && index.type().typeEnum() == TypeEnum.String) { + Val present = ((FieldTester) operand).isSet(index); + if (present == False) { + return none(); + } + if (present != True) { + return isMissingAccess(present) ? none() : present; + } + } + if (operand instanceof Mapper) { + Val value = ((Mapper) operand).find(index); + return value == null ? none() : of(value); + } + if (operand instanceof Indexer) { + Val value = ((Indexer) operand).get(index); + return isMissingAccess(value) ? none() : of(value); + } + return noSuchOverload(operand, "optional access", index); + } + + private static boolean isMissingAccess(Val value) { + if (!(value instanceof Err)) { + return false; + } + String error = value.toString(); + return error.startsWith("no such key") + || error.startsWith("no such field") + || error.startsWith("invalid_argument") + || error.startsWith("index out of bounds"); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof Val)) { + return false; + } + return equal((Val) o) == True; + } + + @Override + public int hashCode() { + return present ? Objects.hash(OptionalType, value) : Objects.hash(OptionalType); + } + + @Override + public String toString() { + return present ? String.format("optional.of(%s)", value) : "optional.none()"; + } +} diff --git a/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java b/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java index 0fb182425..f56a8e59c 100644 --- a/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java +++ b/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java @@ -17,26 +17,53 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; +import static org.projectnessie.cel.common.types.OptionalT.OptionalType; +import com.google.api.expr.v1alpha1.Expr; +import com.google.api.expr.v1alpha1.Expr.ExprKindCase; import java.util.List; import org.projectnessie.cel.EnvOption; import org.projectnessie.cel.Library; import org.projectnessie.cel.ProgramOption; import org.projectnessie.cel.checker.Decls; +import org.projectnessie.cel.common.ErrorWithLocation; +import org.projectnessie.cel.common.Location; +import org.projectnessie.cel.common.operators.Operator; +import org.projectnessie.cel.common.types.OptionalT; +import org.projectnessie.cel.interpreter.functions.Overload; +import org.projectnessie.cel.parser.ExprHelper; +import org.projectnessie.cel.parser.Macro; /** - * OptionalLib provides compile-time declarations for CEL optional helper functions. + * OptionalLib provides CEL optional helper functions. * - *

The current implementation intentionally exposes type-checking support only. It is sufficient - * for check-only conformance cases that exercise optional type deduction, but it does not provide - * runtime optional values or optional-selection semantics. + *

This library provides runtime optional values, ordinary optional constructors/receiver + * methods, optional access operators, and lazy optMap/optFlatMap macro expansion. */ public final class OptionalLib implements Library { private static final String OPTIONAL_TYPE = "optional_type"; private static final String OPTIONAL_NONE = "optional.none"; private static final String OPTIONAL_OF = "optional.of"; private static final String OPTIONAL_OF_NON_ZERO_VALUE = "optional.ofNonZeroValue"; + private static final String OPTIONAL_HAS_VALUE = "hasValue"; + private static final String OPTIONAL_VALUE = "value"; + private static final String OPTIONAL_OR = "or"; + private static final String OPTIONAL_OR_VALUE = "orValue"; + private static final String OPTIONAL_OPT_MAP = "optMap"; + private static final String OPTIONAL_OPT_FLAT_MAP = "optFlatMap"; + private static final String OPTIONAL_NONE_OVERLOAD = "optional_none"; + private static final String OPTIONAL_OF_OVERLOAD = "optional_of"; + private static final String OPTIONAL_OF_NON_ZERO_VALUE_OVERLOAD = "optional_of_non_zero_value"; + private static final String OPTIONAL_SELECT_OVERLOAD = "optional_select"; + private static final String OPTIONAL_INDEX_OVERLOAD = "optional_index"; + private static final String OPTIONAL_INDEX_OPTIONAL_OVERLOAD = "optional_index_optional"; + private static final String OPTIONAL_HAS_VALUE_OVERLOAD = "optional_has_value"; + private static final String OPTIONAL_VALUE_OVERLOAD = "optional_value"; + private static final String OPTIONAL_OR_OVERLOAD = "optional_or"; + private static final String OPTIONAL_OR_VALUE_OVERLOAD = "optional_or_value"; private static final String TYPE_PARAM_A = "A"; + private static final String OPTIONAL_MACRO_TARGET = "@optional_target"; + private static final String OPTIONAL_MACRO_RESULT = "@optional_result"; private OptionalLib() {} @@ -51,26 +78,136 @@ public List getCompileOptions() { var typeParams = singletonList(TYPE_PARAM_A); return List.of( + EnvOption.types(singletonList(OptionalType)), + EnvOption.macros( + Macro.newReceiverMacro(OPTIONAL_OPT_MAP, 2, OptionalLib::makeOptMap), + Macro.newReceiverMacro(OPTIONAL_OPT_FLAT_MAP, 2, OptionalLib::makeOptFlatMap)), EnvOption.declarations( + Decls.newVar(OPTIONAL_TYPE, Decls.newTypeType(optionalA)), Decls.newFunction( OPTIONAL_NONE, Decls.newParameterizedOverload( - "optional_none", emptyList(), optionalA, typeParams)), + OPTIONAL_NONE_OVERLOAD, emptyList(), optionalA, typeParams)), Decls.newFunction( OPTIONAL_OF, Decls.newParameterizedOverload( - "optional_of", singletonList(typeParamA), optionalA, typeParams)), + OPTIONAL_OF_OVERLOAD, singletonList(typeParamA), optionalA, typeParams)), Decls.newFunction( OPTIONAL_OF_NON_ZERO_VALUE, Decls.newParameterizedOverload( - "optional_of_non_zero_value", + OPTIONAL_OF_NON_ZERO_VALUE_OVERLOAD, singletonList(typeParamA), optionalA, + typeParams)), + Decls.newFunction( + Operator.OptionalSelect.id, + Decls.newOverload( + OPTIONAL_SELECT_OVERLOAD, + List.of(Decls.Dyn, Decls.String), + Decls.newAbstractType(OPTIONAL_TYPE, singletonList(Decls.Dyn)))), + Decls.newFunction( + Operator.OptionalIndex.id, + Decls.newOverload( + OPTIONAL_INDEX_OVERLOAD, + List.of(Decls.Dyn, Decls.Dyn), + Decls.newAbstractType(OPTIONAL_TYPE, singletonList(Decls.Dyn)))), + Decls.newFunction( + Operator.Index.id, + Decls.newParameterizedOverload( + OPTIONAL_INDEX_OPTIONAL_OVERLOAD, + List.of(optionalA, Decls.Dyn), + Decls.newAbstractType(OPTIONAL_TYPE, singletonList(Decls.Dyn)), + typeParams)), + Decls.newFunction( + OPTIONAL_HAS_VALUE, + Decls.newParameterizedInstanceOverload( + OPTIONAL_HAS_VALUE_OVERLOAD, singletonList(optionalA), Decls.Bool, typeParams)), + Decls.newFunction( + OPTIONAL_VALUE, + Decls.newParameterizedInstanceOverload( + OPTIONAL_VALUE_OVERLOAD, singletonList(optionalA), typeParamA, typeParams)), + Decls.newFunction( + OPTIONAL_OR, + Decls.newParameterizedInstanceOverload( + OPTIONAL_OR_OVERLOAD, List.of(optionalA, optionalA), optionalA, typeParams)), + Decls.newFunction( + OPTIONAL_OR_VALUE, + Decls.newParameterizedInstanceOverload( + OPTIONAL_OR_VALUE_OVERLOAD, + List.of(optionalA, typeParamA), + typeParamA, typeParams)))); } @Override public List getProgramOptions() { - return emptyList(); + return List.of( + ProgramOption.functions( + Overload.function(OPTIONAL_NONE, args -> OptionalT.none()), + Overload.function(OPTIONAL_NONE_OVERLOAD, args -> OptionalT.none()), + Overload.unary(OPTIONAL_OF, OptionalT::of), + Overload.unary(OPTIONAL_OF_OVERLOAD, OptionalT::of), + Overload.unary(OPTIONAL_OF_NON_ZERO_VALUE, OptionalT::ofNonZeroValue), + Overload.unary(OPTIONAL_OF_NON_ZERO_VALUE_OVERLOAD, OptionalT::ofNonZeroValue), + Overload.binary(Operator.OptionalSelect.id, OptionalT::optionalSelect), + Overload.binary(OPTIONAL_SELECT_OVERLOAD, OptionalT::optionalSelect), + Overload.binary(Operator.OptionalIndex.id, OptionalT::optionalIndex), + Overload.binary(OPTIONAL_INDEX_OVERLOAD, OptionalT::optionalIndex))); + } + + private static Expr makeOptMap(ExprHelper eh, Expr target, List args) { + return makeOptionalMap(eh, target, args, true); + } + + private static Expr makeOptFlatMap(ExprHelper eh, Expr target, List args) { + return makeOptionalMap(eh, target, args, false); + } + + private static Expr makeOptionalMap( + ExprHelper eh, Expr target, List args, boolean wrapResult) { + String variable = extractIdent(args.get(0)); + if (variable == null) { + Location location = eh.offsetLocation(args.get(0).getId()); + throw new ErrorWithLocation(location, "argument must be a simple name"); + } + + Expr boundTarget = eh.ident(OPTIONAL_MACRO_TARGET); + Expr value = eh.receiverCall(OPTIONAL_VALUE, boundTarget, emptyList()); + Expr iterRange = + eh.globalCall( + Operator.Conditional.id, + eh.receiverCall(OPTIONAL_HAS_VALUE, boundTarget, emptyList()), + eh.newList(value), + eh.newList()); + Expr init = eh.globalCall(OPTIONAL_NONE); + Expr step = wrapResult ? eh.globalCall(OPTIONAL_OF, args.get(1)) : args.get(1); + Expr accuIdent = eh.ident(Macro.AccumulatorName); + Expr result = + eh.fold( + variable, + iterRange, + Macro.AccumulatorName, + init, + eh.literalBool(true), + step, + accuIdent); + + Expr outerAccu = eh.ident(OPTIONAL_MACRO_RESULT); + Expr dynNull = eh.globalCall("dyn", eh.literalNull()); + return eh.fold( + OPTIONAL_MACRO_TARGET, + eh.newList(target), + OPTIONAL_MACRO_RESULT, + dynNull, + eh.literalBool(true), + result, + outerAccu); + } + + private static String extractIdent(Expr expression) { + if (expression.getExprKindCase() == ExprKindCase.IDENT_EXPR) { + return expression.getIdentExpr().getName(); + } + return null; } } diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java b/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java index ed0c3f3df..a35737b80 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java @@ -44,6 +44,7 @@ import org.projectnessie.cel.common.types.IteratorT; import org.projectnessie.cel.common.types.ListT; import org.projectnessie.cel.common.types.MapT; +import org.projectnessie.cel.common.types.OptionalT; import org.projectnessie.cel.common.types.Overloads; import org.projectnessie.cel.common.types.StringT; import org.projectnessie.cel.common.types.ref.FieldType; @@ -877,18 +878,24 @@ public String toString() { final class EvalList extends AbstractEval implements Coster { final Interpretable[] elems; + final boolean[] optionalIndices; private final TypeAdapter adapter; EvalList(long id, Interpretable[] elems, TypeAdapter adapter) { + this(id, elems, new boolean[elems.length], adapter); + } + + EvalList(long id, Interpretable[] elems, boolean[] optionalIndices, TypeAdapter adapter) { super(id); this.elems = elems; + this.optionalIndices = optionalIndices; this.adapter = adapter; } /** Eval implements the Interpretable interface method. */ @Override public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { - Val[] elemVals = new Val[elems.length]; + List elemVals = new ArrayList<>(elems.length); // If any argument is unknown or error early terminate. for (int i = 0; i < elems.length; i++) { Interpretable elem = elems[i]; @@ -896,9 +903,19 @@ public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { if (isUnknownOrError(elemVal)) { return elemVal; } - elemVals[i] = elemVal; + if (optionalIndices[i]) { + if (!(elemVal instanceof OptionalT)) { + return newErr("optional list element is not optional"); + } + OptionalT optional = (OptionalT) elemVal; + if (!optional.hasValue()) { + continue; + } + elemVal = optional.getValue(); + } + elemVals.add(elemVal); } - return adapter.nativeToValue(elemVals); + return adapter.nativeToValue(elemVals.toArray(Val[]::new)); } /** Cost implements the Coster interface method. */ @@ -916,12 +933,23 @@ public String toString() { final class EvalMap extends AbstractEval implements Coster { final Interpretable[] keys; final Interpretable[] vals; + final boolean[] optionalEntries; private final TypeAdapter adapter; EvalMap(long id, Interpretable[] keys, Interpretable[] vals, TypeAdapter adapter) { + this(id, keys, vals, new boolean[keys.length], adapter); + } + + EvalMap( + long id, + Interpretable[] keys, + Interpretable[] vals, + boolean[] optionalEntries, + TypeAdapter adapter) { super(id); this.keys = keys; this.vals = vals; + this.optionalEntries = optionalEntries; this.adapter = adapter; } @@ -943,6 +971,16 @@ public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { if (isUnknownOrError(valVal)) { return valVal; } + if (optionalEntries[i]) { + if (!(valVal instanceof OptionalT)) { + return newErr("optional map entry is not optional"); + } + OptionalT optional = (OptionalT) valVal; + if (!optional.hasValue()) { + continue; + } + valVal = optional.getValue(); + } if (entries.putIfAbsent(keyVal, valVal) != null) { // Prevent duplicate keys, error out. return newErr("Failed with repeated key"); @@ -976,14 +1014,26 @@ final class EvalObj extends AbstractEval implements Coster { private final String typeName; private final String[] fields; private final Interpretable[] vals; + private final boolean[] optionalEntries; private final TypeProvider provider; EvalObj( long id, String typeName, String[] fields, Interpretable[] vals, TypeProvider provider) { + this(id, typeName, fields, vals, new boolean[fields.length], provider); + } + + EvalObj( + long id, + String typeName, + String[] fields, + Interpretable[] vals, + boolean[] optionalEntries, + TypeProvider provider) { super(id); this.typeName = Objects.requireNonNull(typeName); this.fields = Objects.requireNonNull(fields); this.vals = Objects.requireNonNull(vals); + this.optionalEntries = optionalEntries; this.provider = Objects.requireNonNull(provider); } @@ -998,6 +1048,16 @@ public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { if (isUnknownOrError(val)) { return val; } + if (optionalEntries[i]) { + if (!(val instanceof OptionalT)) { + return newErr("optional message field is not optional"); + } + OptionalT optional = (OptionalT) val; + if (!optional.hasValue()) { + continue; + } + val = optional.getValue(); + } fieldVals.put(field, val); } return provider.newValue(typeName, fieldVals); diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java b/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java index f550e22ad..8c9b48a96 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java @@ -560,6 +560,10 @@ Interpretable planCallIndex(Expr expr, Interpretable... args) { Interpretable planCreateList(Expr expr) { CreateList list = expr.getListExpr(); Interpretable[] elems = new Interpretable[list.getElementsCount()]; + boolean[] optionalIndices = new boolean[list.getElementsCount()]; + for (int index : list.getOptionalIndicesList()) { + optionalIndices[index] = true; + } for (int i = 0; i < list.getElementsCount(); i++) { Expr elem = list.getElements(i); Interpretable elemVal = plan(elem); @@ -568,7 +572,7 @@ Interpretable planCreateList(Expr expr) { } elems[i] = elemVal; } - return new EvalList(expr.getId(), elems, adapter); + return new EvalList(expr.getId(), elems, optionalIndices, adapter); } /** planCreateStruct generates a map or object construction Interpretable. */ @@ -580,8 +584,10 @@ Interpretable planCreateStruct(Expr expr) { List entries = str.getEntriesList(); Interpretable[] keys = new Interpretable[entries.size()]; Interpretable[] vals = new Interpretable[entries.size()]; + boolean[] optionalEntries = new boolean[entries.size()]; for (int i = 0; i < entries.size(); i++) { Entry entry = entries.get(i); + optionalEntries[i] = entry.getOptionalEntry(); Interpretable keyVal = plan(entry.getMapKey()); if (keyVal == null) { return null; @@ -594,7 +600,7 @@ Interpretable planCreateStruct(Expr expr) { } vals[i] = valVal; } - return new EvalMap(expr.getId(), keys, vals, adapter); + return new EvalMap(expr.getId(), keys, vals, optionalEntries, adapter); } /** planCreateObj generates an object construction Interpretable. */ @@ -607,16 +613,18 @@ Interpretable planCreateObj(Expr expr) { List entries = obj.getEntriesList(); String[] fields = new String[entries.size()]; Interpretable[] vals = new Interpretable[entries.size()]; + boolean[] optionalEntries = new boolean[entries.size()]; for (int i = 0; i < entries.size(); i++) { Entry entry = entries.get(i); fields[i] = entry.getFieldKey(); + optionalEntries[i] = entry.getOptionalEntry(); Interpretable val = plan(entry.getValue()); if (val == null) { return null; } vals[i] = val; } - return new EvalObj(expr.getId(), typeName, fields, vals, provider); + return new EvalObj(expr.getId(), typeName, fields, vals, optionalEntries, provider); } /** planComprehension generates an Interpretable fold operation. */ diff --git a/core/src/main/java/org/projectnessie/cel/parser/Helper.java b/core/src/main/java/org/projectnessie/cel/parser/Helper.java index 1f7e93309..bad63c290 100644 --- a/core/src/main/java/org/projectnessie/cel/parser/Helper.java +++ b/core/src/main/java/org/projectnessie/cel/parser/Helper.java @@ -121,8 +121,13 @@ Expr newReceiverCall(Object ctx, String function, Expr target, List args) } Expr newList(Object ctx, List elements) { + return newList(ctx, elements, List.of()); + } + + Expr newList(Object ctx, List elements, List optionalIndices) { return newExprBuilder(ctx) - .setListExpr(CreateList.newBuilder().addAllElements(elements)) + .setListExpr( + CreateList.newBuilder().addAllElements(elements).addAllOptionalIndices(optionalIndices)) .build(); } @@ -133,7 +138,16 @@ Expr newMap(Object ctx, List entries) { } Entry newMapEntry(long entryID, Expr key, Expr value) { - return Entry.newBuilder().setId(entryID).setMapKey(key).setValue(value).build(); + return newMapEntry(entryID, key, value, false); + } + + Entry newMapEntry(long entryID, Expr key, Expr value, boolean optional) { + return Entry.newBuilder() + .setId(entryID) + .setMapKey(key) + .setValue(value) + .setOptionalEntry(optional) + .build(); } Expr newObject(Object ctx, String typeName, List entries) { @@ -143,7 +157,16 @@ Expr newObject(Object ctx, String typeName, List entries) { } Entry newObjectField(long fieldID, String field, Expr value) { - return Entry.newBuilder().setId(fieldID).setFieldKey(field).setValue(value).build(); + return newObjectField(fieldID, field, value, false); + } + + Entry newObjectField(long fieldID, String field, Expr value, boolean optional) { + return Entry.newBuilder() + .setId(fieldID) + .setFieldKey(field) + .setValue(value) + .setOptionalEntry(optional) + .build(); } Expr newComprehension( diff --git a/core/src/main/java/org/projectnessie/cel/parser/Parser.java b/core/src/main/java/org/projectnessie/cel/parser/Parser.java index 4ed51e700..9621933f9 100644 --- a/core/src/main/java/org/projectnessie/cel/parser/Parser.java +++ b/core/src/main/java/org/projectnessie/cel/parser/Parser.java @@ -62,6 +62,7 @@ import org.projectnessie.cel.parser.ast.ExprList; import org.projectnessie.cel.parser.ast.Field; import org.projectnessie.cel.parser.ast.FieldInitializerList; +import org.projectnessie.cel.parser.ast.ListInitializerList; import org.projectnessie.cel.parser.ast.MapInitializerList; import org.projectnessie.cel.parser.ast.Start; @@ -321,7 +322,11 @@ public Expr visitPrimary(Node node) { } else if (isToken(first, LPAREN)) { return exprVisit(children.get(1)); } else if (isToken(first, LBRACKET)) { - return helper.newList(helper.id(first), expressionsBetween(children, 1, RBRACKET)); + long listID = helper.id(first); + ListInitializerList list = firstChildOfType(children, ListInitializerList.class); + ListElements elements = + list != null ? listElements(list) : listElements(children.subList(1, children.size())); + return helper.newList(listID, elements.expressions(), elements.optionalIndices()); } else if (isToken(first, LBRACE)) { return helper.newMap( helper.id(first), mapEntries(firstChildOfType(children, MapInitializerList.class))); @@ -377,8 +382,17 @@ public Expr visitMember(Node node) { if (i >= children.size()) { return helper.newExpr(node); } + boolean optional = false; + Node optionalNode = null; + if (isToken(children.get(i), QUESTIONMARK)) { + optional = true; + optionalNode = children.get(i++); + } String id = fieldName(children.get(i++)); if (i < children.size() && isToken(children.get(i), LPAREN)) { + if (optional) { + return reportError(optionalNode, "optional select does not support function calls"); + } Node open = children.get(i++); long openID = helper.id(open); List args = expressionsBetween(children, i, RPAREN); @@ -389,16 +403,30 @@ public Expr visitMember(Node node) { i++; } operand = receiverCallOrMacro(openID, id, operand, args); + } else if (optional) { + operand = + globalCallOrMacro( + helper.id(optionalNode), + Operator.OptionalSelect.id, + operand, + helper.newLiteralString(optionalNode, id)); } else { operand = helper.newSelect(op, operand, id); } } else if (isToken(op, LBRACKET)) { long opID = helper.id(op); + boolean optional = false; + if (isToken(children.get(i), QUESTIONMARK)) { + optional = true; + opID = helper.id(children.get(i++)); + } Expr index = exprVisit(children.get(i++)); if (i < children.size() && isToken(children.get(i), RBRACKET)) { i++; } - operand = globalCallOrMacro(opID, Operator.Index.id, operand, index); + operand = + globalCallOrMacro( + opID, optional ? Operator.OptionalIndex.id : Operator.Index.id, operand, index); } else if (isToken(op, LBRACE)) { String messageName = extractQualifiedName(operand); FieldInitializerList fields = @@ -530,6 +558,34 @@ private List expressionsIn(ExprList list) { return result; } + private ListElements listElements(ListInitializerList list) { + if (list == null) { + return new ListElements(Collections.emptyList(), Collections.emptyList()); + } + return listElements(significantChildren(list)); + } + + private ListElements listElements(List children) { + List expressions = new ArrayList<>(); + List optionalIndices = new ArrayList<>(); + boolean optional = false; + for (Node child : children) { + if (isToken(child, COMMA) || isToken(child, RBRACKET)) { + continue; + } + if (isToken(child, QUESTIONMARK)) { + optional = true; + continue; + } + if (optional) { + optionalIndices.add(expressions.size()); + optional = false; + } + expressions.add(exprVisit(child)); + } + return new ListElements(expressions, optionalIndices); + } + private List expressionsBetween(List children, int start, Token.TokenType end) { List result = new ArrayList<>(); for (int i = start; i < children.size() && !isToken(children.get(i), end); i++) { @@ -553,6 +609,11 @@ private List objectFields(FieldInitializerList fields) { List children = significantChildren(fields); List result = new ArrayList<>(); for (int i = 0; i < children.size(); ) { + boolean optional = false; + if (isToken(children.get(i), QUESTIONMARK)) { + optional = true; + i++; + } Node field = children.get(i++); if (i >= children.size() || !isToken(children.get(i), COLON)) { break; @@ -563,7 +624,7 @@ private List objectFields(FieldInitializerList fields) { } long colonID = helper.id(colon); Expr value = exprVisit(children.get(i++)); - result.add(helper.newObjectField(colonID, fieldName(field), value)); + result.add(helper.newObjectField(colonID, fieldName(field), value, optional)); if (i < children.size() && isToken(children.get(i), COMMA)) { i++; } @@ -578,6 +639,11 @@ private List mapEntries(MapInitializerList entries) { List children = significantChildren(entries); List result = new ArrayList<>(); for (int i = 0; i < children.size(); ) { + boolean optional = false; + if (isToken(children.get(i), QUESTIONMARK)) { + optional = true; + i++; + } Node keyNode = children.get(i++); if (i >= children.size() || !isToken(children.get(i), COLON)) { break; @@ -589,7 +655,7 @@ private List mapEntries(MapInitializerList entries) { break; } Expr value = exprVisit(children.get(i++)); - result.add(helper.newMapEntry(colonID, key, value)); + result.add(helper.newMapEntry(colonID, key, value, optional)); if (i < children.size() && isToken(children.get(i), COMMA)) { i++; } @@ -775,4 +841,6 @@ private static T firstChildOfType(List children, Class } return null; } + + private record ListElements(List expressions, List optionalIndices) {} } diff --git a/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java b/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java index 9fd7fef21..2b651b900 100644 --- a/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java +++ b/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java @@ -18,12 +18,20 @@ import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.projectnessie.cel.Env.newEnv; +import static org.projectnessie.cel.common.types.BoolT.False; +import static org.projectnessie.cel.common.types.BoolT.True; +import static org.projectnessie.cel.common.types.IntT.intOf; +import static org.projectnessie.cel.common.types.NullT.NullValue; +import static org.projectnessie.cel.common.types.StringT.stringOf; import static org.projectnessie.cel.extension.OptionalLib.optionals; import com.google.api.expr.v1alpha1.Type; +import java.util.Map; import org.junit.jupiter.api.Test; import org.projectnessie.cel.Env; +import org.projectnessie.cel.Program; import org.projectnessie.cel.checker.Decls; +import org.projectnessie.cel.common.types.Err; class OptionalLibTest { @@ -48,6 +56,98 @@ void keepsNullableOptionalType() { assertCheckedType("[optional.of(1), null][0]", optional(Decls.Int)); } + @Test + void evaluatesPresentNull() { + assertEvaluates("optional.of(null).hasValue()", True); + assertEvaluates("optional.of(null).value()", NullValue); + } + + @Test + void evaluatesAbsentForNullZeroAndEmptyValues() { + assertEvaluates("optional.ofNonZeroValue(null).hasValue()", False); + assertEvaluates("optional.ofNonZeroValue(false).hasValue()", False); + assertEvaluates("optional.ofNonZeroValue(0).hasValue()", False); + assertEvaluates("optional.ofNonZeroValue(0u).hasValue()", False); + assertEvaluates("optional.ofNonZeroValue(0.0).hasValue()", False); + assertEvaluates("optional.ofNonZeroValue('').hasValue()", False); + assertEvaluates("optional.ofNonZeroValue([]).hasValue()", False); + assertEvaluates("optional.ofNonZeroValue({}).hasValue()", False); + } + + @Test + void evaluatesPresentForNonZeroValues() { + assertEvaluates("optional.ofNonZeroValue(true).value()", True); + assertEvaluates("optional.ofNonZeroValue(42).value()", intOf(42)); + assertEvaluates("optional.ofNonZeroValue('x').hasValue()", True); + } + + @Test + void evaluatesOrAndOrValue() { + assertEvaluates("optional.none().or(optional.none()).orValue(42)", intOf(42)); + assertEvaluates("optional.none().or(optional.of(21)).orValue(42)", intOf(21)); + assertEvaluates("optional.of(7).or(optional.of(21)).orValue(42)", intOf(7)); + } + + @Test + void evaluatesOptionalEquality() { + assertEvaluates("optional.none() == optional.none()", True); + assertEvaluates("optional.none() == optional.of(1)", False); + assertEvaluates("optional.of(1) == optional.none()", False); + assertEvaluates("optional.of(1) == optional.of(1)", True); + assertEvaluates("optional.none() != optional.none()", False); + assertEvaluates("optional.none() != optional.of(1)", True); + assertEvaluates("optional.of(1) != optional.none()", True); + assertEvaluates("optional.of(1) != optional.of(1)", False); + } + + @Test + void evaluatesOptionalTypeIdentifier() { + assertEvaluates("type(optional.none()) == optional_type", True); + } + + @Test + void evaluatesOptMap() { + assertEvaluates("optional.of(1).optMap(x, x + 1).value()", intOf(2)); + assertEvaluates("optional.ofNonZeroValue(0).optMap(x, x / 0).hasValue()", False); + } + + @Test + void evaluatesOptFlatMap() { + assertEvaluates("optional.of(1).optFlatMap(x, optional.of(x + 1)).value()", intOf(2)); + assertEvaluates( + "optional.ofNonZeroValue(0).optFlatMap(x, optional.of(x / 0)).hasValue()", False); + } + + @Test + void evaluatesOptionalSelectAndIndex() { + assertEvaluates("{}.?c.hasValue()", False); + assertEvaluates("{'c': 'x'}.?c.value()", stringOf("x")); + assertEvaluates("[][?0].hasValue()", False); + assertEvaluates("['foo'][?0].value()", stringOf("foo")); + } + + @Test + void evaluatesOptionalChaining() { + assertEvaluates( + "optional.of({'c': {}}).c.missing.or(optional.of(['list-value'])[0]).orValue('default value')", + stringOf("list-value")); + assertEvaluates( + "has(optional.of({'c': {'entry': 'hello world'}}).c)" + + " && !has(optional.of({'c': {'entry': 'hello world'}}).c.missing)", + True); + } + + @Test + void evaluatesOptionalAggregateEntries() { + assertEvaluates("[?{}.?c, ?optional.of(42), ?optional.none()].size()", intOf(1)); + assertEvaluates("{?'foo': optional.none()}.size()", intOf(0)); + } + + @Test + void absentValueReturnsError() { + assertThat(evaluate("optional.none().value()").getVal()).isInstanceOf(Err.class); + } + private static void assertCheckedType(String expression, Type expectedType) { Env env = newEnv(optionals()); Env.AstIssuesTuple parsed = env.parse(expression); @@ -58,6 +158,22 @@ private static void assertCheckedType(String expression, Type expectedType) { assertThat(checked.getAst().getResultType()).isEqualTo(expectedType); } + private static void assertEvaluates(String expression, Object expectedValue) { + assertThat(evaluate(expression).getVal()).describedAs(expression).isEqualTo(expectedValue); + } + + private static Program.EvalResult evaluate(String expression) { + Env env = newEnv(optionals()); + Env.AstIssuesTuple parsed = env.parse(expression); + assertThat(parsed.hasIssues()).isFalse(); + + Env.AstIssuesTuple checked = env.check(parsed.getAst()); + assertThat(checked.hasIssues()).describedAs(checked.getIssues().toString()).isFalse(); + + Program program = env.program(checked.getAst()); + return program.eval(Map.of()); + } + private static Type optional(Type type) { return Decls.newAbstractType("optional_type", singletonList(type)); } From b4211c20a53d56225b61e20e03d360ec8be4622d Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Thu, 17 Sep 2026 11:30:08 +0200 Subject: [PATCH 2/4] Fix optional CEL conformance edge cases Preserve lazy evaluation for optional.or() and optional.orValue(). Validate optional aggregate entries, selection types, and typed optional indexes. Treat zero durations and epoch timestamps as zero values. --- .../projectnessie/cel/checker/Checker.java | 88 +++++++++++++------ .../cel/common/types/OptionalT.java | 8 ++ .../cel/extension/OptionalLib.java | 58 +++++++++--- .../cel/interpreter/Interpretable.java | 79 +++++++++++++++++ .../cel/interpreter/InterpretablePlanner.java | 7 ++ .../cel/extension/OptionalLibTest.java | 33 +++++++ 6 files changed, 237 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/org/projectnessie/cel/checker/Checker.java b/core/src/main/java/org/projectnessie/cel/checker/Checker.java index 5fdc2cc29..5df5d15f1 100644 --- a/core/src/main/java/org/projectnessie/cel/checker/Checker.java +++ b/core/src/main/java/org/projectnessie/cel/checker/Checker.java @@ -56,6 +56,7 @@ import org.projectnessie.cel.common.Location; import org.projectnessie.cel.common.Source; import org.projectnessie.cel.common.containers.Container; +import org.projectnessie.cel.common.operators.Operator; import org.projectnessie.cel.common.types.Err.ErrException; import org.projectnessie.cel.common.types.ref.FieldType; import org.projectnessie.cel.parser.Parser.ParseResult; @@ -264,10 +265,23 @@ void checkSelect(Expr.Builder e) { } } + Type resultType = checkSelectField(e, sel.getOperandBuilder(), sel.getField(), false); + if (sel.getTestOnly()) { + resultType = Decls.Bool; + } + setType(e, resultType); + } + + private Type checkSelectField( + Expr.Builder e, Expr.Builder operand, String field, boolean optionalSelect) { // Interpret as field selection, first traversing down the operand. - check(sel.getOperandBuilder()); + check(operand); - Type targetType = getType(sel.getOperandBuilder()); + Type targetType = getType(operand); + boolean optionalOperand = isOptionalType(targetType); + if (optionalOperand) { + targetType = optionalValueType(targetType); + } // Assume error type by default as most types do not support field selection. Type resultType = Decls.Error; switch (kindOf(targetType)) { @@ -279,18 +293,13 @@ void checkSelect(Expr.Builder e) { case kindObject: // Objects yield their field type declaration as the selection result type, but only if // the field is defined. - FieldType fieldType = - lookupFieldType(location(e), targetType.getMessageType(), sel.getField()); + FieldType fieldType = lookupFieldType(location(e), targetType.getMessageType(), field); if (fieldType != null) { resultType = fieldType.type; } break; case kindAbstract: - if (isOptionalType(targetType)) { - resultType = Decls.newAbstractType("optional_type", Collections.singletonList(Decls.Dyn)); - } else { - errors.typeDoesNotSupportFieldSelection(location(e), targetType); - } + errors.typeDoesNotSupportFieldSelection(location(e), targetType); break; case kindTypeParam: // Set the operand type to DYN to prevent assignment to a potentionally incorrect type @@ -310,14 +319,16 @@ void checkSelect(Expr.Builder e) { } break; } - if (sel.getTestOnly()) { - resultType = Decls.Bool; + if (optionalOperand || optionalSelect) { + return Decls.newAbstractType("optional_type", Collections.singletonList(resultType)); } - setType(e, resultType); + return resultType; } private static boolean isOptionalType(Type type) { - return type.hasAbstractType() && "optional_type".equals(type.getAbstractType().getName()); + return type != null + && type.hasAbstractType() + && "optional_type".equals(type.getAbstractType().getName()); } private boolean isQualifiedLocalVariableSelection(Expr.Builder e) { @@ -338,6 +349,11 @@ void checkCall(Expr.Builder e) { List args = call.getArgsBuilderList(); String fnName = call.getFunction(); + if (fnName.equals(Operator.OptionalSelect.id)) { + checkOptionalSelect(e, call, args); + return; + } + // Traverse arguments. for (Expr.Builder arg : args) { check(arg); @@ -392,6 +408,27 @@ void checkCall(Expr.Builder e) { errors.undeclaredReference(location(e), env.container.name(), fnName); } + private void checkOptionalSelect(Expr.Builder e, Call.Builder call, List args) { + if (call.getTarget() != Expr.getDefaultInstance() || args.size() != 2) { + errors.noMatchingOverload(location(e), call.getFunction(), List.of(), false); + setType(e, Decls.Error); + return; + } + + Expr.Builder field = args.get(1); + check(field); + if (field.getExprKindCase() != Expr.ExprKindCase.CONST_EXPR + || field.getConstExpr().getConstantKindCase() != Constant.ConstantKindCase.STRING_VALUE) { + errors.typeMismatch(location(field), Decls.String, getType(field)); + setType(e, Decls.Error); + return; + } + + Type resultType = checkSelectField(e, args.get(0), field.getConstExpr().getStringValue(), true); + setType(e, resultType); + setReference(e, newFunctionReference(Collections.singletonList("optional_select"))); + } + void resolveOverloadOrError( Location loc, Expr.Builder e, Decl fn, Expr.Builder target, List args) { // Attempt to resolve the overload. @@ -489,10 +526,7 @@ void checkCreateList(Expr.Builder e) { check(el); Type type = getType(el); if (optionalIndices[i]) { - Type unwrapped = optionalValueType(type); - if (unwrapped != null) { - type = unwrapped; - } + type = unwrapOptionalEntry(location(el), type); } elemType = joinTypes(location(el), elemType, type); } @@ -525,10 +559,7 @@ void checkCreateMap(Expr.Builder e) { check(val); Type type = getType(val); if (ent.getOptionalEntry()) { - Type unwrapped = optionalValueType(type); - if (unwrapped != null) { - type = unwrapped; - } + type = unwrapOptionalEntry(location(val), type); } valueType = joinTypes(location(val), valueType, type); } @@ -584,10 +615,7 @@ void checkCreateMessage(Expr.Builder e) { } Type valueType = getType(value); if (ent.getOptionalEntry()) { - Type unwrapped = optionalValueType(valueType); - if (unwrapped != null) { - valueType = unwrapped; - } + valueType = unwrapOptionalEntry(location(value), valueType); } if (!isAssignable(fieldType, valueType)) { errors.fieldTypeMismatch(locationByID(ent.getId()), field, fieldType, getType(value)); @@ -602,6 +630,16 @@ private static Type optionalValueType(Type type) { return type.getAbstractType().getParameterTypes(0); } + private Type unwrapOptionalEntry(Location location, Type type) { + Type unwrapped = optionalValueType(type); + if (unwrapped != null || isDyn(type)) { + return unwrapped != null ? unwrapped : type; + } + errors.typeMismatch( + location, Decls.newAbstractType("optional_type", Collections.singletonList(type)), type); + return type; + } + void checkComprehension(Expr.Builder e) { Comprehension.Builder comp = e.getComprehensionExprBuilder(); check(comp.getIterRangeBuilder()); diff --git a/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java b/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java index 18d67086b..cb199406c 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java @@ -25,6 +25,9 @@ import static org.projectnessie.cel.common.types.TypeT.newObjectTypeValue; import com.google.protobuf.Message; +import java.time.Duration; +import java.time.Instant; +import java.time.ZonedDateTime; import java.util.Objects; import org.projectnessie.cel.common.types.ref.BaseVal; import org.projectnessie.cel.common.types.ref.Type; @@ -202,6 +205,11 @@ private static boolean isZeroValue(Val value) { return value.intValue() == 0L; case Double: return value.doubleValue() == 0.0d; + case Duration: + return Duration.ZERO.equals(value.value()); + case Timestamp: + return value.value() instanceof ZonedDateTime timestamp + && timestamp.toInstant().equals(Instant.EPOCH); case String: case Bytes: case List: diff --git a/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java b/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java index f56a8e59c..146473aba 100644 --- a/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java +++ b/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java @@ -55,13 +55,21 @@ public final class OptionalLib implements Library { private static final String OPTIONAL_OF_OVERLOAD = "optional_of"; private static final String OPTIONAL_OF_NON_ZERO_VALUE_OVERLOAD = "optional_of_non_zero_value"; private static final String OPTIONAL_SELECT_OVERLOAD = "optional_select"; - private static final String OPTIONAL_INDEX_OVERLOAD = "optional_index"; - private static final String OPTIONAL_INDEX_OPTIONAL_OVERLOAD = "optional_index_optional"; + private static final String OPTIONAL_LIST_INDEX_OVERLOAD = "optional_list_index"; + private static final String OPTIONAL_OPTIONAL_LIST_INDEX_OVERLOAD = + "optional_optional_list_index"; + private static final String OPTIONAL_MAP_INDEX_OVERLOAD = "optional_map_index"; + private static final String OPTIONAL_OPTIONAL_MAP_INDEX_OVERLOAD = "optional_optional_map_index"; + private static final String OPTIONAL_LIST_INDEX_OPTIONAL_OVERLOAD = + "optional_list_index_optional"; + private static final String OPTIONAL_MAP_INDEX_OPTIONAL_OVERLOAD = "optional_map_index_optional"; private static final String OPTIONAL_HAS_VALUE_OVERLOAD = "optional_has_value"; private static final String OPTIONAL_VALUE_OVERLOAD = "optional_value"; private static final String OPTIONAL_OR_OVERLOAD = "optional_or"; private static final String OPTIONAL_OR_VALUE_OVERLOAD = "optional_or_value"; private static final String TYPE_PARAM_A = "A"; + private static final String TYPE_PARAM_K = "K"; + private static final String TYPE_PARAM_V = "V"; private static final String OPTIONAL_MACRO_TARGET = "@optional_target"; private static final String OPTIONAL_MACRO_RESULT = "@optional_result"; @@ -76,6 +84,13 @@ public List getCompileOptions() { var typeParamA = Decls.newTypeParamType(TYPE_PARAM_A); var optionalA = Decls.newAbstractType(OPTIONAL_TYPE, singletonList(typeParamA)); var typeParams = singletonList(TYPE_PARAM_A); + var typeParamK = Decls.newTypeParamType(TYPE_PARAM_K); + var typeParamV = Decls.newTypeParamType(TYPE_PARAM_V); + var listA = Decls.newListType(typeParamA); + var optionalListA = Decls.newAbstractType(OPTIONAL_TYPE, singletonList(listA)); + var mapKV = Decls.newMapType(typeParamK, typeParamV); + var optionalMapKV = Decls.newAbstractType(OPTIONAL_TYPE, singletonList(mapKV)); + var optionalValueV = Decls.newAbstractType(OPTIONAL_TYPE, singletonList(typeParamV)); return List.of( EnvOption.types(singletonList(OptionalType)), @@ -107,17 +122,35 @@ OPTIONAL_OF_OVERLOAD, singletonList(typeParamA), optionalA, typeParams)), Decls.newAbstractType(OPTIONAL_TYPE, singletonList(Decls.Dyn)))), Decls.newFunction( Operator.OptionalIndex.id, - Decls.newOverload( - OPTIONAL_INDEX_OVERLOAD, - List.of(Decls.Dyn, Decls.Dyn), - Decls.newAbstractType(OPTIONAL_TYPE, singletonList(Decls.Dyn)))), + Decls.newParameterizedOverload( + OPTIONAL_LIST_INDEX_OVERLOAD, List.of(listA, Decls.Int), optionalA, typeParams), + Decls.newParameterizedOverload( + OPTIONAL_OPTIONAL_LIST_INDEX_OVERLOAD, + List.of(optionalListA, Decls.Int), + optionalA, + typeParams), + Decls.newParameterizedOverload( + OPTIONAL_MAP_INDEX_OVERLOAD, + List.of(mapKV, typeParamK), + optionalValueV, + List.of(TYPE_PARAM_K, TYPE_PARAM_V)), + Decls.newParameterizedOverload( + OPTIONAL_OPTIONAL_MAP_INDEX_OVERLOAD, + List.of(optionalMapKV, typeParamK), + optionalValueV, + List.of(TYPE_PARAM_K, TYPE_PARAM_V))), Decls.newFunction( Operator.Index.id, Decls.newParameterizedOverload( - OPTIONAL_INDEX_OPTIONAL_OVERLOAD, - List.of(optionalA, Decls.Dyn), - Decls.newAbstractType(OPTIONAL_TYPE, singletonList(Decls.Dyn)), - typeParams)), + OPTIONAL_LIST_INDEX_OPTIONAL_OVERLOAD, + List.of(optionalListA, Decls.Int), + optionalA, + typeParams), + Decls.newParameterizedOverload( + OPTIONAL_MAP_INDEX_OPTIONAL_OVERLOAD, + List.of(optionalMapKV, typeParamK), + optionalValueV, + List.of(TYPE_PARAM_K, TYPE_PARAM_V))), Decls.newFunction( OPTIONAL_HAS_VALUE, Decls.newParameterizedInstanceOverload( @@ -152,7 +185,10 @@ public List getProgramOptions() { Overload.binary(Operator.OptionalSelect.id, OptionalT::optionalSelect), Overload.binary(OPTIONAL_SELECT_OVERLOAD, OptionalT::optionalSelect), Overload.binary(Operator.OptionalIndex.id, OptionalT::optionalIndex), - Overload.binary(OPTIONAL_INDEX_OVERLOAD, OptionalT::optionalIndex))); + Overload.binary(OPTIONAL_LIST_INDEX_OVERLOAD, OptionalT::optionalIndex), + Overload.binary(OPTIONAL_OPTIONAL_LIST_INDEX_OVERLOAD, OptionalT::optionalIndex), + Overload.binary(OPTIONAL_MAP_INDEX_OVERLOAD, OptionalT::optionalIndex), + Overload.binary(OPTIONAL_OPTIONAL_MAP_INDEX_OVERLOAD, OptionalT::optionalIndex))); } private static Expr makeOptMap(ExprHelper eh, Expr target, List args) { diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java b/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java index a35737b80..0da9f719c 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/Interpretable.java @@ -433,6 +433,85 @@ public String toString() { } } + final class EvalOptionalOr extends AbstractEvalLhsRhs implements InterpretableCall { + private final String function; + private final String overload; + private final boolean returnValue; + + EvalOptionalOr( + long id, + String function, + String overload, + Interpretable lhs, + Interpretable rhs, + boolean returnValue) { + super(id, lhs, rhs); + this.function = Objects.requireNonNull(function); + this.overload = Objects.requireNonNull(overload); + this.returnValue = returnValue; + } + + @Override + public Val eval(org.projectnessie.cel.interpreter.Activation ctx) { + Val left = lhs.eval(ctx); + if (isUnknownOrError(left)) { + return left; + } + if (left instanceof OptionalT optional && optional.hasValue()) { + return returnValue ? optional.getValue() : optional; + } + + Val right = rhs.eval(ctx); + if (isUnknownOrError(right)) { + return right; + } + if (left.type().hasTrait(Trait.ReceiverType)) { + return ((Receiver) left).receive(function, overload, right); + } + return noSuchOverload(left, function, overload, new Val[] {right}); + } + + @Override + public Cost cost() { + return calShortCircuitBinaryOpsCost(lhs, rhs); + } + + @Override + public String function() { + return function; + } + + @Override + public String overloadID() { + return overload; + } + + @Override + public Interpretable[] args() { + return new Interpretable[] {lhs, rhs}; + } + + @Override + public String toString() { + return "EvalOptionalOr{" + + "id=" + + id + + ", lhs=" + + lhs + + ", rhs=" + + rhs + + ", function='" + + function + + '\'' + + ", overload='" + + overload + + '\'' + + ", returnValue=" + + returnValue + + '}'; + } + } + static Cost calShortCircuitBinaryOpsCost(Interpretable lhs, Interpretable rhs) { Cost l = estimateCost(lhs); Cost r = estimateCost(rhs); diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java b/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java index 8c9b48a96..b427378c6 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java @@ -62,6 +62,7 @@ import org.projectnessie.cel.interpreter.Interpretable.EvalMapFold; import org.projectnessie.cel.interpreter.Interpretable.EvalNe; import org.projectnessie.cel.interpreter.Interpretable.EvalObj; +import org.projectnessie.cel.interpreter.Interpretable.EvalOptionalOr; import org.projectnessie.cel.interpreter.Interpretable.EvalOr; import org.projectnessie.cel.interpreter.Interpretable.EvalReceiverVarArgs; import org.projectnessie.cel.interpreter.Interpretable.EvalTestOnly; @@ -394,6 +395,12 @@ Interpretable planCall(Expr expr) { if (resolvedFunc.fnName.equals(Operator.Equals.id)) return planCallEqual(expr, args); if (resolvedFunc.fnName.equals(Operator.NotEquals.id)) return planCallNotEqual(expr, args); if (resolvedFunc.fnName.equals(Operator.Index.id)) return planCallIndex(expr, args); + if ("optional_or".equals(resolvedFunc.overloadId)) + return new EvalOptionalOr( + expr.getId(), resolvedFunc.fnName, resolvedFunc.overloadId, args[0], args[1], false); + if ("optional_or_value".equals(resolvedFunc.overloadId)) + return new EvalOptionalOr( + expr.getId(), resolvedFunc.fnName, resolvedFunc.overloadId, args[0], args[1], true); // Otherwise, generate Interpretable calls specialized by argument count. // Try to find the specific function by overload id. diff --git a/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java b/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java index 2b651b900..86e3f3044 100644 --- a/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java +++ b/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java @@ -72,6 +72,8 @@ void evaluatesAbsentForNullZeroAndEmptyValues() { assertEvaluates("optional.ofNonZeroValue('').hasValue()", False); assertEvaluates("optional.ofNonZeroValue([]).hasValue()", False); assertEvaluates("optional.ofNonZeroValue({}).hasValue()", False); + assertEvaluates("optional.ofNonZeroValue(duration('0s')).hasValue()", False); + assertEvaluates("optional.ofNonZeroValue(timestamp(0)).hasValue()", False); } @Test @@ -86,6 +88,8 @@ void evaluatesOrAndOrValue() { assertEvaluates("optional.none().or(optional.none()).orValue(42)", intOf(42)); assertEvaluates("optional.none().or(optional.of(21)).orValue(42)", intOf(21)); assertEvaluates("optional.of(7).or(optional.of(21)).orValue(42)", intOf(7)); + assertEvaluates("optional.of(7).or(optional.of(1 / 0)).value()", intOf(7)); + assertEvaluates("optional.of(7).orValue(1 / 0)", intOf(7)); } @Test @@ -124,6 +128,10 @@ void evaluatesOptionalSelectAndIndex() { assertEvaluates("{'c': 'x'}.?c.value()", stringOf("x")); assertEvaluates("[][?0].hasValue()", False); assertEvaluates("['foo'][?0].value()", stringOf("foo")); + assertCheckedType("['foo'][?0]", optional(Decls.String)); + assertCheckedType("{'foo': 1}[?'foo']", optional(Decls.Int)); + assertCheckedType("optional.of(['foo'])[0]", optional(Decls.String)); + assertCheckedType("optional.of({'foo': 1})['foo']", optional(Decls.Int)); } @Test @@ -141,6 +149,22 @@ void evaluatesOptionalChaining() { void evaluatesOptionalAggregateEntries() { assertEvaluates("[?{}.?c, ?optional.of(42), ?optional.none()].size()", intOf(1)); assertEvaluates("{?'foo': optional.none()}.size()", intOf(0)); + assertCheckFails("[?1]"); + assertCheckFails("{?'foo': 1}"); + } + + @Test + void checksOptionalSelectionsAgainstContainedTypes() { + assertCheckedType("optional.of({'field': 1}).field", optional(Decls.Int)); + assertCheckedType("{'field': 1}.?field", optional(Decls.Int)); + assertCheckFails("optional.of(1).missing"); + assertCheckFails("1.?missing"); + } + + @Test + void rejectsInvalidOptionalIndexes() { + assertCheckFails("optional.of(1)[0]"); + assertCheckFails("['foo'][?'foo']"); } @Test @@ -162,6 +186,15 @@ private static void assertEvaluates(String expression, Object expectedValue) { assertThat(evaluate(expression).getVal()).describedAs(expression).isEqualTo(expectedValue); } + private static void assertCheckFails(String expression) { + Env env = newEnv(optionals()); + Env.AstIssuesTuple parsed = env.parse(expression); + assertThat(parsed.hasIssues()).describedAs(expression).isFalse(); + + Env.AstIssuesTuple checked = env.check(parsed.getAst()); + assertThat(checked.hasIssues()).describedAs(expression).isTrue(); + } + private static Program.EvalResult evaluate(String expression) { Env env = newEnv(optionals()); Env.AstIssuesTuple parsed = env.parse(expression); From fc43773a87b351d860f028d20ff9fa8f49a342e1 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Thu, 17 Sep 2026 13:16:58 +0200 Subject: [PATCH 3/4] Fix optional access error handling Return invalid list-index errors instead of wrapping them as present optional values. Preserve lazy optional fallbacks for parse-only expressions. --- .../cel/common/types/OptionalT.java | 8 +++++-- .../cel/extension/OptionalLib.java | 20 +++++++++++++++++- .../cel/interpreter/InterpretablePlanner.java | 10 +++++++-- .../cel/extension/OptionalLibTest.java | 21 +++++++++++++++++++ 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java b/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java index cb199406c..838af151e 100644 --- a/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java +++ b/core/src/main/java/org/projectnessie/cel/common/types/OptionalT.java @@ -243,7 +243,10 @@ private static Val optionalAccess(Val operand, Val index) { } if (operand instanceof Indexer) { Val value = ((Indexer) operand).get(index); - return isMissingAccess(value) ? none() : of(value); + if (isMissingAccess(value)) { + return none(); + } + return value instanceof Err ? value : of(value); } return noSuchOverload(operand, "optional access", index); } @@ -255,7 +258,8 @@ private static boolean isMissingAccess(Val value) { String error = value.toString(); return error.startsWith("no such key") || error.startsWith("no such field") - || error.startsWith("invalid_argument") + || (error.startsWith("invalid_argument: index ") + && error.contains(" out of range in list ")) || error.startsWith("index out of bounds"); } diff --git a/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java b/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java index 146473aba..7f46375b4 100644 --- a/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java +++ b/core/src/main/java/org/projectnessie/cel/extension/OptionalLib.java @@ -17,6 +17,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; +import static org.projectnessie.cel.common.types.Err.noSuchOverload; import static org.projectnessie.cel.common.types.OptionalT.OptionalType; import com.google.api.expr.v1alpha1.Expr; @@ -30,6 +31,7 @@ import org.projectnessie.cel.common.Location; import org.projectnessie.cel.common.operators.Operator; import org.projectnessie.cel.common.types.OptionalT; +import org.projectnessie.cel.common.types.ref.Val; import org.projectnessie.cel.interpreter.functions.Overload; import org.projectnessie.cel.parser.ExprHelper; import org.projectnessie.cel.parser.Macro; @@ -188,7 +190,23 @@ public List getProgramOptions() { Overload.binary(OPTIONAL_LIST_INDEX_OVERLOAD, OptionalT::optionalIndex), Overload.binary(OPTIONAL_OPTIONAL_LIST_INDEX_OVERLOAD, OptionalT::optionalIndex), Overload.binary(OPTIONAL_MAP_INDEX_OVERLOAD, OptionalT::optionalIndex), - Overload.binary(OPTIONAL_OPTIONAL_MAP_INDEX_OVERLOAD, OptionalT::optionalIndex))); + Overload.binary(OPTIONAL_OPTIONAL_MAP_INDEX_OVERLOAD, OptionalT::optionalIndex), + Overload.binary(OPTIONAL_OR_OVERLOAD, OptionalLib::optionalOr), + Overload.binary(OPTIONAL_OR_VALUE_OVERLOAD, OptionalLib::optionalOrValue))); + } + + private static Val optionalOr(Val optional, Val alternative) { + if (optional instanceof OptionalT optionalValue) { + return optionalValue.receive(OPTIONAL_OR, OPTIONAL_OR_OVERLOAD, alternative); + } + return noSuchOverload(optional, OPTIONAL_OR, alternative); + } + + private static Val optionalOrValue(Val optional, Val alternative) { + if (optional instanceof OptionalT optionalValue) { + return optionalValue.receive(OPTIONAL_OR_VALUE, OPTIONAL_OR_VALUE_OVERLOAD, alternative); + } + return noSuchOverload(optional, OPTIONAL_OR_VALUE, alternative); } private static Expr makeOptMap(ExprHelper eh, Expr target, List args) { diff --git a/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java b/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java index b427378c6..07a72709e 100644 --- a/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java +++ b/core/src/main/java/org/projectnessie/cel/interpreter/InterpretablePlanner.java @@ -395,10 +395,16 @@ Interpretable planCall(Expr expr) { if (resolvedFunc.fnName.equals(Operator.Equals.id)) return planCallEqual(expr, args); if (resolvedFunc.fnName.equals(Operator.NotEquals.id)) return planCallNotEqual(expr, args); if (resolvedFunc.fnName.equals(Operator.Index.id)) return planCallIndex(expr, args); - if ("optional_or".equals(resolvedFunc.overloadId)) + if ("optional_or".equals(resolvedFunc.overloadId) + || (resolvedFunc.overloadId.isEmpty() + && "or".equals(resolvedFunc.fnName) + && disp.findOverload("optional_or") != null)) return new EvalOptionalOr( expr.getId(), resolvedFunc.fnName, resolvedFunc.overloadId, args[0], args[1], false); - if ("optional_or_value".equals(resolvedFunc.overloadId)) + if ("optional_or_value".equals(resolvedFunc.overloadId) + || (resolvedFunc.overloadId.isEmpty() + && "orValue".equals(resolvedFunc.fnName) + && disp.findOverload("optional_or_value") != null)) return new EvalOptionalOr( expr.getId(), resolvedFunc.fnName, resolvedFunc.overloadId, args[0], args[1], true); diff --git a/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java b/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java index 86e3f3044..f1ecbb88b 100644 --- a/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java +++ b/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java @@ -92,6 +92,12 @@ void evaluatesOrAndOrValue() { assertEvaluates("optional.of(7).orValue(1 / 0)", intOf(7)); } + @Test + void evaluatesParseOnlyOrAndOrValue() { + assertEvaluatesParsed("optional.of(7).or(optional.of(1 / 0)).value()", intOf(7)); + assertEvaluatesParsed("optional.of(7).orValue(1 / 0)", intOf(7)); + } + @Test void evaluatesOptionalEquality() { assertEvaluates("optional.none() == optional.none()", True); @@ -165,6 +171,7 @@ void checksOptionalSelectionsAgainstContainedTypes() { void rejectsInvalidOptionalIndexes() { assertCheckFails("optional.of(1)[0]"); assertCheckFails("['foo'][?'foo']"); + assertThat(evaluate("['foo'][?dyn(1.5)]").getVal()).isInstanceOf(Err.class); } @Test @@ -186,6 +193,12 @@ private static void assertEvaluates(String expression, Object expectedValue) { assertThat(evaluate(expression).getVal()).describedAs(expression).isEqualTo(expectedValue); } + private static void assertEvaluatesParsed(String expression, Object expectedValue) { + assertThat(evaluateParsed(expression).getVal()) + .describedAs(expression) + .isEqualTo(expectedValue); + } + private static void assertCheckFails(String expression) { Env env = newEnv(optionals()); Env.AstIssuesTuple parsed = env.parse(expression); @@ -207,6 +220,14 @@ private static Program.EvalResult evaluate(String expression) { return program.eval(Map.of()); } + private static Program.EvalResult evaluateParsed(String expression) { + Env env = newEnv(optionals()); + Env.AstIssuesTuple parsed = env.parse(expression); + assertThat(parsed.hasIssues()).describedAs(expression).isFalse(); + + return env.program(parsed.getAst()).eval(Map.of()); + } + private static Type optional(Type type) { return Decls.newAbstractType("optional_type", singletonList(type)); } From f462dc95baef8108dca7e235b3b2c986b5196861 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Thu, 17 Sep 2026 13:23:35 +0200 Subject: [PATCH 4/4] Validate optional select integration Assign a distinct ID to synthesized optional-select fields and reject optional-select syntax when the optional library is not configured. --- .../org/projectnessie/cel/checker/Checker.java | 9 +++++++++ .../java/org/projectnessie/cel/parser/Parser.java | 5 +++-- .../cel/extension/OptionalLibTest.java | 14 ++++++++++++++ .../org/projectnessie/cel/parser/ParserTest.java | 10 ++++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/projectnessie/cel/checker/Checker.java b/core/src/main/java/org/projectnessie/cel/checker/Checker.java index 5df5d15f1..e167826db 100644 --- a/core/src/main/java/org/projectnessie/cel/checker/Checker.java +++ b/core/src/main/java/org/projectnessie/cel/checker/Checker.java @@ -350,6 +350,15 @@ void checkCall(Expr.Builder e) { String fnName = call.getFunction(); if (fnName.equals(Operator.OptionalSelect.id)) { + Decl fn = env.lookupFunction(fnName); + if (fn == null) { + for (Expr.Builder arg : args) { + check(arg); + } + errors.undeclaredReference(location(e), env.container.name(), fnName); + setType(e, Decls.Error); + return; + } checkOptionalSelect(e, call, args); return; } diff --git a/core/src/main/java/org/projectnessie/cel/parser/Parser.java b/core/src/main/java/org/projectnessie/cel/parser/Parser.java index 9621933f9..ef2623e6b 100644 --- a/core/src/main/java/org/projectnessie/cel/parser/Parser.java +++ b/core/src/main/java/org/projectnessie/cel/parser/Parser.java @@ -388,7 +388,8 @@ public Expr visitMember(Node node) { optional = true; optionalNode = children.get(i++); } - String id = fieldName(children.get(i++)); + Node field = children.get(i++); + String id = fieldName(field); if (i < children.size() && isToken(children.get(i), LPAREN)) { if (optional) { return reportError(optionalNode, "optional select does not support function calls"); @@ -409,7 +410,7 @@ public Expr visitMember(Node node) { helper.id(optionalNode), Operator.OptionalSelect.id, operand, - helper.newLiteralString(optionalNode, id)); + helper.newLiteralString(field, id)); } else { operand = helper.newSelect(op, operand, id); } diff --git a/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java b/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java index f1ecbb88b..6ac5fd33f 100644 --- a/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java +++ b/core/src/test/java/org/projectnessie/cel/extension/OptionalLibTest.java @@ -167,6 +167,11 @@ void checksOptionalSelectionsAgainstContainedTypes() { assertCheckFails("1.?missing"); } + @Test + void rejectsOptionalSelectionsWithoutTheOptionalLibrary() { + assertCheckFailsWithoutOptionals("{}.?field"); + } + @Test void rejectsInvalidOptionalIndexes() { assertCheckFails("optional.of(1)[0]"); @@ -208,6 +213,15 @@ private static void assertCheckFails(String expression) { assertThat(checked.hasIssues()).describedAs(expression).isTrue(); } + private static void assertCheckFailsWithoutOptionals(String expression) { + Env env = newEnv(); + Env.AstIssuesTuple parsed = env.parse(expression); + assertThat(parsed.hasIssues()).describedAs(expression).isFalse(); + + Env.AstIssuesTuple checked = env.check(parsed.getAst()); + assertThat(checked.hasIssues()).describedAs(expression).isTrue(); + } + private static Program.EvalResult evaluate(String expression) { Env env = newEnv(optionals()); Env.AstIssuesTuple parsed = env.parse(expression); diff --git a/core/src/test/java/org/projectnessie/cel/parser/ParserTest.java b/core/src/test/java/org/projectnessie/cel/parser/ParserTest.java index a8131e2e6..e719a0d8b 100644 --- a/core/src/test/java/org/projectnessie/cel/parser/ParserTest.java +++ b/core/src/test/java/org/projectnessie/cel/parser/ParserTest.java @@ -1360,6 +1360,16 @@ void expressionSizeCodePointLimit() { "expression code point size exceeds limit: size: 3, limit 2")); } + @Test + void optionalSelectUsesDistinctExpressionIds() { + ParseResult parsed = Parser.parseAllMacros(Source.newTextSource("target.?field")); + assertThat(parsed.getErrors().getErrors()).isEmpty(); + + Expr optionalSelect = parsed.getExpr(); + assertThat(optionalSelect.getId()) + .isNotEqualTo(optionalSelect.getCallExpr().getArgs(1).getId()); + } + static class KindAndIdAdorner implements Debug.Adorner { @Override