Skip to content

Conversation

@sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Mar 10, 2025

Summary

This fixes a bug I noticed while looking at ecosystem results. The MRE version of it is this:

def sub(x: float, y: float):
    # Red Knot: Operator `-` is unsupported between objects of type `int | float` and `int | float`
    return x - y

It's yet another instance of the general problem that our way of representing union types does not prevent us from handling unions (and intersections) incorrectly. The pattern is usually this: we implement operation X by explicitly matching on some cases that we handle explicitly (e.g. literals). For all other types, we fall back to some generic behavior (calling some functions that work for every Type). That generic behavior also compiles for Type::Union/Type::Intersection, but what we really want is to recursively call X for each element, which also gives us that special behavior for elements of the union.

For intersections, I'll open a ticket.

Test Plan

  • New Markdown tests.
  • Expected diff in the ecosystem checks

@sharkdp sharkdp added the ty Multi-file analysis & type inference label Mar 10, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Mar 10, 2025

mypy_primer results

Changes were detected when running on open source projects
mypy_primer (https://github.com/hauntsaninja/mypy_primer)
- 
-     |
-     |
- error: lint:unsupported-operator
-    --> /tmp/mypy_primer/projects/mypy_primer/mypy_primer/utils.py:114:79
- 112 |     if check and proc.returncode:
- 113 |         raise subprocess.CalledProcessError(proc.returncode, cmd, output=stdout, stderr=stderr)
- 114 |     return subprocess.CompletedProcess(cmd, proc.returncode, stdout, stderr), end_t - start_t
-     |                                                                               ^^^^^^^^^^^^^^^ Operator `-` is unsupported between objects of type `int | float` and `int | float`
- Found 50 diagnostics
+ Found 49 diagnostics

@sharkdp

This comment was marked as resolved.

@sharkdp sharkdp force-pushed the david/binary-operator-unions branch from 0303719 to 2273374 Compare March 10, 2025 14:38
@MichaReiser

This comment was marked as resolved.

@sharkdp

This comment was marked as resolved.

sharkdp added a commit that referenced this pull request Mar 10, 2025
## Summary

Strip ANSI codes in the mypy_primer diff before uploading.

## Test Plan

Successful run here: #16601
@sharkdp sharkdp force-pushed the david/binary-operator-unions branch from 2273374 to 86933b9 Compare March 10, 2025 16:33
@sharkdp

This comment was marked as resolved.

AlexWaygood pushed a commit that referenced this pull request Mar 10, 2025
## Summary

Strip ANSI codes in the mypy_primer diff before uploading.

## Test Plan

Successful run here: #16601
@sharkdp sharkdp force-pushed the david/binary-operator-unions branch from 86933b9 to e06ac75 Compare March 11, 2025 10:14
@sharkdp sharkdp force-pushed the david/binary-operator-unions branch from e06ac75 to 760bbd4 Compare March 11, 2025 21:03
(Type::Union(lhs_union), rhs, _) => {
let mut union = UnionBuilder::new(self.db());
for lhs in lhs_union.elements(self.db()) {
let result = self.infer_binary_expression_type(*lhs, rhs, op)?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of short-circuiting here, we could probably also build a better diagnostic, telling you which exact element-combination didn't work. But it seems low priority and doesn't necessarily have to be included here, I think.

@sharkdp sharkdp marked this pull request as ready for review March 11, 2025 21:20
Copy link
Contributor

@carljm carljm 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!

@sharkdp sharkdp merged commit 860b95a into main Mar 12, 2025
22 checks passed
@sharkdp sharkdp deleted the david/binary-operator-unions branch March 12, 2025 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants