ronantakizawa/codereview-bench huggingface dataset:
Repository: astral-sh/uv
File: crates/uv-warnings/src/lib.rs (Rust)

Code before:
mod tests {
    use crate::write_error_chain;
    use anyhow::anyhow;
    use indoc::indoc;
    use insta::assert_snapshot;
    use owo_colors::AnsiColors;

    #[test]
    fn format_multiline_message() {
        let err_middle = indoc! {"Failed to fetch https://example.com/upload/python3.13.tar.zst
        Server says: This endpoint only support POST requests.

        For downloads, please refer to https://example.com/download/python3.13.tar.zst"};
        let err = anyhow!("Caused By: HTTP Error 400")
            .context(err_middle)
            .context("Failed to download Python 3.12");

        let mut rendered = String::new();
        write_error_chain(err.as_ref(), &mut rendered, "error", AnsiColors::Red).unwrap();
        let rendered = anstream::adapter::strip_str(&rendered);

        assert_snapshot!(rendered, @r"
        error: Failed to download Python 3.12
          Caused by: Failed to fetch https://example.com/upload/python3.13.tar.zst
                     Server says: This endpoint only support POST requests.
                     
                     For downloads, please refer to https://example.com/download/python3.13.tar.zst
          Caused by: Caused By: HTTP Error 400
        ");
    }
}


Review comment (suggestion):
A lot of IDEs are configured to strip trailing whitespace, which will strip this. Could we instead avoid indenting empty lines? This would be more consistent with ruff and rustfmt, which both don't indent empty lines.

Code after:
        }
    }
    Ok(())
}

#[cfg(test)]
mod tests {
    use crate::write_error_chain;
    use anyhow::anyhow;
    use indoc::indoc;
    use insta::assert_snapshot;
    use owo_colors::AnsiColors;

    #[test]
    fn format_multiline_message() {
        let err_middle = indoc! {"Failed to fetch https://example.com/upload/python3.13.tar.zst
        Server says: This endpoint only support POST requests.

        For downloads, please refer to https://example.com/download/python3.13.tar.zst"};
        let err = anyhow!("Caused By: HTTP Error 400")
            .context(err_middle)
            .context("Failed to download Python 3.12");

        let mut rendered = String::new();
        write_error_chain(err.as_ref(), &mut rendered, "error", AnsiColors::Red).unwrap();
        let rendered = anstream::adapter::strip_str(&rendered);

        assert_snapshot!(rendered, @r"
        error: Failed to download Python 3.12
          Caused by: Failed to fetch https://example.com/upload/python3.13.tar.zst
                     Server says: This endpoint only support POST requests.

                     For downloads, please refer to https://example.com/download/python3.13.tar.zst
          Caused by: Caused By: HTTP Error 400
        ");
    }
}
Repository: astral-sh/uv
File: crates/uv/tests/it/pip_install.rs (Rust)

Code before:
    let scripts_dir = context.temp_dir.child("scripts");
    scripts_dir.child("my_script").write_str(indoc! {r#"
        #!python
        print("Hello from script")
    "#})?;

    let scripts_dir = context.temp_dir.child("headers");
    scripts_dir.child("project.h").write_str(indoc! {r"
        #include <stdio.h>
    "})?;

    context.pip_install().arg(".").assert().success();

    // Read the RECORD file and check that all paths use forward slashes.
    let record_path = context
        .site_packages()
        .join("project-0.1.0.dist-info")
        .join("RECORD");

    let record = fs::read_to_string(&record_path)?;
    let record_lines: Vec<_> = record
        .lines()
        .filter(|line| !line.trim().is_empty())
        .collect();
    assert!(record_lines.iter().any(|line| line.contains("my_script")));
    assert!(record_lines.iter().any(|line| line.contains("project.h")));
    for line in record_lines {
        // Each RECORD line is: path,hash,size
        let path = line.split(',').next().unwrap();
        assert!(!path.contains('\\'), "{path}");
    }

    Ok(())
}


Review comment (suggestion):
Might be nonsense but: Maybe we should also that there _are_ forward slashes (e.g. that we didn't accidentally just remove all slashes)?

```suggestion
    assert!(record_lines.iter().any(|line| line.contains("/my_script")));
    assert!(record_lines.iter().any(|line| line.contains("/project.h")));
```

Code after:
    let scripts_dir = context.temp_dir.child("scripts");
    scripts_dir.child("my_script").write_str(indoc! {r#"
        #!python
        print("Hello from script")
    "#})?;

    let scripts_dir = context.temp_dir.child("headers");
    scripts_dir.child("project.h").write_str(indoc! {r"
        #include <stdio.h>
    "})?;

    context.pip_install().arg(".").assert().success();

    // Read the RECORD file and check that all paths use forward slashes.
    let record_path = context
        .site_packages()
        .join("project-0.1.0.dist-info")
        .join("RECORD");

    let record = fs::read_to_string(&record_path)?;
    let record_lines: Vec<_> = record
        .lines()
        .filter(|line| !line.trim().is_empty())
        .collect();
    assert!(record_lines.iter().any(|line| line.contains("/my_script")));
    assert!(record_lines.iter().any(|line| line.contains("/project.h")));
    for line in record_lines {
        // Each RECORD line is: path,hash,size
        let path = line.split(',').next().unwrap();
        assert!(!path.contains('\\'), "{path}");
    }

    Ok(())
}
Repository: astral-sh/uv
File: crates/uv/tests/it/sync.rs (Rust)

Code before:

    Ok(())
}

#[test]
fn sync_python_missing_download_hint() -> Result<()> {
    let context = TestContext::new_with_versions(&["3.12"]).with_managed_python_dirs();

    let pyproject_toml = context.temp_dir.child("pyproject.toml");
    pyproject_toml.write_str(
        r#"
        [project]
        name = "project"
        version = "0.1.0"
        requires-python = ">=3.12"
        dependencies = []
        "#,
    )?;

    let mut filters = context.filters();
    if cfg!(windows) {
        filters.push((
            "managed installations, search path, or registry",
            "managed installations or search path",
        ));
    }

    uv_snapshot!(filters, context.sync().arg("-p").arg("3.100"), @r"
    success: false
    exit_code: 2
    ----- stdout -----

    ----- stderr -----
    error: No interpreter found for Python 3.100 in managed installations or search path

    hint: No Python download is available for Python 3.100. This version might be newer than your uv. Update uv and retry.
    ");

    Ok(())
}

#[test]
fn sync_config_settings_package() -> Result<()> {
    let context = TestContext::new("3.12").with_exclude_newer("2025-07-25T00:00:00Z");

    // Create a child project that uses `setuptools`.
    let dependency = context.temp_dir.child("dependency");
    dependency.child("pyproject.toml").write_str(
        r#"
        [project]
        name = "dependency"

Review comment (suggestion):
Use `.with_filtered_python_sources()` on the context instead. i.e. `let context = TestContext::new_with_versions(&["3.12"]).with_managed_python_dirs().with_filtered_python_sources()`.

Then you can drop this.

Code after:

    Ok(())
}

#[test]
fn sync_python_missing_download_hint() -> Result<()> {
    let context = TestContext::new_with_versions(&["3.12"])
        .with_managed_python_dirs()
        .with_filtered_python_sources();

    let pyproject_toml = context.temp_dir.child("pyproject.toml");
    pyproject_toml.write_str(
        r#"
        [project]
        name = "project"
        version = "0.1.0"
        requires-python = ">=3.12"
        dependencies = []
        "#,
    )?;

    uv_snapshot!(context.filters(), context.sync().arg("-p").arg("3.100"), @r"
    success: false
    exit_code: 2
    ----- stdout -----

    ----- stderr -----
    error: No interpreter found for Python 3.100 in [PYTHON SOURCES]

    hint: uv embeds available Python downloads and may require an update to install new versions. Consider retrying on a newer version of uv.
    ");

    Ok(())
}

#[test]
fn sync_config_settings_package() -> Result<()> {
    let context = TestContext::new("3.12").with_exclude_newer("2025-07-25T00:00:00Z");

    // Create a child project that uses `setuptools`.
    let dependency = context.temp_dir.child("dependency");
    dependency.child("pyproject.toml").write_str(
        r#"
        [project]
        name = "dependency"
        version = "0.1.0"
        requires-python = ">=3.12"
        dependencies = []
        [build-system]
        requires = ["setuptools>=42"]
        build-backend = "setuptools.build_meta"
Repository: astral-sh/uv
File: crates/uv/tests/it/python_find.rs (Rust)

Code before:
    // Create a virtual environment
    uv_snapshot!(context.filters(), context.venv().arg("--python").arg("3.12").arg("-q"), @"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    ");

    // We should find it first
    // TODO(zanieb): On Windows, this has in a different display path for virtual environments which
    // is super annoying and requires some changes to how we represent working directories in the
    // test context to resolve.
    #[cfg(not(windows))]
    uv_snapshot!(context.filters(), context.python_find(), @"
    success: true
    exit_code: 0
    ----- stdout -----
    [VENV]/[BIN]/[PYTHON]

    ----- stderr -----
    ");

    // Even if the `VIRTUAL_ENV` is not set
    #[cfg(not(windows))]
    uv_snapshot!(context.filters(), context.python_find(), @"
    success: true
    exit_code: 0
    ----- stdout -----
    [VENV]/[BIN]/[PYTHON]

    ----- stderr -----
    ");

    let child_dir = context.temp_dir.child("child");
    child_dir.create_dir_all().unwrap();

    // Unless the system flag is passed
    uv_snapshot!(context.filters(), context.python_find().arg("--system"), @"
    success: true
    exit_code: 0
    ----- stdout -----
    [PYTHON-3.11]

    ----- stderr -----
    ");

    // Or, `UV_SYSTEM_PYTHON` is set
    uv_snapshot!(context.filters(), context.python_find().env(EnvVars::UV_SYSTEM_PYTHON, "1"), @"
    success: true
    exit_code: 0

Review comment (suggestion):
@zanieb `python_find` doesn't activate the venv, so it seems like this and the branch above are testing the same thing

Code after:
    // Create a virtual environment
    uv_snapshot!(context.filters(), context.venv().arg("--python").arg("3.12").arg("-q"), @"
    success: true
    exit_code: 0
    ----- stdout -----

    ----- stderr -----
    ");

    // We should find it first
    // TODO(zanieb): On Windows, this has in a different display path for virtual environments which
    // is super annoying and requires some changes to how we represent working directories in the
    // test context to resolve.
    #[cfg(not(windows))]
    uv_snapshot!(context.filters(), context.python_find(), @"
    success: true
    exit_code: 0
    ----- stdout -----
    [VENV]/[BIN]/[PYTHON]

    ----- stderr -----
    ");

    let child_dir = context.temp_dir.child("child");
    child_dir.create_dir_all().unwrap();

    // Unless the system flag is passed
    uv_snapshot!(context.filters(), context.python_find().arg("--system"), @"
    success: true
    exit_code: 0
    ----- stdout -----
    [PYTHON-3.11]

    ----- stderr -----
    ");

    // Or, `UV_SYSTEM_PYTHON` is set
    uv_snapshot!(context.filters(), context.python_find().env(EnvVars::UV_SYSTEM_PYTHON, "1"), @"
    success: true
    exit_code: 0
    ----- stdout -----
    [PYTHON-3.11]

    ----- stderr -----
    ");

    // Unless, `--no-system` is included
    // TODO(zanieb): Report this as a bug upstream — this should be allowed.
    uv_snapshot!(context.filters(), context.python_find().arg("--no-system").env(EnvVars::UV_SYSTEM_PYTHON, "1"), @"
    success: false
    exit_code: 2
Repository: astral-sh/uv
File: crates/uv-platform-tags/src/abi_tag.rs (Rust)

Code before:
use std::fmt::{Display, Formatter, Write};
use std::ops::{Deref, DerefMut};
use std::str::FromStr;

#[derive(
    Default,
    Debug,
    Copy,
    Clone,
    Eq,
    PartialEq,
    Ord,
    PartialOrd,
    Hash,
    rkyv::Archive,
    rkyv::Deserialize,
    rkyv::Serialize,
)]
#[rkyv(derive(Debug))]
#[repr(transparent)]
pub struct CPythonAbiModifiers(u8);

bitflags::bitflags! {
    impl CPythonAbiModifiers: u8 {
        /// A freethreading build of CPython without GIL, Python 3.13+.
        const Freethreading = 1 << 0;
        /// Historical, not needed on CPython 3.3+.
        const WideUnicode = 1 << 1;
        /// A debug build of CPython.
        const Debug = 1 << 2;
        /// Historical, not needed on CPython 3.4+, but only removed with CPython 3.8+.
        ///
        /// <https://github.com/python/cpython/issues/80888#issuecomment-1093821707>
        const Pymalloc = 1 << 3;
    }
}

impl Deref for CPythonAbiModifiers {
    type Target = u8;

    fn deref(&self) -> &u8 {
        &self.0
    }
}

impl DerefMut for CPythonAbiModifiers {

Review comment (bug):
Can we call them "Variants" just for consistency with the way debug and free-threading are used elsewhere.

Code after:
use std::fmt::{Display, Formatter, Write};
use std::ops::{Deref, DerefMut};
use std::str::FromStr;

#[derive(
    Default,
    Debug,
    Copy,
    Clone,
    Eq,
    PartialEq,
    Ord,
    PartialOrd,
    Hash,
    rkyv::Archive,
    rkyv::Deserialize,
    rkyv::Serialize,
)]
#[rkyv(derive(Debug))]
#[repr(transparent)]
pub struct CPythonAbiVariants(u8);

bitflags::bitflags! {
    impl CPythonAbiVariants: u8 {
        /// A freethreading build of CPython without GIL, Python 3.13+.
        const Freethreading = 1 << 0;
        /// Historical, not needed on CPython 3.3+.
        const WideUnicode = 1 << 1;
        /// A debug build of CPython.
        const Debug = 1 << 2;
        /// Historical, not needed on CPython 3.4+, but only removed with CPython 3.8+.
        ///
        /// <https://github.com/python/cpython/issues/80888#issuecomment-1093821707>
        const Pymalloc = 1 << 3;
    }
}

impl Deref for CPythonAbiVariants {
    type Target = u8;

    fn deref(&self) -> &u8 {
        &self.0
    }
}

impl DerefMut for CPythonAbiVariants {
Repository: atom/atom
File: script/lib/transpile-packages-with-custom-transpiler-paths.js (JavaScript)

Code before:
'use strict'

const CompileCache = require('../../src/compile-cache')
const fs = require('fs')
const glob = require('glob')
const path = require('path')

const CONFIG = require('../config')

module.exports = function () {
  console.log(`Transpiling packages with custom transpiler configurations in ${CONFIG.intermediateAppPath}`)
  let pathsToCompile = []
  for (let packageName of Object.keys(CONFIG.appMetadata.packageDependencies)) {
    const packagePath = path.join(CONFIG.intermediateAppPath, 'node_modules', packageName)
    const metadataPath = path.join(packagePath, 'package.json')
    const metadata = require(metadataPath)
    if (metadata.atomTranspilers) {
      CompileCache.addTranspilerConfigForPath(packagePath, metadata.name, metadata, metadata.atomTranspilers)
      for (let config of metadata.atomTranspilers) {
        pathsToCompile = pathsToCompile.concat(glob.sync(path.join(packagePath, config.glob), {nodir: true}))
      }
    }
  }

  for (let path of pathsToCompile) {
    transpilePath(path)
  }
}

function transpilePath (path) {
  fs.writeFileSync(path, CompileCache.addPathToCache(path, CONFIG.atomHomeDirPath))
}


Review comment (suggestion):
Fly-by comment, maybe use [`chalk`](https://www.npmjs.com/package/chalk) in the future instead of using a module that modifies prototypes (because modules modifying prototypes are prone to conflicts and generally unrecommended)

Code after:
'use strict'

const CompileCache = require('../../src/compile-cache')
const fs = require('fs-extra')
const glob = require('glob')
const path = require('path')

const CONFIG = require('../config')
const backupNodeModules = require('./backup-node-modules')
const runApmInstall = require('./run-apm-install')

require('colors')


module.exports = function () {
  console.log(`Transpiling packages with custom transpiler configurations in ${CONFIG.intermediateAppPath}`)
  for (let packageName of Object.keys(CONFIG.appMetadata.packageDependencies)) {
    const packagePath = path.join(CONFIG.intermediateAppPath, 'node_modules', packageName)
    const metadataPath = path.join(packagePath, 'package.json')
    const metadata = require(metadataPath)
    if (metadata.atomTranspilers) {
      console.log(' transpiling for package '.cyan + packageName.cyan)
      const restoreNodeModules = backupNodeModules(packagePath)
      runApmInstall(packagePath)

      CompileCache.addTranspilerConfigForPath(packagePath, metadata.name, metadata, metadata.atomTranspilers)
      for (let config of metadata.atomTranspilers) {
        const pathsToCompile = glob.sync(path.join(packagePath, config.glob), {nodir: true})
        pathsToCompile.forEach(transpilePath)
      }

      restoreNodeModules()
    }
  }
}

function transpilePath (path) {
Repository: atom/atom
File: spec/notification-manager-spec.js (JavaScript)

Code before:

    it('emits an info notification when ::addInfo has been called', () => {
      manager.addInfo('Something!', {icon: 'someIcon'})
      expect(addSpy).toHaveBeenCalled()
      const notification = addSpy.mostRecentCall.args[0]
      expect(notification.getType()).toBe('info')
    })

    it('emits a success notification when ::addSuccess has been called', () => {
      manager.addSuccess('Something!', {icon: 'someIcon'})
      expect(addSpy).toHaveBeenCalled()
      const notification = addSpy.mostRecentCall.args[0]
      expect(notification.getType()).toBe('success')
    })
  })

  describe('clearing notifications', function () {
    it('clears the notifications when ::clear has been called', function(){
      manager.addSuccess('success')
      expect(manager.getNotifications().length).toBe(1)
      manager.clear()
      expect(manager.getNotifications().length).toBe(0)
    })

    describe('adding events', () => {
      let addSpy

      beforeEach(() => {
        addSpy = jasmine.createSpy()
        manager.onDidClearNotifications(addSpy)
      })

      it('emits an event when the notifications have been cleared', () => {
        manager.clear()
        expect(addSpy).toHaveBeenCalled()
      })
    })
  })
})


Review comment (suggestion):
The name `addSpy` doesn't make sense in this context. It looks like that name is used in the previous `describe` because it is an event listener that is registered using `onDidAddNotification`. I think `clearSpy` would make more sense here.

Code after:

    it('emits an info notification when ::addInfo has been called', () => {
      manager.addInfo('Something!', {icon: 'someIcon'})
      expect(addSpy).toHaveBeenCalled()
      const notification = addSpy.mostRecentCall.args[0]
      expect(notification.getType()).toBe('info')
    })

    it('emits a success notification when ::addSuccess has been called', () => {
      manager.addSuccess('Something!', {icon: 'someIcon'})
      expect(addSpy).toHaveBeenCalled()
      const notification = addSpy.mostRecentCall.args[0]
      expect(notification.getType()).toBe('success')
    })
  })

  describe('clearing notifications', () => {
    it('clears the notifications when ::clear has been called', () => {
      manager.addSuccess('success')
      expect(manager.getNotifications().length).toBe(1)
      manager.clear()
      expect(manager.getNotifications().length).toBe(0)
    })

    describe('adding events', () => {
      let clearSpy

      beforeEach(() => {
        clearSpy = jasmine.createSpy()
        manager.onDidClearNotifications(clearSpy)
      })

      it('emits an event when the notifications have been cleared', () => {
        manager.clear()
        expect(clearSpy).toHaveBeenCalled()
      })
    })
  })
})
Repository: atom/atom
File: src/buffered-process.js (JavaScript)

Code before:
    const args = [
      'process',
      'where',
      `(ParentProcessId=${parentPid})`,
      'get',
      'processid'
    ]

    let wmicProcess

    try {
      wmicProcess = ChildProcess.spawn(cmd, args)
    } catch (spawnError) {
      this.killProcess()
      return
    }

    wmicProcess.on('error', () => {}) // ignore errors

    let output = ''
    wmicProcess.stdout.on('data', (data) => {
      output += data
    })
    wmicProcess.stdout.on('close', () => {
      for (let pid of output.split(/\s+/)) {
        if (!isFinite(pid)) continue

        pid = parseInt(pid, 10)

        if (!pid || pid === parentPid || pid < 0 || pid === Infinity) continue

        try {
          process.kill(pid)
        } catch (error) {}
      }

      this.killProcess()
    })
  }

  killProcess () {
    if (this.process) this.process.kill()
    this.process = null
  }

  isExplorerCommand (command) {
    if (command === 'explorer.exe' || command === 'explorer') {
      return true
    } else if (process.env.SystemRoot) {
      return command === path.join(process.env.SystemRoot, 'explorer.exe') || command === path.join(process.env.SystemRoot, 'explorer')
    } else {

Review comment (suggestion):
Should be redundant because `parseInt` only returns an integer value or `NaN`, both cases are covered at the check below.

E: Also returns `Infinity` for a long enough string, but that is also covered below.

Code after:
    const args = [
      'process',
      'where',
      `(ParentProcessId=${parentPid})`,
      'get',
      'processid'
    ]

    let wmicProcess

    try {
      wmicProcess = ChildProcess.spawn(cmd, args)
    } catch (spawnError) {
      this.killProcess()
      return
    }

    wmicProcess.on('error', () => {}) // ignore errors

    let output = ''
    wmicProcess.stdout.on('data', (data) => {
      output += data
    })
    wmicProcess.stdout.on('close', () => {
      for (let pid of output.split(/\s+/)) {
        if (!/^\d{1,10}$/.test(pid)) continue
        pid = parseInt(pid, 10)

        if (!pid || pid === parentPid) continue

        try {
          process.kill(pid)
        } catch (error) {}
      }

      this.killProcess()
    })
  }

  killProcess () {
    if (this.process) this.process.kill()
    this.process = null
  }

  isExplorerCommand (command) {
    if (command === 'explorer.exe' || command === 'explorer') {
      return true
    } else if (process.env.SystemRoot) {
      return command === path.join(process.env.SystemRoot, 'explorer.exe') || command === path.join(process.env.SystemRoot, 'explorer')
    } else {
      return false