Skip to content

Commit

Permalink
Fix IDE warnings and remove usage of several deprecated fields. (#6397)
Browse files Browse the repository at this point in the history
Signed-off-by: Will Beason <[email protected]>
Co-authored-by: Johan Fylling <[email protected]>
  • Loading branch information
willbeason and johanfylling authored Nov 13, 2023
1 parent 619957c commit 8119dc0
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 31 deletions.
16 changes: 8 additions & 8 deletions ast/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (a *Annotations) GetTargetPath() Ref {
case *Package:
return n.Path
case *Rule:
return n.Path()
return n.Ref().GroundPrefix()
default:
return nil
}
Expand Down Expand Up @@ -352,12 +352,12 @@ func compareRelatedResources(a, b []*RelatedResourceAnnotation) int {
}

func compareSchemas(a, b []*SchemaAnnotation) int {
max := len(a)
if len(b) < max {
max = len(b)
maxLen := len(a)
if len(b) < maxLen {
maxLen = len(b)
}

for i := 0; i < max; i++ {
for i := 0; i < maxLen; i++ {
if cmp := a[i].Compare(b[i]); cmp != 0 {
return cmp
}
Expand Down Expand Up @@ -719,7 +719,7 @@ func (as *AnnotationSet) add(a *Annotations) *Error {
}
case annotationScopeDocument:
if rule, ok := a.node.(*Rule); ok {
path := rule.Path()
path := rule.Ref().GroundPrefix()
x := as.byPath.get(path)
if x != nil {
return errAnnotationRedeclared(a, x.Value.Location)
Expand Down Expand Up @@ -815,7 +815,7 @@ func (as *AnnotationSet) Chain(rule *Rule) AnnotationsRefSet {
// Make sure there is always a leading entry representing the passed rule, even if it has no annotations
refs = append(refs, &AnnotationsRef{
Location: rule.Location,
Path: rule.Path(),
Path: rule.Ref().GroundPrefix(),
node: rule,
})
}
Expand All @@ -827,7 +827,7 @@ func (as *AnnotationSet) Chain(rule *Rule) AnnotationsRefSet {
})
}

docAnnots := as.GetDocumentScope(rule.Path())
docAnnots := as.GetDocumentScope(rule.Ref().GroundPrefix())
if docAnnots != nil {
refs = append(refs, NewAnnotationsRef(docAnnots))
}
Expand Down
2 changes: 1 addition & 1 deletion ast/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -3287,7 +3287,7 @@ func (b *Builtin) Ref() Ref {
// IsTargetPos returns true if a variable in the i-th position will be bound by
// evaluating the call expression.
func (b *Builtin) IsTargetPos(i int) bool {
return len(b.Decl.Args()) == i
return len(b.Decl.FuncArgs().Args) == i
}

func init() {
Expand Down
4 changes: 2 additions & 2 deletions ast/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ func (tc *typeChecker) checkExprWith(env *TypeEnv, expr *Expr, i int) *Error {
switch v := valueType.(type) {
case *types.Function: // ...by function
if !unifies(targetType, valueType) {
return newArgError(expr.With[i].Loc(), target.Value.(Ref), "arity mismatch", v.Args(), t.NamedFuncArgs())
return newArgError(expr.With[i].Loc(), target.Value.(Ref), "arity mismatch", v.FuncArgs().Args, t.NamedFuncArgs())
}
default: // ... by value, nothing to check
}
Expand Down Expand Up @@ -1241,7 +1241,7 @@ func getRuleAnnotation(as *AnnotationSet, rule *Rule) (result []*SchemaAnnotatio
result = append(result, x.Schemas...)
}

if x := as.GetDocumentScope(rule.Path()); x != nil {
if x := as.GetDocumentScope(rule.Ref().GroundPrefix()); x != nil {
result = append(result, x.Schemas...)
}

Expand Down
20 changes: 11 additions & 9 deletions ast/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package ast

import (
"errors"
"fmt"
"io"
"sort"
Expand Down Expand Up @@ -528,7 +529,7 @@ func (c *Compiler) ComprehensionIndex(term *Term) *ComprehensionIndex {
// otherwise, the ref is used to perform a ruleset lookup.
func (c *Compiler) GetArity(ref Ref) int {
if bi := c.builtins[ref.String()]; bi != nil {
return len(bi.Decl.Args())
return len(bi.Decl.FuncArgs().Args)
}
rules := c.GetRulesExact(ref)
if len(rules) == 0 {
Expand Down Expand Up @@ -1703,12 +1704,12 @@ func checkKeywordOverrides(node interface{}, strict bool) Errors {
return nil
}

errors := Errors{}
errs := Errors{}

WalkRules(node, func(rule *Rule) bool {
name := rule.Head.Name.String()
if RootDocumentRefs.Contains(RefTerm(VarTerm(name))) {
errors = append(errors, NewError(CompileErr, rule.Location, "rules must not shadow %v (use a different rule name)", name))
errs = append(errs, NewError(CompileErr, rule.Location, "rules must not shadow %v (use a different rule name)", name))
}
return true
})
Expand All @@ -1717,13 +1718,13 @@ func checkKeywordOverrides(node interface{}, strict bool) Errors {
if expr.IsAssignment() {
name := expr.Operand(0).String()
if RootDocumentRefs.Contains(RefTerm(VarTerm(name))) {
errors = append(errors, NewError(CompileErr, expr.Location, "variables must not shadow %v (use a different variable name)", name))
errs = append(errs, NewError(CompileErr, expr.Location, "variables must not shadow %v (use a different variable name)", name))
}
}
return false
})

return errors
return errs
}

// resolveAllRefs resolves references in expressions to their fully qualified values.
Expand Down Expand Up @@ -2814,7 +2815,8 @@ func (qc *queryCompiler) TypeEnv() *TypeEnv {
}

func (qc *queryCompiler) applyErrorLimit(err error) error {
if errs, ok := err.(Errors); ok {
var errs Errors
if errors.As(err, &errs) {
if qc.compiler.maxErrs > 0 && len(errs) > qc.compiler.maxErrs {
err = append(errs[:qc.compiler.maxErrs], errLimitReached)
}
Expand Down Expand Up @@ -3145,7 +3147,7 @@ type comprehensionIndexRegressionCheckVisitor struct {
// values or not. It's unlikely that anything outside of OPA does this today so this
// solution is fine for now.
var comprehensionIndexBlacklist = map[string]int{
WalkBuiltin.Name: len(WalkBuiltin.Decl.Args()),
WalkBuiltin.Name: len(WalkBuiltin.Decl.FuncArgs().Args),
}

func newComprehensionIndexRegressionCheckVisitor(candidates VarSet) *comprehensionIndexRegressionCheckVisitor {
Expand Down Expand Up @@ -3782,7 +3784,7 @@ func reorderBodyForSafety(builtins map[string]*Builtin, arity func(Ref) int, glo

// check closures: is this expression closing over variables that
// haven't been made safe by what's already included in `reordered`?
vs := unsafeVarsInClosures(e, arity, safe)
vs := unsafeVarsInClosures(e)
cv := vs.Intersect(bodyVars).Diff(globals)
uv := cv.Diff(outputVarsForBody(reordered, arity, safe))

Expand Down Expand Up @@ -3915,7 +3917,7 @@ func (xform *bodySafetyTransformer) reorderSetComprehensionSafety(sc *SetCompreh

// unsafeVarsInClosures collects vars that are contained in closures within
// this expression.
func unsafeVarsInClosures(e *Expr, arity func(Ref) int, safe VarSet) VarSet {
func unsafeVarsInClosures(e *Expr) VarSet {
vs := VarSet{}
WalkClosures(e, func(x interface{}) bool {
vis := &VarVisitor{vars: vs}
Expand Down
8 changes: 4 additions & 4 deletions ast/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9931,14 +9931,14 @@ allow {
}
}

errors := NewCompiler().WithSchemas(schemaSet).PassesTypeCheckRules(elems)
errs := NewCompiler().WithSchemas(schemaSet).PassesTypeCheckRules(elems)

if len(errors) > 0 {
if len(errs) > 0 {
if len(tc.errs) == 0 {
t.Fatalf("Unexpected error: %v", errors)
t.Fatalf("Unexpected error: %v", errs)
}

result := compilerErrsToStringSlice(errors)
result := compilerErrsToStringSlice(errs)

if len(result) != len(tc.errs) {
t.Fatalf("Expected %d:\n%v\nBut got %d:\n%v", len(tc.errs), strings.Join(tc.errs, "\n"), len(result), strings.Join(result, "\n"))
Expand Down
4 changes: 2 additions & 2 deletions ast/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (c parsedTermCache) String() string {
s.WriteRune('{')
var e *parsedTermCacheItem
for e = c.m; e != nil; e = e.next {
fmt.Fprintf(&s, "%v", e)
s.WriteString(fmt.Sprintf("%v", e))
}
s.WriteRune('}')
return s.String()
Expand Down Expand Up @@ -1988,7 +1988,7 @@ func (p *Parser) error(loc *location.Location, reason string) {

func (p *Parser) errorf(loc *location.Location, f string, a ...interface{}) {
msg := strings.Builder{}
fmt.Fprintf(&msg, f, a...)
msg.WriteString(fmt.Sprintf(f, a...))

switch len(p.s.hints) {
case 0: // nothing to do
Expand Down
6 changes: 3 additions & 3 deletions ast/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ metadata := 7
t.Fatalf("Expected roundtripped module to be equal to original:\nExpected:\n\n%v\n\nGot:\n\n%v\n", mod, roundtrip)
}

if mod.Rules[3].Path().String() != "data.a.b.c.t" {
t.Fatal("expected path data.a.b.c.t for 4th rule in module but got:", mod.Rules[3].Path())
if mod.Rules[3].Ref().GroundPrefix().String() != "data.a.b.c.t" {
t.Fatal("expected path data.a.b.c.t for 4th rule in module but got:", mod.Rules[3].Ref().GroundPrefix())
}

if len(roundtrip.Annotations) != 1 {
Expand Down Expand Up @@ -608,7 +608,7 @@ func TestRuleString(t *testing.T) {
func TestRulePath(t *testing.T) {
ruleWithMod := func(r string) Ref {
mod := MustParseModule("package pkg\n" + r)
return mod.Rules[0].Path()
return mod.Rules[0].Ref().GroundPrefix()
}
if exp, act := MustParseRef("data.pkg.p.q.r"), ruleWithMod("p.q.r { true }"); !exp.Equal(act) {
t.Errorf("expected %v, got %v", exp, act)
Expand Down
4 changes: 2 additions & 2 deletions ast/term.go
Original file line number Diff line number Diff line change
Expand Up @@ -892,8 +892,8 @@ func PtrRef(head *Term, s string) (Ref, error) {
return Ref{head}, nil
}
parts := strings.Split(s, "/")
if max := math.MaxInt32; len(parts) >= max {
return nil, fmt.Errorf("path too long: %s, %d > %d (max)", s, len(parts), max)
if maxLen := math.MaxInt32; len(parts) >= maxLen {
return nil, fmt.Errorf("path too long: %s, %d > %d (max)", s, len(parts), maxLen)
}
ref := make(Ref, uint(len(parts))+1)
ref[0] = head
Expand Down

0 comments on commit 8119dc0

Please sign in to comment.