Skip to content

Commit f6bf5ae

Browse files
committed
formatter: self-review #1
1 parent 6ad195e commit f6bf5ae

File tree

5 files changed

+49
-68
lines changed

5 files changed

+49
-68
lines changed

README.md

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ https://golangci-lint.run/usage/linters/#testifylint
9393
| [bool-compare](#bool-compare) |||
9494
| [compares](#compares) |||
9595
| [empty](#empty) |||
96-
| [error-is-as](#error-is-as) || |
96+
| [error-is-as](#error-is-as) || 🤏 |
9797
| [error-nil](#error-nil) |||
9898
| [expected-actual](#expected-actual) |||
9999
| [float-compare](#float-compare) |||
@@ -117,19 +117,19 @@ https://golangci-lint.run/usage/linters/#testifylint
117117
```go
118118
119119
import (
120-
"testing"
121-
122-
_ "github.com/stretchr/testify"
123-
_ "github.com/stretchr/testify/assert"
124-
_ "github.com/stretchr/testify/http"
125-
_ "github.com/stretchr/testify/mock"
126-
_ "github.com/stretchr/testify/require"
127-
_ "github.com/stretchr/testify/suite"
120+
"testing"
121+
122+
_ "github.com/stretchr/testify"
123+
_ "github.com/stretchr/testify/assert"
124+
_ "github.com/stretchr/testify/http"
125+
_ "github.com/stretchr/testify/mock"
126+
_ "github.com/stretchr/testify/require"
127+
_ "github.com/stretchr/testify/suite"
128128
)
129129

130130
131131
import (
132-
"testing"
132+
"testing"
133133
)
134134
```
135135

@@ -258,7 +258,7 @@ assert.NotErrorIs(t, err, errSentinel)
258258
assert.ErrorAs(t, err, &target)
259259
```
260260

261-
**Autofix**: true. <br>
261+
**Autofix**: partially. <br>
262262
**Enabled by default**: true. <br>
263263
**Reason**: In the first two cases, a common mistake that leads to hiding the incorrect wrapping of sentinel errors.
264264
In the rest cases – more appropriate `testify` API with clearer failure message.
@@ -390,7 +390,7 @@ Detecting unnecessary `fmt.Sprintf` in assertions. Somewhat reminiscent of the
390390
#### 2)
391391

392392
Validating consistency of assertions format strings and corresponding arguments, using a patched fork of `go vet`'s
393-
[printf](https://pkg.go.dev/golang.org/x/tools@v0.21.0/go/analysis/passes/printf#hdr-Analyzer_printf) analyzer. To
393+
[printf](https://cs.opensource.google/go/x/tools/+/master:go/analysis/passes/printf/printf.go) analyzer. To
394394
disable this feature, use `--formatter.check-format-string=false` flag.
395395

396396
#### 3)
@@ -405,14 +405,15 @@ and sets the stage for moving to `v2` of `testify`. In this way the checker rese
405405
[golangci-lint](https://golangci-lint.run/usage/linters/)). Also format string in f-assertions is highlighted by IDE
406406
, e.g. GoLand:
407407

408-
<img width="600" alt="F-assertion highlighting" src="https://github.com/Antonboom/testifylint/assets/17127404/9bdab802-d6eb-477d-a411-6cba043d33a5">
408+
<img width="600" alt="F-assertion IDE highlighting" src="https://github.com/Antonboom/testifylint/assets/17127404/9bdab802-d6eb-477d-a411-6cba043d33a5">
409409

410410
#### Historical Reference
411411

412412
<details>
413413

414414
<summary>Expand...</summary>
415415

416+
<br>
416417
Those who are new to `testify` may be discouraged by the duplicative API:
417418

418419
```go
@@ -466,11 +467,8 @@ Now **printf** only checked Golang standard library functions (unless configured
466467

467468
Despite this, f-functions have already been released, giving rise to ambiguous API.
468469

469-
But surely the maintainers had no choice to change the signatures in accordance with the implicit Go convention
470-
471-
> Printf-like functions must end with `f`
472-
473-
because it would break backwards compatibility:
470+
But surely the maintainers had no choice to change the signatures in accordance with Go convention, because it would
471+
break backwards compatibility:
474472

475473
```go
476474
func Equal(t TestingT, expected, actual any) bool
@@ -520,19 +518,17 @@ Try to execute them in the main goroutine and distribute the data necessary for
520518
([example](https://github.com/ipfs/kubo/issues/2043#issuecomment-164136026)).
521519

522520
Also a bad solution would be to simply replace all `require` in goroutines with `assert`
523-
(
524-
like [here](https://github.com/gravitational/teleport/pull/22567/files#diff-9f5fd20913c5fe80c85263153fa9a0b28dbd1407e53da4ab5d09e13d2774c5dbR7377))
521+
(like
522+
[here](https://github.com/gravitational/teleport/pull/22567/files#diff-9f5fd20913c5fe80c85263153fa9a0b28dbd1407e53da4ab5d09e13d2774c5dbR7377))
525523
– this will only mask the problem.
526524

527525
The checker is enabled by default, because `testinggoroutine` is enabled by default in `go vet`.
528526

529527
In addition, the checker warns about `require` in HTTP handlers (functions and methods whose signature matches
530528
[http.HandlerFunc](https://pkg.go.dev/net/http#HandlerFunc)), because handlers run in a separate
531529
[service goroutine](https://cs.opensource.google/go/go/+/refs/tags/go1.22.3:src/net/http/server.go;l=2782;drc=1d45a7ef560a76318ed59dfdb178cecd58caf948)
532-
that
533-
services the HTTP connection. Terminating these goroutines can lead to undefined behaviour and difficulty debugging
534-
tests.
535-
You can turn off the check using the `--go-require.ignore-http-handlers` flag.
530+
that services the HTTP connection. Terminating these goroutines can lead to undefined behaviour and difficulty debugging
531+
tests. You can turn off the check using the `--go-require.ignore-http-handlers` flag.
536532

537533
P.S. Look at [testify's issue](https://github.com/stretchr/testify/issues/772), related to assertions in the goroutines.
538534

@@ -620,7 +616,7 @@ assert.Equal(t, (chan Event)(nil), eventsChan)
620616
assert.NotEqual(t, (chan Event)(nil), eventsChan)
621617
```
622618

623-
But in the case of `Equal`, `NotEqual` and `Exactly` it still doesn't work for the function type.
619+
But in the case of `Equal`, `NotEqual` and `Exactly` type casting approach still doesn't work for the function type.
624620

625621
The best option here is to just use `Nil` / `NotNil` (see [details](https://github.com/stretchr/testify/issues/1524)).
626622

analyzer/testdata/src/checkers-default/blank-import/blank_import_test.go

Lines changed: 23 additions & 37 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

analyzer/testdata/src/checkers-default/suite-dont-use-pkg/suite_dont_use_pkg_test.go

Lines changed: 2 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/checkers/call_meta.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
//
1717
// assert.Equal(t, 42, result, "helpful comment")
1818
type CallMeta struct {
19-
// Call stores the original call expression.
19+
// Call stores the original AST call expression.
2020
Call *ast.CallExpr
2121
// Range contains start and end position of assertion call.
2222
analysis.Range
@@ -114,7 +114,7 @@ func NewCallMeta(pass *analysis.Pass, ce *ast.CallExpr) *CallMeta {
114114
Name: fnName,
115115
NameFTrimmed: strings.TrimSuffix(fnName, "f"),
116116
IsFmt: strings.HasSuffix(fnName, "f"),
117-
Signature: funcObj.Type().(*types.Signature),
117+
Signature: funcObj.Type().(*types.Signature), // NOTE(a.telyshev): Func's Type() is always a *Signature.
118118
},
119119
Args: trimTArg(pass, ce.Args),
120120
ArgsRaw: ce.Args,

internal/checkers/helpers_format.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"github.com/Antonboom/testifylint/internal/analysisutil"
1010
)
1111

12-
// formatAsCallArgs joins a, b and c return bytes like `a, b, c`.
12+
// formatAsCallArgs joins a, b and c and returns bytes like `a, b, c`.
1313
func formatAsCallArgs(pass *analysis.Pass, args ...ast.Expr) []byte {
1414
if len(args) == 0 {
1515
return []byte("")

0 commit comments

Comments
 (0)