ronantakizawa/codereview-bench huggingface dataset:
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/tsup.config.ts (TypeScript)

Code before:
import { defineConfig } from 'tsup'

export default defineConfig([
  {
    format: ['esm'],
    minify: true,
    dts: true,
    entry: {
      lib: 'src/index.ts',
      plugin: 'src/plugin.ts',
      colors: 'src/compat/colors.ts',
      'default-theme': 'src/compat/default-theme.ts',
      flattenColorPalette: 'src/compat/flatten-color-palette.ts',
    },
  },
  {
    format: ['cjs'],
    minify: true,
    dts: true,
    entry: {
      plugin: 'src/plugin.cts',
      lib: 'src/index.cts',
      colors: 'src/compat/colors.cts',
      'default-theme': 'src/compat/default-theme.cts',
      flattenColorPalette: 'src/compat/flatten-color-palette.cts',
    },
  },
])


Review comment (suggestion):
```suggestion
      'flatten-color-palette': 'src/compat/flatten-color-palette.cts',
```

Code after:
import { defineConfig } from 'tsup'

export default defineConfig([
  {
    format: ['esm'],
    minify: true,
    dts: true,
    entry: {
      lib: 'src/index.ts',
      plugin: 'src/plugin.ts',
      colors: 'src/compat/colors.ts',
      'default-theme': 'src/compat/default-theme.ts',
      'flatten-color-palette': 'src/compat/flatten-color-palette.ts',
    },
  },
  {
    format: ['cjs'],
    minify: true,
    dts: true,
    entry: {
      plugin: 'src/plugin.cts',
      lib: 'src/index.cts',
      colors: 'src/compat/colors.cts',
      'default-theme': 'src/compat/default-theme.cts',
      'flatten-color-palette': 'src/compat/flatten-color-palette.cts',
    },
  },
])
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/css-functions.ts (TypeScript)

Code before:
import { Features } from '.'
import { walk, type AstNode } from './ast'
import type { DesignSystem } from './design-system'
import { withAlpha } from './utilities'
import { segment } from './utils/segment'
import * as ValueParser from './value-parser'

const functions: Record<string, (designSystem: DesignSystem, ...args: string[]) => any> = {
  '--alpha': alpha,
  '--spacing': spacing,
  '--theme': theme,
  theme,
}

function alpha(_designSystem: DesignSystem, value: string, alpha: string, ...rest: string[]) {
  if (!value || !alpha) {
    throw new Error(
      `--alpha(…) requires 2 arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
    )
  }

  if (rest.length > 0) {
    throw new Error(
      `--alpha(…) only aaccepts 2 arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
    )
  }

  return withAlpha(value, alpha)
}

function spacing(designSystem: DesignSystem, value: string, ...rest: string[]) {
  if (!value) {
    throw new Error(`--spacing(…) requires a single value, but received none.`)
  }

  if (rest.length > 0) {
    throw new Error(
      `--spacing(…) only accepts a single value, but received ${rest.length + 1} values.`,
    )
  }

  let multiplier = designSystem.theme.resolve(null, ['--spacing'])
  if (!multiplier) {

Review comment (suggestion):
Is it weird starting with `--alpha()`? Not sure what we've done in the past in these situations, but would something like this be better?

```suggestion
      `The --alpha(…) function requires 2 arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
```

Code after:
import { Features } from '.'
import { walk, type AstNode } from './ast'
import type { DesignSystem } from './design-system'
import { withAlpha } from './utilities'
import { segment } from './utils/segment'
import * as ValueParser from './value-parser'

const functions: Record<string, (designSystem: DesignSystem, ...args: string[]) => any> = {
  '--alpha': alpha,
  '--spacing': spacing,
  '--theme': theme,
  theme,
}

function alpha(_designSystem: DesignSystem, value: string, alpha: string, ...rest: string[]) {
  if (!value || !alpha) {
    throw new Error(
      `The --alpha(…) function requires two arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
    )
  }

  if (rest.length > 0) {
    throw new Error(
      `The --alpha(…) function only accepts two arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
    )
  }

  return withAlpha(value, alpha)
}

function spacing(designSystem: DesignSystem, value: string, ...rest: string[]) {
  if (!value) {
    throw new Error(`The --spacing(…) function requires an argument, but received none.`)
  }

  if (rest.length > 0) {
    throw new Error(
      `The --spacing(…) function only accepts a single argument, but received ${rest.length + 1}.`,
    )
  }

  let multiplier = designSystem.theme.resolve(null, ['--spacing'])
  if (!multiplier) {
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/css-functions.ts (TypeScript)

Code before:
    )
  }

  if (rest.length > 0) {
    throw new Error(
      `--alpha(…) only aaccepts 2 arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
    )
  }

  return withAlpha(value, alpha)
}

function spacing(designSystem: DesignSystem, value: string, ...rest: string[]) {
  if (!value) {
    throw new Error(`--spacing(…) requires a single value, but received none.`)
  }

  if (rest.length > 0) {
    throw new Error(
      `--spacing(…) only accepts a single value, but received ${rest.length + 1} values.`,
    )
  }

  let multiplier = designSystem.theme.resolve(null, ['--spacing'])
  if (!multiplier) {
    throw new Error('--spacing(…) depends on the `--spacing` theme value, but it was not found.')
  }

  return `calc(${multiplier} * ${value})`
}

function theme(designSystem: DesignSystem, path: string, ...fallback: string[]) {
  path = eventuallyUnquote(path)

  let resolvedValue = designSystem.resolveThemeValue(path)

  if (!resolvedValue && fallback.length > 0) {
    return fallback.join(', ')
  }

  if (!resolvedValue) {
    throw new Error(
      `Could not resolve value for theme function: \`theme(${path})\`. Consider checking if the path is correct or provide a fallback value to silence this error.`,
    )
  }

  return resolvedValue
}

export const THEME_FUNCTION_INVOCATION = new RegExp(
  Object.keys(functions)

Review comment (suggestion):
```suggestion
    throw new Error('The --spacing(…) function requires that the `--spacing` theme variable be set, but it was not found.')
```

Code after:
    )
  }

  if (rest.length > 0) {
    throw new Error(
      `The --alpha(…) function only accepts two arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
    )
  }

  return withAlpha(value, alpha)
}

function spacing(designSystem: DesignSystem, value: string, ...rest: string[]) {
  if (!value) {
    throw new Error(`The --spacing(…) function requires an argument, but received none.`)
  }

  if (rest.length > 0) {
    throw new Error(
      `The --spacing(…) function only accepts a single argument, but received ${rest.length + 1}.`,
    )
  }

  let multiplier = designSystem.theme.resolve(null, ['--spacing'])
  if (!multiplier) {
    throw new Error(
      'The --spacing(…) function requires that the `--spacing` theme variable exists, but it was not found.',
    )
  }

  return `calc(${multiplier} * ${value})`
}

function theme(designSystem: DesignSystem, path: string, ...fallback: string[]) {
  path = eventuallyUnquote(path)

  let resolvedValue = designSystem.resolveThemeValue(path)

  if (!resolvedValue && fallback.length > 0) {
    return fallback.join(', ')
  }

  if (!resolvedValue) {
    throw new Error(
      `Could not resolve value for theme function: \`theme(${path})\`. Consider checking if the path is correct or provide a fallback value to silence this error.`,
    )
  }

  return resolvedValue
}
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/css-functions.ts (TypeScript)

Code before:
import { Features } from '.'
import { walk, type AstNode } from './ast'
import type { DesignSystem } from './design-system'
import { withAlpha } from './utilities'
import { segment } from './utils/segment'
import * as ValueParser from './value-parser'

const functions: Record<string, (designSystem: DesignSystem, ...args: string[]) => any> = {
  '--alpha': alpha,
  '--spacing': spacing,
  '--theme': theme,
  theme,
}

function alpha(_designSystem: DesignSystem, value: string, alpha: string, ...rest: string[]) {
  if (!value || !alpha) {
    throw new Error(
      `The --alpha(…) function requires 2 arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
    )
  }

  if (rest.length > 0) {
    throw new Error(
      `--alpha(…) only aaccepts 2 arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
    )
  }

  return withAlpha(value, alpha)
}

function spacing(designSystem: DesignSystem, value: string, ...rest: string[]) {
  if (!value) {
    throw new Error(`The --spacing(…) function requires an argument, but received none.`)
  }

  if (rest.length > 0) {
    throw new Error(
      `The --spacing(…) function only accepts a single argument, but received ${rest.length + 1}.`,
    )
  }

  let multiplier = designSystem.theme.resolve(null, ['--spacing'])
  if (!multiplier) {

Review comment (suggestion):
Not sure this is right but generally if the number is less than 10 I write it out.

```suggestion
      `The --alpha(…) function requires two arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
```

Code after:
import { Features } from '.'
import { walk, type AstNode } from './ast'
import type { DesignSystem } from './design-system'
import { withAlpha } from './utilities'
import { segment } from './utils/segment'
import * as ValueParser from './value-parser'

const functions: Record<string, (designSystem: DesignSystem, ...args: string[]) => any> = {
  '--alpha': alpha,
  '--spacing': spacing,
  '--theme': theme,
  theme,
}

function alpha(_designSystem: DesignSystem, value: string, alpha: string, ...rest: string[]) {
  if (!value || !alpha) {
    throw new Error(
      `The --alpha(…) function requires two arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
    )
  }

  if (rest.length > 0) {
    throw new Error(
      `The --alpha(…) function only accepts two arguments, e.g.: \`--alpha(${value || 'var(--my-color)'}, ${alpha || '50%'})\``,
    )
  }

  return withAlpha(value, alpha)
}

function spacing(designSystem: DesignSystem, value: string, ...rest: string[]) {
  if (!value) {
    throw new Error(`The --spacing(…) function requires an argument, but received none.`)
  }

  if (rest.length > 0) {
    throw new Error(
      `The --spacing(…) function only accepts a single argument, but received ${rest.length + 1}.`,
    )
  }

  let multiplier = designSystem.theme.resolve(null, ['--spacing'])
  if (!multiplier) {
Repository: tailwindlabs/tailwindcss
File: crates/oxide/src/scanner/allowed_paths.rs (Rust)

Code before:

  // If we are in a git repo then require it to ensure that only rules within
  // the repo are used. For example, we don't want to consider a .gitnore file
  // in the user's home folder if we're in a git repo.
  //
  // The alternative is using a call like `.parents(false)` but that will
  // prevent looking at parent directories for .gitignore files from within
  // the repo and that's not what we want.
  //
  // For example, in a project with this structure:
  //
  // home
  // .gitignore
  //  my-project
  //   .gitignore
  //   apps
  //     .gitignore
  //     web
  //       {root}
  //
  // We do want to consider all .gitignore files listed:
  // - home/.gitignore
  // - my-project/.gitignore
  // - my-project/apps/.gitignore
  //
  // However, if a repo is initalized inside my-project then only the following
  // make sense for consideration:
  // - my-project/.gitignore
  // - my-project/apps/.gitignore
  //
  // Setting the require_git(true) flag conditionally allows us to do this.
  for parent in root.ancestors() {
    if parent.join(".git").exists() {
      builder.require_git(true);
      break;
    }
  }

  builder
}

pub fn is_allowed_content_path(path: &Path) -> bool {
    // Skip known ignored files
    if path
        .file_name()
        .unwrap()
        .to_str()
        .map(|s| IGNORED_FILES.contains(&s))
        .unwrap_or(false)
    {
        return false;

Review comment (suggestion):
```suggestion
  // However, if a repo is initialized inside my-project then only the following
```

Code after:
    // the repo are used. For example, we don't want to consider a .gitignore file
    // in the user's home folder if we're in a git repo.
    //
    // The alternative is using a call like `.parents(false)` but that will
    // prevent looking at parent directories for .gitignore files from within
    // the repo and that's not what we want.
    //
    // For example, in a project with this structure:
    //
    // home
    // .gitignore
    //  my-project
    //   .gitignore
    //   apps
    //     .gitignore
    //     web
    //       {root}
    //
    // We do want to consider all .gitignore files listed:
    // - home/.gitignore
    // - my-project/.gitignore
    // - my-project/apps/.gitignore
    //
    // However, if a repo is initialized inside my-project then only the following
    // make sense for consideration:
    // - my-project/.gitignore
    // - my-project/apps/.gitignore
    //
    // Setting the require_git(true) flag conditionally allows us to do this.
    for parent in root.ancestors() {
        if parent.join(".git").exists() {
            builder.require_git(true);
            break;
        }
    }

    builder
}

pub fn is_allowed_content_path(path: &Path) -> bool {
    // Skip known ignored files
    if path
        .file_name()
        .unwrap()
        .to_str()
        .map(|s| IGNORED_FILES.contains(&s))
        .unwrap_or(false)
    {
        return false;
    }
Repository: tailwindlabs/tailwindcss
File: crates/oxide/src/lib.rs (Rust)

Code before:
}

fn read_changed_content(c: ChangedContent) -> Option<Vec<u8>> {
    if let Some(content) = c.content {
        return Some(content.into_bytes());
    }

    let Some(file) = c.file else {
        return Default::default();
    };

    let Ok(content) = std::fs::read(&file).map_err(|e| {
        event!(tracing::Level::ERROR, "Failed to read file: {:?}", e);
        e
    }) else {
        return Default::default();
    };

    let Some(extension) = file.extension().map(|x| x.to_str()) else {
        return Some(content);
    };

    match extension {
        // Angular class shorthand
        Some("html") => Some(content.replace("[class.", "[")),
        Some("svelte") => Some(content.replace(" class:", " ").replace("\tclass:", " ")),
        _ => Some(content),
    }
}

#[tracing::instrument(skip_all)]
fn read_all_files(changed_content: Vec<ChangedContent>) -> Vec<Vec<u8>> {
    event!(
        tracing::Level::INFO,
        "Reading {:?} file(s)",
        changed_content.len()
    );

    changed_content
        .into_par_iter()
        .filter_map(read_changed_content)
        .collect()
}

#[tracing::instrument(skip_all)]
fn parse_all_blobs(blobs: Vec<Vec<u8>>) -> Vec<String> {
    let mut result: Vec<_> = blobs
        .par_iter()
        .flat_map(|blob| blob.par_split(|x| matches!(x, b'\n')))
        .map(|blob| Extractor::unique(blob, Default::default()))
        .reduce(Default::default, |mut a, b| {

Review comment (suggestion):
```suggestion
        Some("svelte") => Some(content.replace(" class:", " ").replace("\tclass:", " ").replace("\nclass:", " ")),
```

Code after:
}

fn read_changed_content(c: ChangedContent) -> Option<Vec<u8>> {
    if let Some(content) = c.content {
        return Some(content.into_bytes());
    }

    let Some(file) = c.file else {
        return Default::default();
    };

    let Ok(content) = std::fs::read(&file).map_err(|e| {
        event!(tracing::Level::ERROR, "Failed to read file: {:?}", e);
        e
    }) else {
        return Default::default();
    };

    let Some(extension) = file.extension().map(|x| x.to_str()) else {
        return Some(content);
    };

    match extension {
        // Angular class shorthand
        Some("html") => Some(content.replace("[class.", "[")),
        Some("svelte") => Some(
            content
                .replace(" class:", " ")
                .replace("\tclass:", " ")
                .replace("\nclass:", " "),
        ),
        _ => Some(content),
    }
}

#[tracing::instrument(skip_all)]
fn read_all_files(changed_content: Vec<ChangedContent>) -> Vec<Vec<u8>> {
    event!(
        tracing::Level::INFO,
        "Reading {:?} file(s)",
        changed_content.len()
    );

    changed_content
        .into_par_iter()
        .filter_map(read_changed_content)
        .collect()
}

#[tracing::instrument(skip_all)]
fn parse_all_blobs(blobs: Vec<Vec<u8>>) -> Vec<String> {
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/css-parser.ts (TypeScript)

Code before:
        }

        // End of a block.
        else if (
          peekChar === CLOSE_PAREN ||
          peekChar === CLOSE_BRACKET ||
          peekChar === CLOSE_CURLY
        ) {
          if (
            closingBracketStack.length > 0 &&
            input[j] === closingBracketStack[closingBracketStack.length - 1]
          ) {
            closingBracketStack = closingBracketStack.slice(0, -1)
          }
        }
      }

      let declaration = parseDeclaration(buffer, colonIdx)
      if (declaration) {
        if (parent) {
          parent.nodes.push(declaration)
        } else {
          ast.push(declaration)
        }
      } else {
        throw new Error(`Invalid custom property, expected a value`)
      }

      buffer = ''
    }

    // End of a body-less at-rule.
    //
    // E.g.:
    //
    // ```css
    // @charset "UTF-8";
    //                 ^
    // ```
    else if (currentChar === SEMICOLON && buffer.charCodeAt(0) === AT_SIGN) {
      node = parseAtRule(buffer)

      // At-rule is nested inside of a rule, attach it to the parent.
      if (parent) {
        parent.nodes.push(node)
      }

      // We are the root node which means we are done with the current node.
      else {
        ast.push(node)
      }

Review comment (suggestion):
Now that we're throwing errors maybe we can eliminate some of the nesting here?

```js
let declaration = parseDeclaration(buffer, colonIdx)
if (!declaration) {
  throw new Error(`Invalid custom property, expected a value`)
}

if (parent) {
  parent.nodes.push(declaration)
} else {
  ast.push(declaration)
}
```


Code after:
        }

        // End of a block.
        else if (
          peekChar === CLOSE_PAREN ||
          peekChar === CLOSE_BRACKET ||
          peekChar === CLOSE_CURLY
        ) {
          if (
            closingBracketStack.length > 0 &&
            input[j] === closingBracketStack[closingBracketStack.length - 1]
          ) {
            closingBracketStack = closingBracketStack.slice(0, -1)
          }
        }
      }

      let declaration = parseDeclaration(buffer, colonIdx)
      if (!declaration) throw new Error(`Invalid custom property, expected a value`)

      if (parent) {
        parent.nodes.push(declaration)
      } else {
        ast.push(declaration)
      }

      buffer = ''
    }

    // End of a body-less at-rule.
    //
    // E.g.:
    //
    // ```css
    // @charset "UTF-8";
    //                 ^
    // ```
    else if (currentChar === SEMICOLON && buffer.charCodeAt(0) === AT_SIGN) {
      node = parseAtRule(buffer)

      // At-rule is nested inside of a rule, attach it to the parent.
      if (parent) {
        parent.nodes.push(node)
      }

      // We are the root node which means we are done with the current node.
      else {
        ast.push(node)
      }

      // Reset the state for the next node.
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/ast.ts (TypeScript)

Code before:
    else if (node.kind === 'at-rule' && node.name === '@property' && depth === 0) {
      // Don't output duplicate `@property` rules
      if (seenAtProperties.has(node.params)) {
        return
      }

      seenAtProperties.add(node.params)

      let copy = { ...node, nodes: [] }
      for (let child of node.nodes) {
        transform(child, copy.nodes, depth + 1)
      }
      parent.push(copy)
    }

    // AtRule
    else if (node.kind === 'at-rule') {
      let copy = { ...node, nodes: [] }
      for (let child of node.nodes) {
        transform(child, copy.nodes, depth + 1)
      }
      if (
        copy.nodes.length > 0 ||
        copy.name === '@layer' ||
        copy.name === '@charset' ||
        copy.name === '@custom-media'
      ) {
        parent.push(copy)
      }
    }

    // AtRoot
    else if (node.kind === 'at-root') {
      for (let child of node.nodes) {
        let newParent: AstNode[] = []
        transform(child, newParent, 0)
        for (let child of newParent) {
          atRoots.push(child)
        }
      }
    }

    // Context
    else if (node.kind === 'context') {
      // Remove reference imports from printing
      if (node.context.reference) {
        return
      }

      for (let child of node.nodes) {
        transform(child, parent, depth)

Review comment (suggestion):
```suggestion
        copy.name === '@custom-media' ||
        copy.name === '@namespace'
```

https://developer.mozilla.org/en-US/docs/Web/CSS/@namespace

Code after:
    else if (node.kind === 'at-rule' && node.name === '@property' && depth === 0) {
      // Don't output duplicate `@property` rules
      if (seenAtProperties.has(node.params)) {
        return
      }

      seenAtProperties.add(node.params)

      let copy = { ...node, nodes: [] }
      for (let child of node.nodes) {
        transform(child, copy.nodes, depth + 1)
      }
      parent.push(copy)
    }

    // AtRule
    else if (node.kind === 'at-rule') {
      let copy = { ...node, nodes: [] }
      for (let child of node.nodes) {
        transform(child, copy.nodes, depth + 1)
      }
      if (
        copy.nodes.length > 0 ||
        copy.name === '@layer' ||
        copy.name === '@charset' ||
        copy.name === '@custom-media' ||
        copy.name === '@namespace'
      ) {
        parent.push(copy)
      }
    }

    // AtRoot
    else if (node.kind === 'at-root') {
      for (let child of node.nodes) {
        let newParent: AstNode[] = []
        transform(child, newParent, 0)
        for (let child of newParent) {
          atRoots.push(child)
        }
      }
    }

    // Context
    else if (node.kind === 'context') {
      // Remove reference imports from printing
      if (node.context.reference) {
        return
      }

      for (let child of node.nodes) {
Repository: tailwindlabs/tailwindcss
File: packages/tailwindcss/src/ast.test.ts (TypeScript)

Code before:
        }
        &:focus {
        }
      }
    }
    @media (min-width: 768px);
    @media (min-width: 768px) {
      .foo {
      }
      @media (min-width: 1024px) {
        .bar {
        }
      }
    }
    @charset 'UTF-8';
    @layer foo, bar, baz;
    @custom-media --modern (color), (hover);
    "
  `)

  expect(toCss(optimizeAst(ast))).toMatchInlineSnapshot(`
    "@charset 'UTF-8';
    @layer foo, bar, baz;
    @custom-media --modern (color), (hover);
    "
  `)
})


Review comment (suggestion):
```suggestion
    @custom-media --modern (color), (hover);
    @namespace \"http://www.w3.org/1999/xhtml\";
```

Code after:
        &:hover {
        }
        &:focus {
        }
      }
    }
    @media (min-width: 768px);
    @media (min-width: 768px) {
      .foo {
      }
      @media (min-width: 1024px) {
        .bar {
        }
      }
    }
    @charset "UTF-8";
    @layer foo, bar, baz;
    @custom-media --modern (color), (hover);
    @namespace 'http://www.w3.org/1999/xhtml';
    "
  `)

  expect(toCss(optimizeAst(ast))).toMatchInlineSnapshot(`
    "@charset "UTF-8";
    @layer foo, bar, baz;
    @custom-media --modern (color), (hover);
    @namespace 'http://www.w3.org/1999/xhtml';
    "
  `)
})