loader: report all type and syntax errors possible

Previously the loader would only report the first error. With this
change, all syntax and type errors will be reported.

cgo/libclang.go

@@ -642,13 +642,6 @@ func (p *cgoPackage) addErrorAfter(pos token.Pos, after, msg string) {
 
 // addErrorAt is a utility function to add an error to the list of errors.
 func (p *cgoPackage) addErrorAt(position token.Position, msg string) {
-	if filepath.IsAbs(position.Filename) {
-		// Relative paths for readability, like other Go parser errors.
-		relpath, err := filepath.Rel(p.currentDir, position.Filename)
-		if err == nil {
-			position.Filename = relpath
-		}
-	}
 	p.errors = append(p.errors, scanner.Error{
 		Pos: position,
 		Msg: msg,

errors_test.go

@@ -0,0 +1,101 @@
+package main
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/tinygo-org/tinygo/compileopts"
+)
+
+// Test the error messages of the TinyGo compiler.
+func TestErrors(t *testing.T) {
+	for _, name := range []string{
+		"cgo",
+		"multi",
+		"loader-importcycle",
+		"loader-invaliddep",
+		"loader-invalidpackage",
+		"loader-nopackage",
+		"syntax",
+		"types",
+	} {
+		t.Run(name, func(t *testing.T) {
+			testErrorMessages(t, "./testdata/errors/"+name+".go")
+		})
+	}
+}
+
+func testErrorMessages(t *testing.T, filename string) {
+	// Parse expected error messages.
+	expected := readErrorMessages(t, filename)
+
+	// Try to build a binary (this should fail with an error).
+	tmpdir := t.TempDir()
+	err := Build(filename, tmpdir+"/out", &compileopts.Options{
+		Target:        "wasip1",
+		Semaphore:     sema,
+		InterpTimeout: 180 * time.Second,
+		Debug:         true,
+		VerifyIR:      true,
+		Opt:           "z",
+	})
+	if err == nil {
+		t.Fatal("expected to get a compiler error")
+	}
+
+	// Get the full ./testdata/errors directory.
+	wd, absErr := filepath.Abs("testdata/errors")
+	if absErr != nil {
+		t.Fatal(absErr)
+	}
+
+	// Write error message out as plain text.
+	var buf bytes.Buffer
+	printCompilerError(err, func(v ...interface{}) {
+		fmt.Fprintln(&buf, v...)
+	}, wd)
+	actual := strings.TrimRight(buf.String(), "\n")
+
+	// Check whether the error is as expected.
+	if canonicalizeErrors(actual) != canonicalizeErrors(expected) {
+		t.Errorf("expected error:\n%s\ngot:\n%s", indentText(expected, "> "), indentText(actual, "> "))
+	}
+}
+
+func canonicalizeErrors(text string) string {
+	// Fix for Windows: replace all backslashes with forward slashes so that
+	// paths will be the same as on POSIX systems.
+	// (It may also change some other backslashes, but since this is only for
+	// comparing text it should be fine).
+	if runtime.GOOS == "windows" {
+		text = strings.ReplaceAll(text, "\\", "/")
+	}
+	return text
+}
+
+// Indent the given text with a given indentation string.
+func indentText(text, indent string) string {
+	return indent + strings.ReplaceAll(text, "\n", "\n"+indent)
+}
+
+// Read "// ERROR:" prefixed messages from the given file.
+func readErrorMessages(t *testing.T, file string) string {
+	data, err := os.ReadFile(file)
+	if err != nil {
+		t.Fatal("could not read input file:", err)
+	}
+
+	var errors []string
+	for _, line := range strings.Split(string(data), "\n") {
+		if strings.HasPrefix(line, "// ERROR: ") {
+			errors = append(errors, strings.TrimRight(line[len("// ERROR: "):], "\r\n"))
+		}
+	}
+	return strings.Join(errors, "\n")
+}

loader/loader.go

@@ -128,7 +128,7 @@ func Load(config *compileopts.Config, inputPkg string, typeChecker types.Config)
 	}
 
 	// List the dependencies of this package, in raw JSON format.
-	extraArgs := []string{"-json", "-deps"}
+	extraArgs := []string{"-json", "-deps", "-e"}
 	if config.TestConfig.CompileTestBinary {
 		extraArgs = append(extraArgs, "-test")
 	}
@@ -149,6 +149,7 @@ func Load(config *compileopts.Config, inputPkg string, typeChecker types.Config)
 
 	// Parse the returned json from `go list`.
 	decoder := json.NewDecoder(buf)
+	var pkgErrors []error
 	for {
 		pkg := &Package{
 			program:      p,
@@ -188,6 +189,12 @@ func Load(config *compileopts.Config, inputPkg string, typeChecker types.Config)
 					pos.Filename = strings.Join(fields[:len(fields)-1], ":")
 					pos.Line, _ = strconv.Atoi(fields[len(fields)-1])
 				}
+				if abs, err := filepath.Abs(pos.Filename); err == nil {
+					// Make the path absolute, so that error messages will be
+					// prettier (it will be turned back into a relative path
+					// when printing the error).
+					pos.Filename = abs
+				}
 				pos.Filename = p.getOriginalPath(pos.Filename)
 			}
 			err := scanner.Error{
@@ -195,10 +202,11 @@ func Load(config *compileopts.Config, inputPkg string, typeChecker types.Config)
 				Msg: pkg.Error.Err,
 			}
 			if len(pkg.Error.ImportStack) != 0 {
-				return nil, Error{
+				pkgErrors = append(pkgErrors, Error{
 					ImportStack: pkg.Error.ImportStack,
 					Err:         err,
-				}
+				})
+				continue
 			}
 			return nil, err
 		}
@@ -241,6 +249,13 @@ func Load(config *compileopts.Config, inputPkg string, typeChecker types.Config)
 		p.Packages[pkg.ImportPath] = pkg
 	}
 
+	if len(pkgErrors) != 0 {
+		// TODO: use errors.Join in Go 1.20.
+		return nil, Errors{
+			Errs: pkgErrors,
+		}
+	}
+
 	if config.TestConfig.CompileTestBinary && !strings.HasSuffix(p.sorted[len(p.sorted)-1].ImportPath, ".test") {
 		// Trying to compile a test binary but there are no test files in this
 		// package.
@@ -303,18 +318,34 @@ func (p *Program) MainPkg() *Package {
 func (p *Program) Parse() error {
 	// Parse all packages.
 	// TODO: do this in parallel.
+	var errors []error
 	for _, pkg := range p.sorted {
 		err := pkg.Parse()
 		if err != nil {
-			return err
+			errors = append(errors, err)
 		}
 	}
 
 	// Typecheck all packages.
 	for _, pkg := range p.sorted {
+		if !pkg.isParsed() || !pkg.allImportsChecked() {
+			if len(errors) == 0 {
+				// Sanity check.
+				// If there are no errors, all packages should have been parsed.
+				panic("unreachable")
+			}
+			continue
+		}
 		err := pkg.Check()
 		if err != nil {
-			return err
+			errors = append(errors, err)
+		}
+	}
+
+	if len(errors) != 0 {
+		// TODO: use errors.Join in Go 1.20.
+		return Errors{
+			Errs: errors,
 		}
 	}
 
@@ -339,12 +370,12 @@ func (p *Package) parseFile(path string, mode parser.Mode) (*ast.File, error) {
 	return parser.ParseFile(p.program.fset, originalPath, data, mode)
 }
 
-// Parse parses and typechecks this package.
+// Parse parses this package.
 //
 // Idempotent.
 func (p *Package) Parse() error {
-	if len(p.Files) != 0 {
-		return nil // nothing to do (?)
+	if p.isParsed() {
+		return nil // nothing to do
 	}
 
 	// Load the AST.
@@ -364,6 +395,24 @@ func (p *Package) Parse() error {
 	return nil
 }
 
+// isParsed returns whether this package has been parsed.
+func (p *Package) isParsed() bool {
+	// Special case: the unsafe package doesn't have files to parse but does
+	// have p.Pkg set once it is parsed.
+	return len(p.Files) != 0 || p.Pkg != nil
+}
+
+// allImportsChecked returns whether all imports of this package have been
+// type-checked.
+func (p *Package) allImportsChecked() bool {
+	for _, dep := range p.Imports {
+		if p.program.Packages[dep].Pkg == nil {
+			return false
+		}
+	}
+	return true
+}
+
 // Check runs the package through the typechecker. The package must already be
 // loaded and all dependencies must have been checked already.
 //

main.go

@@ -1288,10 +1288,9 @@ func usage(command string) {
 
 // try to make the path relative to the current working directory. If any error
 // occurs, this error is ignored and the absolute path is returned instead.
-func tryToMakePathRelative(dir string) string {
-	wd, err := os.Getwd()
-	if err != nil {
-		return dir
+func tryToMakePathRelative(dir, wd string) string {
+	if wd == "" {
+		return dir // working directory not found
 	}
 	relpath, err := filepath.Rel(wd, dir)
 	if err != nil {
@@ -1302,28 +1301,25 @@ func tryToMakePathRelative(dir string) string {
 
 // printCompilerError prints compiler errors using the provided logger function
 // (similar to fmt.Println).
-//
-// There is one exception: interp errors may print to stderr unconditionally due
-// to limitations in the LLVM bindings.
-func printCompilerError(logln func(...interface{}), err error) {
+func printCompilerError(err error, logln func(...interface{}), wd string) {
 	switch err := err.(type) {
 	case types.Error:
-		printCompilerError(logln, scanner.Error{
+		printCompilerError(scanner.Error{
 			Pos: err.Fset.Position(err.Pos),
 			Msg: err.Msg,
-		})
+		}, logln, wd)
 	case scanner.Error:
 		if !strings.HasPrefix(err.Pos.Filename, filepath.Join(goenv.Get("GOROOT"), "src")) && !strings.HasPrefix(err.Pos.Filename, filepath.Join(goenv.Get("TINYGOROOT"), "src")) {
 			// This file is not from the standard library (either the GOROOT or
 			// the TINYGOROOT). Make the path relative, for easier reading.
 			// Ignore any errors in the process (falling back to the absolute
 			// path).
-			err.Pos.Filename = tryToMakePathRelative(err.Pos.Filename)
+			err.Pos.Filename = tryToMakePathRelative(err.Pos.Filename, wd)
 		}
 		logln(err)
 	case scanner.ErrorList:
 		for _, scannerErr := range err {
-			printCompilerError(logln, *scannerErr)
+			printCompilerError(*scannerErr, logln, wd)
 		}
 	case *interp.Error:
 		logln("#", err.ImportPath)
@@ -1339,19 +1335,35 @@ func printCompilerError(logln func(...interface{}), err error) {
 			}
 		}
 	case loader.Errors:
-		logln("#", err.Pkg.ImportPath)
+		// Parser errors, typechecking errors, or `go list` errors.
+		// err.Pkg is nil for `go list` errors.
+		if err.Pkg != nil {
+			logln("#", err.Pkg.ImportPath)
+		}
 		for _, err := range err.Errs {
-			printCompilerError(logln, err)
+			printCompilerError(err, logln, wd)
 		}
 	case loader.Error:
-		logln(err.Err.Error())
-		logln("package", err.ImportStack[0])
-		for _, pkgPath := range err.ImportStack[1:] {
-			logln("\timports", pkgPath)
+		if err.Err.Pos.Filename != "" {
+			// Probably a syntax error in a dependency.
+			printCompilerError(err.Err, logln, wd)
+		} else {
+			// Probably an "import cycle not allowed" error.
+			logln("package", err.ImportStack[0])
+			for i := 1; i < len(err.ImportStack); i++ {
+				pkgPath := err.ImportStack[i]
+				if i == len(err.ImportStack)-1 {
+					// last package
+					logln("\timports", pkgPath+": "+err.Err.Error())
+				} else {
+					// not the last pacakge
+					logln("\timports", pkgPath)
+				}
+			}
 		}
 	case *builder.MultiError:
 		for _, err := range err.Errs {
-			printCompilerError(logln, err)
+			printCompilerError(err, logln, wd)
 		}
 	default:
 		logln("error:", err)
@@ -1359,10 +1371,14 @@ func printCompilerError(logln func(...interface{}), err error) {
 }
 
 func handleCompilerError(err error) {
+	wd, getwdErr := os.Getwd()
+	if getwdErr != nil {
+		wd = ""
+	}
 	if err != nil {
-		printCompilerError(func(args ...interface{}) {
+		printCompilerError(err, func(args ...interface{}) {
 			fmt.Fprintln(os.Stderr, args...)
-		}, err)
+		}, wd)
 		os.Exit(1)
 	}
 }
@@ -1764,9 +1780,13 @@ func main() {
 				stderr := (*testStderr)(buf)
 				passed, err := Test(pkgName, stdout, stderr, options, outpath)
 				if err != nil {
-					printCompilerError(func(args ...interface{}) {
+					wd, err := os.Getwd()
+					if err != nil {
+						wd = ""
+					}
+					printCompilerError(err, func(args ...interface{}) {
 						fmt.Fprintln(stderr, args...)
-					}, err)
+					}, wd)
 				}
 				if !passed {
 					select {

main_test.go

@@ -380,7 +380,7 @@ func runTestWithConfig(name string, t *testing.T, options compileopts.Options, c
 		return cmd.Run()
 	})
 	if err != nil {
-		printCompilerError(t.Log, err)
+		printCompilerError(err, t.Log, "")
 		t.Fail()
 		return
 	}

testdata/errors/cgo.go

@@ -0,0 +1,12 @@
+package main
+
+// #error hello
+// )))
+import "C"
+
+func main() {
+}
+
+// ERROR: # command-line-arguments
+// ERROR: cgo.go:3:5: error: hello
+// ERROR: cgo.go:4:4: error: expected identifier or '('

testdata/errors/importcycle/cycle.go

@@ -0,0 +1,3 @@
+package importcycle
+
+import _ "github.com/tinygo-org/tinygo/testdata/errors/importcycle"

testdata/errors/invaliddep/invaliddep.go

@@ -0,0 +1 @@
+ppackage // syntax error

testdata/errors/loader-importcycle.go

@@ -0,0 +1,10 @@
+package main
+
+import _ "github.com/tinygo-org/tinygo/testdata/errors/importcycle"
+
+func main() {
+}
+
+// ERROR: package command-line-arguments
+// ERROR: 	imports github.com/tinygo-org/tinygo/testdata/errors/importcycle
+// ERROR: 	imports github.com/tinygo-org/tinygo/testdata/errors/importcycle: import cycle not allowed

testdata/errors/loader-invaliddep.go

@@ -0,0 +1,8 @@
+package main
+
+import _ "github.com/tinygo-org/tinygo/testdata/errors/invaliddep"
+
+func main() {
+}
+
+// ERROR: invaliddep/invaliddep.go:1:1: expected 'package', found ppackage

testdata/errors/loader-invalidpackage.go

@@ -0,0 +1,3 @@
+ppackage // syntax error
+
+// ERROR: loader-invalidpackage.go:1:1: expected 'package', found ppackage

testdata/errors/loader-nopackage.go

@@ -0,0 +1,14 @@
+package main
+
+import (
+	_ "github.com/tinygo-org/tinygo/testdata/errors/non-existing-package"
+	_ "github.com/tinygo-org/tinygo/testdata/errors/non-existing-package-2"
+)
+
+func main() {
+}
+
+// ERROR: loader-nopackage.go:4:2: no required module provides package github.com/tinygo-org/tinygo/testdata/errors/non-existing-package; to add it:
+// ERROR: 	go get github.com/tinygo-org/tinygo/testdata/errors/non-existing-package
+// ERROR: loader-nopackage.go:5:2: no required module provides package github.com/tinygo-org/tinygo/testdata/errors/non-existing-package-2; to add it:
+// ERROR: 	go get github.com/tinygo-org/tinygo/testdata/errors/non-existing-package-2

testdata/errors/multi-1/types.go

@@ -0,0 +1,3 @@
+package typesmulti3
+
+var _ int = 3.14

testdata/errors/multi-2/syntax.go

@@ -0,0 +1,4 @@
+package syntaxmulti2
+
+func Foo2(type) {
+}

testdata/errors/multi-3/syntax.go

@@ -0,0 +1,4 @@
+package syntaxmulti1
+
+func Foo1(import) {
+}

testdata/errors/multi.go

@@ -0,0 +1,17 @@
+package main
+
+import (
+	_ "github.com/tinygo-org/tinygo/testdata/errors/multi-1"
+	_ "github.com/tinygo-org/tinygo/testdata/errors/multi-2"
+	_ "github.com/tinygo-org/tinygo/testdata/errors/multi-3"
+)
+
+func main() {
+}
+
+// ERROR: # github.com/tinygo-org/tinygo/testdata/errors/multi-2
+// ERROR: multi-2/syntax.go:3:11: expected ')', found 'type'
+// ERROR: # github.com/tinygo-org/tinygo/testdata/errors/multi-3
+// ERROR: multi-3/syntax.go:3:11: expected ')', found 'import'
+// ERROR: # github.com/tinygo-org/tinygo/testdata/errors/multi-1
+// ERROR: multi-1/types.go:3:13: cannot use 3.14 (untyped float constant) as int value in variable declaration (truncated)

testdata/errors/syntax.go

@@ -0,0 +1,7 @@
+package main
+
+func main(var) { // syntax error
+}
+
+// ERROR: # command-line-arguments
+// ERROR: syntax.go:3:11: expected ')', found 'var'

testdata/errors/types.go

@@ -0,0 +1,12 @@
+package main
+
+func main() {
+	var a int
+	a = "foobar"
+	nonexisting()
+}
+
+// ERROR: # command-line-arguments
+// ERROR: types.go:5:6: cannot use "foobar" (untyped string constant) as int value in assignment
+// ERROR: types.go:6:2: undefined: nonexisting
+// ERROR: types.go:4:6: a declared and not used