Page MenuHomePhabricator

"This regular expression matches the empty string" false positive with variables
Open, Needs TriagePublicBUG REPORT

Description

Example code to reproduce:

added_lines irlike timestamp

This is a regression of T273809, likely because we use VariableManager::GET_BC which returns DNULL for variables, not DUNDEFINED, so that fix doesn't work anymore. I think perhaps the GET_ mode could be injected into the FilterEvaluator, and anything except for actually checking the filters would use GET_STRICT or GET_LAX, whereas we'd still use GET_BC when checking filters.

Event Timeline

See also T283966.

The explanation might be that T273809 hid the editor warnings, but because it was passing an invalid pattern (no longer "munged"), thus never matching. Then T283966 fixed that issue, and the editor warnings came back.

(for the record, also refs Gerrit 720412, where the preg_match() has been prepended with a @ to drop the PHP warnings)

Also note we are encountering this editor warning as soon as a variable is used in the pattern, even if the pattern has other parts:

'haystack' rlike (rescape(user_name) + 'foo'); /* warning here */

bar := rescape(user_name) + 'bar';
'haystack' rlike (bar); /* warning here */

I really don't understand what causes the editor warning…

('foo' + rescape(user_name) + 'bar'); /* gives "foobar" */

/* but... */
"haystack" rlike ('foo' + rescape(user_name) + 'bar'); /* warning! */

The unset variable seems to be a DNULL. But whether it would be a DNULL (casts to an empty string, concatenated to the other strings) or a DUNDEFINED (adding to the other strings results in DUNDEFINED), in both cases checkRegexMatchesEmpty() shouldn't emit the editor warning…