feat: Add TimeEqual checker to enforce proper time comparisons

[semihbkgr]

fix #242
closes #60

@Antonboom

If you're okay with the new checker, I'll add the corresponding unit tests and refactor the diagnostic messages and fixes.

[ccoVeille]

The idea seems legitimate.

But then, I wonder whether bool_compare checker will have to be updated

[ccoVeille]

Now, I have read through this

• Asserting equality for Dates stretchr/testify#666

I'm unsure about the PR.

What do you think @semihbkgr

cc @brackendawson @Antonboom @mmorel-35

[ccoVeille]

More context

• assert: add TimeEqual function to better compare times stretchr/testify#1240 (comment)

Using assert.WithinDuration seems a better option that could avoid having testifylint rules that may conflicts each other.

[semihbkgr]

@ccoVeille

It would be better to suggest using WithinDuration instead of True(t1.Equal(t2)).

Just to clarify, are we all in agreement that there should be a warning when two time values are compared using the assert.Equal function? I'm a bit confused after reading the discussion in the issue you linked in your previous comment, particularly because you said "I'm unsure about the PR."

[ccoVeille]

@ccoVeille

It would be better to suggest using WithinDuration instead of True(t1.Equal(t2)).

Yes.

So here I would say when we find True(t1.Equal(t2))

I'm a bit confused after reading the discussion in the issue you linked in your previous comment, particularly because you said "I'm unsure about the PR."

I was unsure when I wrote it, and like you, it was worst after reading the threads and discussions I quoted.

Just to clarify, are we all in agreement that there should be a warning when two time values are compared using the assert.Equal function?

For me, yes.

Also, I feel like there is a need for a time_compare rule (instead of simply time-equal). That people would enable or disable depending on their needs.

This rule should detect:

• Equal(t, t1, t2)
• True(t, t1.Equal(t2))

And suggested WithinDuration(t, t1, t2, 0)

[ccoVeille]

Maybe later, this rule could give ideas for what we do with time.Time and time.Duration, such as:

• True(t, t1.After(t2))
• True(t, t1.Before(t2))
• Positive(t, t1.Sub(t2))
  ...

These are only ideas, that needs to be double checked, depending on how clearer would be the error message that testify would report.

[ccoVeille]

Yes, I know you asked about one specific point and I somehow replied by popping out 3 new ideas, and thus problems that would pops out with it.

It's my super power ! 😅