`regex_syntax::escape()` does not escape whitespace

[H2CO3]

What version of regex are you using?

If it isn't the latest version, then please upgrade and check whether the bug
is still present.

I'm using the latest version, 0.8.8 on the Rust playground as of today.

Describe the bug at a high level.

The regex_escape() function does not escape whitespace characters, which leads to them being incorrectly ignored in verbose mode. Since the documentation states:

The string returned may be safely used as a literal in a regular expression.

I think the less surprising behavior would be to escape whitespace characters as well.

What are the steps to reproduce the behavior?

See the following code (Playground):

use regex_syntax; // 0.8.8
use regex::Regex;

fn main() {
    let pattern = format!("^(?x:{lit})$", lit = regex_syntax::escape(" "));
    let regex = Regex::new(&pattern).unwrap();
    let m = regex.captures(" ").unwrap(); // one literal space
    // this panics because the literal space in the pattern does not match the space in the haystack
    dbg!(m.get_match().as_str(), m.get_match().range());
}

What is the actual behavior?

As per the aforementioned comment, this panics, demonstrating that the literal, un-escaped space pattern does not match the space in the haystack if verbose mode is enabled, which is surprising:

   Compiling playground v0.0.1 (/playground)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.82s
     Running `target/debug/playground`

thread 'main' (13) panicked at src/main.rs:7:33:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

What is the expected behavior?

I would expect escaping to take the behavior of possible flags (e.g. ?x) into account, and escape whitespaces. For example, given the following function successfully accomplishes this task (Playground):

fn regex_escape(s: &str) -> String {
    let mut result = String::with_capacity(s.len());
    let mut last_idx = 0;
    
    for (i, p) in s.match_indices(regex_syntax::is_escapeable_character) {
        result.push_str(unsafe { 
            s.get_unchecked(last_idx..i)
        });
        last_idx = i + p.len();
        write!(result, "{}", p.escape_unicode()).unwrap();
    }
    
    result.push_str(unsafe {
        s.get_unchecked(last_idx..)
    });
    
    dbg!(result)
}

[BurntSushi]

I don't think this is typical of escaping routines in other regex engines. For example, Go's regexp.QuoteMeta doesn't escape whitespace.

I would expect escaping to take the behavior of possible flags (e.g. ?x) into account

Broadly speaking, I'm not sure this is actually reasonable. For example, if you escape abc you get abc. But if you have the i flag set, then abc will also match ABC and is thus not, per se, a true literal. Now, it doesn't result in the regex becoming invalid, but it is arguably covered by what you're saying here.

What is a reason not to do this? I think there's an implied contract here that escaping a regex does the smallest thing it can do. Turning every possible escapeable character into an escape sequence---even when it's never necessary---seems like a bridge too far. But maybe limiting it to whitespace is reasonable?

I think I'd want to survey the escaping routines of other regex libraries before making a decision here.

[H2CO3]

I think there's an implied contract here that escaping a regex does the smallest thing it can do.

Hmm, I'm not sure I follow. Where does that implication come from? Isn't the point of escaping to safely include literals in a pattern? If not, what is it, and if so, why is it acceptable for it to not fulfil its purpose?

For example, if you escape abc you get abc. But if you have the i flag set, then abc will also match ABC and is thus not, per se, a true literal.

I don't think the two cases are comparable – case insensitively matching something when you explicitly asked for it is not surprising. A literal not being matched with its exact counterpart in the haystack is highly surprising.

[BurntSushi]

Where does that implication come from?

From the current behavior and how other regex escape routines work. They don't escape every possible thing. And in the limit, every character could be turned into a Unicode escape. But I would contend that end users would find this undesirable because escaping things that don't need to be escaped makes the result more difficult for humans to read. This is the pressure against escaping whitespace.

I don't think the two cases are comparable – case insensitively matching something when you explicitly asked for it is not surprising. A literal not being matched with its exact counterpart in the haystack is highly surprising.

I don't know what the significance of "not comparable" means in this context. They are obviously different. I'm just going by the maxim you proposed and trying to tease it apart.

[H2CO3]

I don't know what the significance of "not comparable" means in this context.

I just meant that citing the behavior-changing nature of ignoring case seems irrelevant when trying to argue against escaping WS, because the former doesn't cause badly surprising results/bugs, while the latter does.

[BurntSushi]

It's surprising if you start with the broad premise that escaping takes regex flags into account.

[H2CO3]

It's surprising, period – even if you don't immediately think about how flags and escaping might interact. (That's the whole point, in fact.) Calling a function that promises safe interpolation of the result into a regex will inevitably lead to the expectation that it should work.

[BurntSushi]

I'm responding to this statement you made, which is very broad:

I would expect escaping to take the behavior of possible flags (e.g. ?x) into account

And "it should work" is itself also very broad. If you expect "work" to not just mean "the regex is valid" but that "the escaped text is matched literally" then your expectation will be violated by the i flag.