Skip to content

Commit

Permalink
Merge pull request #102 from Antonboom/feat/formatter
Browse files Browse the repository at this point in the history
New checker `formatter`
  • Loading branch information
Antonboom committed Jun 4, 2024
2 parents e10b593 + 01a9d0a commit af4a8c9
Show file tree
Hide file tree
Showing 42 changed files with 3,128 additions and 230 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:

env:
GO_VERSION: ^1.20
GOLANGCI_LINT_VERSION: v1.56.2
GOLANGCI_LINT_VERSION: v1.59.0

permissions:
contents: read
Expand Down
6 changes: 6 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ issues:
# revive should check comments for exported (and not internal) code.
include: [ EXC0012, EXC0013, EXC0014 ]

exclude-dirs:
- internal/checkers/printf # A patched fork of go vet's printf.

exclude-rules:
- path: "internal/testgen"
linters: [ "revive" ]
Expand All @@ -22,6 +25,9 @@ issues:
- path: "_test\\.go"
linters: [ "lll" ]

- source: ' // want "'
linters: [ "lll" ]

linters-settings:
depguard:
rules:
Expand Down
39 changes: 0 additions & 39 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ Describe a new checker in [checkers section](./README.md#checkers).
- [float-compare](#float-compare)
- [http-const](#http-const)
- [http-sugar](#http-sugar)
- [no-fmt-mess](#no-fmt-mess)
- [require-len](#require-len)
- [suite-run](#suite-run)
- [suite-test-name](#suite-test-name)
Expand Down Expand Up @@ -305,44 +304,6 @@ And similar idea for `assert.InEpsilonSlice` / `assert.InDeltaSlice`.

---

### no-fmt-mess

**Autofix**: true. <br>
**Enabled by default**: maybe? <br>
**Related issues**: [#33](https://github.com/Antonboom/testifylint/issues/33)

Those who are new to `testify` may be discouraged by the duplicative API:

```go
func Equal(t TestingT, expected, actual interface{}, msgAndArgs ...interface{}) bool
func Equalf(t TestingT, expected interface{}, actual interface{}, msg string, args ...interface{}) bool
```

The f-functions [were added a long time ago](https://github.com/stretchr/testify/pull/356) to eliminate
`govet` [complain](https://go.googlesource.com/tools/+/refs/heads/release-branch.go1.12/go/analysis/passes/printf/printf.go?pli=1#506).

This introduces some inconsistency into the test code, and the next strategies are seen for the checker:

1) Forbid f-functions at all (also could be done through the [forbidigo](https://golangci-lint.run/usage/linters/#forbidigo) linter).

This will make it easier to migrate to [v2](https://github.com/stretchr/testify/issues/1089), because

> Format functions should not be accepted as they are equivalent to their "non-f" counterparts.
But it doesn't look like a "go way" and the `govet` won't be happy.

2) IMHO, a more appropriate option is to disallow the use of `msgAndArgs` in non-f assertions. Look at
[the comment](https://github.com/stretchr/testify/issues/1089#issuecomment-1695059265).

But there will be no non-stylistic benefits from the checker in this case (depends on the view of API in `v2`).

Also, in the first iteration `no-fmt-mess` checker could help to avoid code like this:
```go
assert.Error(t, err, fmt.Sprintf("Profile %s should not be valid", test.profile))
```

---

### require-len

The main idea: if code contains array/slice indexing,
Expand Down
Loading

0 comments on commit af4a8c9

Please sign in to comment.