golangci / golangci-lint

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

gci and goimports --fix wrongly removing packages #2161

Open caevv opened 3 years ago

caevv commented 3 years ago

Welcome

Description of the problem

When I have enabled GCI and goimports and run with --fix certain packages are being removed.

Now if I run gci and goimports manually, the commands fix the issues and does not remove the packages, later on running golangci-lint run --fix nothing happens.

Packages being removed for the provided example:

--- i/service/pkg/logger/logger.go
+++ w/service/pkg/logger/logger.go
@@ -3,7 +3,6 @@ package logger
 import (
    "log"

-   "github.com/company/project/pkg/logs"
    "go.uber.org/zap"
 )

diff --git i/service/pkg/service/service.go w/service/pkg/service/service.go
--- i/service/pkg/service/service.go
+++ w/service/pkg/service/service.go
@@ -8,8 +8,6 @@ import (
    "github.com/aws/aws-sdk-go/aws/session"
-   "github.com/company/project/service/pkg/logger"
-   "go.uber.org/zap"
 )

Running with --fix -v:

```console $ golangci-lint run --fix -v INFO Fix issue &result.Issue{FromLinter:"goimports", Text:"File is not `goimports`-ed with -local github.com/company/project", Severity:"", SourceLines:[]string{"\t\"github.com/company/project/pkg/logs\""}, Replacement:(*result.Replacement)(0xc000cd5f50), Pkg:(*packages.Package)(0xc0011d9400), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"service/pkg/logger/logger.go", Offset:0, Line:6, Column:0}, HunkPos:4, ExpectNoLint:false, ExpectedNoLintLinter:""} with range {6 6} INFO Line 12 has multiple issues but at least one of them isn't inline: []result.Issue{result.Issue{FromLinter:"gci", Text:"File is not `gci`-ed with -local github.com/company/project", Severity:"", SourceLines:[]string{"\t\"go.uber.org/zap\""}, Replacement:(*result.Replacement)(0xc003c7e600), Pkg:(*packages.Package)(0xc0011e0780), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"service/pkg/service/service.go", Offset:0, Line:12, Column:0}, HunkPos:11, ExpectNoLint:false, ExpectedNoLintLinter:""}, result.Issue{FromLinter:"goimports", Text:"File is not `goimports`-ed with -local github.com/company/project", Severity:"", SourceLines:[]string{"\t\"go.uber.org/zap\""}, Replacement:(*result.Replacement)(0xc003c7e660), Pkg:(*packages.Package)(0xc0011e0780), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"service/pkg/service/service.go", Offset:0, Line:12, Column:0}, HunkPos:11, ExpectNoLint:false, ExpectedNoLintLinter:""}} INFO Fix issue &result.Issue{FromLinter:"goimports", Text:"File is not `goimports`-ed with -local github.com/company/project", Severity:"", SourceLines:[]string{"\t\"github.com/company/project/service/pkg/logger\""}, Replacement:(*result.Replacement)(0xc003c7e630), Pkg:(*packages.Package)(0xc0011e0780), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"service/pkg/service/service.go", Offset:0, Line:11, Column:0}, HunkPos:10, ExpectNoLint:false, ExpectedNoLintLinter:""} with range {11 11} INFO Fix issue &result.Issue{FromLinter:"gci", Text:"File is not `gci`-ed with -local github.com/company/project", Severity:"", SourceLines:[]string{"\t\"go.uber.org/zap\""}, Replacement:(*result.Replacement)(0xc003c7e600), Pkg:(*packages.Package)(0xc0011e0780), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"service/pkg/service/service.go", Offset:0, Line:12, Column:0}, HunkPos:11, ExpectNoLint:false, ExpectedNoLintLinter:""} with range {12 12} ```

Version of golangci-lint

```console $ golangci-lint --version golangci-lint has version v1.41.1 built from (unknown, mod sum: "h1:KH28pTSqRu6DTXIAANl1sPXNCmqg4VEH21z6G9Wj4SM=") on (unknown) ```

Configuration file

```console $ cat .golangci.yml linters-settings: stylecheck: # Select the Go version to target. The default is '1.13'. go: "1.16" # https://staticcheck.io/docs/options#checks checks: ["all", "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-ST1023"] # https://staticcheck.io/docs/options#dot_import_whitelist dot-import-whitelist: - fmt # https://staticcheck.io/docs/options#initialisms initialisms: [ "ACL", "API", "ASCII", "CPU", "CSS", "DNS", "EOF", "GUID", "HTML", "HTTP", "HTTPS", "ID", "IP", "JSON", "QPS", "RAM", "RPC", "SLA", "SMTP", "SQL", "SSH", "TCP", "TLS", "TTL", "UDP", "UI", "GID", "UID", "UUID", "URI", "URL", "UTF8", "VM", "XML", "XMPP", "XSRF", "XSS", ] # https://staticcheck.io/docs/options#http_status_code_whitelist http-status-code-whitelist: ["200", "400", "404", "500"] nolintlint: # Enable to ensure that nolint directives are all used. Default is true. allow-unused: true # Exclude following linters from requiring an explanation. Default is []. allow-no-explanation: ["lll"] # Enable to require an explanation of nonzero length after each nolint directive. Default is false. require-explanation: true # Enable to require nolint directives to mention the specific linter being suppressed. Default is false. require-specific: true dupl: threshold: 100 funlen: lines: 100 statements: 50 gci: local-prefixes: github.com/company/project goconst: min-len: 2 min-occurrences: 3 gocyclo: min-complexity: 15 goimports: local-prefixes: github.com/company/project errorlint: # Report non-wrapping error creation using fmt.Errorf errorf: true exhaustive: # check switch statements in generated files also check-generated: false # 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: true govet: check-shadowing: true lll: line-length: 160 maligned: suggest-new: true misspell: locale: UK errcheck: ignore: go.uber.org/zap:Sync # see: https://github.com/uber-go/zap/issues/880 linters: # please, do not use `enable-all`: it's deprecated and will be removed soon. # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint disable-all: true enable: - bodyclose - deadcode - dogsled - dupl - errcheck - exhaustive - funlen - goconst - gocyclo - gofmt - goimports - goprintffuncname - gosec - gosimple - govet - ineffassign - lll - misspell - nakedret - noctx - rowserrcheck - exportloopref - staticcheck - structcheck - stylecheck - typecheck - unconvert - unparam - unused - varcheck - whitespace - errorlint - gci - nolintlint issues: # Excluding configuration per-path, per-linter, per-text and per-source exclude-rules: - path: _test\.go linters: - stylecheck - dupl - gocyclo - errcheck - dupl - gosec - errorlint run: skip-dirs: - ^node_modules - /node_modules/ - node_modules/ ```

Go environment

```console $ go version && go env go version go1.16.1 linux/amd64 GO111MODULE="" GOARCH="amd64" GOBIN="" GOCACHE="/home/user/.cache/go-build" GOENV="/home/user/.config/go/env" GOEXE="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="linux" GOINSECURE="" GOMODCACHE="/home/user/go/pkg/mod" GONOPROXY="" GONOSUMDB="" GOOS="linux" GOPATH="/home/user/go" GOPRIVATE="" 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.16.1" GCCGO="gccgo" AR="ar" CC="gcc" CXX="g++" CGO_ENABLED="1" GOMOD="/home/user/projects/project/go.mod" 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-build2051264545=/tmp/go-build -gno-record-gcc-switches" ```

Verbose output of running

```console $ golangci-lint cache clean $ golangci-lint run -v INFO [config_reader] Config search paths: [./ /home/user/projects/project /home/user/projects /home/user /home /] INFO [config_reader] Used config file .golangci.yml INFO [lintersdb] Active 34 linters: [bodyclose deadcode dogsled dupl errcheck errorlint exhaustive exportloopref funlen gci goconst gocyclo gofmt goimports goprintffuncname gosec gosimple govet ineffassign lll misspell nakedret noctx nolintlint rowserrcheck staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck whitespace] INFO [loader] Go packages loading at mode 575 (compiled_files|deps|exports_file|files|imports|name|types_sizes) took 503.252426ms INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 9.539576ms INFO [linters context/goanalysis] analyzers took 2m5.180974569s with top 10 stages: buildir: 29.90375527s, buildssa: 10.532986377s, dupl: 4.000728454s, nilness: 3.17112127s, unconvert: 3.013323896s, exhaustive: 2.500218923s, inspect: 2.184417116s, unparam: 2.069441226s, fact_deprecated: 2.067950955s, goimports: 1.917180161s INFO [runner] Processors filtering stat (out/in): exclude-rules: 173/289, source_code: 3/3, path_prefixer: 3/3, sort_results: 3/3, skip_dirs: 566/873, path_prettifier: 873/873, skip_files: 873/873, autogenerated_exclude: 289/566, identifier_marker: 289/289, nolint: 133/173, diff: 3/123, max_same_issues: 3/3, filename_unadjuster: 873/873, path_shortener: 3/3, max_from_linter: 3/3, exclude: 289/289, uniq_by_line: 123/133, max_per_file_from_linter: 3/3, severity-rules: 3/3, cgo: 873/873 INFO [runner] processing took 1.23197445s with stages: diff: 1.203060295s, nolint: 11.469006ms, exclude-rules: 9.399085ms, identifier_marker: 3.931502ms, path_prettifier: 1.13589ms, skip_dirs: 1.12035ms, cgo: 1.115101ms, autogenerated_exclude: 632.101µs, source_code: 40.73µs, filename_unadjuster: 34.36µs, uniq_by_line: 23.95µs, max_per_file_from_linter: 4.34µs, max_same_issues: 3.19µs, path_shortener: 1.41µs, max_from_linter: 1.3µs, sort_results: 730ns, exclude: 410ns, skip_files: 350ns, severity-rules: 230ns, path_prefixer: 120ns INFO [runner] linters took 6.553331594s with stages: goanalysis_metalinter: 5.321278403s service/pkg/service/service.go:12: File is not `gci`-ed with -local github.com/company/project (gci) "go.uber.org/zap" service/pkg/logger/logger.go:6: File is not `goimports`-ed with -local github.com/company/project (goimports) "github.com/company/project/pkg/logs" service/pkg/service/service.go:11: File is not `goimports`-ed with -local github.com/company/project (goimports) "github.com/company/project/service/pkg/logger" INFO File cache stats: 366 entries of total size 1.2MiB INFO Memory: 72 samples, avg is 1070.5MB, max is 1555.8MB INFO Execution took 7.071730958s ```

Code example or link to a public repository

logger.go ```go package logger import ( "log" "github.com/company/project/pkg/logs" "go.uber.org/zap" ) func New() *zap.Logger { logger, err := logs.NewZap("dev") if err != nil { log.Fatalf("unable to build zap logger: %v", err) } return logger } ``` service.go: ```go package service import ( "encoding/json" "fmt" "github.com/aws/aws-sdk-go/aws" "github.com/company/project/service/pkg/logger" "go.uber.org/zap" ) type S struct { logger *zap.Logger } func New() *S { p.logger = logger.New() return &p } func (p *S) A(d) error { p.logger.Error("sample", zap.String("key", "value")) return nil } ```

EDIT

.
├── go.mod
├── go.sum
├── logger
└── pkg
    ├── logger
    │   └── logger.go
    ├── logs
    │   └── logs.go
    └── service
        └── service.go
pkg/logger/logger.go ```go package logger import ( "log" "github.com/golangci/sandbox/pkg/logs" "go.uber.org/zap" ) func New() *zap.Logger { logger, err := logs.NewZap("dev") if err != nil { log.Fatalf("unable to build zap logger: %v", err) } return logger } ```
pkg/logs/logs.go ```go package logs import "go.uber.org/zap" func NewZap(s string) (*zap.Logger, error) { return nil, nil } ```
pkg/service/service.go ```go package service import ( "encoding/json" "fmt" "go.uber.org/zap" "github.com/golangci/sandbox/pkg/logger" ) type Service struct { logger *zap.Logger } func New() *Service { p := Service{} p.logger = logger.New() return &p } func (p *Service) Method(d string) error { p.logger.Error("sample", zap.String("key", "value")) fmt.Println("test") _ = json.Encoder{} return nil } ```
.golangci.yml ```yaml linters-settings: gci: local-prefixes: github.com/golangci/sandbox goimports: local-prefixes: github.com/golangci/sandbox linters: disable-all: true enable: - goimports - gci ```
boring-cyborg[bot] commented 3 years ago

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

miporto commented 3 years ago

I'm facing the same issue with golangci-lint v1.41.1. I'm only using goimports though.

Airblader commented 2 years ago

We're also hitting an issue where gci removes imports, breaking the build.

Edit: In our case, the import section contained the following two lines at the bottom, and removing them causes gci to no longer erroneously remove the import:

import (
  // …

  //nolint
  //+kubebuilder:scaffold:imports
)