ronantakizawa/codereview-bench huggingface dataset:
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/property-order.ts (TypeScript)

Code before:
  'grid-column',
  'grid-column-start',
  'grid-column-end',
  'grid-row',
  'grid-row-start',
  'grid-row-end',
  'float',
  'clear',

  // How do we make `mx-0` come before `mt-0`?
  // Idea: `margin-x` property that we compile away with a Visitor plugin?
  'margin',
  'margin-inline',
  'margin-block',
  'margin-inline-start',
  'margin-inline-end',
  'margin-top',
  'margin-right',
  'margin-bottom',
  'margin-left',

  'box-sizing',
  'display',
  'field-sizing',

  'aspect-ratio',
  'height',
  'max-height',
  'min-height',
  'width',
  'max-width',
  'min-width',

  'flex',
  'flex-shrink',
  'flex-grow',
  'flex-basis',

  'table-layout',
  'caption-side',
  'border-collapse',

  // There's no `border-spacing-x` property, we use variables, how to sort?
  'border-spacing',
  // '--tw-border-spacing-x',
  // '--tw-border-spacing-y',

  'transform-origin',

  'translate',
  '--tw-translate-x',

Review comment (suggestion):
```suggestion

  'field-sizing',
  'aspect-ratio',
```

Code after:
  'grid-column',
  'grid-column-start',
  'grid-column-end',
  'grid-row',
  'grid-row-start',
  'grid-row-end',
  'float',
  'clear',

  // How do we make `mx-0` come before `mt-0`?
  // Idea: `margin-x` property that we compile away with a Visitor plugin?
  'margin',
  'margin-inline',
  'margin-block',
  'margin-inline-start',
  'margin-inline-end',
  'margin-top',
  'margin-right',
  'margin-bottom',
  'margin-left',

  'box-sizing',
  'display',

  'field-sizing',
  'aspect-ratio',
  'height',
  'max-height',
  'min-height',
  'width',
  'max-width',
  'min-width',

  'flex',
  'flex-shrink',
  'flex-grow',
  'flex-basis',

  'table-layout',
  'caption-side',
  'border-collapse',

  // There's no `border-spacing-x` property, we use variables, how to sort?
  'border-spacing',
  // '--tw-border-spacing-x',
  // '--tw-border-spacing-y',

  'transform-origin',

  'translate',
  '--tw-translate-x',
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/template/codemods/automatic-var-injection.ts (TypeScript)

Code before:

  // Shorthand properties
  'scroll-timeline',
  'animation-timeline',
  'view-timeline',
  'position-try',
])
function isAutomaticVarInjectionException(
  designSystem: DesignSystem,
  candidate: Candidate,
): boolean {
  let ast = designSystem.compileAstNodes(candidate).map((n) => n.node)

  console.dir(ast, { depth: null })
  let isException = false
  walk(ast, (node) => {
    if (
      node.kind === 'declaration' &&
      AUTO_VAR_INJECTION_EXCEPTIONS.has(node.property) &&
      node.value?.startsWith('--')
    ) {
      isException = true
    }
  })
  return isException
}


Review comment (refactor):
This _kinda_ works, unless you use a utility that would takes an arbitrary value with `--` but uses it in a _legit_ declaration _while also adding one of the exceptions to the AST. E.g.:

```
matchUtilities({
  foo: (value) => ({ 
    'scrollbar-color': value,
    'scroll-timeline-name': --my-other-thing,
  }),
})
```

I’m not sure how common this usage is but maaybe we can pass the arbitrary value in to this function and ensure that it uses that exact value we're trying to mod in a declaration of that exception to be extra sure?

Code after:
    if (compoundDidChange) {
      didChange = true
    }
  }

  return didChange
}

function createEmptyCandidate(variant: Variant) {
  return {
    kind: 'arbitrary' as const,
    property: 'color',
    value: 'red',
    modifier: null,
    variants: [variant],
    important: false,
    raw: 'candidate',
  } satisfies Candidate
}

const AUTO_VAR_INJECTION_EXCEPTIONS = new Set([
  // Concrete properties
  'scroll-timeline-name',
  'timeline-scope',
  'view-timeline-name',
  'font-palette',
  'anchor-name',
  'anchor-scope',
  'position-anchor',
  'position-try-options',

  // Shorthand properties
  'scroll-timeline',
  'animation-timeline',
  'view-timeline',
  'position-try',
])
// Some properties never had var() injection in v3. We need to convert the candidate to CSS
// so we can check the properties used by the utility.
function isAutomaticVarInjectionException(
  designSystem: DesignSystem,
  candidate: Candidate,
  value: string,
): boolean {
  let ast = designSystem.compileAstNodes(candidate).map((n) => n.node)

  let isException = false
  walk(ast, (node) => {
    if (
      node.kind === 'declaration' &&
      AUTO_VAR_INJECTION_EXCEPTIONS.has(node.property) &&
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/template/codemods/automatic-var-injection.ts (TypeScript)

Code before:
  'position-anchor',
  'position-try-options',

  // Shorthand properties
  'scroll-timeline',
  'animation-timeline',
  'view-timeline',
  'position-try',
])
// Some properties never had var() injection in v3. We need to convert the candidate to CSS
// so we can check the properties used by the utility.
function isAutomaticVarInjectionException(
  designSystem: DesignSystem,
  candidate: Candidate,
  value: string,
): boolean {
  let ast = designSystem.compileAstNodes(candidate).map((n) => n.node)

  let isException = false
  walk(ast, (node) => {
    if (
      node.kind === 'declaration' &&
      AUTO_VAR_INJECTION_EXCEPTIONS.has(node.property) &&
      node.value == value
    ) {
      isException = true
    }
  })
  return isException
}


Review comment (suggestion):
I think once we found an exception, we can stop walking over anything else:

```suggestion
      isException = true
      return WalkAction.Stop
```

Code after:
  'position-anchor',
  'position-try-options',

  // Shorthand properties
  'scroll-timeline',
  'animation-timeline',
  'view-timeline',
  'position-try',
])
// Some properties never had var() injection in v3. We need to convert the candidate to CSS
// so we can check the properties used by the utility.
function isAutomaticVarInjectionException(
  designSystem: DesignSystem,
  candidate: Candidate,
  value: string,
): boolean {
  let ast = designSystem.compileAstNodes(candidate).map((n) => n.node)

  let isException = false
  walk(ast, (node) => {
    if (
      node.kind === 'declaration' &&
      AUTO_VAR_INJECTION_EXCEPTIONS.has(node.property) &&
      node.value == value
    ) {
      isException = true
      return WalkAction.Stop
    }
  })
  return isException
}
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/codemods/migrate-at-apply.test.ts (TypeScript)

Code before:
      .foo {
        @apply flex flex-col #{!important};
      }
    `),
  ).toMatchInlineSnapshot(`
    ".foo {
      @apply flex! flex-col!;
    }"
  `)
})

it('should move the legacy `!` prefix, to the new `!` postfix notation', async () => {
  expect(
    await migrateWithoutConfig(css`
      .foo {
        @apply !flex flex-col! hover:!items-start items-center;
      }
    `),
  ).toMatchInlineSnapshot(`
    ".foo {
      @apply flex! flex-col! hover:items-start! items-center;
    }"
  `)
})

it('should apply all candidate migration when migrating with a config', async () => {
  async function migrateWithConfig(input: string) {
    return postcss()
      .use(
        migrateAtApply({
          designSystem: await __unstable__loadDesignSystem(
            css`
              @import 'tailwindcss' prefix(tw);
            `,
            { base: __dirname },
          ),
          userConfig: {
            prefix: 'tw_',
          },
        }),
      )
      .process(input, { from: expect.getState().testPath })
      .then((result) => result.css)
  }

  expect(
    await migrateWithConfig(css`
      .foo {
        @apply !tw_flex [color:--my-color];
      }
    `),

Review comment (suggestion):
```suggestion
it('should apply all candidate migrations when migrating with a config', async () => {
```

Code after:
      .foo {
        @apply flex flex-col #{!important};
      }
    `),
  ).toMatchInlineSnapshot(`
    ".foo {
      @apply flex! flex-col!;
    }"
  `)
})

it('should move the legacy `!` prefix, to the new `!` postfix notation', async () => {
  expect(
    await migrateWithoutConfig(css`
      .foo {
        @apply !flex flex-col! hover:!items-start items-center;
      }
    `),
  ).toMatchInlineSnapshot(`
    ".foo {
      @apply flex! flex-col! hover:items-start! items-center;
    }"
  `)
})

it('should apply all candidate migration when migrating with a config', async () => {
  async function migrateWithConfig(input: string) {
    return postcss()
      .use(
        migrateAtApply({
          designSystem: await __unstable__loadDesignSystem(
            css`
              @import 'tailwindcss' prefix(tw);
            `,
            { base: __dirname },
          ),
          userConfig: {
            prefix: 'tw_',
          },
        }),
      )
      .process(input, { from: expect.getState().testPath })
      .then((result) => result.css)
  }

  expect(
    await migrateWithConfig(css`
      .foo {
        @apply !tw_flex [color:--my-color] tw_bg-gradient-to-t;
      }
    `),
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-upgrade/src/index.ts (TypeScript)

Code before:
#!/usr/bin/env node

import { globby } from 'globby'
import path from 'node:path'
import { help } from './commands/help'
import { migrate as migrateStylesheet } from './migrate'
import { migrate as migrateTemplate } from './template/migrate'
import { prepareConfig } from './template/prepareConfig'
import { args, type Arg } from './utils/args'
import { isRepoDirty } from './utils/git'
import { eprintln, error, header, highlight, info, success } from './utils/renderer'

const options = {
  '--config': { type: 'string', description: 'Path to the configuration file', alias: '-c' },
  '--help': { type: 'boolean', description: 'Display usage information', alias: '-h' },
  '--force': { type: 'boolean', description: 'Force the migration', alias: '-f' },
  '--version': { type: 'boolean', description: 'Display the version number', alias: '-v' },
} satisfies Arg
const flags = args(options)

if (flags['--help']) {
  help({
    usage: ['npx @tailwindcss/upgrade'],
    options,
  })
  process.exit(0)
}

async function run() {
  eprintln(header())
  eprintln()

  if (!flags['--force']) {

Review comment (suggestion):
```suggestion
import { prepareConfig } from './template/prepare-config'
```

Code after:
#!/usr/bin/env node

import { globby } from 'globby'
import path from 'node:path'
import { help } from './commands/help'
import { migrate as migrateStylesheet } from './migrate'
import { migrate as migrateTemplate } from './template/migrate'
import { prepareConfig } from './template/prepare-config'
import { args, type Arg } from './utils/args'
import { isRepoDirty } from './utils/git'
import { eprintln, error, header, highlight, info, success } from './utils/renderer'

const options = {
  '--config': { type: 'string', description: 'Path to the configuration file', alias: '-c' },
  '--help': { type: 'boolean', description: 'Display usage information', alias: '-h' },
  '--force': { type: 'boolean', description: 'Force the migration', alias: '-f' },
  '--version': { type: 'boolean', description: 'Display the version number', alias: '-v' },
} satisfies Arg
const flags = args(options)

if (flags['--help']) {
  help({
    usage: ['npx @tailwindcss/upgrade'],
    options,
  })
  process.exit(0)
}

async function run() {
  eprintln(header())
  eprintln()

  if (!flags['--force']) {
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/compat/apply-compat-hooks.ts (TypeScript)

Code before:
      }),
    ),
    Promise.all(
      pluginPaths.map(async ([{ id, base }, pluginOptions]) => {
        let loaded = await loadModule(id, base, 'plugin')
        return {
          path: id,
          base: loaded.base,
          plugin: loaded.module as Plugin,
          options: pluginOptions,
        }
      }),
    ),
  ])

  upgradeToFullPluginSupport({
    designSystem,
    base,
    ast,
    globs,
    configs,
    pluginDetails,
  })
}

let alreadyUpgradedSymbol = Symbol('alreadyUpgraded')
function upgradeToFullPluginSupport({
  designSystem,
  base,
  ast,
  globs,
  configs,
  pluginDetails,
}: {
  designSystem: DesignSystem
  base: string
  ast: AstNode[]
  globs: { origin?: string; pattern: string }[]
  configs: {
    path: string
    base: string
    config: UserConfig
  }[]
  pluginDetails: {
    path: string
    base: string
    plugin: Plugin
    options: CssPluginOptions | null
  }[]
}) {
  if ((designSystem as any)[alreadyUpgradedSymbol]) return

Review comment (suggestion):
```suggestion
let alreadyUpgradedSymbol = Symbol()
```

Code after:

function upgradeToFullPluginSupport({
  designSystem,
  base,
  ast,
  globs,
  configs,
  pluginDetails,
}: {
  designSystem: DesignSystem
  base: string
  ast: AstNode[]
  globs: { origin?: string; pattern: string }[]
  configs: {
    path: string
    base: string
    config: UserConfig
  }[]
  pluginDetails: {
    path: string
    base: string
    plugin: Plugin
    options: CssPluginOptions | null
  }[]
}) {
  let pluginConfigs = pluginDetails.map((detail) => {
    if (!detail.options) {
      return { config: { plugins: [detail.plugin] }, base: detail.base }
    }

    if ('__isOptionsFunction' in detail.plugin) {
      return { config: { plugins: [detail.plugin(detail.options)] }, base: detail.base }
    }

    throw new Error(`The plugin "${detail.path}" does not accept options`)
  })

  let userConfig = [...pluginConfigs, ...configs]

  let resolvedConfig = resolveConfig(designSystem, [
    { config: createCompatConfig(designSystem.theme), base },
    ...userConfig,
    { config: { plugins: [darkModePlugin] }, base },
  ])
  let resolvedUserConfig = resolveConfig(designSystem, userConfig)

  let pluginApi = buildPluginApi(designSystem, ast, resolvedConfig)

  for (let { handler } of resolvedConfig.plugins) {
    handler(pluginApi)
  }
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-cli/src/commands/build/index.ts (TypeScript)

Code before:
import watcher from '@parcel/watcher'
import { compile, env } from '@tailwindcss/node'
import { clearRequireCache } from '@tailwindcss/node/require-cache'
import { Scanner, type ChangedContent } from '@tailwindcss/oxide'
import { Features, transform } from 'lightningcss'
import { existsSync, Stats } from 'node:fs'
import fs from 'node:fs/promises'
import path from 'node:path'
import type { Arg, Result } from '../../utils/args'
import { Disposables } from '../../utils/disposables'
import {
  eprintln,
  formatDuration,
  header,
  highlight,
  println,
  relative,
} from '../../utils/renderer'
import { drainStdin, outputFile } from './utils'

const css = String.raw

export function options() {
  return {
    '--input': {
      type: 'string',
      description: 'Input file',
      alias: '-i',
    },
    '--output': {
      type: 'string',

Review comment (suggestion):
```suggestion
import { existsSync, type Stats } from 'node:fs'
```

Code after:
import watcher from '@parcel/watcher'
import { compile, env } from '@tailwindcss/node'
import { clearRequireCache } from '@tailwindcss/node/require-cache'
import { Scanner, type ChangedContent } from '@tailwindcss/oxide'
import { Features, transform } from 'lightningcss'
import { existsSync, type Stats } from 'node:fs'
import fs from 'node:fs/promises'
import path from 'node:path'
import type { Arg, Result } from '../../utils/args'
import { Disposables } from '../../utils/disposables'
import {
  eprintln,
  formatDuration,
  header,
  highlight,
  println,
  relative,
} from '../../utils/renderer'
import { drainStdin, outputFile } from './utils'

const css = String.raw

export function options() {
  return {
    '--input': {
      type: 'string',
      description: 'Input file',
      alias: '-i',
    },
    '--output': {
      type: 'string',
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-cli/src/commands/build/index.ts (TypeScript)

Code before:
    })
  }

  // Setup a watcher for every directory.
  for (let dir of dirs) {
    let { unsubscribe } = await watcher.subscribe(dir, async (err, events) => {
      // Whenever an error occurs we want to let the user know about it but we
      // want to keep watching for changes.
      if (err) {
        console.error(err)
        return
      }

      await Promise.all(
        events.map(async (event) => {
          // We currently don't handle deleted files because it doesn't influence
          // the CSS output. This is because we currently keep all scanned
          // candidates in a cache for performance reasons.
          if (event.type === 'delete') return

          // Ignore directory changes. We only care about file changes
          let stats: Stats | null = null
          try {
            stats = await fs.lstat(event.path)
          } catch {}
          if (stats === null || stats.isDirectory()) {
            return
          }

          // Track the changed file.
          files.add(event.path)
        }),
      )

      // Handle the tracked files at some point in the future.
      enqueueCallback()
    })

    // Ensure we cleanup the watcher when we're done.
    watchers.add(unsubscribe)
  }

  // Cleanup
  return () => {
    watchers.dispose()
    debounceQueue.dispose()
  }
}

function optimizeCss(
  input: string,

Review comment (suggestion):
```suggestion
          if (!stats?.isFile()) {
```

Code after:
    })
  }

  // Setup a watcher for every directory.
  for (let dir of dirs) {
    let { unsubscribe } = await watcher.subscribe(dir, async (err, events) => {
      // Whenever an error occurs we want to let the user know about it but we
      // want to keep watching for changes.
      if (err) {
        console.error(err)
        return
      }

      await Promise.all(
        events.map(async (event) => {
          // We currently don't handle deleted files because it doesn't influence
          // the CSS output. This is because we currently keep all scanned
          // candidates in a cache for performance reasons.
          if (event.type === 'delete') return

          // Ignore directory changes. We only care about file changes
          let stats: Stats | null = null
          try {
            stats = await fs.lstat(event.path)
          } catch {}
          if (!stats?.isFile() && !stats?.isSymbolicLink()) {
            return
          }

          // Track the changed file.
          files.add(event.path)
        }),
      )

      // Handle the tracked files at some point in the future.
      enqueueCallback()
    })

    // Ensure we cleanup the watcher when we're done.
    watchers.add(unsubscribe)
  }

  // Cleanup
  return () => {
    watchers.dispose()
    debounceQueue.dispose()
  }
}

function optimizeCss(
  input: string,
Repository: tailwindlabs/tailwindcss
File: packages/@tailwindcss-cli/src/commands/build/index.ts (TypeScript)

Code before:
    })
  }

  // Setup a watcher for every directory.
  for (let dir of dirs) {
    let { unsubscribe } = await watcher.subscribe(dir, async (err, events) => {
      // Whenever an error occurs we want to let the user know about it but we
      // want to keep watching for changes.
      if (err) {
        console.error(err)
        return
      }

      await Promise.all(
        events.map(async (event) => {
          // We currently don't handle deleted files because it doesn't influence
          // the CSS output. This is because we currently keep all scanned
          // candidates in a cache for performance reasons.
          if (event.type === 'delete') return

          // Ignore directory changes. We only care about file changes
          let stats: Stats | null = null
          try {
            stats = await fs.lstat(event.path)
          } catch {}
          if (!stats?.isFile() || !stats?.isSymbolicLink()) {
            return
          }

          // Track the changed file.
          files.add(event.path)
        }),
      )

      // Handle the tracked files at some point in the future.
      enqueueCallback()
    })

    // Ensure we cleanup the watcher when we're done.
    watchers.add(unsubscribe)
  }

  // Cleanup
  return () => {
    watchers.dispose()
    debounceQueue.dispose()
  }
}

function optimizeCss(
  input: string,

Review comment (suggestion):
```suggestion
          if (!stats?.isFile() && !stats?.isSymbolicLink()) {
```

Code after:
    })
  }

  // Setup a watcher for every directory.
  for (let dir of dirs) {
    let { unsubscribe } = await watcher.subscribe(dir, async (err, events) => {
      // Whenever an error occurs we want to let the user know about it but we
      // want to keep watching for changes.
      if (err) {
        console.error(err)
        return
      }

      await Promise.all(
        events.map(async (event) => {
          // We currently don't handle deleted files because it doesn't influence
          // the CSS output. This is because we currently keep all scanned
          // candidates in a cache for performance reasons.
          if (event.type === 'delete') return

          // Ignore directory changes. We only care about file changes
          let stats: Stats | null = null
          try {
            stats = await fs.lstat(event.path)
          } catch {}
          if (!stats?.isFile() && !stats?.isSymbolicLink()) {
            return
          }

          // Track the changed file.
          files.add(event.path)
        }),
      )

      // Handle the tracked files at some point in the future.
      enqueueCallback()
    })

    // Ensure we cleanup the watcher when we're done.
    watchers.add(unsubscribe)
  }

  // Cleanup
  return () => {
    watchers.dispose()
    debounceQueue.dispose()
  }
}

function optimizeCss(
  input: string,
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/ast.ts (TypeScript)

Code before:
export type Rule = {
  kind: 'rule'
  selector: string
  nodes: AstNode[]
}

export type Declaration = {
  kind: 'declaration'
  property: string
  value: string | undefined
  important: boolean
}

export type Comment = {
  kind: 'comment'
  value: string
}

export type Context = {
  kind: 'context'
  context: Record<string, string>
  nodes: AstNode[]
}

export type AstNode = Rule | Declaration | Comment | Context

export function rule(selector: string, nodes: AstNode[]): Rule {
  return {
    kind: 'rule',
    selector,
    nodes,
  }
}

export function decl(property: string, value: string | undefined): Declaration {
  return {
    kind: 'declaration',
    property,
    value,
    important: false,
  }
}

export function comment(value: string): Comment {
  return {
    kind: 'comment',
    value: value,
  }
}

export function context(context: Record<string, string>, nodes: AstNode[]): Context {

Review comment (suggestion):
If we ever need more like this, we could have a more generic `marker` node where you attach an additional sub-kind or w/e. I think this is the best solution for now though.

Code after:
export type Rule = {
  kind: 'rule'
  selector: string
  nodes: AstNode[]
}

export type Declaration = {
  kind: 'declaration'
  property: string
  value: string | undefined
  important: boolean
}

export type Comment = {
  kind: 'comment'
  value: string
}

export type Context = {
  kind: 'context'
  context: Record<string, string>
  nodes: AstNode[]
}

export type AtRoot = {
  kind: 'at-root'
  nodes: AstNode[]
}

export type AstNode = Rule | Declaration | Comment | Context | AtRoot

export function rule(selector: string, nodes: AstNode[]): Rule {
  return {
    kind: 'rule',
    selector,
    nodes,
  }
}

export function decl(property: string, value: string | undefined): Declaration {
  return {
    kind: 'declaration',
    property,
    value,
    important: false,
  }
}

export function comment(value: string): Comment {
  return {
    kind: 'comment',