golangci-lint
golangci-lint copied to clipboard
gci+goimports = duplicated imports
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
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"
)
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
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.
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.
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
![](https://user-images.githubusercontent.com/4712430/110511627-03c62b00-80ca-11eb-8cca-ea2800a9368c.png)
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
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
@danielhochman, trying to understand:
This is in fixer.go
, would erroneous behavior in those three functions trigger if I weren't using --fix
?
Ignore this comment, something is weirder, I have to do more research
the fixer is only used when --fix
is called.
edit: I removed my previous comment because I misread your comment.
the fixer is only used when
--fix
is called.edit: I removed my previous comment because I misread your comment.
In which case, as all functions you mentioned are in fixer.go
I am not sure the behaviour is related.
I have to do some more research, but I did notice some very strange behaviour when I started doing A/B tests.
I'll have a nice little spreadsheet for you tomorrow. :-)
Ok so I think it's off-topic, you can continue in https://github.com/golangci/golangci-lint/discussions/1875
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.
Hi @ldez isnt this fixed by PR #2653?
After merging PR #2653 both linters should be compatible.
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.
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)
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
Same for me, but i don't have any small example app
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"
)
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
.
Workaround:
golangci-lint v1.57.0 introduced the --enable-only
flag (https://github.com/golangci/golangci-lint/pull/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
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.