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) ?

[ccoVeille]

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) ?

Good remark, I will have to check what it does, but I would say assert.Zero(t, zeroTime) would be more logic, and shorter

but I have to check if it works first

[ccoVeille]

Just noticed @dolmen added a 👍 to #75 (comment)

so I would say, we had a kind of approval 😄

[ccoVeille]

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) ?

thanks @mmorel-35 for your feedback and your interest in the discussion I brought

[ccoVeille]

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) ?

Good remark, I will have to check what it does, but I would say assert.Zero(t, zeroTime) would be more logic, and shorter

but I have to check if it works first

confirmed it works perfectly https://go.dev/play/p/LjKyrg0sn1H

So I would recommend using assert.Zero for checking zero time

thanks for your suggestion @mmorel-35

I edited #75 (comment)

[mmorel-35]

@ccoVeille ,
I found zero being suggested as a New linter for golangci-lint. It might be helpful to identify "Zero-able" types . It might worth having a look to start working on this feature?

[ccoVeille]

The project is somehow abandoned, but it could be a good source of inspiration

[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

@mmorel-35 do you think you could use the zero project to work on populating the document I started writing?

Do you agree with me that having such a table out of current checker consideration would be a good thing?

I'm busy with my new born kid 👨‍👩‍👧‍👧, I'm pretty active on GitHub but I'm mostly using my phone. Most of my time is spent I'm trying to make my baby girl sleep 😪

any help would be appreciated.

[ccoVeille]

@ccoVeille ,
I found zero being suggested as a New linter for golangci-lint. It might be helpful to identify "Zero-able" types . It might worth having a look to start working on this feature?

@mmorel-35 by being suggested you meant here?

golangci/golangci-lint#3491 (comment)

golangci/golangci-lint#3529

[ccoVeille]

@mmorel-35 I'm curious how zero differs from what I found in testify and already talked about here

#75 (comment)

I will check a bit further in the code of testify, but I think is what we should look for\n\nhttps://github.com/stretchr/testify/blob/master/assert/assertions_test.go#L23-L85\n\nI would say Zero checker should report to be used when using Equal, Equalf, Exactly and Exactlyf for the following type\n\ncomplex64(0), complex128(0),\nfloat32(0), float64(0),\nint(0), int8(0), int16(0), int32(0), int64(0),\nuint(0), uint8(0), uint16(0), uint32(0), uint64(0

[mmorel-35]

do you think you could use the zero project to work on populating the document I started writing?

It seems to be close to what you started to identify. There probably be some differences of course

Do you agree with me that having such a table out of current checker consideration would be a good thing?

It's probably easier to have inside the project but probably isolated so it can be outside in a second time

I'm busy with my new born kid 👨‍👩‍👧‍👧, I'm pretty active on GitHub but I'm mostly using my phone. Most of my time is spent I'm trying to make my baby girl sleep 😪

any help would be appreciated.

Good luck with your child. I won't be able to help on the development part. I wanted to share what I found that could help the good improvements testifylint is providing .

@ccoVeille ,
I found zero being suggested as a New linter for golangci-lint. It might be helpful to identify "Zero-able" types . It might worth having a look to start working on this feature?

@mmorel-35 by being suggested you meant here?

golangci/golangci-lint#3491 (comment)

golangci/golangci-lint#3529

Exactly here, yes

[Antonboom]

@ccoVeille

I don't see the benefits from wasting resources on support matrix above.

And we have a lot of more useful checkers (see issues list), that protect from real bugs and not just stylish thing.
So, please turn your attention to them.

Thanks!