Skip to content

fix ci#7774

Merged
youknowone merged 2 commits intoRustPython:mainfrom
ShaharNaveh:fix-ci-clippy
May 4, 2026
Merged

fix ci#7774
youknowone merged 2 commits intoRustPython:mainfrom
ShaharNaveh:fix-ci-clippy

Conversation

@ShaharNaveh
Copy link
Copy Markdown
Contributor

@ShaharNaveh ShaharNaveh commented May 4, 2026

close #7772

Summary by CodeRabbit

  • Chores
    • Added compiler attributes to error-checking functions to improve code quality and reduce potential developer oversights.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 4, 2026

📝 Walkthrough

Walkthrough

This PR adds #[must_use] compiler attributes to the check_err helper functions in both the Unix and Windows platform modules within crates/host_env/src/select.rs. The attributes enforce compile-time warnings when callers ignore the result, improving error handling discipline.

Changes

Platform Error Checking Attributes

Layer / File(s) Summary
Lint Attributes
crates/host_env/src/select.rs
#[must_use] added to Unix module's check_err(x: i32) -> bool (line 9) and Windows module's check_err(x: i32) -> bool (line 56) to encourage proper error handling.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 A rabbit hops through code so neat,
With must_use attributes, the fix is complete!
"Don't forget to check!" the compiler now cries,
No error ignored—much to my surprise! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix ci' is vague and generic, failing to convey meaningful information about the specific change (adding #[must_use] attributes to platform-specific check_err helpers). Use a more descriptive title that explains the actual change, such as 'Add #[must_use] attributes to platform-specific check_err helpers' to improve clarity for code history.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR successfully addresses the linked issue #7772 by adding #[must_use] attributes to platform-specific check_err functions to satisfy Clippy pedantic lint requirements.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing Clippy pedantic lint warnings by adding #[must_use] attributes; no out-of-scope modifications are present.
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 unit tests (beta)
  • Create PR with unit tests

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
Review rate limit: 6/8 reviews remaining, refill in 8 minutes and 59 seconds.

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

Copy link
Copy Markdown
Contributor

@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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/host_env/src/select.rs (1)

62-70: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

WASI check_err is missing the #[must_use] attribute.

The Unix (line 9) and Windows (line 56) check_err helpers both received #[must_use], but the identical function in the wasi module at line 67 was not updated. It has the same signature (pub const fn check_err(x: i32) -> bool) and the same Clippy pedantic lint (clippy::must_use_candidate) would fire there too, leaving CI broken for WASI targets.

🐛 Proposed fix
 #[cfg(target_os = "wasi")]
 pub mod platform {
     pub use libc::{FD_SETSIZE, timeval};
     pub use std::os::fd::RawFd;

+    #[must_use]
     pub const fn check_err(x: i32) -> bool {
         x < 0
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/host_env/src/select.rs` around lines 62 - 70, The WASI module's pub
const fn check_err(x: i32) -> bool is missing the #[must_use] attribute like the
Unix and Windows variants; add the #[must_use] attribute directly above the wasi
check_err function declaration to silence the clippy must_use_candidate lint and
keep behavior identical to the other platform modules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@crates/host_env/src/select.rs`:
- Around line 62-70: The WASI module's pub const fn check_err(x: i32) -> bool is
missing the #[must_use] attribute like the Unix and Windows variants; add the
#[must_use] attribute directly above the wasi check_err function declaration to
silence the clippy must_use_candidate lint and keep behavior identical to the
other platform modules.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: b9a7e731-8634-461c-be04-86979794cea3

📥 Commits

Reviewing files that changed from the base of the PR and between 83002d7 and f76be0a.

📒 Files selected for processing (1)
  • crates/host_env/src/select.rs

@youknowone youknowone merged commit 4db0aca into RustPython:main May 4, 2026
21 checks passed
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.

3 participants