Fix error / warning messages

Open youknowone opened this issue 3 weeks ago • 1 comments

Dec 30 '25 07:12 youknowone

📝 Walkthrough

Walkthrough

This PR introduces deprecation warnings for multi-argument throw signatures across async generators, coroutines, and generators. It enhances error messages for function positional argument mismatches and updates PyPartial's module attribute and representation logic.

Changes

Cohort / File(s)	Summary
Deprecation Warnings for throw() Methods `crates/vm/src/builtins/asyncgenerator.rs`, `crates/vm/src/builtins/coroutine.rs`, `crates/vm/src/builtins/generator.rs`	Each throw method now invokes `warn_deprecated_throw_signature()` before delegating to inner throw logic, enabling pre-throw validation that can abort operation if the hook raises an error.
warn_deprecated_throw_signature Implementation `crates/vm/src/coroutine.rs`	New public function added that emits a deprecation warning when either `exc_val` or `exc_tb` arguments are present in throw calls.
Function Argument Error Messages `crates/vm/src/builtins/function.rs`	Updated error message generation for positional argument mismatches: now dynamically calculates required vs. defaulted arguments and constructs messages in "from X to Y" format with proper pluralization instead of static messages.
PyPartial Representation Updates `crates/vm/src/stdlib/functools.rs`	Module attribute changed from `"_functools"` to `"functools"`; `__repr__` logic now uses `__qualname__` with module prefix for non-builtins to construct qualified names.

Sequence Diagram

sequenceDiagram
    participant AsyncGen as AsyncGenerator/Coroutine/Generator
    participant Warn as warn_deprecated_throw_signature()
    participant VM as VirtualMachine
    participant Inner as Inner throw()

    AsyncGen->>Warn: Call with exc_val, exc_tb
    alt exc_val or exc_tb present
        Warn->>VM: Emit deprecation warning
        VM-->>Warn: PyResult
        rect rgb(200, 220, 240)
            Note over Warn: If hook raises,<br/>error propagates
        end
        Warn-->>AsyncGen: Result (Ok or Err)
    else Both absent
        Warn-->>AsyncGen: Ok(())
    end
    
    alt warn_result is Ok
        AsyncGen->>Inner: Proceed with throw()
        Inner-->>AsyncGen: Result
    else warn_result is Err
        rect rgb(240, 200, 200)
            Note over AsyncGen: Control flow altered:<br/>throw aborted, error returned
        end
        AsyncGen-->>AsyncGen: Return error
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

RustPython/RustPython#6439: Modifies throw-related code paths in asyncgenerator.rs and coroutine.rs with new behavior invoked in the same throw implementations.
RustPython/RustPython#6427: Touches throw-related implementations across coroutine, generator, and asyncgenerator modules including PyAnextAwaitable::throw.
RustPython/RustPython#5825: Implements PyPartial in \_functools and defines its repr/qualification behavior; this PR switches its module to functools and alters qualname logic.

Poem

🐰 A hop through deprecation's gentle way,
Where throw calls whisper what they say,
New warnings bloom in async streams,
While functools glows with cleaner schemes,
Error messages now dance and sway! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Fix error / warning messages' is overly vague and generic. It uses non-descriptive language that doesn't convey meaningful information about the specific changes across multiple files (coroutine deprecation warnings, function argument error messages, functools module naming).	Provide a more specific title that captures the main change, such as 'Add deprecation warning for throw signature in async generators, generators, and coroutines' or 'Improve error and deprecation warning messages across VM builtins'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

[ ] 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 37c47fca6ba9a411192059e5ef671a470b286fbf and 60f5185359ae96c38cefa7c2cce76a8c9bc8c8dd.

⛔ Files ignored due to path filters (6)

Lib/test/test_dataclasses/__init__.py is excluded by !Lib/**
Lib/test/test_functools.py is excluded by !Lib/**
Lib/test/test_generators.py is excluded by !Lib/**
Lib/test/test_keywordonlyarg.py is excluded by !Lib/**
Lib/test/test_positional_only_arg.py is excluded by !Lib/**
Lib/test/test_typing.py is excluded by !Lib/**

📒 Files selected for processing (6)

crates/vm/src/builtins/asyncgenerator.rs
crates/vm/src/builtins/coroutine.rs
crates/vm/src/builtins/function.rs
crates/vm/src/builtins/generator.rs
crates/vm/src/coroutine.rs
crates/vm/src/stdlib/functools.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes Follow Rust best practices for error handling and memory management Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/vm/src/builtins/function.rs
crates/vm/src/coroutine.rs
crates/vm/src/stdlib/functools.rs
crates/vm/src/builtins/coroutine.rs
crates/vm/src/builtins/generator.rs
crates/vm/src/builtins/asyncgenerator.rs

🧠 Learnings (1)

📚 Learning: 2025-12-27T14:03:49.034Z

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

crates/vm/src/coroutine.rs

🧬 Code graph analysis (5)

crates/vm/src/coroutine.rs (1)

crates/vm/src/warn.rs (1)

warn (83-94)

crates/vm/src/stdlib/functools.rs (2)

crates/vm/src/builtins/type.rs (1)

name (589-594)

crates/vm/src/protocol/object.rs (1)

s (379-379)

crates/vm/src/builtins/coroutine.rs (1)

crates/vm/src/coroutine.rs (1)

warn_deprecated_throw_signature (217-235)

crates/vm/src/builtins/generator.rs (1)

crates/vm/src/coroutine.rs (1)

warn_deprecated_throw_signature (217-235)

crates/vm/src/builtins/asyncgenerator.rs (1)

crates/vm/src/coroutine.rs (1)

warn_deprecated_throw_signature (217-235)

🔇 Additional comments (14)

crates/vm/src/builtins/coroutine.rs (3)

5-5: LGTM!

Import of warn_deprecated_throw_signature is correctly added alongside Coro.

111-111: LGTM!

Deprecation warning correctly invoked before delegating to the inner throw logic. The ? operator properly propagates any error from the warning function.

185-185: LGTM!

Consistent with PyCoroutine::throw - deprecation warning is called before the inner coroutine's throw method.

crates/vm/src/coroutine.rs (2)

7-7: LGTM!

Import of OptionalArg is necessary for the new warn_deprecated_throw_signature function signature.

216-235: Well-implemented deprecation warning function.

The implementation correctly:

Takes references to OptionalArg to avoid unnecessary cloning

Uses OR logic to trigger warning when either exc_val or exc_tb is present

Propagates errors via ? in case warnings are configured to raise exceptions

Uses stack_level=1 to point to the caller's location

Matches CPython's deprecation message for the 3-argument throw() signature

Based on learnings, this appropriately follows the project's pattern for implementing Python functionality in Rust.

crates/vm/src/builtins/generator.rs (2)

9-9: LGTM!

Import correctly added for the deprecation warning function.

102-102: LGTM!

Deprecation warning properly invoked before delegating to the inner Coro::throw. This is consistent with the pattern applied to coroutines and async generators.

crates/vm/src/builtins/asyncgenerator.rs (4)

7-7: LGTM!

Import correctly added for the deprecation warning function.

315-315: LGTM!

Deprecation warning correctly invoked in PyAsyncGenASend::throw before the inner throw logic.

435-435: LGTM!

Deprecation warning correctly invoked in PyAsyncGenAThrow::throw before the inner throw logic.

606-606: LGTM!

Deprecation warning correctly invoked in PyAnextAwaitable::throw before the inner throw logic. This completes the consistent application of the deprecation warning across all async generator throw entry points.

crates/vm/src/stdlib/functools.rs (2)

322-338: Improved repr logic with proper module-qualified naming.

The updated logic correctly:

Extracts __qualname__ with a fallback to the class name

Handles module prefixing for non-builtins modules

Uses a consistent fallback (qualname_str) when module extraction fails

This produces cleaner repr output like functools.partial(...) instead of potentially exposing internal module names.

46-46: Module attribute change aligns with CPython.

Changing from _functools to functools matches how CPython exposes partial — it is publicly documented as part of the functools module, so users see functools.partial rather than _functools.partial in type information and module references.

crates/vm/src/builtins/function.rs (1)

134-153: Implementation correctly matches CPython's pluralization logic.

The error message uses the simple pluralization rule: singular "argument" when n_expected_args == 1, plural "arguments" otherwise. This applies consistently even for range messages like "from 0 to 1 positional argument", which matches CPython's behavior exactly. No changes needed.

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

Dec 30 '25 07:12 coderabbitai[bot]