Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/parser: emits invalid ast.ImportSpec when missing end quote of import path #6748

Closed
gopherbot opened this issue Nov 11, 2013 · 8 comments
Closed

Comments

@gopherbot
Copy link

by jessevdk:

I've been using ast/parser and friends to parse some go sources. I've come across a nil
dereference panic when processing a file containing errors. Specifically, I've tried to
parse something like the following:

import (
    "fmt
)

var _ = fmt.Println

func main() {
}

The problem which occurs is that the resulting AST contains an ast.ImportSpec node with
a Path set to nil. This is ok, but s.Pos() will try to dereference the nil pointer when
called. It would be nice if the ImportSpec node would still be able to report a valid
Pos.

This also affects go.tools/types which does not check for Path being nil.
@griesemer
Copy link
Contributor

Comment 1:

Owner changed to @griesemer.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 2:

Labels changed: added go1.3maybe.

@dsymonds
Copy link
Contributor

dsymonds commented Dec 2, 2013

Comment 3:

Labels changed: added priority-later, packagebug, removed priority-triage.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 4:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 5:

Labels changed: added repo-main.

@griesemer
Copy link
Contributor

Comment 6:

I cannot reproduce an ast.ImportSpec with nil Path with your example. Specifically, for:
$ cat x.go
package main
import (
    "fmt
)
var _ = fmt.Println
func main() {
}
I get a "correctly" looking AST given the error:
$ gotype -ast x.go
     0  *ast.File {
     1  .  Package: x.go:1:1
     2  .  Name: *ast.Ident {
     3  .  .  NamePos: x.go:1:9
     4  .  .  Name: "main"
     5  .  }
     6  .  Decls: []ast.Decl (len = 1) {
     7  .  .  0: *ast.GenDecl {
     8  .  .  .  TokPos: x.go:3:1
     9  .  .  .  Tok: import
    10  .  .  .  Lparen: x.go:3:8
    11  .  .  .  Specs: []ast.Spec (len = 2) {
    12  .  .  .  .  0: *ast.ImportSpec {
    13  .  .  .  .  .  Path: *ast.BasicLit {
    14  .  .  .  .  .  .  ValuePos: x.go:4:5
    15  .  .  .  .  .  .  Kind: STRING
    16  .  .  .  .  .  .  Value: "\"fmt\n)"
    17  .  .  .  .  .  }
    18  .  .  .  .  .  EndPos: -
    19  .  .  .  .  }
    20  .  .  .  .  1: *ast.ImportSpec {
    21  .  .  .  .  .  Path: *ast.BasicLit {
    22  .  .  .  .  .  .  ValuePos: x.go:7:1
    23  .  .  .  .  .  .  Kind: STRING
    24  .  .  .  .  .  .  Value: ""
    25  .  .  .  .  .  }
    26  .  .  .  .  .  EndPos: -
    27  .  .  .  .  }
    28  .  .  .  }
    29  .  .  .  Rparen: x.go:10:3
    30  .  .  }
    31  .  }
    32  .  Scope: *ast.Scope {
    33  .  .  Objects: map[string]*ast.Object (len = 0) {}
    34  .  }
    35  .  Imports: []*ast.ImportSpec (len = 2) {
    36  .  .  0: *(obj @ 12)
    37  .  .  1: *(obj @ 20)
    38  .  }
    39  }
x.go:4:5: string not terminated
x.go:7:1: expected 'STRING', found 'var'
x.go:10:3: expected ')', found 'EOF'
The AST does contain 2 imports specs (line 12, line 20) due to the error which leads to
a synchronization problem. But both of them have a non-nil Path (line 13, line 21, with
position information).
Inspecting the parser source (go/parser/parser.go:2136 ff) shows that the only place in
the entire parser where an ast.ImportSpec node is created is on line 2164, and that
ImportSpec node always has a non-nil ast.BasicLit (line 2167); i.e., the ImportSpec.Path
cannot be nil by construction (if constructed with the parser). Finally, the
parseImportSpec function always returns a non-nil ImportSpec, equally by construction.
If you can reproduce this, please provide the complete code to reproduce. The error may
be elsewhere.
Related to this (but not causing a problem here) is issue #7091.

Status changed to WaitingForReply.

@gopherbot
Copy link
Author

Comment 7 by jessevdk:

Looks like this was fixed in 5ae672cfe7f4

@griesemer
Copy link
Contributor

Comment 8:

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants