Skip to content

chore: migrate to Rust edition 2024 and adopt let-chains - #3782

Merged
KuSh merged 6 commits into
rtk-ai:developfrom
KuSh:chore/rust-edition-2024
Sep 14, 2026
Merged

KuSh merged 6 commits into
rtk-ai:developfrom
KuSh:chore/rust-edition-2024

Conversation

@KuSh

@KuSh KuSh commented Aug 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Migrates the crate from Rust edition 2021 to 2024, and adopts the readability win
the edition unlocks: let-chains.

rust-version stays at 1.91, already well above the 1.85 floor edition 2024
needs, so this costs no MSRV bump. No CI workflow changes — every job already uses
dtolnay/rust-toolchain@stable.

Typed chore: rather than feat:: there is no new user-facing capability here, and
release-please would otherwise publish "migrate to Rust edition 2024" under
Features in the changelog.

Six commits, each independently green (cargo fmt --check and
cargo clippy --all-targets verified at every one), reviewable a commit at a time:

Commit Change
chore: reformat under rustfmt style edition 2024 Pin style_edition, reformat (60 files)
chore: list command modules explicitly instead of via automod Closes a CI blind spot — see below
chore: migrate to Rust edition 2024 Edition bump + the three things it forces
refactor: adopt let-chains cargo clippy --fix
fix(hooks): don't abort init --status on an unreadable Cursor hook Two-line dead read
chore: ignore the style-edition reformat in git blame .git-blame-ignore-revs

A pre-existing bug this surfaced: cargo fmt was checking half the crate

Every ecosystem mod.rs under src/cmds/ generated its module list with
automod::dir!(). rustfmt only follows literal mod items and cannot expand macros,
so cargo fmt never reached any of those files — and neither did CI's
cargo fmt --all -- --check gate.

Reproduce on develop by appending this to any file under src/cmds/:

fn   __probe( )->u8{let    x=1;x}

cargo fmt --all -- --check reports no diff. With an explicit pub mod, it
reports the diff and cargo fmt --all fixes it.

Module lists are now written out by hand, which drops the automod dependency. That
trades one failure mode for another, so build.rs now guards the new one: automod
compiled any stray .rs file, whereas an explicit list silently drops a file whose
pub mod line is forgotten — never compiled, never linted, tests never run, with
check-test-presence.sh still reporting PASS because it only greps the file for
#[cfg(test)]. The build now fails and names the missing declaration:

src/cmds/system/zzdemo_cmd.rs is not declared in src/cmds/system/mod.rs.
Add `pub mod zzdemo_cmd;` there, keeping the list alphabetical.
An undeclared module is never compiled, linted, or tested.

Declaring the modules explicitly also exposed clippy::module_inception on
cmds::git::git, which clippy skips inside macro-generated code — so automod had
been suppressing two separate tools. src/cmds/git/git.rs is renamed to
git_cmd.rs, resolving the lint rather than allowing it and matching the convention
its siblings already follow (diff_cmd, gh_cmd, glab_cmd, gt_cmd). Git records
it as a rename. Docs pointing at the module are updated; sample command output that
merely shows a git.rs path is left alone.

What the edition forced

1. std::env::{set_var,remove_var} are unsafe in 2024, with no safe std
replacement. Wrapping the test call sites in unsafe would collide with
unsafe_code = "deny" and the unsafe-block rule in .semgrep.yml, so they route
through temp-env, a dev-only dependency
whose closure API is safe.

Beyond avoiding unsafe, this is a correctness improvement: temp-env restores the
previous value even when the body panics, which the hand-rolled helpers did not.
CLAUDE_DIR_LOCK and PI_DIR_LOCK existed only to serialise env mutation and are
now redundant; CWD_LOCK stays (it guards the working directory) and tracking.rs
keeps ENV_LOCK, which orders more than one test.

2. unsafe_op_in_unsafe_fn is deny-by-default, so the libc calls in the proxy
signal handler need explicit blocks. This is the only new unsafe in the
migration
, scoped to the libc:: calls themselves with no safe code inside, and
carrying the repo's existing // nosemgrep: unsafe-block annotation.

3. clippy::collapsible_if fires on every nested if let / if pair. With
warnings = "deny" these are build errors, not suggestions — adopting let-chains is
mandatory here, not optional polish.

// before
if let Some(cutoff_time) = cutoff {
    if let Ok(meta) = fs::metadata(file_path) {
        if let Ok(mtime) = meta.modified() {
            if mtime < cutoff_time { continue; }
        }
    }
}
// after
if let Some(cutoff_time) = cutoff
    && let Ok(meta) = fs::metadata(file_path)
    && let Ok(mtime) = meta.modified()
    && mtime < cutoff_time
{
    continue;
}

rustfmt.toml is kept, not dropped with the style-edition pin: cargo fmt passes
--edition from Cargo.toml, but a bare rustfmt invocation — format-on-save,
rust-analyzer.rustfmt.overrideCommand, a pre-commit hook — has no crate context and
falls back to edition 2015, which cannot even parse the let-chains this PR
introduces.

Two smaller items: the two if_let_rescope sites in show_claude_config take
cargo fix --edition's match rewrite, preserving the 2021 drop timing; and edition
2024's tighter temporary scopes let clippy prove the binding in setup_test_env
inlinable, so let_and_return now fires there.

Verification

Rebased onto develop at d402152. Because this PR replaces automod::dir!() with
explicit module lists and reformats the crate, it is rebuilt on each rebase rather than
replayed — a plain rebase would carry forward stale mod.rs files and drop newly merged
command modules without any conflict marker.

Behavioural equivalence (the point of most of this work — nothing here should change
what rtk prints):

Check Result
78 commands run through the base binary and this PR's, stdout+stderr+exit code compared 0 differences
9 stdin-driven rtk pipe cases, same comparison 0 differences
scripts/benchmark.sh, both binaries, same working directory identical: 43 good / 23 warn / 0 negative / 0 fail, 2,167,209 → 151,955 (-92%)
scripts/test-all.sh, both binaries identical: 109 passed / 13 failed / 4 skipped, byte-identical failure sets (all 13 pre-existing)
scripts/test-tracking.sh, both binaries identical: 8 passed, 0 failed
Startup, hyperfine -m 200 after warmup 5.9 ms vs 5.8 ms base — within noise, under the 10 ms target

Correctness and gates:

  • cargo fmt --check and cargo clippy --all-targets clean at every one of the six
    commits
    , not just at the head
  • 3,781 tests pass, over three repeated runs and once with --test-threads=1
  • cargo test --all -- --ignored (real-process integration tests): 7 passed, 0 failed
  • semgrep scan --config .semgrep.yml --baseline-commit <base> --error (exactly what CI
    runs): 0 new findings — the // nosemgrep: unsafe-block annotations on the new
    unsafe blocks are correctly placed
  • scripts/check-test-presence.sh and scripts/validate-docs.sh: pass
  • Zero rust-2024-compatibility lints remain
  • build.rs guard re-verified: a .rs file under src/cmds/ with no pub mod line fails
    the build with exit 101, naming the missing declaration

Reproducing the suite

Everything above is scripted. BASE is the merge-base with develop; build both binaries
into directories where each is named rtk, so clap's argv[0] does not show up as a
spurious diff:

BASE=$(git merge-base HEAD upstream/develop)
cargo build --release && mkdir -p /tmp/bin-pr && cp target/release/rtk /tmp/bin-pr/
git worktree add /tmp/base "$BASE" && (cd /tmp/base && cargo build --release) \
  && mkdir -p /tmp/bin-base && cp /tmp/base/target/release/rtk /tmp/bin-base/

Per-commit gates — each of the six commits, not just the head:

for c in $(git log --format=%h "$BASE"..HEAD | tac); do
  git checkout -q $c
  echo "$c fmt=$(cargo fmt --all -- --check 2>/dev/null | grep -c '^')" \
       "clippy=$(cargo clippy --all-targets --message-format short 2>&1 | grep -cE '^(src|tests)/')"
done

Tests, including the real-process ones and a serialised run (temp-env changes how
env-touching tests interleave, so that ordering is worth checking explicitly):

cargo test --all                        # x3
cargo test --all -- --test-threads=1
cargo test --all -- --ignored

Behavioural equivalence — run any command through both binaries and compare
stdout+stderr+exit code, normalising only timings and temp paths:

run() { b=$(/tmp/bin-base/rtk "$@" 2>&1); bc=$?; p=$(/tmp/bin-pr/rtk "$@" 2>&1); pc=$?
        [ "$b" = "$p" ] && [ "$bc" = "$pc" ] || echo "DIFF: $*"; }
run git log -20; run git status; run ls -la src; run grep LazyLock .
run rewrite 'cargo test && git push'; run init --status
# plus `rtk <cmd> --help` for all 47 command surfaces, and stdin cases via `rtk pipe`

Project validation scripts. Benchmark and smoke both resolve the binary from the working
directory or PATH, so swap the binary rather than the directory to keep inputs identical:

bash scripts/check-test-presence.sh upstream/develop
bash scripts/validate-docs.sh
cp /tmp/bin-base/rtk target/release/rtk && bash scripts/benchmark.sh   # base
cp /tmp/bin-pr/rtk   target/release/rtk && bash scripts/benchmark.sh   # PR
PATH=/tmp/bin-base:$PATH bash scripts/test-all.sh
PATH=/tmp/bin-pr:$PATH   bash scripts/test-all.sh
PATH=/tmp/bin-pr:$PATH   bash scripts/test-tracking.sh

Security scan, the same invocation CI uses, and startup:

docker run --rm -v "$PWD:/src" -w /src semgrep/semgrep \
  semgrep scan --config .semgrep.yml --baseline-commit "$BASE" --error
hyperfine --warmup 20 -N -m 200 '/tmp/bin-pr/rtk git log -10' '/tmp/bin-base/rtk git log -10'

The build.rs guard, which should fail the build rather than silently skip the file:

echo 'pub fn run() {}' > src/cmds/python/zz_probe.rs
cargo build; echo "exit=$?"   # 101, naming the missing `pub mod`
rm src/cmds/python/zz_probe.rs

Not verified locally: the Windows target — cargo check --target x86_64-pc-windows-msvc
cannot run the ring / libsqlite3-sys build scripts without an MSVC C toolchain. The
Windows-gated code was audited by hand (its only unsafe is a block inside a safe fn, so
unsafe_op_in_unsafe_fn does not apply) and test (windows-latest) passes in CI.
cargo-audit is not installed locally; CI's Security Scan job covers it.

Notes for reviewers

  • temp-env is a dev-dependency, so the shipped binary is unaffected
    (cargo tree --edges normal -i temp-env finds nothing). automod is removed, so
    this is net-neutral on non-dev dependencies.
  • Edition 2024 implies resolver 3. Cargo.lock is already v4 and is unchanged
    apart from the temp-env/automod swap; resolver 3 affects future cargo update
    runs, which become MSRV-aware against rust-version = "1.91".
  • Four tail_expr_drop_order sites were reviewed and are benign (plain
    String/Result temporaries, no locks or file handles). Note the lint is
    allow-by-default after the bump, so it cannot flag them again — they were checked
    while it still could.
  • .git-blame-ignore-revs lists only the reformat. The let-chain commit touches
    many files but changes control flow and drops a crate-wide lint allow, so its
    authorship should stay visible. CONTRIBUTING now documents the one-time
    git config blame.ignoreRevsFile opt-in, since it is otherwise inert outside
    GitHub's web UI.

🤖 Generated with Claude Code

@KuSh
KuSh force-pushed the chore/rust-edition-2024 branch 4 times, most recently from 325e794 to 24f505a Compare August 31, 2026 00:18
@KuSh
KuSh force-pushed the chore/rust-edition-2024 branch 3 times, most recently from 6cdab99 to 4b9e0c2 Compare September 2, 2026 16:55
@KuSh KuSh changed the title feat: migrate to Rust edition 2024 and adopt let-chains chore: migrate to Rust edition 2024 and adopt let-chains Sep 2, 2026
@KuSh
KuSh force-pushed the chore/rust-edition-2024 branch from 4b9e0c2 to 9124395 Compare September 4, 2026 22:35
@KuSh
KuSh force-pushed the chore/rust-edition-2024 branch from 9124395 to 7ce7668 Compare September 13, 2026 01:10
@aeppling

Copy link
Copy Markdown
Contributor

Hey @KuSh

lgtm once conflict resolved and ci green

@KuSh
KuSh force-pushed the chore/rust-edition-2024 branch from 7ce7668 to 3e83aba Compare September 13, 2026 18:50
@KuSh
KuSh requested a review from aeppling September 13, 2026 18:58
KuSh and others added 6 commits September 14, 2026 01:26
Pin style_edition = "2024" in rustfmt.toml and reformat, ahead of the
edition 2024 bump. Setting style_edition explicitly decouples the reformat
from the crate edition, so this lands on edition 2021 and CI's
`cargo fmt --all -- --check` still passes.

Run through rustfmt directly rather than `cargo fmt`. Every module under
src/cmds/ is currently declared by automod::dir!, and rustfmt only follows
literal `mod` items, so `cargo fmt` cannot see those files -- the next
commit makes them reachable. Roughly two thirds of the diff is therefore in
src/cmds/, mostly the version-sorted imports style edition 2024 introduces.

No logic changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every ecosystem mod.rs under src/cmds/ generated its module list with
automod::dir!(). rustfmt only follows literal `mod` items and cannot expand
macros, so `cargo fmt` never reached any of those files -- and neither did
CI's `cargo fmt --all -- --check` gate, which has been silently passing
over half the crate.

Demonstrated by appending `fn   __probe( )->u8{let    x=1;x}` to a file
under src/cmds/: with automod, `cargo fmt --all -- --check` reports no
diff; with an explicit `pub mod`, it reports the diff and `cargo fmt --all`
fixes it. The preceding commit had to invoke rustfmt directly for the same
reason; from here `cargo fmt` covers the crate on its own.

Listing modules by hand trades one failure mode for another, so build.rs
now guards the new one: automod compiled any stray .rs file, whereas an
explicit list silently drops a file whose `pub mod` line is forgotten --
never compiled, never linted, tests never run, and check-test-presence.sh
still reporting PASS because it only greps the file for #[cfg(test)]. The
build now fails with the missing declaration named.

Declaring the modules explicitly also exposed clippy::module_inception on
cmds::git::git, which clippy skips inside macro-generated code. git.rs is
renamed to git_cmd.rs, which resolves the lint and matches the convention
its siblings already follow -- diff_cmd, gh_cmd, glab_cmd, gt_cmd. Docs
that point at the module are updated; sample command output that merely
shows a "git.rs" path is left alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bump edition 2021 -> 2024. rust-version stays at 1.91, already well above
the 1.85 floor the edition needs; docs/guide/resources/troubleshooting.md
still claimed 1.70+, which is where a failed `cargo install --git` lands.

Four things the edition forces:

- std::env::{set_var,remove_var} are unsafe in 2024 with no safe std
  replacement. Rather than wrap the test call sites in unsafe -- which the
  crate denies and .semgrep.yml flags -- route them through temp-env, a
  dev-only dependency whose closure API is safe and which restores the
  previous value even when the body panics. The hand-rolled CLAUDE_DIR_LOCK
  and PI_DIR_LOCK guards existed only to serialise those mutations and are
  now redundant; CWD_LOCK and TEST_ENV_LOCK stay, they order more than the
  env var itself.

- unsafe_op_in_unsafe_fn is on by default, so the libc calls in the proxy
  signal handler and in stream.rs's relay handler need explicit unsafe
  blocks, scoped to the libc calls themselves.

- `gen` is a reserved keyword, so the closure by that name in diff_cmd.rs
  becomes make_lines.

- Tightened tail-expression temporary scopes let clippy prove the binding in
  setup_test_env is inlinable, so let_and_return now fires there.

if_let_rescope changes when the scrutinee temporary drops in an if let/else.
The two sites in show_claude_config take cargo fix --edition's match rewrite,
which keeps the 2021 drop timing.

rustfmt.toml is kept rather than dropped: cargo fmt passes --edition from
Cargo.toml, but a bare rustfmt invocation has no crate context and falls
back to edition 2015, which cannot parse the let-chains the next commit
introduces. Pinning it there keeps format-on-save and pre-commit hooks in
agreement with CI.

clippy::collapsible_if is allowed crate-wide for now; the follow-up commit
adopts let-chains and removes the allow.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Let-chains are edition 2024 syntax, so clippy::collapsible_if now fires on
every nested `if let` / `if` pair, and with warnings = "deny" they are build
errors rather than suggestions. Applied with cargo clippy --fix, which
handles all of them in one pass, then cargo fmt to re-indent the collapsed
bodies.

The deepest cases flatten four levels of nesting into one condition:

    if let Some(cutoff_time) = cutoff
        && let Ok(meta) = fs::metadata(file_path)
        && let Ok(mtime) = meta.modified()
        && mtime < cutoff_time
    {
        continue;
    }

Drops the crate-wide clippy::collapsible_if allow added in the previous
commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
show_claude_config read the Cursor hook file only to compute _is_thin, which
nothing reads. The `?` on that read meant an unreadable hook file aborted
`rtk init --status` before it printed the rest of the report -- a status
command failing on the thing it is meant to report about. The executable-bit
check above it is what the branch actually uses.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The style-edition reformat touched 69 files without changing behaviour,
which buries the real authorship of those lines. GitHub's web blame applies
.git-blame-ignore-revs automatically; locally it needs blame.ignoreRevsFile
set, so CONTRIBUTING now says so where contributors first clone.

Only the reformat is listed. The let-chain commit also touches many files
but changes control flow and drops a crate-wide lint allow, so its
authorship is real and should stay visible in blame.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@KuSh
KuSh force-pushed the chore/rust-edition-2024 branch from 3e83aba to bf2f7c7 Compare September 13, 2026 23:34
@KuSh
KuSh merged commit 5e0f92c into rtk-ai:develop Sep 14, 2026
12 checks passed
@KuSh
KuSh deleted the chore/rust-edition-2024 branch September 14, 2026 18:21
@rtk-release-bot rtk-release-bot Bot mentioned this pull request Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants