Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arbitrary vars in rule refs #5913

Merged
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
525c93d
topdown: Adding support for multiple variables in rule refs
johanfylling Apr 6, 2023
5f70376
Merge branch 'main' into jf/5685/multi-var-rule-refs
johanfylling Jul 3, 2023
d387cfd
Updating commented-out tests.
johanfylling Jul 6, 2023
d1dcb03
Fixing yaml test expecting multiple results.
johanfylling Jul 10, 2023
2073c3d
Not running yaml test for WASM
johanfylling Jul 12, 2023
29c2a2e
Merge branch 'main' into jf/5685/multi-var-rule-refs
johanfylling Aug 1, 2023
659ef2a
Moving type merge from `typeChecker.checkRule()` to `typeTreeNode.Ins…
johanfylling Jul 4, 2023
692770e
Updating type-checker to handle generic rule refs
johanfylling Jul 4, 2023
9707e66
Updating tests
johanfylling Jul 5, 2023
391d469
Generalizing `types.Object.Merge()``
johanfylling Jul 5, 2023
ea52a01
Merging sub-objects in `types.Object.Merge()`
johanfylling Jul 5, 2023
996cdec
Removing comment in test
johanfylling Jul 10, 2023
1065755
Updating tests
johanfylling Jul 5, 2023
67ba943
Aligning hint-key and entry for general refs on virtual partial eval
johanfylling Jul 11, 2023
8ffcf1d
Enforcing no-else parser rule on general ref heads
johanfylling Aug 10, 2023
f02441d
PE impl
johanfylling Aug 14, 2023
68f9436
Removing redundant legacy PE eval branch
johanfylling Aug 14, 2023
bec96d1
Fixing issue where PE-produced query contained non-namespaced local vars
johanfylling Aug 18, 2023
33ebc2f
Adding PE tests
johanfylling Aug 18, 2023
80620a0
Invoking evalOneRulePostUnify() for any ref with unknowns overlapping…
johanfylling Aug 21, 2023
98178fb
fmt: Not using rule ref ground prefix if ref has dynamic part
johanfylling Aug 21, 2023
372def8
Fixing linter warnings
johanfylling Aug 21, 2023
9e544c6
Ignoring additional yaml tests not supported by Wasm
johanfylling Aug 22, 2023
0fa5652
Merge branch 'main' into jf/5685/multi-var-rule-refs
johanfylling Aug 22, 2023
52bb04d
Cleanup
johanfylling Aug 22, 2023
18702a3
Producing cache hint keys containing entire applicable ref for genera…
johanfylling Aug 22, 2023
d4ac3b0
Merge branch 'main' into jf/5685/multi-var-rule-refs
johanfylling Aug 29, 2023
9e46555
Updating type-tree construction
johanfylling Aug 29, 2023
e1c0a23
Merge branch 'main' into jf/5685/multi-var-rule-refs
johanfylling Aug 29, 2023
2e7121b
Removing debug print
johanfylling Aug 29, 2023
a98a6ee
Merge branch 'main' into jf/5685/multi-var-rule-refs
johanfylling Aug 30, 2023
9dcaac8
Adding docs
johanfylling Aug 30, 2023
c0d38ea
Merge branch 'main' into jf/5685/multi-var-rule-refs
johanfylling Aug 31, 2023
ebf1503
Making changes suggested by @srenatus
johanfylling Aug 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 36 additions & 17 deletions ast/check.go
Expand Up @@ -231,37 +231,30 @@ func (tc *typeChecker) checkRule(env *TypeEnv, as *AnnotationSet, rule *Rule) {

f := types.NewFunction(args, cpy.Get(rule.Head.Value))

// Union with existing.
exist := env.tree.Get(path)
tpe = types.Or(exist, f)

tpe = f
} else {
switch rule.Head.RuleKind() {
case SingleValue:
typeV := cpy.Get(rule.Head.Value)
if last := path[len(path)-1]; !last.IsGround() {

// e.g. store object[string: whatever] at data.p.q.r, not data.p.q.r[x]
if !path.IsGround() {
// e.g. store object[string: whatever] at data.p.q.r, not data.p.q.r[x] or data.p.q.r[x].y[z]
objPath := path.DynamicSuffix()
path = path.GroundPrefix()

typeK := cpy.Get(last)
if typeK != nil && typeV != nil {
exist := env.tree.Get(path)
typeV = types.Or(types.Values(exist), typeV)
typeK = types.Or(types.Keys(exist), typeK)
tpe = types.NewObject(nil, types.NewDynamicProperty(typeK, typeV))
var err error
tpe, err = nestedObject(cpy, objPath, typeV)
if err != nil {
tc.err([]*Error{NewError(TypeErr, rule.Head.Location, err.Error())})
tpe = nil
}
} else {
if typeV != nil {
exist := env.tree.Get(path)
tpe = types.Or(typeV, exist)
tpe = typeV
}
}
case MultiValue:
typeK := cpy.Get(rule.Head.Key)
if typeK != nil {
exist := env.tree.Get(path)
typeK = types.Or(types.Keys(exist), typeK)
tpe = types.NewSet(typeK)
}
}
Expand All @@ -272,6 +265,32 @@ func (tc *typeChecker) checkRule(env *TypeEnv, as *AnnotationSet, rule *Rule) {
}
}

// nestedObject creates a nested structure of object types, where each term on path corresponds to a level in the
// nesting. Each term in the path only contributes to the dynamic portion of its corresponding object.
func nestedObject(env *TypeEnv, path Ref, tpe types.Type) (types.Type, error) {
if len(path) == 0 {
return tpe, nil
}

k := path[0]
typeV, err := nestedObject(env, path[1:], tpe)
if err != nil {
return nil, err
}
if typeV == nil {
return nil, nil
}

var dynamicProperty *types.DynamicProperty
typeK := env.Get(k)
if typeK == nil {
return nil, nil
}
dynamicProperty = types.NewDynamicProperty(typeK, typeV)

return types.NewObject(nil, dynamicProperty), nil
}

func (tc *typeChecker) checkExpr(env *TypeEnv, expr *Expr) *Error {
if err := tc.checkExprWith(env, expr, 0); err != nil {
return err
Expand Down
109 changes: 109 additions & 0 deletions ast/check_test.go
Expand Up @@ -358,6 +358,16 @@ func TestCheckInferenceRules(t *testing.T) {
{`overlap`, `p.q2.a = input.a { true }`},
{`overlap`, `p.q2[56] = input.a { true }`},
}
ruleset3 := [][2]string{
{`simple`, `p.q[r][s] = 42 { x = ["a", "b"]; r = x[s] }`},
{`mixed`, `p.q[r].s[t] = 42 { x = ["a", "b"]; r = x[t] }`},
{`overrides`, `p.q[r] = "foo" { x = ["a", "b"]; r = x[_] }`},
{`overrides`, `p.q.r[s] = 42 { x = ["a", "b"]; x[s] }`},
{`overrides`, `p.q[r].s = true { x = [true, false]; r = x[_] }`},
{`overrides_static`, `p.q[r].a = "foo" { r = "bar"; s = "baz" }`},
{`overrides_static`, `p.q[r].b = 42 { r = "bar" }`},
{`overrides_static`, `p.q[r].c = true { r = "bar" }`},
}

tests := []struct {
note string
Expand Down Expand Up @@ -549,6 +559,104 @@ func TestCheckInferenceRules(t *testing.T) {
types.NewDynamicProperty(types.Any{types.N, types.S}, types.Any{types.B, types.N, types.S}),
),
},
{
note: "general ref-rules, only vars in obj-path, complete obj access",
rules: ruleset3,
ref: "data.simple.p.q",
expected: types.NewObject(
[]*types.StaticProperty{},
types.NewDynamicProperty(types.S,
types.NewObject(
[]*types.StaticProperty{},
types.NewDynamicProperty(types.N, types.N),
),
),
),
},
{
note: "general ref-rules, only vars in obj-path, intermediate obj access",
rules: ruleset3,
ref: "data.simple.p.q.b",
expected: types.NewObject(
[]*types.StaticProperty{},
types.NewDynamicProperty(types.N, types.N),
),
},
{
note: "general ref-rules, only vars in obj-path, leaf access",
rules: ruleset3,
ref: "data.simple.p.q.b[1]",
expected: types.N,
},
{
note: "general ref-rules, vars and constants in obj-path, complete obj access",
rules: ruleset3,
ref: "data.mixed.p.q",
expected: types.NewObject(
[]*types.StaticProperty{},
types.NewDynamicProperty(types.S,
types.NewObject(nil,
types.NewDynamicProperty(types.S, types.NewObject(nil,
types.NewDynamicProperty(types.N, types.N))),
),
),
),
},
{
note: "general ref-rules, key overrides, complete obj access",
rules: ruleset3,
ref: "data.overrides.p.q",
expected: types.NewObject(nil, types.NewDynamicProperty(
types.Or(types.B, types.S),
types.Any{
types.S,
types.NewObject(nil, types.NewDynamicProperty(
types.Any{types.N, types.S},
types.Any{types.B, types.N})),
},
),
),
},
{
note: "general ref-rules, multiple static key overrides, complete obj access",
rules: ruleset3,
ref: "data.overrides_static.p.q",
expected: types.NewObject(
[]*types.StaticProperty{},
types.NewDynamicProperty(types.S,
types.NewObject(
nil,
types.NewDynamicProperty(types.S, types.Any{types.B, types.N, types.S}),
),
),
),
},
{
note: "general ref-rules, multiple static key overrides, intermediate obj access",
rules: ruleset3,
ref: "data.overrides_static.p.q.foo",
expected: types.NewObject(nil,
types.NewDynamicProperty(types.S, types.Any{types.B, types.N, types.S}),
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 took me a moment to understand this, but I had overlooked that it's p.q[r].b = 42 and not p.q[r] = 42. So, the extra S keys would (could) be "a", "b", "c". ✔️

),
},
{
note: "general ref-rules, multiple static key overrides, leaf access (a)",
rules: ruleset3,
ref: "data.overrides_static.p.q.foo.a",
expected: types.Any{types.B, types.N, types.S}, // Dynamically build object types don't have static properties, so even though we "know" the 'a' key has a string value, we've lost this information.
},
{
note: "general ref-rules, multiple static key overrides, leaf access (b)",
rules: ruleset3,
ref: "data.overrides_static.p.q.bar.b",
expected: types.Any{types.B, types.N, types.S},
},
{
note: "general ref-rules, multiple static key overrides, leaf access (c)",
rules: ruleset3,
ref: "data.overrides_static.p.q.baz.c",
expected: types.Any{types.B, types.N, types.S},
},
}

for _, tc := range tests {
Expand Down Expand Up @@ -579,6 +687,7 @@ func TestCheckInferenceRules(t *testing.T) {
t.Fatalf("Unexpected error %v:", err)
}

fmt.Println(env.tree.String())
Copy link
Member

Choose a reason for hiding this comment

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

Left from testing probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, yea 🤦‍♂️ 😄

result := env.Get(ref)
if tc.expected == nil {
if result != nil {
Expand Down