gci and goimports --fix wrongly removing packages

[caevv]

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. (https://golangci-lint.run/usage/linters/)

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:

$ 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

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

Configuration file

$ 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

$ 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

$ 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

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:

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

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

package logs

import "go.uber.org/zap"

func NewZap(s string) (*zap.Logger, error) {
	return nil, nil
}

pkg/service/service.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

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]

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

[miporto]

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

[Airblader]

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
)

[jdmaguire]

For me, my imports looked like this:

import (
	"flag"
	"github.com/myOrg/myRepo/internal/config"
	"github.com/myOrg/otherRepo/log"
	"net/http"
	"os"
)

The two imports from myOrg were getting deleted.

Writing it in this order alleviate my problem:

import (
	"flag"
	"net/http"
	"os"

	"github.com/myOrg/myRepo/internal/config"
	"github.com/myOrg/otherRepo/log"
)

I have some linter configured (don't know which) that specifies that these imports should be below stdlib imports. That's probably where the confusion is happening for the linter. It is doing the removal from the wrong area but not the insertion into the right area.