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 all commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Ensure utilities are sorted based on their actual property order ([#16995](https://github.com/tailwindlabs/tailwindcss/pull/16995))
- Ensure strings in Pug and Slim templates are handled correctly ([#17000](https://github.com/tailwindlabs/tailwindcss/pull/17000))
- Ensure `}` and `{` are valid boundary characters when extracting candidates ([#17001](https://github.com/tailwindlabs/tailwindcss/pull/17001))

## [4.0.11] - 2025-03-06

Expand Down
17 changes: 10 additions & 7 deletions crates/oxide/src/extractor/candidate_machine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ fn is_valid_common_boundary(c: &u8) -> bool {
/// A candidate must be preceded by any of these characters.
#[inline(always)]
fn is_valid_before_boundary(c: &u8) -> bool {
is_valid_common_boundary(c) || matches!(c, b'.')
is_valid_common_boundary(c) || matches!(c, b'.' | b'}')
}

/// A candidate must be followed by any of these characters.
Expand All @@ -200,8 +200,8 @@ 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 {
is_valid_common_boundary(c) || matches!(c, b'}' | b']' | b'=')
pub fn is_valid_after_boundary(c: &u8) -> bool {
is_valid_common_boundary(c) || matches!(c, b'}' | b']' | b'=' | b'{')
}

#[inline(always)]
Expand Down Expand Up @@ -316,13 +316,16 @@ mod tests {
//
// HTML
// Inside a class (on its own)
(r#"<div class="{}"></div>"#, vec![]),
(r#"<div class="{}"></div>"#, vec!["class"]),
// Inside a class (first)
(r#"<div class="{} foo"></div>"#, vec!["foo"]),
(r#"<div class="{} foo"></div>"#, vec!["class", "foo"]),
// Inside a class (second)
(r#"<div class="foo {}"></div>"#, vec!["foo"]),
(r#"<div class="foo {}"></div>"#, vec!["class", "foo"]),
// Inside a class (surrounded)
(r#"<div class="foo {} bar"></div>"#, vec!["foo", "bar"]),
(
r#"<div class="foo {} bar"></div>"#,
vec!["class", "foo", "bar"],
),
// --------------------------
//
// JavaScript
Expand Down
62 changes: 53 additions & 9 deletions crates/oxide/src/extractor/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,22 @@ mod tests {
assert_eq!(actual, expected);
}

fn assert_extract_candidates_contains(input: &str, expected: Vec<&str>) {
let actual = extract_sorted_candidates(input);

let mut missing = vec![];
for item in &expected {
if !actual.contains(item) {
missing.push(item);
}
}

if !missing.is_empty() {
dbg!(&actual, &missing);
panic!("Missing some items");
}
}

fn assert_extract_sorted_css_variables(input: &str, expected: Vec<&str>) {
let actual = extract_sorted_css_variables(input);

Expand Down Expand Up @@ -311,6 +327,7 @@ mod tests {
(
r#"<div class="flex items-center px-2.5 bg-[#0088cc] text-(--my-color)"></div>"#,
vec![
"class",
"flex",
"items-center",
"px-2.5",
Expand Down Expand Up @@ -363,7 +380,7 @@ mod tests {
("{ underline: true }", vec!["underline", "true"]),
(
r#" <CheckIcon className={clsx('h-4 w-4', { invisible: index !== 0 })} />"#,
vec!["h-4", "w-4", "invisible", "index"],
vec!["className", "h-4", "w-4", "invisible", "index"],
),
// You can have variants but in a string. Vue example.
(
Expand Down Expand Up @@ -480,13 +497,16 @@ mod tests {
//
// HTML
// Inside a class (on its own)
(r#"<div class="{}"></div>"#, vec![]),
(r#"<div class="{}"></div>"#, vec!["class"]),
// Inside a class (first)
(r#"<div class="{} foo"></div>"#, vec!["foo"]),
(r#"<div class="{} foo"></div>"#, vec!["class", "foo"]),
// Inside a class (second)
(r#"<div class="foo {}"></div>"#, vec!["foo"]),
(r#"<div class="foo {}"></div>"#, vec!["class", "foo"]),
// Inside a class (surrounded)
(r#"<div class="foo {} bar"></div>"#, vec!["foo", "bar"]),
(
r#"<div class="foo {} bar"></div>"#,
vec!["class", "foo", "bar"],
),
// --------------------------
//
// JavaScript
Expand Down Expand Up @@ -590,7 +610,7 @@ mod tests {
// Quoted attribute
(
r#"input(type="checkbox" class="px-2.5")"#,
vec!["checkbox", "px-2.5"],
vec!["checkbox", "class", "px-2.5"],
),
] {
assert_extract_sorted_candidates(&pre_process_input(input, "pug"), expected);
Expand All @@ -611,7 +631,7 @@ mod tests {
vec!["bg-blue-100", "2xl:bg-red-100"],
),
// Quoted attribute
(r#"div class="px-2.5""#, vec!["div", "px-2.5"]),
(r#"div class="px-2.5""#, vec!["div", "class", "px-2.5"]),
] {
assert_extract_sorted_candidates(&pre_process_input(input, "slim"), expected);
}
Expand Down Expand Up @@ -831,6 +851,25 @@ mod tests {
&pre_process_input(r#"<div class:px-4='condition'></div>"#, "svelte"),
vec!["class", "px-4", "condition"],
);
assert_extract_sorted_candidates(
&pre_process_input(r#"<div class:flex='condition'></div>"#, "svelte"),
vec!["class", "flex", "condition"],
);
}

// https://github.com/tailwindlabs/tailwindcss/issues/16999
#[test]
fn test_twig_syntax() {
assert_extract_candidates_contains(
r#"<div class="flex items-center mx-4{% if session.isValid %}{% else %} h-4{% endif %}"></div>"#,
vec!["flex", "items-center", "mx-4", "h-4"],
);

// With touching both `}` and `{`
assert_extract_candidates_contains(
r#"<div class="{% if true %}flex{% else %}block{% endif %}">"#,
vec!["flex", "block"],
);
}

// https://github.com/tailwindlabs/tailwindcss/issues/16982
Expand All @@ -839,6 +878,7 @@ mod tests {
assert_extract_sorted_candidates(
r#"<div class="@md:flex @max-md:flex @-[36rem]:flex @[36rem]:flex"></div>"#,
vec![
"class",
"@md:flex",
"@max-md:flex",
"@-[36rem]:flex",
Expand All @@ -852,7 +892,7 @@ mod tests {
fn test_classes_containing_number_followed_by_dash_or_underscore() {
assert_extract_sorted_candidates(
r#"<div class="text-Title1_Strong"></div>"#,
vec!["text-Title1_Strong"],
vec!["class", "text-Title1_Strong"],
);
}

Expand All @@ -861,7 +901,11 @@ mod tests {
fn test_arbitrary_variable_with_data_type() {
assert_extract_sorted_candidates(
r#"<div class="bg-(length:--my-length) bg-[color:var(--my-color)]"></div>"#,
vec!["bg-(length:--my-length)", "bg-[color:var(--my-color)]"],
vec![
"class",
"bg-(length:--my-length)",
"bg-[color:var(--my-color)]",
],
);
}

Expand Down
63 changes: 38 additions & 25 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 Expand Up @@ -454,15 +464,15 @@ mod tests {
//
// HTML
// Inside a class (on its own)
(r#"<div class="{}"></div>"#, vec!["div"]),
(r#"<div class="{}"></div>"#, vec!["div", "class"]),
// Inside a class (first)
(r#"<div class="{} foo"></div>"#, vec!["div", "foo"]),
(r#"<div class="{} foo"></div>"#, vec!["div", "class", "foo"]),
// Inside a class (second)
(r#"<div class="foo {}"></div>"#, vec!["div", "foo"]),
(r#"<div class="foo {}"></div>"#, vec!["div", "class", "foo"]),
// Inside a class (surrounded)
(
r#"<div class="foo {} bar"></div>"#,
vec!["div", "foo", "bar"],
vec!["div", "class", "foo", "bar"],
),
// --------------------------
//
Expand All @@ -475,7 +485,10 @@ mod tests {
vec!["let", "classes", "true"],
),
// Inside an object (no spaces, key)
(r#"let classes = {'{}':true};"#, vec!["let", "classes"]),
(
r#"let classes = {'{}':true};"#,
vec!["let", "classes", "true"],
),
// Inside an object (value)
(
r#"let classes = { primary: '{}' };"#,
Expand Down
15 changes: 8 additions & 7 deletions crates/oxide/src/extractor/utility_machine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,6 @@ mod tests {
"bg-(--my-color) flex px-(--my-padding)",
vec!["bg-(--my-color)", "flex", "px-(--my-padding)"],
),
// Pug syntax
(".flex.bg-red-500", vec!["flex", "bg-red-500"]),
// --------------------------------------------------------

// Exceptions:
Expand All @@ -293,15 +291,15 @@ mod tests {
//
// HTML
// Inside a class (on its own)
(r#"<div class="{}"></div>"#, vec!["div"]),
(r#"<div class="{}"></div>"#, vec!["div", "class"]),
// Inside a class (first)
(r#"<div class="{} foo"></div>"#, vec!["div", "foo"]),
(r#"<div class="{} foo"></div>"#, vec!["div", "class", "foo"]),
// Inside a class (second)
(r#"<div class="foo {}"></div>"#, vec!["div", "foo"]),
(r#"<div class="foo {}"></div>"#, vec!["div", "class", "foo"]),
// Inside a class (surrounded)
(
r#"<div class="foo {} bar"></div>"#,
vec!["div", "foo", "bar"],
vec!["div", "class", "foo", "bar"],
),
// --------------------------
//
Expand All @@ -314,7 +312,10 @@ mod tests {
vec!["let", "classes", "true"],
),
// Inside an object (no spaces, key)
(r#"let classes = {'{}':true};"#, vec!["let", "classes"]),
(
r#"let classes = {'{}':true};"#,
vec!["let", "classes", "true"],
),
// Inside an object (value)
(
r#"let classes = { primary: '{}' };"#,
Expand Down
3 changes: 2 additions & 1 deletion crates/oxide/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::scanner::allowed_paths::resolve_paths;
use crate::scanner::detect_sources::DetectSources;
use bexpand::Expression;
use bstr::ByteSlice;
use extractor::string_machine::StringMachine;
use extractor::{Extracted, Extractor};
use fast_glob::glob_match;
use fxhash::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -541,6 +540,7 @@ mod tests {
(
r#"<div class="!tw__flex sm:!tw__block tw__bg-gradient-to-t flex tw:[color:red] group-[]:tw__flex"#,
vec![
("class".to_string(), 5),
("!tw__flex".to_string(), 12),
("sm:!tw__block".to_string(), 22),
("tw__bg-gradient-to-t".to_string(), 36),
Expand All @@ -553,6 +553,7 @@ mod tests {
(
r#"<div class="tw:flex! tw:sm:block! tw:bg-linear-to-t flex tw:[color:red] tw:in-[.tw\:group]:flex"></div>"#,
vec![
("class".to_string(), 5),
("tw:flex!".to_string(), 12),
("tw:sm:block!".to_string(), 21),
("tw:bg-linear-to-t".to_string(), 34),
Expand Down