From 0d3f690eb1d28d0f46d3b00a28a3143ca115ea4c Mon Sep 17 00:00:00 2001 From: Gabor Javorszky Date: Fri, 17 Feb 2023 23:36:44 +0000 Subject: [PATCH 1/6] Added linter specific testdata file --- test/testdata/responsewriterlint.go | 66 +++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 test/testdata/responsewriterlint.go diff --git a/test/testdata/responsewriterlint.go b/test/testdata/responsewriterlint.go new file mode 100644 index 000000000000..6e27ee927e63 --- /dev/null +++ b/test/testdata/responsewriterlint.go @@ -0,0 +1,66 @@ +//golangcitest:args -Eresponsewriterlint +package testdata + +import ( + "errors" + "fmt" + "net/http" +) + +type notAResponseWriter struct{} + +func (narw notAResponseWriter) Write(in []byte) (int, error) { + // actually do nothing + return 42, errors.New("no") +} + +func (narw notAResponseWriter) WriteHeader(code int) { + // also do nothing +} + +func (narw notAResponseWriter) Header() http.Header { + return http.Header{} +} + +type rwlRandom struct{} + +func rwlExampleOne(w http.ResponseWriter, r *http.Request) { + w.Header().Add("some header", "value") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`boys in the yard`)) +} + +func rwlExampleTwo(s string, r *http.Request) error { + fmt.Printf("this is a thing") + + return nil +} + +func rwlFakeWriter(w notAResponseWriter) { + w.Header().Add("something", "other") + w.WriteHeader(420) + _, _ = w.Write([]byte(`fooled ya`)) +} + +func (b rwlRandom) method(w http.ResponseWriter, r *http.Request) { + w.Header().Add("some header", "value") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`boys in the yard`)) +} + +func (b *rwlRandom) methodPointer(w http.ResponseWriter, r *http.Request) { + w.Header().Add("some header", "value") + _, _ = w.Write([]byte(`boys in the yard`)) + w.WriteHeader(http.StatusOK) // want "function methodPointer: http.ResponseWriter.Write is called before http.ResponseWriter.WriteHeader. Headers are already sent, this has no effect." +} + +func rwlExampleThree(bloe http.ResponseWriter, r *http.Request) { + _, _ = bloe.Write([]byte(`hellyea`)) // want "function rwlExampleThree: Multiple calls to http.ResponseWriter.Write in the same function body. This is most probably a bug." + + bloe.WriteHeader(http.StatusBadRequest) // want "function rwlExampleThree: Multiple calls to http.ResponseWriter.WriteHeader in the same function body. This is most probably a bug." + _, _ = bloe.Write([]byte(`hellyelamdflmda`)) // want "function rwlExampleThree: Multiple calls to http.ResponseWriter.Write in the same function body. This is most probably a bug." + bloe.WriteHeader(http.StatusInternalServerError) // want "function rwlExampleThree: Multiple calls to http.ResponseWriter.WriteHeader in the same function body. This is most probably a bug." + + bloe.Header().Set("help", "somebody") // want "function rwlExampleThree: http.ResponseWriter.Header called after calling http.ResponseWriter.Write. This has no effect." "function rwlExampleThree: http.ResponseWriter.Header called after calling http.ResponseWriter.Write. This has no effect." "function rwlExampleThree: http.ResponseWriter.Header called after calling http.ResponseWriter.WriteHeader. This has no effect." "function rwlExampleThree: http.ResponseWriter.Header called after calling http.ResponseWriter.WriteHeader. This has no effect." + bloe.Header().Set("dddd", "someboddaady") // want "function rwlExampleThree: http.ResponseWriter.Header called after calling http.ResponseWriter.Write. This has no effect." "function rwlExampleThree: http.ResponseWriter.Header called after calling http.ResponseWriter.Write. This has no effect." "function rwlExampleThree: http.ResponseWriter.Header called after calling http.ResponseWriter.WriteHeader. This has no effect." "function rwlExampleThree: http.ResponseWriter.Header called after calling http.ResponseWriter.WriteHeader. This has no effect." +} From 17eb3fcee99879e440acd257d412c5a662f6c80f Mon Sep 17 00:00:00 2001 From: Gabor Javorszky Date: Sat, 18 Feb 2023 13:40:07 +0000 Subject: [PATCH 2/6] Add all configs, structs and entries that the linter needs --- .golangci.reference.yml | 1 + go.mod | 1 + go.sum | 2 ++ pkg/golinters/responsewriterlint.go | 19 +++++++++++++++++++ pkg/lint/lintersdb/manager.go | 6 ++++++ 5 files changed, 29 insertions(+) create mode 100644 pkg/golinters/responsewriterlint.go diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 7de210b98ba4..d98c0cf52730 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -2072,6 +2072,7 @@ linters: - predeclared - promlinter - reassign + - responsewriterlint - revive - rowserrcheck - scopelint diff --git a/go.mod b/go.mod index 84e4478593de..513942a57bc5 100644 --- a/go.mod +++ b/go.mod @@ -48,6 +48,7 @@ require ( github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-version v1.6.0 github.com/hexops/gotextdiff v1.0.3 + github.com/javorszky/go-responsewriter-lint v0.1.2 github.com/jgautheron/goconst v1.5.1 github.com/jingyugao/rowserrcheck v1.1.1 github.com/jirfag/go-printf-func-name v0.0.0-20200119135958-7558a9eaa5af diff --git a/go.sum b/go.sum index 8bde49ab6f8c..363096f3454a 100644 --- a/go.sum +++ b/go.sum @@ -292,6 +292,8 @@ github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1: github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/inconshreveable/mousetrap v1.0.1 h1:U3uMjPSQEBMNp1lFxmllqCPM6P5u/Xq7Pgzkat/bFNc= github.com/inconshreveable/mousetrap v1.0.1/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/javorszky/go-responsewriter-lint v0.1.2 h1:tidNkspygGMNpulecVWku/rzt+KkRhu+LuPLgWVjUAo= +github.com/javorszky/go-responsewriter-lint v0.1.2/go.mod h1:n+m48J/7g3XTWeEE3lnG/hSPVwRPo9ZgT8xpzLu86ks= github.com/jgautheron/goconst v1.5.1 h1:HxVbL1MhydKs8R8n/HE5NPvzfaYmQJA3o879lE4+WcM= github.com/jgautheron/goconst v1.5.1/go.mod h1:aAosetZ5zaeC/2EfMeRswtxUFBpe2Hr7HzkgX4fanO4= github.com/jingyugao/rowserrcheck v1.1.1 h1:zibz55j/MJtLsjP1OF4bSdgXxwL1b+Vn7Tjzq7gFzUs= diff --git a/pkg/golinters/responsewriterlint.go b/pkg/golinters/responsewriterlint.go new file mode 100644 index 000000000000..550f6d953760 --- /dev/null +++ b/pkg/golinters/responsewriterlint.go @@ -0,0 +1,19 @@ +package golinters + +import ( + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" + "github.com/javorszky/go-responsewriter-lint/pkg/analyzer" + "golang.org/x/tools/go/analysis" +) + +func NewResponseWriterLint(_ *config.ReassignSettings) *goanalysis.Linter { + a := analyzer.New() + + return goanalysis.NewLinter( + a.Name, + a.Doc, + []*analysis.Analyzer{a}, + nil, + ).WithLoadMode(goanalysis.LoadModeTypesInfo) +} diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 6f406f7d26e3..f7270dee0f07 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -722,6 +722,12 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithLoadForGoAnalysis(). WithURL("https://github.com/curioswitch/go-reassign"), + linter.NewConfig(golinters.NewResponseWriterLint(nil)). + WithSince("1.52.0"). + WithPresets(linter.PresetBugs). + ConsiderSlow(). + WithURL("https://github.com/javorszky/go-responsewriter-lint"), + linter.NewConfig(golinters.NewRevive(reviveCfg)). WithSince("v1.37.0"). WithPresets(linter.PresetStyle, linter.PresetMetaLinter). From 30c940361c0d5c8f337121e028671959c1173934 Mon Sep 17 00:00:00 2001 From: Gabor Javorszky Date: Sat, 18 Feb 2023 14:02:47 +0000 Subject: [PATCH 3/6] Update pkg/lint/lintersdb/manager.go Accepted suggestion. Co-authored-by: Ludovic Fernandez --- pkg/lint/lintersdb/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index f7270dee0f07..56ffe0866c14 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -725,7 +725,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { linter.NewConfig(golinters.NewResponseWriterLint(nil)). WithSince("1.52.0"). WithPresets(linter.PresetBugs). - ConsiderSlow(). + WithLoadForGoAnalysis(). WithURL("https://github.com/javorszky/go-responsewriter-lint"), linter.NewConfig(golinters.NewRevive(reviveCfg)). From 395eef33b7e43e01bd8edebda3ce7ffdd8811ecb Mon Sep 17 00:00:00 2001 From: Gabor Javorszky Date: Sat, 18 Feb 2023 20:17:26 +0000 Subject: [PATCH 4/6] Rework imports to goimports is happy --- pkg/golinters/responsewriterlint.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/golinters/responsewriterlint.go b/pkg/golinters/responsewriterlint.go index 550f6d953760..407f9a0aa7fa 100644 --- a/pkg/golinters/responsewriterlint.go +++ b/pkg/golinters/responsewriterlint.go @@ -1,10 +1,11 @@ package golinters import ( - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/javorszky/go-responsewriter-lint/pkg/analyzer" "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" ) func NewResponseWriterLint(_ *config.ReassignSettings) *goanalysis.Linter { From 09734de9ac33626d71471d501279f2f52afecc4c Mon Sep 17 00:00:00 2001 From: Gabor Javorszky Date: Sun, 19 Feb 2023 11:23:32 +0000 Subject: [PATCH 5/6] Don't pass in nil config, especially not reassign's if linter isn't using it Good catch @leonklingele, thank you! Co-authored-by: leonklingele --- pkg/golinters/responsewriterlint.go | 2 +- pkg/lint/lintersdb/manager.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/golinters/responsewriterlint.go b/pkg/golinters/responsewriterlint.go index 407f9a0aa7fa..7de2ab8917ba 100644 --- a/pkg/golinters/responsewriterlint.go +++ b/pkg/golinters/responsewriterlint.go @@ -8,7 +8,7 @@ import ( "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" ) -func NewResponseWriterLint(_ *config.ReassignSettings) *goanalysis.Linter { +func NewResponseWriterLint() *goanalysis.Linter { a := analyzer.New() return goanalysis.NewLinter( diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 56ffe0866c14..97de628ee939 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -722,7 +722,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithLoadForGoAnalysis(). WithURL("https://github.com/curioswitch/go-reassign"), - linter.NewConfig(golinters.NewResponseWriterLint(nil)). + linter.NewConfig(golinters.NewResponseWriterLint()). WithSince("1.52.0"). WithPresets(linter.PresetBugs). WithLoadForGoAnalysis(). From 75019a3d74f20fdd9c7f267ee19089ee2fa29987 Mon Sep 17 00:00:00 2001 From: Gabor Javorszky Date: Sun, 19 Feb 2023 11:25:55 +0000 Subject: [PATCH 6/6] Remove unused config import --- pkg/golinters/responsewriterlint.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/golinters/responsewriterlint.go b/pkg/golinters/responsewriterlint.go index 7de2ab8917ba..365967de56b4 100644 --- a/pkg/golinters/responsewriterlint.go +++ b/pkg/golinters/responsewriterlint.go @@ -4,7 +4,6 @@ import ( "github.com/javorszky/go-responsewriter-lint/pkg/analyzer" "golang.org/x/tools/go/analysis" - "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" )