new checker http-const

Antonboom · Jun 25, 2024 · 2a080ae · 2a080ae
1 parent cea8c9c
commit 2a080ae
Show file tree

Hide file tree

Showing 12 changed files with 439 additions and 44 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -138,7 +138,6 @@ Describe a new checker in [checkers section](./README.md#checkers).
 - [equal-values](#equal-values)
 - [graceful-teardown](#graceful-teardown)
 - [float-compare](#float-compare)
-- [http-const](#http-const)
 - [http-sugar](#http-sugar)
 - [require-len](#require-len)
 - [suite-test-name](#suite-test-name)
@@ -268,24 +267,6 @@ And similar idea for `assert.InEpsilonSlice` / `assert.InDeltaSlice`.
 
 ---
 
-### http-const
-
-```go
-❌   assert.HTTPStatusCode(t, handler, "GET", "/index", nil, 200)
-     assert.HTTPBodyContains(t, handler, "GET", "/index", nil, "counter")
-     // etc.
-
-✅   assert.HTTPStatusCode(t, handler, http.MethodGet, "/index", nil, http.StatusOK)
-     assert.HTTPBodyContains(t, handler, http.MethodGet, "/index", nil, "counter")
-```
-
-**Autofix**: true. <br>
-**Enabled by default**: true. <br>
-**Reason**: Is similar to the [usestdlibvars](https://golangci-lint.run/usage/linters/#usestdlibvars) linter. <br>
-**Related issues**: [#141](https://github.com/Antonboom/testifylint/issues/141)
-
----
-
 ### http-sugar
 
 ```go

diff --git a/README.md b/README.md
@@ -91,27 +91,28 @@ https://golangci-lint.run/usage/linters/#testifylint
 
 | Name                                                | Enabled By Default | Autofix |
 |-----------------------------------------------------|--------------------|---------|
-| [blank-import](#blank-import)                       | ✅                  | ❌       |
-| [bool-compare](#bool-compare)                       | ✅                  | ✅       |
-| [compares](#compares)                               | ✅                  | ✅       |
-| [empty](#empty)                                     | ✅                  | ✅       |
-| [error-is-as](#error-is-as)                         | ✅                  | 🤏      |
-| [error-nil](#error-nil)                             | ✅                  | ✅       |
-| [expected-actual](#expected-actual)                 | ✅                  | ✅       |
-| [float-compare](#float-compare)                     | ✅                  | ❌       |
-| [formatter](#formatter)                             | ✅                  | 🤏      |
-| [go-require](#go-require)                           | ✅                  | ❌       |
-| [len](#len)                                         | ✅                  | ✅       |
-| [negative-positive](#negative-positive)             | ✅                  | ✅       |
-| [nil-compare](#nil-compare)                         | ✅                  | ✅       |
-| [regexp](#regexp)                                   | ✅                  | ✅       |
-| [require-error](#require-error)                     | ✅                  | ❌       |
-| [suite-broken-parallel](#suite-broken-parallel)     | ✅                  | ✅       |
-| [suite-dont-use-pkg](#suite-dont-use-pkg)           | ✅                  | ✅       |
-| [suite-extra-assert-call](#suite-extra-assert-call) | ✅                  | ✅       |
-| [suite-subtest-run](#suite-subtest-run)             | ✅                  | ❌       |
-| [suite-thelper](#suite-thelper)                     | ❌                  | ✅       |
-| [useless-assert](#useless-assert)                   | ✅                  | ❌       |
+| [blank-import](#blank-import)                       | ✅                 | ❌     |
+| [bool-compare](#bool-compare)                       | ✅                 | ✅     |
+| [compares](#compares)                               | ✅                 | ✅     |
+| [empty](#empty)                                     | ✅                 | ✅     |
+| [error-is-as](#error-is-as)                         | ✅                 | 🤏     |
+| [error-nil](#error-nil)                             | ✅                 | ✅     |
+| [expected-actual](#expected-actual)                 | ✅                 | ✅     |
+| [float-compare](#float-compare)                     | ✅                 | ❌     |
+| [formatter](#formatter)                             | ✅                 | 🤏     |
+| [http-const](#http-const)                           | ✅                 | ✅     |
+| [go-require](#go-require)                           | ✅                 | ❌     |
+| [len](#len)                                         | ✅                 | ✅     |
+| [negative-positive](#negative-positive)             | ✅                 | ✅     |
+| [nil-compare](#nil-compare)                         | ✅                 | ✅     |
+| [regexp](#regexp)                                   | ✅                 | ✅     |
+| [require-error](#require-error)                     | ✅                 | ❌     |
+| [suite-broken-parallel](#suite-broken-parallel)     | ✅                 | ✅     |
+| [suite-dont-use-pkg](#suite-dont-use-pkg)           | ✅                 | ✅     |
+| [suite-extra-assert-call](#suite-extra-assert-call) | ✅                 | ✅     |
+| [suite-subtest-run](#suite-subtest-run)             | ✅                 | ❌     |
+| [suite-thelper](#suite-thelper)                     | ❌                 | ✅     |
+| [useless-assert](#useless-assert)                   | ✅                 | ❌     |
 
 > ⚠️ Also look at open for contribution [checkers](CONTRIBUTING.md#open-for-contribution)
 
@@ -544,6 +545,24 @@ P.S. Look at [testify's issue](https://github.com/stretchr/testify/issues/772),
 
 ---
 
+### http-const
+
+```go
+❌
+assert.HTTPStatusCode(t, handler, "GET", "/index", nil, 200)
+assert.HTTPBodyContains(t, handler, "GET", "/index", nil, "counter")
+
+✅
+assert.HTTPStatusCode(t, handler, http.MethodGet, "/index", nil, http.StatusOK)
+assert.HTTPBodyContains(t, handler, http.MethodGet, "/index", nil, "counter")
+```
+
+**Autofix**: true. <br>
+**Enabled by default**: true. <br>
+**Reason**: Is similar to the [usestdlibvars](https://golangci-lint.run/usage/linters/#usestdlibvars) linter. <br>
+
+---
+
 ### len
 
 ```go

diff --git a/analyzer/checkers_factory_test.go b/analyzer/checkers_factory_test.go
@@ -26,6 +26,7 @@ func Test_newCheckers(t *testing.T) {
 		checkers.NewErrorIsAs(),
 		checkers.NewExpectedActual(),
 		checkers.NewRegexp(),
+		checkers.NewHTTPConst(),
 		checkers.NewSuiteExtraAssertCall(),
 		checkers.NewSuiteDontUsePkg(),
 		checkers.NewUselessAssert(),
@@ -43,6 +44,7 @@ func Test_newCheckers(t *testing.T) {
 		checkers.NewErrorIsAs(),
 		checkers.NewExpectedActual(),
 		checkers.NewRegexp(),
+		checkers.NewHTTPConst(),
 		checkers.NewSuiteExtraAssertCall(),
 		checkers.NewSuiteDontUsePkg(),
 		checkers.NewUselessAssert(),

diff --git a/analyzer/testdata/src/checkers-default/http-const/http_const_test.go b/analyzer/testdata/src/checkers-default/http-const/http_const_test.go
diff --git a/analyzer/testdata/src/checkers-default/http-const/http_const_test.go.golden b/analyzer/testdata/src/checkers-default/http-const/http_const_test.go.golden
@@ -0,0 +1,40 @@
+// Code generated by testifylint/internal/testgen. DO NOT EDIT.
+package httpconst
+
+import (
+	"fmt"
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func httpOK(w http.ResponseWriter, r *http.Request) {
+	w.WriteHeader(http.StatusOK)
+}
+
+func httpHelloName(w http.ResponseWriter, r *http.Request) {
+	name := r.FormValue("name")
+	_, _ = fmt.Fprintf(w, "Hello, %s!", name)
+}
+
+func TestHttpConstChecker(t *testing.T) {
+	// Invalid.
+	{
+		assert.HTTPStatusCode(t, httpOK, http.MethodGet, "/index", nil, http.StatusOK)                                                                           // want "http-const: use net/http constants instead of value"
+		assert.HTTPStatusCodef(t, httpOK, http.MethodGet, "/index", nil, http.StatusOK, "msg with args %d %s", 42, "42")                                         // want "http-const: use net/http constants instead of value"
+		assert.HTTPBodyContains(t, httpHelloName, http.MethodGet, "/", url.Values{"name": []string{"World"}}, "Hello, World!")                                   // want "http-const: use net/http constants instead of value"
+		assert.HTTPBodyContainsf(t, httpHelloName, http.MethodGet, "/", url.Values{"name": []string{"World"}}, "Hello, World!", "msg with args %d %s", 42, "42") // want "http-const: use net/http constants instead of value"
+	}
+	// Valid.
+	{
+		assert.HTTPStatusCode(t, httpOK, http.MethodGet, "/index", nil, http.StatusOK)
+		assert.HTTPStatusCodef(t, httpOK, http.MethodGet, "/index", nil, http.StatusOK, "msg with args %d %s", 42, "42")
+		assert.HTTPBodyContains(t, httpHelloName, http.MethodGet, "/", url.Values{"name": []string{"World"}}, "Hello, World!")
+		assert.HTTPBodyContainsf(t, httpHelloName, http.MethodGet, "/", url.Values{"name": []string{"World"}}, "Hello, World!", "msg with args %d %s", 42, "42")
+	}
+	// Ignored.
+	{
+	}
+}
diff --git a/internal/checkers/checkers_registry.go b/internal/checkers/checkers_registry.go
@@ -18,6 +18,7 @@ var registry = checkersRegistry{
 	{factory: asCheckerFactory(NewErrorIsAs), enabledByDefault: true},
 	{factory: asCheckerFactory(NewExpectedActual), enabledByDefault: true},
 	{factory: asCheckerFactory(NewRegexp), enabledByDefault: true},
+	{factory: asCheckerFactory(NewHTTPConst), enabledByDefault: true},
 	{factory: asCheckerFactory(NewSuiteExtraAssertCall), enabledByDefault: true},
 	{factory: asCheckerFactory(NewSuiteDontUsePkg), enabledByDefault: true},
 	{factory: asCheckerFactory(NewUselessAssert), enabledByDefault: true},

diff --git a/internal/checkers/checkers_registry_test.go b/internal/checkers/checkers_registry_test.go
@@ -46,6 +46,7 @@ func TestAll(t *testing.T) {
 		"error-is-as",
 		"expected-actual",
 		"regexp",
+		"http-const",
 		"suite-extra-assert-call",
 		"suite-dont-use-pkg",
 		"useless-assert",
@@ -81,6 +82,7 @@ func TestEnabledByDefault(t *testing.T) {
 		"error-is-as",
 		"expected-actual",
 		"regexp",
+		"http-const",
 		"suite-extra-assert-call",
 		"suite-dont-use-pkg",
 		"useless-assert",

diff --git a/internal/checkers/helpers_basic_type.go b/internal/checkers/helpers_basic_type.go
@@ -56,9 +56,14 @@ func isTypedIntNumber(e ast.Expr, v int, types ...string) bool {
 	return false
 }
 
-func isIntNumber(e ast.Expr, v int) bool {
+func safeTypedBasicLit(e ast.Expr, typ token.Token) (*ast.BasicLit, bool) {
 	bl, ok := e.(*ast.BasicLit)
-	return ok && bl.Kind == token.INT && bl.Value == fmt.Sprintf("%d", v)
+	return bl, ok && bl.Kind == typ
+}
+
+func isIntNumber(e ast.Expr, v int) bool {
+	bl, ok := safeTypedBasicLit(e, token.INT)
+	return ok && bl.Value == fmt.Sprintf("%d", v)
 }
 
 func isBasicLit(e ast.Expr) bool {
@@ -67,8 +72,8 @@ func isBasicLit(e ast.Expr) bool {
 }
 
 func isIntBasicLit(e ast.Expr) bool {
-	bl, ok := e.(*ast.BasicLit)
-	return ok && bl.Kind == token.INT
+	_, ok := safeTypedBasicLit(e, token.INT)
+	return ok
 }
 
 func isUntypedConst(pass *analysis.Pass, e ast.Expr) bool {

diff --git a/internal/checkers/helpers_http.go b/internal/checkers/helpers_http.go
@@ -3,12 +3,95 @@ package checkers
 import (
 	"go/ast"
 	"go/types"
+	"net/http"
+	"strconv"
 
 	"golang.org/x/tools/go/analysis"
 
 	"github.com/Antonboom/testifylint/internal/analysisutil"
 )
 
+var httpMethod = map[string]string{
+	http.MethodGet:     "http.MethodGet",
+	http.MethodHead:    "http.MethodHead",
+	http.MethodPost:    "http.MethodPost",
+	http.MethodPut:     "http.MethodPut",
+	http.MethodPatch:   "http.MethodPatch",
+	http.MethodDelete:  "http.MethodDelete",
+	http.MethodConnect: "http.MethodConnect",
+	http.MethodOptions: "http.MethodOptions",
+	http.MethodTrace:   "http.MethodTrace",
+}
+
+var httpStatusCode = map[string]string{
+	strconv.Itoa(http.StatusContinue):           "http.StatusContinue",
+	strconv.Itoa(http.StatusSwitchingProtocols): "http.StatusSwitchingProtocols",
+	strconv.Itoa(http.StatusProcessing):         "http.StatusProcessing",
+	strconv.Itoa(http.StatusEarlyHints):         "http.StatusEarlyHints",
+
+	strconv.Itoa(http.StatusOK):                   "http.StatusOK",
+	strconv.Itoa(http.StatusCreated):              "http.StatusCreated",
+	strconv.Itoa(http.StatusAccepted):             "http.StatusAccepted",
+	strconv.Itoa(http.StatusNonAuthoritativeInfo): "http.StatusNonAuthoritativeInfo",
+	strconv.Itoa(http.StatusNoContent):            "http.StatusNoContent",
+	strconv.Itoa(http.StatusResetContent):         "http.StatusResetContent",
+	strconv.Itoa(http.StatusPartialContent):       "http.StatusPartialContent",
+	strconv.Itoa(http.StatusMultiStatus):          "http.StatusMultiStatus",
+	strconv.Itoa(http.StatusAlreadyReported):      "http.StatusAlreadyReported",
+	strconv.Itoa(http.StatusIMUsed):               "http.StatusIMUsed",
+
+	strconv.Itoa(http.StatusMultipleChoices):   "http.StatusMultipleChoices",
+	strconv.Itoa(http.StatusMovedPermanently):  "http.StatusMovedPermanently",
+	strconv.Itoa(http.StatusFound):             "http.StatusFound",
+	strconv.Itoa(http.StatusSeeOther):          "http.StatusSeeOther",
+	strconv.Itoa(http.StatusNotModified):       "http.StatusNotModified",
+	strconv.Itoa(http.StatusUseProxy):          "http.StatusUseProxy",
+	strconv.Itoa(http.StatusTemporaryRedirect): "http.StatusTemporaryRedirect",
+	strconv.Itoa(http.StatusPermanentRedirect): "http.StatusPermanentRedirect",
+
+	strconv.Itoa(http.StatusBadRequest):                   "http.StatusBadRequest",
+	strconv.Itoa(http.StatusUnauthorized):                 "http.StatusUnauthorized",
+	strconv.Itoa(http.StatusPaymentRequired):              "http.StatusPaymentRequired",
+	strconv.Itoa(http.StatusForbidden):                    "http.StatusForbidden",
+	strconv.Itoa(http.StatusNotFound):                     "http.StatusNotFound",
+	strconv.Itoa(http.StatusMethodNotAllowed):             "http.StatusMethodNotAllowed",
+	strconv.Itoa(http.StatusNotAcceptable):                "http.StatusNotAcceptable",
+	strconv.Itoa(http.StatusProxyAuthRequired):            "http.StatusProxyAuthRequired",
+	strconv.Itoa(http.StatusRequestTimeout):               "http.StatusRequestTimeout",
+	strconv.Itoa(http.StatusConflict):                     "http.StatusConflict",
+	strconv.Itoa(http.StatusGone):                         "http.StatusGone",
+	strconv.Itoa(http.StatusLengthRequired):               "http.StatusLengthRequired",
+	strconv.Itoa(http.StatusPreconditionFailed):           "http.StatusPreconditionFailed",
+	strconv.Itoa(http.StatusRequestEntityTooLarge):        "http.StatusRequestEntityTooLarge",
+	strconv.Itoa(http.StatusRequestURITooLong):            "http.StatusRequestURITooLong",
+	strconv.Itoa(http.StatusUnsupportedMediaType):         "http.StatusUnsupportedMediaType",
+	strconv.Itoa(http.StatusRequestedRangeNotSatisfiable): "http.StatusRequestedRangeNotSatisfiable",
+	strconv.Itoa(http.StatusExpectationFailed):            "http.StatusExpectationFailed",
+	strconv.Itoa(http.StatusTeapot):                       "http.StatusTeapot",
+	strconv.Itoa(http.StatusMisdirectedRequest):           "http.StatusMisdirectedRequest",
+	strconv.Itoa(http.StatusUnprocessableEntity):          "http.StatusUnprocessableEntity",
+	strconv.Itoa(http.StatusLocked):                       "http.StatusLocked",
+	strconv.Itoa(http.StatusFailedDependency):             "http.StatusFailedDependency",
+	strconv.Itoa(http.StatusTooEarly):                     "http.StatusTooEarly",
+	strconv.Itoa(http.StatusUpgradeRequired):              "http.StatusUpgradeRequired",
+	strconv.Itoa(http.StatusPreconditionRequired):         "http.StatusPreconditionRequired",
+	strconv.Itoa(http.StatusTooManyRequests):              "http.StatusTooManyRequests",
+	strconv.Itoa(http.StatusRequestHeaderFieldsTooLarge):  "http.StatusRequestHeaderFieldsTooLarge",
+	strconv.Itoa(http.StatusUnavailableForLegalReasons):   "http.StatusUnavailableForLegalReasons",
+
+	strconv.Itoa(http.StatusInternalServerError):           "http.StatusInternalServerError",
+	strconv.Itoa(http.StatusNotImplemented):                "http.StatusNotImplemented",
+	strconv.Itoa(http.StatusBadGateway):                    "http.StatusBadGateway",
+	strconv.Itoa(http.StatusServiceUnavailable):            "http.StatusServiceUnavailable",
+	strconv.Itoa(http.StatusGatewayTimeout):                "http.StatusGatewayTimeout",
+	strconv.Itoa(http.StatusHTTPVersionNotSupported):       "http.StatusHTTPVersionNotSupported",
+	strconv.Itoa(http.StatusVariantAlsoNegotiates):         "http.StatusVariantAlsoNegotiates",
+	strconv.Itoa(http.StatusInsufficientStorage):           "http.StatusInsufficientStorage",
+	strconv.Itoa(http.StatusLoopDetected):                  "http.StatusLoopDetected",
+	strconv.Itoa(http.StatusNotExtended):                   "http.StatusNotExtended",
+	strconv.Itoa(http.StatusNetworkAuthenticationRequired): "http.StatusNetworkAuthenticationRequired",
+}
+
 func mimicHTTPHandler(pass *analysis.Pass, fType *ast.FuncType) bool {
 	httpHandlerFuncObj := analysisutil.ObjectOf(pass.Pkg, "net/http", "HandlerFunc")
 	if httpHandlerFuncObj == nil {