diagnostics: move diagnostic printing to a new package

This is a refactor, which should (in theory) not change the behavior of
the compiler. But since this is a pretty large change, there is a chance
there will be some regressions. For that reason, the previous commits
added a bunch of tests to make sure most error messages will not be
changed due to this refactor.

diagnostics/diagnostics.go

@@ -0,0 +1,196 @@
+// Package diagnostics formats compiler errors and prints them in a consistent
+// way.
+package diagnostics
+
+import (
+	"bytes"
+	"fmt"
+	"go/scanner"
+	"go/token"
+	"go/types"
+	"io"
+	"path/filepath"
+	"strings"
+
+	"github.com/tinygo-org/tinygo/builder"
+	"github.com/tinygo-org/tinygo/goenv"
+	"github.com/tinygo-org/tinygo/interp"
+	"github.com/tinygo-org/tinygo/loader"
+)
+
+// A single diagnostic.
+type Diagnostic struct {
+	Pos token.Position
+	Msg string
+}
+
+// One or multiple errors of a particular package.
+// It can also represent whole-program errors (like linker errors) that can't
+// easily be connected to a single package.
+type PackageDiagnostic struct {
+	ImportPath  string // the same ImportPath as in `go list -json`
+	Diagnostics []Diagnostic
+}
+
+// Diagnostics of a whole program. This can include errors belonging to multiple
+// packages, or just a single package.
+type ProgramDiagnostic []PackageDiagnostic
+
+// CreateDiagnostics reads the underlying errors in the error object and creates
+// a set of diagnostics that's sorted and can be readily printed.
+func CreateDiagnostics(err error) ProgramDiagnostic {
+	if err == nil {
+		return nil
+	}
+	switch err := err.(type) {
+	case *builder.MultiError:
+		var diags ProgramDiagnostic
+		for _, err := range err.Errs {
+			diags = append(diags, createPackageDiagnostic(err))
+		}
+		return diags
+	default:
+		return ProgramDiagnostic{
+			createPackageDiagnostic(err),
+		}
+	}
+}
+
+// Create diagnostics for a single package (though, in practice, it may also be
+// used for whole-program diagnostics in some cases).
+func createPackageDiagnostic(err error) PackageDiagnostic {
+	var pkgDiag PackageDiagnostic
+	switch err := err.(type) {
+	case loader.Errors:
+		if err.Pkg != nil {
+			pkgDiag.ImportPath = err.Pkg.ImportPath
+		}
+		for _, err := range err.Errs {
+			diags := createDiagnostics(err)
+			pkgDiag.Diagnostics = append(pkgDiag.Diagnostics, diags...)
+		}
+	case *interp.Error:
+		pkgDiag.ImportPath = err.ImportPath
+		w := &bytes.Buffer{}
+		fmt.Fprintln(w, err.Error())
+		if len(err.Inst) != 0 {
+			fmt.Fprintln(w, err.Inst)
+		}
+		if len(err.Traceback) > 0 {
+			fmt.Fprintln(w, "\ntraceback:")
+			for _, line := range err.Traceback {
+				fmt.Fprintln(w, line.Pos.String()+":")
+				fmt.Fprintln(w, line.Inst)
+			}
+		}
+		pkgDiag.Diagnostics = append(pkgDiag.Diagnostics, Diagnostic{
+			Msg: w.String(),
+		})
+	default:
+		pkgDiag.Diagnostics = createDiagnostics(err)
+	}
+	// TODO: sort
+	return pkgDiag
+}
+
+// Extract diagnostics from the given error message and return them as a slice
+// of errors (which in many cases will just be a single diagnostic).
+func createDiagnostics(err error) []Diagnostic {
+	switch err := err.(type) {
+	case types.Error:
+		return []Diagnostic{
+			{
+				Pos: err.Fset.Position(err.Pos),
+				Msg: err.Msg,
+			},
+		}
+	case scanner.Error:
+		return []Diagnostic{
+			{
+				Pos: err.Pos,
+				Msg: err.Msg,
+			},
+		}
+	case scanner.ErrorList:
+		var diags []Diagnostic
+		for _, err := range err {
+			diags = append(diags, createDiagnostics(*err)...)
+		}
+		return diags
+	case loader.Error:
+		if err.Err.Pos.Filename != "" {
+			// Probably a syntax error in a dependency.
+			return createDiagnostics(err.Err)
+		} else {
+			// Probably an "import cycle not allowed" error.
+			buf := &bytes.Buffer{}
+			fmt.Fprintln(buf, "package", err.ImportStack[0])
+			for i := 1; i < len(err.ImportStack); i++ {
+				pkgPath := err.ImportStack[i]
+				if i == len(err.ImportStack)-1 {
+					// last package
+					fmt.Fprintln(buf, "\timports", pkgPath+": "+err.Err.Error())
+				} else {
+					// not the last pacakge
+					fmt.Fprintln(buf, "\timports", pkgPath)
+				}
+			}
+			return []Diagnostic{
+				{Msg: buf.String()},
+			}
+		}
+	default:
+		return []Diagnostic{
+			{Msg: err.Error()},
+		}
+	}
+}
+
+// Write program diagnostics to the given writer with 'wd' as the relative
+// working directory.
+func (progDiag ProgramDiagnostic) WriteTo(w io.Writer, wd string) {
+	for _, pkgDiag := range progDiag {
+		pkgDiag.WriteTo(w, wd)
+	}
+}
+
+// Write package diagnostics to the given writer with 'wd' as the relative
+// working directory.
+func (pkgDiag PackageDiagnostic) WriteTo(w io.Writer, wd string) {
+	if pkgDiag.ImportPath != "" {
+		fmt.Fprintln(w, "#", pkgDiag.ImportPath)
+	}
+	for _, diag := range pkgDiag.Diagnostics {
+		diag.WriteTo(w, wd)
+	}
+}
+
+// Write this diagnostic to the given writer with 'wd' as the relative working
+// directory.
+func (diag Diagnostic) WriteTo(w io.Writer, wd string) {
+	if diag.Pos == (token.Position{}) {
+		fmt.Fprintln(w, diag.Msg)
+		return
+	}
+	pos := diag.Pos // make a copy
+	if !strings.HasPrefix(pos.Filename, filepath.Join(goenv.Get("GOROOT"), "src")) && !strings.HasPrefix(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).
+		pos.Filename = tryToMakePathRelative(pos.Filename, wd)
+	}
+	fmt.Fprintf(w, "%s: %s\n", pos, diag.Msg)
+}
+
+// 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, wd string) string {
+	if wd == "" {
+		return dir // working directory not found
+	}
+	relpath, err := filepath.Rel(wd, dir)
+	if err != nil {
+		return dir
+	}
+	return relpath
+}

errors_test.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"bytes"
-	"fmt"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -11,6 +10,7 @@ import (
 	"time"
 
 	"github.com/tinygo-org/tinygo/compileopts"
+	"github.com/tinygo-org/tinygo/diagnostics"
 )
 
 // Test the error messages of the TinyGo compiler.
@@ -59,9 +59,7 @@ func testErrorMessages(t *testing.T, filename string) {
 
 	// Write error message out as plain text.
 	var buf bytes.Buffer
-	printCompilerError(err, func(v ...interface{}) {
+	diagnostics.CreateDiagnostics(err).WriteTo(&buf, wd)
-		fmt.Fprintln(&buf, v...)
-	}, wd)
 	actual := strings.TrimRight(buf.String(), "\n")
 
 	// Check whether the error is as expected.

main.go

@@ -8,8 +8,6 @@ import (
 	"errors"
 	"flag"
 	"fmt"
-	"go/scanner"
-	"go/types"
 	"io"
 	"os"
 	"os/exec"
@@ -30,8 +28,8 @@ import (
 	"github.com/mattn/go-colorable"
 	"github.com/tinygo-org/tinygo/builder"
 	"github.com/tinygo-org/tinygo/compileopts"
+	"github.com/tinygo-org/tinygo/diagnostics"
 	"github.com/tinygo-org/tinygo/goenv"
-	"github.com/tinygo-org/tinygo/interp"
 	"github.com/tinygo-org/tinygo/loader"
 	"golang.org/x/tools/go/buildutil"
 	"tinygo.org/x/go-llvm"
@@ -1292,99 +1290,13 @@ 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, wd string) string {
-	if wd == "" {
-		return dir // working directory not found
-	}
-	relpath, err := filepath.Rel(wd, dir)
-	if err != nil {
-		return dir
-	}
-	return relpath
-}
-
-// printCompilerError prints compiler errors using the provided logger function
-// (similar to fmt.Println).
-func printCompilerError(err error, logln func(...interface{}), wd string) {
-	switch err := err.(type) {
-	case types.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, wd)
-		}
-		logln(err)
-	case scanner.ErrorList:
-		for _, scannerErr := range err {
-			printCompilerError(*scannerErr, logln, wd)
-		}
-	case *interp.Error:
-		logln("#", err.ImportPath)
-		logln(err.Error())
-		if len(err.Inst) != 0 {
-			logln(err.Inst)
-		}
-		if len(err.Traceback) > 0 {
-			logln("\ntraceback:")
-			for _, line := range err.Traceback {
-				logln(line.Pos.String() + ":")
-				logln(line.Inst)
-			}
-		}
-	case loader.Errors:
-		// 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:
-		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(err, logln, wd)
-		}
-	default:
-		logln("error:", err)
-	}
-}
-
 func handleCompilerError(err error) {
 	if err != nil {
 		wd, getwdErr := os.Getwd()
 		if getwdErr != nil {
 			wd = ""
 		}
-		printCompilerError(err, func(args ...interface{}) {
+		diagnostics.CreateDiagnostics(err).WriteTo(os.Stderr, wd)
-			fmt.Fprintln(os.Stderr, args...)
-		}, wd)
 		os.Exit(1)
 	}
 }
@@ -1790,9 +1702,7 @@ func main() {
 					if getwdErr != nil {
 						wd = ""
 					}
-					printCompilerError(err, func(args ...interface{}) {
+					diagnostics.CreateDiagnostics(err).WriteTo(os.Stderr, wd)
-						fmt.Fprintln(stderr, args...)
-					}, wd)
 				}
 				if !passed {
 					select {

main_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/aykevl/go-wasm"
 	"github.com/tinygo-org/tinygo/builder"
 	"github.com/tinygo-org/tinygo/compileopts"
+	"github.com/tinygo-org/tinygo/diagnostics"
 	"github.com/tinygo-org/tinygo/goenv"
 )
 
@@ -380,7 +381,11 @@ func runTestWithConfig(name string, t *testing.T, options compileopts.Options, c
 		return cmd.Run()
 	})
 	if err != nil {
-		printCompilerError(err, t.Log, "")
+		w := &bytes.Buffer{}
+		diagnostics.CreateDiagnostics(err).WriteTo(w, "")
+		for _, line := range strings.Split(strings.TrimRight(w.String(), "\n"), "\n") {
+			t.Log(line)
+		}
 		t.Fail()
 		return
 	}