zero: use zero instead of `asserr.Equal(t, 0, v)`

[ccoVeille]

For records, I started working on the implementation of this feature request.

https://github.com/Antonboom/testifylint/blob/master/CONTRIBUTING.md#zero

[ccoVeille]

I've read the consideration about the doubt about using Zero vs Empty

I'm not sure if anyone uses assert.Zero – it looks strange and conflicts with assert.Empty:
Maybe it's better to make configurable support for other types in the empty checker and vice versa to prohibit the Zero?
Source: https://github.com/Antonboom/testifylint/blob/master/CONTRIBUTING.md#zero

I'll explain in this issue what I plan to implement, so we could talk about the implementation here

[ccoVeille]

I created the issue to keep a track of it, and allow other people to discuss it.

Stay tuned, and ping me if you see no progress in the next month.

[ccoVeille]

I've read the consideration about the doubt about using Zero vs Empty

I'm not sure if anyone uses assert.Zero – it looks strange and conflicts with assert.Empty:
Maybe it's better to make configurable support for other types in the empty checker and vice versa to prohibit the Zero?
Source: https://github.com/Antonboom/testifylint/blob/master/CONTRIBUTING.md#zero

I'll explain in this issue what I plan to implement, so we could talk about the implementation here

I agree with you @Antonboom the use case assert.Zero covers are close to the one assert.Empty

I will check a bit further in the code of testify, but I think is what we should look for

https://github.com/stretchr/testify/blob/master/assert/assertions_test.go#L23-L85

I would say Zero checker should report to be used when using Equal, Equalf, Exactly and Exactlyf for the following type

• complex64(0), complex128(0),
• float32(0), float64(0),
• int(0), int8(0), int16(0), int32(0), int64(0),
• uint(0), uint8(0), uint16(0), uint32(0), uint64(0

And same for NotZero with NotEqual and, NotEqualf

Everything other types that assert.Zero supports should be reported to use Empty or NotEmpty checkers, or simply maybe ignored.

[ccoVeille]

For this reason, and because I struggle in making it works with Empty checker without conflicts in my current implementation, it seems simpler to add the Zero detection to existing Empty checker. What do you think about it ?

[Antonboom]

@ccoVeille hi

I reread carefully the messages and examples above and looks like I was wrong

1. Empty is concentrated on equals with len.
2. Zero is concentrated on equals with zero value objects.

And these are separate checkers, and zero should cover not only zeros, but the full list:

	zeros = []interface{}{
		false,
		byte(0),
		complex64(0),
		complex128(0),
		float32(0),
		float64(0),
		int(0),
		int8(0),
		int16(0),
		int32(0),
		int64(0),
		rune(0),
		uint(0),
		uint8(0),
		uint16(0),
		uint32(0),
		uint64(0),
		uintptr(0),
		"",
		[0]interface{}{},
		[]interface{}(nil),
		struct{ x int }{},
		(*interface{})(nil),
		(func())(nil),
		nil,
		interface{}(nil),
		map[interface{}]interface{}(nil),
		(chan interface{})(nil),
		(<-chan interface{})(nil),
		(chan<- interface{})(nil),
	}

If user uses assert.Equal(t, 0, len(warningMsg)) then we suggest assert.Empy(t, errors).
If uses uses assert.Equal(t, "", warningMsg) then we suggest assert.Zero(t, warningMsg).

---

Also need to grep some statistics and understand the history of creation of Zero.
To enable it by default or not? Really it useful or not?

[ccoVeille]

Thanks for digging further into the checks.

There are definitely testify assert that covers the same kind of check.

I mean an assert.Empty(t, nilPointer), assert.Zero(t, nilPointer), is valid, but assert.Nil(t, nilPointer) would be better.

Your tool is there to suggest using the right asserted.

So I think while each checker COULD handle them all, BUT your tool will have to provide opinionated suggestions, and document them.

For me, using Zero on empty string would be strange, I would expect to see Empty being suggested.

Considering a string is a list of character, you can often find people using len(emptyString) == 0, while others are using emptyString == ""

So here is the same, Empty and Zero have lot in common, you will have to define what is the best for each type.

Let's continue the discussion

[ccoVeille]

Also, as I previously said, I think empty and zero could be handled in the same checker, as they are very close.

[ccoVeille]

I think the project would need something like this:

Nil is suggested for

• Empty(nil)
• Zero(nil)

NoError is suggested for

• Empty(nilErrorPointer)
• Zero(nilErrorPointer)
• Nil(nilErrorPointer)

Empty is suggested for

• Len(0, list)
• Zero(emptyString)

Zero is suggested for

• Empty(int)
• Empty(time.Time)
• Equal(time.Time, 0)
• True(t, time.Time.IsZero())
  …

This list has to be completed, and discussed of course. I let out all the compare operators and co.

My approach is the opposite of "this checker does that". It's more a "what is expected from testifylint".

I'm not talking about how it's implemented, but what would be the good way to use testify. So, a guidance, it would be opinionated. It could be used as a reference for new checkers implementation.

I could write a design note in a markdown file you could review.

EDIT: 2024-05-28 Zero was completed with "zero time" use cases

[ccoVeille]

You didn't reply to my suggestion and questions. It's OK. But I'm busy IRL for a while.

Could you please unassign me from the issue? Thanks

[ccoVeille]

I think the project would need something like this:

Nil is suggested for

• Empty(nil)
• assert.Zero(nil)

NoError is suggested for

• Empty(nilErrorPointer)
• Zero(nilErrorPointer)
• Nil(nilErrorPointer)

Empty is suggested for

• Len(0, list)
• Zero(emptyString)

Zero is suggested for

• Empty(int)
  …

This list has to be completed, and discussed of course. I let out all the compare operators and co.

My approach is the opposite of "this checker does that". It's more a "what is expected from testifylint".

I'm not talking about how it's implemented, but what would be the good way to use testify. So, a guidance, it would be opinionated. It could be used as a reference for new checkers implementation.

I could write a design note in a markdown file you could review.

Hi @dolmen

I'm curious about your thoughts about this.

Thanks

[ccoVeille]

@dolmen didn't reply and it's OK, he might be busy

Maybe, I should bring the discussion on https://gophers.slack.com/archives/C18A3680H

I also thought about opening a discussion on https://github.com/stretchr/testify/discussions

It would require me to work on the matrix I talked about #75 (comment) before opening the discussion of course

what do you think @Antonboom ?

[mmorel-35]

time.Time has a https://pkg.go.dev/time#Time.IsZero function. How do you recommend checking a time.Time is zero ?

using assert.True(t, time.Time.IsZero()) or assert.Zero(t, time.Time) ?