Skip to content

Commit

Permalink
go/scanner: report errors for incorrect line directives
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@griesemer
griesemer committed Mar 15, 2018
1 parent 9d42153 commit 546bab8
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 75 deletions.
5 changes: 2 additions & 3 deletions src/go/printer/testdata/comments.golden
View file
Expand Up @@ -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
}

Expand Down
5 changes: 2 additions & 3 deletions src/go/printer/testdata/comments.input
View file
Expand Up @@ -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
}

Expand Down
30 changes: 5 additions & 25 deletions src/go/scanner/scanner.go
View file
Expand Up @@ -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 "*/"
Expand All @@ -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
}

Expand All @@ -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"
Expand All @@ -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
}

Expand Down
112 changes: 68 additions & 44 deletions src/go/scanner/scanner_test.go
View file
Expand Up @@ -9,7 +9,6 @@ import (
"io/ioutil"
"os"
"path/filepath"
"runtime"
"testing"
)

Expand Down Expand Up @@ -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{
Expand All @@ -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))
}
}

Expand Down

0 comments on commit 546bab8

Please sign in to comment.