Avoid concatenating superfluous empty list for aggregate semantics

PiperOrigin-RevId: 966065339

Author: l46kok

policy/src/main/java/dev/cel/policy/RuleComposer.java

@@ -148,8 +148,12 @@ private Step optimizeRule(Cel cel, CelCompiledRule compiledRule, boolean asList)
 
   private @Nullable Step createBaseStep(boolean returnList, boolean hasOptionalOutput) {
     if (returnList) {
-      // If the rule is evaluated as a list (AGGREGATE), the base case is an empty list.
-      return Step.newUnconditionalNonOptionalStep(newTrueLiteral(), newList());
+      if (hasOptionalOutput) {
+        // If a nested rule inside an aggregate context has an optional output, the last result in
+        // the ternary should return an empty list to allow concatenation with other branches.
+        return Step.newUnconditionalNonOptionalStep(newTrueLiteral(), newList());
+      }
+      return null;
     }
 
     if (hasOptionalOutput) {
@@ -280,6 +284,10 @@ private Step combineAggregate(AstMutator astMutator, Step currentStep, Step accu
       conditionalListPart = currentListPart;
     }
 
+    if (accumulatedStep == null) {
+      return Step.newUnconditionalNonOptionalStep(trueCondition, conditionalListPart);
+    }
+
     CelMutableAst concatenated =
         astMutator.newGlobalCall(
             Operator.ADD.getFunction(), conditionalListPart, accumulatedStep.expr);

policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java

@@ -261,8 +261,127 @@ public void compileYamlPolicy_aggregate_macrosPreserved() throws Exception {
     String unparsed = CelUnparserFactory.newUnparser().unparse(ast);
     assertThat(unparsed)
         .isEqualTo(
-            "(cond ? [payload.filter(x, x > 10, x).exists(y, y % 2 == 0)] : []) "
-                + "+ ([payload.all(x, x > 0)] + [])");
+            "(cond ? [payload.filter(x, x > 10).exists(y, y % 2 == 0)] : []) "
+                + "+ [payload.all(x, x > 0)]");
+  }
+
+  @Test
+  public void compileYamlPolicy_aggregateSingleMatch_noSuperfluousConcatenation() throws Exception {
+    String policySource =
+        "name: aggregate_single_match\n"
+            + "rule:\n"
+            + "  aggregate:\n"
+            + "    - condition: \"cond\"\n"
+            + "      output: \"payload.filter(x, x > 10).exists(y, y % 2 == 0)\"\n";
+    Cel cel =
+        newCel()
+            .toCelBuilder()
+            .addVar("cond", SimpleType.BOOL)
+            .addVar("payload", ListType.create(SimpleType.INT))
+            .build();
+
+    CelPolicy policy = POLICY_PARSER.parse(policySource);
+
+    CelAbstractSyntaxTree ast =
+        CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy);
+
+    String unparsed = CelUnparserFactory.newUnparser().unparse(ast);
+    assertThat(unparsed).isEqualTo("cond ? [payload.filter(x, x > 10).exists(y, y % 2 == 0)] : []");
+  }
+
+  @Test
+  public void compileYamlPolicy_aggregateMultipleConditionalMatches_noSuperfluousConcatenation()
+      throws Exception {
+    String policySource =
+        "name: aggregate_multiple_conditional\n"
+            + "rule:\n"
+            + "  aggregate:\n"
+            + "    - condition: \"cond1\"\n"
+            + "      output: \"payload.all(x, x > 0)\"\n"
+            + "    - condition: \"cond2\"\n"
+            + "      output: \"payload.exists(x, x == 0)\"\n";
+    Cel cel =
+        newCel()
+            .toCelBuilder()
+            .addVar("cond1", SimpleType.BOOL)
+            .addVar("cond2", SimpleType.BOOL)
+            .addVar("payload", ListType.create(SimpleType.INT))
+            .build();
+
+    CelPolicy policy = POLICY_PARSER.parse(policySource);
+
+    CelAbstractSyntaxTree ast =
+        CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy);
+
+    String unparsed = CelUnparserFactory.newUnparser().unparse(ast);
+    assertThat(unparsed)
+        .isEqualTo(
+            "(cond1 ? [payload.all(x, x > 0)] : []) + (cond2 ? [payload.exists(x, x == 0)] : [])");
+  }
+
+  @Test
+  public void evaluateYamlPolicy_aggregateWithNestedMatch_optionalFallback() throws Exception {
+    String policySource =
+        "name: nested_match_in_aggregate\n"
+            + "rule:\n"
+            + "  aggregate:\n"
+            + "    - condition: \"cond\"\n"
+            + "      rule:\n"
+            + "        match:\n"
+            + "          - condition: \"x > 10\"\n"
+            + "            output: \"'HIGH'\"\n"
+            + "          - condition: \"x > 0\"\n"
+            + "            output: \"'LOW'\"\n"
+            + "    - condition: \"true\"\n"
+            + "      output: \"'DEFAULT'\"\n";
+    Cel cel =
+        newCel().toCelBuilder().addVar("cond", SimpleType.BOOL).addVar("x", SimpleType.INT).build();
+
+    CelPolicy policy = POLICY_PARSER.parse(policySource);
+
+    CelAbstractSyntaxTree ast =
+        CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy);
+
+    String unparsed = CelUnparserFactory.newUnparser().unparse(ast);
+    assertThat(unparsed)
+        .isEqualTo(
+            "(cond ? ((x > 10) ? [\"HIGH\"] : ((x > 0) ? [\"LOW\"] : [])) : []) + [\"DEFAULT\"]");
+
+    assertThat(cel.createProgram(ast).eval(ImmutableMap.of("cond", true, "x", 15)))
+        .isEqualTo(ImmutableList.of("HIGH", "DEFAULT"));
+    assertThat(cel.createProgram(ast).eval(ImmutableMap.of("cond", true, "x", 5)))
+        .isEqualTo(ImmutableList.of("LOW", "DEFAULT"));
+    assertThat(cel.createProgram(ast).eval(ImmutableMap.of("cond", true, "x", -1)))
+        .isEqualTo(ImmutableList.of("DEFAULT"));
+    assertThat(cel.createProgram(ast).eval(ImmutableMap.of("cond", false, "x", 15)))
+        .isEqualTo(ImmutableList.of("DEFAULT"));
+  }
+
+  @Test
+  public void evaluateYamlPolicy_aggregateSingleConditionalMatchNestedInAggregate()
+      throws Exception {
+    String policySource =
+        "name: single_conditional_match_nested_in_aggregate\n"
+            + "rule:\n"
+            + "  aggregate:\n"
+            + "    - condition: \"true\"\n"
+            + "      rule:\n"
+            + "        match:\n"
+            + "          - condition: \"x > 10\"\n"
+            + "            output: \"'GT10'\"\n";
+    Cel cel = newCel().toCelBuilder().addVar("x", SimpleType.INT).build();
+
+    CelPolicy policy = POLICY_PARSER.parse(policySource);
+
+    CelAbstractSyntaxTree ast =
+        CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy);
+
+    String unparsed = CelUnparserFactory.newUnparser().unparse(ast);
+    assertThat(unparsed).isEqualTo("(x > 10) ? [\"GT10\"] : []");
+
+    assertThat(cel.createProgram(ast).eval(ImmutableMap.of("x", 5))).isEqualTo(ImmutableList.of());
+    assertThat(cel.createProgram(ast).eval(ImmutableMap.of("x", 15)))
+        .isEqualTo(ImmutableList.of("GT10"));
   }
 
   @Test