From edb5783ee5e981d269d4f0a57fd644109af0de87 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 2 Jan 2022 18:34:18 +0300 Subject: [PATCH 1/6] add quickfix for gocritic --- pkg/golinters/gocritic.go | 19 ++++++++++++++++--- test/ruleguard/rangeExprCopy.go | 14 ++++++++++++++ test/testdata/configs/gocritic.yml | 2 +- test/testdata/fix/in/gocritic.go | 11 +++++++++++ test/testdata/fix/out/gocritic.go | 11 +++++++++++ 5 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 test/ruleguard/rangeExprCopy.go create mode 100644 test/testdata/fix/in/gocritic.go create mode 100644 test/testdata/fix/out/gocritic.go diff --git a/pkg/golinters/gocritic.go b/pkg/golinters/gocritic.go index d292aacd71ae..94b091f02f95 100644 --- a/pkg/golinters/gocritic.go +++ b/pkg/golinters/gocritic.go @@ -48,8 +48,9 @@ Dynamic rules are written declaratively with AST patterns, filters, report messa } linterCtx.SetPackageInfo(pass.TypesInfo, pass.Pkg) - var res []goanalysis.Issue pkgIssues := runGocriticOnPackage(linterCtx, enabledCheckers, pass.Files) + res := make([]goanalysis.Issue, 0, len(pkgIssues)) + for i := range pkgIssues { res = append(res, goanalysis.NewIssue(&pkgIssues[i], pass)) } @@ -179,11 +180,23 @@ func runGocriticOnFile(ctx *gocriticlinter.Context, f *ast.File, checkers []*goc // as read-only structure, so no copying is required. for _, warn := range c.Check(f) { pos := ctx.FileSet.Position(warn.Node.Pos()) - res = append(res, result.Issue{ + issue := result.Issue{ Pos: pos, Text: fmt.Sprintf("%s: %s", c.Info.Name, warn.Text), FromLinter: gocriticName, - }) + } + + if warn.HasQuickFix() { + issue.Replacement = &result.Replacement{ + Inline: &result.InlineFix{ + StartCol: pos.Column - 1, + Length: int(warn.Node.End()) - int(warn.Node.Pos()), + NewString: string(warn.Suggestion.Replacement), + }, + } + } + + res = append(res, issue) } } diff --git a/test/ruleguard/rangeExprCopy.go b/test/ruleguard/rangeExprCopy.go new file mode 100644 index 000000000000..9b63adae970d --- /dev/null +++ b/test/ruleguard/rangeExprCopy.go @@ -0,0 +1,14 @@ +// go:build ruleguard +package ruleguard + +import ( + "github.com/quasilyte/go-ruleguard/dsl" +) + +func rangeExprVal(m dsl.Matcher) { + m.Match(`for _, $_ := range $x { $*_ }`, `for _, $_ = range $x { $*_ }`). + Where(m["x"].Addressable && m["x"].Type.Size >= 512). + Report(`$x copy can be avoided with &$x`). + At(m["x"]). + Suggest(`&$x`) +} diff --git a/test/testdata/configs/gocritic.yml b/test/testdata/configs/gocritic.yml index 5cdda3736af5..d1f71ba5663e 100644 --- a/test/testdata/configs/gocritic.yml +++ b/test/testdata/configs/gocritic.yml @@ -16,4 +16,4 @@ linters-settings: # The ruleguard files used in functional tests cannot be under the 'testdata' directory. # This is because they import the 'github.com/quasilyte/go-ruleguard/dsl' package, # which needs to be added to go.mod. The testdata directory is ignored by go mod. - rules: '${configDir}/../../ruleguard/strings_simplify.go,${configDir}/../../ruleguard/dup.go' + rules: '${configDir}/../../ruleguard/strings_simplify.go,${configDir}/../../ruleguard/dup.go,${configDir}/../../ruleguard/rangeExprCopy.go' diff --git a/test/testdata/fix/in/gocritic.go b/test/testdata/fix/in/gocritic.go new file mode 100644 index 000000000000..eaa3236ea8e7 --- /dev/null +++ b/test/testdata/fix/in/gocritic.go @@ -0,0 +1,11 @@ +//args: -Egocritic +package p + +func gocritic() { + var xs [2048]byte + + // xs -> &xs + for _, x := range &xs { + print(x) + } +} diff --git a/test/testdata/fix/out/gocritic.go b/test/testdata/fix/out/gocritic.go new file mode 100644 index 000000000000..eaa3236ea8e7 --- /dev/null +++ b/test/testdata/fix/out/gocritic.go @@ -0,0 +1,11 @@ +//args: -Egocritic +package p + +func gocritic() { + var xs [2048]byte + + // xs -> &xs + for _, x := range &xs { + print(x) + } +} From 9e5e0c936dbdbac7ebeece8af4a29fe2cf88d9a0 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 2 Jan 2022 18:52:46 +0300 Subject: [PATCH 2/6] fix test --- test/testdata/fix/in/gocritic.go | 3 ++- test/testdata/fix/out/gocritic.go | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test/testdata/fix/in/gocritic.go b/test/testdata/fix/in/gocritic.go index eaa3236ea8e7..f713d3e7d7af 100644 --- a/test/testdata/fix/in/gocritic.go +++ b/test/testdata/fix/in/gocritic.go @@ -1,11 +1,12 @@ //args: -Egocritic +//config_path: testdata/configs/gocritic.yml package p func gocritic() { var xs [2048]byte // xs -> &xs - for _, x := range &xs { + for _, x := range xs { print(x) } } diff --git a/test/testdata/fix/out/gocritic.go b/test/testdata/fix/out/gocritic.go index eaa3236ea8e7..d2e2b4112eea 100644 --- a/test/testdata/fix/out/gocritic.go +++ b/test/testdata/fix/out/gocritic.go @@ -1,4 +1,5 @@ //args: -Egocritic +//config_path: testdata/configs/gocritic.yml package p func gocritic() { From 584664068df165ad5b358945e7b2d192252afafa Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 2 Jan 2022 19:18:57 +0300 Subject: [PATCH 3/6] update rule name --- test/ruleguard/rangeExprCopy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ruleguard/rangeExprCopy.go b/test/ruleguard/rangeExprCopy.go index 9b63adae970d..d68b45e61158 100644 --- a/test/ruleguard/rangeExprCopy.go +++ b/test/ruleguard/rangeExprCopy.go @@ -5,7 +5,7 @@ import ( "github.com/quasilyte/go-ruleguard/dsl" ) -func rangeExprVal(m dsl.Matcher) { +func RangeExprVal(m dsl.Matcher) { m.Match(`for _, $_ := range $x { $*_ }`, `for _, $_ = range $x { $*_ }`). Where(m["x"].Addressable && m["x"].Type.Size >= 512). Report(`$x copy can be avoided with &$x`). From ef1d7dca19fd035ae07d560c4a8aa03117aaa700 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 2 Jan 2022 20:45:44 +0300 Subject: [PATCH 4/6] fix test config --- Makefile | 4 ++++ test/testdata/configs/gocritic.yml | 2 +- test/testdata/fix/in/gocritic.go | 3 ++- test/testdata/fix/out/gocritic.go | 3 ++- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 1f94ecb691be..d70b396aad34 100644 --- a/Makefile +++ b/Makefile @@ -29,6 +29,10 @@ test: build GL_TEST_RUN=1 go test -v -parallel 2 ./... .PHONY: test +test_fix: build + GL_TEST_RUN=1 go test -v ./test -count 1 -run TestFix/$T +.PHONY: test_fix + test_race: build_race GL_TEST_RUN=1 ./golangci-lint run -v --timeout=5m .PHONY: test_race diff --git a/test/testdata/configs/gocritic.yml b/test/testdata/configs/gocritic.yml index d1f71ba5663e..5cdda3736af5 100644 --- a/test/testdata/configs/gocritic.yml +++ b/test/testdata/configs/gocritic.yml @@ -16,4 +16,4 @@ linters-settings: # The ruleguard files used in functional tests cannot be under the 'testdata' directory. # This is because they import the 'github.com/quasilyte/go-ruleguard/dsl' package, # which needs to be added to go.mod. The testdata directory is ignored by go mod. - rules: '${configDir}/../../ruleguard/strings_simplify.go,${configDir}/../../ruleguard/dup.go,${configDir}/../../ruleguard/rangeExprCopy.go' + rules: '${configDir}/../../ruleguard/strings_simplify.go,${configDir}/../../ruleguard/dup.go' diff --git a/test/testdata/fix/in/gocritic.go b/test/testdata/fix/in/gocritic.go index f713d3e7d7af..423bc0c52a07 100644 --- a/test/testdata/fix/in/gocritic.go +++ b/test/testdata/fix/in/gocritic.go @@ -1,5 +1,6 @@ //args: -Egocritic -//config_path: testdata/configs/gocritic.yml +//config: linters-settings.gocritic.enabled-checks=ruleguard +//config: linters-settings.gocritic.settings.ruleguard.rules=ruleguard/rangeExprCopy.go package p func gocritic() { diff --git a/test/testdata/fix/out/gocritic.go b/test/testdata/fix/out/gocritic.go index d2e2b4112eea..714359dda13b 100644 --- a/test/testdata/fix/out/gocritic.go +++ b/test/testdata/fix/out/gocritic.go @@ -1,5 +1,6 @@ //args: -Egocritic -//config_path: testdata/configs/gocritic.yml +//config: linters-settings.gocritic.enabled-checks=ruleguard +//config: linters-settings.gocritic.settings.ruleguard.rules=ruleguard/rangeExprCopy.go package p func gocritic() { From a473907d0d922eeed734abd20277e30bb5480ffb Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 2 Jan 2022 20:59:29 +0300 Subject: [PATCH 5/6] add test cases --- test/testdata/fix/in/gocritic.go | 11 ++++++++++- test/testdata/fix/out/gocritic.go | 11 ++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/test/testdata/fix/in/gocritic.go b/test/testdata/fix/in/gocritic.go index 423bc0c52a07..4191eea1319a 100644 --- a/test/testdata/fix/in/gocritic.go +++ b/test/testdata/fix/in/gocritic.go @@ -1,8 +1,12 @@ //args: -Egocritic //config: linters-settings.gocritic.enabled-checks=ruleguard -//config: linters-settings.gocritic.settings.ruleguard.rules=ruleguard/rangeExprCopy.go +//config: linters-settings.gocritic.settings.ruleguard.rules=ruleguard/rangeExprCopy.go,ruleguard/strings_simplify.go package p +import ( + "strings" +) + func gocritic() { var xs [2048]byte @@ -10,4 +14,9 @@ func gocritic() { for _, x := range xs { print(x) } + + // strings.Count("foo", "bar") == 0 -> !strings.Contains("foo", "bar") + if strings.Count("foo", "bar") == 0 { + print("qu") + } } diff --git a/test/testdata/fix/out/gocritic.go b/test/testdata/fix/out/gocritic.go index 714359dda13b..2c66ef21b9f5 100644 --- a/test/testdata/fix/out/gocritic.go +++ b/test/testdata/fix/out/gocritic.go @@ -1,8 +1,12 @@ //args: -Egocritic //config: linters-settings.gocritic.enabled-checks=ruleguard -//config: linters-settings.gocritic.settings.ruleguard.rules=ruleguard/rangeExprCopy.go +//config: linters-settings.gocritic.settings.ruleguard.rules=ruleguard/rangeExprCopy.go,ruleguard/strings_simplify.go package p +import ( + "strings" +) + func gocritic() { var xs [2048]byte @@ -10,4 +14,9 @@ func gocritic() { for _, x := range &xs { print(x) } + + // strings.Count("foo", "bar") == 0 -> !strings.Contains("foo", "bar") + if !strings.Contains("foo", "bar") { + print("qu") + } } From 15ec9ab9f7e693cbac0977808aae9e1a1202c516 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 3 Jan 2022 22:44:29 +0100 Subject: [PATCH 6/6] review --- Makefile | 2 ++ pkg/golinters/gocritic.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index d70b396aad34..1ba54cb23ebe 100644 --- a/Makefile +++ b/Makefile @@ -29,6 +29,8 @@ test: build GL_TEST_RUN=1 go test -v -parallel 2 ./... .PHONY: test +# ex: T=gofmt.go make test_fix +# the value of `T` is the name of a file from `test/testdata/fix` test_fix: build GL_TEST_RUN=1 go test -v ./test -count 1 -run TestFix/$T .PHONY: test_fix diff --git a/pkg/golinters/gocritic.go b/pkg/golinters/gocritic.go index 94b091f02f95..ea3a3cbcbe8b 100644 --- a/pkg/golinters/gocritic.go +++ b/pkg/golinters/gocritic.go @@ -190,7 +190,7 @@ func runGocriticOnFile(ctx *gocriticlinter.Context, f *ast.File, checkers []*goc issue.Replacement = &result.Replacement{ Inline: &result.InlineFix{ StartCol: pos.Column - 1, - Length: int(warn.Node.End()) - int(warn.Node.Pos()), + Length: int(warn.Node.End() - warn.Node.Pos()), NewString: string(warn.Suggestion.Replacement), }, }