golangci-lint run --fix generates broken code

[IrvinFan]

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

golangci-lint run reports an issue where a comment has no leading space.

Then, run golangci-lint run --fix to auto fix it. Instead of adding the space it removes the whole line.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.52.2 built with go1.20.2 from da04413 on 2023-03-23T16:18:48Z

Configuration file

$ cat .golangci.yml
# Config for golanglint-ci
# Coming from/keeping in sync with https://github.com/fortio/fortio/blob/master/.golangci.yml

# output configuration options

# all available settings of specific linters
linters-settings:
  gocritic:
    disabled-checks:
      - ifElseChain
  dupl:
    # tokens count to trigger issue, 150 by default
    threshold: 100
  exhaustive:
    # indicates that switch statements are to be considered exhaustive if a
    # 'default' case is present, even if all enum members aren't listed in the
    # switch
    default-signifies-exhaustive: false
  funlen:
    lines: 140
    statements: 70
  gocognit:
    # minimal code complexity to report, 30 by default (but we recommend 10-20)
    min-complexity: 42
  nestif:
    # minimal complexity of if statements to report, 5 by default
    min-complexity: 4
  gocyclo:
    # minimal code complexity to report, 30 by default (but we recommend 10-20)
    min-complexity: 30
  godot:
    # check all top-level comments, not only declarations
    check-all: false
  govet:
    # report about shadowed variables
    check-shadowing: true
    # settings per analyzer
    settings:
      printf: # analyzer name, run `go tool vet help` to see all analyzers
        funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Printf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).FErrf
    enable-all: true
    disable-all: false
  depguard:
    list-type: blacklist
    include-go-root: false
    packages:
      - github.com/sirupsen/logrus
    packages-with-error-message:
      # specify an error message to output when a blacklisted package is used
      - github.com/sirupsen/logrus: "logging is allowed only by fortio.log"
  lll:
    # max line length, lines longer will be reported. Default is 120.
    # '\t' is counted as 1 character by default, and can be changed with the tab-width option
    line-length: 132
    # tab width in spaces. Default to 1.
    tab-width: 1
  misspell:
    # Correct spellings using locale preferences for US or UK.
    # Default is to use a neutral variety of English.
    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
    locale: US
    ignore-words:
      - fortio
  nakedret:
    # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
    max-func-lines: 30
  nolintlint:
    require-specific: true
  whitespace:
    multi-if: false   # Enforces newlines (or comments) after every multi-line if statement
    multi-func: false # Enforces newlines (or comments) after every multi-line function signature
  gofumpt:
    # Choose whether or not to use the extra rules that are disabled
    # by default
    extra-rules: false


linters:
  disable:
    # bad ones:
    - musttag
    # Deprecated ones:
    - scopelint
    - golint
    - interfacer
    - maligned
    - varcheck
    - structcheck
    - nosnakecase
    - deadcode
    # Weird/bad ones:
    - wsl
    - nlreturn
    - gochecknoinits
    - gochecknoglobals
    - gomnd
    - testpackage
    - wrapcheck
    - exhaustivestruct
    - tagliatelle
    - nonamedreturns
    - varnamelen
    - exhaustruct # seems like a good idea at first but actually a pain and go does have zero values for a reason.
# TODO consider putting these back, when they stop being bugged (ifshort, wastedassign,...)
    - paralleltest
    - thelper
    - forbidigo
    - ifshort
    - wastedassign
    - cyclop
    - forcetypeassert
    - ireturn
  enable-all: true
  disable-all: false
  # Must not use fast: true in newer golangci-lint or it'll just skip a bunch of linter instead of doing caching like before (!)
  fast: false


issues:
  # Excluding configuration per-path, per-linter, per-text and per-source
  exclude-rules:
    # Exclude some linters from running on tests files.
    - path: _test\.go
      linters:
        - gocyclo
        - errcheck
        - dupl
        - gosec
        - gochecknoinits
        - gochecknoglobals
        - forcetypeassert
        - nosnakecase
        - noctx

    # Exclude lll issues for long lines with go:generate
    - linters:
        - lll
      source: "^//go:generate "
    - linters:
        - goerr113
      text: "do not define dynamic errors"
    - linters:
        - govet
      text: "fieldalignment:"
    - linters:
        - godox
      text: "TODO"
    - linters:
        - nosnakecase
      text: "grpc_|_SERVING|O_"

  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
  max-issues-per-linter: 0

  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
  max-same-issues: 0

severity:
  # Default value is empty string.
  # Set the default severity for issues. If severity rules are defined and the issues
  # do not match or no severity is provided to the rule this will be the default
  # severity applied. Severities should match the supported severity names of the
  # selected out format.
  # - Code climate: https://docs.codeclimate.com/docs/issues#issue-severity
  # -   Checkstyle: https://checkstyle.sourceforge.io/property_types.html#severity
  # -       Github: https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message
  default-severity: error

  # The default value is false.
  # If set to true severity-rules regular expressions become case sensitive.
  case-sensitive: false

Go environment

$ go version && go env
go version go1.20.3 darwin/arm64
GO111MODULE="on"
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/yfan/Library/Caches/go-build"
GOENV="/Users/yfan/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/yfan/go/pkg/mod"
GONOPROXY=""
GONOSUMDB="gitlab.eng.roku.com"
GOOS="darwin"
GOPATH="/Users/yfan/go"
GOPRIVATE=""
GOPROXY="https://artifactory.tools.roku.com/artifactory/api/go/go"
GOROOT="/opt/homebrew/Cellar/go/1.20.3/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.20.3/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.3"
GCCGO="gccgo"
AR="ar"
CC="cc"
CXX="c++"
CGO_ENABLED="1"
GOMOD="/Users/yfan/Code/aspen-go/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/jq/0gf89dfd6kgcgb76ybp7dt4h0000gq/T/go-build1418575869=/tmp/go-build -gno-record-gcc-switches -fno-common"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v ./test.go
INFO [config_reader] Config search paths: [./ /Users/yfan/Code/aspen-go /Users/yfan/Code /Users/yfan /Users /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 74 linters: [asasalint asciicheck bidichk bodyclose containedctx contextcheck decorder depguard dogsled dupl dupword durationcheck errcheck errchkjson errname errorlint execinquery exhaustive exportloopref funlen gci ginkgolinter gocheckcompilerdirectives gocognit goconst gocritic gocyclo godot godox goerr113 gofmt gofumpt goheader goimports gomoddirectives gomodguard goprintffuncname gosec gosimple govet grouper importas ineffassign interfacebloat lll loggercheck maintidx makezero misspell nakedret nestif nilerr nilnil noctx nolintlint nosprintfhostport prealloc predeclared promlinter reassign revive rowserrcheck sqlclosecheck staticcheck stylecheck tenv testableexamples tparallel typecheck unconvert unparam unused usestdlibvars whitespace] 
INFO [loader] Go packages loading at mode 575 (types_sizes|deps|exports_file|imports|compiled_files|files|name) took 84.745625ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 246.209µs 
INFO [linters_context] importas settings found, but no aliases listed. List aliases under alias: key. 
INFO [linters_context/goanalysis] analyzers took 2.264392212s with top 10 stages: exhaustive: 682.859415ms, buildir: 523.698125ms, buildssa: 271.55471ms, the_only_name: 189.124584ms, ctrlflow: 148.452167ms, findcall: 52.697749ms, printf: 48.468084ms, fact_deprecated: 46.733918ms, SA5012: 44.383377ms, fact_purity: 42.742875ms 
INFO [runner] Issues before processing: 5, after processing: 1 
INFO [runner] Processors filtering stat (out/in): skip_dirs: 5/5, autogenerated_exclude: 5/5, identifier_marker: 5/5, exclude: 5/5, uniq_by_line: 1/2, source_code: 1/1, fixer: 1/1, sort_results: 1/1, diff: 1/1, max_same_issues: 1/1, max_from_linter: 1/1, path_shortener: 1/1, filename_unadjuster: 5/5, path_prettifier: 5/5, skip_files: 5/5, nolint: 2/2, max_per_file_from_linter: 1/1, cgo: 5/5, exclude-rules: 2/5, severity-rules: 1/1, path_prefixer: 1/1 
INFO [runner] processing took 496.791µs with stages: autogenerated_exclude: 143.667µs, exclude-rules: 103µs, nolint: 80.375µs, identifier_marker: 75.625µs, path_prettifier: 54.417µs, source_code: 24.083µs, skip_dirs: 8.208µs, uniq_by_line: 3.041µs, cgo: 791ns, path_shortener: 625ns, filename_unadjuster: 541ns, max_same_issues: 500ns, skip_files: 334ns, max_per_file_from_linter: 333ns, fixer: 333ns, severity-rules: 292ns, exclude: 209ns, sort_results: 167ns, max_from_linter: 125ns, diff: 83ns, path_prefixer: 42ns 
INFO [runner] linters took 1.739605667s with stages: goanalysis_metalinter: 1.7390365s 
test.go:10:26: commentFormatting: put a space between `//` and comment text (gocritic)
        if os.IsNotExist(err) { //file does not exist
                                ^
INFO File cache stats: 2 entries of total size 512B 
INFO Memory: 20 samples, avg is 171.1MB, max is 306.0MB 
INFO Execution took 1.832225792s           

$ golangci-lint run -v --fix ./test.go 
INFO [config_reader] Config search paths: [./ /Users/yfan/Code/aspen-go /Users/yfan/Code /Users/yfan /Users /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 74 linters: [asasalint asciicheck bidichk bodyclose containedctx contextcheck decorder depguard dogsled dupl dupword durationcheck errcheck errchkjson errname errorlint execinquery exhaustive exportloopref funlen gci ginkgolinter gocheckcompilerdirectives gocognit goconst gocritic gocyclo godot godox goerr113 gofmt gofumpt goheader goimports gomoddirectives gomodguard goprintffuncname gosec gosimple govet grouper importas ineffassign interfacebloat lll loggercheck maintidx makezero misspell nakedret nestif nilerr nilnil noctx nolintlint nosprintfhostport prealloc predeclared promlinter reassign revive rowserrcheck sqlclosecheck staticcheck stylecheck tenv testableexamples tparallel typecheck unconvert unparam unused usestdlibvars whitespace] 
INFO [loader] Go packages loading at mode 575 (deps|imports|types_sizes|compiled_files|exports_file|files|name) took 135.208083ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 137.75µs 
INFO [linters_context] importas settings found, but no aliases listed. List aliases under alias: key. 
INFO [linters_context/goanalysis] analyzers took 197.208418ms with top 10 stages: the_only_name: 151.267042ms, buildssa: 5.929164ms, gocritic: 3.890792ms, buildir: 3.560713ms, gci: 3.190375ms, dupl: 3.112625ms, depguard: 3.098375ms, unconvert: 2.377667ms, printf: 2.174669ms, fact_deprecated: 2.000871ms 
INFO [runner] Line 10 has multiple issues but at least one of them isn't inline: []result.Issue{result.Issue{FromLinter:"gocritic", Text:"commentFormatting: put a space between `//` and comment text", Severity:"error", SourceLines:[]string{"\tif os.IsNotExist(err) { //file does not exist"}, Replacement:(*result.Replacement)(0x1400211ec90), Pkg:(*packages.Package)(0x140006e7980), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"test.go", Offset:124, Line:10, Column:26}, HunkPos:0, ExpectNoLint:false, ExpectedNoLintLinter:""}, result.Issue{FromLinter:"gofumpt", Text:"File is not `gofumpt`-ed", Severity:"error", SourceLines:[]string{"\tif os.IsNotExist(err) { //file does not exist"}, Replacement:(*result.Replacement)(0x14000a84350), Pkg:(*packages.Package)(0x140006e7980), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"test.go", Offset:0, Line:10, Column:0}, HunkPos:0, ExpectNoLint:false, ExpectedNoLintLinter:""}} 
INFO [runner] Fix issue &result.Issue{FromLinter:"gocritic", Text:"commentFormatting: put a space between `//` and comment text", Severity:"error", SourceLines:[]string{"\tif os.IsNotExist(err) { //file does not exist"}, Replacement:(*result.Replacement)(0x1400211ec90), Pkg:(*packages.Package)(0x140006e7980), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"test.go", Offset:124, Line:10, Column:26}, HunkPos:0, ExpectNoLint:false, ExpectedNoLintLinter:""} with range {10 10} 
INFO [runner] fixer took 510.625µs with stages: all: 510.625µs 
INFO [runner] Issues before processing: 5, after processing: 0 
INFO [runner] Processors filtering stat (out/in): cgo: 5/5, path_prettifier: 5/5, exclude: 5/5, exclude-rules: 2/5, path_shortener: 2/2, max_same_issues: 2/2, filename_unadjuster: 5/5, skip_files: 5/5, skip_dirs: 5/5, autogenerated_exclude: 5/5, identifier_marker: 5/5, max_per_file_from_linter: 2/2, source_code: 2/2, severity-rules: 2/2, nolint: 2/2, uniq_by_line: 2/2, diff: 2/2, max_from_linter: 2/2, fixer: 0/2 
INFO [runner] processing took 875.458µs with stages: fixer: 523.458µs, exclude-rules: 99.917µs, identifier_marker: 75.041µs, nolint: 64.417µs, path_prettifier: 55.042µs, autogenerated_exclude: 26.666µs, source_code: 19.666µs, skip_dirs: 6.876µs, cgo: 708ns, uniq_by_line: 584ns, filename_unadjuster: 458ns, path_shortener: 458ns, max_same_issues: 375ns, skip_files: 334ns, severity-rules: 334ns, sort_results: 291ns, exclude: 209ns, max_per_file_from_linter: 209ns, diff: 166ns, max_from_linter: 125ns, path_prefixer: 124ns 
INFO [runner] linters took 642.215083ms with stages: goanalysis_metalinter: 641.29ms 
INFO File cache stats: 2 entries of total size 514B 
INFO Memory: 9 samples, avg is 47.0MB, max is 60.9MB 
INFO Execution took 785.87625ms                   

$ git diff
diff --git a/test.go b/test.go
index a3b354b..861c7e3 100644
--- a/test.go
+++ b/test.go
@@ -7,7 +7,7 @@ import (
 
 func MyTest(filename string) {
        _, err := os.Stat(filename)
-       if os.IsNotExist(err) { //file does not exist
+
                fmt.Printf("File %s does not exist.\n", filename)
        } else {
                fmt.Printf("File %s exists.\n", filename)

Code example or link to a public repository

package test

import (
	"fmt"
	"os"
)

func MyTest(filename string) {
	_, err := os.Stat(filename)
	if os.IsNotExist(err) { //file does not exist
		fmt.Printf("File %s does not exist.\n", filename)
	} else {
		fmt.Printf("File %s exists.\n", filename)
	}
}

[boring-cyborg]

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

[bombsimon]

This happens when two linters has a fixer for the same range. In your case you're enabling 74 linters, the two that report an issue here with a fixer are (at least) gocritic and gofumpt.

› golangci-lint run --no-config --disable-all --print-issued-lines=false --enable gofumpt
main.go:10: File is not `gofumpt`-ed (gofumpt)

› golangci-lint run --no-config --disable-all --print-issued-lines=false --enable gocritic
main.go:10:26: commentFormatting: put a space between `//` and comment text (gocritic)

Running golangci-lint run --no-config --disable-all --enable gocritic,gofumpt --fix reproduces this issue.

Somewhat a duplicate of #3454 and #1490 but I don't think there's a linter agnostic issue about this. I don't know what the proper way for golangci-lint to handle this would be, generally enabling linters for the same diagnostic isn't recommended but as more linters are adding fixes but only share slight overlaps this might become a more common issue.

[IrvinFan]

If the two linters provide contradicting fixes for the same range I do see the problem with that. But if both have the same exact fix why can't golangci-lint handle that? This would be similar to the case where git-merge works perfectly fine for two commits that makes the same change to a line. Why can't golangci-lint do the same?

Even in the case when two linters have contradicting fixes shouldn't it be better that golangci-lint throws an error instead of silently removing the line completely?

[bombsimon]

Found exact duplicate #3230 så closing this one.