Skip to content

Commit

Permalink
spanner/spansql: fix DDL.LeadingComment for prior-line inline comments
Browse files Browse the repository at this point in the history
Previously, LeadingComment would return an inline comment from the
previous line, which is not the intent of this helper. Now it will
ignore inline comments, which have an associated node, and only return a
comment that is the only thing on its line.

Change-Id: If5b9d580429dba1fd855ab2b79106e23cc5bbc45
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/52893
Reviewed-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Knut Olav Løite <koloite@gmail.com>
  • Loading branch information
dsymonds committed Mar 5, 2020
1 parent d458bb3 commit 4095c76
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 9 deletions.
14 changes: 9 additions & 5 deletions spanner/spansql/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,11 @@ func ParseDDL(filename, s string) (*DDL, error) {
// Handle comments.
for _, com := range p.comments {
c := &Comment{
Marker: com.marker,
Start: com.start,
End: com.end,
Text: com.text,
Marker: com.marker,
Isolated: com.isolated,
Start: com.start,
End: com.end,
Text: com.text,
}

// Strip common whitespace prefix and any whitespace suffix.
Expand Down Expand Up @@ -239,6 +240,7 @@ type parser struct {

type comment struct {
marker string // "#" or "--" or "/*"
isolated bool // if it starts on its own line
start, end Position
text []string
}
Expand Down Expand Up @@ -670,6 +672,7 @@ func isSpace(c byte) bool {

// skipSpace skips past any space or comments.
func (p *parser) skipSpace() bool {
initLine := p.line
// If we start capturing a comment in this method,
// this is set to its comment value. Multi-line comments
// are only joined during a single skipSpace invocation.
Expand Down Expand Up @@ -710,7 +713,8 @@ func (p *parser) skipSpace() bool {
if com == nil {
// New comment.
p.comments = append(p.comments, comment{
marker: marker,
marker: marker,
isolated: (p.line != initLine) || p.line == 1,
start: Position{
Line: p.line,
Offset: p.offset + i,
Expand Down
4 changes: 1 addition & 3 deletions spanner/spansql/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func TestParseDDL(t *testing.T) {
Text: []string{"This is another comment."}},
{Marker: "/*", Start: line(4), End: line(5),
Text: []string{" This is a", "\t\t\t\t\t\t * multiline comment."}},
{Marker: "--", Start: line(26), End: line(27),
{Marker: "--", Isolated: true, Start: line(26), End: line(27),
Text: []string{"This table has some commentary", "that spans multiple lines."}},
// These comments shouldn't get combined:
{Marker: "--", Start: line(29), End: line(29), Text: []string{"dummy comment"}},
Expand Down Expand Up @@ -430,13 +430,11 @@ func TestParseDDL(t *testing.T) {
}
// There are no leading comments on the columns of NonScalars,
// even though there's often a comment on the previous line.
/* TODO: This is broken; LeadingComment needs fixing.
for _, cd := range tableByName(t, ddl, "NonScalars").Columns {
if com := ddl.LeadingComment(cd); com != nil {
t.Errorf("Leading comment found for NonScalars.%s: %v", cd.Name, com)
}
}
*/
}

func tableByName(t *testing.T, ddl *DDL, name string) *CreateTable {
Expand Down
7 changes: 6 additions & 1 deletion spanner/spansql/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,8 @@ type DMLStmt interface {

// Comment represents a comment.
type Comment struct {
Marker string // opening marker; one of "#", "--", "/*"
Marker string // Opening marker; one of "#", "--", "/*".
Isolated bool // Whether this comment is on its own line.
// Start and End are the position of the opening and terminating marker.
Start, End Position
Text []string
Expand Down Expand Up @@ -521,6 +522,10 @@ func (ddl *DDL) LeadingComment(n Node) *Comment {
if ci >= len(ddl.Comments) || ddl.Comments[ci].End.Line != lineEnd {
return nil
}
if !ddl.Comments[ci].Isolated {
// This is an inline comment for a previous node.
return nil
}
return ddl.Comments[ci]
}

Expand Down

0 comments on commit 4095c76

Please sign in to comment.