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

Unit test bundle for type checking and function signatures #87

Closed
4 tasks
hinshun opened this issue Apr 14, 2020 · 3 comments
Closed
4 tasks

Unit test bundle for type checking and function signatures #87

hinshun opened this issue Apr 14, 2020 · 3 comments
Assignees
Labels
exp/beginner Good for newcomers

Comments

@hinshun
Copy link
Contributor

hinshun commented Apr 14, 2020

Tests should go into compile_test.go, and the input program should be minimal as possible. There may be more than one test case per bullet point.

  • Calling function that is wrong type for the function it is called in should produce error.
  • Calling function with args that don't match its signature should produce error (length, basic literal type, function type, func lit type).
  • Calling function with option that doesn't match the subtype should produce error.
  • Correctly calling function with variadic field in its signature should succeed.
@hinshun hinshun added the exp/beginner Good for newcomers label Apr 14, 2020
@slushie slushie self-assigned this Apr 15, 2020
@slushie
Copy link
Contributor

slushie commented Apr 22, 2020

#104 addresses these unit test, but some have been commented out because they currently fail on master.

Those tests highlight a few missing features in the checker and parser.

Checker currently doesn't identify incorrect types used as, or inside of, option blocks. That is with wrongOptionType and with option { wrongOptionType; } both pass, but shouldn't. It seems this also affects usage like myfunc option::run { someCopyOpt; } where the option block is passed as a value.

In discussion with @hinshun we realized the need for CallSmts to be parsed as expressions, possibly as CallExpr. The parser and checker should also accept BlockStmts that contain BasicLits, so that string myDir { "/var/lib/foo"; } is a valid statement that assigns "/var/lib/foo" as the return value of myDir.

@slushie
Copy link
Contributor

slushie commented Apr 22, 2020

	// Option blocks may be empty and may refer to identifiers or function
	// literals that don't have a sub-type, so we check them differently.
	if strings.HasPrefix(string(typ), string(parser.Option)) {
		return c.checkOptionBlockStmt(scope, typ, block)
	}

in checker.checkBlockStmt seems to be the culprit for the checker issue.

@hinshun could you provide an example hlb program that has an option with a func literal that has no subtype? I'm not sure I know what that looks like and I want to write a test case for it so that I can try to fix the checker. NB if I comment out that block, most of the checks seem to work consistently.

@hinshun
Copy link
Contributor Author

hinshun commented Apr 23, 2020

@hinshun could you provide an example hlb program that has an option with a func literal that has no subtype? I'm not sure I know what that looks like and I want to write a test case for it so that I can try to fix the checker. NB if I comment out that block, most of the checks seem to work consistently.

I think what I meant by that comment is the following.

Option blocks may be empty.

fs default() {
    image "alpine"
    run "echo foo" with option {}
}

may refer to identifiers

fs default() {
    image "alpine"
    run "echo foo" with foo
}

option::run foo() {
    env "key" "value"
}

or function literals that don't have sub-type

fs default() {
    image "alpine"
    run "echo foo" with option { # <<- no sub-type.
        env "key" "value"
    }
}

When I wrote sub-type I was referring to (*parser.Type).Secondary(). The identifier after the primary type (in this case, option) followed by ::<sub-type>. We should probably reword this comment. Let me know if that clarifies things!

@hinshun hinshun closed this as completed Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/beginner Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants