bazel-contrib / bazel-contrib/rules_go

nogo nolint comments don't work across dependencies when using nilaway

Open
#3,774 10 comments 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.43.0

### What version of gazelle are you using?

v0.34.0

### What version of Bazel are you using?

6.4.0

### Does this issue reproduce with the latest releases of all the above?

Yes

### What operating system and processor architecture are you using?

macOS, arm64

### What did you do?

Repro in https://github.com/illicitonion/repro-bazel-rules_go-nogo-nilaway

Note: I'm not confident that rules_go is the place at fault here, there seem to be a few interacting pieces.

[nilaway](https://github.com/uber-go/nilaway) is a static analyser compatible with nogo.

https://github.com/bazelbuild/rules_go/pull/3562 added the ability to ignore diagnostics using comments.

Because nilaway performs call-chain analysis, it may detect diagnostics which "belong" across multiple files, e.g.
```
my-lib/panicboi.go:9:1: error: Potential nil panic detected. Observed nil flow from source to dereference point:
-> my-lib/panicboi.go:14:12: literal `nil` returned from `MakeANilPanicStruct()` in position 0
-> __main__/main.go:12:2: result 0 of `MakeANilPanicStruct()` used as receiver to call `OhnoIMayPanic()`
-> my-lib/panicboi.go:9:10: read by method receiver `p` accessed field `anField`
```

In this case, the lint is _actually_ raised when linting `main.go`, but the `Position` of the diagnostic is considered to be in `my-lib/panicboi.go`.

I tried to ignore this diagnostic by adding comments to both `main.go` and `panicboi.go` expecting at least one of them to work, but was unable to silence the diagnostic.

It appears impossible to ignore these diagnostics using comments. Ignore comments in `panicboi.go` aren't propagated across dependency edges, and there's no way to identify when reporting the diagnostic that it touched some code in `main.go` that had a comment associated.

I'm not sure how we should be addressing this; it feels like some combination of:
1. We should propagate nolint comments across dependency edges so that nogo would see there were comments in panicboi.go
2. nilaway could propagate the `Related` field on its `Diagnostic` showing each file in the flow path, which may allow us to at least filter based on `main.go:12` being in the report.

But I don't really have enough context to suggest fixes here.

Contributor guide

Open the contributing guide

Research direction

Start with the linked reproduction repository and compare the behavior of main.go and my-lib/panicboi.go under nogo and nilaway. Read rules_go PR 3562 and inspect how nolint comments, dependency edges, and Diagnostic positions or Related entries are handled. Done means the cross-file diagnostic can be intentionally silenced or the supported limitation is documented with a regression test.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
build-system, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.