Fix infinite recursion in theme variable lookups (#1473)

Fixes #1463

We'll have to bail in some cases that we didn't before to fix this. This
seems fine though. The core problem was that given this theme:
```css
@theme {
  --radius: calc(var(--radius));
}
```

We'd see this CSS:
```css
.rounded {
  border-radius: calc(var(--radius));
}
```

And we'd try to replace variables in `calc(var(--radius))`. This doesn't
_start_ with a var(…) so our recursion guards didn't catch this.

This entire system is a bit fragile and I'll be working to clean this up
along with how we handle pixel equivalents, light-dark replacements,
color-mix replacements, etc… in some future PRs (#1330 is one of those
but I want to reorganize + tweak some code first in an earlier PR which
will make that one "nicer")

Author: thecrypticace

packages/tailwindcss-language-service/src/util/rewriting/index.test.ts

@@ -111,7 +111,8 @@ test('recursive theme replacements', () => {
   }
 
   expect(replaceCssVarsWithFallbacks(state, 'var(--color-a)')).toBe('var(--color-a)')
-  expect(replaceCssVarsWithFallbacks(state, 'var(--color-b)')).toBe('rgb(var(--color-b))')
+  // TODO: -> rgb(var(--color-b))
+  expect(replaceCssVarsWithFallbacks(state, 'var(--color-b)')).toBe('var(--color-b)')
   expect(replaceCssVarsWithFallbacks(state, 'var(--color-c)')).toBe('rgb(255 255 255)')
 
   // This one is wrong but fixing it without breaking the infinite recursion guard is complex
@@ -152,7 +153,8 @@ test('recursive theme replacements (inlined)', () => {
   }
 
   expect(inlineThemeValues('var(--color-a)', state)).toBe('var(--color-a)')
-  expect(inlineThemeValues('var(--color-b)', state)).toBe('rgb(var(--color-b))')
+  // TODO: -> rgb(var(--color-b))
+  expect(inlineThemeValues('var(--color-b)', state)).toBe('var(--color-b)')
   expect(inlineThemeValues('var(--color-c)', state)).toBe('rgb(255 255 255)')
 
   // This one is wrong but fixing it without breaking the infinite recursion guard is complex
@@ -192,6 +194,9 @@ test('Inlining calc expressions using the design system', () => {
     ['--spacing', '0.25rem'],
     ['--color-red-500', 'oklch(0.637 0.237 25.331)'],
     ['--font-size.md', '1rem'],
+    ['--wrapped', 'calc(var(--wrapped))'],
+    ['--unbalanced-1', 'calc(var(--unbalanced-1)'],
+    ['--unbalanced-2', 'calc(1px + 2px'],
   ])
 
   let state: State = {
@@ -246,4 +251,31 @@ test('Inlining calc expressions using the design system', () => {
   expect(addThemeValues('var(--font-size\\.md)', state, settings)).toBe(
     'var(--font-size\\.md) /* 1rem = 10px */',
   )
+
+  // Variables that replace with themselves. This could be handled better
+  expect(addThemeValues('calc(var(--wrapped))', state, settings)).toBe(
+    'calc(var(--wrapped) /* calc(var(--wrapped)) */)',
+  )
+  expect(addThemeValues('var(--wrapped)', state, settings)).toBe(
+    'var(--wrapped) /* calc(var(--wrapped)) */',
+  )
+
+  // Unbalanced calc expressions. Whether or not these "work" is undefined. If
+  // these change results that's okay.
+  expect(addThemeValues('calc(var(--unbalanced-1))', state, settings)).toBe(
+    'calc(var(--unbalanced-1) /* calc(var(--unbalanced-1) */)',
+  )
+  expect(addThemeValues('var(--unbalanced-1)', state, settings)).toBe(
+    'var(--unbalanced-1) /* calc(var(--unbalanced-1) */',
+  )
+
+  // It would be fine for either of these to not replace at all
+  expect(addThemeValues('calc(var(--unbalanced-2))', state, settings)).toBe(
+    'calc(var(--unbalanced-2)) /* 3px */',
+  )
+  expect(addThemeValues('var(--unbalanced-2)', state, settings)).toBe(
+    'var(--unbalanced-2) /* calc(1px + 2px */',
+  )
+
+  expect(addThemeValues('calc(1 + 2', state, settings)).toBe('calc(1 + 2')
 })

packages/tailwindcss-language-service/src/util/rewriting/replacements.ts

@@ -76,6 +76,12 @@ export function replaceCssVars(
         })
 
         if (replacement !== null) {
+          // If we're replacing this variable with a reference back it *itself*
+          // we should skip over it
+          if (replacement.includes(`var(${varName})`) || replacement.includes(`var(${varName},`)) {
+            break
+          }
+
           str = str.slice(0, i) + replacement + str.slice(j + 1)
         }
 
@@ -121,7 +127,7 @@ export function replaceCssCalc(str: string, replace: CssCalcReplacer): string {
 
     let depth = 0
 
-    for (let j = i + 5; i < str.length; ++j) {
+    for (let j = i + 5; j < str.length; ++j) {
       if (str[j] === '(') {
         depth++
       } else if (str[j] === ')' && depth > 0) {

packages/vscode-tailwindcss/CHANGELOG.md

@@ -2,7 +2,8 @@
 
 ## Prerelease
 
-- Nothing yet!
+- Fix infinite recursion in theme variable lookups ([#1473](https://github.com/tailwindlabs/tailwindcss-intellisense/pull/1473))
+- Fix infinite recursion when replacing unbalanced calc expressions ([#1473](https://github.com/tailwindlabs/tailwindcss-intellisense/pull/1473))
 
 ## 0.14.27