Skip to content

Ensure } and { are valid boundary characters #17001

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

Merged
merged 9 commits into from
Mar 6, 2025
Merged

Ensure } and { are valid boundary characters #17001

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
41eecaa
add `assert_extract_candidates_contains` helper
RobinMalfait Mar 6, 2025
84f3fe3
add failing test for `twig` syntax
RobinMalfait Mar 6, 2025
ea2eec5
ensure `}…{` are valid boundary characters
RobinMalfait Mar 6, 2025
7258503
cleanup unused import
RobinMalfait Mar 6, 2025
609b0af
update changelog
RobinMalfait Mar 6, 2025
0002af6
re-use `is_valid_after_boundary`
RobinMalfait Mar 6, 2025
f0ba1d2
add additional test case
RobinMalfait Mar 6, 2025
b5cfa04
drop `pug syntax` tests from `utility_machine`
RobinMalfait Mar 6, 2025
7f93659
accept `class` in `<div class="…">`
RobinMalfait Mar 6, 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
2 changes: 1 addition & 1 deletion crates/oxide/src/extractor/candidate_machine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ fn is_valid_before_boundary(c: &u8) -> bool {
/// E.g.: `<div class:flex="bool">` Svelte
/// ^
#[inline(always)]
fn is_valid_after_boundary(c: &u8) -> bool {
pub fn is_valid_after_boundary(c: &u8) -> bool {
is_valid_common_boundary(c) || matches!(c, b'}' | b']' | b'=' | b'{')
}

Expand Down
50 changes: 30 additions & 20 deletions crates/oxide/src/extractor/named_utility_machine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::cursor;
use crate::extractor::arbitrary_value_machine::ArbitraryValueMachine;
use crate::extractor::arbitrary_variable_machine::ArbitraryVariableMachine;
use crate::extractor::candidate_machine::is_valid_after_boundary;
use crate::extractor::machine::{Machine, MachineState};
use classification_macros::ClassifyBytes;

Expand Down Expand Up @@ -120,19 +121,22 @@ impl Machine for NamedUtilityMachine {
// E.g.: `:div="{ flex: true }"` (JavaScript object syntax)
// ^
Class::AlphaLower | Class::AlphaUpper => {
match cursor.next.into() {
Class::Quote
| Class::Whitespace
| Class::CloseBracket
| Class::Dot
| Class::Colon
| Class::End
| Class::Slash
| Class::Exclamation => return self.done(self.start_pos, cursor),

// Still valid characters
_ => cursor.advance(),
if is_valid_after_boundary(&cursor.next) || {
// Or any of these characters
//
// - `:`, because of JS object keys
// - `/`, because of modifiers
// - `!`, because of important
matches!(
cursor.next.into(),
Class::Colon | Class::Slash | Class::Exclamation
)
} {
return self.done(self.start_pos, cursor);
}

// Still valid characters
cursor.advance()
Copy link
Contributor

@thecrypticace thecrypticace Mar 6, 2025

Choose a reason for hiding this comment

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

This is fine for now but we'll want to perf test this. I wouldn't be surprised if this implementation causes a bit of a hit

Copy link
Member Author

@RobinMalfait RobinMalfait Mar 6, 2025

Choose a reason for hiding this comment

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

Yep will do some classification next, and I still have a WIP branch to get rid of the Cursor

}

Class::Dash => match cursor.next.into() {
Expand Down Expand Up @@ -213,14 +217,20 @@ impl Machine for NamedUtilityMachine {
// ^
// E.g.: `:div="{ flex: true }"` (JavaScript object syntax)
// ^
Class::Quote
| Class::Whitespace
| Class::CloseBracket
| Class::Dot
| Class::Colon
| Class::End
| Class::Slash
| Class::Exclamation => return self.done(self.start_pos, cursor),
_ if is_valid_after_boundary(&cursor.next) || {
// Or any of these characters
//
// - `:`, because of JS object keys
// - `/`, because of modifiers
// - `!`, because of important
matches!(
cursor.next.into(),
Class::Colon | Class::Slash | Class::Exclamation
)
} =>
{
return self.done(self.start_pos, cursor)
}
Copy link
Contributor

@thecrypticace thecrypticace Mar 6, 2025

Choose a reason for hiding this comment

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

ditto here too


// Everything else is invalid
_ => return self.restart(),
Expand Down