Repository navigation
Don't let quotes in Pug regex literals hide the end of an attribute list - #20572
Conversation
…he line A quote that doesn't close on its line, like the `"` in a regex such as `input(pattern=/"/.source)`, hid the `)` of the attribute list, so the list stayed open and class shorthands on later lines were not extracted. Scan the rest of the line again with quotes as text instead.
 [Medium impact] Changes how Pug template attribute parsing handles quotes. The PR still needs the existing fix for regex quotes followed by real quoted attribute values before merging. Reviews (3) · Last reviewed commit: "Merge branch 'main' into robin/pug-regex..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| quotes_are_text = true; | ||
| cursor.move_to(quote_start + 1); |
There was a problem hiding this comment.
For input(pattern=/"/.source title='a (b') followed by span.underline.font-bold km, the replay disables the real single quotes too. The ( inside title is pushed onto bracket_stack, so the attribute list's ) closes that bracket instead of the list. The next line's dots stay unchanged, and underline is missed.
Preserve real quoted values during recovery and add this combined regression case.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughPug::process now records the opening position of quoted values. If a non-template quote remains open at a newline or the end of input, the processor resumes scanning just after that opening quote and treats quotes as text. It resets this mode at the next newline. New tests cover regex expressions containing quotes in Pug attributes, including input without a trailing newline. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description clearly explains the Pug parsing bug, the fix, the regression tests, and the test command. |
| Title check | |
The title clearly identifies the main change: preventing quotes in Pug regex literals from hiding the end of an attribute list. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Preserve the attribute stack on mismatched delimiters. · pug.rs:58-79
crates/oxide/src/extractor/pre_processors/pug.rs:58-79
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the attribute stack on mismatched delimiters.
When the rescan handles
input(pattern=/[")]/.source)across a line break, the)and]inside the regex character class do not match the outer attribute-list delimiter.BracketStack::popstill removes entries before checking the delimiter, so the rescan closes the attribute context early. A following multiline attribute such ashref="x.flex"is then processed outside the attribute list, and its dot is replaced with a space. Make mismatched pops non-mutating.Suggested fix
if self.bracket_stack_len == 0 { return false; } - self.bracket_stack_len -= 1; unsafe { if *self.bracket_stack.get_unchecked(self.bracket_stack_len) != bracket { return false; } } + self.bracket_stack_len -= 1; true
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
e6cbf47e-07e9-4730-ae33-63f155cef0c5
📒 Files selected for processing: 1
crates/oxide/src/extractor/pre_processors/pug.rs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
A regex quote on the last line of a file without a trailing newline kept its string open until the end, so the line was not scanned again and class shorthands after the attribute list were not extracted. E.g.: `div(data-pattern=/"/.source): span.underline.font-bold`
1 participant
This is a follow-up to #20547.
That PR tracks strings inside Pug attribute lists, so that brackets in values like
a(title="a (b")are ignored. But a quote that isn't a string delimiter, like the"in a regex such asinput(pattern=/"/.source), opened a string that never closed. That string hid the)of the attribute list. The list stayed open, so dots in class shorthands on later lines were not turned into spaces, and classes likeunderlineinspan.underline.font-boldwere not extracted.Solution
Only template literals can span lines in JavaScript. So when a
'or"string reaches the end of its line without a closing quote, it was not a string. In that case, we go back to just after the quote and scan the rest of the line again, with quotes as plain text. Each line is scanned at most twice, so this stays linear.Detecting regex literals directly needs a JavaScript tokenizer, because a
/can also be a division. This approach also covers other unpaired quotes, like// don't cachein ascript.block.The regression is not released yet, so there is no changelog entry: the existing entry of #20547 covers it.
Test plan
Added cases to
test_unbalanced_brackets_in_quoted_attribute_valuesinpug.rs. They fail without this change.