From d5ca7b7d208dc3a6d8995de7e390a1c1e847d30d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 21:06:41 +0000 Subject: [PATCH 1/5] bun_core: make clippy pass on the Windows and FreeBSD targets, lint them in CI `cargo clippy -p bun_core` fails with 36 errors on a Windows host (and 2 on FreeBSD): every lint is in cfg-gated code that the ubuntu-only Clippy job never compiles. Since every other crate depends on bun_core, clippy on a Windows host reports nothing else until this crate is clean. - lib.rs: SAFETY comments for the `os::ENVIRON` accessors, the `_strnicmp` block and each Windows `Zeroable` impl (one per impl, as the libc group above already does). - util.rs: `PathBuffer::uninit` allows `large_stack_frames` (two MAX_PATH_BYTES locals, ~196 KB on Windows, neither written); drop the no-op `HANDLE as *mut c_void` casts; strip the `\\?\` prefix in `self_exe_path` in place instead of reallocating; `&raw mut` for the `CommandLineToArgvW` out-param; `.cast()` plus a SAFETY comment in the FreeBSD `fd_path_raw` branch. - debug.rs: `&raw mut` for the RtlLookupFunctionEntry/RtlVirtualUnwind out-params. scripts/rust-clippy-cross.ts runs the workspace lint with --target for x86_64-pc-windows-msvc and x86_64-unknown-freebsd from any host, excluding the crates that still fail there (a ratchet: entries are removed as crates get cleaned up). The Clippy workflow installs those two targets' std and runs it after the host lint; test/internal/rust-clippy-cross.test.ts pins bun_core itself on each target. --- .github/workflows/clippy.yml | 22 ++++++- CLAUDE.md | 2 +- package.json | 1 + scripts/rust-clippy-cross.ts | 83 +++++++++++++++++++++++++ src/bun_core/debug.rs | 12 ++-- src/bun_core/lib.rs | 36 ++++++++++- src/bun_core/util.rs | 28 ++++++--- test/internal/rust-clippy-cross.test.ts | 81 ++++++++++++++++++++++++ 8 files changed, 246 insertions(+), 19 deletions(-) create mode 100644 scripts/rust-clippy-cross.ts create mode 100644 test/internal/rust-clippy-cross.test.ts diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 5d9a350671c8..73c4f9a51395 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -13,6 +13,7 @@ on: - "src/codegen/**" - "scripts/build/**" - "scripts/build.ts" + - "scripts/rust-clippy-cross.ts" - "Cargo.toml" - "Cargo.lock" - "clippy.toml" @@ -26,9 +27,14 @@ env: BUN_VERSION: "1.3.14" LLVM_VERSION_MAJOR: "21" # Pin the toolchain explicitly so rustup ignores rust-toolchain.toml's - # `targets` list (11 cross triples ≈ 450 MB of prebuilt std we don't need - # to lint the host). Keep in sync with `channel` in rust-toolchain.toml. + # `targets` list (11 cross triples ≈ 450 MB of prebuilt std); only the + # triples below get their std. Keep in sync with `channel` in + # rust-toolchain.toml. RUSTUP_TOOLCHAIN: nightly-2026-07-20 + # The triples `rust:clippy-cross` lints (clippy needs their std, nothing + # else). Keep in sync with CROSS_CLIPPY_TARGETS in scripts/rust-clippy-cross.ts; + # if it drifts, cargo fails naming the triple that is missing. + CROSS_CLIPPY_TARGETS: x86_64-pc-windows-msvc,x86_64-unknown-freebsd jobs: clippy: @@ -59,12 +65,13 @@ jobs: - name: Setup Rust run: | - rustup toolchain install "$RUSTUP_TOOLCHAIN" --profile minimal --component clippy + rustup toolchain install "$RUSTUP_TOOLCHAIN" --profile minimal --component clippy --target "$CROSS_CLIPPY_TARGETS" rustup override set "$RUSTUP_TOOLCHAIN" # rustc/clippy diagnostics → inline PR annotations echo "::add-matcher::.github/rust-matcher.json" - name: Generate codegen + id: codegen # bun_runtime/bun_jsc/bun_core `include!()` files under # build/debug/codegen/; clippy can't lint those crates without them. # `clone-lolhtml` fetches the vendored Rust path-dep @@ -78,3 +85,12 @@ jobs: env: BUN_CODEGEN_DIR: ${{ github.workspace }}/build/debug/codegen run: bun run rust:clippy + + - name: cargo clippy (cross targets) + # `cfg(windows)` / `cfg(target_os = "freebsd")` code is invisible to + # the host run above. Runs even when that one failed so a PR gets both + # sets of diagnostics at once. + if: ${{ !cancelled() && steps.codegen.outcome == 'success' }} + env: + BUN_CODEGEN_DIR: ${{ github.workspace }}/build/debug/codegen + run: bun run rust:clippy-cross diff --git a/CLAUDE.md b/CLAUDE.md index 0b28f133d3f6..49fc1d3c5392 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -198,7 +198,7 @@ Several situational sections live in `.claude/docs/landing-prs.md` — read the 6. **Use absolute paths** - Always use absolute paths in file operations 7. **Avoid shell commands** - Don't use `find` or `grep` in tests; use Bun's Glob and built-in tools 8. **Memory management** - Prefer RAII (`Drop`) over manual cleanup. Arena edge case: values allocated in an arena (`Arena`/`bumpalo`) do **not** run `Drop` on arena reset — types owning a heap allocation or refcount must be freed/deref'd explicitly first, mirroring the original Zig `deinit()` order. -9. **Cross-platform** - Run `bun run rust:check-all` to compile across all targets (linux/macos/windows × x64/aarch64) when making platform-specific changes. `#[cfg(...)]`-gated code is not type-checked unless the matching target is built. +9. **Cross-platform** - Run `bun run rust:check-all` to compile across all targets (linux/macos/windows × x64/aarch64) when making platform-specific changes. `#[cfg(...)]`-gated code is not type-checked unless the matching target is built. Same for clippy: CI lints the Linux host plus the targets listed in `scripts/rust-clippy-cross.ts`, so also run `bun run rust:clippy-cross` when touching `cfg(windows)` / `cfg(target_os = ...)` code. 10. **Debug builds** - Use `BUN_DEBUG_QUIET_LOGS=1` to disable debug logging, or `BUN_DEBUG_=1` to enable a specific `bun_core::output` scoped logger 11. **Be humble & honest** - NEVER overstate what you got done or what actually works in commits, PRs or in messages to the user. 12. **Branch names must start with `claude/`** - This is a requirement for the CI to work. diff --git a/package.json b/package.json index 2cad3d1c6d8a..ad80f3ead346 100644 --- a/package.json +++ b/package.json @@ -71,6 +71,7 @@ "rust:check": "cargo check --workspace --keep-going", "rust:check-all": "bun scripts/rust-check-all.ts", "rust:clippy": "cargo clippy --workspace --no-deps --keep-going", + "rust:clippy-cross": "bun scripts/rust-clippy-cross.ts", "rust:miri": "bun scripts/rust-miri.ts", "rust:timings": "bun scripts/rust-timings.ts", "codegen:string-maps": "for f in src/**/*.string-map.ts; do bun src/codegen/generate-string-map.ts \"$f\" \"${f%.string-map.ts}.generated.rs\"; done", diff --git a/scripts/rust-clippy-cross.ts b/scripts/rust-clippy-cross.ts new file mode 100644 index 000000000000..cc6c0e15f570 --- /dev/null +++ b/scripts/rust-clippy-cross.ts @@ -0,0 +1,83 @@ +#!/usr/bin/env bun +/** + * `cargo clippy` for targets other than the host. + * + * `bun run rust:clippy` (the CI Clippy job, ubuntu) only lints the code that is + * compiled for x86_64-unknown-linux-gnu; everything behind `#[cfg(windows)]`, + * `#[cfg(target_os = "freebsd")]`, ... is never seen by clippy unless someone + * runs it on that OS. This runs the same workspace lint with `--target` for + * the targets below (clippy only needs the target's prebuilt std, not a + * linker or sysroot, so it works from any host). + * + * Each target lists the crates that still fail on it; they are `--exclude`d + * so every other crate stays pinned. The lists are a ratchet: delete a crate + * once it lints clean on that target, never add one (a listed-clean crate + * that starts failing is a regression in the change that broke it). An empty + * list lints the whole workspace, same as the host run. + * + * Usage: + * bun run rust:clippy-cross # every target below + * bun run rust:clippy-cross -p bun_core # extra args replace the package selection + * + * Needs the configure step (`bun run build --configure-only` + + * `ninja -C build/debug clone-lolhtml`) and `rustup target add ` for + * each target; cargo names the missing one if it is not installed. + */ + +import { spawnSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { resolve } from "node:path"; + +export const CROSS_CLIPPY_TARGETS: Record = { + // aarch64-pc-windows-msvc reports the identical set; one Windows triple is + // enough to cover `cfg(windows)`. + "x86_64-pc-windows-msvc": [ + "bun_bundler", + "bun_bunfig", + "bun_cares_sys", + "bun_crash_handler", + "bun_install", + "bun_io", + "bun_jsc", + "bun_libuv_sys", + "bun_md", + "bun_patch", + "bun_paths", + "bun_resolver", + "bun_router", + "bun_runtime", + "bun_spawn", + "bun_spawn_sys", + "bun_standalone_graph", + "bun_sys", + "bun_threading", + "bun_uws_sys", + "bun_watcher", + "bun_which", + ], + "x86_64-unknown-freebsd": ["bun_crash_handler", "bun_glob", "bun_http", "bun_runtime", "bun_sys", "bun_threading"], +}; + +if (import.meta.main) { + const repo = resolve(import.meta.dirname, ".."); + const codegenDir = process.env.BUN_CODEGEN_DIR ?? resolve(repo, "build/debug/codegen"); + for (const [path, hint] of [ + [resolve(codegenDir, "build_options.rs"), "bun run build --configure-only"], + [resolve(repo, "vendor/lolhtml/Cargo.toml"), "ninja -C build/debug clone-lolhtml"], + ] as const) { + if (!existsSync(path)) { + console.error(`\x1b[31m[error]\x1b[0m ${path} is missing; run: ${hint}`); + process.exit(1); + } + } + + const extraArgs = process.argv.slice(2); + let failed = 0; + for (const [triple, excluded] of Object.entries(CROSS_CLIPPY_TARGETS)) { + const selection = extraArgs.length > 0 ? extraArgs : ["--workspace", ...excluded.flatMap(c => ["--exclude", c])]; + const args = ["clippy", "--no-deps", "--keep-going", "--target", triple, ...selection]; + console.log(`\x1b[36m[clippy]\x1b[0m cargo ${args.join(" ")}`); + if (spawnSync("cargo", args, { stdio: "inherit", cwd: repo }).status !== 0) failed++; + } + process.exit(failed > 0 ? 1 : 0); +} diff --git a/src/bun_core/debug.rs b/src/bun_core/debug.rs index 9ff90f16af01..53701172f27f 100644 --- a/src/bun_core/debug.rs +++ b/src/bun_core/debug.rs @@ -385,7 +385,11 @@ pub fn capture_from_context(pc: usize, fp: usize, out: &mut [usize]) -> usize { // SAFETY: `control_pc` is a code address from the fault context; // `image_base` is valid for write; history table may be null. let rf = unsafe { - ntdll::RtlLookupFunctionEntry(control_pc, &mut image_base, core::ptr::null_mut()) + ntdll::RtlLookupFunctionEntry( + control_pc, + &raw mut image_base, + core::ptr::null_mut(), + ) }; if rf.is_null() { // Leaf function with no `.pdata` entry: manually pop the @@ -424,9 +428,9 @@ pub fn capture_from_context(pc: usize, fp: usize, out: &mut [usize]) -> usize { image_base, control_pc, rf, - &mut ctx, - &mut handler_data, - &mut establisher_frame, + &raw mut ctx, + &raw mut handler_data, + &raw mut establisher_frame, core::ptr::null_mut(), ); } diff --git a/src/bun_core/lib.rs b/src/bun_core/lib.rs index 7a6664d9e134..483147ddf089 100644 --- a/src/bun_core/lib.rs +++ b/src/bun_core/lib.rs @@ -230,11 +230,16 @@ pub mod os { pub unsafe fn take_environ() -> (*mut *mut c_char, usize) { // `&raw mut` (no intermediate `&mut`) — `static_mut_refs` is hard-denied // under rust_2024_compatibility, and we never need a borrow here. + // SAFETY: `ENVIRON` is a valid, initialized static; the caller's + // single-threaded-startup contract rules out a concurrent access. unsafe { core::ptr::replace(&raw mut ENVIRON, (core::ptr::null_mut(), 0)) } } /// SAFETY: single-threaded startup only; `ptr` must be valid for `len` /// elements for the process lifetime (leaked allocation). pub unsafe fn set_environ(ptr: *mut *mut c_char, len: usize) { + // SAFETY: `ENVIRON` is a valid static of `Copy` data (nothing to drop); + // the caller's single-threaded-startup contract rules out a concurrent + // access. unsafe { core::ptr::write(&raw mut ENVIRON, (ptr, len)); } @@ -242,6 +247,10 @@ pub mod os { /// Borrowed view of the current envp slice (read side of the `ENVIRON` global). /// SAFETY: caller must not race with `set_environ`. pub unsafe fn environ() -> &'static [*mut c_char] { + // SAFETY: `ENVIRON` is a valid, initialized static and the caller + // guarantees no concurrent `set_environ`. A non-null `p` was stored by + // `set_environ`, whose contract makes it valid for `n` elements for the + // rest of the process, so the `'static` slice is sound. unsafe { let (p, n) = core::ptr::read(&raw const ENVIRON); if p.is_null() { @@ -1284,6 +1293,7 @@ pub(crate) mod strings_impl { libc::strncasecmp(a.as_ptr().cast(), b.as_ptr().cast(), a.len()) == 0 } // Windows MSVC libc has no `strncasecmp`; `_strnicmp` is the equivalent. + // SAFETY: a.len() <= b.len() here; _strnicmp reads at most a.len() bytes from each. #[cfg(windows)] unsafe { unsafe extern "C" { @@ -2611,50 +2621,72 @@ pub mod ffi { unsafe impl Zeroable for libc::_umtx_time {} // Windows POD — `bun_windows_sys` `#[repr(C)]` out-param structs that are - // zero-init before the kernel fills them. All fields are integers / raw - // pointers / nested POD; audited against the Win32 SDK headers (S016). + // zero-init before the kernel fills them; audited against the Win32 SDK + // headers (S016). + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::IO_STATUS_BLOCK {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::FILE_BASIC_INFORMATION {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::FILE_ALL_INFORMATION {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::FILE_FS_DEVICE_INFORMATION {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::FILE_FS_VOLUME_INFORMATION {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::BY_HANDLE_FILE_INFORMATION {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::WIN32_FILE_ATTRIBUTE_DATA {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::WIN32_FIND_DATAW {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::OBJECT_ATTRIBUTES {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::UNICODE_STRING {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::SECURITY_ATTRIBUTES {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::FILETIME {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::ws2_32::WSADATA {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::ws2_32::sockaddr_storage {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::ws2_32::sockaddr_in {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::ws2_32::sockaddr_in6 {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::ws2_32::addrinfo {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::IO_COUNTERS {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::JOBOBJECT_BASIC_LIMIT_INFORMATION {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::JOBOBJECT_EXTENDED_LIMIT_INFORMATION {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::OVERLAPPED {} + // SAFETY: C POD (integer/array/raw-pointer fields only); all-zero is valid. #[cfg(windows)] unsafe impl Zeroable for bun_windows_sys::externs::PROCESS_INFORMATION {} diff --git a/src/bun_core/util.rs b/src/bun_core/util.rs index 1e90288ed229..ce3b7192a9ce 100644 --- a/src/bun_core/util.rs +++ b/src/bun_core/util.rs @@ -742,6 +742,12 @@ impl PathBuffer { /// the leak/stress tests. Leave the bytes uninit. #[inline] #[allow(invalid_value, clippy::uninit_assumed_init)] + #[allow( + clippy::large_stack_frames, + reason = "the frame is two `MAX_PATH_BYTES` locals (the `MaybeUninit` temp and the return \ + slot), ~196 KB on Windows; neither is written, so optimized builds emit nothing, \ + and the caller is committing to a `MAX_PATH_BYTES` local either way" + )] pub fn uninit() -> Self { // SAFETY: `PathBuffer` is `repr(transparent)` over `[u8; N]`; every bit // pattern is a valid `u8`, and callers treat this as a write-only @@ -1348,8 +1354,10 @@ pub unsafe fn fd_path_raw(fd: Fd, buf: *mut u8, cap: usize) -> isize { 0 }; } + // SAFETY: `kif` is a live local; `addr_of!` only forms the field's + // address, it reads nothing. + let path = unsafe { addr_of!((*kif.as_ptr()).kf_path) }.cast::(); // SAFETY: kernel wrote a NUL-terminated path into kf_path. - let path = unsafe { addr_of!((*kif.as_ptr()).kf_path) } as *const u8; let len = unsafe { libc::strlen(path.cast()) }; let n = len.min(cap); // SAFETY: path has `len` initialized bytes; buf has `cap` bytes. @@ -1548,9 +1556,9 @@ pub mod fd { unsafe { let pp = (*crate::windows_sys::peb()).ProcessParameters; ProcessParametersStdio { - hStdInput: (*pp).hStdInput as *mut c_void, - hStdOutput: (*pp).hStdOutput as *mut c_void, - hStdError: (*pp).hStdError as *mut c_void, + hStdInput: (*pp).hStdInput, + hStdOutput: (*pp).hStdOutput, + hStdError: (*pp).hStdError, } } } @@ -2794,10 +2802,12 @@ pub fn self_exe_path() -> crate::CrateResult<&'static ZStr> { // `canonicalize()` on Windows returns a verbatim `\\?\` path; strip // that back to a plain DOS path before WTF-8 encoding (Node's // `process.execPath` is never verbatim-prefixed). - if let Some(rest) = s.strip_prefix(r"\\?\UNC\") { - s = format!(r"\\{}", rest); - } else if let Some(rest) = s.strip_prefix(r"\\?\") { - s = rest.to_owned(); + const VERBATIM_UNC: &str = r"\\?\UNC\"; + const VERBATIM: &str = r"\\?\"; + if s.starts_with(VERBATIM_UNC) { + s.replace_range(..VERBATIM_UNC.len(), r"\\"); + } else if s.starts_with(VERBATIM) { + s.drain(..VERBATIM.len()); } Ok(ZBox::from_vec_with_nul(s.into_bytes())) } @@ -3789,7 +3799,7 @@ fn argv_storage() -> &'static [ZBox] { // `CommandLineToArgvW` allocates its own array (lifetime managed // by the system — intentionally not `LocalFree`d, the // argv strings are referenced for the process lifetime). - let argvw = unsafe { CommandLineToArgvW(GetCommandLineW(), &mut argc) }; + let argvw = unsafe { CommandLineToArgvW(GetCommandLineW(), &raw mut argc) }; if !argvw.is_null() { let argc = argc.max(0) as usize; // SAFETY: `CommandLineToArgvW` returned `argc` valid `LPWSTR`s. diff --git a/test/internal/rust-clippy-cross.test.ts b/test/internal/rust-clippy-cross.test.ts new file mode 100644 index 000000000000..1c4fd6924c42 --- /dev/null +++ b/test/internal/rust-clippy-cross.test.ts @@ -0,0 +1,81 @@ +// The CI Clippy job lints the host target only, so code behind `#[cfg(windows)]` +// / `#[cfg(target_os = "freebsd")]` can fail clippy without anything noticing +// until someone runs `cargo clippy` on that OS. scripts/rust-clippy-cross.ts +// lints those targets from any host; this pins bun_core on each of them. +// bun_core is the root of the crate graph, so when it fails, `cargo clippy` +// reports nothing about any other crate on that target (the full per-target +// crate list is CI's job: `bun run rust:clippy-cross`, ~1 min per target). +// +// Skipped where the workspace is not resolvable (test-only CI lanes run a +// prebuilt binary and have neither vendor/lolhtml nor the configure output; +// same check as linear-fifo.test.ts) or where a target's std is not installed +// (`rustup target add `; rust-toolchain.toml installs them all). +import { expect, test } from "bun:test"; +import { existsSync } from "node:fs"; +import path from "node:path"; +import { CROSS_CLIPPY_TARGETS } from "../../scripts/rust-clippy-cross.ts"; + +const repoRoot = path.resolve(import.meta.dir, "..", ".."); +const cargo = Bun.which("cargo"); +const rustup = Bun.which("rustup"); +const codegenDir = process.env.BUN_CODEGEN_DIR ?? path.join(repoRoot, "build", "debug", "codegen"); +const workspaceResolvable = + existsSync(path.join(repoRoot, "vendor", "lolhtml", "Cargo.toml")) && + existsSync(path.join(codegenDir, "build_options.rs")); + +// Run from the repo root so rustup reports the toolchain rust-toolchain.toml +// pins, i.e. the one `cargo` below will use. +const installedTargets = new Set( + cargo && rustup && workspaceResolvable + ? Bun.spawnSync({ + cmd: [rustup, "target", "list", "--installed"], + cwd: repoRoot, + stdout: "pipe", + stderr: "ignore", + timeout: 30_000, + }) + .stdout.toString() + .split(/\r?\n/) + : [], +); + +for (const triple of Object.keys(CROSS_CLIPPY_TARGETS)) { + test.skipIf(!installedTargets.has(triple))( + `bun_core is clippy-clean for ${triple}`, + async () => { + await using proc = Bun.spawn({ + cmd: [ + cargo!, + "clippy", + "--locked", + "--no-deps", + "-p", + "bun_core", + "--target", + triple, + "--message-format=short", + ], + cwd: repoRoot, + env: { ...process.env, CARGO_TERM_COLOR: "never" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // `--message-format=short` prints one `file:line:col: level: message` line per diagnostic. + const diagnostics = stderr.split(/\r?\n/).filter(line => /^\S+\.rs:\d+:\d+: (error|warning)/.test(line)); + if (exitCode !== 0 && diagnostics.length === 0) { + // Failed for a non-lint reason (build script, dependency), which the assertions below cannot show. + console.error(stderr || stdout); + } + expect(diagnostics).toEqual([]); + expect(exitCode).toBe(0); + }, + // Fully cached this is ~150ms per target, but any change to bun_core or to + // build_options.rs (its SHA constant changes with every commit) re-lints + // the crate (~3s), and a cold target dir first checks its ~40 dependencies + // for the triple (~10s): past the default per-test timeout. Serial on + // purpose: concurrent cargo invocations just block on each other's + // build-directory lock. + 120_000, + ); +} From 650e8edb5dbd95e902c7ac4d1de443a8b1705eb1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 06:30:38 +0000 Subject: [PATCH 2/5] rust-clippy-cross: per-crate hit budgets instead of crate excludes, five targets The exclude list left the 22 crates with existing hits entirely unlinted (most of the cfg(windows) code in the tree) and could only be tightened by hand. The script now lints the whole workspace per target with lints capped to warnings, counts the diagnostics per crate and compares them with scripts/rust-clippy-cross-budgets.json: over budget fails, under budget fails until `--update` lowers the entry, unlisted crates are budgeted at zero. Linting every crate in one invocation is also what keeps the numbers honest: with --no-deps, a crate compiled as a dependency of a `-p` selection is not linted and cargo reuses that artifact when the crate is selected later, so the `-p` mode is gone and the test builds into its own target dir. Targets: the two with hits (windows, freebsd) plus musl (1 hit), aarch64-unknown-linux-gnu (the only CI triple where c_char is u8) and aarch64-apple-darwin (the cfg(apple) and cfg(target_arch = "aarch64") code), which are at zero today and stay there. aarch64-linux-android is left out: clippy's thread_local_initializer_can_be_made_const fires on 42 initializers that already are `const { .. }` on that target. --- .github/workflows/clippy.yml | 12 +- CLAUDE.md | 2 +- scripts/rust-clippy-cross-budgets.json | 39 +++++ scripts/rust-clippy-cross.ts | 199 +++++++++++++++++------- test/internal/rust-clippy-cross.test.ts | 39 +++-- 5 files changed, 215 insertions(+), 76 deletions(-) create mode 100644 scripts/rust-clippy-cross-budgets.json diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 73c4f9a51395..12d5159f5562 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -14,6 +14,7 @@ on: - "scripts/build/**" - "scripts/build.ts" - "scripts/rust-clippy-cross.ts" + - "scripts/rust-clippy-cross-budgets.json" - "Cargo.toml" - "Cargo.lock" - "clippy.toml" @@ -32,9 +33,9 @@ env: # rust-toolchain.toml. RUSTUP_TOOLCHAIN: nightly-2026-07-20 # The triples `rust:clippy-cross` lints (clippy needs their std, nothing - # else). Keep in sync with CROSS_CLIPPY_TARGETS in scripts/rust-clippy-cross.ts; + # else). Keep in sync with the keys of scripts/rust-clippy-cross-budgets.json; # if it drifts, cargo fails naming the triple that is missing. - CROSS_CLIPPY_TARGETS: x86_64-pc-windows-msvc,x86_64-unknown-freebsd + CROSS_CLIPPY_TARGETS: x86_64-pc-windows-msvc,x86_64-unknown-freebsd,x86_64-unknown-linux-musl,aarch64-unknown-linux-gnu,aarch64-apple-darwin jobs: clippy: @@ -87,9 +88,10 @@ jobs: run: bun run rust:clippy - name: cargo clippy (cross targets) - # `cfg(windows)` / `cfg(target_os = "freebsd")` code is invisible to - # the host run above. Runs even when that one failed so a PR gets both - # sets of diagnostics at once. + # Code gated on another OS / arch / libc is invisible to the host run + # above; this lints it per target against the per-crate budgets in + # scripts/rust-clippy-cross-budgets.json. Runs even when the host run + # failed so a PR gets both sets of diagnostics at once. if: ${{ !cancelled() && steps.codegen.outcome == 'success' }} env: BUN_CODEGEN_DIR: ${{ github.workspace }}/build/debug/codegen diff --git a/CLAUDE.md b/CLAUDE.md index 49fc1d3c5392..8981f513bca5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -198,7 +198,7 @@ Several situational sections live in `.claude/docs/landing-prs.md` — read the 6. **Use absolute paths** - Always use absolute paths in file operations 7. **Avoid shell commands** - Don't use `find` or `grep` in tests; use Bun's Glob and built-in tools 8. **Memory management** - Prefer RAII (`Drop`) over manual cleanup. Arena edge case: values allocated in an arena (`Arena`/`bumpalo`) do **not** run `Drop` on arena reset — types owning a heap allocation or refcount must be freed/deref'd explicitly first, mirroring the original Zig `deinit()` order. -9. **Cross-platform** - Run `bun run rust:check-all` to compile across all targets (linux/macos/windows × x64/aarch64) when making platform-specific changes. `#[cfg(...)]`-gated code is not type-checked unless the matching target is built. Same for clippy: CI lints the Linux host plus the targets listed in `scripts/rust-clippy-cross.ts`, so also run `bun run rust:clippy-cross` when touching `cfg(windows)` / `cfg(target_os = ...)` code. +9. **Cross-platform** - Run `bun run rust:check-all` to compile across all targets (linux/macos/windows × x64/aarch64) when making platform-specific changes. `#[cfg(...)]`-gated code is not type-checked unless the matching target is built. Same for clippy: CI lints the Linux host plus the targets in `scripts/rust-clippy-cross-budgets.json` (per-crate budgets of remaining hits), so also run `bun run rust:clippy-cross` when touching `cfg(windows)` / `cfg(target_os = ...)` code. 10. **Debug builds** - Use `BUN_DEBUG_QUIET_LOGS=1` to disable debug logging, or `BUN_DEBUG_=1` to enable a specific `bun_core::output` scoped logger 11. **Be humble & honest** - NEVER overstate what you got done or what actually works in commits, PRs or in messages to the user. 12. **Branch names must start with `claude/`** - This is a requirement for the CI to work. diff --git a/scripts/rust-clippy-cross-budgets.json b/scripts/rust-clippy-cross-budgets.json new file mode 100644 index 000000000000..ef6331e5cbd7 --- /dev/null +++ b/scripts/rust-clippy-cross-budgets.json @@ -0,0 +1,39 @@ +{ + "x86_64-pc-windows-msvc": { + "bun_bundler": 5, + "bun_bunfig": 1, + "bun_cares_sys": 2, + "bun_crash_handler": 9, + "bun_install": 93, + "bun_io": 30, + "bun_jsc": 5, + "bun_libuv_sys": 85, + "bun_md": 1, + "bun_patch": 4, + "bun_paths": 5, + "bun_resolver": 6, + "bun_router": 1, + "bun_runtime": 276, + "bun_spawn": 21, + "bun_spawn_sys": 5, + "bun_standalone_graph": 2, + "bun_sys": 130, + "bun_threading": 5, + "bun_uws_sys": 4, + "bun_watcher": 14, + "bun_which": 1 + }, + "x86_64-unknown-freebsd": { + "bun_crash_handler": 1, + "bun_glob": 1, + "bun_http": 1, + "bun_runtime": 9, + "bun_sys": 9, + "bun_threading": 1 + }, + "x86_64-unknown-linux-musl": { + "bun_crash_handler": 1 + }, + "aarch64-unknown-linux-gnu": {}, + "aarch64-apple-darwin": {} +} diff --git a/scripts/rust-clippy-cross.ts b/scripts/rust-clippy-cross.ts index cc6c0e15f570..2f76ce4ef325 100644 --- a/scripts/rust-clippy-cross.ts +++ b/scripts/rust-clippy-cross.ts @@ -2,64 +2,125 @@ /** * `cargo clippy` for targets other than the host. * - * `bun run rust:clippy` (the CI Clippy job, ubuntu) only lints the code that is + * `bun run rust:clippy` (the CI Clippy job, ubuntu) only sees the code that is * compiled for x86_64-unknown-linux-gnu; everything behind `#[cfg(windows)]`, - * `#[cfg(target_os = "freebsd")]`, ... is never seen by clippy unless someone - * runs it on that OS. This runs the same workspace lint with `--target` for - * the targets below (clippy only needs the target's prebuilt std, not a - * linker or sysroot, so it works from any host). + * `#[cfg(target_os = "freebsd")]`, `#[cfg(target_arch = "aarch64")]`, ... is + * linted only if somebody runs clippy on such a machine. This lints the whole + * workspace with `--target` for every triple in rust-clippy-cross-budgets.json + * (clippy needs the triple's `rust-std` and nothing else, so it works from any + * host) and counts the diagnostics per crate. * - * Each target lists the crates that still fail on it; they are `--exclude`d - * so every other crate stays pinned. The lists are a ratchet: delete a crate - * once it lints clean on that target, never add one (a listed-clean crate - * that starts failing is a regression in the change that broke it). An empty - * list lints the whole workspace, same as the host run. + * The JSON file is the budget: the number of hits each crate still has on that + * target (unlisted crate = 0). A crate over its budget is a regression in the + * change that added the hit; a crate under it must have its entry lowered + * (`--update` rewrites the file from the current tree) so the budget only ever + * goes down. Adding a target is adding its triple to the file with `{}` and + * running `--update`; the Clippy workflow installs the std of every listed + * triple. + * + * Lints are capped to warnings for the run (`--cap-lints=warn`), which is what + * makes one `--workspace` invocation lint every crate: under the workspace's + * deny-level lints the first failing crate would stop its dependents from + * being checked at all. Always linting the whole workspace also keeps the + * counts trustworthy: with `--no-deps`, a crate that was compiled as a + * dependency of some `-p` selection is not linted, and cargo happily reuses + * that artifact when the crate is selected later. * * Usage: - * bun run rust:clippy-cross # every target below - * bun run rust:clippy-cross -p bun_core # extra args replace the package selection + * bun run rust:clippy-cross # every target in the budget file + * bun run rust:clippy-cross x86_64-pc-windows-msvc # just these targets + * bun run rust:clippy-cross --update # rewrite the budget file * - * Needs the configure step (`bun run build --configure-only` + - * `ninja -C build/debug clone-lolhtml`) and `rustup target add ` for - * each target; cargo names the missing one if it is not installed. + * Needs the configure step first (`bun run build --configure-only` and + * `ninja -C build/debug clone-lolhtml`), plus `rustup target add `. */ -import { spawnSync } from "node:child_process"; import { existsSync } from "node:fs"; import { resolve } from "node:path"; -export const CROSS_CLIPPY_TARGETS: Record = { - // aarch64-pc-windows-msvc reports the identical set; one Windows triple is - // enough to cover `cfg(windows)`. - "x86_64-pc-windows-msvc": [ - "bun_bundler", - "bun_bunfig", - "bun_cares_sys", - "bun_crash_handler", - "bun_install", - "bun_io", - "bun_jsc", - "bun_libuv_sys", - "bun_md", - "bun_patch", - "bun_paths", - "bun_resolver", - "bun_router", - "bun_runtime", - "bun_spawn", - "bun_spawn_sys", - "bun_standalone_graph", - "bun_sys", - "bun_threading", - "bun_uws_sys", - "bun_watcher", - "bun_which", - ], - "x86_64-unknown-freebsd": ["bun_crash_handler", "bun_glob", "bun_http", "bun_runtime", "bun_sys", "bun_threading"], -}; +const repo = resolve(import.meta.dirname, ".."); +const budgetsPath = resolve(import.meta.dirname, "rust-clippy-cross-budgets.json"); + +type Budgets = Record>; + +/** One `cargo --message-format=json` line; only the fields used here. */ +interface CargoMessage { + reason: string; + package_id?: string; + message?: { level: string; code: unknown; rendered: string }; +} + +/** Package name from a cargo package id: `path+file:///x/src/sys#bun_sys@0.0.0` or, when the directory is named like the package, `path+file:///x/src/bun_core#0.0.0`. */ +function packageName(packageId: string): string { + const [location, fragment = ""] = packageId.split("#"); + const at = fragment.indexOf("@"); + return at !== -1 ? fragment.slice(0, at) : location.slice(location.lastIndexOf("/") + 1); +} + +/** Lints the workspace for `triple`; returns the rendered lint diagnostics per crate, or null (after printing the compiler errors) if something did not compile. */ +function lint(triple: string): Map | null { + const cmd = [ + "cargo", + "clippy", + "--workspace", + "--no-deps", + "--keep-going", + "--target", + triple, + "--message-format=json", + "--", + "--cap-lints=warn", + ]; + console.log(`\x1b[36m[clippy]\x1b[0m ${cmd.join(" ")}`); + const proc = Bun.spawnSync({ cmd, cwd: repo, stdout: "pipe", stderr: "inherit" }); + + // With --cap-lints=warn every lint arrives as a warning; an error is a real + // compile failure (the target is broken, which `rust:check-all` would show too). + const byCrate = new Map(); + const errors: string[] = []; + const seen = new Set(); + for (const line of proc.stdout.toString().split("\n")) { + if (!line.startsWith("{")) continue; + const msg: CargoMessage = JSON.parse(line); + if (msg.reason !== "compiler-message" || !msg.package_id || !msg.message) continue; + const { level, code, rendered } = msg.message; + if (level === "error") errors.push(rendered); + // `code` is null on the per-crate "N warnings emitted" summary. + if (level !== "warning" || code === null) continue; + const crate = packageName(msg.package_id); + // cargo emits a diagnostic once per configuration a crate is built in + // (e.g. for the target and again for the host when a build script needs it). + if (!seen.add(`${crate}\0${rendered}`)) continue; + let list = byCrate.get(crate); + if (!list) byCrate.set(crate, (list = [])); + list.push(rendered); + } + if (proc.exitCode !== 0) { + console.log(errors.join("")); + console.error(`\x1b[31m[error]\x1b[0m cargo clippy failed for ${triple} (exit ${proc.exitCode})`); + return null; + } + return byCrate; +} + +/** Compares one target's counts with its budget; returns the problems, each already formatted for the user. */ +function checkBudget(budget: Record, counts: Map): string[] { + const problems: string[] = []; + for (const crate of [...new Set([...Object.keys(budget), ...counts.keys()])].sort()) { + const allowed = budget[crate] ?? 0; + const actual = counts.get(crate) ?? 0; + if (actual > allowed) { + problems.push(`${crate}: ${actual} clippy hits, budget is ${allowed}; fix the new ones (printed above)`); + } else if (actual < allowed) { + problems.push( + `${crate}: ${actual} clippy hits, budget is ${allowed}; run \`bun run rust:clippy-cross --update\``, + ); + } + } + return problems; +} if (import.meta.main) { - const repo = resolve(import.meta.dirname, ".."); const codegenDir = process.env.BUN_CODEGEN_DIR ?? resolve(repo, "build/debug/codegen"); for (const [path, hint] of [ [resolve(codegenDir, "build_options.rs"), "bun run build --configure-only"], @@ -71,13 +132,43 @@ if (import.meta.main) { } } - const extraArgs = process.argv.slice(2); - let failed = 0; - for (const [triple, excluded] of Object.entries(CROSS_CLIPPY_TARGETS)) { - const selection = extraArgs.length > 0 ? extraArgs : ["--workspace", ...excluded.flatMap(c => ["--exclude", c])]; - const args = ["clippy", "--no-deps", "--keep-going", "--target", triple, ...selection]; - console.log(`\x1b[36m[clippy]\x1b[0m cargo ${args.join(" ")}`); - if (spawnSync("cargo", args, { stdio: "inherit", cwd: repo }).status !== 0) failed++; + const budgets: Budgets = await Bun.file(budgetsPath).json(); + const args = process.argv.slice(2); + const update = args.includes("--update"); + const triples = args.filter(a => a !== "--update"); + for (const triple of triples) { + if (!(triple in budgets)) { + console.error(`\x1b[31m[error]\x1b[0m ${triple} is not in ${budgetsPath} (add it as {} to start linting it)`); + process.exit(1); + } + } + + let failed = false; + for (const triple of triples.length > 0 ? triples : Object.keys(budgets)) { + const diagnostics = lint(triple); + if (!diagnostics) { + failed = true; + continue; + } + const counts = new Map([...diagnostics].map(([crate, list]) => [crate, list.length])); + if (update) { + budgets[triple] = Object.fromEntries([...counts].sort(([a], [b]) => (a < b ? -1 : 1))); + continue; + } + const budget = budgets[triple]; + for (const [crate, rendered] of diagnostics) { + if (rendered.length > (budget[crate] ?? 0)) console.log(rendered.join("")); + } + const problems = checkBudget(budget, counts); + for (const problem of problems) console.error(`\x1b[31m[${triple}]\x1b[0m ${problem}`); + if (problems.length > 0) failed = true; + else + console.log(`\x1b[32m[${triple}]\x1b[0m within budget (${[...counts.values()].reduce((a, b) => a + b, 0)} hits)`); + } + + if (update) { + await Bun.write(budgetsPath, JSON.stringify(budgets, null, 2) + "\n"); + console.log(`wrote ${budgetsPath}`); } - process.exit(failed > 0 ? 1 : 0); + process.exit(failed ? 1 : 0); } diff --git a/test/internal/rust-clippy-cross.test.ts b/test/internal/rust-clippy-cross.test.ts index 1c4fd6924c42..b8e3df1b8d20 100644 --- a/test/internal/rust-clippy-cross.test.ts +++ b/test/internal/rust-clippy-cross.test.ts @@ -1,10 +1,9 @@ -// The CI Clippy job lints the host target only, so code behind `#[cfg(windows)]` -// / `#[cfg(target_os = "freebsd")]` can fail clippy without anything noticing -// until someone runs `cargo clippy` on that OS. scripts/rust-clippy-cross.ts -// lints those targets from any host; this pins bun_core on each of them. -// bun_core is the root of the crate graph, so when it fails, `cargo clippy` -// reports nothing about any other crate on that target (the full per-target -// crate list is CI's job: `bun run rust:clippy-cross`, ~1 min per target). +// The CI Clippy job lints the host target, and scripts/rust-clippy-cross.ts +// lints the targets in scripts/rust-clippy-cross-budgets.json, where every +// crate has a budget of remaining hits. This pins the one crate that is not +// allowed any on any of those targets: bun_core is the root of the crate +// graph, so a hit in its cfg-gated code is what used to make `cargo clippy` +// on a Windows host fail before reporting anything about any other crate. // // Skipped where the workspace is not resolvable (test-only CI lanes run a // prebuilt binary and have neither vendor/lolhtml nor the configure output; @@ -13,9 +12,11 @@ import { expect, test } from "bun:test"; import { existsSync } from "node:fs"; import path from "node:path"; -import { CROSS_CLIPPY_TARGETS } from "../../scripts/rust-clippy-cross.ts"; const repoRoot = path.resolve(import.meta.dir, "..", ".."); +const budgets: Record = await Bun.file( + path.join(repoRoot, "scripts", "rust-clippy-cross-budgets.json"), +).json(); const cargo = Bun.which("cargo"); const rustup = Bun.which("rustup"); const codegenDir = process.env.BUN_CODEGEN_DIR ?? path.join(repoRoot, "build", "debug", "codegen"); @@ -39,7 +40,13 @@ const installedTargets = new Set( : [], ); -for (const triple of Object.keys(CROSS_CLIPPY_TARGETS)) { +// Own target dir: with `--no-deps`, cargo reuses a bun_core that some earlier +// `cargo clippy -p ` compiled as an unlinted dependency, and would +// report it clean here without looking at it. Nothing else builds into this +// directory, so bun_core is always linted when it is (re)built here. +const targetDir = path.join(process.env.CARGO_TARGET_DIR ?? path.join(repoRoot, "target"), "rust-clippy-cross-test"); + +for (const triple of Object.keys(budgets)) { test.skipIf(!installedTargets.has(triple))( `bun_core is clippy-clean for ${triple}`, async () => { @@ -56,11 +63,12 @@ for (const triple of Object.keys(CROSS_CLIPPY_TARGETS)) { "--message-format=short", ], cwd: repoRoot, - env: { ...process.env, CARGO_TERM_COLOR: "never" }, + env: { ...process.env, CARGO_TARGET_DIR: targetDir, CARGO_TERM_COLOR: "never" }, stdout: "pipe", stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // `--message-format=short` prints one `file:line:col: level: message` line per diagnostic. const diagnostics = stderr.split(/\r?\n/).filter(line => /^\S+\.rs:\d+:\d+: (error|warning)/.test(line)); if (exitCode !== 0 && diagnostics.length === 0) { @@ -70,12 +78,11 @@ for (const triple of Object.keys(CROSS_CLIPPY_TARGETS)) { expect(diagnostics).toEqual([]); expect(exitCode).toBe(0); }, - // Fully cached this is ~150ms per target, but any change to bun_core or to - // build_options.rs (its SHA constant changes with every commit) re-lints - // the crate (~3s), and a cold target dir first checks its ~40 dependencies - // for the triple (~10s): past the default per-test timeout. Serial on - // purpose: concurrent cargo invocations just block on each other's - // build-directory lock. + // The first run per triple checks bun_core's ~40 dependencies into the + // private target dir (~10s) and any later run re-lints bun_core after a + // change to it or to build_options.rs (its SHA constant changes with every + // commit; ~3s): past the default per-test timeout. Serial on purpose: + // concurrent cargo invocations just block on the target dir's lock. 120_000, ); } From 115567c31aa8217dd833728478fb81da0670570e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 06:46:26 +0000 Subject: [PATCH 3/5] rust-clippy-cross: actually dedupe per-configuration duplicates Set.prototype.add returns the set, so the `!seen.add(key)` guard never skipped anything. No budget changes: none of the crates that are currently compiled in more than one configuration (bun_output_tags is built three times per --target run) has a hit today, but one new hit in such a crate was counted three times. --- scripts/rust-clippy-cross.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scripts/rust-clippy-cross.ts b/scripts/rust-clippy-cross.ts index 2f76ce4ef325..c70e7418f94c 100644 --- a/scripts/rust-clippy-cross.ts +++ b/scripts/rust-clippy-cross.ts @@ -88,9 +88,12 @@ function lint(triple: string): Map | null { // `code` is null on the per-crate "N warnings emitted" summary. if (level !== "warning" || code === null) continue; const crate = packageName(msg.package_id); - // cargo emits a diagnostic once per configuration a crate is built in - // (e.g. for the target and again for the host when a build script needs it). - if (!seen.add(`${crate}\0${rendered}`)) continue; + // A crate that proc-macros or build scripts also depend on is compiled for + // the host as well as for the target (bun_output_tags: three times), and + // every configuration reports the same hit again. + const key = `${crate}\0${rendered}`; + if (seen.has(key)) continue; + seen.add(key); let list = byCrate.get(crate); if (!list) byCrate.set(crate, (list = [])); list.push(rendered); From fa74b7a48da03c63b27422a76b2227e81842b50f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 07:21:06 +0000 Subject: [PATCH 4/5] rust-clippy-cross: rebase budgets onto current main CI lints the merge commit, and between this branch's base and now main gained Windows-only hits: #37490 (net +1 in bun_cares_sys, cast_ptr_alignment) and #31829 (+5 borrow_as_ptr in bun_runtime, net +4), which is exactly what the first run of the workflow reported. Regenerated on the rebased tree the budgets match that run on all five targets. The over-budget message now also names the stale-budget case. --- scripts/rust-clippy-cross-budgets.json | 4 ++-- scripts/rust-clippy-cross.ts | 17 +++++++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/scripts/rust-clippy-cross-budgets.json b/scripts/rust-clippy-cross-budgets.json index ef6331e5cbd7..0f40dd369ab4 100644 --- a/scripts/rust-clippy-cross-budgets.json +++ b/scripts/rust-clippy-cross-budgets.json @@ -2,7 +2,7 @@ "x86_64-pc-windows-msvc": { "bun_bundler": 5, "bun_bunfig": 1, - "bun_cares_sys": 2, + "bun_cares_sys": 3, "bun_crash_handler": 9, "bun_install": 93, "bun_io": 30, @@ -13,7 +13,7 @@ "bun_paths": 5, "bun_resolver": 6, "bun_router": 1, - "bun_runtime": 276, + "bun_runtime": 280, "bun_spawn": 21, "bun_spawn_sys": 5, "bun_standalone_graph": 2, diff --git a/scripts/rust-clippy-cross.ts b/scripts/rust-clippy-cross.ts index c70e7418f94c..0092d7c8124c 100644 --- a/scripts/rust-clippy-cross.ts +++ b/scripts/rust-clippy-cross.ts @@ -12,11 +12,13 @@ * * The JSON file is the budget: the number of hits each crate still has on that * target (unlisted crate = 0). A crate over its budget is a regression in the - * change that added the hit; a crate under it must have its entry lowered - * (`--update` rewrites the file from the current tree) so the budget only ever - * goes down. Adding a target is adding its triple to the file with `{}` and - * running `--update`; the Clippy workflow installs the std of every listed - * triple. + * change that added the hit, to be fixed there (raising an entry is the + * exception and needs the same justification as an `#[allow]`); a crate under + * it must have its entry lowered, which is what `--update` does (it rewrites + * the file from the current tree). The counts depend only on the sources, so + * CI's run on the merge commit agrees with a local run on the same tree. + * Adding a target is adding its triple to the file with `{}` and running + * `--update`; the Clippy workflow installs the std of every listed triple. * * Lints are capped to warnings for the run (`--cap-lints=warn`), which is what * makes one `--workspace` invocation lint every crate: under the workspace's @@ -113,7 +115,10 @@ function checkBudget(budget: Record, counts: Map const allowed = budget[crate] ?? 0; const actual = counts.get(crate) ?? 0; if (actual > allowed) { - problems.push(`${crate}: ${actual} clippy hits, budget is ${allowed}; fix the new ones (printed above)`); + problems.push( + `${crate}: ${actual} clippy hits, budget is ${allowed}; the crate's hits are printed above. ` + + "If none of the extra ones come from your change, the budget file is behind main: rebase first.", + ); } else if (actual < allowed) { problems.push( `${crate}: ${actual} clippy hits, budget is ${allowed}; run \`bun run rust:clippy-cross --update\``, From cbc78b7bc4b9b91764f407a181cd8763b5abcda3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 12 Aug 2026 08:53:22 +0000 Subject: [PATCH 5/5] rust-clippy-cross: budget per file and lint, read the triples from the budget file Keyed per crate, a regression in bun_runtime printed (and annotated) all 280 of its pre-existing hits, so the new one was not what showed up, and a hit of one lint replacing a hit of another in the same crate was invisible. The budget is now keyed by ` ` per target, the shape the dead-code ratchet already uses, so every (file, lint) pair without an entry is pinned at zero and a failure prints exactly the pair's diagnostics: with the budgets as of before #31829, the current tree reports that PR's five borrow_as_ptr sites and nothing else. Same 710 / 22 / 1 / 0 / 0 hits as before, now in 257 / 10 / 1 / 0 / 0 entries. The workflow reads the triples to install from the budget file instead of carrying its own copy of the list, which the script's header had already claimed it did. --- .github/workflows/clippy.yml | 17 +- CLAUDE.md | 2 +- scripts/rust-clippy-cross-budgets.json | 297 +++++++++++++++++++++--- scripts/rust-clippy-cross.ts | 105 +++++---- test/internal/rust-clippy-cross.test.ts | 11 +- 5 files changed, 335 insertions(+), 97 deletions(-) diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 12d5159f5562..b6dafe0baee2 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -28,14 +28,10 @@ env: BUN_VERSION: "1.3.14" LLVM_VERSION_MAJOR: "21" # Pin the toolchain explicitly so rustup ignores rust-toolchain.toml's - # `targets` list (11 cross triples ≈ 450 MB of prebuilt std); only the - # triples below get their std. Keep in sync with `channel` in - # rust-toolchain.toml. + # `targets` list (11 cross triples ≈ 450 MB of prebuilt std); the Setup Rust + # step adds the std of just the triples `rust:clippy-cross` lints. Keep in + # sync with `channel` in rust-toolchain.toml. RUSTUP_TOOLCHAIN: nightly-2026-07-20 - # The triples `rust:clippy-cross` lints (clippy needs their std, nothing - # else). Keep in sync with the keys of scripts/rust-clippy-cross-budgets.json; - # if it drifts, cargo fails naming the triple that is missing. - CROSS_CLIPPY_TARGETS: x86_64-pc-windows-msvc,x86_64-unknown-freebsd,x86_64-unknown-linux-musl,aarch64-unknown-linux-gnu,aarch64-apple-darwin jobs: clippy: @@ -66,7 +62,10 @@ jobs: - name: Setup Rust run: | - rustup toolchain install "$RUSTUP_TOOLCHAIN" --profile minimal --component clippy --target "$CROSS_CLIPPY_TARGETS" + # The budget file's keys are the triples the cross step lints; clippy + # needs their rust-std and nothing else. + cross_targets=$(bun -e 'console.log(Object.keys(await Bun.file("scripts/rust-clippy-cross-budgets.json").json()).join(","))') + rustup toolchain install "$RUSTUP_TOOLCHAIN" --profile minimal --component clippy --target "$cross_targets" rustup override set "$RUSTUP_TOOLCHAIN" # rustc/clippy diagnostics → inline PR annotations echo "::add-matcher::.github/rust-matcher.json" @@ -89,7 +88,7 @@ jobs: - name: cargo clippy (cross targets) # Code gated on another OS / arch / libc is invisible to the host run - # above; this lints it per target against the per-crate budgets in + # above; this lints it per target against the per-file budgets in # scripts/rust-clippy-cross-budgets.json. Runs even when the host run # failed so a PR gets both sets of diagnostics at once. if: ${{ !cancelled() && steps.codegen.outcome == 'success' }} diff --git a/CLAUDE.md b/CLAUDE.md index 8981f513bca5..8137e9854f94 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -198,7 +198,7 @@ Several situational sections live in `.claude/docs/landing-prs.md` — read the 6. **Use absolute paths** - Always use absolute paths in file operations 7. **Avoid shell commands** - Don't use `find` or `grep` in tests; use Bun's Glob and built-in tools 8. **Memory management** - Prefer RAII (`Drop`) over manual cleanup. Arena edge case: values allocated in an arena (`Arena`/`bumpalo`) do **not** run `Drop` on arena reset — types owning a heap allocation or refcount must be freed/deref'd explicitly first, mirroring the original Zig `deinit()` order. -9. **Cross-platform** - Run `bun run rust:check-all` to compile across all targets (linux/macos/windows × x64/aarch64) when making platform-specific changes. `#[cfg(...)]`-gated code is not type-checked unless the matching target is built. Same for clippy: CI lints the Linux host plus the targets in `scripts/rust-clippy-cross-budgets.json` (per-crate budgets of remaining hits), so also run `bun run rust:clippy-cross` when touching `cfg(windows)` / `cfg(target_os = ...)` code. +9. **Cross-platform** - Run `bun run rust:check-all` to compile across all targets (linux/macos/windows × x64/aarch64) when making platform-specific changes. `#[cfg(...)]`-gated code is not type-checked unless the matching target is built. Same for clippy: CI lints the Linux host plus the targets in `scripts/rust-clippy-cross-budgets.json` (per-file budgets of the remaining hits), so also run `bun run rust:clippy-cross` when touching `cfg(windows)` / `cfg(target_os = ...)` code. 10. **Debug builds** - Use `BUN_DEBUG_QUIET_LOGS=1` to disable debug logging, or `BUN_DEBUG_=1` to enable a specific `bun_core::output` scoped logger 11. **Be humble & honest** - NEVER overstate what you got done or what actually works in commits, PRs or in messages to the user. 12. **Branch names must start with `claude/`** - This is a requirement for the CI to work. diff --git a/scripts/rust-clippy-cross-budgets.json b/scripts/rust-clippy-cross-budgets.json index 0f40dd369ab4..fc1966f3c8ae 100644 --- a/scripts/rust-clippy-cross-budgets.json +++ b/scripts/rust-clippy-cross-budgets.json @@ -1,38 +1,277 @@ { "x86_64-pc-windows-msvc": { - "bun_bundler": 5, - "bun_bunfig": 1, - "bun_cares_sys": 3, - "bun_crash_handler": 9, - "bun_install": 93, - "bun_io": 30, - "bun_jsc": 5, - "bun_libuv_sys": 85, - "bun_md": 1, - "bun_patch": 4, - "bun_paths": 5, - "bun_resolver": 6, - "bun_router": 1, - "bun_runtime": 280, - "bun_spawn": 21, - "bun_spawn_sys": 5, - "bun_standalone_graph": 2, - "bun_sys": 130, - "bun_threading": 5, - "bun_uws_sys": 4, - "bun_watcher": 14, - "bun_which": 1 + "src/bundler/OutputFile.rs clippy::large_stack_frames": 1, + "src/bundler/entry_points.rs clippy::large_stack_frames": 1, + "src/bundler/linker_context/generateChunksInParallel.rs clippy::large_stack_frames": 1, + "src/bundler/linker_context/writeOutputFilesToDisk.rs clippy::large_stack_frames": 1, + "src/bundler/options.rs clippy::byte_char_slices": 1, + "src/bunfig/arguments.rs clippy::large_stack_frames": 1, + "src/cares_sys/c_ares.rs clippy::cast_ptr_alignment": 3, + "src/crash_handler/lib.rs clippy::borrow_as_ptr": 4, + "src/crash_handler/lib.rs clippy::ptr_as_ptr": 1, + "src/crash_handler/lib.rs clippy::undocumented_unsafe_blocks": 3, + "src/crash_handler/lib.rs clippy::write_with_newline": 1, + "src/install/PackageInstall.rs clippy::large_stack_frames": 1, + "src/install/PackageInstall.rs clippy::ref_as_ptr": 1, + "src/install/PackageInstall.rs clippy::undocumented_unsafe_blocks": 2, + "src/install/PackageInstaller.rs clippy::large_stack_frames": 3, + "src/install/PackageManager.rs clippy::large_stack_frames": 1, + "src/install/PackageManager/CommandLineArguments.rs clippy::large_stack_frames": 1, + "src/install/PackageManager/PackageManagerDirectories.rs clippy::large_stack_frames": 2, + "src/install/PackageManager/PackageManagerEnqueue.rs clippy::large_stack_frames": 1, + "src/install/PackageManager/PackageManagerOptions.rs clippy::large_stack_frames": 2, + "src/install/PackageManager/patchPackage.rs clippy::large_stack_frames": 2, + "src/install/PackageManager/security_scanner.rs clippy::borrow_as_ptr": 1, + "src/install/PackageManager/security_scanner.rs clippy::undocumented_unsafe_blocks": 1, + "src/install/PackageManager/updatePackageJSONAndInstall.rs clippy::large_stack_frames": 1, + "src/install/bin.rs clippy::large_stack_frames": 1, + "src/install/extract_tarball.rs clippy::large_stack_frames": 2, + "src/install/hoisted_install.rs clippy::large_stack_frames": 1, + "src/install/isolated_install.rs clippy::large_stack_frames": 1, + "src/install/isolated_install/Hardlinker.rs clippy::undocumented_unsafe_blocks": 1, + "src/install/lockfile.rs clippy::large_stack_frames": 2, + "src/install/lockfile/Package.rs clippy::large_stack_frames": 1, + "src/install/lockfile/Tree.rs clippy::large_stack_frames": 2, + "src/install/lockfile/bun.lock.rs clippy::large_stack_frames": 3, + "src/install/lockfile/lockfile_json_stringify_for_debugging.rs clippy::large_stack_frames": 1, + "src/install/lockfile/printer/tree_printer.rs clippy::large_stack_frames": 1, + "src/install/migration.rs clippy::large_stack_frames": 1, + "src/install/npm.rs clippy::large_stack_frames": 1, + "src/install/patch_install.rs clippy::large_stack_frames": 1, + "src/install/patch_install.rs clippy::useless_conversion": 1, + "src/install/repository.rs clippy::large_stack_frames": 1, + "src/install/resolvers/folder_resolver.rs clippy::large_stack_frames": 1, + "src/install/windows-shim/bun_shim_impl.rs clippy::borrow_as_ptr": 8, + "src/install/windows-shim/bun_shim_impl.rs clippy::cast_ptr_alignment": 6, + "src/install/windows-shim/bun_shim_impl.rs clippy::identity_op": 4, + "src/install/windows-shim/bun_shim_impl.rs clippy::int_plus_one": 1, + "src/install/windows-shim/bun_shim_impl.rs clippy::large_stack_frames": 1, + "src/install/windows-shim/bun_shim_impl.rs clippy::manual_is_multiple_of": 1, + "src/install/windows-shim/bun_shim_impl.rs clippy::map_or_identity": 1, + "src/install/windows-shim/bun_shim_impl.rs clippy::needless_pass_by_value": 3, + "src/install/windows-shim/bun_shim_impl.rs clippy::undocumented_unsafe_blocks": 26, + "src/install/yarn.rs clippy::large_stack_frames": 1, + "src/io/PipeReader.rs clippy::borrow_as_ptr": 4, + "src/io/PipeReader.rs clippy::mem_forget": 2, + "src/io/PipeReader.rs clippy::ptr_as_ptr": 1, + "src/io/PipeReader.rs clippy::question_mark": 1, + "src/io/PipeReader.rs clippy::ref_as_ptr": 3, + "src/io/PipeReader.rs clippy::undocumented_unsafe_blocks": 3, + "src/io/PipeWriter.rs clippy::borrow_as_ptr": 4, + "src/io/PipeWriter.rs clippy::question_mark": 1, + "src/io/PipeWriter.rs clippy::ref_as_ptr": 3, + "src/io/lib.rs clippy::borrow_as_ptr": 1, + "src/io/lib.rs clippy::not_unsafe_ptr_arg_deref": 2, + "src/io/lib.rs clippy::undocumented_unsafe_blocks": 1, + "src/io/source.rs clippy::box_default": 1, + "src/io/source.rs clippy::ref_as_ptr": 1, + "src/io/windows_event_loop.rs clippy::undocumented_unsafe_blocks": 1, + "src/jsc/BunCPUProfiler.rs clippy::borrow_as_ptr": 1, + "src/jsc/NodeCompileCache.rs clippy::large_stack_frames": 1, + "src/jsc/VirtualMachine.rs clippy::large_stack_frames": 2, + "src/jsc/array_buffer.rs clippy::unnecessary_min_or_max": 1, + "src/libuv_sys/libuv.rs clippy::borrow_as_ptr": 1, + "src/libuv_sys/libuv.rs clippy::disallowed_macros": 14, + "src/libuv_sys/libuv.rs clippy::disallowed_methods": 1, + "src/libuv_sys/libuv.rs clippy::not_unsafe_ptr_arg_deref": 12, + "src/libuv_sys/libuv.rs clippy::ref_as_ptr": 9, + "src/libuv_sys/libuv.rs clippy::undocumented_unsafe_blocks": 47, + "src/libuv_sys/libuv.rs clippy::while_immutable_condition": 1, + "src/libuv_sys/open_handles.rs clippy::disallowed_types": 3, + "src/md/ansi_renderer.rs clippy::large_stack_frames": 1, + "src/patch/lib.rs clippy::large_stack_frames": 2, + "src/patch/lib.rs clippy::needless_borrow": 1, + "src/patch/lib.rs clippy::question_mark": 1, + "src/paths/Path.rs clippy::manual_is_ascii_check": 2, + "src/paths/resolve_path.rs clippy::int_plus_one": 2, + "src/paths/resolve_path.rs clippy::large_stack_frames": 1, + "src/resolver/lib.rs clippy::borrow_as_ptr": 1, + "src/resolver/lib.rs clippy::clone_on_copy": 1, + "src/resolver/lib.rs clippy::large_stack_frames": 3, + "src/resolver/package_json.rs clippy::large_stack_frames": 1, + "src/router/lib.rs clippy::large_stack_frames": 1, + "src/runtime/api/Archive.rs clippy::large_stack_frames": 1, + "src/runtime/api/bun/Terminal.rs clippy::borrow_as_ptr": 1, + "src/runtime/api/bun/Terminal.rs clippy::undocumented_unsafe_blocks": 1, + "src/runtime/api/bun/js_bun_spawn_bindings.rs clippy::undocumented_unsafe_blocks": 1, + "src/runtime/api/bun/spawn/stdio.rs clippy::match_like_matches_macro": 1, + "src/runtime/api/bun/subprocess/Writable.rs clippy::ref_as_ptr": 2, + "src/runtime/api/cron.rs clippy::manual_c_str_literals": 10, + "src/runtime/api/cron.rs clippy::undocumented_unsafe_blocks": 2, + "src/runtime/api/filesystem_router.rs clippy::large_stack_frames": 1, + "src/runtime/api/glob.rs clippy::large_stack_frames": 1, + "src/runtime/bake/DevServer.rs clippy::needless_borrows_for_generic_args": 1, + "src/runtime/bake/FrameworkRouter.rs clippy::large_stack_frames": 1, + "src/runtime/bake/production.rs clippy::large_stack_frames": 1, + "src/runtime/cli/Arguments.rs clippy::large_stack_frames": 1, + "src/runtime/cli/build_command.rs clippy::large_stack_frames": 1, + "src/runtime/cli/bunx_command.rs clippy::borrow_as_ptr": 4, + "src/runtime/cli/bunx_command.rs clippy::large_stack_frames": 1, + "src/runtime/cli/bunx_command.rs clippy::ref_as_ptr": 1, + "src/runtime/cli/create_command.rs clippy::large_stack_frames": 1, + "src/runtime/cli/link_command.rs clippy::large_stack_frames": 1, + "src/runtime/cli/open.rs clippy::large_stack_frames": 1, + "src/runtime/cli/pack_command.rs clippy::large_stack_frames": 1, + "src/runtime/cli/package_manager_command.rs clippy::large_stack_frames": 1, + "src/runtime/cli/pm_trusted_command.rs clippy::large_stack_frames": 2, + "src/runtime/cli/pm_view_command.rs clippy::large_stack_frames": 1, + "src/runtime/cli/run_command.rs clippy::borrow_as_ptr": 1, + "src/runtime/cli/run_command.rs clippy::large_stack_frames": 6, + "src/runtime/cli/run_command.rs clippy::redundant_slicing": 1, + "src/runtime/cli/run_command.rs clippy::undocumented_unsafe_blocks": 1, + "src/runtime/cli/test/ChangedFilesFilter.rs clippy::large_stack_frames": 1, + "src/runtime/cli/test/Scanner.rs clippy::large_stack_frames": 1, + "src/runtime/cli/test/parallel/Coordinator.rs clippy::borrow_as_ptr": 1, + "src/runtime/cli/test/parallel/aggregate.rs clippy::large_stack_frames": 1, + "src/runtime/cli/test_command.rs clippy::large_stack_frames": 2, + "src/runtime/cli/test_command.rs clippy::question_mark": 1, + "src/runtime/cli/unlink_command.rs clippy::large_stack_frames": 1, + "src/runtime/cli/update_interactive_command.rs clippy::borrow_as_ptr": 1, + "src/runtime/cli/upgrade_command.rs clippy::borrow_as_ptr": 4, + "src/runtime/cli/upgrade_command.rs clippy::large_stack_frames": 2, + "src/runtime/dns_jsc/dns.rs clippy::borrow_as_ptr": 5, + "src/runtime/dns_jsc/dns.rs clippy::boxed_local": 1, + "src/runtime/dns_jsc/dns.rs clippy::cast_ptr_alignment": 2, + "src/runtime/dns_jsc/dns.rs clippy::large_enum_variant": 1, + "src/runtime/dns_jsc/dns.rs clippy::needless_pass_by_value": 1, + "src/runtime/dns_jsc/dns.rs clippy::ref_as_ptr": 1, + "src/runtime/dns_jsc/dns.rs clippy::undocumented_unsafe_blocks": 5, + "src/runtime/image/Image.rs clippy::unnecessary_min_or_max": 1, + "src/runtime/image/Image.rs clippy::useless_conversion": 1, + "src/runtime/image/backend_wic.rs clippy::borrow_as_ptr": 13, + "src/runtime/image/backend_wic.rs clippy::missing_transmute_annotations": 1, + "src/runtime/image/backend_wic.rs clippy::ptr_as_ptr": 4, + "src/runtime/image/backend_wic.rs clippy::undocumented_unsafe_blocks": 19, + "src/runtime/ipc.rs clippy::borrow_as_ptr": 1, + "src/runtime/ipc.rs clippy::mut_from_ref": 1, + "src/runtime/ipc.rs clippy::needless_borrow": 1, + "src/runtime/ipc.rs clippy::undocumented_unsafe_blocks": 4, + "src/runtime/ipc_host.rs clippy::disallowed_methods": 1, + "src/runtime/ipc_host.rs clippy::needless_borrows_for_generic_args": 1, + "src/runtime/ipc_host.rs clippy::redundant_pattern_matching": 1, + "src/runtime/jsc_hooks.rs clippy::large_stack_frames": 1, + "src/runtime/jsc_hooks.rs clippy::undocumented_unsafe_blocks": 2, + "src/runtime/node/dir_iterator.rs clippy::borrow_as_ptr": 2, + "src/runtime/node/dir_iterator.rs clippy::ptr_as_ptr": 3, + "src/runtime/node/dir_iterator.rs clippy::ptr_cast_constness": 1, + "src/runtime/node/memory_pressure.rs clippy::needless_pass_by_value": 1, + "src/runtime/node/node_cluster_binding.rs clippy::borrow_as_ptr": 4, + "src/runtime/node/node_fs.rs clippy::borrow_as_ptr": 20, + "src/runtime/node/node_fs.rs clippy::large_stack_frames": 4, + "src/runtime/node/node_fs.rs clippy::ptr_as_ptr": 8, + "src/runtime/node/node_fs.rs clippy::question_mark": 1, + "src/runtime/node/node_fs.rs clippy::redundant_pattern_matching": 1, + "src/runtime/node/node_fs.rs clippy::undocumented_unsafe_blocks": 16, + "src/runtime/node/node_fs_binding.rs clippy::large_stack_frames": 3, + "src/runtime/node/node_fs_watcher.rs clippy::mut_from_ref": 1, + "src/runtime/node/node_fs_watcher.rs clippy::ptr_as_ptr": 1, + "src/runtime/node/node_os.rs clippy::borrow_as_ptr": 9, + "src/runtime/node/node_os.rs clippy::undocumented_unsafe_blocks": 2, + "src/runtime/node/path.rs clippy::large_stack_frames": 1, + "src/runtime/node/win_watcher.rs clippy::iter_cloned_collect": 3, + "src/runtime/node/win_watcher.rs clippy::question_mark": 1, + "src/runtime/node/win_watcher.rs clippy::undocumented_unsafe_blocks": 1, + "src/runtime/server/DirectoryRoute.rs clippy::unnecessary_min_or_max": 1, + "src/runtime/server/DirectoryRoute.rs clippy::useless_conversion": 1, + "src/runtime/server/FileRoute.rs clippy::unnecessary_min_or_max": 1, + "src/runtime/server/FileRoute.rs clippy::useless_conversion": 1, + "src/runtime/server/RequestContext.rs clippy::unnecessary_min_or_max": 1, + "src/runtime/shell/EnvMap.rs clippy::default_constructed_unit_structs": 1, + "src/runtime/shell/IOWriter.rs clippy::ptr_cast_constness": 2, + "src/runtime/shell/builtin/cp.rs clippy::large_enum_variant": 1, + "src/runtime/shell/builtin/cp.rs clippy::large_stack_frames": 2, + "src/runtime/shell/builtin/cp.rs clippy::undocumented_unsafe_blocks": 1, + "src/runtime/shell/builtin/cp.rs clippy::unnecessary_map_or": 4, + "src/runtime/shell/interpreter.rs clippy::if_same_then_else": 1, + "src/runtime/shell/subproc.rs clippy::undocumented_unsafe_blocks": 1, + "src/runtime/socket/Listener.rs clippy::large_stack_frames": 1, + "src/runtime/socket/Listener.rs clippy::mem_replace_option_with_none": 1, + "src/runtime/socket/Listener.rs clippy::redundant_clone": 4, + "src/runtime/socket/WindowsNamedPipe.rs clippy::undocumented_unsafe_blocks": 2, + "src/runtime/timer/mod.rs clippy::borrow_as_ptr": 1, + "src/runtime/webcore/Blob.rs clippy::large_stack_frames": 1, + "src/runtime/webcore/Blob.rs clippy::ptr_as_ptr": 2, + "src/runtime/webcore/Blob.rs clippy::undocumented_unsafe_blocks": 5, + "src/runtime/webcore/Blob.rs clippy::unnecessary_min_or_max": 2, + "src/runtime/webcore/FileSink.rs clippy::borrow_as_ptr": 1, + "src/runtime/webcore/FileSink.rs clippy::ptr_as_ptr": 1, + "src/runtime/webcore/blob/copy_file.rs clippy::borrow_as_ptr": 5, + "src/runtime/webcore/blob/copy_file.rs clippy::large_stack_frames": 1, + "src/runtime/webcore/blob/copy_file.rs clippy::manual_ok_err": 1, + "src/runtime/webcore/blob/copy_file.rs clippy::needless_pass_by_value": 1, + "src/runtime/webcore/blob/copy_file.rs clippy::ptr_cast_constness": 2, + "src/runtime/webcore/blob/copy_file.rs clippy::ref_as_ptr": 3, + "src/runtime/webcore/blob/copy_file.rs clippy::useless_conversion": 1, + "src/runtime/webcore/blob/read_file.rs clippy::borrow_as_ptr": 2, + "src/runtime/webcore/blob/read_file.rs clippy::needless_pass_by_value": 1, + "src/runtime/webcore/blob/read_file.rs clippy::not_unsafe_ptr_arg_deref": 1, + "src/runtime/webcore/blob/write_file.rs clippy::borrow_as_ptr": 4, + "src/runtime/webcore/blob/write_file.rs clippy::manual_ok_err": 1, + "src/runtime/webcore/blob/write_file.rs clippy::needless_pass_by_value": 1, + "src/runtime/webcore/blob/write_file.rs clippy::ref_as_ptr": 1, + "src/runtime/webcore/fetch.rs clippy::large_stack_frames": 1, + "src/spawn/lib.rs clippy::borrow_deref_ref": 1, + "src/spawn/lib.rs clippy::disallowed_types": 1, + "src/spawn/process.rs clippy::borrow_as_ptr": 4, + "src/spawn/process.rs clippy::clone_on_copy": 1, + "src/spawn/process.rs clippy::derivable_impls": 1, + "src/spawn/process.rs clippy::if_same_then_else": 1, + "src/spawn/process.rs clippy::large_enum_variant": 1, + "src/spawn/process.rs clippy::not_unsafe_ptr_arg_deref": 4, + "src/spawn/process.rs clippy::ptr_as_ptr": 2, + "src/spawn/process.rs clippy::undocumented_unsafe_blocks": 1, + "src/spawn/process.rs clippy::unnecessary_mut_passed": 1, + "src/spawn/process.rs clippy::while_immutable_condition": 1, + "src/spawn_sys/spawn_process.rs clippy::borrow_as_ptr": 5, + "src/standalone_graph/StandaloneModuleGraph.rs clippy::large_stack_frames": 2, + "src/sys/dir.rs clippy::large_stack_frames": 1, + "src/sys/dir.rs clippy::useless_conversion": 1, + "src/sys/fd.rs clippy::borrow_as_ptr": 1, + "src/sys/fd.rs clippy::manual_map": 1, + "src/sys/lib.rs clippy::borrow_as_ptr": 32, + "src/sys/lib.rs clippy::large_stack_frames": 8, + "src/sys/lib.rs clippy::ref_as_ptr": 1, + "src/sys/lib.rs clippy::undocumented_unsafe_blocks": 11, + "src/sys/sys_uv.rs clippy::borrow_as_ptr": 22, + "src/sys/sys_uv.rs clippy::ptr_cast_constness": 1, + "src/sys/sys_uv.rs clippy::question_mark": 4, + "src/sys/sys_uv.rs clippy::ref_as_ptr": 1, + "src/sys/sys_uv.rs clippy::undocumented_unsafe_blocks": 2, + "src/sys/windows/mod.rs clippy::borrow_as_ptr": 13, + "src/sys/windows/mod.rs clippy::cast_ptr_alignment": 2, + "src/sys/windows/mod.rs clippy::derivable_impls": 1, + "src/sys/windows/mod.rs clippy::needless_pass_by_value": 2, + "src/sys/windows/mod.rs clippy::not_unsafe_ptr_arg_deref": 8, + "src/sys/windows/mod.rs clippy::question_mark": 1, + "src/sys/windows/mod.rs clippy::useless_conversion": 17, + "src/threading/Condition.rs clippy::undocumented_unsafe_blocks": 1, + "src/threading/Futex.rs clippy::borrow_as_ptr": 2, + "src/threading/Mutex.rs clippy::undocumented_unsafe_blocks": 2, + "src/uws_sys/Loop.rs clippy::ptr_as_ptr": 1, + "src/uws_sys/Response.rs clippy::ptr_as_ptr": 1, + "src/uws_sys/lib.rs clippy::undocumented_unsafe_blocks": 2, + "src/watcher/Watcher.rs clippy::large_stack_frames": 1, + "src/watcher/WindowsWatcher.rs clippy::borrow_as_ptr": 9, + "src/watcher/WindowsWatcher.rs clippy::cast_ptr_alignment": 1, + "src/watcher/WindowsWatcher.rs clippy::large_stack_frames": 1, + "src/watcher/WindowsWatcher.rs clippy::ptr_eq": 1, + "src/watcher/WindowsWatcher.rs clippy::question_mark": 1, + "src/which/lib.rs clippy::disallowed_methods": 1 }, "x86_64-unknown-freebsd": { - "bun_crash_handler": 1, - "bun_glob": 1, - "bun_http": 1, - "bun_runtime": 9, - "bun_sys": 9, - "bun_threading": 1 + "src/crash_handler/lib.rs clippy::write_with_newline": 1, + "src/glob/GlobWalker.rs clippy::large_enum_variant": 1, + "src/http/SendFile.rs clippy::borrow_as_ptr": 1, + "src/runtime/node/dir_iterator.rs clippy::undocumented_unsafe_blocks": 3, + "src/runtime/node/node_fs.rs clippy::borrow_as_ptr": 2, + "src/runtime/node/node_fs.rs clippy::question_mark": 3, + "src/runtime/node/node_os.rs clippy::identity_op": 1, + "src/sys/lib.rs clippy::ptr_as_ptr": 1, + "src/sys/lib.rs clippy::undocumented_unsafe_blocks": 8, + "src/threading/Futex.rs clippy::borrow_as_ptr": 1 }, "x86_64-unknown-linux-musl": { - "bun_crash_handler": 1 + "src/crash_handler/lib.rs clippy::write_with_newline": 1 }, "aarch64-unknown-linux-gnu": {}, "aarch64-apple-darwin": {} diff --git a/scripts/rust-clippy-cross.ts b/scripts/rust-clippy-cross.ts index 0092d7c8124c..feaa50a78592 100644 --- a/scripts/rust-clippy-cross.ts +++ b/scripts/rust-clippy-cross.ts @@ -8,17 +8,20 @@ * linted only if somebody runs clippy on such a machine. This lints the whole * workspace with `--target` for every triple in rust-clippy-cross-budgets.json * (clippy needs the triple's `rust-std` and nothing else, so it works from any - * host) and counts the diagnostics per crate. + * host) and counts the diagnostics per ` `. * - * The JSON file is the budget: the number of hits each crate still has on that - * target (unlisted crate = 0). A crate over its budget is a regression in the - * change that added the hit, to be fixed there (raising an entry is the - * exception and needs the same justification as an `#[allow]`); a crate under - * it must have its entry lowered, which is what `--update` does (it rewrites - * the file from the current tree). The counts depend only on the sources, so - * CI's run on the merge commit agrees with a local run on the same tree. - * Adding a target is adding its triple to the file with `{}` and running - * `--update`; the Clippy workflow installs the std of every listed triple. + * The JSON file is the budget: per target, the hits each (file, lint) pair + * still has; anything not listed is allowed none, same shape as + * test/internal/source-lints/dead-code-escape-limits.json. A pair over its + * budget fails and prints exactly that pair's diagnostics, so the new hit is + * what shows up (and what the workflow annotates); raising an entry for it + * needs the same justification as an `#[allow]`. A pair under its budget fails + * too, until `--update` rewrites the file from the current tree, so the file + * tracks the real state and only shrinks otherwise. The counts depend on the + * sources alone: CI's run on the merge commit reports the same numbers as a + * local run on the same tree. To add a target, add its triple to the file as + * `{}` and run `--update`; the workflow reads the triples to install from the + * same file. * * Lints are capped to warnings for the run (`--cap-lints=warn`), which is what * makes one `--workspace` invocation lint every crate: under the workspace's @@ -43,23 +46,21 @@ import { resolve } from "node:path"; const repo = resolve(import.meta.dirname, ".."); const budgetsPath = resolve(import.meta.dirname, "rust-clippy-cross-budgets.json"); +/** triple -> ` ` -> allowed hits */ type Budgets = Record>; /** One `cargo --message-format=json` line; only the fields used here. */ interface CargoMessage { reason: string; - package_id?: string; - message?: { level: string; code: unknown; rendered: string }; + message?: { + level: string; + code: { code: string } | null; + rendered: string; + spans: { file_name: string; is_primary: boolean }[]; + }; } -/** Package name from a cargo package id: `path+file:///x/src/sys#bun_sys@0.0.0` or, when the directory is named like the package, `path+file:///x/src/bun_core#0.0.0`. */ -function packageName(packageId: string): string { - const [location, fragment = ""] = packageId.split("#"); - const at = fragment.indexOf("@"); - return at !== -1 ? fragment.slice(0, at) : location.slice(location.lastIndexOf("/") + 1); -} - -/** Lints the workspace for `triple`; returns the rendered lint diagnostics per crate, or null (after printing the compiler errors) if something did not compile. */ +/** Lints the workspace for `triple`; returns the rendered diagnostics per ` `, or null (after printing the compiler errors) if something did not compile. */ function lint(triple: string): Map | null { const cmd = [ "cargo", @@ -78,26 +79,28 @@ function lint(triple: string): Map | null { // With --cap-lints=warn every lint arrives as a warning; an error is a real // compile failure (the target is broken, which `rust:check-all` would show too). - const byCrate = new Map(); + const hits = new Map(); const errors: string[] = []; const seen = new Set(); for (const line of proc.stdout.toString().split("\n")) { if (!line.startsWith("{")) continue; const msg: CargoMessage = JSON.parse(line); - if (msg.reason !== "compiler-message" || !msg.package_id || !msg.message) continue; - const { level, code, rendered } = msg.message; + if (msg.reason !== "compiler-message" || !msg.message) continue; + const { level, code, rendered, spans } = msg.message; if (level === "error") errors.push(rendered); // `code` is null on the per-crate "N warnings emitted" summary. if (level !== "warning" || code === null) continue; - const crate = packageName(msg.package_id); // A crate that proc-macros or build scripts also depend on is compiled for // the host as well as for the target (bun_output_tags: three times), and // every configuration reports the same hit again. - const key = `${crate}\0${rendered}`; - if (seen.has(key)) continue; - seen.add(key); - let list = byCrate.get(crate); - if (!list) byCrate.set(crate, (list = [])); + if (seen.has(rendered)) continue; + seen.add(rendered); + // rustc prints paths relative to the workspace root; Windows hosts print + // them with backslashes. + const file = (spans.find(span => span.is_primary) ?? spans[0])?.file_name.replaceAll("\\", "/") ?? "(no span)"; + const key = `${file} ${code.code}`; + let list = hits.get(key); + if (!list) hits.set(key, (list = [])); list.push(rendered); } if (proc.exitCode !== 0) { @@ -105,23 +108,21 @@ function lint(triple: string): Map | null { console.error(`\x1b[31m[error]\x1b[0m cargo clippy failed for ${triple} (exit ${proc.exitCode})`); return null; } - return byCrate; + return hits; } -/** Compares one target's counts with its budget; returns the problems, each already formatted for the user. */ -function checkBudget(budget: Record, counts: Map): string[] { +/** Prints the diagnostics of every key over its budget; returns one line per key whose count differs from the budget. */ +function checkBudget(budget: Record, hits: Map): string[] { const problems: string[] = []; - for (const crate of [...new Set([...Object.keys(budget), ...counts.keys()])].sort()) { - const allowed = budget[crate] ?? 0; - const actual = counts.get(crate) ?? 0; - if (actual > allowed) { - problems.push( - `${crate}: ${actual} clippy hits, budget is ${allowed}; the crate's hits are printed above. ` + - "If none of the extra ones come from your change, the budget file is behind main: rebase first.", - ); - } else if (actual < allowed) { + for (const key of [...new Set([...Object.keys(budget), ...hits.keys()])].sort()) { + const allowed = budget[key] ?? 0; + const rendered = hits.get(key) ?? []; + if (rendered.length > allowed) { + console.log(rendered.join("")); + problems.push(`${key}: ${rendered.length} hits, budget is ${allowed} (all ${rendered.length} are printed above)`); + } else if (rendered.length < allowed) { problems.push( - `${crate}: ${actual} clippy hits, budget is ${allowed}; run \`bun run rust:clippy-cross --update\``, + `${key}: ${rendered.length} hits, budget is ${allowed}; run \`bun run rust:clippy-cross --update\``, ); } } @@ -153,25 +154,23 @@ if (import.meta.main) { let failed = false; for (const triple of triples.length > 0 ? triples : Object.keys(budgets)) { - const diagnostics = lint(triple); - if (!diagnostics) { + const hits = lint(triple); + if (!hits) { failed = true; continue; } - const counts = new Map([...diagnostics].map(([crate, list]) => [crate, list.length])); + const total = [...hits.values()].reduce((n, list) => n + list.length, 0); if (update) { - budgets[triple] = Object.fromEntries([...counts].sort(([a], [b]) => (a < b ? -1 : 1))); + budgets[triple] = Object.fromEntries( + [...hits].sort(([a], [b]) => (a < b ? -1 : 1)).map(([key, list]) => [key, list.length]), + ); + console.log(`\x1b[36m[${triple}]\x1b[0m ${total} hits in ${hits.size} file/lint pairs`); continue; } - const budget = budgets[triple]; - for (const [crate, rendered] of diagnostics) { - if (rendered.length > (budget[crate] ?? 0)) console.log(rendered.join("")); - } - const problems = checkBudget(budget, counts); + const problems = checkBudget(budgets[triple], hits); for (const problem of problems) console.error(`\x1b[31m[${triple}]\x1b[0m ${problem}`); if (problems.length > 0) failed = true; - else - console.log(`\x1b[32m[${triple}]\x1b[0m within budget (${[...counts.values()].reduce((a, b) => a + b, 0)} hits)`); + else console.log(`\x1b[32m[${triple}]\x1b[0m within budget (${total} hits in ${hits.size} file/lint pairs)`); } if (update) { diff --git a/test/internal/rust-clippy-cross.test.ts b/test/internal/rust-clippy-cross.test.ts index b8e3df1b8d20..67bc33ea5146 100644 --- a/test/internal/rust-clippy-cross.test.ts +++ b/test/internal/rust-clippy-cross.test.ts @@ -1,9 +1,10 @@ // The CI Clippy job lints the host target, and scripts/rust-clippy-cross.ts -// lints the targets in scripts/rust-clippy-cross-budgets.json, where every -// crate has a budget of remaining hits. This pins the one crate that is not -// allowed any on any of those targets: bun_core is the root of the crate -// graph, so a hit in its cfg-gated code is what used to make `cargo clippy` -// on a Windows host fail before reporting anything about any other crate. +// lints the targets in scripts/rust-clippy-cross-budgets.json against per-file +// budgets of the remaining hits. This runs the plain deny-level lint of one +// crate for each of those targets, the way `cargo clippy -p bun_core` runs on +// such a machine: bun_core is the root of the crate graph, so a hit in its +// cfg-gated code used to make that command fail on a Windows host before it +// reported anything about any other crate. // // Skipped where the workspace is not resolvable (test-only CI lanes run a // prebuilt binary and have neither vendor/lolhtml nor the configure output;