Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions policy/src/main/java/dev/cel/policy/CelCompiledRule.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,17 @@ public boolean hasOptionalOutput() {
for (CelCompiledMatch match : matches()) {
if (match.result().kind().equals(CelCompiledMatch.Result.Kind.RULE)
&& match.result().rule().hasOptionalOutput()) {
return true;
}

if (match.isConditionTriviallyTrue()) {
// If the nested rule is unconditional, the matching may fallthrough to the next match
// in this context (unwrapping the optional value from the nested rule).
if (!match.isConditionTriviallyTrue()) {
return true;
}
isOptionalOutput = true;
} else if (match.isConditionTriviallyTrue()) {
return false;
} else {
isOptionalOutput = true;
}

isOptionalOutput = true;
}

return isOptionalOutput;
Expand Down
11 changes: 9 additions & 2 deletions policy/src/main/java/dev/cel/policy/CelPolicyCompilerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -249,14 +249,21 @@ private CelCompiledRule compileRuleImpl(
}

private void checkUnreachableCode(CelCompiledRule compiledRule, CompilerContext compilerContext) {
boolean ruleHasOptional = compiledRule.hasOptionalOutput();
ImmutableList<CelCompiledMatch> compiledMatches = compiledRule.matches();
int matchCount = compiledMatches.size();
for (int i = matchCount - 1; i >= 0; i--) {
CelCompiledMatch compiledMatch = compiledMatches.get(i);
boolean isTriviallyTrue = compiledMatch.isConditionTriviallyTrue();

if (isTriviallyTrue && !ruleHasOptional && i != matchCount - 1) {
// If the match is a single output or a nested rule that always returns a value, it is
// exhaustive. If the condition is trivially true, then all subsequent branches are
// unreachable.
boolean isExhaustive =
isTriviallyTrue
&& (compiledMatch.result().kind().equals(Kind.OUTPUT)
|| !compiledMatch.result().rule().hasOptionalOutput());

if (isExhaustive && i != matchCount - 1) {
if (compiledMatch.result().kind().equals(Kind.OUTPUT)) {
compilerContext.addIssue(
compiledMatch.sourceId(),
Expand Down
26 changes: 26 additions & 0 deletions policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,32 @@ public void compileYamlPolicy_withImportsOnNestedRules() throws Exception {
assertThat(ast.getResultType()).isEqualTo(OptionalType.create(SimpleType.BOOL));
}

@Test
public void compileYamlPolicy_nestedRuleOptionalFallbackDivergence() throws Exception {
Cel cel = newCel().toCelBuilder().addVar("input_val", SimpleType.INT).build();
String policySource =
"name: grandparent_policy\n"
+ "rule:\n"
+ " match:\n"
+ " - condition: 'input_val == 2'\n" // conditional grandparent match
+ " rule:\n"
+ " id: parent_rule\n"
+ " match:\n"
+ " - condition: 'input_val == 1'\n" // conditional parent match
+ " rule:\n"
+ " id: nested_rule\n"
+ " match:\n"
+ " - condition: 'input_val == 3'\n"
+ " output: 'true'\n"
+ " - output: 'true'\n" // fallback (optional)
+ " - output: 'true'\n";
CelPolicy policy = POLICY_PARSER.parse(policySource);
CelAbstractSyntaxTree ast =
CelPolicyCompilerFactory.newPolicyCompiler(cel).build().compile(policy);

assertThat(ast.getResultType()).isEqualTo(OptionalType.create(SimpleType.BOOL));
}

@Test
public void compileYamlPolicy_containsCompilationError_throws(
@TestParameter TestErrorYamlPolicy testCase) throws Exception {
Expand Down
Loading