Skip to content

Conversation

@elmjag
Copy link
Contributor

@elmjag elmjag commented Feb 3, 2026

Adds support for performing '|' operation between Union objects and strings, e.g. forward type references.

For example following code:

from typing import Union

U1 = Union[int, str]
U1 | "float"

The result of the operation above becomes:

int | str | ForwardRef('float')

Summary by CodeRabbit

  • Refactor
    • Union-type combination now follows a typing-based path with a fast path for common cases, improving correctness and error handling.
    • Internal typing helper consolidated into a shared typing call to simplify type-variable and typing operations.
    • Reduced public exposure of an internal unionability helper while preserving outward union behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

Type's or_ now delegates union construction to a new public union::or_op, which applies a fast-path unionability check or falls back to typing._type_check before constructing the union.

Changes

Cohort / File(s) Summary
Type delegation
crates/vm/src/builtins/type.rs
Replaced manual union checks and tuple construction in or_ with a call to union::or_op(zelf, other, vm).
Union module refactor & API
crates/vm/src/builtins/union.rs
Added public or_op(zelf, other, vm); introduced private helpers type_check and has_union_operands; reduced visibility of is_unionable and make_parameters; or_op performs fast-path checks or delegates to typing-based _type_check then calls make_union.
Typing helper exposed
crates/vm/src/stdlib/typing.rs
Added public call_typing_func_object to import typing and call its functions; removed an internal duplicate helper.
TypeVar wiring
crates/vm/src/stdlib/typevar.rs
Replaced local _call_typing_func_object usages with the new call_typing_func_object import and removed the local helper.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant Type as crates::vm::builtins::type::or_
    participant Union as crates::vm::builtins::union::or_op
    participant Typing as typing._type_check
    participant MakeUnion as make_union

    Caller->>Type: or_(left, right)
    Type->>Union: or_op(left, right, vm)
    Union->>Union: has_union_operands / is_unionable fast-path
    alt fast-path
        Union-->>Type: use operand(s) directly
    else fallback
        Union->>Typing: call typing._type_check(arg, message)
        Typing-->>Union: validated/adjusted arg or error
    end
    Union->>MakeUnion: make_union(tuple_of_two_args)
    MakeUnion-->>Union: union object
    Union-->>Type: return union
    Type-->>Caller: return result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • arihant2math

Poem

🐰 I hopped from Type into Union's lair,
Swapped checks for a shortcut, light as air.
A calling to Typing, a tuple in tow,
Then make_union hummed — and off we go! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'support | operation between typing.Union and strings' clearly describes the main feature being added—enabling the bitwise-or operator for Union types with string references, which aligns with the changeset's core functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Important

Action Needed: IP Allowlist Update

If your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:

  • 136.113.208.247/32 (new)
  • 34.170.211.100/32
  • 35.222.179.152/32

Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist.


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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@crates/vm/src/builtins/union.rs`:
- Around line 206-214: The helper function _call_typing_func_object is
duplicated in union.rs, typing.rs, and typevar.rs; extract it into a single pub
function at module level in typing.rs (make the existing implementation public,
e.g., pub fn _call_typing_func_object(...)) and remove the duplicate definitions
in union.rs and typevar.rs, replacing them with a use/import of the shared
function (call the same symbol _call_typing_func_object) and call it where
needed; ensure the signature stays compatible with VirtualMachine, AsPyStr and
IntoFuncArgs so existing call sites (including code using TypeAliasType in
union.rs) compile without further edits.
🧹 Nitpick comments (1)
crates/vm/src/builtins/union.rs (1)

240-252: Consider using the ? operator for cleaner error propagation.

The match statements can be simplified using Rust's ? operator.

♻️ Suggested simplification
-    let left = match type_check(zelf, vm) {
-        Ok(v) => v,
-        err => {
-            return err;
-        }
-    };
-
-    let right = match type_check(other, vm) {
-        Ok(v) => v,
-        err => {
-            return err;
-        }
-    };
+    let left = type_check(zelf, vm)?;
+    let right = type_check(other, vm)?;

Comment on lines 240 to 245
let left = match type_check(zelf, vm) {
Ok(v) => v,
err => {
return err;
}
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let left = match type_check(zelf, vm) {
Ok(v) => v,
err => {
return err;
}
};
let left = type_check(zelf, vm)?;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, yes. Changed to use ?.

Move _call_typing_func_object() code to
stdlib::typing::call_typing_func_object().
Use that function everywhere.
Adds support for performing '|' operation between Union objects and
strings, e.g. forward type references.

For example following code:

    from typing import Union

    U1 = Union[int, str]
    U1 | "float"

The result of the operation above becomes:

   int | str | ForwardRef('float')
@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2026

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/typing.py
[ ] test: cpython/Lib/test/test_typing.py (TODO: 20)
[ ] test: cpython/Lib/test/test_type_aliases.py
[ ] test: cpython/Lib/test/test_type_annotations.py
[ ] test: cpython/Lib/test/test_type_params.py
[ ] test: cpython/Lib/test/test_genericalias.py (TODO: 3)

dependencies:

  • typing

dependent tests: (10 tests)

  • typing: test_builtin test_enum test_fractions test_functools test_genericalias test_grammar test_importlib test_isinstance test_types test_typing

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elmjag Thank you so much for contributing! And welcome to RustPython project.

Copy link
Contributor

@moreal moreal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contribution!

@youknowone youknowone merged commit ffc4622 into RustPython:main Feb 4, 2026
24 of 25 checks passed
@elmjag elmjag deleted the union-or branch February 4, 2026 15:19
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.

3 participants