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

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

Open
5 tasks done
wiegell opened this issue Aug 10, 2023 · 7 comments
Open
5 tasks done

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

wiegell opened this issue Aug 10, 2023 · 7 comments
Labels
bug Something isn't working feedback required Requires additional feedback

Comments

@wiegell
Copy link

wiegell commented Aug 10, 2023

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

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

Configuration

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

$ 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

$ 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:

//gofumpt:diagnose version: v0.5.0 (go1.20.5) flags: -lang=v1.20 -modpath=

with modpath:

//gofumpt:diagnose version: v0.5.0 (go1.20.5) flags: -lang=v1.17 -modpath=some-module-name

vanilla gofumpt:

//gofumpt:diagnose version: v0.5.0 (go1.20.5) flags: -lang=v1.17 -modpath=some-module-name

Validation

  • Yes, I've included all information above (version, config, etc.).
@wiegell wiegell added the bug Something isn't working label Aug 10, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Aug 10, 2023

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

@ldez
Copy link
Member

ldez commented Aug 10, 2023

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.

@ldez ldez added the feedback required Requires additional feedback label Aug 10, 2023
tricktron added a commit to tricktron/learn-go-with-tests that referenced this issue Nov 16, 2023
in golangci-lint config. This fixes the false positive `file is not
gofumpt-ed` error although the file is correctly formatted [1].

[1]: golangci/golangci-lint#4004
@tricktron
Copy link

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

@pierre-emmanuelJ

This comment was marked as off-topic.

@ldez

This comment was marked as off-topic.

@ldez

This comment was marked as off-topic.

@pierre-emmanuelJ

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feedback required Requires additional feedback
Projects
None yet
Development

No branches or pull requests

4 participants