Skip to content

Skip comments in Ruby files when checking for class names #19243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+136 −5
Open

Skip comments in Ruby files when checking for class names #19243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c29506b
Skip over strings in Ruby files
thecrypticace Oct 31, 2025
fa2be75
Replace comments in Ruby code
thecrypticace Oct 31, 2025
128775d
Discard arbitrary properties with `!` at the top-level
thecrypticace Oct 31, 2025
1ed473a
Fix formatting
thecrypticace Oct 31, 2025
41d3f41
Add tests
thecrypticace Nov 11, 2025
bff4739
Support single line string literal specifiers
thecrypticace Nov 11, 2025
ef3a8cc
Only match expected string literal specifiers
thecrypticace Nov 11, 2025
c19d255
Fix formatting
thecrypticace Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions crates/oxide/src/extractor/arbitrary_property_machine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,18 @@ impl Machine for ArbitraryPropertyMachine<ParsingValueState> {
return self.restart()
}

// An `!` at the top-level must be followed by "important" *and* be at the end
// otherwise its invalid
Class::Exclamation if self.bracket_stack.is_empty() => {
if cursor.input[cursor.pos..].starts_with(b"!important]") {
cursor.advance_by(10);

return self.done(self.start_pos, cursor);
}

return self.restart();
}
Comment on lines +237 to +244
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Handle !important case-insensitively.

starts_with(b"!important]") now rejects valid ASCII-insensitive variants like [color:red!IMPORTANT], which CSS accepts and the prior implementation extracted. Please keep the comparison case-insensitive so we don’t regress these inputs.

Apply this diff to make the comparison ASCII-insensitive:

-                    if cursor.input[cursor.pos..].starts_with(b"!important]") {
-                        cursor.advance_by(10);
+                    const IMPORTANT: &[u8; 10] = b"!important";
+                    let remaining = &cursor.input[cursor.pos..];
+
+                    if remaining.len() >= IMPORTANT.len() + 1
+                        && remaining[IMPORTANT.len()] == b']'
+                        && remaining[..IMPORTANT.len()]
+                            .iter()
+                            .zip(IMPORTANT)
+                            .all(|(byte, expected)| byte.to_ascii_lowercase() == *expected)
+                    {
+                        cursor.advance_by(IMPORTANT.len());

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In crates/oxide/src/extractor/arbitrary_property_machine.rs around lines
237-244, the code currently uses starts_with(b"!important]") which is
case-sensitive; replace it with an ASCII-insensitive check by first ensuring the
remaining slice has at least 10 bytes (use get(..10) or a bounds check), then
compare each byte lowercased (byte.to_ascii_lowercase()) against the literal
b"!important]" (or lowercased literal) — if they all match, advance by 10 and
call done(...), otherwise restart; this preserves ASCII case-insensitivity and
avoids panics on short slices.


// Everything else is valid
_ => cursor.advance(),
};
Expand Down Expand Up @@ -293,6 +305,9 @@ enum Class {
#[bytes(b'/')]
Slash,

#[bytes(b'!')]
Exclamation,

#[bytes(b' ', b'\t', b'\n', b'\r', b'\x0C')]
Whitespace,

Expand Down Expand Up @@ -369,6 +384,10 @@ mod tests {
"[background:url(https://example.com?q={[{[([{[[2]]}])]}]})]",
vec!["[background:url(https://example.com?q={[{[([{[[2]]}])]}]})]"],
),
// A property containing `!` at the top-level is invalid
("[color:red!]", vec![]),
// Unless its part of `!important at the end
("[color:red!important]", vec!["[color:red!important]"]),
] {
for wrapper in [
// No wrapper
Expand Down
120 changes: 117 additions & 3 deletions crates/oxide/src/extractor/pre_processors/ruby.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,74 @@ impl PreProcessor for Ruby {

// Ruby extraction
while cursor.pos < len {
// Looking for `%w` or `%W`
if cursor.curr != b'%' && !matches!(cursor.next, b'w' | b'W') {
match cursor.curr {
b'"' => {
cursor.advance();

while cursor.pos < len {
match cursor.curr {
// Escaped character, skip ahead to the next character
b'\\' => cursor.advance_twice(),

// End of the string
b'"' => break,

// Everything else is valid
_ => cursor.advance(),
};
}

cursor.advance();
continue;
}

b'\'' => {
cursor.advance();

while cursor.pos < len {
match cursor.curr {
// Escaped character, skip ahead to the next character
b'\\' => cursor.advance_twice(),

// End of the string
b'\'' => break,

// Everything else is valid
_ => cursor.advance(),
};
}

cursor.advance();
continue;
}
Comment on lines +81 to +119
Copy link
Member

@RobinMalfait RobinMalfait Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some tests where we handle quotes? We have to be careful with these because it could be that you start a sentence with a ' in it and never close it.

%p a quote here can't exist, or can it 
-#
  This won't be displayed (there is a quote here as well)
    Nor will this
                   Nor will this.
%p a quote here can't exist, or can it

Copy link
Contributor Author

@thecrypticace thecrypticace Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added tests and updated the implementation a bit


// Replace comments in Ruby files
b'#' => {
result[cursor.pos] = b' ';
cursor.advance();

while cursor.pos < len {
match cursor.curr {
// End of the comment
b'\n' => break,

// Everything else is part of the comment and replaced
_ => {
result[cursor.pos] = b' ';
cursor.advance();
}
};
}

cursor.advance();
continue;
}

_ => {}
}

// Looking for `%w`, `%W`, or `%p`
if cursor.curr != b'%' || !matches!(cursor.next, b'w' | b'W' | b'p') {
cursor.advance();
continue;
}
Expand All @@ -90,6 +156,7 @@ impl PreProcessor for Ruby {
b'[' => b']',
b'(' => b')',
b'{' => b'}',
b' ' => b'\n',
_ => {
cursor.advance();
continue;
Expand Down Expand Up @@ -131,7 +198,10 @@ impl PreProcessor for Ruby {

// End of the pattern, replace the boundary character with a space
_ if cursor.curr == boundary => {
result[cursor.pos] = b' ';
if boundary != b'\n' {
result[cursor.pos] = b' ';
}

break;
}

Expand Down Expand Up @@ -173,12 +243,46 @@ mod tests {
"%w(flex data-[state=pending]:bg-(--my-color) flex-col)",
"%w flex data-[state=pending]:bg-(--my-color) flex-col ",
),

// %w …\n
("%w flex px-2.5\n", "%w flex px-2.5\n"),

// Use backslash to embed spaces in the strings.
(r#"%w[foo\ bar baz\ bat]"#, r#"%w foo bar baz bat "#),
(r#"%W[foo\ bar baz\ bat]"#, r#"%W foo bar baz bat "#),

// The nested delimiters evaluated to a flat array of strings
// (not nested array).
(r#"%w[foo[bar baz]qux]"#, r#"%w foo[bar baz]qux "#),

(
"# test\n# test\n# {ActiveRecord::Base#save!}[rdoc-ref:Persistence#save!]\n%w[flex px-2.5]",
" \n \n \n%w flex px-2.5 "
),

(r#""foo # bar""#, r#""foo # bar""#),
(r#"'foo # bar'"#, r#"'foo # bar'"#),
(
r#"def call = tag.span "Foo", class: %w[rounded-full h-0.75 w-0.75]"#,
r#"def call = tag.span "Foo", class: %w rounded-full h-0.75 w-0.75 "#
),

(r#"%w[foo ' bar]"#, r#"%w foo ' bar "#),
(r#"%w[foo " bar]"#, r#"%w foo " bar "#),
(r#"%W[foo ' bar]"#, r#"%W foo ' bar "#),
(r#"%W[foo " bar]"#, r#"%W foo " bar "#),

(r#"%p foo ' bar "#, r#"%p foo ' bar "#),
(r#"%p foo " bar "#, r#"%p foo " bar "#),

(
"%p has a ' quote\n# this should be removed\n%p has a ' quote",
"%p has a ' quote\n \n%p has a ' quote"
),
(
"%p has a \" quote\n# this should be removed\n%p has a \" quote",
"%p has a \" quote\n \n%p has a \" quote"
),
] {
Ruby::test(input, expected);
}
Expand Down Expand Up @@ -211,6 +315,16 @@ mod tests {
"%w(flex data-[state=pending]:bg-(--my-color) flex-col)",
vec!["flex", "data-[state=pending]:bg-(--my-color)", "flex-col"],
),

(
"# test\n# test\n# {ActiveRecord::Base#save!}[rdoc-ref:Persistence#save!]\n%w[flex px-2.5]",
vec!["flex", "px-2.5"],
),

(r#""foo # bar""#, vec!["foo", "bar"]),
(r#"'foo # bar'"#, vec!["foo", "bar"]),

(r#"%w[foo ' bar]"#, vec!["foo", "bar"]),
] {
Ruby::test_extract_contains(input, expected);
}
Expand Down
2 changes: 0 additions & 2 deletions crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
.relative
^^^^^^^^
- # Blurred background star
^^^^^^^^^^ ^^^^
.absolute.left-0.z-0{ class: "-top-[400px] -right-[400px]" }
^^^^^^^^ ^^^^^^ ^^^ ^^^^^ ^^^^^^^^^^^^ ^^^^^^^^^^^^^^
.flex.justify-end.blur-3xl
Expand Down Expand Up @@ -196,7 +195,6 @@
^^^^^^^^ ^^^^ ^^^ ^^^^ ^^
:escaped
- # app/components/character_component.html.haml
^^^^
= part(:component) do
^^
= part(:head)
Expand Down