incorrect-assert: add type checks for assert.Regexp regexp argument

[dolmen]

The signature for assert.Regexp (and associated methods) is:

func Regexp(t TestingT, rx interface{}, str interface{}, msgAndArgs ...interface{}) bool

However rx should only be *regexp.Regexp or string.

Related: stretchr/testify#1585
Cc: @kevinburkesegment

[ccoVeille]

It's interesting.

Could you explain the unexpected behavior you found with this method, especially the "around fmt.Sprint()"

Do you have examples of interface value passed to assert.Regexp ?

[dolmen]

I just tried a code search on GitHub of bad usage of assert.NotRegexp, and found none so far, so bad usage is probably quite rare and usually easily caught by running the test.

But that would be the point of this checker: catch the remaining ones.

About fmt.Sprint() traps: conversion of []byte is often not what one expects. See https://go.dev/play/p/3XsaeWgDsUD

[ccoVeille]

Side note, if something is implemented it cannot be only about detecting the value is a regexp object or a string. The code use fmt.Sprint, so it relies also in being able to convert into a string.

So it should also support fmt.Stringer

https://github.com/golang/go/blob/master/src%2Ffmt%2Fprint.go

[ccoVeille]

Current code of testify is also able to support crazy people doing things like this

assert.Regexp(t, 17, " foo 17 bar")

https://go.dev/play/p/2usnMRVizun

This should be discouraged maybe

I didn't test but I think it would be the same for assert.Regexp(t, true, "it's true")

[dolmen]

@ccoVeille We are discussing about a linter.

As a Testify maintainer I can't change the crazy things that are allowed by the current implementation. But I expect the linter to tell the user that the crazy thing he does is most probably just a mistake.

[ccoVeille]

Yes, I agree that's why I listed the things the linter could raise.

I agree with you, it's not about changing testify, but to guide user to use the right asserter

[ccoVeille]

@ccoVeille We are discussing about a linter.

As a Testify maintainer I can't change the crazy things that are allowed by the current implementation. But I expect the linter to tell the user that the crazy thing he does is most probably just a mistake.

About this, because I'm unsure about your reply.

Your issue was about accepting:

• string
• *regexp.Regexp

I suggested current implementation also support any struct with a stringer.

So something like this:

type alpha struct{}

func (*alpha) String() string {
  return "[a-z]+"
}

(This is pseudo code made on a phone)

Would you expect the linter to report sending any object from this type to assert.Regexp as invalid?

[Antonboom]

+ case when order of arguments is invalid

assert.Regexp(t, out, `\[.*\] DEBUG \(.*TestNew.*\): message`)

[Antonboom]

+ stretchr/testify#1587