golangci / golangci-lint

Fast linters runner for Go
https://golangci-lint.run
GNU General Public License v3.0
15.33k stars 1.37k forks source link

File is not gofumpt-ed on properly formatted code #4004

Open wiegell opened 1 year ago

wiegell commented 1 year ago

Welcome

Description of the problem

The problem has already been discussed in detail here: #2711

Golangci-lint sorts imports differently than standalone gofumpt. This is especially an issue when using vs code, where gopls (gofumpt) therefore formats the imports in a way that is invalid in CI-run of gofumpt (via golangci-lint).

When doing a: //gofumpt:diagnose One can see, that the golangci-lint --fix does not find the modpath automatically if not explicitly set in the lint-settings in .golangci.yaml. Because of this, it is not supported to have one common .golangci.yaml in a workspace, but instead multiple are needed - one for each module, which is not very dry.

Version of golangci-lint

```console $ golangci-lint --version golangci-lint has version 1.53.3 built with go1.20.5 from 2dcd82f on 2023-06-14T21:13:17Z ```

Configuration

```console linters: disable-all: true enable: # Defaults: - errcheck # errcheck is a program for checking for unchecked errors in Go code. These unchecked errors can be critical bugs in some cases - gosimple # Linter for Go source code that specializes in simplifying code - govet # Vet examines Go source code and reports suspicious constructs - ineffassign # Detects when assignments to existing variables are not used - staticcheck # It's a set of rules from staticcheck. It's not the same thing as the staticcheck binary. - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code # - unused # Checks Go code for unused constants, variables, functions and types # Extras: - bodyclose # checks whether HTTP response body is closed successfully - dogsled # Checks assignments with too many blank identifiers (e.g. x, , , _, := f()) - dupl # Tool for code clone detection - exportloopref # checks for pointers to enclosing loop variables - goconst # Finds repeated strings that could be replaced by a constant - gocritic # Provides diagnostics that check for bugs, performance and style issues. - gocyclo # Computes and checks the cyclomatic complexity of functions - gofumpt # Gofmt checks whether code was gofmt-ed - goimports # Check import statements are formatted according to the 'goimport' command - goprintffuncname # Checks that printf-like functions are named with f at the end - gosec # Inspects source code for security problems - nakedret # Finds naked returns in functions greater than a specified function length - noctx # noctx finds sending http request without context.Context - nolintlint # Reports ill-formed or insufficient nolint directives - revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. - stylecheck # Stylecheck is a replacement for golint - unconvert # Remove unnecessary type conversions - whitespace # Tool for detection of leading and trailing whitespace # https://golangci-lint.run/usage/linters/ linters-settings: revive: rules: - name: line-length-limit severity: warning disabled: true arguments: [80] # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#var-naming - name: var-naming severity: warning disabled: true arguments: - ["ID"] # AllowList - ["VM"] # DenyList gofumpt: # Deprecated: use the global `run.go` instead. lang-version: "1.17" # Module path which contains the source code being formatted. # Default: "" module-path: some-module-name stylecheck: # STxxxx checks in https://staticcheck.io/docs/configuration/options/#checks # Default: ["*"] checks: ["all", "-ST1003"] # https://staticcheck.io/docs/configuration/options/#dot_import_whitelist # Default: ["github.com/mmcloughlin/avo/build", "github.com/mmcloughlin/avo/operand", "github.com/mmcloughlin/avo/reg"] dot-import-whitelist: - fmt ```

Go environment

```console $ go version && go env go version go1.20.5 darwin/arm64 GO111MODULE="" GOARCH="arm64" GOBIN="/Users/USER/gocode/bin" GOCACHE="/Users/USER/Library/Caches/go-build" GOENV="/Users/USER/Library/Application Support/go/env" GOEXE="" GOEXPERIMENT="" GOFLAGS="" GOHOSTARCH="arm64" GOHOSTOS="darwin" GOINSECURE="" GOMODCACHE="/Users/USER/gocode/pkg/mod" GONOPROXY="" GONOSUMDB="" GOOS="darwin" GOPATH="/Users/USER/gocode" GOPRIVATE="" GOPROXY="https://proxy.golang.org,direct" GOROOT="/Users/USER/sdk/go1.20.5" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/Users/USER/sdk/go1.20.5/pkg/tool/darwin_arm64" GOVCS="" GOVERSION="go1.20.5" GCCGO="gccgo" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/dev/null" GOWORK="/Users/USER/REPOROOT/go.work" 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/hg/3s1q3w8j4xlgbp55q9cbsnkm0000gn/T/go-build3686403820=/tmp/go-build -gno-record-gcc-switches -fno-common" ```

Verbose output of running

```console $ golangci-lint cache clean $ golangci-lint run -v This is with the config file in the module path (no problem) INFO [config_reader] Config search paths: [./ PATHTOGOWORKSPACE/graphql-server PATHTOGOWORKSPACE cd .. cd .. etc. etc.] INFO [config_reader] Used config file .golangci.yaml INFO [lintersdb] Active 24 linters: [bodyclose dogsled dupl errcheck exportloopref goconst gocritic gocyclo gofumpt goimports goprintffuncname gosec gosimple govet ineffassign nakedret noctx nolintlint revive staticcheck stylecheck typecheck unconvert whitespace] INFO [loader] Go packages loading at mode 575 (name|types_sizes|compiled_files|exports_file|files|imports|deps) took 425.644208ms INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 8.722375ms INFO [linters_context/goanalysis] analyzers took 13.300978294s with top 10 stages: buildir: 7.366484553s, fact_deprecated: 778.121454ms, ctrlflow: 528.624521ms, printf: 504.223613ms, inspect: 503.29328ms, nilness: 479.458845ms, fact_purity: 404.526466ms, typedness: 350.321912ms, SA5012: 321.207791ms, buildssa: 300.882584ms INFO [runner] Issues before processing: 582, after processing: 0 INFO [runner] Processors filtering stat (out/in): path_prettifier: 582/582, skip_files: 582/582, filename_unadjuster: 582/582, identifier_marker: 43/43, exclude: 43/43, autogenerated_exclude: 43/582, skip_dirs: 582/582, exclude-rules: 0/43, cgo: 582/582 INFO [runner] processing took 4.735334ms with stages: autogenerated_exclude: 2.554042ms, path_prettifier: 1.074666ms, exclude-rules: 592.749µs, identifier_marker: 393.334µs, skip_dirs: 79.042µs, filename_unadjuster: 22.5µs, cgo: 16.25µs, nolint: 457ns, max_same_issues: 334ns, fixer: 292ns, path_shortener: 291ns, sort_results: 250ns, severity-rules: 250ns, source_code: 209ns, exclude: 167ns, uniq_by_line: 167ns, max_per_file_from_linter: 125ns, max_from_linter: 84ns, diff: 83ns, skip_files: 42ns, path_prefixer: 0s INFO [runner] linters took 2.761537958s with stages: goanalysis_metalinter: 2.756748958s INFO File cache stats: 1 entries of total size 311.2KiB INFO Memory: 34 samples, avg is 321.9MB, max is 586.6MB INFO Execution took 3.214834083s ```

Code example or link to a public repository

Diagnose in module without modpath in golangci-lint.yaml: ```go //gofumpt:diagnose version: v0.5.0 (go1.20.5) flags: -lang=v1.20 -modpath= ``` with modpath: ```go //gofumpt:diagnose version: v0.5.0 (go1.20.5) flags: -lang=v1.17 -modpath=some-module-name ``` vanilla gofumpt: ```go //gofumpt:diagnose version: v0.5.0 (go1.20.5) flags: -lang=v1.17 -modpath=some-module-name ```

Validation

boring-cyborg[bot] commented 1 year ago

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

ldez commented 1 year ago

The go version is extracted from the go.mod of the module and used by gofumpt. module-path is empty by default.

I think the problem is not really related to module-path but to the detection of the go.mod inside a workspace.

Can you provide a fully reproducible example?

Note: golangci-lint is not able to run on all modules of a workspace in one run.

tricktron commented 10 months ago

I could reproduce this issue both locally and in the CI.

pierre-emmanuelJ commented 4 months ago

Probably the same issue here: https://github.com/exoscale/egoscale/pull/631

I can also reproduce locally, what is weird is, when I run goimports v3/generator/schemas/schemas.go on the go file, there is exactly no diff.

golangci-lint run --timeout 5m ./v3/...
WARN [config_reader] The configuration option `run.skip-dirs` is deprecated, please use `issues.exclude-dirs`.
v3/generator/schemas/schemas.go:10: File is not `goimports`-ed with -local github.com (goimports)

The output is saying line 10 in schemas.go

package schemas

import (
    "bytes"
    "fmt"
    "go/format"
    "log/slog"
    "os"
    "strings"

    "github.com/exoscale/egoscale/v3/generator/helpers"
    "github.com/pb33f/libopenapi"
    "github.com/pb33f/libopenapi/datamodel/high/base"
    "github.com/pb33f/libopenapi/orderedmap"
    "gopkg.in/yaml.v3"
)

line 10 is the space automatically formatted by gofmt between:

"strings"

"github.com/exoscale/egoscale/v3/generator/helpers"

Note: not very useful but this fix the lint errors (not the right solution)

package schemas

import (
    "bytes"
    "fmt"
    "go/format"
    "log/slog"
    "os"
    "strings"

    // nolint: goimports
    "github.com/exoscale/egoscale/v3/generator/helpers"
    "github.com/pb33f/libopenapi"
    "github.com/pb33f/libopenapi/datamodel/high/base"
    "github.com/pb33f/libopenapi/orderedmap"
    "gopkg.in/yaml.v3" // nolint: goimports
)

Probably you have an idea on this, thank you!

ldez commented 4 months ago

@pierre-emmanuelJ Your problem is related to your configuration: https://github.com/exoscale/egoscale/blob/192c5a5e661a6a04b789e68755e2af830ea4061e/.golangci.yml#L7-L8

Run the following command:

golangci-lint run --enable-only goimports --fix
ldez commented 4 months ago

I think your configuration is not "logical": a local prefix is for the module name (ex: github.com/exoscale/egoscale) or the organization (ex: github.com/exoscale).

The fact that using github.com as a prefix, required splitting the import like this:


import (
    "bytes"
    "fmt"
    "go/format"
    "log/slog"
    "os"
    "strings"

    "gopkg.in/yaml.v3"

    "github.com/exoscale/egoscale/v3/generator/helpers"
    "github.com/pb33f/libopenapi"
    "github.com/pb33f/libopenapi/datamodel/high/base"
    "github.com/pb33f/libopenapi/orderedmap"
)

I recommend changing your configuration to:

linters-settings:
  goimports:
    local-prefixes: github.com/exoscale/egoscale

or just remove this configuration because std imports and external imports are always splitted.

pierre-emmanuelJ commented 4 months ago

Thanks, @ldez :) I'll apply your suggestion

aimuz commented 4 months ago

I was using gofumpt and it would accidentally cause import to be removed, when I removed gofumpt it all worked fine!

```yaml run: timeout: 5m linters-settings: goimports: local-prefixes: github.com/aimuz/go-template gofumpt: # Module path which contains the source code being formatted. # Default: "" module-path: github.com/aimuz/go-template # Choose whether to use the extra rules. # Default: false extra-rules: true gofmt: rewrite-rules: - pattern: 'interface{}' replacement: 'any' interfacebloat: # The maximum number of methods allowed for an interface. # Default: 10 max: 10 misspell: locale: US depguard: rules: main: deny: - pkg: "sync/atomic" desc: "Use go.uber.org/atomic instead of sync/atomic" - pkg: "github.com/stretchr/testify/assert" desc: "Use github.com/stretchr/testify/require instead of github.com/stretchr/testify/assert" - pkg: "io/ioutil" desc: "Use corresponding 'os' or 'io' functions instead." - pkg: "regexp" desc: "Use github.com/grafana/regexp instead of regexp" - pkg: "github.com/pkg/errors" desc: "Use 'errors' or 'fmt' instead of github.com/pkg/errors" - pkg: "gzip" desc: "Use github.com/klauspost/compress instead of gzip" - pkg: "zlib" desc: "Use github.com/klauspost/compress instead of zlib" - pkg: "golang.org/x/exp/slices" desc: "Use 'slices' instead." linters: enable: - bodyclose - depguard - errcheck - errname - gosimple - govet - ineffassign - staticcheck - unused - goimports - gofmt - gofumpt - gocheckcompilerdirectives - gochecknoinits - nolintlint - sloglint - intrange - interfacebloat - nilerr - nilnil - revive - misspell # - gocritic issues: include: - EXC0011 - EXC0012 - EXC0013 exclude-generated-strict: true ``` golangci-lint run --fix