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

cmd/go: relative position filenames in errors are inconsistent with tools like gofmt and vet #24344

Open
mvdan opened this Issue Mar 10, 2018 · 7 comments

Comments

Projects
None yet
4 participants
@mvdan
Member

mvdan commented Mar 10, 2018

Given a single file with a trivial syntax error in the current directory:

$ cat f.go
package p

func
$ go build f.go
# command-line-arguments
./f.go:4:1: syntax error: unexpected EOF, expecting name or (
$ gofmt f.go
f.go:3:6: expected 'IDENT', found 'EOF'

The compiler likes to use the ./ prefix, while the older packages like go/parser don't. Is there a good reason for this inconsistency? If not, I imagine that they should be consistent.

Labelling as NeedsDecision as it's not immediately clear to me if there should be a fix, and what that fix would look like.

/cc @griesemer @mdempsky

@mvdan mvdan added the NeedsDecision label Mar 10, 2018

@mvdan

This comment has been minimized.

Member

mvdan commented Mar 10, 2018

Also, the ./ prefix seems to happen both inside and outside of GOPATH, and independently of whether I'm building files or packages.

@griesemer

This comment has been minimized.

Contributor

griesemer commented Mar 12, 2018

go tool compile f.go produces f.go:3:5: syntax error: unexpected EOF, expecting name or ( for me.
go build f.go or go.build produces ./f.go:3:5: syntax error: unexpected EOF, expecting name or (. This is for go version devel +99c30211b1 Fri Mar 9 23:11:59 2018 +0000 darwin/amd64 .

Finally, if there are line directives, the result changes again. For:

package p

//line foo:1
func

(with EOF after func), go tool compile f.go produces foo:1: syntax error: unexpected EOF, expecting name or (; go build f.go produces the same, and gofmt f.go produces foo:1: expected 'IDENT', found 'EOF'.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Mar 12, 2018

The file name is coming from cmd/go, not cmd/compile.

I think we need a clearer example of where an inconsistency happens. I don't think it is a problem if go/parser and cmd/go wind up with different file names, since they are being invoked differently.

@mvdan mvdan changed the title from cmd/compile: relative position filenames in errors are inconsistent with go/parser to cmd/go: relative position filenames in errors are inconsistent with go/parser Mar 12, 2018

@mvdan

This comment has been minimized.

Member

mvdan commented Mar 12, 2018

@ianlancetaylor note that my point is about the inconsistency between go/parser based tools and the main cmd/... tools. For example, now that go test runs vet by default, an error with a filename might contain foo.go or ./foo.go, depending on whether the error comes from vet or from the compiler.

@mvdan mvdan changed the title from cmd/go: relative position filenames in errors are inconsistent with go/parser to cmd/go: relative position filenames in errors are inconsistent with tools like gofmt and vet Mar 12, 2018

@griesemer

This comment has been minimized.

Contributor

griesemer commented Mar 12, 2018

@mvdan I think what @ianlancetaylor is alluding to is that we need a clearer example of why it matters. The fact that one tool reports ./foo.go and the other one foo.fo doesn't necessarily mean one of them needs to change. Are there situations that cause actual problems because of this?

@mvdan

This comment has been minimized.

Member

mvdan commented Mar 12, 2018

Ah, I see. It hasn't caused me any trouble other than me noticing and being slightly bothered by it :)

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Mar 12, 2018

I actually do like the example of go test referring to the filename in different ways depending on whether it is reporting a compiler error or a vet error. I wasn't clear on that before. I think we should fix at least that case to be consistent, one way or another.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment