Skip to content

assert: report no match for a typed nil *regexp.Regexp instead of panicking - #1974

Open
feiiiiii5 wants to merge 1 commit into
stretchr:masterfrom
feiiiiii5:fix/regexp-nil-typed
Open

feiiiiii5 wants to merge 1 commit into
stretchr:masterfrom
feiiiiii5:fix/regexp-nil-typed

Conversation

@feiiiiii5

Copy link
Copy Markdown

Summary

Regexp and NotRegexp panic on a typed nil *regexp.Regexp instead of reporting a result.

Changes

  • matchRegexp (used by Regexp and NotRegexp): a typed nil *regexp.Regexp reports no match instead of being dereferenced.
  • One regression test covering both Regexp and NotRegexp.

Split out of #1971 at dolmen's request — the two causes are unrelated. That PR keeps the nil condition and non-positive tick guards, which are about arguments that reach time.NewTicker and the goroutine this package spawns.

Motivation

matchRegexp type-asserts rx to *regexp.Regexp before using it. A typed nil passes the assertion — it has the right type, and a nil value — so MatchString then dereferences it. The panic lands in the caller's test run with the trace pointing into this package rather than at the call, and there is no way for the caller to recover from it:

--- FAIL: TestRegexpNilTypedRegexp (0.00s)
        Error:  func (assert.PanicTestFunc)(0x10291e580) should not panic
               Panic value: runtime error: invalid memory address or nil pointer dereference
        panic({0x1029fdc20?, 0x1029fdc20?})

A typed nil here is a *regexp.Regexp that was declared and never assigned. There is no pattern behind it, so nothing can match — false, which NotRegexp inverts as usual. The alternative, treating it as an error, would change what callers of Regexp see for a value that is not a usable pattern either way, and would be a larger change than the bug needs.

Related issues

Split from #1971. No separate issue — happy to open one if you prefer the issue-first route.

…icking

matchRegexp type-asserts rx to *regexp.Regexp before use, so a typed
nil passes the assertion and MatchString then dereferences it. Both
Regexp and NotRegexp panic where a result was expected, in the caller's
test run, with the trace pointing into this package.

A typed nil is a *regexp.Regexp that was declared and never assigned.
There is no pattern behind it, so nothing can match: report false, and
let NotRegexp invert it as usual.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant