Skip to content

Test function call semantics #104

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func (c *checker) checkBlockStmt(scope *parser.Scope, typ parser.ObjType, block
continue
}
default:
panic("implementation error")
panic("checkBlockStmt: implementation error")
}

// If the function is not a builtin, retrieve it from the scope and then
Expand Down Expand Up @@ -408,7 +408,7 @@ func (c *checker) checkCallSignature(scope *parser.Scope, typ parser.ObjType, ca
case *parser.ImportDecl:
panic("todo: ErrCallImport")
default:
panic("implementation error")
panic("checkCallSignature: implementation error")
}
}
}
Expand Down
217 changes: 212 additions & 5 deletions checker/checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ func cleanup(value string) string {
return result
}

func makePos(line, col int) lexer.Position { //nolint:unparam
return lexer.Position{
Filename: "<stdin>",
Line: line,
Column: col,
}
}

func TestChecker_Check(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -297,15 +305,214 @@ func TestChecker_Check(t *testing.T) {
Name: "myFunction",
},
},
}} {
}, {
"variadic options with bad type",
`
fs default() {
myfunc "string"
}
fs myfunc(variadic option::run opts) {
image "busybox"
run "echo hi" with opts
}
`,
ErrWrongArgType{
Pos: makePos(2, 8),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we should abstract the testing away from these node positions? Or do you think they belong here?

Expected: "option::run",
Found: "string",
},
}, /*{
"variadic options with bad method type",
`
fs default() {
myfunc option::run {
copyOpt
}
}
fs myfunc(variadic option::run opts) {
image "busybox"
run "echo hi" with opts
}
option::copy copyOpt() {}
`,
ErrWrongArgType{},
},*/{
"variadic options with mixed types",
`
fs default() {
myfunc option::run {} "string"
}
fs myfunc(variadic option::run opts) {
image "busybox"
run "echo hi" with opts
}
`,
ErrWrongArgType{
Pos: makePos(2, 23),
Expected: "option::run",
Found: "string",
},
}, {
"func call with bad arg count",
`
fs default() {
myfunc "a" "b"
}
fs myfunc(string cmd) {
image "busybox"
run cmd
}
`,
ErrNumArgs{
Expected: 1,
CallStmt: &parser.CallStmt{
Pos: makePos(2, 1),
Args: make([]*parser.Expr, 2),
},
},
}, {
"func call with bad arg type: basic literal",
`
fs default() {
myfunc 1
}
fs myfunc(string cmd) {
image "busybox"
run cmd
}
`,
ErrWrongArgType{
Pos: makePos(2, 8),
Expected: "string",
Found: "int",
},
}, /*{
"func call with bad arg: basic ident",
`
fs default() {
myfunc s
}
string s() { value "string"; }
int myfunc(int i) {}
`,
ErrWrongArgType{},
},*/ /*{
"func call with bad arg: func ident",
`
fs default() {
myfunc foo
}
fs foo() {}
fs myfunc(string cmd) {
image "busybox"
run cmd
}
`,
ErrWrongArgType{},
},*/{
"func call with bad arg type: func literal",
`
fs default() {
myfunc fs {}
}
fs myfunc(string cmd) {
image "busybox"
run cmd
}
`,
ErrWrongArgType{
Pos: makePos(2, 8),
Expected: "string",
Found: "fs",
},
}, {
"func call with bad subtype",
`
fs default() {
runOpt
}
option::run runOpt() {}
fs myfunc(string cmd) {
image "busybox"
run cmd
}
`,
ErrWrongArgType{
Pos: makePos(2, 1),
Expected: "fs",
Found: "option::run",
},
}, {
"func call with option",
`
fs default() {
scratch
run "foo" with option {
mount fs { scratch; } "/"
}
}
`,
nil,
}, {
"func call with option: inline literal",
`
fs default() {
scratch
run "foo" with option {
fooOpt
}
}
option::run fooOpt() {}
`,
nil,
}, {
"func call with option: ident",
`
fs default() {
scratch
run "foo" with fooOpt
}
option::run fooOpt() {}
`,
nil,
}, /*{
"func call with bad option: inline literal",
`
fs default() {
scratch
run "foo" with option {
fooOpt
}
}
option::copy fooOpt() {}
`,
ErrWrongArgType{},
},*/ /*{
"func call with bad option: ident",
`
fs default() {
scratch
run "foo" with fooOpt
}
option::copy fooOpt() {}
`,
ErrWrongArgType{},
}*/} {
tc := tc
t.Run(tc.name, func(t *testing.T) {
in := strings.NewReader(cleanup(tc.input))

mod, err := parser.Parse(in)
require.NoError(t, err)

err = Check(mod)
var r interface{}
func() {
defer func() {
r = recover()
}()
err = Check(mod)
}()
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably return errors.WithStack instead of panicking in Check.

require.Nil(t, r, "panic: %+v", r)
validateError(t, tc.errType, err)
})
}
Expand Down Expand Up @@ -383,10 +590,10 @@ func validateError(t *testing.T, expectedError error, actualError error) {
// assume if we got a semantic error we really want
// to validate the underlying error
if semErr, ok := actualError.(ErrSemantic); ok {
require.IsType(t, expectedError, semErr.Errs[0])
require.Equal(t, expectedError.Error(), semErr.Errs[0].Error())
require.IsType(t, expectedError, semErr.Errs[0], "type %T", semErr.Errs[0])
require.Equal(t, expectedError.Error(), semErr.Errs[0].Error(), "error: %v", actualError)
} else {
require.IsType(t, expectedError, actualError)
require.IsType(t, expectedError, actualError, "error: %v", actualError)
}
}
}
3 changes: 3 additions & 0 deletions checker/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ type ErrNumArgs struct {
}

func (e ErrNumArgs) Error() string {
if e.CallStmt == nil {
return "<invalid ErrNumArgs>"
}
return fmt.Sprintf("%s expected %d args, found %d", FormatPos(e.CallStmt.Pos), e.Expected, len(e.CallStmt.Args))
}

Expand Down