search

dominikh / go-tools

Staticcheck - The advanced Go linter
https://staticcheck.dev
MIT License
6.18k stars 378 forks source link

Report errors relative to os.Getwd #29

Closed myitcv closed 7 years ago

myitcv commented 7 years ago 
# version of unused
$ (command cd $(go list -f '{{.Dir}}' honnef.co/go/tools/cmd/unused) && git rev-parse HEAD)
1ba7faeb10037a3afd6711b66c299ffb74f08959

My specific example is unused; output from unused always uses the absolute path of a file. To my eye this is unnecessarily noisy. Might a better pattern here be go (build|install) which outputs errors relative to the current working directory.

e.g. currently

$ pwd
/home/myitcv/mygo/src/github.com/myitcv/g/cmd/gai
$ unused
/home/myitcv/mygo/src/github.com/myitcv/g/cmd/gai/gai.go:27:2: const maxGoType is unused (U1000)
/home/myitcv/mygo/src/github.com/myitcv/g/cmd/gai/gai.go:442:6: func resolvePkgSpec is unused (U1000)

vs the much cleaner:

$ unused
gai.go:27:2: const maxGoType is unused (U1000)
gai.go:442:6: func resolvePkgSpec is unused (U1000)

Thoughts?

On a semi-related note (in so far as using the output from go (build|install) as a "pattern" is concerned), would it also make sense to group the lines of output by package import path using "comment headers" in case more than one package is implied/specified via the command line?

$ go list
github.com/myitcv/g/cmd/gai

$ go install  # current directory implied; single package specified
gai.go:6: syntax error: non-declaration statement outside function body

$ go install ./...  # could match multiple; group by package import path
# github.com/myitcv/g/cmd/gai
gai.go:6: syntax error: non-declaration statement outside function body
dominikh commented 7 years ago

We should probably emit relative paths, yes. I'll evaluate that idea later.

I don't think grouping the output is particularly useful, and it complicates consumption of the output, now requiring parsing out those comments.

go build can have multi-line output, and a lot of visual noise when using the -x flag, so the grouping is somewhat useful; our output is strictly one line per issue, and it's easy enough to group visually by scanning the prefixes.

myitcv commented 7 years ago

We should probably emit relative paths, yes. I'll evaluate that idea later.

Great. I'm happy to submit a PR if that's helpful?

I don't think grouping the output is particularly useful, and it complicates consumption of the output, now requiring parsing out those comments.

This part is, I grant you, less well defined. So totally happy with your conclusion not to do anything.

dominikh commented 7 years ago

Great. I'm happy to submit a PR if that's helpful?

Thanks, but I'll work on that myself.

myitcv commented 7 years ago

👍 thanks @dominikh