Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

golangci-lint does not allow differentiated comments for nolint directives #2745

Open
4 tasks done
kortschak opened this issue Apr 5, 2022 · 3 comments
Open
4 tasks done
Labels
area: nolint Related to nolint directive and nolintlint enhancement New feature or improvement

Comments

@kortschak
Copy link
Contributor

kortschak commented Apr 5, 2022

Welcome

  • Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).
  • Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

If there are two issues that require //nolint directives that have different reasons, it is not possible to disctinguish the reason by having them on separate lines. This should work as directives are supposed to attach to the next token in the source.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version v1.45.2 built from (unknown, mod sum: "h1:9I3PzkvscJkFAQpTQi5Ga0V4qWdJERajX1UZ7QqkW+I=") on (unknown)

Configuration file

$ cat .golangci.yml
linters:
  disable-all: true
  enable:
    - deadcode
    - godox

Go environment

$ go version && go env
go version go1.18 linux/amd64
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/user/bin"
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/user/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/user/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/user/src/github.com/golangci/golangci-lint/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3827020002=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
main.go:5:5: `zombie` is unused (deadcode)
var zombie = "words" // TODO: Stuff explaining why this is important to work on later.
    ^

or

$ golangci-lint cache clean
$ golangci-lint run -v
main.go:5: main.go:5: Line contains TODO/BUG/FIXME: "TODO: Stuff explaining why this is impor..." (godox)
var zombie = "words" // TODO: Stuff explaining why this is important to work on later.

Depending on the ordering of the lines.

Code example or link to a public repository

package main

//nolint:deadcode,unused // Reasons why this is important to keep here.
//nolint:godox // Reasons why it's silly to complain about TODO notes.
var zombie = "words" // TODO: Stuff explaining why this is important to work on later.

func main() {}

or

package main

//nolint:godox // Reasons why it's silly to complain about TODO notes.
//nolint:deadcode,unused // Reasons why this is important to keep here.
var zombie = "words" // TODO: Stuff explaining why this is important to work on later.

func main() {}
@kortschak kortschak added the bug Something isn't working label Apr 5, 2022
@ldez ldez added enhancement New feature or improvement and removed bug Something isn't working labels Apr 5, 2022
@ldez
Copy link
Member

ldez commented Apr 5, 2022

Hello,

Currently, the behavior doesn't depend on comments: golangci-lint supports only 1 nolint directive on a section.

As this is the current global behavior, it's difficult to say that is a bug, so I put the label 'enhancement'.

Otherwise, even if I guess that is just an example, I recommend defining a different configuration for godox instead of adding nolint directives for TODOs, ex:

linters-settings:
  godox:
    keywords:
      - FIXME

https://golangci-lint.run/usage/linters/#godox

@kortschak
Copy link
Contributor Author

This is not an enhancement, it is a failure to follow the standard directive comment behaviour. Please see https://datatracker.ietf.org/doc/html/rfc9225 for why this is a broken approach to managing issues.

@kortschak kortschak added bug Something isn't working and removed enhancement New feature or improvement labels Apr 5, 2022
@ldez
Copy link
Member

ldez commented Apr 6, 2022

Currently, we don't support multiple nolint directives, so we cannot have bugs on something that we don't support.

it is a failure to follow the standard directive comment behaviour.

For now, we don't follow the standard directive style and you already know that #1658

So yes we are not on track with the recommendations.

this is a broken approach to managing issues

The labels bugs or enhancement are just a way to define a kind of priority.

@ldez ldez added enhancement New feature or improvement and removed bug Something isn't working labels Apr 6, 2022
@ldez ldez added the area: nolint Related to nolint directive and nolintlint label Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: nolint Related to nolint directive and nolintlint enhancement New feature or improvement
Projects
None yet
Development

No branches or pull requests

2 participants