Skip to content

Clippy warn uninlined_format_args & redundant_else#7873

Merged
youknowone merged 12 commits into
RustPython:mainfrom
ShaharNaveh:clippy-pedantic-rules-0
May 15, 2026
Merged

Clippy warn uninlined_format_args & redundant_else#7873
youknowone merged 12 commits into
RustPython:mainfrom
ShaharNaveh:clippy-pedantic-rules-0

Conversation

@ShaharNaveh
Copy link
Copy Markdown
Contributor

@ShaharNaveh ShaharNaveh commented May 14, 2026

Summary by CodeRabbit

  • Chores

    • Standardized string formatting across the codebase and enabled two additional lints for cleaner code.
    • Simplified control flow in multiple modules to reduce unnecessary branches and clarify logic.
  • Bug Fixes

    • Improved and unified error and diagnostic message wording for consistency.
  • Performance Improvements

    • Preserved single-element identity in string joining to avoid an extra allocation.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

Note

Reviews paused

It 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:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: ef1715ef-88bc-4c51-b7b6-7b0aae222a75

📥 Commits

Reviewing files that changed from the base of the PR and between 7c9297e and 82ab35c.

📒 Files selected for processing (5)
  • crates/stdlib/src/locale.rs
  • crates/stdlib/src/mmap.rs
  • crates/stdlib/src/overlapped.rs
  • crates/stdlib/src/socket.rs
  • crates/stdlib/src/ssl.rs
✅ Files skipped from review due to trivial changes (4)
  • crates/stdlib/src/locale.rs
  • crates/stdlib/src/mmap.rs
  • crates/stdlib/src/ssl.rs
  • crates/stdlib/src/socket.rs

📝 Walkthrough

Walkthrough

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

Changes

Clippy lint adoption and code quality improvements

Layer / File(s) Summary
Clippy configuration and format string style foundation
Cargo.toml
Added redundant_else = "warn" and uninlined_format_args = "warn" to workspace Clippy configuration.
Borrowing & API ownership changes
crates/codegen/src/compile.rs, crates/derive-impl/src/compile_bytecode.rs, crates/derive-impl/src/pyclass.rs, crates/derive/src/lib.rs
Changed several APIs and call sites to accept &str or &Item instead of owned String/Item, adjusting module naming and nursery add_item methods to avoid unnecessary allocations.
Format string modernization, redundant-else removal, and misc refactors
crates/... (compiler, codegen, common, stdlib, vm, jit tests)
Replaced positional {} format placeholders with named/captured formatting across compiler, codegen, common, stdlib, VM, and supporting crates for error, debug, and repr strings; refactored control flow to remove redundant else blocks and annotated intentional empty else branches; applied small behavioral refinements (e.g., PyStr::join fast-path identity preservation, __complex__ downcast handling, added derives).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • youknowone
  • coolreader18

Poem

🐰 I tidied strings and trimmed each nested else,

Borrowed names, no clones to swell.
Derives snugly added, fast paths kept true,
Clippy nods kindly — this rabbit says "woo"!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately reflects the main change: enabling two Clippy lints (uninlined_format_args and redundant_else) across the codebase.
Docstring Coverage ✅ Passed Docstring coverage is 81.98% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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

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 (3)
crates/stdlib/src/unicodedata.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
             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_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 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 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
             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

📥 Commits

Reviewing files that changed from the base of the PR and between 11e991f and f33bfcc.

📒 Files selected for processing (55)
  • Cargo.toml
  • crates/codegen/src/compile.rs
  • crates/codegen/src/symboltable.rs
  • crates/common/src/cformat.rs
  • crates/common/src/format.rs
  • crates/compiler/src/lib.rs
  • crates/derive-impl/src/compile_bytecode.rs
  • crates/derive-impl/src/lib.rs
  • crates/derive-impl/src/pyclass.rs
  • crates/derive-impl/src/pystructseq.rs
  • crates/derive/src/lib.rs
  • crates/host_env/src/fileutils.rs
  • crates/stdlib/src/_asyncio.rs
  • crates/stdlib/src/binascii.rs
  • crates/stdlib/src/faulthandler.rs
  • crates/stdlib/src/math.rs
  • crates/stdlib/src/ssl.rs
  • crates/stdlib/src/unicodedata.rs
  • crates/vm/src/builtins/bool.rs
  • crates/vm/src/builtins/code.rs
  • crates/vm/src/builtins/complex.rs
  • crates/vm/src/builtins/descriptor.rs
  • crates/vm/src/builtins/function.rs
  • crates/vm/src/builtins/str.rs
  • crates/vm/src/builtins/template.rs
  • crates/vm/src/builtins/type.rs
  • crates/vm/src/bytes_inner.rs
  • crates/vm/src/dict_inner.rs
  • crates/vm/src/exception_group.rs
  • crates/vm/src/exceptions.rs
  • crates/vm/src/frame.rs
  • crates/vm/src/getpath.rs
  • crates/vm/src/import.rs
  • crates/vm/src/ospath.rs
  • crates/vm/src/stdlib/_ast.rs
  • crates/vm/src/stdlib/_ctypes.rs
  • crates/vm/src/stdlib/_ctypes/base.rs
  • crates/vm/src/stdlib/_ctypes/function.rs
  • crates/vm/src/stdlib/_ctypes/pointer.rs
  • crates/vm/src/stdlib/_ctypes/simple.rs
  • crates/vm/src/stdlib/_ctypes/structure.rs
  • crates/vm/src/stdlib/_ctypes/union.rs
  • crates/vm/src/stdlib/_functools.rs
  • crates/vm/src/stdlib/_io.rs
  • crates/vm/src/stdlib/_signal.rs
  • crates/vm/src/stdlib/_typing.rs
  • crates/vm/src/stdlib/_warnings.rs
  • crates/vm/src/stdlib/builtins.rs
  • crates/vm/src/stdlib/gc.rs
  • crates/vm/src/stdlib/itertools.rs
  • crates/vm/src/stdlib/posix.rs
  • crates/vm/src/stdlib/sys.rs
  • crates/vm/src/types/slot.rs
  • crates/vm/src/types/structseq.rs
  • crates/vm/src/vm/mod.rs

@youknowone youknowone merged commit 460b1d3 into RustPython:main May 15, 2026
25 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request May 15, 2026
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.

2 participants