Skip to content

ast: name the enclosing rule in unsafe var errors on shared lines - #9011

Merged
sspaink merged 2 commits into
open-policy-agent:mainfrom
sspaink:fix/4967-unsafe-var-enclosing-rule
Aug 13, 2026
Merged

ast: name the enclosing rule in unsafe var errors on shared lines#9011
sspaink merged 2 commits into
open-policy-agent:mainfrom
sspaink:fix/4967-unsafe-var-enclosing-rule

Conversation

@sspaink

@sspaink sspaink commented Aug 12, 2026

Copy link
Copy Markdown
Member

fix: #4967

Safety errors now name the enclosing rule when its line hold a rule of a different name.

Similar to the existing evaluation compliance tests (v1/test/cases), added v1/test/compilecases to be a place we can put compile compliance tests. Just the test cases covering this change. If we are happy with this layout I can follow up with adding more tests.

Rego allows several rules to be declared on one line, so the location in
an unsafe var error doesn't always identify the rule the var was found
in -- most confusingly when an identifier meant as a keyword isn't one in
the OPA version being run, and silently starts a rule of its own. Name
the enclosing rule when it shares a line with a differently named rule,
leaving the common case of one rule per line unchanged.

Fixes: open-policy-agent#4967
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
The evaluation corpus in v1/test/cases, which Rego implementations
outside this repository consume as a conformance suite, cannot express a
policy that fails to compile, so nothing shared pins the errors OPA
reports before evaluation begins. Add v1/test/compilecases as a sibling
of it, seeded with the unsafe var cases from open-policy-agent#4967, each pinning the
error code, position and message of the diagnostics a policy must
produce.

Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>

@johanfylling johanfylling left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sspaink
sspaink merged commit fb8353d into open-policy-agent:main Aug 13, 2026
44 checks passed
@github-actions

Copy link
Copy Markdown
Benchmark Comparison (`7aaf1c538fe602581690253cf6adf2cb5519d0d9` vs `fb8353d50a64ad594ef1600826923493524dd76e`)
benchmark delta
CapabilitiesCurrentVersion -2.85%
ParseModuleRulesBase/1 -1.41%
ParseModuleRulesBase/1000 -2.12%
ParseStatementBasicCall -1.94%
ParseStatementMixedJSON -2.38%
ParseStatementSimpleArray/10 -1.65%
ParseStatementNestedObjects/5x1 -1.71%
ParseStatementNestedObjects/10x1 -1.79%
ParseStatementNestedObjects/1x5 -1.41%
ParseStatementNestedObjects/1x10 -2.26%
ParseEvery -2.21%
ParseDeepNesting/NestedArrays/depth-10 -2.24%
ParseDeepNesting/NestedArrays/depth-50 -2.83%
ParseDeepNesting/NestedObjects/depth-10 -2.22%
ParseDeepNesting/NestedObjects/depth-50 -1.72%
ParseStatementNestedObjectsOrSets/1 -1.61%
ParseStatementNestedObjectsOrSets/5 -2.00%
ParseStatementNestedObjectsOrSets/10 -1.71%
ParseStatementNestedObjectsOrSets/15 -1.93%
ParseStatementNestedObjectsOrSets/20 -2.03%
ParseVars -1.84%
ParseAnnotations -1.84%
ParseManyAnnotations -1.05%
ExprString/simple_expr -3.41%
ExprAppendText/complex_expr +2.53%
ObjectGet/existing_interned_key +0.35%
ObjectGet/existing_float_key -0.25%
ObjectGet/missing_string_key +0.41%
ObjectFind/5_5 +1.52%
ObjectCreationAndLookup/5 +2.54%
ObjectCreateWithInsertVsTermsArray/terms_array +5.40%
LazyObjectLookup/5 +4.46%
LazyObjectLookup/50 +1.73%
LazyObjectLookup/5000 +2.14%
LazyObjectFind/5_5 -3.87%
LazyObjectFind/5_50 -3.55%
LazyObjectFind/50_5 -2.69%
LazyObjectFind/50_50 -3.74%
LazyObjectFind/500_5 -4.15%
LazyObjectFind/500_50 -3.55%
LazyObjectFind/5000_5 -3.42%
LazyObjectFind/5000_50 -3.55%
SetCreationAndLookup/5 +5.25%
SetIntersection/5 +6.66%
SetIntersection/50 +4.10%
SetIntersectionDifferentSize/4 +6.50%
SetIntersectionDifferentSize/50 +4.86%
SetIntersectionDifferentSize/500 +4.57%
SetUnion/5 +3.82%
SetUnion/50 +6.43%
SetUnion/500 +5.63%
SetUnionOverlapping/5 +5.15%
SetUnionOverlapping/50 +3.61%
SetMembership/5 +3.70%
SetCopy +6.08%
ObjectCopy +6.12%
ArrayCopy/5 +4.29%
ArrayCopy/50 +9.20%
ArrayCopy/500 +6.23%
RefCopy/5 +5.36%
RefCopy/10 +6.89%
RefCopy/20 +6.40%
RefCopyNonGround/fully_ground +4.74%
RefCopyNonGround/mixed +3.45%
RefCopyNonGround/fully_ground/Copy +6.03%
RefCopyNonGround/mixed/Copy +4.89%
ObjectString/500/json.Marshal +5.05%
ObjectString/5000/json.Marshal +4.74%
ObjectStringInterfaces/5000/String() +6.46%
ObjectConstruction/shuffled_keys/5000 +3.01%
ArrayString/5/json.Marshal +3.84%
ArrayString/50/String() +5.78%
ArrayString/50/json.Marshal +3.62%
ArrayString/500/String() +4.78%
ArrayString/500/json.Marshal +5.14%
ArrayString/5000/String() +5.89%
ArrayString/5000/json.Marshal +6.62%
SetMarshalJSON/50/json.Marshal +1.75%
SetMarshalJSON/500/json.Marshal +1.36%
InterfaceToValueInt/non-interned_int_value +5.18%
ValueToInterfaceInt +4.74%
ObjectMergeWith/without_conflict +4.02%
ObjectMergeWith/with_conflict +4.15%
StringPrefix -2.07%
Ptr/with_escape +7.67%
Ptr/without_escape +7.81%
EscapeTemplateStringStringPart/{{{{{{{{{{_*_100 +5.42%
CountUnescapedLeftCurly/{_*_100 -3.51%
TemplateStringToString +3.59%
VarSetUpdateEmpty +4.74%
GenericVisitorWalkVsTypeVisitor/GenericVisitor +4.71%
InterningAccessValue/package_var_value +4.88%
InterningAccessValue/interned_value +45.24%
NoNodeTypeAllocatesOnAppend/module_annotated +3.45%
NoNodeTypeAllocatesOnAppend/head +4.73%
NoNodeTypeAllocatesOnAppend/some_decl +4.71%
NoNodeTypeAllocatesOnAppend/or,_implicit,_expanded +6.16%
NoNodeTypeAllocatesOnAppend/or,_explicit +4.70%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/string +3.59%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/object_comprehension_infix_value +3.33%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/array_comprehension_nested_infix_operators +1.62%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/array_comprehension_nested_infix_with_function_call +5.99%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/set_comprehension +2.15%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/not,_explicit_body,_one-line +6.75%

This comment was automatically generated by the benchmarks workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make error messages less ambiguous when rules are declared on same line

2 participants