Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New linter: Go responsewriter lint #3614

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.reference.yml
Expand Up @@ -2072,6 +2072,7 @@ linters:
- predeclared
- promlinter
- reassign
- responsewriterlint
- revive
- rowserrcheck
- scopelint
Expand Down
1 change: 1 addition & 0 deletions go.mod
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions 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 {
javorszky marked this conversation as resolved.
Show resolved Hide resolved
a := analyzer.New()

return goanalysis.NewLinter(
a.Name,
a.Doc,
[]*analysis.Analyzer{a},
nil,
).WithLoadMode(goanalysis.LoadModeTypesInfo)
}
6 changes: 6 additions & 0 deletions pkg/lint/lintersdb/manager.go
Expand Up @@ -722,6 +722,12 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
WithLoadForGoAnalysis().
WithURL("https://github.com/curioswitch/go-reassign"),

linter.NewConfig(golinters.NewResponseWriterLint(nil)).
javorszky marked this conversation as resolved.
Show resolved Hide resolved
WithSince("1.52.0").
WithPresets(linter.PresetBugs).
ConsiderSlow().
javorszky marked this conversation as resolved.
Show resolved Hide resolved
WithURL("https://github.com/javorszky/go-responsewriter-lint"),

linter.NewConfig(golinters.NewRevive(reviveCfg)).
WithSince("v1.37.0").
WithPresets(linter.PresetStyle, linter.PresetMetaLinter).
Expand Down
66 changes: 66 additions & 0 deletions 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."
}