bazel-contrib / bazel-contrib/rules_go

Nogo: Incorrect position information for upstream packages when analyzing downstream packages

Open
#3,483 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.