I was tired of seeing this warning when running gazelle:
gazelle: finding module path for import github.com/go-critic/go-critic/framework/linter: go: downloading github.com/go-critic/go-critic v0.11.4
go: module github.com/go-critic/go-critic@upgrade found (v0.11.4), but does not contain package github.com/go-critic/go-critic/framework/linter
This updated go-critic to the latest version which makes the warning go
away since it now uses the new package path.
Test Plan: gazelle is happy and CI is happy
For certain types, we do not want the zero-value initialization for structs.
This means we need to trade off readability vs exhaustive initialization
checking, as the Go syntax `Foo{Bar: bar}` is more readable, but doesn't do
exhaustiveness checking, and `Foo{bar}` does check for exhaustiveness but can be less
readable depending on context.
For now, the check is only introduced for one type, and is meant to be
opt-in so that code authors may choose for stricter checking.
Go 1.22 changes loop variables to have more sensible
semantics where the variable is not reused across iterations
by the codegen, so we can simplify a bunch of code
working around that counterintuitive behavior.
This removes the linter that checks for incorrectly formatted
`go:generate` directives from `sg lint` and replace it with a proper
linter handled by `nogo`.
## Test plan
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
Locally tested + CI
Adds https://github.com/mvdan/unparam as a nogo linter
Without `//nolint:unparam`
```
bb //dev/linters/...
INFO: Analyzed 136 targets (0 packages loaded, 0 targets configured).
INFO: Found 136 targets...
ERROR: /Users/william/code/sourcegraph/dev/linters/unparam/BUILD.bazel:3:11: GoCompilePkg dev/linters/unparam/unparam.a failed: (Exit 1): builder failed: error executing command (from target //dev/linters/unparam:unparam) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/go_sdk/builder_reset/builder compilepkg -sdk external/go_sdk -installsuffix darwin_arm64 -src dev/linters/unparam/unparam.go -embedroot '' ... (remaining 30 arguments skipped)
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
compilepkg: nogo: errors found by nogo during build-time code analysis:
dev/linters/unparam/unparam.go:27:21: Test - b is unused (unparam)
INFO: Elapsed time: 0.374s, Critical Path: 0.17s
INFO: 3 processes: 2 internal, 1 darwin-sandbox.
FAILED: Build did NOT complete successfully
```
With `//nolint:unparam`
```
bb //dev/linters/...
INFO: Analyzed 136 targets (0 packages loaded, 0 targets configured).
INFO: Found 136 targets...
INFO: Elapsed time: 0.261s, Critical Path: 0.11s
INFO: 3 processes: 1 internal, 2 darwin-sandbox.
INFO: Build completed successfully, 3 total actions
```
## Test plan
* Checked that a small function in the linter is picked up when the
`//nolint:unparam` directive is removed
* Greem CI
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
We cannot use the staticcheck analyzer directly, since it's actually a
collection of analyzers. So we have to pull out each "micro" analyzer
and create a seperate bazel target with it.
Each bazel target actually just uses `staticcheck.go` and embeds the
analyzer it SHOULD use when invoked as that target. Ex:
```
bazel build //dev/linters/staticcheck:SA6001
```
The above target will do the following:
* Set `AnalyzerName` to `SA6001` in `staticcheck.go` uses x_def
* Set the importpath to
`github.com/sourcegraph/sourcegraph/dev/llinters/staticcheck/SA6001`.
**This is very important, otherwise there won't be different libraries
with different analyzer names, just one library with one analyzer set
invoked by different targets**.
## Test plan
bazel build //cmd/frontend/...
green ci
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
---------
Co-authored-by: Jean-Hadrien Chabran <jh@chabran.fr>
Add the ineffassign linter and let it get executed as part of build
anallysis.
### Linter fixes
Made a few fixes the linter found.
## Test plan
`bazel build //cmd/...`
`bazel build //internal/...`
`bazel build //enterprise/...`
`bazel build //dev/...`
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
Adds forbidigo as a linter than can be executed by nogo.
### What are these other BUILD files for?
The build didn't work without them so had to add them. They were auto
generated by `bazel run //:gazelle`
### Example of errors reported by this linter
```
external/com_github_prometheus_common/model/time.go:164:10: use of `fmt.Errorf` forbidden by pattern `^fmt\.Errorf$` (forbidigo)
external/com_github_prometheus_common/model/time.go:196:13: use of `fmt.Errorf` forbidden by pattern `^fmt\.Errorf$` (forbidigo)
external/com_github_prometheus_common/model/time.go:200:13: use of `fmt.Errorf` forbidden by pattern `^fmt\.Errorf$` (forbidigo)
external/com_github_prometheus_common/model/value.go:53:10: use of `fmt.Errorf` forbidden by pattern `^fmt\.Errorf$` (forbidigo)
external/com_github_prometheus_common/model/value.go:279:10: use of `fmt.Errorf` forbidden by pattern `^fmt\.Errorf$` (forbidigo)
external/com_github_prometheus_common/model/value.go:327:10: use of `fmt.Errorf` forbidden by pattern `^fmt\.Errorf$` (forbidigo)
[609 / 1,838] 10 actions running
GoCompilePkg external/com_github_yuin_goldmark_emoji/definition/definition.a; 3s darwin-sandbox
GoCompilePkg external/com_github_go_enry_go_enry_v2/data/data.a; 3s darwin-sandbox
ERROR: build interrupted
INFO: Elapsed time: 10.957s, Critical Path: 4.42s
INFO: 403 processes: 101 internal, 302 darwin-sandbox.
FAILED: Build did NOT complete successfully
```
### Why do we have our own runAnalysis
Annoyingly, the `analyzer.NewAnalyzer` from forbidigo does not allow you
to configure it in anyway, and we want to add our patterns into it. So
`runAnalysis` was copied from `forbidigo` and slightly modified so that
we can specify patterns
## Test plan
`bazel build //cmd/frontend/...`
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
This adds nogo to lint our code from within bazel.
## Test plan
`bazel build //cmd/frontend`
need to do a full build still
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
---------
Co-authored-by: Jean-Hadrien Chabran <jh@chabran.fr>
