From d7773d3e86e82a64c79a04fa70f740daf333a3be Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Tue, 9 Jul 2024 18:08:51 +0200 Subject: [PATCH] loader: handle `go list` errors inside TinyGo Instead of exiting with an error, handle these errors internally. This will enable a few improvements in the future. --- errors_test.go | 18 +++++++++++++++- loader/loader.go | 21 ++++++++++++++++--- main.go | 26 +++++++++++++++++++----- testdata/errors/importcycle/cycle.go | 3 +++ testdata/errors/invaliddep/invaliddep.go | 1 + testdata/errors/loader-importcycle.go | 10 +++++++++ testdata/errors/loader-invaliddep.go | 8 ++++++++ testdata/errors/loader-invalidpackage.go | 3 +++ testdata/errors/loader-nopackage.go | 14 +++++++++++++ 9 files changed, 95 insertions(+), 9 deletions(-) create mode 100644 testdata/errors/importcycle/cycle.go create mode 100644 testdata/errors/invaliddep/invaliddep.go create mode 100644 testdata/errors/loader-importcycle.go create mode 100644 testdata/errors/loader-invaliddep.go create mode 100644 testdata/errors/loader-invalidpackage.go create mode 100644 testdata/errors/loader-nopackage.go diff --git a/errors_test.go b/errors_test.go index 69c29148..1ee9a0e1 100644 --- a/errors_test.go +++ b/errors_test.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -16,6 +17,10 @@ import ( func TestErrors(t *testing.T) { for _, name := range []string{ "cgo", + "loader-importcycle", + "loader-invaliddep", + "loader-invalidpackage", + "loader-nopackage", "syntax", "types", } { @@ -57,11 +62,22 @@ func testErrorMessages(t *testing.T, filename string) { actual := strings.TrimRight(buf.String(), "\n") // Check whether the error is as expected. - if actual != 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) diff --git a/loader/loader.go b/loader/loader.go index a874291a..fe75e6c9 100644 --- a/loader/loader.go +++ b/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. diff --git a/main.go b/main.go index e18f12f1..66a17213 100644 --- a/main.go +++ b/main.go @@ -1340,15 +1340,31 @@ func printCompilerError(err error, logln func(...interface{}), wd string) { } } 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(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 { diff --git a/testdata/errors/importcycle/cycle.go b/testdata/errors/importcycle/cycle.go new file mode 100644 index 00000000..40ecf5e2 --- /dev/null +++ b/testdata/errors/importcycle/cycle.go @@ -0,0 +1,3 @@ +package importcycle + +import _ "github.com/tinygo-org/tinygo/testdata/errors/importcycle" diff --git a/testdata/errors/invaliddep/invaliddep.go b/testdata/errors/invaliddep/invaliddep.go new file mode 100644 index 00000000..9b0b1c57 --- /dev/null +++ b/testdata/errors/invaliddep/invaliddep.go @@ -0,0 +1 @@ +ppackage // syntax error diff --git a/testdata/errors/loader-importcycle.go b/testdata/errors/loader-importcycle.go new file mode 100644 index 00000000..4571bdb4 --- /dev/null +++ b/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 diff --git a/testdata/errors/loader-invaliddep.go b/testdata/errors/loader-invaliddep.go new file mode 100644 index 00000000..db935d38 --- /dev/null +++ b/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 diff --git a/testdata/errors/loader-invalidpackage.go b/testdata/errors/loader-invalidpackage.go new file mode 100644 index 00000000..6d788104 --- /dev/null +++ b/testdata/errors/loader-invalidpackage.go @@ -0,0 +1,3 @@ +ppackage // syntax error + +// ERROR: loader-invalidpackage.go:1:1: expected 'package', found ppackage diff --git a/testdata/errors/loader-nopackage.go b/testdata/errors/loader-nopackage.go new file mode 100644 index 00000000..c0087fc0 --- /dev/null +++ b/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