]> Cypherpunks repositories - gostls13.git/commitdiff
go/scanner: report errors for incorrect line directives
authorRobert Griesemer <gri@golang.org>
Tue, 13 Mar 2018 00:00:55 +0000 (17:00 -0700)
committerRobert Griesemer <gri@golang.org>
Thu, 15 Mar 2018 18:01:05 +0000 (18:01 +0000)
Based on decision for #24183. This makes the go/scanner behavior
match cmd/compile behavior. Adjusted a go/printer test that assumed
silent behavior for invalid line directive, and added more scanner
tests verifying the correct error position and message for invalid
line directives.

The filenames in line directives now remain untouched by the scanner;
there is no cleanup or conversion of relative into absolute paths
anymore, in sync with what the compiler's scanner/parser are doing.
Any kind of filename transformation has to be done by a client. This
makes the scanner code simpler and also more predictable.

For #24183.

Change-Id: Ia091548e1d3d89dfdf6e7d82dab50bea05742ce3
Reviewed-on: https://go-review.googlesource.com/100235
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
src/go/printer/testdata/comments.golden
src/go/printer/testdata/comments.input
src/go/scanner/scanner.go
src/go/scanner/scanner_test.go

index b91e79dbf2886c80abd2c95647683c4e4b2d4dbb..1a21fff331473753c408478c794099348c8e140e 100644 (file)
@@ -702,9 +702,8 @@ func _() {
        //line foo:2
        _ = 2
 
-       // The following is not a legal line directive (negative line number), but
-       // it looks like one, so don't indent it:
-//line foo:-3
+       // The following is not a legal line directive (missing colon):
+//line foo -3
        _ = 3
 }
 
index 18337a4995af527775723f4b8170cf318921505f..aa428a2aa68bfe4838fa5401f5e3b5d816a4e412 100644 (file)
@@ -699,9 +699,8 @@ func _() {
        //line foo:2
        _ = 2
 
-// The following is not a legal line directive (negative line number), but
-// it looks like one, so don't indent it:
-//line foo:-3
+// The following is not a legal line directive (missing colon):
+//line foo -3
        _ = 3
 }
 
index 83a6ca07fc4e5462ac7dcaa4f9db62d71dfe2e8d..9f5855662d9e340e37e33f13cc70cf634f09598b 100644 (file)
@@ -214,10 +214,6 @@ var prefix = []byte("line ")
 // as a line directive. If successful, it updates the line info table
 // for the position next per the line directive.
 func (s *Scanner) updateLineInfo(next, offs int, text []byte) {
-       // the existing code used to ignore incorrect line/column values
-       // TODO(gri) adjust once we agree on the directive syntax (issue #24183)
-       reportErrors := false
-
        // extract comment text
        if text[1] == '*' {
                text = text[:len(text)-2] // lop off trailing "*/"
@@ -233,9 +229,7 @@ func (s *Scanner) updateLineInfo(next, offs int, text []byte) {
 
        if !ok {
                // text has a suffix :xxx but xxx is not a number
-               if reportErrors {
-                       s.error(offs+i, "invalid line number: "+string(text[i:]))
-               }
+               s.error(offs+i, "invalid line number: "+string(text[i:]))
                return
        }
 
@@ -246,9 +240,7 @@ func (s *Scanner) updateLineInfo(next, offs int, text []byte) {
                i, i2 = i2, i
                line, col = n2, n
                if col == 0 {
-                       if reportErrors {
-                               s.error(offs+i2, "invalid column number: "+string(text[i2:]))
-                       }
+                       s.error(offs+i2, "invalid column number: "+string(text[i2:]))
                        return
                }
                text = text[:i2-1] // lop off ":col"
@@ -258,26 +250,14 @@ func (s *Scanner) updateLineInfo(next, offs int, text []byte) {
        }
 
        if line == 0 {
-               if reportErrors {
-                       s.error(offs+i, "invalid line number: "+string(text[i:]))
-               }
+               s.error(offs+i, "invalid line number: "+string(text[i:]))
                return
        }
 
-       // the existing code used to trim whitespace around filenames
-       // TODO(gri) adjust once we agree on the directive syntax (issue #24183)
-       filename := string(bytes.TrimSpace(text[:i-1])) // lop off ":line", and trim white space
-
        // If we have a column (//line filename:line:col form),
        // an empty filename means to use the previous filename.
-       if filename != "" {
-               filename = filepath.Clean(filename)
-               if !filepath.IsAbs(filename) {
-                       // make filename relative to current directory
-                       filename = filepath.Join(s.dir, filename)
-               }
-       } else if ok2 {
-               // use existing filename
+       filename := string(text[:i-1]) // lop off ":line", and trim white space
+       if filename == "" && ok2 {
                filename = s.file.Position(s.file.Pos(offs)).Filename
        }
 
index 7204c385375d4c1460e9092c25518b6f2855d444..7cc79fa820d1c5376ee6bfebbc5afd66ee848cbf 100644 (file)
@@ -9,7 +9,6 @@ import (
        "io/ioutil"
        "os"
        "path/filepath"
-       "runtime"
        "testing"
 )
 
@@ -504,69 +503,55 @@ func TestSemis(t *testing.T) {
 
 type segment struct {
        srcline      string // a line of source text
-       filename     string // filename for current token
-       line, column int    // line number for current token
+       filename     string // filename for current token; error message for invalid line directives
+       line, column int    // line and column for current token; error position for invalid line directives
 }
 
 var segments = []segment{
        // exactly one token per line since the test consumes one token per segment
-       {"  line1", filepath.Join("dir", "TestLineDirectives"), 1, 3},
-       {"\nline2", filepath.Join("dir", "TestLineDirectives"), 2, 1},
-       {"\nline3  //line File1.go:100", filepath.Join("dir", "TestLineDirectives"), 3, 1}, // bad line comment, ignored
-       {"\nline4", filepath.Join("dir", "TestLineDirectives"), 4, 1},
-       {"\n//line File1.go:100\n  line100", filepath.Join("dir", "File1.go"), 100, 0},
-       {"\n//line  \t :42\n  line1", "", 42, 0},
-       {"\n//line File2.go:200\n  line200", filepath.Join("dir", "File2.go"), 200, 0},
-       {"\n//line foo\t:42\n  line42", filepath.Join("dir", "foo"), 42, 0},
-       {"\n //line foo:42\n  line44", filepath.Join("dir", "foo"), 44, 0},           // bad line comment, ignored
-       {"\n//line foo 42\n  line46", filepath.Join("dir", "foo"), 46, 0},            // bad line comment, ignored
-       {"\n//line foo:42 extra text\n  line48", filepath.Join("dir", "foo"), 48, 0}, // bad line comment, ignored
-       {"\n//line ./foo:42\n  line42", filepath.Join("dir", "foo"), 42, 0},
-       {"\n//line a/b/c/File1.go:100\n  line100", filepath.Join("dir", "a", "b", "c", "File1.go"), 100, 0},
+       {"  line1", "TestLineDirectives", 1, 3},
+       {"\nline2", "TestLineDirectives", 2, 1},
+       {"\nline3  //line File1.go:100", "TestLineDirectives", 3, 1}, // bad line comment, ignored
+       {"\nline4", "TestLineDirectives", 4, 1},
+       {"\n//line File1.go:100\n  line100", "File1.go", 100, 0},
+       {"\n//line  \t :42\n  line1", " \t ", 42, 0},
+       {"\n//line File2.go:200\n  line200", "File2.go", 200, 0},
+       {"\n//line foo\t:42\n  line42", "foo\t", 42, 0},
+       {"\n //line foo:42\n  line43", "foo\t", 44, 0}, // bad line comment, ignored (use existing, prior filename)
+       {"\n//line foo 42\n  line44", "foo\t", 46, 0},  // bad line comment, ignored (use existing, prior filename)
+       {"\n//line /bar:42\n  line45", "/bar", 42, 0},
+       {"\n//line ./foo:42\n  line46", "./foo", 42, 0},
+       {"\n//line a/b/c/File1.go:100\n  line100", "a/b/c/File1.go", 100, 0},
+       {"\n//line c:\\bar:42\n  line200", "c:\\bar", 42, 0},
+       {"\n//line c:\\dir\\File1.go:100\n  line201", "c:\\dir\\File1.go", 100, 0},
 
        // tests for new line directive syntax
        {"\n//line :100\na1", "", 100, 0}, // missing filename means empty filename
-       {"\n//line bar:100\nb1", filepath.Join("dir", "bar"), 100, 0},
-       {"\n//line :100:10\nc1", filepath.Join("dir", "bar"), 100, 10}, // missing filename means current filename
-       {"\n//line foo:100:10\nd1", filepath.Join("dir", "foo"), 100, 10},
+       {"\n//line bar:100\nb1", "bar", 100, 0},
+       {"\n//line :100:10\nc1", "bar", 100, 10}, // missing filename means current filename
+       {"\n//line foo:100:10\nd1", "foo", 100, 10},
 
        {"\n/*line :100*/a2", "", 100, 0}, // missing filename means empty filename
-       {"\n/*line bar:100*/b2", filepath.Join("dir", "bar"), 100, 0},
-       {"\n/*line :100:10*/c2", filepath.Join("dir", "bar"), 100, 10}, // missing filename means current filename
-       {"\n/*line foo:100:10*/d2", filepath.Join("dir", "foo"), 100, 10},
-       {"\n/*line foo:100:10*/    e2", filepath.Join("dir", "foo"), 100, 14}, // line-directive relative column
-       {"\n/*line foo:100:10*/\n\nf2", filepath.Join("dir", "foo"), 102, 1},  // absolute column since on new line
-}
-
-var unixsegments = []segment{
-       {"\n//line /bar:42\n  line42", "/bar", 42, 0},
-}
-
-var winsegments = []segment{
-       {"\n//line c:\\bar:42\n  line42", "c:\\bar", 42, 0},
-       {"\n//line c:\\dir\\File1.go:100\n  line100", "c:\\dir\\File1.go", 100, 0},
+       {"\n/*line bar:100*/b2", "bar", 100, 0},
+       {"\n/*line :100:10*/c2", "bar", 100, 10}, // missing filename means current filename
+       {"\n/*line foo:100:10*/d2", "foo", 100, 10},
+       {"\n/*line foo:100:10*/    e2", "foo", 100, 14}, // line-directive relative column
+       {"\n/*line foo:100:10*/\n\nf2", "foo", 102, 1},  // absolute column since on new line
 }
 
 // Verify that line directives are interpreted correctly.
 func TestLineDirectives(t *testing.T) {
-       segs := segments
-       if runtime.GOOS == "windows" {
-               segs = append(segs, winsegments...)
-       } else {
-               segs = append(segs, unixsegments...)
-       }
-
        // make source
        var src string
-       for _, e := range segs {
+       for _, e := range segments {
                src += e.srcline
        }
 
        // verify scan
        var S Scanner
-       file := fset.AddFile(filepath.Join("dir", "TestLineDirectives"), fset.Base(), len(src))
+       file := fset.AddFile("TestLineDirectives", fset.Base(), len(src))
        S.Init(file, []byte(src), func(pos token.Position, msg string) { t.Error(Error{pos, msg}) }, dontInsertSemis)
-       for _, s := range segs {
+       for _, s := range segments {
                p, _, lit := S.Scan()
                pos := file.Position(p)
                checkPos(t, lit, p, token.Position{
@@ -578,7 +563,46 @@ func TestLineDirectives(t *testing.T) {
        }
 
        if S.ErrorCount != 0 {
-               t.Errorf("found %d errors", S.ErrorCount)
+               t.Errorf("got %d errors", S.ErrorCount)
+       }
+}
+
+// The filename is used for the error message in these test cases.
+// The first line directive is valid and used to control the expected error line.
+var invalidSegments = []segment{
+       {"\n//line :1:1\n//line foo:42 extra text\ndummy", "invalid line number: 42 extra text", 1, 12},
+       {"\n//line :2:1\n//line foobar:\ndummy", "invalid line number: ", 2, 15},
+       {"\n//line :5:1\n//line :0\ndummy", "invalid line number: 0", 5, 9},
+       {"\n//line :10:1\n//line :1:0\ndummy", "invalid column number: 0", 10, 11},
+       {"\n//line :1:1\n//line :foo:0\ndummy", "invalid line number: 0", 1, 13}, // foo is considered part of the filename
+}
+
+// Verify that invalid line directives get the correct error message.
+func TestInvalidLineDirectives(t *testing.T) {
+       // make source
+       var src string
+       for _, e := range invalidSegments {
+               src += e.srcline
+       }
+
+       // verify scan
+       var S Scanner
+       var s segment // current segment
+       file := fset.AddFile(filepath.Join("dir", "TestInvalidLineDirectives"), fset.Base(), len(src))
+       S.Init(file, []byte(src), func(pos token.Position, msg string) {
+               if msg != s.filename {
+                       t.Errorf("got error %q; want %q", msg, s.filename)
+               }
+               if pos.Line != s.line || pos.Column != s.column {
+                       t.Errorf("got position %d:%d; want %d:%d", pos.Line, pos.Column, s.line, s.column)
+               }
+       }, dontInsertSemis)
+       for _, s = range invalidSegments {
+               S.Scan()
+       }
+
+       if S.ErrorCount != len(invalidSegments) {
+               t.Errorf("go %d errors; want %d", S.ErrorCount, len(invalidSegments))
        }
 }