Clippy warn uninlined_format_args & redundant_else#7873
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (5)
✅ Files skipped from review due to trivial changes (4)
📝 WalkthroughWalkthroughAdds two Clippy lints and applies workspace-wide edits: convert positional format strings to named captures, remove or document redundant ChangesClippy lint adoption and code quality improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/stdlib/src/unicodedata.rs (1)
111-118:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDead code: the
Somebranch at line 113 is unreachable.The match at lines 112-118 is redundant. If
result.downcast_ref::<PyComplex>()succeeds, the code returns early at line 109. Therefore, by the time execution reaches line 112,resultcannot be aPyComplex, and the match will always take theNonebranch. TheSome(complex_obj)arm is dead code.Remove this entire match block—the error case it's trying to handle (non-complex return from
__complex__) is already covered by the fallthrough to line 122 and beyond.🐛 Proposed fix: remove redundant match
return Ok(Some((ret.value, true))); } - - return match result.downcast_ref::<PyComplex>() { - Some(complex_obj) => Ok(Some((complex_obj.value, true))), - None => Err(vm.new_type_error(format!( - "__complex__ returned non-complex (type '{}')", - result.class().name() - ))), - }; } // `complex` does not have a `__complex__` by default, so subclasses might not either,🤖 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/unicodedata.rs` around lines 111 - 118, Remove the redundant match over result.downcast_ref::<PyComplex>() and its Some(complex_obj) arm: the code path that handles PyComplex already returns early (via the earlier downcast) so the match's Some branch is unreachable; delete the entire match block and keep the existing fallthrough/error handling after it (i.e., rely on the prior early return and the subsequent non-complex handling), ensuring references to result and c remain consistent in the enclosing function.crates/stdlib/src/binascii.rs (1)
690-694:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
rlecode_hqxexits early and truncates output on firstRUN_CHAR.The
return Ok(out_data);in theRUN_CHARescape path stops processing the rest of the buffer, so encoded output is incomplete whenever0x90appears before the end.Suggested fix
if ch == RUN_CHAR { out_data.push(RUN_CHAR); out_data.push(0); - return Ok(out_data); + idx += 1; + continue; }Also applies to: 696-709
🤖 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/binascii.rs` around lines 690 - 694, The rlecode_hqx implementation prematurely returns from the RUN_CHAR escape path, truncating the encoded output; in the rlecode_hqx function, replace the early "return Ok(out_data)" in the branch that handles RUN_CHAR (and mirror the same fix for the similar block around lines 696–709) so that after pushing RUN_CHAR and the zero sentinel you continue processing the remainder of the input instead of returning—i.e., remove the early return and let the function loop/process the remaining bytes, preserving existing behavior for run-length encoding and ensuring out_data is returned only after the whole input is processed.crates/vm/src/builtins/complex.rs (1)
97-118:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUnreachable code: the
matchon line 112 is redundant.The
downcast_ref::<PyComplex>()call on line 97 already checks ifresultis aPyComplex. If it matches, the method returns on line 109. Therefore, when execution reaches thematchon line 112, the samedowncast_refcall will always returnNone(because it would have returnedSomeon line 97 and exited early).This means:
- The
Some(complex_obj)branch on line 113 is unreachable- The
Nonebranch on line 114-117 will always execute after line 97 failsThis suggests incomplete refactoring. If the intent is to handle subclasses differently from exact types, consider using
downcast_ref_if_exactfor one of the checks. Otherwise, remove the redundantmatchand raise the TypeError directly after line 110.Proposed fix: remove redundant match and raise TypeError directly
if let Some(ret) = result.downcast_ref::<PyComplex>() { _warnings::warn( vm.ctx.exceptions.deprecation_warning, format!( "__complex__ returned non-complex (type {ret_class}). \ The ability to return an instance of a strict subclass of complex \ is deprecated, and may be removed in a future version of Python." ), 1, vm, )?; return Ok(Some((ret.value, true))); } - - return match result.downcast_ref::<PyComplex>() { - Some(complex_obj) => Ok(Some((complex_obj.value, true))), - None => Err(vm.new_type_error(format!( - "__complex__ returned non-complex (type '{}')", - result.class().name() - ))), - }; + + return Err(vm.new_type_error(format!( + "__complex__ returned non-complex (type '{}')", + result.class().name() + ))); }🤖 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/vm/src/builtins/complex.rs` around lines 97 - 118, The code currently calls result.downcast_ref::<PyComplex>() twice in the __complex__ result handling in complex.rs, making the second match unreachable; remove the redundant match and return the TypeError directly after the warning path. Specifically, inside the block that checks result.downcast_ref::<PyComplex>() and emits the deprecation warning, keep the Ok(Some((ret.value, true))) return for the exact-match case and replace the subsequent match on result.downcast_ref::<PyComplex>() with a direct Err(vm.new_type_error(...)) to raise "__complex__ returned non-complex (type '{}')" using result.class().name().
🤖 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.
Outside diff comments:
In `@crates/stdlib/src/binascii.rs`:
- Around line 690-694: The rlecode_hqx implementation prematurely returns from
the RUN_CHAR escape path, truncating the encoded output; in the rlecode_hqx
function, replace the early "return Ok(out_data)" in the branch that handles
RUN_CHAR (and mirror the same fix for the similar block around lines 696–709) so
that after pushing RUN_CHAR and the zero sentinel you continue processing the
remainder of the input instead of returning—i.e., remove the early return and
let the function loop/process the remaining bytes, preserving existing behavior
for run-length encoding and ensuring out_data is returned only after the whole
input is processed.
In `@crates/stdlib/src/unicodedata.rs`:
- Around line 111-118: Remove the redundant match over
result.downcast_ref::<PyComplex>() and its Some(complex_obj) arm: the code path
that handles PyComplex already returns early (via the earlier downcast) so the
match's Some branch is unreachable; delete the entire match block and keep the
existing fallthrough/error handling after it (i.e., rely on the prior early
return and the subsequent non-complex handling), ensuring references to result
and c remain consistent in the enclosing function.
In `@crates/vm/src/builtins/complex.rs`:
- Around line 97-118: The code currently calls
result.downcast_ref::<PyComplex>() twice in the __complex__ result handling in
complex.rs, making the second match unreachable; remove the redundant match and
return the TypeError directly after the warning path. Specifically, inside the
block that checks result.downcast_ref::<PyComplex>() and emits the deprecation
warning, keep the Ok(Some((ret.value, true))) return for the exact-match case
and replace the subsequent match on result.downcast_ref::<PyComplex>() with a
direct Err(vm.new_type_error(...)) to raise "__complex__ returned non-complex
(type '{}')" using result.class().name().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: e87f1506-2436-4019-9d84-cbd3827d5c95
📒 Files selected for processing (55)
Cargo.tomlcrates/codegen/src/compile.rscrates/codegen/src/symboltable.rscrates/common/src/cformat.rscrates/common/src/format.rscrates/compiler/src/lib.rscrates/derive-impl/src/compile_bytecode.rscrates/derive-impl/src/lib.rscrates/derive-impl/src/pyclass.rscrates/derive-impl/src/pystructseq.rscrates/derive/src/lib.rscrates/host_env/src/fileutils.rscrates/stdlib/src/_asyncio.rscrates/stdlib/src/binascii.rscrates/stdlib/src/faulthandler.rscrates/stdlib/src/math.rscrates/stdlib/src/ssl.rscrates/stdlib/src/unicodedata.rscrates/vm/src/builtins/bool.rscrates/vm/src/builtins/code.rscrates/vm/src/builtins/complex.rscrates/vm/src/builtins/descriptor.rscrates/vm/src/builtins/function.rscrates/vm/src/builtins/str.rscrates/vm/src/builtins/template.rscrates/vm/src/builtins/type.rscrates/vm/src/bytes_inner.rscrates/vm/src/dict_inner.rscrates/vm/src/exception_group.rscrates/vm/src/exceptions.rscrates/vm/src/frame.rscrates/vm/src/getpath.rscrates/vm/src/import.rscrates/vm/src/ospath.rscrates/vm/src/stdlib/_ast.rscrates/vm/src/stdlib/_ctypes.rscrates/vm/src/stdlib/_ctypes/base.rscrates/vm/src/stdlib/_ctypes/function.rscrates/vm/src/stdlib/_ctypes/pointer.rscrates/vm/src/stdlib/_ctypes/simple.rscrates/vm/src/stdlib/_ctypes/structure.rscrates/vm/src/stdlib/_ctypes/union.rscrates/vm/src/stdlib/_functools.rscrates/vm/src/stdlib/_io.rscrates/vm/src/stdlib/_signal.rscrates/vm/src/stdlib/_typing.rscrates/vm/src/stdlib/_warnings.rscrates/vm/src/stdlib/builtins.rscrates/vm/src/stdlib/gc.rscrates/vm/src/stdlib/itertools.rscrates/vm/src/stdlib/posix.rscrates/vm/src/stdlib/sys.rscrates/vm/src/types/slot.rscrates/vm/src/types/structseq.rscrates/vm/src/vm/mod.rs
Summary by CodeRabbit
Chores
Bug Fixes
Performance Improvements