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

empty: report special message for len and string-cast cases #149

Closed
ldez opened this issue Jun 22, 2024 · 3 comments · Fixed by #189
Closed

empty: report special message for len and string-cast cases #149

ldez opened this issue Jun 22, 2024 · 3 comments · Fixed by #189
Labels
bug Something isn't working

Comments

@ldez
Copy link

ldez commented Jun 22, 2024

package sandbox

import (
	"testing"

	"github.com/stretchr/testify/assert"
)

func TestName(t *testing.T) {
	assert.NotEmptyf(t, len([]string{"e"}), "text: %s", "a")
	assert.NotEmptyf(t, []string{"e"}, "text: %s", "a")
}
$ ./golangci-lint run --enable-only testifylint
main_test.go:10:2: empty: use assert.NotEmptyf (testifylint)
        assert.NotEmptyf(t, len([]string{"e"}), "text: %s", "a")
        ^

I used a version of golangci-lint that uses testifylint v1.4.3.

@Antonboom Antonboom added the bug Something isn't working label Jun 22, 2024
@ldez ldez changed the title the NotEmptyf message may be improved. the NotEmptyf message may be improved Jun 22, 2024
@ldez
Copy link
Author

ldez commented Jun 22, 2024

I think you already know that but it's the same for NotEmpty, Emptyf, Empty.

I have not tested all the assertions.

package sandbox

import (
	"testing"

	"github.com/stretchr/testify/assert"
)

func TestName(t *testing.T) {
	assert.NotEmptyf(t, len([]string{"e"}), "text: %s", "a")
	assert.NotEmptyf(t, []string{"e"}, "text: %s", "a")

	assert.NotEmpty(t, len([]string{"e"}))
	assert.NotEmpty(t, []string{"e"})

	assert.Emptyf(t, len([]string{"e"}), "text: %s", "a")
	assert.Emptyf(t, []string{"e"}, "text: %s", "a")

	assert.Empty(t, len([]string{"e"}))
	assert.Empty(t, []string{"e"})
}
$ ./golangci-lint run --enable-only testifylint
main_test.go:10:2: empty: use assert.NotEmptyf (testifylint)
        assert.NotEmptyf(t, len([]string{"e"}), "text: %s", "a")
        ^
main_test.go:13:2: empty: use assert.NotEmpty (testifylint)
        assert.NotEmpty(t, len([]string{"e"}))
        ^
main_test.go:16:2: empty: use assert.Emptyf (testifylint)
        assert.Emptyf(t, len([]string{"e"}), "text: %s", "a")
        ^
main_test.go:19:2: empty: use assert.Empty (testifylint)
        assert.Empty(t, len([]string{"e"}))
        ^

@ccoVeille
Copy link
Contributor

ccoVeille commented Jun 22, 2024

Good catch, the way it reports now could let people think it's a false negative, while it's not, testifylint suggests to simplify.

So I agree with you the message needs to be adapted

@Antonboom Antonboom changed the title the NotEmptyf message may be improved ` message may be improved Jun 23, 2024
@Antonboom Antonboom changed the title ` message may be improved empty: report special message for len and string-cast cases Jun 23, 2024
@Antonboom
Copy link
Owner

After fix:

$ testifylint --disable-all --enable=empty ./...                                                                        
/tmp/sandbox/main_test.go:10:2: empty: remove unnecessary len
/tmp/sandbox/main_test.go:13:2: empty: remove unnecessary len
/tmp/sandbox/main_test.go:16:2: empty: remove unnecessary len
/tmp/sandbox/main_test.go:19:2: empty: remove unnecessary len

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants