Follow-up corner cases for import-alias function-call resolution (#769)
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 346
- Forks
- 75
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 15
Description
Follow-up to #769 (fixes #747). That PR resolves function calls through import aliases in the interpreter and RVM. During deep review, several corner cases were identified that are either small bugs, a known RVM limitation, or intentional OPA divergences that should be pinned by tests. Tracking them here.
Actionable bugs (interpreter with handling)
1. Default-only imported function is dropped as a with … as replacement (src/interpreter.rs:~1331)
resolve_fcn_path_through_imports accepts default-only functions, but the caller re-filters with functions.contains_key(r), dropping them.
Fix: .filter(|r| self.compiled_policy.functions.contains_key(r) || self.is_default_function(r)).
2. Default-only imported function as a with target is misclassified as a data override (src/interpreter.rs:~1291)
target_is_function only checks lookup_function_by_name + is_builtin, missing default-only functions after alias rewrite.
Fix: also consider self.is_default_function(&target).
3. with … as <alias.fn> replacement resolves a local-module function before the import alias (src/interpreter.rs:~1322)
The replacement path is looked up with the current-module prefix before the leading import alias is resolved, so a colliding local rule wins over the imported target.
Known limitation
4. RVM rejects imported default-only functions (src/languages/rego/compiler/function_calls.rs:~223)
Alias resolution gates on functions.contains_key only. Mirror the interpreter's is_default_function detection (via rule_paths + default_rules), then unskip default_function_value_via_import in tests/rvm/rego/cases/imports.yaml.
Documented divergences to pin with tests (not bugs)
5. Bracketed dotted-key import flattening — import data["a.b"] is collapsed to data.a.b by get_path_string/join("."). Pre-existing engine-wide limitation (value refs behave the same); low priority. Add a pinning test.
6. Alias whose target lacks the called member falls back to builtin/local — intentional per #769 ("the rewrite never changes the meaning of existing policies"), diverges from OPA. Add pinning tests so the behavior can't change silently.
Proposed test cases
Interpreter cases → tests/interpreter/cases/import/tests.yaml; RVM equivalents → tests/rvm/rego/cases/imports.yaml. Each want_result/want_error marks the desired outcome; comments note where current behavior differs.
# Bug #1 — default-only imported fn as `with ... as` replacement (currently undefined)
- note: with_replacement_is_default_only_imported_function
modules:
- |
package real
import rego.v1
double(x) := 2 * x
- |
package mock
import rego.v1
default repl(_) := 99
- |
package rules
import rego.v1
import data.mock
main := y if {
y := data.real.double(1) with data.real.double as mock.repl
}
query: data.rules.main
want_result: 99 # current (buggy): undefined / skip_exec
# Bug #2 — default-only imported fn as `with` target (currently a data override)
- note: with_target_is_default_only_imported_function
modules:
- |
package lib
import rego.v1
default dfl(_) := 1
dfl(x) := x if x == "a"
- |
package rules
import rego.v1
import data.lib
main := lib.dfl("a") with lib.dfl as 7
query: data.rules.main
want_result: 7 # current (buggy): override not applied
# Bug #3 — replacement alias must win over a colliding local rule head
- note: with_replacement_alias_precedes_local_function
modules:
- |
package real
import rego.v1
double(x) := 2 * x
- |
package other
import rego.v1
fake(x) := 1000 + x
- |
package rules
import rego.v1
import data.other as repl
repl.fake(x) := x # local head collides with the alias `repl`
main := y if {
y := data.real.double(5) with data.real.double as repl.fake
}
query: data.rules.main
want_result: 1005 # expected data.other.fake; current may pick local
# Limitation #4 — RVM default-only imported function (unskip after RVM fix)
# note: default_function_value_via_import # already present, skip:true in RVM suite
# -> remove `skip: true` once the RVM compiler mirrors is_default_function
# Limitation #5 — bracketed dotted key must not collapse into a nested package
- note: bracketed_key_import_not_confused_with_nested_package
modules:
- |
package a.b
import rego.v1
f(_) := "nested-package"
- |
package rules
import rego.v1
import data["a.b"] as ab
result := ab.f(1)
query: data.rules.result
# DESIRED (OPA): data["a.b"].f is undefined -> unknown function
# CURRENT: flattened to data.a.b.f -> wrongly calls the package fn
want_error: "Unknown function"
# Divergence #6 — intentional builtin fallback when alias target lacks the member (pin it)
- note: alias_missing_member_falls_back_to_builtin_intentional
modules:
- |
package lib
import rego.v1
other(_) := 1 # no `unmarshal` defined
- |
package rules
import rego.v1
import data.lib as json
result := json.unmarshal(`[1,2]`)
query: data.rules.result
want_result: [1, 2] # INTENTIONAL: builtin json.unmarshal runs (diverges from OPA)
Identified via deep code review of #769. Related: #747.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/interpreter.rs around lines 1291, 1322, and 1331, then inspect src/languages/rego/compiler/function_calls.rs around line 223. Add or update the proposed cases in tests/interpreter/cases/import/tests.yaml and tests/rvm/rego/cases/imports.yaml, including the skipped RVM case. Done means the actionable corner cases are fixed and the documented divergences are pinned by tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- compilers, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 65/100