Disallow empty arbitrary values (#15055)

This PR makes the candidate parser more strict by not allowing empty
arbitrary values.

Examples that are not allowed anymore:

- `bg-[]` — arbitrary value
- `bg-()` — arbitrary value, var shorthand
- `bg-[length:]` — arbitrary value, with typehint
- `bg-(length:)` — arbitrary value, with typehint, var shorthand
- `bg-red-500/[]` — arbitrary modifier
- `bg-red-500/()` — arbitrary modifier, var shorthand
- `data-[]:flex` — arbitrary value for variant
- `data-():flex` — arbitrary value for variant, var shorthand
- `group-visible/[]:flex` — arbitrary modifier for variant
- `group-visible/():flex` — arbitrary modifier for variant, var
shorthand

If you are trying to trick the parser by injecting some spaces like
this:

- `bg-[_]`

Then that is also not allowed.

Author: RobinMalfait

CHANGELOG.md

@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Use single drop shadow values instead of multiple ([#15056](https://github.com/tailwindlabs/tailwindcss/pull/15056))
+- Do not parse invalid candidates with empty arbitrary values ([#15055](https://github.com/tailwindlabs/tailwindcss/pull/15055))
 
 ## [4.0.0-alpha.35] - 2024-11-20
 

crates/oxide/src/parser.rs

@@ -538,14 +538,17 @@ impl<'a> Extractor<'a> {
                         return ParseAction::Consume;
                     }
 
-                    if let Arbitrary::Brackets { start_idx } = self.arbitrary {
+                    if let Arbitrary::Brackets { start_idx: _ } = self.arbitrary {
                         trace!("Arbitrary::End\t");
                         self.arbitrary = Arbitrary::None;
 
-                        if self.cursor.pos - start_idx == 1 {
-                            // We have an empty arbitrary value, which is not allowed
-                            return ParseAction::Skip;
-                        }
+                        // TODO: This is temporarily disabled such that the upgrade tool can work
+                        // with legacy arbitrary values. This will be re-enabled in the future (or
+                        // with a flag)
+                        // if self.cursor.pos - start_idx == 1 {
+                        //     // We have an empty arbitrary value, which is not allowed
+                        //     return ParseAction::Skip;
+                        // }
                     }
                 }
 
@@ -1517,4 +1520,23 @@ mod test {
 
         assert_eq!(candidates, vec![("div", 1), ("class", 5), ("flex", 12),]);
     }
+
+    #[test]
+    fn empty_arbitrary_values_are_allowed_for_codemods() {
+        let candidates = run(
+            r#"<div class="group-[]:flex group-[]/name:flex peer-[]:flex peer-[]/name:flex"></div>"#,
+            false,
+        );
+        assert_eq!(
+            candidates,
+            vec![
+                "div",
+                "class",
+                "group-[]:flex",
+                "group-[]/name:flex",
+                "peer-[]:flex",
+                "peer-[]/name:flex"
+            ]
+        );
+    }
 }

integrations/upgrade/index.test.ts

@@ -68,7 +68,7 @@ test(
       'src/index.html': html`
         <h1>🤠👋</h1>
         <div
-          class="!flex sm:!block bg-gradient-to-t bg-[--my-red] max-w-screen-md ml-[theme(screens.md)]"
+          class="!flex sm:!block bg-gradient-to-t bg-[--my-red] max-w-screen-md ml-[theme(screens.md)] group-[]:flex"
         ></div>
         <!-- Migrate to sm -->
         <div class="blur shadow rounded inset-shadow drop-shadow"></div>
@@ -100,7 +100,7 @@ test(
       --- ./src/index.html ---
       <h1>🤠👋</h1>
       <div
-        class="flex! sm:block! bg-linear-to-t bg-(--my-red) max-w-(--breakpoint-md) ml-(--breakpoint-md)"
+        class="flex! sm:block! bg-linear-to-t bg-(--my-red) max-w-(--breakpoint-md) ml-(--breakpoint-md) in-[.group]:flex"
       ></div>
       <!-- Migrate to sm -->
       <div class="blur-sm shadow-sm rounded-sm inset-shadow-sm drop-shadow-sm"></div>
@@ -194,7 +194,7 @@ test(
       `,
       'src/index.html': html`
         <div
-          class="!tw__flex sm:!tw__block tw__bg-gradient-to-t flex [color:red]"
+          class="!tw__flex sm:!tw__block tw__bg-gradient-to-t flex [color:red] group-[]:tw__flex"
         ></div>
       `,
       'src/input.css': css`
@@ -215,7 +215,7 @@ test(
       "
       --- ./src/index.html ---
       <div
-        class="tw:flex! tw:sm:block! tw:bg-linear-to-t flex tw:[color:red]"
+        class="tw:flex! tw:sm:block! tw:bg-linear-to-t flex tw:[color:red] tw:in-[.tw\\:group]:flex"
       ></div>
 
       --- ./src/input.css ---

packages/@tailwindcss-upgrade/src/template/codemods/handle-empty-arbitrary-values.test.ts

@@ -0,0 +1,37 @@
+import { __unstable__loadDesignSystem } from '@tailwindcss/node'
+import { expect, test } from 'vitest'
+import { handleEmptyArbitraryValues } from './handle-empty-arbitrary-values'
+import { prefix } from './prefix'
+
+test.each([
+  ['group-[]:flex', 'group-[&]:flex'],
+  ['group-[]/name:flex', 'group-[&]/name:flex'],
+
+  ['peer-[]:flex', 'peer-[&]:flex'],
+  ['peer-[]/name:flex', 'peer-[&]/name:flex'],
+])('%s => %s (%#)', async (candidate, result) => {
+  let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', {
+    base: __dirname,
+  })
+
+  expect(handleEmptyArbitraryValues(designSystem, {}, candidate)).toEqual(result)
+})
+
+test.each([
+  ['group-[]:tw-flex', 'tw:group-[&]:flex'],
+  ['group-[]/name:tw-flex', 'tw:group-[&]/name:flex'],
+
+  ['peer-[]:tw-flex', 'tw:peer-[&]:flex'],
+  ['peer-[]/name:tw-flex', 'tw:peer-[&]/name:flex'],
+])('%s => %s (%#)', async (candidate, result) => {
+  let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss" prefix(tw);', {
+    base: __dirname,
+  })
+
+  expect(
+    [handleEmptyArbitraryValues, prefix].reduce(
+      (acc, step) => step(designSystem, { prefix: 'tw-' }, acc),
+      candidate,
+    ),
+  ).toEqual(result)
+})

packages/@tailwindcss-upgrade/src/template/codemods/handle-empty-arbitrary-values.ts

@@ -0,0 +1,27 @@
+import type { Config } from '../../../../tailwindcss/src/compat/plugin-api'
+import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
+
+export function handleEmptyArbitraryValues(
+  designSystem: DesignSystem,
+  _userConfig: Config,
+  rawCandidate: string,
+): string {
+  // We can parse the candidate, nothing to do
+  if (designSystem.parseCandidate(rawCandidate).length > 0) {
+    return rawCandidate
+  }
+
+  // No need to handle empty arbitrary values
+  if (!rawCandidate.includes('[]')) {
+    return rawCandidate
+  }
+
+  // Add the `&` placeholder to the empty arbitrary values. Other codemods might
+  // migrate these away, but if not, then it's at least valid to parse.
+  //
+  // E.g.: `group-[]:flex` => `group-[&]:flex`
+  // E.g.: `group-[]/name:flex` => `group-[&]/name:flex`
+  return rawCandidate
+    .replaceAll('-[]:', '-[&]:') // End of variant
+    .replaceAll('-[]/', '-[&]/') // With modifier
+}

packages/@tailwindcss-upgrade/src/template/codemods/modernize-arbitrary-values.test.ts

@@ -1,5 +1,6 @@
 import { __unstable__loadDesignSystem } from '@tailwindcss/node'
 import { expect, test } from 'vitest'
+import { handleEmptyArbitraryValues } from './handle-empty-arbitrary-values'
 import { modernizeArbitraryValues } from './modernize-arbitrary-values'
 import { prefix } from './prefix'
 
@@ -31,6 +32,10 @@ test.each([
   ['group-[]:flex', 'in-[.group]:flex'],
   ['group-[]/name:flex', 'in-[.group\\/name]:flex'],
 
+  // Migrate `peer-[]` to a parsable `peer-[&]` instead:
+  ['peer-[]:flex', 'peer-[&]:flex'],
+  ['peer-[]/name:flex', 'peer-[&]/name:flex'],
+
   // These shouldn't happen in the real world (because compound variants are
   // new). But this could happen once we allow codemods to run in v4+ projects.
   ['has-group-[]:flex', 'has-in-[.group]:flex'],
@@ -85,12 +90,17 @@ test.each([
   ['has-[[aria-visible]]:flex', 'has-aria-[visible]:flex'],
 
   ['has-[&:not(:nth-child(even))]:flex', 'has-odd:flex'],
-])('%s => %s', async (candidate, result) => {
+])('%s => %s (%#)', async (candidate, result) => {
   let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', {
     base: __dirname,
   })
 
-  expect(modernizeArbitraryValues(designSystem, {}, candidate)).toEqual(result)
+  expect(
+    [handleEmptyArbitraryValues, modernizeArbitraryValues].reduce(
+      (acc, step) => step(designSystem, {}, acc),
+      candidate,
+    ),
+  ).toEqual(result)
 })
 
 test.each([
@@ -101,6 +111,10 @@ test.each([
   ['group-[]:tw-flex', 'tw:in-[.tw\\:group]:flex'],
   ['group-[]/name:tw-flex', 'tw:in-[.tw\\:group\\/name]:flex'],
 
+  // Migrate `peer-[]` to a parsable `peer-[&]` instead:
+  ['peer-[]:tw-flex', 'tw:peer-[&]:flex'],
+  ['peer-[]/name:tw-flex', 'tw:peer-[&]/name:flex'],
+
   // However, `.group` inside of an arbitrary variant should not be prefixed:
   ['[.group_&]:tw-flex', 'tw:in-[.group]:flex'],
 
@@ -110,13 +124,13 @@ test.each([
   ['has-group-[]/name:tw-flex', 'tw:has-in-[.tw\\:group\\/name]:flex'],
   ['not-group-[]:tw-flex', 'tw:not-in-[.tw\\:group]:flex'],
   ['not-group-[]/name:tw-flex', 'tw:not-in-[.tw\\:group\\/name]:flex'],
-])('%s => %s', async (candidate, result) => {
+])('%s => %s (%#)', async (candidate, result) => {
   let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss" prefix(tw);', {
     base: __dirname,
   })
 
   expect(
-    [prefix, modernizeArbitraryValues].reduce(
+    [handleEmptyArbitraryValues, prefix, modernizeArbitraryValues].reduce(
       (acc, step) => step(designSystem, { prefix: 'tw-' }, acc),
       candidate,
     ),

packages/@tailwindcss-upgrade/src/template/codemods/modernize-arbitrary-values.ts

@@ -43,7 +43,7 @@ export function modernizeArbitraryValues(
         variant.kind === 'compound' &&
         variant.root === 'group' &&
         variant.variant.kind === 'arbitrary' &&
-        variant.variant.selector === '&:is()'
+        variant.variant.selector === '&'
       ) {
         // `group-[]`
         if (variant.modifier === null) {

packages/@tailwindcss-upgrade/src/template/migrate.ts

@@ -6,6 +6,7 @@ import { extractRawCandidates } from './candidates'
 import { arbitraryValueToBareValue } from './codemods/arbitrary-value-to-bare-value'
 import { automaticVarInjection } from './codemods/automatic-var-injection'
 import { bgGradient } from './codemods/bg-gradient'
+import { handleEmptyArbitraryValues } from './codemods/handle-empty-arbitrary-values'
 import { important } from './codemods/important'
 import { legacyArbitraryValues } from './codemods/legacy-arbitrary-values'
 import { legacyClasses } from './codemods/legacy-classes'
@@ -29,6 +30,7 @@ export type Migration = (
 ) => string | Promise<string>
 
 export const DEFAULT_MIGRATIONS: Migration[] = [
+  handleEmptyArbitraryValues,
   prefix,
   important,
   bgGradient,

packages/tailwindcss/src/candidate.test.ts

@@ -1416,3 +1416,130 @@ it('should parse candidates with a prefix', () => {
     ]
   `)
 })
+
+it.each([
+  // Empty arbitrary value
+  'bg-[]',
+  'bg-()',
+  // — Tricking the parser with a space is not allowed
+  'bg-[_]',
+  'bg-(_)',
+
+  // Empty arbitrary value, with typehint
+  'bg-[color:]',
+  'bg-(color:)',
+  // — Tricking the parser with a space is not allowed
+  'bg-[color:_]',
+  'bg-(color:_)',
+
+  // Empty arbitrary modifier
+  'bg-red-500/[]',
+  'bg-red-500/()',
+  // — Tricking the parser with a space is not allowed
+  'bg-red-500/[_]',
+  'bg-red-500/(_)',
+
+  // Empty arbitrary modifier for arbitrary properties
+  '[color:red]/[]',
+  '[color:red]/()',
+  // — Tricking the parser with a space is not allowed
+  '[color:red]/[_]',
+  '[color:red]/(_)',
+
+  // Empty arbitrary value and modifier
+  'bg-[]/[]',
+  'bg-()/[]',
+  'bg-[]/()',
+  'bg-()/()',
+  // — Tricking the parser with a space is not allowed
+  'bg-[_]/[]',
+  'bg-(_)/[]',
+  'bg-[_]/()',
+  'bg-(_)/()',
+  'bg-[]/[_]',
+  'bg-()/[_]',
+  'bg-[]/(_)',
+  'bg-()/(_)',
+  'bg-[_]/[_]',
+  'bg-(_)/[_]',
+  'bg-[_]/(_)',
+  'bg-(_)/(_)',
+
+  // Functional variants
+  // Empty arbitrary value in variant
+  'data-[]:flex',
+  'data-():flex',
+  // — Tricking the parser with a space is not allowed
+  'data-[_]:flex',
+  'data-(_):flex',
+
+  // Empty arbitrary modifier in variant
+  'data-foo/[]:flex',
+  'data-foo/():flex',
+  // — Tricking the parser with a space is not allowed
+  'data-foo/[_]:flex',
+  'data-foo/(_):flex',
+
+  // Empty arbitrary value and modifier in variant
+  'data-[]/[]:flex',
+  'data-()/[]:flex',
+  'data-[]/():flex',
+  'data-()/():flex',
+  // — Tricking the parser with a space is not allowed
+  'data-[_]/[]:flex',
+  'data-(_)/[]:flex',
+  'data-[_]/():flex',
+  'data-(_)/():flex',
+  'data-[]/[_]:flex',
+  'data-()/[_]:flex',
+  'data-[]/(_):flex',
+  'data-()/(_):flex',
+  'data-[_]/[_]:flex',
+  'data-(_)/[_]:flex',
+  'data-[_]/(_):flex',
+  'data-(_)/(_):flex',
+
+  // Compound variants
+  // Empty arbitrary value in variant
+  'group-data-[]:flex',
+  'group-data-():flex',
+  // — Tricking the parser with a space is not allowed
+  'group-data-[_]:flex',
+  'group-data-(_):flex',
+
+  // Empty arbitrary modifier in variant
+  'group-data-foo/[]:flex',
+  'group-data-foo/():flex',
+  // — Tricking the parser with a space is not allowed
+  'group-data-foo/[_]:flex',
+  'group-data-foo/(_):flex',
+
+  // Empty arbitrary value and modifier in variant
+  'group-data-[]/[]:flex',
+  'group-data-()/[]:flex',
+  'group-data-[]/():flex',
+  'group-data-()/():flex',
+  // — Tricking the parser with a space is not allowed
+  'group-data-[_]/[]:flex',
+  'group-data-(_)/[]:flex',
+  'group-data-[_]/():flex',
+  'group-data-(_)/():flex',
+  'group-data-[]/[_]:flex',
+  'group-data-()/[_]:flex',
+  'group-data-[]/(_):flex',
+  'group-data-()/(_):flex',
+  'group-data-[_]/[_]:flex',
+  'group-data-(_)/[_]:flex',
+  'group-data-[_]/(_):flex',
+  'group-data-(_)/(_):flex',
+])('should not parse invalid empty arbitrary values: %s', (rawCandidate) => {
+  let utilities = new Utilities()
+  utilities.static('flex', () => [])
+  utilities.functional('bg', () => [])
+
+  let variants = new Variants()
+  variants.functional('data', () => {})
+  variants.compound('group', Compounds.StyleRules, () => {})
+
+  expect(run(rawCandidate, { utilities, variants })).toEqual([])
+})