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

goimports adds leading space in front of nolint #3554

Closed
4 tasks done
rickwang7712 opened this issue Feb 3, 2023 · 3 comments
Closed
4 tasks done

goimports adds leading space in front of nolint #3554

rickwang7712 opened this issue Feb 3, 2023 · 3 comments
Labels
area: nolint Related to nolint directive and nolintlint question Further information is requested

Comments

@rickwang7712
Copy link

rickwang7712 commented Feb 3, 2023

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 (e.g., gocritic, go vet, etc.). (https://golangci-lint.run/usage/linters/)

Description of the problem

My "nolint" comment wasn't written with leading space, but after I ran golangci-lint, I found that goimports appends leading space for me, it conflicts with the new rule described here.

Here is the logs showed by golangci-lint:

INFO Fix issue &result.Issue{FromLinter:"goimports", Text:"File is not `goimports`-ed", Severity:"", SourceLines:[]string{"//nolint: gochecknoglobals, gochecknoinits, gosec // for metrics"}, Replacement:(*result.Replacement)(0xc0031a60d0), Pkg:(*packages.Package)(0xc00051c180), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"internal/lib/ooo/xxx.go", Offset:0, Line:1, Column:0}, HunkPos:1, ExpectNoLint:false, ExpectedNoLintLinter:""} with range {1 1} 
INFO Fix issue &result.Issue{FromLinter:"goimports", Text:"File is not `goimports`-ed", Severity:"", SourceLines:[]string{"//nolint: lll // for testing"}, Replacement:(*result.Replacement)(0xc00bc15310), Pkg:(*packages.Package)(0xc000532d80), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"internal/utils/xxx.go", Offset:0, Line:1, Column:0}, HunkPos:1, ExpectNoLint:false, ExpectedNoLintLinter:""} with range {1 1} 

The git diff result will be like:

-//nolint: lll // for testing
+// nolint: lll // for testing

But If I ran goimports -d internal/utils/xxx.go to the un-fixed original file, there will be no fixing diff. So I guess there is something different on how golangci-lint integrates goimports.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.51.0 built from 6d3f06c5 on 2023-02-02T08:24:52Z

Configuration file

$ cat .golangci.yml
# ref: https://gist.github.com/maratori/47a4d00457a92aa426dbd48a18776322
run:
  timeout: 1m # default 1m
linters-settings:
  errcheck:
    check-type-assertions: true # default false
    ignore: "fmt:.*" # default fmt:.*
  govet:
    enable-all: true
    disable:
      - fieldalignment # too strict
    settings:
      shadow:
        strict: true # default false
  cyclop:
    max-complexity: 30 # the maximal code complexity to report # default 10
    package-average: 10.0 # the maximal average package complexity. If it's higher than 0.0 (float) the check is enabled # default 0.0
  funlen:
    lines: 100 # default 60
    statements: 50 # default 40
  gocognit:
    min-complexity: 20 # minimal code complexity to report, 30 by default (but we recommend 10-20)
  gocritic:
    settings:
      captLocal:
        paramsOnly: false # whether to restrict checker to params only # default true
      elseif:
        skipBalanced: false # whether to skip balanced if-else pairs # default true
      underef:
        skipRecvDeref: false # whether to skip (*x).method() calls where x is a pointer receiver # default true
  gomodguard:
    blocked:
      modules:
        - github.com/golang/protobuf:
            recommendations:
              - google.golang.org/protobuf
            reason: "see https://developers.google.com/protocol-buffers/docs/reference/go/faq#modules"
        - github.com/satori/go.uuid:
            recommendations:
              - github.com/google/uuid
            reason: "satori's package is not maintained"
        - github.com/gofrs/uuid:
            recommendations:
              - github.com/google/uuid
            reason: "see recommendation from dev-infra team: https://confluence.gtforge.com/x/gQI6Aw"
      local_replace_directives: true # default false
  lll:
    line-length: 120 # default 120
  makezero:
    always: false # default false
  maligned:
    suggest-new: true # default false
  misspell:
    locale: US
    ignore-words: 
      - "" # default: ""
  nakedret:
    max-func-lines: 0 # default 30
  nestif:
    min-complexity: 4 # default 5
  nolintlint:
    allow-no-explanation: [funlen, gocognit, lll] # default []
    require-explanation: true # default false
    require-specific: true # default false
  prealloc:
    simple: false # default true
  rowserrcheck:
    packages:
      - database/sql
      - github.com/jmoiron/sqlx
  tenv:
    all: true # check all functions in _test.go, not only test functions # default false
  unparam:
    check-exported: true # default false
 
linters:
  disable-all: true
  enable:
    ## enabled by default
    - errcheck
    - gosimple
    - govet
    - ineffassign
    - staticcheck
    - typecheck
    - unused
    ## disabled by default
    - asciicheck
    - bidichk
    - bodyclose
    - contextcheck
    - cyclop
    - dupl
    - durationcheck
    - errname
    - errorlint
    - exhaustive
    - exportloopref
    - funlen
    - forbidigo
    - gochecknoglobals
    - gochecknoinits
    - gocognit
    - goconst
    - gocritic
    - gocyclo
    - godot
    - goimports
    - gomnd
    - gomoddirectives
    - gomodguard
    - goprintffuncname
    - gosec
    - lll
    - makezero
    - nakedret
    - nestif
    - nilerr
    - nilnil
    - noctx
    - nolintlint
    - prealloc
    - predeclared
    - promlinter
    - revive
    - rowserrcheck
    - sqlclosecheck
    - stylecheck
    - tenv
    - tparallel
    - unconvert
    - unparam
    - wastedassign
    - whitespace
    ## disabled
    #- varcheck # deprecated - The owner seems to have abandoned the linter.
    #- structcheck # deprecated - The owner seems to have abandoned the linter.
    #- deadcode # deprecated - The owner seems to have abandoned the linter.
    #- containedctx # is not used - linter that detects struct contained context.Context field
    #- decorder # is not used - a declaration order and number linter for golang
    #- depguard # replaced with gomodguard
    #- dogsled # is not used - сhecks assignments with too many blank identifiers (e.g. x, _, _, _, := f())
    #- errchkjson # is not used - checks types that are json encoded - reports unsupported types and unnecessary error checks
    #- exhaustivestruct # too strict - finds structs that have uninitialized fields # TODO: maybe enable for some packages?
    #- forcetypeassert # errcheck is used instead
    #- gci # is not used - sorts imports
    #- godox # is not used - complains about TODOs in comments
    #- goerr113 # too strict - checks the errors handling expressions
    #- gofmt # replaced with goimports
    #- gofumpt # replaced with goimports, gofumports is not available yet
    #- goheader # is not used - checks that each file has the licence at the beginning
    #- golint # deprecated - revive is used instead
    #- grouper # is not used - analyze expression groups, require 'import' declaration groups
    #- ifshort # is not used - checks that your code uses short syntax for if-statements whenever possible
    #- importas # is not used - enforces consistent import aliases
    #- interfacer # deprecated and has false positives
    #- ireturn # good, but too strict - accept interfaces, return concrete types
    #- maintidx # is not used - measures the maintainability index of each function
    #- maligned # deprecated
    #- misspell # useless - correct commonly misspelled English words... quickly
    #- nlreturn # too strict - requires a new line before return and branch statements
    #- paralleltest # too many false positives
    #- scopelint # deprecated
    #- tagliatelle # is not used - checks the struct tags
    #- thelper # is not used - requires to use t.Helper()
    #- wrapcheck # too strict - requires wrapping errors from external packages (even from the same repo) and interfaces
    #- varnamelen # great idea, but too many false positives - checking length of variable's name matches its usage scope
    #- wsl # too strict - enforces empty lines at the right places
 
output:
  uniq-by-line: false # default true
 
issues:
  new-from-rev: 5e152aca7ca51878f12b657b5fcbdf992aa0b83c
  fix: true
  max-issues-per-linter: 0
  max-same-issues: 0
  exclude-rules:
    - source: "^//\\s*go:generate\\s"
      linters: 
        - lll
    - source: "(noinspection|TODO)"
      linters: 
        - godot
    - source: "//noinspection"
      linters: 
        - gocritic
    - path: "_test\\.go"
      linters:
        - bodyclose
        - dupl
        - funlen
        - goconst
        - noctx
        - wrapcheck

Go environment

$ go version && go env
go version go1.19.2 linux/amd64
GO111MODULE="auto"
GOARCH="amd64"
GOBIN="/ssdsrc/go/bin"
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/ssdsrc/go/pkg/mod"
GONOPROXY="git.xxx.inc/xxx"
GONOSUMDB="git.xxx.inc/xxx"
GOOS="linux"
GOPATH="/ssdsrc/go"
GOPRIVATE="git.xxx.inc/xxx"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/ssdsrc/go/src/git.xxx.inc/xxx/ooo/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 -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1094368719=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /ssdsrc/go/src/git.xxx.inc/xxx/ooo /ssdsrc/go/src/git.xxx.inc/xxx /ssdsrc/go/src/git.xxx.inc /ssdsrc/go/src /ssdsrc/go /ssdsrc / /root] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 54 linters: [asciicheck bidichk bodyclose contextcheck cyclop dupl durationcheck errcheck errname errorlint exhaustive exportloopref forbidigo funlen gochecknoglobals gochecknoinits gocognit goconst gocritic gocyclo godot goimports gomnd gomoddirectives gomodguard goprintffuncname gosec gosimple govet ineffassign lll makezero nakedret nestif nilerr nilnil noctx nolintlint prealloc predeclared promlinter revive rowserrcheck sqlclosecheck staticcheck stylecheck tenv tparallel typecheck unconvert unparam unused wastedassign whitespace] 
INFO [loader] Go packages loading at mode 575 (imports|name|types_sizes|compiled_files|exports_file|deps|files) took 248.962256ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 9.993664ms 
INFO [linters_context/goanalysis] analyzers took 1m21.766136274s with top 10 stages: buildir: 24.955653867s, buildssa: 19.090886048s, exhaustive: 11.692436517s, the_only_name: 9.265313434s, inspect: 2.039355106s, dupl: 1.146377177s, ctrlflow: 1.065698159s, bidichk: 1.054131561s, unconvert: 980.953164ms, goimports: 854.760771ms 
WARN [linters_context] rowserrcheck is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649. 
WARN [linters_context] sqlclosecheck is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649. 
WARN [linters_context] wastedassign is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649. 
INFO [runner] Issues before processing: 412, after processing: 2 
INFO [runner] Processors filtering stat (out/in): filename_unadjuster: 412/412, max_same_issues: 2/2, path_shortener: 2/2, path_prefixer: 2/2, sort_results: 2/2, path_prettifier: 412/412, nolint: 139/177, uniq_by_line: 139/139, max_per_file_from_linter: 2/2, cgo: 412/412, autogenerated_exclude: 365/412, diff: 2/139, severity-rules: 2/2, skip_files: 412/412, skip_dirs: 412/412, identifier_marker: 365/365, exclude: 365/365, exclude-rules: 177/365, max_from_linter: 2/2, source_code: 2/2 
INFO [runner] processing took 52.267707ms with stages: diff: 25.737539ms, exclude-rules: 7.313597ms, identifier_marker: 6.955825ms, nolint: 6.2829ms, path_prettifier: 3.190575ms, autogenerated_exclude: 2.205334ms, skip_dirs: 456.642µs, cgo: 53.448µs, source_code: 35.707µs, filename_unadjuster: 30.509µs, path_shortener: 1.251µs, max_same_issues: 1.101µs, max_per_file_from_linter: 803ns, skip_files: 803ns, uniq_by_line: 406ns, sort_results: 307ns, severity-rules: 279ns, exclude: 258ns, max_from_linter: 224ns, path_prefixer: 199ns 
INFO [runner] linters took 16.037105197s with stages: goanalysis_metalinter: 15.984694873s, rowserrcheck: 18.726µs, sqlclosecheck: 12.336µs, wastedassign: 9.64µs 
INFO Fix issue &result.Issue{FromLinter:"goimports", Text:"File is not `goimports`-ed", Severity:"", SourceLines:[]string{"//nolint: gochecknoglobals, gochecknoinits, gosec // for metrics"}, Replacement:(*result.Replacement)(0xc00906a050), Pkg:(*packages.Package)(0xc001052180), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"internal/lib/ooo/ooo.go", Offset:0, Line:1, Column:0}, HunkPos:1, ExpectNoLint:false, ExpectedNoLintLinter:""} with range {1 1} 
INFO Fix issue &result.Issue{FromLinter:"goimports", Text:"File is not `goimports`-ed", Severity:"", SourceLines:[]string{"//nolint: lll // for testing"}, Replacement:(*result.Replacement)(0xc020b7c710), Pkg:(*packages.Package)(0xc00106ad80), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"internal/utils/xxx.go", Offset:0, Line:1, Column:0}, HunkPos:1, ExpectNoLint:false, ExpectedNoLintLinter:""} with range {1 1} 
INFO fixer took 654.425µs with stages: all: 654.425µs 
INFO File cache stats: 34 entries of total size 105.2KiB 
INFO Memory: 163 samples, avg is 450.6MB, max is 700.0MB 
INFO Execution took 16.302160512s

Code example or link to a public repository

//nolint: lll // for testing
package ooo
@rickwang7712 rickwang7712 added the bug Something isn't working label Feb 3, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 3, 2023

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

@ldez ldez added question Further information is requested and removed bug Something isn't working labels Feb 3, 2023
@ldez
Copy link
Member

ldez commented Feb 3, 2023

Hello,

can you provide an example?

can you remove the space between nolint: and lll ? (//nolint:lll // for testing)

@rickwang7712
Copy link
Author

rickwang7712 commented Feb 3, 2023

After I remove the space between them, it works!
Thank you @ldez
Sorry I didn't notice about that part..

@ldez ldez added the area: nolint Related to nolint directive and nolintlint label Mar 24, 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 question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants