fix(repl): skip regex literals when validating input

Project: denoland/deno | Language: Rust | Concepts: error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/denoland/deno/pull/36375

The problem

The Deno REPL could not correctly handle JavaScript regex literals, especially when the pattern contained certain characters like quotes inside character classes (e.g., /[']/). The lexer, used for input validation, would misinterpret these as division operators and mismatched string literals, causing the REPL to either incorrectly report a syntax error or hang waiting for more input.

Why it's non-trivial

Naively counting slashes or relying on the lexer alone cannot distinguish between division operators and regex literals, especially since recognizing a regex literal requires parser-level context (expression position, etc.). A wrong guess can cause the REPL to block indefinitely, which is worse than letting the JavaScript engine (V8) handle ambiguous or erroneous input.

The approach

The fix reuses an existing helper, find_regex_literal_end, to accurately skip over regex literals during validation, and ensures that if a regex is skipped, the validator never reports an error—only suppresses one, letting V8 handle ambiguous cases.

Key code snippets and their roles:

Track skipped regex literals and their spans:

// Byte offset (in input) up to which the text has already been consumed by
// a detected regex literal; tokens starting before it must be skipped.
let mut skip_until = 0;
// Whether any regex literal was skipped. Deciding that a / starts a regex
// is a guess (we don't track expression position), so once we've acted on
// that guess we only ever suppress a validation error, never report one:
// a wrong "valid" is recovered by V8 reporting the real syntax error, but a
// wrong "invalid"/"incomplete" wedges the REPL waiting for input.

let mut skipped_regex = false;

*Tracks whether a regex literal was detected and up to which byte offset tokens should be skipped, and flags if any regex was skipped so that errors can be suppressed later.*

Skip tokens that are part of a regex literal:

if range.start < skip_until {

 // This token is part of a regex literal that was already consumed.

```
 continue;
```
}

*Prevents double-processing of tokens that are part of a regex literal.*

3. **Detect and skip regex literals using the helper:**
```diff
+        if let Some(regex_end) = find_regex_literal_end(input, range.start) {
+          skip_until = regex_end;
+          skipped_regex = true;
+          continue;
+        }

Attempts to scan for a regex literal at the current position. If found, skips all tokens covered by the regex.

Monotonicity guard: only suppress errors after skipping a regex:

let result = if let Some(error) = queued_validation_error {
error
} else if !stack.is_empty() || in_template || ends_with_dot {
// A trailing . means the user broke a method chain across lines (e.g.
// foo.\nbar()). Wait for more input.
ValidationResult::Incomplete
} else {
ValidationResult::Valid(None)
};
if skipped_regex && !matches!(result, ValidationResult::Valid(_)) {
// We guessed that a / started a regex and skipped over it. If that left
// the input looking unbalanced the guess was probably wrong, so hand it to
// V8 rather than rejecting it or blocking on more input. The skipped span
// can swallow one half of a ` pair or a trailing ., so this covers
// every non-Valid outcome rather than just a queued error or open stack.
// The suppression is whole-input, so one skipped regex on the first line
// also silences a genuine error further down; that keeps the guess strictly
// non-reporting, which is the property this relies on.
return ValidationResult::Valid(None);
}

result

*After computing the validation result, if a regex was skipped and the result is not `Valid`, suppress the error and let V8 handle it. This prevents the REPL from hanging or rejecting input based on a heuristic guess.*

Comprehensive tests for edge cases:

#[test]
fn validate_regex_literal_with_quotes() {
// ... various cases with quotes inside regex character classes ...
assert!(matches!(
```
 validate("let re = /[']/;"),
```
```
 ValidationResult::Valid(_)
```
));
// ... more cases ...
}
#[test]
fn validate_ambiguous_slash_never_reports_an_error() {
// ... ambiguous cases where slash could be division or regex ...
assert!(matches!(
```
 validate("if (x) /re/.test(y)"),
```
```
 ValidationResult::Valid(_)
```
));
// ... more cases ...

}

*Adds targeted tests for the previously failing cases and for ambiguous inputs, ensuring the validator never blocks or rejects input in these scenarios.*

Integration test to ensure REPL does not hang:

+#[test(flaky)] +fn pty_regex_literal_with_quote() {

// A quote inside a character class used to leave the validator waiting for a
// closing string, so the REPL blocked instead of evaluating the line.
// https://github.com/denoland/deno/issues/24963
util::with_pty(&["repl"], |mut console| {
console.write_line("let re = /[']/;");
console.write_line("re.test("'")");
console.expect("true");

}); +}

*Verifies end-to-end that the REPL no longer hangs when entering a regex with a quote inside a character class.*

From the review thread

bartlomieju: Nice diagnosis, and reusing find_regex_literal_end rather than porting the regex_allowed flag is the right call — the reasoning in the PR description about why the highlighter's heuristic isn't good enough for validation is exactly right.

One blocking issue though: the monotonicity guard is incomplete, so the invariant the PR is built on doesn't quite hold.

The final guard checks queued_validation_error and stack, but the Incomplete branch it's meant to neutralize also fires on in_template and ends_with_dot. So skipping a regex can still introduce a new Incomplete and wedge the REPL. Measured against the same build, base vs. this branch:

| input | main | this PR | |---|---|---| | a / `x / y` | Valid | Incomplete | | a / b / c. | Valid | Incomplete |

The first one is the sharp case: the slash-scan finds its "closing /" inside the template string, so the opening backtick ends up in the skipped span but the closing one doesn't. in_template toggles exactly once and the REPL blocks waiting for input.

Suggested fix — compute the result first, then suppress any non-Valid outcome:

let result = if let Some(error) = queued_validation_error { error } else if !stack.is_empty() || in_template || ends_with_dot { ValidationResult::Incomplete } else { ValidationResult::Valid(None) }; if skipped_regex && !matches!(result, ValidationResult::Valid(_)) { // We guessed that a / started a regex and skipped over it. If that left // the input looking unbalanced the guess was probably wrong, so hand it to // V8 rather than rejecting it or blocking on more input. return ValidationResult::Valid(None); } result


> I tried this locally: both cases above go back to `Valid`, `const re = /[` still
returns `Incomplete`, and all existing `tools::repl::editor` tests pass. Both
inputs would be good additions to `validate_ambiguous_slash_never_reports_an_error`
— as written that test only covers stack-balanced inputs, which is why it passes
despite the hole.

> xevrion: thanks, that was a good catch, I have reproduced both cases, and applied the restructure with all the four conditions. I have taken note of the three minor points too.
added a pty test in repl_tests.rs instead of tests/specs/repl/, since that dir runs scripts rather than driving an interactive repl; happy to move it if you'd prefer.

## Takeaways

- When heuristics are necessary (e.g., distinguishing regex literals from division), always prefer to suppress errors and defer to the language runtime rather than risk blocking or rejecting valid input.
- Reuse existing, well-tested helpers (like regex literal scanners) instead of duplicating logic or porting imperfect heuristics from unrelated subsystems (e.g., syntax highlighters).
- Comprehensive, targeted tests for edge cases and ambiguous scenarios are essential to prevent regressions, especially in user-interactive tools like REPLs.
- When suppressing errors based on a guess, ensure the suppression applies to all possible error states, not just the most obvious ones, to maintain monotonicity and avoid user-facing hangs.