Guard against infinitely recursive theme key lookup (#1332)

When constructing the design system we compute color swatches for all
classes. This process involves:
- Computing the CSS for a utility
- Inlining any values from the theme, recursively
- Scanning for CSS color syntax that we can compute a swatch for

We guard against directly self-recursive replacements because of how the
replacement itself functions. However, if you wrapped the variable in
something then we would effectively recurse into the expression and try
to replace the variable again. This caused the expansion to then be
wrapped and then try again resulting in an infinite loop.

This fixes it by keeping track of seen variables during a replacement
and if we've seen it _and_ the expansion contains a var(…) anywhere
we'll only replace it once. This logic is not perfect as the thing that
matters is that the replacement _itself_ results in a recursion but this
is good enough for now.

Fixes #1329

Author: thecrypticace

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

@@ -79,9 +79,49 @@ test('replacing CSS variables with their fallbacks (when they have them)', () =>
   expect(replaceCssVarsWithFallbacks(state, 'var(--level-3)')).toBe('blue')
 
   // Circular replacements don't cause infinite loops
-  expect(replaceCssVarsWithFallbacks(state, 'var(--circular-1)')).toBe('var(--circular-3)')
-  expect(replaceCssVarsWithFallbacks(state, 'var(--circular-2)')).toBe('var(--circular-1)')
-  expect(replaceCssVarsWithFallbacks(state, 'var(--circular-3)')).toBe('var(--circular-2)')
+  expect(replaceCssVarsWithFallbacks(state, 'var(--circular-1)')).toBe('var(--circular-1)')
+  expect(replaceCssVarsWithFallbacks(state, 'var(--circular-2)')).toBe('var(--circular-2)')
+  expect(replaceCssVarsWithFallbacks(state, 'var(--circular-3)')).toBe('var(--circular-3)')
+})
+
+test('recursive theme replacements', () => {
+  let map = new Map<string, string>([
+    ['--color-a', 'var(--color-a)'],
+    ['--color-b', 'rgb(var(--color-b))'],
+    ['--color-c', 'rgb(var(--channel) var(--channel) var(--channel))'],
+    ['--channel', '255'],
+
+    ['--color-d', 'rgb(var(--indirect) var(--indirect) var(--indirect))'],
+    ['--indirect', 'var(--channel)'],
+    ['--channel', '255'],
+
+    ['--mutual-a', 'calc(var(--mutual-b) * 1)'],
+    ['--mutual-b', 'calc(var(--mutual-a) + 1)'],
+  ])
+
+  let state: State = {
+    enabled: true,
+    designSystem: {
+      theme: { prefix: null } as any,
+      resolveThemeValue: (name) => map.get(name) ?? null,
+    } as DesignSystem,
+  }
+
+  expect(replaceCssVarsWithFallbacks(state, 'var(--color-a)')).toBe('var(--color-a)')
+  expect(replaceCssVarsWithFallbacks(state, 'var(--color-b)')).toBe('rgb(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
+  expect(replaceCssVarsWithFallbacks(state, 'var(--color-d)')).toBe(
+    'rgb(255 var(--indirect) var(--indirect))',
+  )
+
+  expect(replaceCssVarsWithFallbacks(state, 'var(--mutual-a)')).toBe(
+    'calc(calc(var(--mutual-a) + 1) * 1)',
+  )
+  expect(replaceCssVarsWithFallbacks(state, 'var(--mutual-b)')).toBe(
+    'calc(calc(var(--mutual-b) * 1) + 1)',
+  )
 })
 
 test('Evaluating CSS calc expressions', () => {

packages/tailwindcss-language-service/src/util/rewriting/var-fallbacks.ts

@@ -3,14 +3,25 @@ import { resolveVariableValue } from './lookup'
 import { replaceCssVars } from './replacements'
 
 export function replaceCssVarsWithFallbacks(state: State, str: string): string {
+  let seen = new Set<string>()
+
   return replaceCssVars(str, {
     replace({ name, fallback }) {
       // Replace with the value from the design system first. The design system
       // take precedences over other sources as that emulates the behavior of a
       // browser where the fallback is only used if the variable is defined.
       if (state.designSystem && name.startsWith('--')) {
+        // TODO: This isn't quite right as we might skip expanding a variable
+        // that should be expanded
+        if (seen.has(name)) return null
         let value = resolveVariableValue(state.designSystem, name)
-        if (value !== null) return value
+        if (value !== null) {
+          if (value.includes('var(')) {
+            seen.add(name)
+          }
+
+          return value
+        }
       }
 
       if (fallback) {

packages/vscode-tailwindcss/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Warn when using a blocklisted class in v4 ([#1310](https://github.com/tailwindlabs/tailwindcss-intellisense/pull/1310))
 - Support class function hovers in Svelte and HTML `<script>` blocks ([#1311](https://github.com/tailwindlabs/tailwindcss-intellisense/pull/1311))
+- Guard against recursive theme key lookup ([#1332](https://github.com/tailwindlabs/tailwindcss-intellisense/pull/1332))
 
 # 0.14.15