Don't escape underscores for the first parameter of var() (#14776)

This PR updates our arbitrary value decoder to:

- No longer require an escaping for underscores in the first parameter
of `var()`. Example:

    ```
    ml-[var(--spacing-1_5,_1rem)]
    ```

- Ensures that properties before an eventual `url()` are properly
unescaped. Example:
    ```
    bg-[no-repeat_url(./image.jpg)]
    ```

I will ensure that this properly works for the migrate use case in a
follow-up PR in the stack.

## Test Plan

Added unit tests as well as tests for the variant decoder. Additionally
this PR also adds a higher-level test using the public Tailwind APIs to
ensure this is properly propagated.

---------

Co-authored-by: Adam Wathan <adam.wathan@gmail.com>

Author: philipp-spiess

CHANGELOG.md

@@ -7,11 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Don't convert underscores in the first argument to `var()` to spaces ([#14776](https://github.com/tailwindlabs/tailwindcss/pull/14776))
+
 ### Fixed
 
 - Ensure individual logical property utilities are sorted later than left/right pair utilities ([#14777](https://github.com/tailwindlabs/tailwindcss/pull/14777))
 - Don't migrate important modifiers inside conditional statements in Vue and Alpine (e.g. `<div v-if="!border" />`) ([#14774](https://github.com/tailwindlabs/tailwindcss/pull/14774))
 - Ensure third-party plugins with `exports` in their `package.json` are resolved correctly ([#14775](https://github.com/tailwindlabs/tailwindcss/pull/14775))
+- Ensure underscores in the `url()` function are never unescaped ([#14776](https://github.com/tailwindlabs/tailwindcss/pull/14776))
 - _Upgrade (experimental)_: Ensure `@import` statements for relative CSS files are actually migrated to use relative path syntax ([#14769](https://github.com/tailwindlabs/tailwindcss/pull/14769))
 
 ## [4.0.0-alpha.29] - 2024-10-23

packages/tailwindcss/src/candidate.test.ts

@@ -1059,24 +1059,50 @@ it('should not replace `_` inside of `url()`', () => {
   let utilities = new Utilities()
   utilities.functional('bg', () => [])
 
-  expect(run('bg-[url(https://example.com/some_page)]', { utilities })).toMatchInlineSnapshot(`
-    [
-      {
-        "important": false,
-        "kind": "functional",
-        "modifier": null,
-        "negative": false,
-        "raw": "bg-[url(https://example.com/some_page)]",
-        "root": "bg",
-        "value": {
-          "dataType": null,
-          "kind": "arbitrary",
-          "value": "url(https://example.com/some_page)",
+  expect(run('bg-[no-repeat_url(https://example.com/some_page)]', { utilities }))
+    .toMatchInlineSnapshot(`
+      [
+        {
+          "important": false,
+          "kind": "functional",
+          "modifier": null,
+          "negative": false,
+          "raw": "bg-[no-repeat_url(https://example.com/some_page)]",
+          "root": "bg",
+          "value": {
+            "dataType": null,
+            "kind": "arbitrary",
+            "value": "no-repeat url(https://example.com/some_page)",
+          },
+          "variants": [],
         },
-        "variants": [],
-      },
-    ]
-  `)
+      ]
+    `)
+})
+
+it('should not replace `_` in the first argument to `var()`', () => {
+  let utilities = new Utilities()
+  utilities.functional('ml', () => [])
+
+  expect(run('ml-[var(--spacing-1_5,_var(--spacing-2_5,_1rem))]', { utilities }))
+    .toMatchInlineSnapshot(`
+      [
+        {
+          "important": false,
+          "kind": "functional",
+          "modifier": null,
+          "negative": false,
+          "raw": "ml-[var(--spacing-1_5,_var(--spacing-2_5,_1rem))]",
+          "root": "ml",
+          "value": {
+            "dataType": null,
+            "kind": "arbitrary",
+            "value": "var(--spacing-1_5, var(--spacing-2_5, 1rem))",
+          },
+          "variants": [],
+        },
+      ]
+    `)
 })
 
 it('should parse arbitrary properties', () => {

packages/tailwindcss/src/index.test.ts

@@ -115,6 +115,34 @@ describe('compiling CSS', () => {
       ),
     ).toMatchSnapshot()
   })
+
+  test('unescapes underscores to spaces inside arbitrary values except for `url()` and first argument of `var()`', async () => {
+    expect(
+      await compileCss(
+        css`
+          @theme {
+            --spacing-1_5: 1.5rem;
+            --spacing-2_5: 2.5rem;
+          }
+          @tailwind utilities;
+        `,
+        ['bg-[no-repeat_url(./my_file.jpg)', 'ml-[var(--spacing-1_5,_var(--spacing-2_5,_1rem))]'],
+      ),
+    ).toMatchInlineSnapshot(`
+      ":root {
+        --spacing-1_5: 1.5rem;
+        --spacing-2_5: 2.5rem;
+      }
+
+      .ml-\\[var\\(--spacing-1_5\\,_var\\(--spacing-2_5\\,_1rem\\)\\)\\] {
+        margin-left: var(--spacing-1_5, var(--spacing-2_5, 1rem));
+      }
+
+      .bg-\\[no-repeat_url\\(\\.\\/my_file\\.jpg\\) {
+        background-color: no-repeat url("./")my file. jpg;
+      }"
+    `)
+  })
 })
 
 describe('arbitrary properties', () => {

packages/tailwindcss/src/utils/decode-arbitrary-value.test.ts

@@ -16,6 +16,17 @@ describe('decoding arbitrary values', () => {
 
   it('should not replace underscores in url()', () => {
     expect(decodeArbitraryValue('url(./my_file.jpg)')).toBe('url(./my_file.jpg)')
+    expect(decodeArbitraryValue('no-repeat_url(./my_file.jpg)')).toBe(
+      'no-repeat url(./my_file.jpg)',
+    )
+  })
+
+  it('should not replace underscores in the first argument of var()', () => {
+    expect(decodeArbitraryValue('var(--spacing-1_5)')).toBe('var(--spacing-1_5)')
+    expect(decodeArbitraryValue('var(--spacing-1_5,_1rem)')).toBe('var(--spacing-1_5, 1rem)')
+    expect(decodeArbitraryValue('var(--spacing-1_5,_var(--spacing-2_5,_1rem))')).toBe(
+      'var(--spacing-1_5, var(--spacing-2_5, 1rem))',
+    )
   })
 
   it('should leave var(…) as is', () => {
@@ -55,8 +66,8 @@ describe('adds spaces around math operators', () => {
     ['calc(24px+(-1rem))', 'calc(24px + (-1rem))'],
     ['calc(24px_+_(-1rem))', 'calc(24px + (-1rem))'],
     [
-      'calc(var(--10-10px,calc(-20px-(-30px--40px)-50px)',
-      'calc(var(--10-10px,calc(-20px - (-30px - -40px) - 50px)',
+      'calc(var(--10-10px,calc(-20px-(-30px--40px)-50px)))',
+      'calc(var(--10-10px,calc(-20px - (-30px - -40px) - 50px)))',
     ],
     ['calc(theme(spacing.1-bar))', 'calc(theme(spacing.1-bar))'],
     ['theme(spacing.1-bar)', 'theme(spacing.1-bar)'],

packages/tailwindcss/src/utils/decode-arbitrary-value.ts

@@ -1,13 +1,16 @@
+import * as ValueParser from '../value-parser'
 import { addWhitespaceAroundMathOperators } from './math-operators'
 
 export function decodeArbitraryValue(input: string): string {
-  // We do not want to normalize anything inside of a url() because if we
-  // replace `_` with ` `, then it will very likely break the url.
-  if (input.startsWith('url(')) {
-    return input
+  // There are definitely no functions in the input, so bail early
+  if (input.indexOf('(') === -1) {
+    return convertUnderscoresToWhitespace(input)
   }
 
-  input = convertUnderscoresToWhitespace(input)
+  let ast = ValueParser.parse(input)
+  recursivelyDecodeArbitraryValues(ast)
+  input = ValueParser.toCss(ast)
+
   input = addWhitespaceAroundMathOperators(input)
 
   return input
@@ -41,3 +44,45 @@ function convertUnderscoresToWhitespace(input: string) {
 
   return output
 }
+
+function recursivelyDecodeArbitraryValues(ast: ValueParser.ValueAstNode[]) {
+  for (let node of ast) {
+    switch (node.kind) {
+      case 'function': {
+        if (node.value === 'url' || node.value.endsWith('_url')) {
+          // Don't decode underscores in url() but do decode the function name
+          node.value = convertUnderscoresToWhitespace(node.value)
+          break
+        }
+
+        if (node.value === 'var' || node.value.endsWith('_var')) {
+          // Don't decode underscores in the first argument of var() but do
+          // decode the function name
+          node.value = convertUnderscoresToWhitespace(node.value)
+          for (let i = 0; i < node.nodes.length; i++) {
+            if (i == 0 && node.nodes[i].kind === 'word') {
+              continue
+            }
+            recursivelyDecodeArbitraryValues([node.nodes[i]])
+          }
+          break
+        }
+
+        node.value = convertUnderscoresToWhitespace(node.value)
+        recursivelyDecodeArbitraryValues(node.nodes)
+        break
+      }
+      case 'separator':
+      case 'word': {
+        node.value = convertUnderscoresToWhitespace(node.value)
+        break
+      }
+      default:
+        never()
+    }
+  }
+}
+
+function never(): never {
+  throw new Error('This should never happen')
+}

packages/tailwindcss/src/utils/math-operators.ts

@@ -28,11 +28,6 @@ export function hasMathFn(input: string) {
 }
 
 export function addWhitespaceAroundMathOperators(input: string) {
-  // There's definitely no functions in the input, so bail early
-  if (input.indexOf('(') === -1) {
-    return input
-  }
-
   // Bail early if there are no math functions in the input
   if (!MATH_FUNCTIONS.some((fn) => input.includes(fn))) {
     return input