Unchecked code fixes#7786
Conversation
Sorry, something went wrong.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughThis PR consolidates SSL/TLS feature configuration by gating the ChangesSSL/TLS Feature Gating & Module Configuration
OpenSSL FFI Portability to
SSL Support Code Cleanup & Warnings
VM Internal Visibility & Instrumentation Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Sorry, something went wrong.
Co-authored-by: fanninpm <27117322+fanninpm@users.noreply.github.com>
Co-authored-by: fanninpm <27117322+fanninpm@users.noreply.github.com>
Co-authored-by: fanninpm <27117322+fanninpm@users.noreply.github.com>
Co-authored-by: fanninpm <27117322+fanninpm@users.noreply.github.com>
|
The reason why I ask "Does this work?" is that |
Sorry, something went wrong.
I'm aware, let's see if that works:) |
Sorry, something went wrong.
It doesn't work. |
Sorry, something went wrong.
Right, that's because it isn't reached without passing |
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/stdlib/src/tkinter.rs (1)
83-83: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win
#[expect(dead_code)]here is brittle and can fail CI when unfulfilledGiven the prior unfulfilled-expectation failure on this PR, annotating the whole
TkAppitem with#[expect(dead_code)]is risky across feature/cfg combinations. Prefer#[allow(dead_code)]for this placeholder phase, or move lint handling to the specific unused fields/methods once identified.In Rust, when does `#[expect(dead_code)]` produce an "unfulfilled lint expectation" warning/error, especially under cfg/feature-gated code? Is `#[allow(dead_code)]` recommended for temporarily-unused structs during staged implementation?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/stdlib/src/tkinter.rs` at line 83, The #[expect(dead_code, reason = "TODO: Impl more methods")] on the TkApp item is brittle and can cause "unfulfilled lint expectation" failures across cfg/feature permutations; replace it with #[allow(dead_code)] on the TkApp declaration (or, even better, move #[allow(dead_code)] to the specific unused fields/methods inside the TkApp impl/struct) so the placeholder doesn't trigger CI failures—look for the TkApp struct/item and change the attribute accordingly.
🧹 Nitpick comments (1)
crates/stdlib/src/ssl/error.rs (1)
128-138: ⚡ Quick winScope dead-code suppression to clippy-only builds.
These unconditional
allow(dead_code)attributes can hide real dead-code drift outside clippy runs. Since this PR is clippy-focused, prefercfg_attr(clippy, ...)here.♻️ Suggested change
- #[allow(dead_code, reason = "This seems like a false positive")] + #[cfg_attr(clippy, allow(dead_code, reason = "False positive under clippy feature matrix"))] pub(crate) fn create_ssl_zero_return_error(vm: &VirtualMachine) -> PyRef<PyOSError> { @@ - #[allow(dead_code, reason = "This seems like a false positive")] + #[cfg_attr(clippy, allow(dead_code, reason = "False positive under clippy feature matrix"))] pub(crate) fn create_ssl_syscall_error(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/stdlib/src/ssl/error.rs` around lines 128 - 138, The unconditional #[allow(dead_code, reason = "...")] on functions like create_ssl_zero_return_error and create_ssl_syscall_error should be scoped to clippy runs; replace those attributes with cfg_attr(clippy, allow(dead_code, reason = "...")) so the dead-code allowance only applies when clippy is enabled, keeping normal builds able to surface true dead code. Ensure you update both function attributes to use cfg_attr(clippy, ...) and preserve the existing reason text.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/vm/src/builtins/function/jit.rs`:
- Line 14: The visibility mismatch is that get_jit_args is pub(crate) but
ArgsError is declared pub(super); fix by promoting ArgsError to pub(crate) so
its visibility matches all public signatures that return it (update the enum
declaration for ArgsError to pub(crate) and ensure related constructors like
new_jit_error and callers get_jit_arg_types and jit_ret_type continue to
compile); alternatively, if you prefer to restrict the API, change get_jit_args
(and any other pub(crate) functions returning ArgsError) to pub(super) so
signatures and ArgsError visibility align—choose one consistent visibility and
apply it to ArgsError and the affected functions.
---
Duplicate comments:
In `@crates/stdlib/src/tkinter.rs`:
- Line 83: The #[expect(dead_code, reason = "TODO: Impl more methods")] on the
TkApp item is brittle and can cause "unfulfilled lint expectation" failures
across cfg/feature permutations; replace it with #[allow(dead_code)] on the
TkApp declaration (or, even better, move #[allow(dead_code)] to the specific
unused fields/methods inside the TkApp impl/struct) so the placeholder doesn't
trigger CI failures—look for the TkApp struct/item and change the attribute
accordingly.
---
Nitpick comments:
In `@crates/stdlib/src/ssl/error.rs`:
- Around line 128-138: The unconditional #[allow(dead_code, reason = "...")] on
functions like create_ssl_zero_return_error and create_ssl_syscall_error should
be scoped to clippy runs; replace those attributes with cfg_attr(clippy,
allow(dead_code, reason = "...")) so the dead-code allowance only applies when
clippy is enabled, keeping normal builds able to surface true dead code. Ensure
you update both function attributes to use cfg_attr(clippy, ...) and preserve
the existing reason text.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: f624171e-21d5-4b0f-8ab0-da1a7efb247e
📒 Files selected for processing (10)
crates/stdlib/src/lib.rscrates/stdlib/src/openssl.rscrates/stdlib/src/openssl/cert.rscrates/stdlib/src/ssl/compat.rscrates/stdlib/src/ssl/error.rscrates/stdlib/src/tkinter.rscrates/vm/src/builtins/function/jit.rscrates/vm/src/frame.rscrates/vm/src/stdlib/_io.rscrates/vm/src/stdlib/_thread.rs
Sorry, something went wrong.
youknowone
merged commit 5ef91c2
into
RustPython:main
May 13, 2026
Reviewers
3 participants
While trying to run:
clippy --all-features --workspaceI've seen lots of errors. This PR goal is to break the fixes needed for it to run on the CI, into smaller chunksSummary by CodeRabbit