Skip to content

all: bump {cargo,flake} deps, fix clippy lints#619

Open
faukah wants to merge 3 commits intomasterfrom
faukah/push-yryuqwzttuzy
Open

all: bump {cargo,flake} deps, fix clippy lints#619
faukah wants to merge 3 commits intomasterfrom
faukah/push-yryuqwzttuzy

Conversation

@faukah
Copy link
Copy Markdown
Contributor

@faukah faukah commented Apr 14, 2026

Sanity Checking

  • I have read and understood the contribution guidelines
  • I have updated the changelog as per my changes
  • I have tested, and self-reviewed my code
  • Style and consistency
    • I ran nix fmt to format my Nix code
    • I ran cargo fmt to format my Rust code
    • I have added appropriate documentation to new code
    • My changes are consistent with the rest of the codebase
  • Correctness
    • I ran cargo clippy and fixed any new linter warnings.
  • If new changes are particularly complex:
    • My code includes comments in particularly complex areas to explain the
      logic
    • I have documented the motive for those changes in the PR body or commit
      description.
  • Tested on platform(s):
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin

Add a 👍 reaction to pull requests you find important.

Summary by CodeRabbit

  • Refactor

    • Internal code paths and JSON handling were cleaned up for consistency and maintainability; no user-facing behavior changes.
  • Chores

    • Bumped a workspace dependency to v0.3.0.
    • Updated workspace and project lint configurations to streamline static-analysis rules.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 14, 2026

Walkthrough

Workspace Clippy config and a dependency update were applied; several small refactors replaced conditional branching with functional patterns, test modules gained explicit Clippy expect annotations, and minor formatting/lazy-evaluation tweaks were made across multiple crates.

Changes

Cohort / File(s) Summary
Workspace & Cargo
Cargo.toml
Bumped workspace dependency clap_mangen from 0.2.31 to 0.3.0 and added workspace-level Clippy lint entry struct_excessive_bools = "allow".
Crate nh-nixos args
crates/nh-nixos/src/args.rs
Removed local #[allow(clippy::struct_excessive_bools)] from the public OsRebuildArgs struct (lint moved to workspace).
Clippy expects in tests
crates/nh-core/src/checks.rs, crates/nh-core/src/installable.rs, crates/nh-core/src/util.rs
Added #[expect(...)] attributes on test modules/arms for clippy lints (expect_used, unwrap_used, panic, unreachable); reformatted some test panic messages and switched EnvGuard { ... } to Self { ... } in tests.
Functional refactors & pattern changes
crates/nh-core/src/command.rs, crates/nh-nixos/src/generations.rs, crates/nh-home/src/home.rs
Replaced conditional if let/else patterns with map_or_else(...) for strip_prefix, JSON parsing (get_closure_size), and environment fallback computation (unwrap_or_else(...)) to defer formatting/evaluation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: dependency bumps for both Cargo and Flake, plus Clippy lint fixes across multiple files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch faukah/push-yryuqwzttuzy

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread crates/nh-core/src/checks.rs Outdated
Comment thread crates/nh-core/src/util.rs Outdated
@faukah faukah force-pushed the faukah/push-yryuqwzttuzy branch 2 times, most recently from cdd0a11 to 291480a Compare April 15, 2026 15:40
@faukah faukah force-pushed the faukah/push-yryuqwzttuzy branch from 291480a to afcac1f Compare April 15, 2026 15:42
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
Cargo.toml (1)

85-85: Replace the workspace-wide Clippy allow with local annotations on specific structs.

The workspace-wide struct_excessive_bools allow at line 85 suppresses this lint across all crates. However, only four structs actually have ≥4 bool fields:

  • NixBuildPassthroughArgs (20 bool fields) in crates/nh-core/src/args.rs:50
  • CleanArgs (5 bool fields) in crates/nh-clean/src/args.rs:24
  • OsRebuildArgs (4 bool fields) in crates/nh-nixos/src/args.rs:143
  • OsRollbackArgs (4 bool fields) in crates/nh-nixos/src/args.rs:216

Use targeted #[allow(clippy::struct_excessive_bools)] on these structs instead to preserve lint signal elsewhere.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` at line 85, The workspace-wide clippy allow
`struct_excessive_bools` should be removed from Cargo.toml and replaced with
targeted annotations: add `#[allow(clippy::struct_excessive_bools)]` directly
above the struct definitions for NixBuildPassthroughArgs, CleanArgs,
OsRebuildArgs, and OsRollbackArgs so the lint is only suppressed for those four
structs while restoring the lint elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Cargo.toml`:
- Line 28: Update the clap_mangen dependency declaration to enable the env
feature so environment-backed arguments are supported: change the clap_mangen
entry in Cargo.toml to include features = ["env"] (keeping version "0.3.0");
this ensures the Main command usage of env = "NH_ELEVATION_STRATEGY" in the
interface.rs code and the man page generation in xtask/src/man.rs will correctly
handle environment variables when producing the man pages.

---

Nitpick comments:
In `@Cargo.toml`:
- Line 85: The workspace-wide clippy allow `struct_excessive_bools` should be
removed from Cargo.toml and replaced with targeted annotations: add
`#[allow(clippy::struct_excessive_bools)]` directly above the struct definitions
for NixBuildPassthroughArgs, CleanArgs, OsRebuildArgs, and OsRollbackArgs so the
lint is only suppressed for those four structs while restoring the lint
elsewhere.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7acb2157-0d33-4359-bef7-c9bab5931d0a

📥 Commits

Reviewing files that changed from the base of the PR and between 291480a and afcac1f.

📒 Files selected for processing (8)
  • Cargo.toml
  • crates/nh-core/src/checks.rs
  • crates/nh-core/src/command.rs
  • crates/nh-core/src/installable.rs
  • crates/nh-core/src/util.rs
  • crates/nh-home/src/home.rs
  • crates/nh-nixos/src/args.rs
  • crates/nh-nixos/src/generations.rs
💤 Files with no reviewable changes (1)
  • crates/nh-nixos/src/args.rs
✅ Files skipped from review due to trivial changes (4)
  • crates/nh-core/src/util.rs
  • crates/nh-home/src/home.rs
  • crates/nh-core/src/command.rs
  • crates/nh-nixos/src/generations.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/nh-core/src/checks.rs

Comment thread Cargo.toml
clap_complete = "4.5.61"
clap_complete_nushell = "4.5.10"
clap_mangen = "0.2.31"
clap_mangen = "0.3.0"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Resolved clap_mangen version in Cargo.lock =="
if [[ -f Cargo.lock ]]; then
  awk '
    $0 ~ /^name = "clap_mangen"$/ {print FNR ":" $0; in_pkg=1; next}
    in_pkg && $0 ~ /^version = / {print FNR ":" $0; in_pkg=0}
  ' Cargo.lock
else
  echo "Cargo.lock not found"
fi

echo
echo "== clap_mangen usage sites in Rust code =="
rg -n --type rust -C3 '\bclap_mangen\b|Man::new\s*\(|generate_to\s*\('

Repository: nix-community/nh

Length of output: 2501


🌐 Web query:

clap_mangen 0.3.0 release notes changelog breaking changes API

💡 Result:

clap_mangen 0.3.0 was released on 2026-03-27 as part of the clap-rs/clap monorepo. Release notes and changelog: Available in clap_mangen/CHANGELOG.md at https://github.com/clap-rs/clap/tree/master/clap_mangen. From the release commit (f0d30d9), the [0.3.0] section lists: ## [0.3.0] - 2026-03-27 ### Compatibility - env feature is now required for environment variable support This appears to be the primary change, listed under "Compatibility," indicating a breaking change for users relying on environment variable support without explicitly enabling the env feature. No other added features, fixes, or breaking API changes are mentioned in the visible diff for this release. Previous versions (e.g., 0.2.33) had changes like MSRV updates, but 0.3.0 specifically highlights the env feature requirement. API documentation: https://docs.rs/clap_mangen/0.3.0/clap_mangen/ Public API includes: - clap_mangen::Man::new(cmd: clap::Command) -> Man - Man::render(&self, writer: &mut impl roff::RoffWrite) -> Result<, roff::Error> - generate_to(path: impl AsRef, cmd: &clap::Command) -> Result<, Error> No explicit breaking API changes noted beyond the feature flag requirement. The crate depends on clap ^4.0.0 and roff ^1.0.0, with MSRV 1.85.

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Checking clap_mangen feature configuration in Cargo.toml =="
rg -A5 -B5 'clap_mangen.*=' Cargo.toml

echo
echo "== Searching for env variable usage in command definition =="
# Search for env-backed arguments in the Main command definition
fd -t rs -path '*/interface/*' | head -20 | while read f; do
  if rg -l 'enum Main|struct Main|fn command' "$f" 2>/dev/null; then
    echo "File: $f"
    rg -n 'env\(|env_var|\.env\(' "$f" -C2
  fi
done

echo
echo "== Searching broadly for .env() calls in Rust code =="
rg -n '\.env\(' --type rust

Repository: nix-community/nh

Length of output: 949


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Finding interface directory and Main definition =="
find . -name "*.rs" -path "*/interface*" | head -20

echo
echo "== Searching for Main command definition =="
rg -n 'fn command\(\)' --type rust

echo
echo "== Searching for env() calls on command setup =="
rg -n '\.env\(' --type rust | head -30

Repository: nix-community/nh

Length of output: 187


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Reading interface.rs to find Main definition and command() =="
wc -l ./crates/nh/src/interface.rs

echo
cat -n ./crates/nh/src/interface.rs | head -150

echo
echo "== Searching for env() method calls in the file =="
rg -n '\.env\(' ./crates/nh/src/interface.rs

Repository: nix-community/nh

Length of output: 3579


Enable env feature for clap_mangen 0.3.0.

Line 28 updates to 0.3.0, which requires the env feature for environment variable support. The Main command at crates/nh/src/interface.rs:46 uses env = "NH_ELEVATION_STRATEGY", so clap_mangen must be updated to:

clap_mangen = { version = "0.3.0", features = ["env"] }

Without this, the man page generation in xtask/src/man.rs:18 may not properly handle environment-backed arguments.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` at line 28, Update the clap_mangen dependency declaration to
enable the env feature so environment-backed arguments are supported: change the
clap_mangen entry in Cargo.toml to include features = ["env"] (keeping version
"0.3.0"); this ensures the Main command usage of env = "NH_ELEVATION_STRATEGY"
in the interface.rs code and the man page generation in xtask/src/man.rs will
correctly handle environment variables when producing the man pages.

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