gci+goimports = duplicated imports #1490

howardjohn · 2020-11-03T01:32:26Z

Config:

linters:
  disable-all: true
  enable:
  - goimports
  - gci
  fast: false

linters-settings:
  goimports:
    local-prefixes: istio.io/

Input:

package main

import (
	"fmt"
	"istio.io/istio/pkg/test/framework/resource"
	"os"

	"istio.io/istio/pkg/test/framework/resource"
)

var (
	_ = resource.Cluster
	_ = fmt.Println
	_ = os.Stdout
)

Output:

package main

import (
	"fmt"
	"os"

	"istio.io/istio/pkg/test/framework/resource"
	"istio.io/istio/pkg/test/framework/resource"

	"istio.io/istio/pkg/test/framework/resource"
)

var (
	_ = resource.Cluster
	_ = fmt.Println
	_ = os.Stdout
)

Output seems nondeterministic

cc @daixiang0

howardjohn · 2020-11-03T01:35:13Z

Input:

import (
	"fmt"
	"os"

	"istio.io/istio/pkg/test/framework/resource"
	"istio.io/istio/pkg/test/framework/resource"
	"istio.io/istio/pkg/test/framework/components/prometheus"
)

Output:

import (
	"fmt"
	"os"

	"istio.io/istio/pkg/test/framework/components/prometheus"
)

howardjohn · 2020-11-03T01:36:32Z

Also can only reproduce through golangci-lint, not direct calls on goimports -local istio.io -w main.go; gci -local istio.io -w main.go

ldez · 2021-02-14T14:06:55Z

I tried with different cases:

import (
	"io/ioutil"
	"github.com/golangci/sandbox/pkg/render"
	"github.com/ghodss/yaml"
	"log"
	"encoding/json"

	"github.com/golangci/sandbox/pkg/router"
)

import (
	"github.com/golangci/sandbox/pkg/render"
	"github.com/golangci/sandbox/pkg/router"
	"encoding/json"
	"io/ioutil"
	"log"
	"github.com/ghodss/yaml"
)

and all the linters that handle the imports (gofmt, goimports, gci, gofumpt).

Results:

linters	results
gofmt + goimports	FAIL
gofmt + gci	FAIL
goimports + gci	FAIL
goimports + gofumpt	FAIL
gofmt + gci + gofumpt	FAIL
gofmt + goimports + gci + gofumpt	FAIL
gci + gofumpt	OK
gofmt + gofumpt	OK

I think that the problem is not related to a specific linter but to how the fixes are applied.

dbraley · 2021-03-09T14:34:51Z

I'm also seeing this problem. I'll add that with a larger set of imports and more randomization, I'm able to reproduce the failure with the gci + gofumpt and gofmt + gofumpt combinations as well.

danielhochman · 2021-03-09T17:30:06Z

I've started debugging this issue. I'll try to post more as I have time to dig into the results.

Input

import (
	"google.golang.org/grpc/codes"
	"google.golang.org/grpc/status"
	"context"
	"errors"
	"fmt"

	"github.com/uber-go/tally"
	"go.uber.org/zap"
	"google.golang.org/grpc"
	"google.golang.org/protobuf/proto"
	"google.golang.org/protobuf/types/known/anypb"

	auditv1 "github.com/lyft/clutch/backend/api/audit/v1"
	"github.com/lyft/clutch/backend/gateway/log"
	"github.com/lyft/clutch/backend/gateway/meta"
	"github.com/lyft/clutch/backend/middleware"
	"github.com/lyft/clutch/backend/service"
	auditservice "github.com/lyft/clutch/backend/service/audit"
	"github.com/lyft/clutch/backend/service/authn"
)

Expected Output

import (
	"context"
	"errors"
	"fmt"

	"github.com/uber-go/tally"
	"go.uber.org/zap"
	"google.golang.org/grpc"
	"google.golang.org/grpc/codes"
	"google.golang.org/grpc/status"
	"google.golang.org/protobuf/proto"
	"google.golang.org/protobuf/types/known/anypb"

	auditv1 "github.com/lyft/clutch/backend/api/audit/v1"
	"github.com/lyft/clutch/backend/gateway/log"
	"github.com/lyft/clutch/backend/gateway/meta"
	"github.com/lyft/clutch/backend/middleware"
	"github.com/lyft/clutch/backend/service"
	auditservice "github.com/lyft/clutch/backend/service/audit"
	"github.com/lyft/clutch/backend/service/authn"
)

Actual Output

Debug Output

I added some extra log statements in a custom build to figure out what's happening internally, so you can see what the linter returns, how golangci-lint parses the diff, and how it merges the diffs.

$ golangci-lint run --fix
DEBU [importissue] goimports diff:
--- github.com/lyft/clutch/backend/middleware/audit/audit.go.orig 2021-03-09 10:57:17.396813013 -0600
+++ github.com/lyft/clutch/backend/middleware/audit/audit.go      2021-03-09 10:57:17.396813013 -0600
@@ -5,12 +5,13 @@
 // <!-- END clutchdoc -->
 
 import (
-       "google.golang.org/grpc/codes"
-       "google.golang.org/grpc/status"
        "context"
        "errors"
        "fmt"
 
+       "google.golang.org/grpc/codes"
+       "google.golang.org/grpc/status"
+
        "github.com/uber-go/tally"
        "go.uber.org/zap"
        "google.golang.org/grpc" 
        
DEBU [importissue] goimports: pos 8     range 8-9
replacement:
- NeedOnlyDelete true
- InlineFix: <nil>
- NewLines: []
 
DEBU [importissue] goimports: pos 13    range <nil>
replacement:
- NeedOnlyDelete false
- InlineFix: <nil>
- NewLines: ['','       "google.golang.org/grpc/codes"','       "google.golang.org/grpc/status"','']

DEBU [importissue] gci diff:
--- github.com/lyft/clutch/backend/middleware/audit/audit.go
+++ github.com/lyft/clutch/backend/middleware/audit/audit.go
@@ -5,8 +5,6 @@
 // <!-- END clutchdoc -->
 
 import (
-       "google.golang.org/grpc/codes"
-       "google.golang.org/grpc/status"
        "context"
        "errors"
        "fmt"
@@ -14,6 +12,8 @@
        "github.com/uber-go/tally"
        "go.uber.org/zap"
        "google.golang.org/grpc"
+       "google.golang.org/grpc/codes"
+       "google.golang.org/grpc/status"
        "google.golang.org/protobuf/proto"
        "google.golang.org/protobuf/types/known/anypb"
  
DEBU [importissue] goimports: pos 8     range 8-9
replacement:
- NeedOnlyDelete true
- InlineFix: <nil>
- NewLines: []
 
DEBU [importissue] goimports: pos 13    range <nil>
replacement:
- NeedOnlyDelete false
- InlineFix: <nil>
- NewLines: ['','       "google.golang.org/grpc/codes"','       "google.golang.org/grpc/status"','']

> Fixer
DEBU [importissue] fixer: Fix issue &result.Issue{FromLinter:"gci", Text:"File is not `gci`-ed with -local github.com/lyft/clutch/backend", Severity:"", SourceLines:[]string{"\t\"google.golang.org/grpc/codes\"", "\t\"google.golang.org/grpc/status\""}, Replacement:(*result.Replacement)(0xc01f0707d0), Pkg:(*packages.Package)(0xc002729b00), LineRange:(*result.Range)(0xc01f0707c0), Pos:token.Position{Filename:"middleware/audit/audit.go", Offset:0, Line:8, Column:0}, HunkPos:0, ExpectNoLint:false, ExpectedNoLintLinter:""} with range 8-9 
DEBU [importissue] fixer: Fix issue &result.Issue{FromLinter:"goimports", Text:"File is not `goimports`-ed with -local github.com/lyft/clutch/backend", Severity:"", SourceLines:[]string{""}, Replacement:(*result.Replacement)(0xc047117210), Pkg:(*packages.Package)(0xc002729b00), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"middleware/audit/audit.go", Offset:0, Line:13, Column:0}, HunkPos:0, ExpectNoLint:false, ExpectedNoLintLinter:""} with range 13-13 
DEBU [importissue] fixer: Fix issue &result.Issue{FromLinter:"gci", Text:"File is not `gci`-ed with -local github.com/lyft/clutch/backend", Severity:"", SourceLines:[]string{"\t\"google.golang.org/grpc\""}, Replacement:(*result.Replacement)(0xc01f070850), Pkg:(*packages.Package)(0xc002729b00), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"middleware/audit/audit.go", Offset:0, Line:16, Column:0}, HunkPos:0, ExpectNoLint:false, ExpectedNoLintLinter:""} with range 16-16

danielhochman · 2021-03-09T17:42:56Z

So far, I'm pretty sure the issue is related to one or all of these code paths:

extractIssuesFromPatch, which analyzes the diff from the linter output to determine what the affected lines are and what action is necessary on those lines
mergeLineIssues (fixer call, func), where the fixer tries to merge all issues from all linters together for a single file
findNotIntersectingIssues (fixer call, func), where the fixer tries to break fixes up into multiple passes? not 100% sure on that yet

sheldonhull · 2021-09-29T20:49:29Z

I recently ran into this as well, and finally found this issue. Thought I was conflicting with other tooling but I've confirmed this behavior is occurring directly when invoked. I tried running golangci-lint run --fix --enable gci,gofumpt --fast --allow-parallel-runners=false to see if it would help with duplicates but no results. Will have to revert to manually invoking commands.

nightlyone · 2022-05-19T17:52:15Z

Hi @ldez isnt this fixed by PR #2653?

After merging PR #2653 both linters should be compatible.

ldez · 2022-05-19T20:00:48Z

no #2653 will not fix this issue. Since v1.45.0, the support of the autofix with gci has been dropped. #2653 just restore the previous behavior and then the bug.

yngvark · 2022-07-18T09:03:26Z

While this is being fixed, my workaround to remove this error is to put the following in my .golangci.yaml:

linters-settings:
    gci:
        sections:
            - standard
            - default
            - prefix(github.com/ossf/scorecard) # Replace with your module name

I found this fix here: https://github.com/ossf/scorecard/pull/1664/files.

I'm using mvdan.cc/[email protected] and [email protected].

The error I'm getting without the fix above:

gofumpt -l -w ./main.go ./pkg/hello/hello_test.go ./pkg/hello/hello.go
golangci-lint run ./...
pkg/hello/hello_test.go:7:1: Expected '\t', Found '\n' at pkg/hello/hello_test.go[line 7,col 1] (gci)

Lpaydat · 2022-10-03T09:13:39Z

I'm not sure if this will help you all, but I added the mentioned flags to the VSCode settings and the warning has gone away.

  "go.formatTool": "gofumpt",
  "go.formatFlags": ["-s"],
  "go.lintTool": "golangci-lint",
  "go.lintFlags": ["--skip-generated", "-s standard,default"],

P.S. I used gci and gofumpt

kluevandrew · 2022-11-17T20:41:30Z

Same for me, but i don't have any small example app

Crocmagnon · 2023-07-21T16:33:17Z

gci + gofumpt OK

I'm getting conflicts between these two in version 1.53.3

golangci-lint run --disable-all --enable gci,gofumpt --fix

in:

import (
	"github.com/Crocmagnon/snippetbox/ui"
	"net/http"
)

out:

import (
	"net/http"

	"net/http"

	"github.com/Crocmagnon/snippetbox/ui"
)

jalaziz · 2023-11-19T03:20:30Z

Running into this issue as well 😞. It seems like the correct thing to do here is to enforce an ordering of linters, applying fixes before running the next linter. gci recommends running goimports before gci.

Unfortunately, it doesn't seem like there's a way to enforce linter ordering with golangci-lint.

piepmatz · 2024-04-26T10:15:09Z

Workaround:
golangci-lint v1.57.0 introduced the --enable-only flag (#4438).
After fixing only the imports first (e.g. using gci), the next run with the usual config works.

golangci-lint run --enable-only gci --fix
golangci-lint run --fix

sheldonhull · 2024-04-26T17:52:01Z

What I do is disable both of those and use gofumpt only as it includes import sort already try that and maybe you'll no longer have a competing format.

therearesomewhocallmetim · 2024-10-17T11:27:08Z

I'm still having this issue with golangci-lint v1.61.0. Is there any workaround or fix?

bombsimon · 2024-10-17T11:51:01Z

I'm still having this issue with golangci-lint v1.61.0. Is there any workaround or fix?

Yeah the issue is still open so still active. A workaround is mentioned in #1490 (comment)

therearesomewhocallmetim · 2024-10-17T11:52:33Z

I seem to have fixed it. It looks like it was caused by having a wrong prefix in goimports.local-prefixes: I copied my config from another project and didn't fix all the configurations. Hope it helps

sayboras added bug Something isn't working good first issue Good for newcomers help wanted Issue that needs help from a contributor labels Nov 3, 2020

ldez mentioned this issue Feb 14, 2021

Using --fix option results in panic: runtime error: index out of range [146] with length 146 #1726

Closed

This was referenced Mar 27, 2021

typecheck: possible wrong cut #1873

Closed

Multiple Issues don't seem to play nice together. #1874

Closed

This comment has been minimized.

Sign in to view

ldez mentioned this issue Apr 29, 2021

Conflicts gofumpt and gci for imports #1942

Closed

4 tasks

nightlyone mentioned this issue May 19, 2022

Support SuggestedFixes #2609

Closed

sleepymole mentioned this issue Oct 13, 2022

Add gci to format imports pingcap/tiflow#7347

Closed

bombsimon mentioned this issue Jan 4, 2023

Running golangci-lint cache clean ; golangci-lint run --fix > /dev/null alternates between "fixed" and "not fixed" formats #3454

Closed

4 tasks

bombsimon mentioned this issue May 12, 2023

golangci-lint run --fix generates broken code #3819

Closed

4 tasks

ldez removed the help wanted Issue that needs help from a contributor label Jul 21, 2023

ldez removed the good first issue Good for newcomers label Jul 21, 2023

Antonboom mentioned this issue Oct 1, 2023

Conflict gofumpt and wsl for empty lines before if statement #1510

Closed

3 tasks

golangci deleted a comment from Antonboom Oct 1, 2023

tranngoclam mentioned this issue Feb 10, 2024

style: enable gci and gofumpt linters openfga/openfga#1357

Closed

4 tasks

ldez mentioned this issue Jul 24, 2024

gci and gofumpt fix to duplicate imports #4885

Closed

7 tasks

scop mentioned this issue Jul 24, 2024

Option to disable imports processing mvdan/gofumpt#308

Open

wulianglongrd mentioned this issue Sep 3, 2024

gencheck_istio output has duplicate imports istio/istio#45595

Closed

bombsimon mentioned this issue Dec 10, 2024

Add support for SuggestedFixes #1779

Closed

ldez mentioned this issue Dec 16, 2024

feat: add support for SuggestedFixes #5232

Merged

ldez closed this as completed in #5232 Dec 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gci+goimports = duplicated imports #1490

gci+goimports = duplicated imports #1490

howardjohn commented Nov 3, 2020

howardjohn commented Nov 3, 2020

howardjohn commented Nov 3, 2020

ldez commented Feb 14, 2021 •

edited

Loading

dbraley commented Mar 9, 2021

danielhochman commented Mar 9, 2021 •

edited by ldez

Loading

danielhochman commented Mar 9, 2021 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

sheldonhull commented Sep 29, 2021

nightlyone commented May 19, 2022

ldez commented May 19, 2022

yngvark commented Jul 18, 2022

Lpaydat commented Oct 3, 2022 •

edited

Loading

kluevandrew commented Nov 17, 2022

Crocmagnon commented Jul 21, 2023 •

edited by ldez

Loading

jalaziz commented Nov 19, 2023 •

edited

Loading

piepmatz commented Apr 26, 2024

sheldonhull commented Apr 26, 2024

therearesomewhocallmetim commented Oct 17, 2024

bombsimon commented Oct 17, 2024

therearesomewhocallmetim commented Oct 17, 2024

gci+goimports = duplicated imports #1490

gci+goimports = duplicated imports #1490

Comments

howardjohn commented Nov 3, 2020

howardjohn commented Nov 3, 2020

howardjohn commented Nov 3, 2020

ldez commented Feb 14, 2021 • edited Loading

dbraley commented Mar 9, 2021

danielhochman commented Mar 9, 2021 • edited by ldez Loading

danielhochman commented Mar 9, 2021 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

sheldonhull commented Sep 29, 2021

nightlyone commented May 19, 2022

ldez commented May 19, 2022

yngvark commented Jul 18, 2022

Lpaydat commented Oct 3, 2022 • edited Loading

kluevandrew commented Nov 17, 2022

Crocmagnon commented Jul 21, 2023 • edited by ldez Loading

jalaziz commented Nov 19, 2023 • edited Loading

piepmatz commented Apr 26, 2024

sheldonhull commented Apr 26, 2024

therearesomewhocallmetim commented Oct 17, 2024

bombsimon commented Oct 17, 2024

therearesomewhocallmetim commented Oct 17, 2024

ldez commented Feb 14, 2021 •

edited

Loading

danielhochman commented Mar 9, 2021 •

edited by ldez

Loading

danielhochman commented Mar 9, 2021 •

edited

Loading

Lpaydat commented Oct 3, 2022 •

edited

Loading

Crocmagnon commented Jul 21, 2023 •

edited by ldez

Loading

jalaziz commented Nov 19, 2023 •

edited

Loading