fix(assert): avoid panic on invalid regex in Regexp/NotRegexp (#1794)

[kdt523]

Summary
Make assert.Regexp / assert.NotRegexp fail the assertion instead of panicking when given an invalid regex pattern.

Changes
Use regexp.Compile (rather than regexp.MustCompile) when compiling user-supplied patterns in assert.
Propagate compile errors so Regexp/NotRegexp report a clear failure message like:
invalid regular expression "":
Add assert/regexp_invalid_test.go with tests that ensure invalid patterns do not cause a panic.
Files touched:
assert/assertions.go
assert/regexp_invalid_test.go
Motivation
Passing a malformed regex used to trigger a runtime panic in tests. That made debugging harder and could abort the whole test run. Reporting a failed assertion with a helpful error message is a safer, more useful behavior for test authors.

Related issues
#1794

[ccoVeille]

This would be better, no?

Suggested change

	Fail(t, fmt.Sprintf("invalid regular expression %q: %v", fmt.Sprint(rx), err), msgAndArgs...)
	Fail(t, fmt.Sprintf("invalid regular expression %q: %v", rx, err), msgAndArgs...)

[ccoVeille]

This would be better, no?

Suggested change

	Fail(t, fmt.Sprintf("invalid regular expression %q: %v", fmt.Sprint(rx), err), msgAndArgs...)
	Fail(t, fmt.Sprintf("invalid regular expression %q: %v", rx, err), msgAndArgs...)

[kdt523]

Hi @ccoVeille — thanks for the suggestion, very helpful to question that line.

Quick reason I didn’t pass rx directly to %q: %q expects a string (or string-like) value. In our code rx is an interface{} and callers sometimes pass a compiled *regexp.Regexp. If you do fmt.Sprintf("%q", rx) with a *regexp.Regexp, Go prints an unhelpful %!q(...) type dump instead of the pattern. That makes the error message confusing.
Small examples:
rx := "start" → fmt.Sprintf("%q", rx) => "start" (good)
rx := regexp.MustCompile("start") → fmt.Sprintf("%q", rx) => %!q(*regexp.Regexp=...) (bad)
Safe alternatives

1.Keep the quoted style by converting rx to a string first:
fmt.Sprintf("invalid regular expression %q: %v", fmt.Sprint(rx), err)
Pros: preserves a quoted pattern; simple.

2.Don’t quote, just use %v:
fmt.Sprintf("invalid regular expression %v: %v", rx, err)
Pros: always works for any type; no weird %!q output. Cons: loses the quoted look.

3.(Preferred) Be explicit: if rx is a *regexp.Regexp, call .String(), otherwise fall back to fmt.Sprint:
var rxStr string
switch v := rx.(type) {
case *regexp.Regexp:
rxStr = v.String()
default:
rxStr = fmt.Sprint(rx)
}
Fail(t, fmt.Sprintf("invalid regular expression %q: %v", rxStr, err), msgAndArgs...)

Pros: most robust and readable; shows the actual pattern for compiled regexps and handles any other types cleanly.
If you’re OK with that, I can apply option 3 (tiny, explicit change) to the fix/assert-regexp-compile-error branch and push it. If you prefer a simpler tweak, I can apply option 1 instead — tell me which and I’ll open a small commit/PR.

[ccoVeille]

I thought %q would behave the same way %s with quote.

I checked https://cs.opensource.google/go/go/+/master:src/fmt/format.go

I'm a bit surprised it would differ.

Your reply sounds like it would, I will have to check further.

But for now, let's say the code can stay like you did.

[ccoVeille]

I'm still doubtful

https://go.dev/play/p/WBwS5ClJU0i

[kdt523]

You’re right that for *regexp.Regexp, %q and %s both output the pattern, with %q just adding quotes. In most cases, this isn’t harmful.

My main reason for switching to %v (or [fmt.Sprint]) was to avoid unnecessary quoting and ensure consistent, clear error messages—especially if the input isn’t a string or is a different type. This way, the formatting is robust for all cases, not just regex patterns.

If you find any edge cases where %q would be preferable, I’m happy to revisit! For now, I think the current approach is a bit safer and more general. Thanks again for your thoughtful feedback!