bazel-contrib / bazel-contrib/rules_go
Nogo: Incorrect position information for upstream packages when analyzing downstream packages
- Dominant language
- Go
- Stars
- 1.5k
- Forks
- 762
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 12
Description
### What version of rules_go are you using?
v0.38.1
### What version of gazelle are you using?
N/A
### What version of Bazel are you using?
6.1.1-homebrew
### Does this issue reproduce with the latest releases of all the above?
Yes
### What operating system and processor architecture are you using?
macOS 13.2.1, M1 Max
### Any other potentially useful information about your toolchain?
N/A
### What did you do?
When writing a nogo analyzer, we simply rely on `analysis.Pass` for our analyses. We do not care how the information in it is collected. However, I'm noticing incorrect information from it for running bazel + nogo (standalone runs do not have this issue).
Say we have the following two packages `upstream` and `downstream`:
example/upstream/main.go
```go
package upstream
var GlobalValue int = 1
```
example/upstream/main2.go
```go
package upstream
func Exported() {
var i int
print(i)
}
```
example/downstream/main.go
```go
package downstream
import "example/upstream"
func main() {
// In downstream package, we use the upstream's global values.
print(upstream.GlobalValue)
upstream.Exported()
}
```
WORKSPACE
```bazel
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
name = "io_bazel_rules_go",
sha256 = "dd926a88a564a9246713a9c00b35315f54cbd46b31a26d5d8fb264c07045f05d",
urls = [
"https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.38.1/rules_go-v0.38.1.zip",
"https://github.com/bazelbuild/rules_go/releases/download/v0.38.1/rules_go-v0.38.1.zip",
],
)
load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
go_rules_dependencies()
go_register_toolchains(nogo = "@//:my_nogo", version = "1.19.5")
```
Now, we run bazel build `//example/...` with our nogo analyzer. Bazel understands the dependencies and will first build `upstream` package, and then `downstream` package, invoking our nogo analyzer twice.
When analyzing the `downstream` package, `pass.Fset` will contain all three files (due to dependencies), `upstream/main.go`, `upstream/main2.go`, and `downstream/main.go`.
However, it seems that the upstream files are incorrectly parsed, their file sizes are somehow always `65536`. Moreover, resolving upstream's `types.Object.Pos()` against downstream's `pass.Fset` resulted in incorrect column information.
As a MWE, I have created a toy nogo analyzer that only logs the information to `/tmp/analyzer_log` file.
main.go
```go
package analyzer
import (
"fmt"
"go/ast"
"go/token"
"go/types"
"log"
"os"
"strings"
"golang.org/x/tools/go/analysis"
)
var Analyzer = &analysis.Analyzer{
Name: "testanalyzer",
Doc: "MWE to show case bazel + nogo token.Pos issue",
Run: run,
}
func WriteToLog(s string) {
f, err := os.OpenFile("/tmp/analyzer_log", os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
if err != nil {
log.Fatalf("error opening file: %v", err)
}
defer f.Close()
log.SetOutput(f)
log.Println(s)
}
func run(pass *analysis.Pass) (interface{}, error) {
WriteToLog(fmt.Sprintf("analyzing %q, %q", pass.Pkg.Path(), pass.Pkg.Name()))
WriteToLog("the file set contains: ")
pass.Fset.Iterate(func(file *token.File) bool {
WriteToLog(fmt.Sprintf("{name: %q, base: %d, size: %d}", file.Name(), file.Base(), file.Size()))
return true
})
var items []string
for i, m := range [...]map[*ast.Ident]types.Object{pass.TypesInfo.Defs, pass.TypesInfo.Uses} {
for _, obj := range m {
if obj == nil {
continue
}
var buf strings.Builder
if i == 0 {
fmt.Fprintf(&buf, "Defs: ")
} else {
fmt.Fprintf(&buf, "Uses: ")
}
fmt.Fprintf(&buf, "| pos: %d ", obj.Pos())
position := pass.Fset.Position(obj.Pos())
fmt.Fprintf(&buf, "| Line:Col %2d:%2d ", position.Line, position.Column)
fmt.Fprintf(&buf, "| File Name: %q ", position.Filename)
if obj.Pkg() != nil {
fmt.Fprintf(&buf, "| Pkg Name: %q ", obj.Pkg().Name())
}
fmt.Fprintf(&buf, "| name: %q ", obj.Name())
items = append(items, buf.String())
}
}
WriteToLog(strings.Join(items, "\n"))
return nil, nil
}
```
and properly set up nogo per [the instructions](https://github.com/bazelbuild/rules_go/blob/master/go/nogo.rst).
When running `bazel clean && bazel build //example/...`, I observed the following logs (simplified and prettified for illustrations):
/tmp/analyzer_log
```
nogo: analyzing "example/upstream", "upstream"
nogo: the file set contains:
nogo: {name: "/__main__/example/upstream/main.go", base: 1, size: 81}
nogo: {name: "/__main__/example/upstream/main2.go", base: 83, size: 59}
nogo: Defs: | pos: 106 | Line:Col 3: 6 | File Name: "/__main__/example/upstream/main2.go" | Pkg Name: "upstream" | name: "Exported"
Defs: | pos: 124 | Line:Col 4: 6 | File Name: "/__main__/example/upstream/main2.go" | Pkg Name: "upstream" | name: "i"
Defs: | pos: 62 | Line:Col 4: 5 | File Name: "/__main__/example/upstream/main.go" | Pkg Name: "upstream" | name: "GlobalValue"
Uses: | pos: 124 | Line:Col 4: 6 | File Name: "/__main__/example/upstream/main2.go" | Pkg Name: "upstream" | name: "i"
nogo: analyzing "example/downstream", "downstream"
nogo: the file set contains:
nogo: {name: "/__main__/example/downstream/main.go", base: 1, size: 177}
nogo: {name: "example/upstream/main2.go", base: 179, size: 65536}
nogo: {name: "example/upstream/main.go", base: 65716, size: 65536}
nogo: Defs: | pos: 53 | Line:Col 5: 6 | File Name: "/__main__/example/downstream/main.go" | Pkg Name: "downstream" | name: "main"
Uses: | pos: 28 | Line:Col 3: 8 | File Name: "/__main__/example/downstream/main.go" | Pkg Name: "downstream" | name: "upstream"
Uses: | pos: 65719 | Line:Col 4: 1 | File Name: "example/upstream/main.go" | Pkg Name: "upstream" | name: "GlobalValue"
Uses: | pos: 28 | Line:Col 3: 8 | File Name: "/__main__/example/downstream/main.go" | Pkg Name: "downstream" | name: "upstream"
Uses: | pos: 181 | Line:Col 3: 1 | File Name: "example/upstream/main2.go" | Pkg Name: "upstream" | name: "Exported"
```
We can see from the log when analyzing downstream packages:
1. The files in `pass.Fset` for upstream packages are incorrect:
```
nogo: {name: "example/upstream/main2.go", base: 179, size: 65536}
nogo: {name: "example/upstream/main.go", base: 65716, size: 65536}
```
Their sizes both appear to be 65536, where they should be 81 and 59 respectively, as observed when analyzing the upstream packages:
```
nogo: {name: "/__main__/example/upstream/main.go", base: 1, size: 81}
nogo: {name: "/__main__/example/upstream/main2.go", base: 83, size: 59}
```
2. When resolving upstream's `types.Object.Pos()` against downstream's `pass.Fset`, we were seeing correct line number, but incorrect column number (observed in downstream vs observed in upstream):
```
Defs: | pos: 62 | Line:Col 4: 5 | File Name: "/__main__/example/upstream/main.go" | Pkg Name: "upstream" | name: "GlobalValue"
vs
Uses: | pos: 65719 | Line:Col 4: 1 | File Name: "example/upstream/main.go" | Pkg Name: "upstream" | name: "GlobalValue"
```
```
nogo: Defs: | pos: 106 | Line:Col 3: 6 | File Name: "/__main__/example/upstream/main2.go" | Pkg Name: "upstream" | name: "Exported"
vs
Uses: | pos: 181 | Line:Col 3: 1 | File Name: "example/upstream/main2.go" | Pkg Name: "upstream" | name: "Exported"
```
------
Building and running analyzer standalone (i.e., via `https://pkg.go.dev/golang.org/x/tools/go/analysis/singlechecker`) does not have this issue (information is all correct). So this seems like a package loader problem since IIUC bazel nogo is using a [custom package loader](https://github.com/bazelbuild/rules_go/blob/47b5a3bbd337e862a868ffe1739f0546c6fb12bc/go/tools/builders/nogo_main.go#L317) instead of the standard `packages.Load` loader. This could indicate some subtle bug in the custom loader.
Or since I assume we are [loading upstream package's information from archive](https://github.com/bazelbuild/rules_go/blob/47b5a3bbd337e862a868ffe1739f0546c6fb12bc/go/tools/builders/nogo_main.go#L523), it might be a archive reading/writing bug?
I have attached the MWE above as a zipfile to this issue. I appreciate any help / directions to further debug / solve this problem 😃
[2023-03-17-bazel-nogo-multi-pkg-issue.zip](https://github.com/bazelbuild/rules_go/files/11007309/2023-03-17-bazel-nogo-multi-pkg-issue.zip)
### What did you expect to see?
upstream's `Fset` information should be correct when analyzing downstream packages.
### What did you see instead?
upstream's `Fset` information and object's `token.Pos` is completely incorrect when analyzing downstream packages.
Contributor guide
Research direction
Start with the attached minimal reproducer and run `bazel clean && bazel build //example/...` using the custom nogo analyzer. Read `go/tools/builders/nogo_main.go` around the custom loader and archive-loading code at the referenced lines 317 and 523, then compare upstream and downstream `analysis.Pass.Fset` sizes and positions. Done means upstream file sizes and `types.Object.Pos()` resolve to correct filenames, lines, and columns in downstream analysis.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- build-system
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100