Go-tools: cmd/staticcheck: hides error from `go list`

[ainar-g]

Not sure what's supposed to happen here. It also might be an upstream issue. Repro script:

#!/usr/bin/env sh

set -e -x

go version
staticcheck --version
cd "${TMPDIR:-/tmp}"
rm -rf ./go
mkdir ./go ./go/a ./go/b
echo 'module example.com/a' > ./go/a/go.mod
echo 'module example.com/b' > ./go/b/go.mod
printf 'package main\n\ntype NoDocs struct{}\n\nfunc main() {}'\
    > ./go/a/main.go
cd "${TMPDIR:-/tmp}/go/b/"
staticcheck --checks '*' -f 'stylish' ../a/... || true
cd "${TMPDIR:-/tmp}/go/a/"
staticcheck --checks '*' -f 'stylish' ./... || true
cd "${TMPDIR:-/tmp}"
rm -rf ./go

(Filtered) output on my machine:

+ go version
go version go1.13.1 linux/amd64
+ staticcheck --version
staticcheck (devel, v0.0.0-20191003111933-b8911aca9f34)
(…)
+ cd /tmp/go/b/
+ staticcheck --checks * -f stylish ../a/...
 ✖ 0 problems (0 errors, 0 warnings)
+ cd /tmp/go/a/
+ staticcheck --checks * -f stylish ./...
/tmp/go/a/main.go
  (3, 6)  U1000  type NoDocs is unused

 ✖ 1 problems (1 errors, 0 warnings)
+ true
(…)

staticcheck seems to miss the errors in the module exmaple.com/a if it's not called from its directory. It doesn't print any errors regarding relative paths either.

But if I comment out this line:

echo 'module example.com/b' > ./go/b/go.mod

Then staticcheck works both from ./go/a and ./go/b.

[dominikh]

Instead of staticcheck, use go list, and see what happens. That is, check what packages go list ../a/... prints. I reckon this is just related to how modules behave.

[ainar-g]

Filtered output with go list:

(…)
+ cd /tmp/go/b/
+ go list ../a/...
go: pattern ../a/... refers to dir /tmp/go/a, outside module root /tmp/go/b
+ true
+ cd /tmp/go/a/
+ go list ./...
example.com/a
(…)

What does that mean for staticcheck? I expect staticcheck ../foo/... to either work the same way as (cd ../foo && staticcheck ./...) or print an error telling me that relative paths crossing module borders are not supported, à la go list.

[dominikh]

I'll investigate why the error isn't being propagated back to us (something something go/packages most likely)

As for how staticcheck _should_ behave: the arguments should be treated identically to how the underlying build system (such as go build or Bazel) treats them. We largely don't have to do anything to achieve that: we pass the arguments to go/packages, which passes them mostly unmodified to the build system. Clearly something went wrong along the way here.

[dominikh]

This is now https://go-review.googlesource.com/c/tools/+/202437