ronantakizawa/codereview-bench huggingface dataset:
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-vite/src/index.ts (TypeScript)

Code before:
            continue
          }

          // These plugins have side effects which, during build, results in CSS
          // being written to the output dir. We need to run them here to ensure
          // the CSS is written before the bundle is generated.
          await transformWithPlugins(this, id, generated)
        }
      },
    },
  ] satisfies Plugin[]
}

function getExtension(id: string) {
  let [filename] = id.split('?', 2)
  return path.extname(filename).slice(1)
}

function isPotentialCssRootFile(id: string) {
  let extension = getExtension(id)
  let isCssFile =
    (extension === 'css' ||
      (extension === 'vue' && id.includes('&lang.css')) ||
      (extension === 'astro' && id.includes('&lang.css'))) &&
    // Don't intercept special static asset resources
    !id.match(SPECIAL_QUERY_RE)

  return isCssFile
}

function optimizeCss(
  input: string,
  { file = 'input.css', minify = false }: { file?: string; minify?: boolean } = {},
) {
  return transform({
    filename: file,
    code: Buffer.from(input),
    minify,
    sourceMap: false,
    drafts: {
      customMedia: true,
    },
    nonStandard: {
      deepSelectorCombinator: true,
    },
    include: Features.Nesting,
    exclude: Features.LogicalProperties,
    targets: {
      safari: (16 << 16) | (4 << 8),
    },
    errorRecovery: true,

Review comment (suggestion):
```suggestion
    !SPECIAL_QUERY_RE.test(id)
```

Code after:
            continue
          }

          // These plugins have side effects which, during build, results in CSS
          // being written to the output dir. We need to run them here to ensure
          // the CSS is written before the bundle is generated.
          await transformWithPlugins(this, id, generated)
        }
      },
    },
  ] satisfies Plugin[]
}

function getExtension(id: string) {
  let [filename] = id.split('?', 2)
  return path.extname(filename).slice(1)
}

function isPotentialCssRootFile(id: string) {
  let extension = getExtension(id)
  let isCssFile =
    (extension === 'css' ||
      (extension === 'vue' && id.includes('&lang.css')) ||
      (extension === 'astro' && id.includes('&lang.css'))) &&
    // Don't intercept special static asset resources
    !SPECIAL_QUERY_RE.test(id)

  return isCssFile
}

function optimizeCss(
  input: string,
  { file = 'input.css', minify = false }: { file?: string; minify?: boolean } = {},
) {
  return transform({
    filename: file,
    code: Buffer.from(input),
    minify,
    sourceMap: false,
    drafts: {
      customMedia: true,
    },
    nonStandard: {
      deepSelectorCombinator: true,
    },
    include: Features.Nesting,
    exclude: Features.LogicalProperties,
    targets: {
      safari: (16 << 16) | (4 << 8),
    },
    errorRecovery: true,
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/migrate-js-config.ts (TypeScript)

Code before:
      continue
    }

    if (key[0] === 'keyframes') {
      continue
    }
    containsThemeKeys = true

    let sectionKey = createSectionKey(key)
    if (sectionKey !== prevSectionKey) {
      css += `\n`
      prevSectionKey = sectionKey
    }

    if (resetNamespaces.has(key[0]) && resetNamespaces.get(key[0]) === false) {
      resetNamespaces.set(key[0], true)
      let property = keyPathToCssProperty([key[0]])
      if (property !== null) {
        css += `  --${escape(property)}-*: initial;\n`
      }
    }

    let property = keyPathToCssProperty(key)
    if (property !== null) {
      css += `  --${escape(property)}: ${value};\n`
    }
  }

  if ('keyframes' in resolvedConfig.theme) {
    containsThemeKeys = true
    css += '\n' + keyframesToCss(resolvedConfig.theme.keyframes)
  }

  if (!containsThemeKeys) {
    return null
  }

  return css + '}\n'
}

function migrateDarkMode(unresolvedConfig: Config & { darkMode: any }): string {
  let variant: string = ''
  let addVariant = (_name: string, _variant: string) => (variant = _variant)
  let config = () => unresolvedConfig.darkMode
  darkModePlugin({ config, addVariant })

  if (variant === '') {
    return ''
  }
  return `@variant dark (${variant});\n`
}

Review comment (security):
It might be wise to prepend the `--` before running escape here, because certain characters need to be escaped if they are the very first character but don't need to be escaped if they are not:

<img width="245" alt="image" src="https://github.com/user-attachments/assets/3c6f65f3-03be-4385-bd04-bfd18fae624f">


Code after:
      continue
    }

    if (key[0] === 'keyframes') {
      continue
    }
    containsThemeKeys = true

    let sectionKey = createSectionKey(key)
    if (sectionKey !== prevSectionKey) {
      css += `\n`
      prevSectionKey = sectionKey
    }

    if (resetNamespaces.has(key[0]) && resetNamespaces.get(key[0]) === false) {
      resetNamespaces.set(key[0], true)
      let property = keyPathToCssProperty([key[0]])
      if (property !== null) {
        css += `  ${escape(`--${property}`)}-*: initial;\n`
      }
    }

    let property = keyPathToCssProperty(key)
    if (property !== null) {
      css += `  ${escape(`--${property}`)}: ${value};\n`
    }
  }

  if ('keyframes' in resolvedConfig.theme) {
    containsThemeKeys = true
    css += '\n' + keyframesToCss(resolvedConfig.theme.keyframes)
  }

  if (!containsThemeKeys) {
    return null
  }

  return css + '}\n'
}

function migrateDarkMode(unresolvedConfig: Config & { darkMode: any }): string {
  let variant: string = ''
  let addVariant = (_name: string, _variant: string) => (variant = _variant)
  let config = () => unresolvedConfig.darkMode
  darkModePlugin({ config, addVariant })

  if (variant === '') {
    return ''
  }
  return `@variant dark (${variant});\n`
}
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/template/codemods/important.ts (TypeScript)

Code before:
        let isQuoteBeforeCandidate = QUOTES.some((quote) =>
          currentLineBeforeCandidate.includes(quote),
        )
        let isQuoteAfterCandidate = QUOTES.some((quote) =>
          currentLineAfterCandidate.includes(quote),
        )
        if (!isQuoteBeforeCandidate || !isQuoteAfterCandidate) {
          continue nextCandidate
        }

        // Heuristics 2: Disallow object access immediately following the candidate
        if (currentLineAfterCandidate[0] === '.') {
          continue nextCandidate
        }

        // Heuristics 3: Disallow logical operators proceeding or following the candidate
        for (let operator of LOGICAL_OPERATORS) {
          if (
            currentLineAfterCandidate.trim().startsWith(operator) ||
            currentLineBeforeCandidate.trim().endsWith(operator)
          ) {
            continue nextCandidate
          }
        }

        // Heuristics 4: Disallow conditional template syntax
        for (let rule of CONDITIONAL_TEMPLATE_SYNTAX) {
          if (rule.test(currentLineBeforeCandidate)) {
            continue nextCandidate
          }
        }
      }

      // The printCandidate function will already put the exclamation mark in
      // the right place, so we just need to mark this candidate as requiring a
      // migration.
      return printCandidate(designSystem, candidate)
    }
  }

  return rawCandidate
}


Review comment (suggestion):
```suggestion
        // Heuristic 4: Disallow conditional template syntax
```

Code after:
        // Heuristic 1: Require the candidate to be inside quotes
        let isQuoteBeforeCandidate = QUOTES.some((quote) =>
          currentLineBeforeCandidate.includes(quote),
        )
        let isQuoteAfterCandidate = QUOTES.some((quote) =>
          currentLineAfterCandidate.includes(quote),
        )
        if (!isQuoteBeforeCandidate || !isQuoteAfterCandidate) {
          continue nextCandidate
        }

        // Heuristic 2: Disallow object access immediately following the candidate
        if (currentLineAfterCandidate[0] === '.') {
          continue nextCandidate
        }

        // Heuristic 3: Disallow logical operators preceding or following the candidate
        for (let operator of LOGICAL_OPERATORS) {
          if (
            currentLineAfterCandidate.trim().startsWith(operator) ||
            currentLineBeforeCandidate.trim().endsWith(operator)
          ) {
            continue nextCandidate
          }
        }

        // Heuristic 4: Disallow conditional template syntax
        for (let rule of CONDITIONAL_TEMPLATE_SYNTAX) {
          if (rule.test(currentLineBeforeCandidate)) {
            continue nextCandidate
          }
        }
      }

      // The printCandidate function will already put the exclamation mark in
      // the right place, so we just need to mark this candidate as requiring a
      // migration.
      return printCandidate(designSystem, candidate)
    }
  }

  return rawCandidate
}
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/candidate.test.ts (TypeScript)

Code before:
it('should not replace `_` inside of `url()`', () => {
  let utilities = new Utilities()
  utilities.functional('bg', () => [])

  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": [],
        },
      ]
    `)
})

it('should not replace `_` for the first value of `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', () => {

Review comment (suggestion):
```suggestion
it('should not replace `_` in the first argument to `var()`', () => {
```

Code after:
it('should not replace `_` inside of `url()`', () => {
  let utilities = new Utilities()
  utilities.functional('bg', () => [])

  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": [],
        },
      ]
    `)
})

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', () => {
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/index.test.ts (TypeScript)

Code before:
    ).toMatchInlineSnapshot(`
      ".flex {
        display: flex;
      }

      .grid {
        display: grid;
      }"
    `)
  })

  test('`@tailwind utilities` is replaced by utilities using the default theme', async () => {
    let defaultTheme = fs.readFileSync(path.resolve(__dirname, '..', 'theme.css'), 'utf-8')

    expect(
      await compileCss(
        css`
          ${defaultTheme}
          @tailwind utilities;
        `,
        ['bg-red-500', 'w-4', 'sm:flex', 'shadow'],
      ),
    ).toMatchSnapshot()
  })

  test('unescapes underscores to spaces inside arbitrary values expect for url() and first parameter 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;
      }"
    `)

Review comment (security):
```suggestion
  test('unescapes underscores to spaces inside arbitrary values except for `url()` and first argument of `var()`', async () => {
```

Code after:
    ).toMatchInlineSnapshot(`
      ".flex {
        display: flex;
      }

      .grid {
        display: grid;
      }"
    `)
  })

  test('`@tailwind utilities` is replaced by utilities using the default theme', async () => {
    let defaultTheme = fs.readFileSync(path.resolve(__dirname, '..', 'theme.css'), 'utf-8')

    expect(
      await compileCss(
        css`
          ${defaultTheme}
          @tailwind utilities;
        `,
        ['bg-red-500', 'w-4', 'sm:flex', 'shadow'],
      ),
    ).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;
      }"
    `)
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/utils/decode-arbitrary-value.ts (TypeScript)

Code before:
import * as ValueParser from '../value-parser'
import { addWhitespaceAroundMathOperators } from './math-operators'

export function decodeArbitraryValue(input: string): string {
  // There's definitely no functions in the input, so bail early
  if (input.indexOf('(') === -1) {
    return convertUnderscoresToWhitespace(input)
  }

  let ast = ValueParser.parse(input)
  recursivelyDecodeArbitraryValues(ast)
  input = ValueParser.toCss(ast)

  input = addWhitespaceAroundMathOperators(input)

  return input
}

/**
 * Convert `_` to ` `, except for escaped underscores `\_` they should be
 * converted to `_` instead.
 */
function convertUnderscoresToWhitespace(input: string) {
  let output = ''
  for (let i = 0; i < input.length; i++) {
    let char = input[i]

    // Escaped underscore
    if (char === '\\' && input[i + 1] === '_') {
      output += '_'

Review comment (suggestion):
```suggestion
  // There are definitely no functions in the input, so bail early
```

Code after:
import * as ValueParser from '../value-parser'
import { addWhitespaceAroundMathOperators } from './math-operators'

export function decodeArbitraryValue(input: string): string {
  // There are definitely no functions in the input, so bail early
  if (input.indexOf('(') === -1) {
    return convertUnderscoresToWhitespace(input)
  }

  let ast = ValueParser.parse(input)
  recursivelyDecodeArbitraryValues(ast)
  input = ValueParser.toCss(ast)

  input = addWhitespaceAroundMathOperators(input)

  return input
}

/**
 * Convert `_` to ` `, except for escaped underscores `\_` they should be
 * converted to `_` instead.
 */
function convertUnderscoresToWhitespace(input: string) {
  let output = ''
  for (let i = 0; i < input.length; i++) {
    let char = input[i]

    // Escaped underscore
    if (char === '\\' && input[i + 1] === '_') {
      output += '_'
Repository: tailwindlabs/tailwindcss
File: integrations/upgrade/index.test.ts (TypeScript)

Code before:
          }
        }
      `,
      'src/my-other.css': css`
        /* All my fonts! */
        @font-face {
        }
      `,
    },
  },
  async ({ fs, exec }) => {
    await exec('npx @tailwindcss/upgrade --force')

    expect(await fs.dumpFiles('./src/**/*.css')).toMatchInlineSnapshot(`
      "
      --- ./src/index.css ---
      @import 'tailwindcss/utilities' layer(utilities);

      /* No layer expected */
      @import './my-components.css';

      /* No layer expected */
      @import './my-utilities.css';

      /* Expecting a layer */
      @import './my-other.css' layer(utilities);

      --- ./src/my-components.css ---
      @utility foo {
        color: red;
      }

      --- ./src/my-other.css ---
      /* All my fonts! */
      @font-face {
      }

      --- ./src/my-utilities.css ---
      @utility css {
        color: red;
      }
      "
    `)
  },
)


Review comment (suggestion):
It's moved to the bottom because we replace `@import 'tailwindcss/utilities';` with the new at-rule. Currently we just drop `@import 'tailwindcss/components';` but we could replace that rule as well instead such that this import lives at the top. 🤔

Code after:
          }
        }
      `,
      'src/my-other.css': css`
        /* All my fonts! */
        @font-face {
        }
      `,
    },
  },
  async ({ fs, exec }) => {
    await exec('npx @tailwindcss/upgrade --force')

    expect(await fs.dumpFiles('./src/**/*.css')).toMatchInlineSnapshot(`
      "
      --- ./src/index.css ---
      /* No layer expected */
      @import './my-components.css';

      /* No layer expected */
      @import './my-utilities.css';

      /* Expecting a layer */
      @import './my-other.css' layer(components);

      @import 'tailwindcss/utilities' layer(utilities);

      --- ./src/my-components.css ---
      @utility foo {
        color: red;
      }

      --- ./src/my-other.css ---
      /* All my fonts! */
      @font-face {
      }

      --- ./src/my-utilities.css ---
      @utility css {
        color: red;
      }
      "
    `)
  },
)
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/utils/packages.ts (TypeScript)

Code before:

class PnpmPackageManager extends PackageManager {
  add(packages: string[], location: 'dependencies' | 'devDependencies') {
    let args = packages.slice()

    if (location === 'devDependencies') {
      args.unshift('--save-dev')
    }

    return this.exec(`pnpm add ${args.join(' ')}`)
  }

  remove(packages: string[]) {
    return this.exec(`pnpm remove ${packages.join(' ')}`)
  }
}

class NpmPackageManager extends PackageManager {
  add(packages: string[], location: 'dependencies' | 'devDependencies') {
    let args = packages.slice()

    if (location === 'devDependencies') {
      args.unshift('--save-dev')
    }

    return this.exec(`npm install ${args.join(' ')}`)
  }

  remove(packages: string[]) {
    return this.exec(`npm remove ${packages.join(' ')}`)
  }
}

let packageManagers = new DefaultMap((base) => {
  return new DefaultMap<string, PackageManager>((pm) => {
    switch (pm) {
      case 'bun':
        return new BunPackageManager(base)
      case 'yarn':
        return new YarnPackageManager(base)
      case 'pnpm':
        return new PnpmPackageManager(base)
      case 'npm':
        return new NpmPackageManager(base)
      default:
        throw new Error(`Unknown package manager: ${pm}`)
    }
  })
})

export function pkg(base: string) {

Review comment (bug):
Added this check to prevent an infinite loop because `dirname('/') === '/'`. This is very unlikely that you would run into this but just in case...

Code after:
    } catch {}
    try {
      await fs.access(resolve(base, 'bun.lock'))
      return 'bun'
    } catch {}
    try {
      await fs.access(resolve(base, 'pnpm-lock.yaml'))
      return 'pnpm'
    } catch {}

    try {
      await fs.access(resolve(base, 'yarn.lock'))
      return 'yarn'
    } catch {}

    try {
      await fs.access(resolve(base, 'package-lock.json'))
      return 'npm'
    } catch {}

    // 3. If no lockfile is found, we might be in a monorepo
    let previousBase = base
    base = dirname(base)

    // Already at the root
    if (previousBase === base) {
      if (!didWarnAboutPackageManager) {
        didWarnAboutPackageManager = true
        warn('Could not detect a package manager. Please manually update `tailwindcss` to v4.')
      }

      return Promise.reject('No package manager detected')
    }
  } while (true)
})