[ty] Make special cases for UnionType slightly narrower

Author: AlexWaygood

crates/ty_python_semantic/resources/mdtest/implicit_type_aliases.md

@@ -135,14 +135,15 @@ def _(int_or_int: IntOrInt, list_of_int_or_list_of_int: ListOfIntOrListOfInt):
 None | None  # error: [unsupported-operator] "Operator `|` is unsupported between objects of type `None` and `None`"
 ```
 
-When constructing something non-sensical like `int | 1`, we could ideally emit a diagnostic for the
-expression itself, as it leads to a `TypeError` at runtime. No other type checker supports this, so
-for now we only emit an error when it is used in a type expression:
+When constructing something nonsensical like `int | 1`, we emit a diagnostic for the expression
+itself, as it leads to a `TypeError` at runtime. The result of the expression is then inferred as
+`Unknown`, so we permit it to be used in a type expression.
 
 ```py
-IntOrOne = int | 1
+IntOrOne = int | 1  # error: [unsupported-operator]
+
+reveal_type(IntOrOne)  # revealed: Unknown
 
-# error: [invalid-type-form] "Variable of type `Literal[1]` is not allowed in a type expression"
 def _(int_or_one: IntOrOne):
     reveal_type(int_or_one)  # revealed: Unknown
 ```
@@ -160,6 +161,70 @@ def f(SomeUnionType: UnionType):
 f(int | str)
 ```
 
+## `|` operator between class objects and non-class objects
+
+Using the `|` operator between a class object and a non-class object does not create a `UnionType`
+instance; it calls the relevant dunder as normal:
+
+```py
+class Foo:
+    def __or__(self, other) -> str:
+        return "foo"
+
+reveal_type(Foo() | int)  # revealed: str
+
+class Bar:
+    def __ror__(self, other) -> str:
+        return "bar"
+
+reveal_type(int | Bar())  # revealed: str
+
+class Invalid:
+    def __or__(self, other: "Invalid") -> str:
+        return "Invalid"
+
+# error: [unsupported-operator]
+reveal_type(int | Invalid())  # revealed: Unknown
+```
+
+## Custom `__(r)or__` methods on metaclasses are only partially respected
+
+A drawback of our extensive special casing of `|` operations between class objects is that
+`__(r)or__` methods on metaclasses are completely disregarded if two classes are `|`'d together. We
+respect the metaclass dunder if a class is `|`'d with a non-class, however:
+
+```py
+class Meta(type):
+    def __or__(self, other) -> str:
+        return "Meta"
+
+class Foo(metaclass=Meta): ...
+class Bar(metaclass=Meta): ...
+
+X = Foo | Bar
+
+# In an ideal world, perhaps we would respect `Meta.__or__` here and reveal `str`?
+# But we still need to record what the elements are, since (according to the typing spec)
+# `X` is still a valid type alias
+reveal_type(X)  # revealed: types.UnionType
+
+def f(obj: X):
+    reveal_type(obj)  # revealed: Foo | Bar
+
+# We do respect the metaclass `__or__` if it's used between a class and a non-class, however:
+
+Y = Foo | 42
+reveal_type(Y)  # revealed: str
+
+Z = Bar | 56
+reveal_type(Z)  # revealed: str
+
+def g(
+    arg1: Y,  # error: [invalid-type-form]
+    arg2: Z,  # error: [invalid-type-form]
+): ...
+```
+
 ## Generic types
 
 Implicit type aliases can also refer to generic types:
@@ -191,7 +256,8 @@ From the [typing spec on type aliases](https://typing.python.org/en/latest/spec/
 > type hint is acceptable in a type alias
 
 However, no other type checker seems to support stringified annotations in implicit type aliases. We
-currently also do not support them:
+currently also do not support them, and we detect places where these attempted unions cause runtime
+errors:
 
 ```py
 AliasForStr = "str"
@@ -200,9 +266,10 @@ AliasForStr = "str"
 def _(s: AliasForStr):
     reveal_type(s)  # revealed: Unknown
 
-IntOrStr = int | "str"
+IntOrStr = int | "str"  # error: [unsupported-operator]
+
+reveal_type(IntOrStr)  # revealed: Unknown
 
-# error: [invalid-type-form] "Variable of type `Literal["str"]` is not allowed in a type expression"
 def _(int_or_str: IntOrStr):
     reveal_type(int_or_str)  # revealed: Unknown
 ```

crates/ty_python_semantic/src/types/call.rs

@@ -1,8 +1,8 @@
 use super::context::InferContext;
 use super::{Signature, Type, TypeContext};
 use crate::Db;
-use crate::types::PropertyInstanceType;
 use crate::types::call::bind::BindingError;
+use crate::types::{MemberLookupPolicy, PropertyInstanceType};
 use ruff_python_ast as ast;
 
 mod arguments;
@@ -16,6 +16,16 @@ impl<'db> Type<'db> {
         left_ty: Type<'db>,
         op: ast::Operator,
         right_ty: Type<'db>,
+    ) -> Result<Bindings<'db>, CallBinOpError> {
+        Self::try_call_bin_op_with_policy(db, left_ty, op, right_ty, MemberLookupPolicy::default())
+    }
+
+    pub(crate) fn try_call_bin_op_with_policy(
+        db: &'db dyn Db,
+        left_ty: Type<'db>,
+        op: ast::Operator,
+        right_ty: Type<'db>,
+        policy: MemberLookupPolicy,
     ) -> Result<Bindings<'db>, CallBinOpError> {
         // We either want to call lhs.__op__ or rhs.__rop__. The full decision tree from
         // the Python spec [1] is:
@@ -43,39 +53,43 @@ impl<'db> Type<'db> {
                 && rhs_reflected != left_class.member(db, reflected_dunder).place
             {
                 return Ok(right_ty
-                    .try_call_dunder(
+                    .try_call_dunder_with_policy(
                         db,
                         reflected_dunder,
-                        CallArguments::positional([left_ty]),
+                        &mut CallArguments::positional([left_ty]),
                         TypeContext::default(),
+                        policy,
                     )
                     .or_else(|_| {
-                        left_ty.try_call_dunder(
+                        left_ty.try_call_dunder_with_policy(
                             db,
                             op.dunder(),
-                            CallArguments::positional([right_ty]),
+                            &mut CallArguments::positional([right_ty]),
                             TypeContext::default(),
+                            policy,
                         )
                     })?);
             }
         }
 
-        let call_on_left_instance = left_ty.try_call_dunder(
+        let call_on_left_instance = left_ty.try_call_dunder_with_policy(
             db,
             op.dunder(),
-            CallArguments::positional([right_ty]),
+            &mut CallArguments::positional([right_ty]),
             TypeContext::default(),
+            policy,
         );
 
         call_on_left_instance.or_else(|_| {
             if left_ty == right_ty {
                 Err(CallBinOpError::NotSupported)
             } else {
-                Ok(right_ty.try_call_dunder(
+                Ok(right_ty.try_call_dunder_with_policy(
                     db,
                     op.reflected_dunder(),
-                    CallArguments::positional([left_ty]),
+                    &mut CallArguments::positional([left_ty]),
                     TypeContext::default(),
+                    policy,
                 )?)
             }
         })

crates/ty_python_semantic/src/types/infer/builder.rs

@@ -8474,42 +8474,75 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                 | Type::GenericAlias(..)
                 | Type::SpecialForm(_)
                 | Type::KnownInstance(KnownInstanceType::UnionType(_)),
-                _,
-                ast::Operator::BitOr,
-            )
-            | (
-                _,
                 Type::ClassLiteral(..)
                 | Type::SubclassOf(..)
                 | Type::GenericAlias(..)
                 | Type::SpecialForm(_)
                 | Type::KnownInstance(KnownInstanceType::UnionType(_)),
                 ast::Operator::BitOr,
             ) if Program::get(self.db()).python_version(self.db()) >= PythonVersion::PY310 => {
-                // For a value expression like `int | None`, the inferred type for `None` will be
-                // a nominal instance of `NoneType`, so we need to convert it to a class literal
-                // such that it can later be converted back to a nominal instance type when calling
-                // `.in_type_expression` on the `UnionType` instance.
-                let convert_none_type = |ty: Type<'db>| {
-                    if ty.is_none(self.db()) {
-                        KnownClass::NoneType.to_class_literal(self.db())
-                    } else {
-                        ty
-                    }
-                };
-
                 if left_ty.is_equivalent_to(self.db(), right_ty) {
                     Some(left_ty)
                 } else {
                     Some(Type::KnownInstance(KnownInstanceType::UnionType(
-                        UnionTypeInstance::new(
-                            self.db(),
-                            convert_none_type(left_ty),
-                            convert_none_type(right_ty),
-                        ),
+                        UnionTypeInstance::new(self.db(), left_ty, right_ty),
                     )))
                 }
             }
+            (
+                not_none @ (Type::ClassLiteral(..)
+                | Type::SubclassOf(..)
+                | Type::GenericAlias(..)
+                | Type::KnownInstance(..)
+                | Type::SpecialForm(..)),
+                none @ Type::NominalInstance(instance),
+                ast::Operator::BitOr,
+            ) if Program::get(self.db()).python_version(self.db()) >= PythonVersion::PY310
+                && instance.has_known_class(self.db(), KnownClass::NoneType) =>
+            {
+                Some(Type::KnownInstance(KnownInstanceType::UnionType(
+                    UnionTypeInstance::new(self.db(), not_none, none),
+                )))
+            }
+            (
+                none @ Type::NominalInstance(instance),
+                not_none @ (Type::ClassLiteral(..)
+                | Type::SubclassOf(..)
+                | Type::GenericAlias(..)
+                | Type::KnownInstance(..)
+                | Type::SpecialForm(..)),
+                ast::Operator::BitOr,
+            ) if Program::get(self.db()).python_version(self.db()) >= PythonVersion::PY310
+                && instance.has_known_class(self.db(), KnownClass::NoneType) =>
+            {
+                Some(Type::KnownInstance(KnownInstanceType::UnionType(
+                    UnionTypeInstance::new(self.db(), none, not_none),
+                )))
+            }
+
+            // We avoid calling `type.__(r)or__`, as typeshed annotates these methods as
+            // accepting `Any` (since typeforms are inexpressable in the type system currently).
+            // This means that many common errors would not be caught if we fell back to typeshed's stubs here.
+            // Note that if a class had a custom metaclass that overrode `__(r)or__`, we would also ignore
+            // that custom method, but this seems like it's probably rare enough that it's acceptable.
+            (
+                Type::ClassLiteral(..) | Type::GenericAlias(..) | Type::SubclassOf(..),
+                _,
+                ast::Operator::BitOr,
+            )
+            | (
+                _,
+                Type::ClassLiteral(..) | Type::GenericAlias(..) | Type::SubclassOf(..),
+                ast::Operator::BitOr,
+            ) => Type::try_call_bin_op_with_policy(
+                self.db(),
+                left_ty,
+                ast::Operator::BitOr,
+                right_ty,
+                MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK,
+            )
+            .ok()
+            .map(|binding| binding.return_type(self.db())),
 
             // We've handled all of the special cases that we support for literals, so we need to
             // fall back on looking for dunder methods on one of the operand types.