modfile: return structured errors from Parse and ParseLax
Parse and ParseLax now return an ErrorList, which is a slice of Errors
that satisfies the error interface. Each Error now contains a file
name and position. The verb and module path fields are optional.
Fixes golang/go#36486
Change-Id: I2a244c8753cfce4bb290ddb8bcea5ee73c45f79f
Reviewed-on: https://go-review.googlesource.com/c/mod/+/214430
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/modfile/read.go b/modfile/read.go
index 616d00e..1577efd 100644
--- a/modfile/read.go
+++ b/modfile/read.go
@@ -9,6 +9,7 @@
import (
"bytes"
+ "errors"
"fmt"
"os"
"strconv"
@@ -333,8 +334,8 @@
endRule int // position of end of current rule
// Parser state.
- file *FileSyntax // returned top-level syntax tree
- parseError error // error encountered during parsing
+ file *FileSyntax // returned top-level syntax tree
+ parseErrors ErrorList // errors encountered during parsing
// Comment assignment state.
pre []Expr // all expressions, in preorder traversal
@@ -358,19 +359,22 @@
// Turn both into error returns. Catching bug panics is
// especially important when processing many files.
defer func() {
- if e := recover(); e != nil {
- if e == in.parseError {
- err = in.parseError
- } else {
- err = fmt.Errorf("%s:%d:%d: internal error: %v", in.filename, in.pos.Line, in.pos.LineRune, e)
- }
+ if e := recover(); e != nil && e != &in.parseErrors {
+ in.parseErrors = append(in.parseErrors, Error{
+ Filename: in.filename,
+ Pos: in.pos,
+ Err: fmt.Errorf("internal error: %v", e),
+ })
+ }
+ if err == nil && len(in.parseErrors) > 0 {
+ err = in.parseErrors
}
}()
// Invoke the parser.
in.parseFile()
- if in.parseError != nil {
- return nil, in.parseError
+ if len(in.parseErrors) > 0 {
+ return nil, in.parseErrors
}
in.file.Name = in.filename
@@ -387,8 +391,12 @@
if s == "syntax error" && in.lastToken != "" {
s += " near " + in.lastToken
}
- in.parseError = fmt.Errorf("%s:%d:%d: %v", in.filename, in.pos.Line, in.pos.LineRune, s)
- panic(in.parseError)
+ in.parseErrors = append(in.parseErrors, Error{
+ Filename: in.filename,
+ Pos: in.pos,
+ Err: errors.New(s),
+ })
+ panic(&in.parseErrors)
}
// eof reports whether the input has reached end of file.
@@ -518,7 +526,7 @@
}
if in.peekPrefix("/*") {
- in.Error(fmt.Sprintf("mod files must use // comments (not /* */ comments)"))
+ in.Error("mod files must use // comments (not /* */ comments)")
}
// Found non-space non-comment.
@@ -587,7 +595,7 @@
break
}
if in.peekPrefix("/*") {
- in.Error(fmt.Sprintf("mod files must use // comments (not /* */ comments)"))
+ in.Error("mod files must use // comments (not /* */ comments)")
}
in.readRune()
}
@@ -668,7 +676,7 @@
for _, x := range in.pre {
start, _ := x.Span()
if debug {
- fmt.Printf("pre %T :%d:%d #%d\n", x, start.Line, start.LineRune, start.Byte)
+ fmt.Fprintf(os.Stderr, "pre %T :%d:%d #%d\n", x, start.Line, start.LineRune, start.Byte)
}
xcom := x.Comment()
for len(line) > 0 && start.Byte >= line[0].Start.Byte {
@@ -695,7 +703,7 @@
start, end := x.Span()
if debug {
- fmt.Printf("post %T :%d:%d #%d :%d:%d #%d\n", x, start.Line, start.LineRune, start.Byte, end.Line, end.LineRune, end.Byte)
+ fmt.Fprintf(os.Stderr, "post %T :%d:%d #%d :%d:%d #%d\n", x, start.Line, start.LineRune, start.Byte, end.Line, end.LineRune, end.Byte)
}
// Do not assign suffix comments to end of line block or whole file.
diff --git a/modfile/rule.go b/modfile/rule.go
index 62af068..67914a8 100644
--- a/modfile/rule.go
+++ b/modfile/rule.go
@@ -5,7 +5,6 @@
package modfile
import (
- "bytes"
"errors"
"fmt"
"path/filepath"
@@ -120,7 +119,7 @@
Syntax: fs,
}
- var errs bytes.Buffer
+ var errs ErrorList
for _, x := range fs.Stmt {
switch x := x.(type) {
case *Line:
@@ -129,14 +128,22 @@
case *LineBlock:
if len(x.Token) > 1 {
if strict {
- fmt.Fprintf(&errs, "%s:%d: unknown block type: %s\n", file, x.Start.Line, strings.Join(x.Token, " "))
+ errs = append(errs, Error{
+ Filename: file,
+ Pos: x.Start,
+ Err: fmt.Errorf("unknown block type: %s", strings.Join(x.Token, " ")),
+ })
}
continue
}
switch x.Token[0] {
default:
if strict {
- fmt.Fprintf(&errs, "%s:%d: unknown block type: %s\n", file, x.Start.Line, strings.Join(x.Token, " "))
+ errs = append(errs, Error{
+ Filename: file,
+ Pos: x.Start,
+ Err: fmt.Errorf("unknown block type: %s", strings.Join(x.Token, " ")),
+ })
}
continue
case "module", "require", "exclude", "replace":
@@ -147,15 +154,15 @@
}
}
- if errs.Len() > 0 {
- return nil, errors.New(strings.TrimRight(errs.String(), "\n"))
+ if len(errs) > 0 {
+ return nil, errs
}
return f, nil
}
var GoVersionRE = lazyregexp.New(`^([1-9][0-9]*)\.(0|[1-9][0-9]*)$`)
-func (f *File) add(errs *bytes.Buffer, line *Line, verb string, args []string, fix VersionFixer, strict bool) {
+func (f *File) add(errs *ErrorList, line *Line, verb string, args []string, fix VersionFixer, strict bool) {
// If strict is false, this module is a dependency.
// We ignore all unknown directives as well as main-module-only
// directives like replace and exclude. It will work better for
@@ -171,60 +178,79 @@
}
}
+ wrapModPathError := func(modPath string, err error) {
+ *errs = append(*errs, Error{
+ Filename: f.Syntax.Name,
+ Pos: line.Start,
+ ModPath: modPath,
+ Verb: verb,
+ Err: err,
+ })
+ }
+ wrapError := func(err error) {
+ *errs = append(*errs, Error{
+ Filename: f.Syntax.Name,
+ Pos: line.Start,
+ Err: err,
+ })
+ }
+ errorf := func(format string, args ...interface{}) {
+ wrapError(fmt.Errorf(format, args...))
+ }
+
switch verb {
default:
- fmt.Fprintf(errs, "%s:%d: unknown directive: %s\n", f.Syntax.Name, line.Start.Line, verb)
+ errorf("unknown directive: %s", verb)
case "go":
if f.Go != nil {
- fmt.Fprintf(errs, "%s:%d: repeated go statement\n", f.Syntax.Name, line.Start.Line)
+ errorf("repeated go statement")
return
}
if len(args) != 1 || !GoVersionRE.MatchString(args[0]) {
- fmt.Fprintf(errs, "%s:%d: usage: go 1.23\n", f.Syntax.Name, line.Start.Line)
+ errorf("usage: go 1.23")
return
}
f.Go = &Go{Syntax: line}
f.Go.Version = args[0]
case "module":
if f.Module != nil {
- fmt.Fprintf(errs, "%s:%d: repeated module statement\n", f.Syntax.Name, line.Start.Line)
+ errorf("repeated module statement")
return
}
f.Module = &Module{Syntax: line}
if len(args) != 1 {
-
- fmt.Fprintf(errs, "%s:%d: usage: module module/path\n", f.Syntax.Name, line.Start.Line)
+ errorf("usage: module module/path")
return
}
s, err := parseString(&args[0])
if err != nil {
- fmt.Fprintf(errs, "%s:%d: invalid quoted string: %v\n", f.Syntax.Name, line.Start.Line, err)
+ errorf("invalid quoted string: %v", err)
return
}
f.Module.Mod = module.Version{Path: s}
case "require", "exclude":
if len(args) != 2 {
- fmt.Fprintf(errs, "%s:%d: usage: %s module/path v1.2.3\n", f.Syntax.Name, line.Start.Line, verb)
+ errorf("usage: %s module/path v1.2.3", verb)
return
}
s, err := parseString(&args[0])
if err != nil {
- fmt.Fprintf(errs, "%s:%d: invalid quoted string: %v\n", f.Syntax.Name, line.Start.Line, err)
+ errorf("invalid quoted string: %v", err)
return
}
v, err := parseVersion(verb, s, &args[1], fix)
if err != nil {
- fmt.Fprintf(errs, "%s:%d: %v\n", f.Syntax.Name, line.Start.Line, err)
+ wrapError(err)
return
}
pathMajor, err := modulePathMajor(s)
if err != nil {
- fmt.Fprintf(errs, "%s:%d: %v\n", f.Syntax.Name, line.Start.Line, err)
+ wrapError(err)
return
}
if err := module.CheckPathMajor(v, pathMajor); err != nil {
- fmt.Fprintf(errs, "%s:%d: %v\n", f.Syntax.Name, line.Start.Line, &Error{Verb: verb, ModPath: s, Err: err})
+ wrapModPathError(s, err)
return
}
if verb == "require" {
@@ -245,55 +271,55 @@
arrow = 1
}
if len(args) < arrow+2 || len(args) > arrow+3 || args[arrow] != "=>" {
- fmt.Fprintf(errs, "%s:%d: usage: %s module/path [v1.2.3] => other/module v1.4\n\t or %s module/path [v1.2.3] => ../local/directory\n", f.Syntax.Name, line.Start.Line, verb, verb)
+ errorf("usage: %s module/path [v1.2.3] => other/module v1.4\n\t or %s module/path [v1.2.3] => ../local/directory", verb, verb)
return
}
s, err := parseString(&args[0])
if err != nil {
- fmt.Fprintf(errs, "%s:%d: invalid quoted string: %v\n", f.Syntax.Name, line.Start.Line, err)
+ errorf("invalid quoted string: %v", err)
return
}
pathMajor, err := modulePathMajor(s)
if err != nil {
- fmt.Fprintf(errs, "%s:%d: %v\n", f.Syntax.Name, line.Start.Line, err)
+ wrapModPathError(s, err)
return
}
var v string
if arrow == 2 {
v, err = parseVersion(verb, s, &args[1], fix)
if err != nil {
- fmt.Fprintf(errs, "%s:%d: %v\n", f.Syntax.Name, line.Start.Line, err)
+ wrapError(err)
return
}
if err := module.CheckPathMajor(v, pathMajor); err != nil {
- fmt.Fprintf(errs, "%s:%d: %v\n", f.Syntax.Name, line.Start.Line, &Error{Verb: verb, ModPath: s, Err: err})
+ wrapModPathError(s, err)
return
}
}
ns, err := parseString(&args[arrow+1])
if err != nil {
- fmt.Fprintf(errs, "%s:%d: invalid quoted string: %v\n", f.Syntax.Name, line.Start.Line, err)
+ errorf("invalid quoted string: %v", err)
return
}
nv := ""
if len(args) == arrow+2 {
if !IsDirectoryPath(ns) {
- fmt.Fprintf(errs, "%s:%d: replacement module without version must be directory path (rooted or starting with ./ or ../)\n", f.Syntax.Name, line.Start.Line)
+ errorf("replacement module without version must be directory path (rooted or starting with ./ or ../)")
return
}
if filepath.Separator == '/' && strings.Contains(ns, `\`) {
- fmt.Fprintf(errs, "%s:%d: replacement directory appears to be Windows path (on a non-windows system)\n", f.Syntax.Name, line.Start.Line)
+ errorf("replacement directory appears to be Windows path (on a non-windows system)")
return
}
}
if len(args) == arrow+3 {
nv, err = parseVersion(verb, ns, &args[arrow+2], fix)
if err != nil {
- fmt.Fprintf(errs, "%s:%d: %v\n", f.Syntax.Name, line.Start.Line, err)
+ wrapError(err)
return
}
if IsDirectoryPath(ns) {
- fmt.Fprintf(errs, "%s:%d: replacement module directory path %q cannot have version\n", f.Syntax.Name, line.Start.Line, ns)
+ errorf("replacement module directory path %q cannot have version", ns)
return
}
}
@@ -405,14 +431,42 @@
return t, nil
}
+type ErrorList []Error
+
+func (e ErrorList) Error() string {
+ errStrs := make([]string, len(e))
+ for i, err := range e {
+ errStrs[i] = err.Error()
+ }
+ return strings.Join(errStrs, "\n")
+}
+
type Error struct {
- Verb string
- ModPath string
- Err error
+ Filename string
+ Pos Position
+ Verb string
+ ModPath string
+ Err error
}
func (e *Error) Error() string {
- return fmt.Sprintf("%s %s: %v", e.Verb, e.ModPath, e.Err)
+ var pos string
+ if e.Pos.LineRune > 1 {
+ // Don't print LineRune if it's 1 (beginning of line).
+ // It's always 1 except in scanner errors, which are rare.
+ pos = fmt.Sprintf("%s:%d:%d: ", e.Filename, e.Pos.Line, e.Pos.LineRune)
+ } else if e.Pos.Line > 0 {
+ pos = fmt.Sprintf("%s:%d: ", e.Filename, e.Pos.Line)
+ } else if e.Filename != "" {
+ pos = fmt.Sprintf("%s: ", e.Filename)
+ }
+
+ var directive string
+ if e.ModPath != "" {
+ directive = fmt.Sprintf("%s %s: ", e.Verb, e.ModPath)
+ }
+
+ return pos + directive + e.Err.Error()
}
func (e *Error) Unwrap() error { return e.Err }
