Skip to content

chore(clippy): clean up accumulated Windows-only clippy warnings#10149

Open
JamBalaya56562 wants to merge 1 commit into
jdx:mainfrom
JamBalaya56562:clippy-windows-cleanup
Open

chore(clippy): clean up accumulated Windows-only clippy warnings#10149
JamBalaya56562 wants to merge 1 commit into
jdx:mainfrom
JamBalaya56562:clippy-windows-cleanup

Conversation

@JamBalaya56562
Copy link
Copy Markdown
Contributor

What

Cleans up the pre-existing cargo clippy --all-targets warnings that fire only on the Windows build (x86_64-pc-windows-msvc). They are unrelated to feature work and were burying genuinely new warnings under noise.

After this change, cargo clippy --all-targets is warning-free on Windows and cargo test --bin mise still passes (943 tests). No behavior change.

Why these aren't simple deletions

Most of the "unused" symbols are actually used on Linux/macOS (or under #[cfg(unix)] tests) and are only dead on Windows, where the relevant code paths are #[cfg]-compiled out. Deleting them would break the Unix builds, so:

  • Imports only used under #[cfg(unix)] / #[cfg(not(windows))] are guarded with the matching #[cfg(...)] (same style as the existing #[cfg(unix)] use ...PermissionsExt), not removed:

    • gem: crate::env
    • pipx: crate::env, crate::file, std::path::PathBuf
    • task_executor: file::is_executable
    • task_confirm / task tests: Config, Task, TaskConfirm, take_captured_fields
  • Dead-code items whose callers are unconditional (so the definitions can't be #[cfg]-gated without a compile error) get #[cfg_attr(windows, allow(dead_code))], mirroring the existing #[cfg_attr(not(windows), allow(dead_code))] in src/path.rs:

    • env: LINUX_GLIBC_VERSION + the non-Linux linux_glibc_version stub
    • github: get_release_with_build_revision, pick_best_build_revision
    • ruby_common: resolve_rubyinstaller_lock_info
    • sandbox: effective_deny_{read,write,net}, apply, SandboxedCommand
  • Collapsed a nested if (clippy::collapsible_if) in tool_version.rs (inside the existing #[cfg(windows)] block).

Verification (Windows, x86_64-pc-windows-msvc)

  • cargo clippy --all-targets → 0 warnings
  • cargo test --bin mise → 943 passed; 0 failed

🤖 Generated with Claude Code

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces conditional compilation attributes and import adjustments to resolve compiler warnings and improve cross-platform compatibility, particularly for Windows. The feedback suggests broadening the #[cfg_attr(windows, allow(dead_code))] attributes in src/env.rs to target all non-Linux platforms, as the affected code is unused on systems like macOS as well.

Comment thread src/env.rs
Comment thread src/env.rs
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 30, 2026

Greptile Summary

This PR suppresses accumulated Windows-only cargo clippy --all-targets warnings across 10 files with no behavior changes. All fixes are mechanical: Unix-only imports are guarded with #[cfg(unix)] or #[cfg(not(windows))], items that must remain unconditionally compiled receive #[cfg_attr(windows, allow(dead_code))], and one nested if/if let is collapsed using stable let_chains.

  • Unix-only imports in gem.rs, pipx.rs, task_executor.rs, task/mod.rs, and task_confirm.rs are wrapped in the appropriate #[cfg] attribute, consistent with the existing PermissionsExt style already used in the codebase.
  • Functions in env.rs, github.rs, ruby_common.rs, and sandbox/mod.rs that cannot be cfg-gated receive #[cfg_attr(windows, allow(dead_code))], mirroring the existing pattern in src/path.rs.
  • The #[cfg(windows)] block in tool_version.rs collapses a nested if / if let into a single if condition && let Ok(p) = ... using the now-stable let_chains feature.

Confidence Score: 5/5

Pure lint suppression with no logic changes; safe to merge.

Every change is either adding a cfg gate to an import that was already platform-specific or annotating an item with allow(dead_code) under a cfg_attr. No executable paths are altered, and the PR author confirms clippy is clean and all 943 tests pass on Windows after these changes.

No files require special attention.

Important Files Changed

Filename Overview
src/backend/gem.rs Moved use crate::env behind #[cfg(unix)] since it's only used in Unix-specific code paths.
src/backend/pipx.rs Added #[cfg(unix)] guards to env, file, and PathBuf imports; split Path/PathBuf into separate use lines to allow individual cfg gating.
src/env.rs Added #[cfg_attr(windows, allow(dead_code))] to LINUX_GLIBC_VERSION and the non-Linux stub of linux_glibc_version.
src/github.rs Added #[cfg_attr(windows, allow(dead_code))] to get_release_with_build_revision and pick_best_build_revision.
src/plugins/core/ruby_common.rs Added #[cfg_attr(windows, allow(dead_code))] to resolve_rubyinstaller_lock_info; its call site is only reachable from non-Windows compilation paths.
src/sandbox/mod.rs Applied #[cfg_attr(windows, allow(dead_code))] to effective_deny_read/write/net, apply, and SandboxedCommand.
src/task/mod.rs Split TaskConfirm import and take_captured_fields helper behind #[cfg(unix)].
src/task/task_confirm.rs Added #[cfg(unix)] to Config and Task/TaskConfirm imports within the test module.
src/task/task_executor.rs Split is_executable out of a combined file import and placed it behind #[cfg(not(windows))].
src/toolset/tool_version.rs Collapsed nested if/if let inside #[cfg(windows)] block using stable let_chains.

Reviews (3): Last reviewed commit: "chore(clippy): clean up accumulated Wind..." | Re-trigger Greptile

@JamBalaya56562 JamBalaya56562 force-pushed the clippy-windows-cleanup branch from c4e175f to e40f44b Compare May 31, 2026 03:07
@jdx jdx enabled auto-merge (squash) May 31, 2026 03:27
These warnings fire only on the Windows build, where Unix-only code paths
(sandbox enforcement, precompiled-Ruby install, glibc detection) and
cfg(unix)/cfg(not(windows)) imports are inactive. They are unrelated to
feature work and were burying genuinely new warnings.

Most "unused" imports are in fact used under #[cfg(unix)] /
#[cfg(not(windows))]; deleting them would break those builds, so they are
guarded instead (matching the existing cfg(unix) PermissionsExt pattern):
- gem: crate::env
- pipx: crate::env, crate::file, std::path::PathBuf
- task_executor: file::is_executable
- task_confirm / task tests: Config, Task, TaskConfirm, take_captured_fields

Dead-code items are used on Linux/macOS but unreachable on Windows; their
callers are unconditional so the definitions cannot be cfg-gated. Add
#[cfg_attr(windows, allow(dead_code))], mirroring the existing
#[cfg_attr(not(windows), allow(dead_code))] usage in path.rs:
- env: LINUX_GLIBC_VERSION + linux_glibc_version stub
- github: get_release_with_build_revision, pick_best_build_revision
- ruby_common: resolve_rubyinstaller_lock_info
- sandbox: effective_deny_{read,write,net}, apply, SandboxedCommand

Also collapse a nested if (clippy::collapsible_if) in tool_version.rs.

No behavior change. cargo clippy --all-targets is now warning-free and
cargo test --bin mise passes (943 tests) on Windows.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
auto-merge was automatically disabled May 31, 2026 04:18

Head branch was pushed to by a user without write access

@JamBalaya56562 JamBalaya56562 force-pushed the clippy-windows-cleanup branch from e40f44b to 78d5b02 Compare May 31, 2026 04:18
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.

1 participant