| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
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 reviews.auto_review.auto_pause_after_reviewed_commits setting. 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: ef1715ef-88bc-4c51-b7b6-7b0aae222a75 📥 CommitsReviewing files that changed from the base of the PR and between 7c9297e and 82ab35c. 📒 Files selected for processing (5)
📝 Walkthrough WalkthroughAdds two Clippy lints and applies workspace-wide edits: convert positional format strings to named captures, remove or document redundant else blocks, switch several APIs to borrow &str/&Item, add derives, and make small iterator/control-flow refinements. 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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
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)crates/stdlib/src/binascii.rs (1)111-118: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
Dead code: the Some branch 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, result cannot be a PyComplex, and the match will always take the None branch. The Some(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🤖 Prompt for AI Agentsreturn 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,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/vm/src/builtins/complex.rs (1)690-694: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
rlecode_hqx exits early and truncates output on first RUN_CHAR.
The return Ok(out_data); in the RUN_CHAR escape path stops processing the rest of the buffer, so encoded output is incomplete whenever 0x90 appears before the end.
Suggested fixif 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 AgentsVerify 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.97-118: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
Unreachable code: the match on line 112 is redundant.
The downcast_ref::<PyComplex>() call on line 97 already checks if result is a PyComplex. If it matches, the method returns on line 109. Therefore, when execution reaches the match on line 112, the same downcast_ref call will always return None (because it would have returned Some on line 97 and exited early).
This means:
- The Some(complex_obj) branch on line 113 is unreachable
- The None branch on line 114-117 will always execute after line 97 fails
This suggests incomplete refactoring. If the intent is to handle subclasses differently from exact types, consider using downcast_ref_if_exact for one of the checks. Otherwise, remove the redundant match and raise the TypeError directly after line 110.
Proposed fix: remove redundant match and raise TypeError directly🤖 Prompt for AI Agentsif 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() + ))); }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().
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().
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: e87f1506-2436-4019-9d84-cbd3827d5c95
📥 CommitsReviewing files that changed from the base of the PR and between 11e991f and f33bfcc.
📒 Files selected for processing (55)
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary by CodeRabbit
Chores
Bug Fixes
Performance Improvements