]> Cypherpunks repositories - gostls13.git/commitdiff
- changed parser to return os.Error, removed ErrorHandler
authorRobert Griesemer <gri@golang.org>
Mon, 18 May 2009 21:59:16 +0000 (14:59 -0700)
committerRobert Griesemer <gri@golang.org>
Mon, 18 May 2009 21:59:16 +0000 (14:59 -0700)
- added IsValid predicate to token.Position
- updated pretty, godoc, gobuild
- updated/expanded test cases

R=rsc
DELTA=265  (97 added, 78 deleted, 90 changed)
OCL=28961
CL=29005

src/cmd/gobuild/util.go
src/lib/go/parser/parser.go
src/lib/go/parser/parser_test.go
src/lib/go/token/token.go
usr/gri/pretty/godoc.go
usr/gri/pretty/pretty.go

index 43d7aed79819558989985107e9733fe69c3267e6..462d2dc9536b443b9ca61d5252002d0b4c781e66 100644 (file)
@@ -179,9 +179,6 @@ func (s MakeString) String() string {
        return dollarString(string(s), "(", ")");
 }
 
-// TODO(rsc): parse.Parse should return an os.Error.
-var ParseError = os.NewError("parse errors");
-
 // TODO(rsc): Should this be in the AST library?
 func LitString(p []*ast.StringLit) (string, os.Error) {
        s := "";
@@ -201,9 +198,9 @@ func PackageImports(file string) (pkg string, imports []string, err1 os.Error) {
                return "", nil, err
        }
 
-       prog, ok := parser.Parse(f, nil, parser.ImportsOnly);
-       if !ok {
-               return "", nil, ParseError;
+       prog, err := parser.Parse(f, parser.ImportsOnly);
+       if err != nil {
+               return "", nil, err;
        }
 
        // Normally one must consult the types of decl and spec,
@@ -214,7 +211,7 @@ func PackageImports(file string) (pkg string, imports []string, err1 os.Error) {
                for _, spec := range decl.(*ast.GenDecl).Specs {
                        str, err := LitString(spec.(*ast.ImportSpec).Path);
                        if err != nil {
-                               return "", nil, ParseError;     // ParseError is better than os.EINVAL
+                               return "", nil, os.NewError("invalid import specifier");        // better than os.EINVAL
                        }
                        PushString(&imp, str);
                }
index c766526af79d9c395717ce04225a150331fd4a4f..8663289f8b34ed8bf7aa7e3d10209743337d8e52 100644 (file)
@@ -16,16 +16,41 @@ import (
        "go/scanner";
        "go/token";
        "io";
+       "os";
 )
 
 
-// An implementation of an ErrorHandler may be provided to the parser.
-// If a syntax error is encountered and a handler was installed, Error
-// is called with a position and an error message. The position points
-// to the beginning of the offending token.
+// A parser error is represented by an Error node. The position Pos, if
+// valid, points to the beginning of the offending token, and the error
+// condition is described by Msg.
 //
-type ErrorHandler interface {
-       Error(pos token.Position, msg string);
+type Error struct {
+       Pos token.Position;
+       Msg string;
+}
+
+
+func (e *Error) String() string {
+       pos := "";
+       if e.Pos.IsValid() {
+               pos = fmt.Sprintf("%d:%d: ", e.Pos.Line, e.Pos.Column);
+       }
+       return pos + e.Msg;
+}
+
+
+// Parser errors are returned as an ErrorList.
+type ErrorList []*Error
+
+
+// ErrorList implements the SortInterface.
+func (p ErrorList) Len() int  { return len(p); }
+func (p ErrorList) Swap(i, j int)  { p[i], p[j] = p[j], p[i]; }
+func (p ErrorList) Less(i, j int) bool  { return p[i].Pos.Offset < p[j].Pos.Offset; }
+
+
+func (p ErrorList) String() string {
+       return fmt.Sprintf("%d syntax errors", len(p));
 }
 
 
@@ -36,9 +61,8 @@ type interval struct {
 
 // The parser structure holds the parser's internal state.
 type parser struct {
+       errors vector.Vector;
        scanner scanner.Scanner;
-       err ErrorHandler;  // nil if no handler installed
-       hasErrors bool;
 
        // Tracing/debugging
        mode uint;  // parsing mode
@@ -185,11 +209,14 @@ func (p *parser) next() {
 }
 
 
-func (p *parser) error(pos token.Position, msg string) {
-       if p.err != nil {
-               p.err.Error(pos, msg);
+// The parser implements scanner.Error.
+func (p *parser) Error(pos token.Position, msg string) {
+       // Don't collect errors that are on the same line as the previous error
+       // in the hope to reduce the number of spurious errors due to incorrect
+       // parser synchronization.
+       if p.errors.Len() == 0 || p.errors.Last().(*Error).Pos.Line != pos.Line {
+               p.errors.Push(&Error{pos, msg});
        }
-       p.hasErrors = true;
 }
 
 
@@ -203,7 +230,7 @@ func (p *parser) error_expected(pos token.Position, msg string) {
                        msg += " " + string(p.lit);
                }
        }
-       p.error(pos, msg);
+       p.Error(pos, msg);
 }
 
 
@@ -470,7 +497,7 @@ func (p *parser) tryParameterType(ellipsis_ok bool) ast.Expr {
                p.next();
                if p.tok != token.RPAREN {
                        // "..." always must be at the very end of a parameter list
-                       p.error(pos, "expected type, found '...'");
+                       p.Error(pos, "expected type, found '...'");
                }
                return &ast.Ellipsis{pos};
        }
@@ -1115,7 +1142,7 @@ func (p *parser) checkExprOrType(x ast.Expr) ast.Expr {
                }
        case *ast.ArrayType:
                if len, is_ellipsis := t.Len.(*ast.Ellipsis); is_ellipsis {
-                       p.error(len.Pos(), "expected array length, found '...'");
+                       p.Error(len.Pos(), "expected array length, found '...'");
                        x = &ast.BadExpr{x.Pos()};
                }
        }
@@ -1223,7 +1250,7 @@ func (p *parser) parseSimpleStmt(label_ok bool) ast.Stmt {
                                return &ast.LabeledStmt{label, p.parseStatement()};
                        }
                }
-               p.error(x[0].Pos(), "illegal label declaration");
+               p.Error(x[0].Pos(), "illegal label declaration");
                return &ast.BadStmt{x[0].Pos()};
 
        case
@@ -1236,13 +1263,13 @@ func (p *parser) parseSimpleStmt(label_ok bool) ast.Stmt {
                p.next();
                y := p.parseExpressionList();
                if len(x) > 1 && len(y) > 1 && len(x) != len(y) {
-                       p.error(x[0].Pos(), "arity of lhs doesn't match rhs");
+                       p.Error(x[0].Pos(), "arity of lhs doesn't match rhs");
                }
                return &ast.AssignStmt{x, pos, tok, y};
        }
 
        if len(x) > 1 {
-               p.error(x[0].Pos(), "only one expression allowed");
+               p.Error(x[0].Pos(), "only one expression allowed");
                // continue with first expression
        }
 
@@ -1343,7 +1370,7 @@ func (p *parser) makeExpr(s ast.Stmt) ast.Expr {
        if es, is_expr := s.(*ast.ExprStmt); is_expr {
                return p.checkExpr(es.X);
        }
-       p.error(s.Pos(), "expected condition, found simple statement");
+       p.Error(s.Pos(), "expected condition, found simple statement");
        return &ast.BadExpr{s.Pos()};
 }
 
@@ -1846,7 +1873,7 @@ func (p *parser) parsePackage() *ast.Program {
        // Don't bother parsing the rest if we had errors already.
        // Likely not a Go source file at all.
 
-       if !p.hasErrors && p.mode & PackageClauseOnly == 0 {
+       if p.errors.Len() == 0 && p.mode & PackageClauseOnly == 0 {
                // import decls
                list := vector.New(0);
                for p.tok == token.IMPORT {
@@ -1886,32 +1913,28 @@ func (p *parser) parsePackage() *ast.Program {
 // ----------------------------------------------------------------------------
 // Parsing of entire programs.
 
-func readSource(src interface{}, err ErrorHandler) []byte {
-       errmsg := "invalid input type (or nil)";
-
-       switch s := src.(type) {
-       case string:
-               return io.StringBytes(s);
-       case []byte:
-               return s;
-       case *io.ByteBuffer:
-               // is io.Read, but src is already available in []byte form
-               if s != nil {
-                       return s.Data();
-               }
-       case io.Reader:
-               var buf io.ByteBuffer;
-               n, os_err := io.Copy(s, &buf);
-               if os_err == nil {
-                       return buf.Data();
+func readSource(src interface{}) ([]byte, os.Error) {
+       if src != nil {
+               switch s := src.(type) {
+               case string:
+                       return io.StringBytes(s), nil;
+               case []byte:
+                       return s, nil;
+               case *io.ByteBuffer:
+                       // is io.Read, but src is already available in []byte form
+                       if s != nil {
+                               return s.Data(), nil;
+                       }
+               case io.Reader:
+                       var buf io.ByteBuffer;
+                       n, err := io.Copy(s, &buf);
+                       if err != nil {
+                               return nil, err;
+                       }
+                       return buf.Data(), nil;
                }
-               errmsg = os_err.String();
        }
-
-       if err != nil {
-               err.Error(noPos, errmsg);
-       }
-       return nil;
+       return nil, os.ErrorString("invalid source");
 }
 
 
@@ -1919,25 +1942,26 @@ func readSource(src interface{}, err ErrorHandler) []byte {
 //
 // The program source src may be provided in a variety of formats. At the
 // moment the following types are supported: string, []byte, and io.Read.
+// The mode parameter controls the amount of source text parsed and other
+// optional parser functionality.
 //
-// The ErrorHandler err, if not nil, is invoked if src cannot be read and
-// for each syntax error found. The mode parameter controls the amount of
-// source text parsed and other optional parser functionality.
-//
-// Parse returns an AST and the boolean value true if no errors occured;
-// it returns a partial AST (or nil if the source couldn't be read) and
-// the boolean value false to indicate failure.
+// Parse returns a complete AST if no error occured. Otherwise, if the
+// source couldn't be read, the returned program is nil and the error
+// indicates the specific failure. If the source was read but syntax
+// errors were found, the result is a partial AST (with ast.BadX nodes
+// representing the fragments of erroneous source code) and an ErrorList
+// describing the syntax errors.
 //
-// If syntax errors were found, the AST may only be constructed partially,
-// with ast.BadX nodes representing the fragments of erroneous source code.
-//
-func Parse(src interface{}, err ErrorHandler, mode uint) (*ast.Program, bool) {
-       data := readSource(src, err);
+func Parse(src interface{}, mode uint) (*ast.Program, os.Error) {
+       data, err := readSource(src);
+       if err != nil {
+               return nil, err;
+       }
 
        // initialize parser state
        var p parser;
-       p.scanner.Init(data, err, mode & ParseComments != 0);
-       p.err = err;
+       p.errors.Init(0);
+       p.scanner.Init(data, &p, mode & ParseComments != 0);
        p.mode = mode;
        p.trace = mode & Trace != 0;  // for convenience (p.trace is used frequently)
        p.comments.Init(0);
@@ -1946,5 +1970,14 @@ func Parse(src interface{}, err ErrorHandler, mode uint) (*ast.Program, bool) {
        // parse program
        prog := p.parsePackage();
 
-       return prog, p.scanner.ErrorCount == 0 && !p.hasErrors;
+       // convert errors list, if any
+       if p.errors.Len() > 0 {
+               errors := make(ErrorList, p.errors.Len());
+               for i := 0; i < p.errors.Len(); i++ {
+                       errors[i] = p.errors.At(i).(*Error);
+               }
+               return prog, errors;
+       }
+
+       return prog, nil;
 }
index e35d18799cd6bcbe164a3b5b226ba0a921f235ee..887fcf80faea6dd56305b6cb609ddb5693bdd6d3 100644 (file)
@@ -12,36 +12,57 @@ import (
 )
 
 
-func TestParse0(t *testing.T) {
-       // test nil []bytes source
-       var src []byte;
-       prog, ok := Parse(src, nil, 0);
-       if ok {
-               t.Errorf("parse should have failed");
-       }
+var illegalInputs = []interface{} {
+       nil,
+       3.14,
+       []byte(nil),
+       "foo!",
 }
 
 
-func TestParse1(t *testing.T) {
-       // test string source
-       src := `package main import "fmt" func main() { fmt.Println("Hello, World!") }`;
-       prog, ok := Parse(src, nil, 0);
-       if !ok {
-               t.Errorf("parse failed");
+func TestParseIllegalInputs(t *testing.T) {
+       for _, src := range illegalInputs {
+               prog, err := Parse(src, 0);
+               if err == nil {
+                       t.Errorf("Parse(%v) should have failed", src);
+               }
        }
 }
 
-func TestParse2(t *testing.T) {
-       // test io.Read source
-       filename := "parser_test.go";
-       src, err := os.Open(filename, os.O_RDONLY, 0);
-       defer src.Close();
-       if err != nil {
-               t.Errorf("cannot open %s (%s)\n", filename, err.String());
+
+var validPrograms = []interface{} {
+       `package main`,
+       `package main import "fmt" func main() { fmt.Println("Hello, World!") }`,
+}
+
+
+func TestParseValidPrograms(t *testing.T) {
+       for _, src := range validPrograms {
+               prog, err := Parse(src, 0);
+               if err != nil {
+                       t.Errorf("Parse(%q) failed: %v", src, err);
+               }
        }
+}
+
+
+var validFiles = []string {
+       "parser.go",
+       "parser_test.go",
+}
+
+
+func TestParse3(t *testing.T) {
+       for _, filename := range validFiles {
+               src, err := os.Open(filename, os.O_RDONLY, 0);
+               defer src.Close();
+               if err != nil {
+                       t.Fatalf("os.Open(%s): %v\n", filename, err);
+               }
 
-       prog, ok := Parse(src, nil, 0);
-       if !ok {
-               t.Errorf("parse failed");
+               prog, err := Parse(src, 0);
+               if err != nil {
+                       t.Errorf("Parse(%q): %v", src, err);
+               }
        }
 }
index b71d0f03d397eccbd1a5bdb8d6d0acbd70dee3f4..a70a75a54023dcf3964eba2858456cad28e919d9 100644 (file)
@@ -324,6 +324,8 @@ func (tok Token) IsKeyword() bool {
 
 
 // Token source positions are represented by a Position value.
+// A Position is valid if the line number is > 0.
+//
 type Position struct {
        Offset int;  // byte offset, starting at 0
        Line int;  // line number, starting at 1
@@ -337,3 +339,9 @@ type Position struct {
 func (pos *Position) Pos() Position {
        return *pos;
 }
+
+
+// IsValid returns true if the position is valid.
+func (pos *Position) IsValid() bool {
+       return pos.Line > 0
+}
index 1848b58a22945f2110fc8ea75114cfc668cc5f63..dcf3aa10281bd1e3bec3a8d1fcad275a5cc0d0b3 100644 (file)
@@ -123,34 +123,6 @@ func ReadFile(name string) ([]byte, os.Error) {
 // ----------------------------------------------------------------------------
 // Parsing
 
-type rawError struct {
-       pos token.Position;
-       msg string;
-}
-
-
-type rawErrorVector struct {
-       vector.Vector;
-}
-
-
-func (v *rawErrorVector) At(i int) rawError { return v.Vector.At(i).(rawError) }
-func (v *rawErrorVector) Less(i, j int) bool { return v.At(i).pos.Offset < v.At(j).pos.Offset; }
-
-
-func (v *rawErrorVector) Error(pos token.Position, msg string) {
-       // only collect errors that are on a new line
-       // in the hope to avoid most follow-up errors
-       lastLine := 0;
-       if n := v.Len(); n > 0 {
-               lastLine = v.At(n - 1).pos.Line;
-       }
-       if lastLine != pos.Line {
-               v.Push(rawError{pos, msg});
-       }
-}
-
-
 // A single error in the parsed file.
 type parseError struct {
        src []byte;     // source before error
@@ -183,25 +155,28 @@ func parse(path string, mode uint) (*ast.Program, *parseErrors) {
                return nil, &parseErrors{path, errs, nil};
        }
 
-       var raw rawErrorVector;
-       prog, ok := parser.Parse(src, &raw, mode);
-       if !ok {
+       prog, err := parser.Parse(src, mode);
+       if err != nil {
                // sort and convert error list
-               sort.Sort(&raw);
-               errs := make([]parseError, raw.Len() + 1);      // +1 for final fragment of source
-               offs := 0;
-               for i := 0; i < raw.Len(); i++ {
-                       r := raw.At(i);
-                       // Should always be true, but check for robustness.
-                       if 0 <= r.pos.Offset && r.pos.Offset <= len(src) {
-                               errs[i].src = src[offs : r.pos.Offset];
-                               offs = r.pos.Offset;
+               if errors, ok := err.(parser.ErrorList); ok {
+                       sort.Sort(errors);
+                       errs := make([]parseError, len(errors) + 1);    // +1 for final fragment of source
+                       offs := 0;
+                       for i, r := range errors {
+                               // Should always be true, but check for robustness.
+                               if 0 <= r.Pos.Offset && r.Pos.Offset <= len(src) {
+                                       errs[i].src = src[offs : r.Pos.Offset];
+                                       offs = r.Pos.Offset;
+                               }
+                               errs[i].line = r.Pos.Line;
+                               errs[i].msg = r.Msg;
                        }
-                       errs[i].line = r.pos.Line;
-                       errs[i].msg = r.msg;
+                       errs[len(errors)].src = src[offs : len(src)];
+                       return nil, &parseErrors{path, errs, src};
+               } else {
+                       // TODO should have some default handling here to be more robust
+                       panic("unreachable");
                }
-               errs[raw.Len()].src = src[offs : len(src)];
-               return nil, &parseErrors{path, errs, src};
        }
 
        return prog, nil;
index 90cc96dbbc5a6a7f86a35ae7e768b8ce8f7fe764..56b36e45a38dbc9d565a4fb0a2962f448d8f0396 100644 (file)
@@ -5,17 +5,17 @@
 package main
 
 import (
+       "astprinter";
        "flag";
        "fmt";
+       "format";
        "go/ast";
        "go/parser";
        "go/token";
        "io";
        "os";
+       "sort";
        "tabwriter";
-
-       "astprinter";
-       "format";
 )
 
 
@@ -70,32 +70,9 @@ func makeTabwriter(writer io.Writer) *tabwriter.Writer {
 }
 
 
-// TODO(gri) move this into parser as default handler
-type ErrorHandler struct {
-       filename string;
-       lastline int;
-}
-
-
-func (h *ErrorHandler) Error(pos token.Position, msg string) {
-       // only report errors that are on a new line
-       // in the hope to avoid most follow-up errors
-       if pos.Line == h.lastline {
-               return;
-       }
-       h.lastline = pos.Line;
-
-       // report error
-       fmt.Fprintf(os.Stderr, "%s:%d:", h.filename, pos.Line);
-       if columns {
-               fmt.Fprintf(os.Stderr, "%d:", pos.Column);
-       }
-       fmt.Fprintf(os.Stderr, " %s\n", msg);
-}
-
-
 func isValidPos(w io.Writer, env, value interface{}, name string) bool {
-       return value.(token.Position).Line > 0;
+       pos := value.(token.Position);
+       return pos.IsValid();
 }
 
 
@@ -159,8 +136,16 @@ func main() {
                        continue;  // proceed with next file
                }
 
-               prog, ok := parser.Parse(src, &ErrorHandler{filename, 0}, mode);
-               if !ok {
+               prog, err := parser.Parse(src, mode);
+               if err != nil {
+                       if errors, ok := err.(parser.ErrorList); ok {
+                               sort.Sort(errors);
+                               for _, e := range errors {
+                                       fmt.Fprintf(os.Stderr, "%s:%v\n", filename, e);
+                               }
+                       } else {
+                               fmt.Fprintf(os.Stderr, "%s: %v\n", filename, err);
+                       }
                        exitcode = 1;
                        continue;  // proceed with next file
                }