Skip to content

Commit

Permalink
new checker http-const
Browse files Browse the repository at this point in the history
Co-authored-by: ccoVeille <[email protected]>
  • Loading branch information
mmorel-35 and ccoVeille committed Jul 7, 2024
1 parent 68ded17 commit f3f9d37
Show file tree
Hide file tree
Showing 13 changed files with 485 additions and 52 deletions.
19 changes: 0 additions & 19 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
61 changes: 40 additions & 21 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,28 +91,29 @@ https://golangci-lint.run/usage/linters/#testifylint

| Name | Enabled By Default | Autofix |
|-----------------------------------------------------|--------------------|---------|
| [blank-import](#blank-import) | | |
| [bool-compare](#bool-compare) | | |
| [compares](#compares) | | |
| [blank-import](#blank-import) |||
| [bool-compare](#bool-compare) |||
| [compares](#compares) |||
| [contains](#contains) |||
| [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) |||
| [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)
Expand Down Expand Up @@ -566,6 +567,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**: Cleaner code and meaningful constants instead of magical numbers/identifiers. This checker is similar to the [usestdlibvars](https://golangci-lint.run/usage/linters/#usestdlibvars) linter. <br>

---

### len

```go
Expand Down
2 changes: 2 additions & 0 deletions analyzer/checkers_factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func Test_newCheckers(t *testing.T) {
checkers.NewErrorIsAs(),
checkers.NewExpectedActual(),
checkers.NewRegexp(),
checkers.NewHTTPConst(),
checkers.NewSuiteExtraAssertCall(),
checkers.NewSuiteDontUsePkg(),
checkers.NewUselessAssert(),
Expand All @@ -45,6 +46,7 @@ func Test_newCheckers(t *testing.T) {
checkers.NewErrorIsAs(),
checkers.NewExpectedActual(),
checkers.NewRegexp(),
checkers.NewHTTPConst(),
checkers.NewSuiteExtraAssertCall(),
checkers.NewSuiteDontUsePkg(),
checkers.NewUselessAssert(),
Expand Down

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// 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.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.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.
{
assert.HTTPStatusCode(t, httpOK, "FOO", "/index", nil, 999)
assert.HTTPStatusCodef(t, httpOK, "FOO", "/index", nil, 999, "msg with args %d %s", 42, "42")
assert.HTTPBodyContains(t, httpHelloName, "FOO", "/", url.Values{"name": []string{"World"}}, "Hello, World!")
assert.HTTPBodyContainsf(t, httpHelloName, "FOO", "/", url.Values{"name": []string{"World"}}, "Hello, World!", "msg with args %d %s", 42, "42")
}
}
1 change: 1 addition & 0 deletions internal/checkers/checkers_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,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},
Expand Down
2 changes: 2 additions & 0 deletions internal/checkers/checkers_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func TestAll(t *testing.T) {
"error-is-as",
"expected-actual",
"regexp",
"http-const",
"suite-extra-assert-call",
"suite-dont-use-pkg",
"useless-assert",
Expand Down Expand Up @@ -83,6 +84,7 @@ func TestEnabledByDefault(t *testing.T) {
"error-is-as",
"expected-actual",
"regexp",
"http-const",
"suite-extra-assert-call",
"suite-dont-use-pkg",
"useless-assert",
Expand Down
22 changes: 18 additions & 4 deletions internal/checkers/helpers_basic_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"go/ast"
"go/token"
"go/types"
"strconv"

"golang.org/x/tools/go/analysis"
)
Expand Down Expand Up @@ -56,9 +57,14 @@ func isTypedIntNumber(e ast.Expr, v int, types ...string) bool {
return false
}

func isIntNumber(e ast.Expr, v int) bool {
func typeSafeBasicLit(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 := typeSafeBasicLit(e, token.INT)
return ok && bl.Value == fmt.Sprintf("%d", v)
}

func isBasicLit(e ast.Expr) bool {
Expand All @@ -67,8 +73,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 := typeSafeBasicLit(e, token.INT)
return ok
}

func isUntypedConst(pass *analysis.Pass, e ast.Expr) bool {
Expand Down Expand Up @@ -111,3 +117,11 @@ func untype(e ast.Expr) ast.Expr {
}
return ce.Args[0]
}

func unquoteBasicLitValue(basicLit *ast.BasicLit) (string, bool) {
value, err := strconv.Unquote(basicLit.Value)
if err != nil {
return "", false
}
return value, true
}
26 changes: 18 additions & 8 deletions internal/checkers/helpers_diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,26 @@ func newDiagnostic(
msg string,
fix *analysis.SuggestedFix,
) *analysis.Diagnostic {
d := analysis.Diagnostic{
Pos: rng.Pos(),
End: rng.End(),
Category: checker,
Message: checker + ": " + msg,
}
var suggestedFixes []analysis.SuggestedFix
if fix != nil {
d.SuggestedFixes = []analysis.SuggestedFix{*fix}
suggestedFixes = append(suggestedFixes, *fix)
}
return newAnalysisDiagnostic(checker, rng, msg, suggestedFixes)
}

func newAnalysisDiagnostic(
checker string,
rng analysis.Range,
msg string,
suggestedFixes []analysis.SuggestedFix,
) *analysis.Diagnostic {
return &analysis.Diagnostic{
Pos: rng.Pos(),
End: rng.End(),
Category: checker,
Message: checker + ": " + msg,
SuggestedFixes: suggestedFixes,
}
return &d
}

func newSuggestedFuncReplacement(
Expand Down
Loading

0 comments on commit f3f9d37

Please sign in to comment.