Skip to content

Commit

Permalink
ast: Add rules/functions that contain errors to the type env
Browse files Browse the repository at this point in the history
Earlier if a rule/function contained an error, the type checker would not update the env with subsequent independent rules resulting in errors such as "undefined functions". This change add erroneous rules/functions to the env to avoid such type errors.

Fixes #2155

Fixes #2091

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
  • Loading branch information
ashutosh-narkar committed Mar 30, 2020
1 parent 46cadc9 commit c43b61b
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 12 deletions.
27 changes: 20 additions & 7 deletions ast/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ func (tc *typeChecker) WithVarRewriter(f rewriteVars) *typeChecker {
// TypeEnv will be able to resolve types of vars contained in the body.
func (tc *typeChecker) CheckBody(env *TypeEnv, body Body) (*TypeEnv, Errors) {

errors := []*Error{}

if env == nil {
env = NewTypeEnv()
} else {
Expand All @@ -58,15 +60,15 @@ func (tc *typeChecker) CheckBody(env *TypeEnv, body Body) (*TypeEnv, Errors) {

closureErrs := tc.checkClosures(env, expr)
for _, err := range closureErrs {
tc.err(err)
errors = append(errors, err)
}

hasClosureErrors := len(closureErrs) > 0

vis := newRefChecker(env, tc.varRewriter)
NewGenericVisitor(vis.Visit).Walk(expr)
for _, err := range vis.errs {
tc.err(err)
errors = append(errors, err)
}

hasRefErrors := len(vis.errs) > 0
Expand All @@ -78,14 +80,14 @@ func (tc *typeChecker) CheckBody(env *TypeEnv, body Body) (*TypeEnv, Errors) {
// likely to be the result of the more specific error.
skip := (hasClosureErrors || hasRefErrors) && causedByNilType(err)
if !skip {
tc.err(err)
errors = append(errors, err)
}
}

return true
})

return env, tc.errs
tc.err(errors)
return env, errors
}

// CheckTypes runs type checking on the rules returns a TypeEnv if no errors
Expand Down Expand Up @@ -205,6 +207,11 @@ func (tc *typeChecker) checkRule(env *TypeEnv, rule *Rule) {
if tpe != nil {
env.tree.Put(path, tpe)
}
} else {
// if the rule/function contains an error, add it to the type env
// so that expressions that refer to this rule/function
// do not encounter type errors
env.tree.Put(rule.Path(), types.A)
}
}

Expand Down Expand Up @@ -241,6 +248,12 @@ func (tc *typeChecker) checkExprBuiltin(env *TypeEnv, expr *Expr) *Error {
return NewError(TypeErr, expr.Location, "undefined function %v", name)
}

// check if the expression refers to a function that contains an error
_, ok := tpe.(types.Any)
if ok {
return nil
}

ftpe, ok := tpe.(*types.Function)
if !ok {
return NewError(TypeErr, expr.Location, "undefined function %v", name)
Expand Down Expand Up @@ -489,8 +502,8 @@ func unify1Set(env *TypeEnv, val Set, tpe *types.Set, union bool) bool {
})
}

func (tc *typeChecker) err(err *Error) {
tc.errs = append(tc.errs, err)
func (tc *typeChecker) err(errors []*Error) {
tc.errs = append(tc.errs, errors...)
}

type refChecker struct {
Expand Down
91 changes: 86 additions & 5 deletions ast/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -678,14 +678,13 @@ func TestVoidBuiltins(t *testing.T) {
for _, tc := range tests {
body := MustParseBody(tc.query)
checker := newTypeChecker()
_, err := checker.CheckBody(newTestEnv(nil), body)
if err != nil && !tc.wantErr {
t.Fatal(err)
} else if err == nil && tc.wantErr {
_, errs := checker.CheckBody(newTestEnv(nil), body)
if len(errs) != 0 && !tc.wantErr {
t.Fatal(errs)
} else if len(errs) == 0 && tc.wantErr {
t.Fatal("Expected error")
}
}

}

func TestCheckRefErrUnsupported(t *testing.T) {
Expand Down Expand Up @@ -967,6 +966,88 @@ func TestFunctionTypeInferenceUnappliedWithObjectVarKey(t *testing.T) {
}
}

func TestCheckValidErrors(t *testing.T) {

module := MustParseModule(`
package test
p {
concat("", 1) # type error
}
q {
r(1)
}
r(x) = x`)

module2 := MustParseModule(`
package test
b {
a(1) # call erroneous function
}
a(x) {
max("foo") # max requires an array
}
m {
1 / "foo" # type error
}
n {
m # call erroneous rule
}`)

module3 := MustParseModule(`
package test
x := {"a" : 1}
y {
z
}
z {
x[1] == 1 # undefined reference error
}`)

tests := map[string]struct {
module *Module
numErr int
query []string
}{
"single_type_error": {module: module, numErr: 1, query: []string{`data.test.p`}},
"multiple_type_error": {module: module2, numErr: 2, query: []string{`data.test.a`, `data.test.m`}},
"undefined_reference_error": {module: module3, numErr: 1, query: []string{`data.test.z`}},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
c := NewCompiler()
c.Compile(map[string]*Module{"test": tc.module})

if !c.Failed() {
t.Errorf("Expected error but got success")
}

if len(c.Errors) != tc.numErr {
t.Fatalf("Expected %v error(s) but got: %v", tc.numErr, c.Errors)
}

// check type of the rule/function that contains an error
for _, q := range tc.query {
tpe := c.TypeEnv.Get(MustParseRef(q))

if types.Compare(tpe, types.NewAny()) != 0 {
t.Fatalf("Expected Any type but got %v", tpe)
}
}
})
}
}

func TestCheckErrorDetails(t *testing.T) {

tests := []struct {
Expand Down

0 comments on commit c43b61b

Please sign in to comment.