[ty] Add a diagnostic for @functools.total_ordering without a defined comparison method (#22183)
## Summary
This raises a `ValueError` at runtime:
```python
from functools import total_ordering
@total_ordering
class NoOrdering:
def __eq__(self, other: object) -> bool:
return True
```
Specifically:
```
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/functools.py", line 193, in total_ordering
raise ValueError('must define at least one ordering operation: < > <= >=')
ValueError: must define at least one ordering operation: < > <= >=
```
See: https://github.com/astral-sh/ty/issues/1202.
This commit is contained in:
@@ -583,7 +583,7 @@ from module import NotFrozenBase
|
||||
|
||||
@final
|
||||
@dataclass(frozen=True)
|
||||
@total_ordering
|
||||
@total_ordering # error: [invalid-total-ordering]
|
||||
class FrozenChild(NotFrozenBase): # error: [invalid-frozen-dataclass-subclass]
|
||||
y: str
|
||||
```
|
||||
|
||||
@@ -194,12 +194,12 @@ reveal_type(p1 >= p2) # revealed: bool
|
||||
## Missing ordering method
|
||||
|
||||
If a class has `@total_ordering` but doesn't define any ordering method (itself or in a superclass),
|
||||
the decorator would fail at runtime. We don't synthesize methods in this case:
|
||||
a diagnostic is emitted at the decorator site:
|
||||
|
||||
```py
|
||||
from functools import total_ordering
|
||||
|
||||
@total_ordering
|
||||
@total_ordering # error: [invalid-total-ordering]
|
||||
class NoOrdering:
|
||||
def __eq__(self, other: object) -> bool:
|
||||
return True
|
||||
@@ -207,7 +207,7 @@ class NoOrdering:
|
||||
n1 = NoOrdering()
|
||||
n2 = NoOrdering()
|
||||
|
||||
# These should error because no ordering method is defined.
|
||||
# Comparison operators also error because no methods were synthesized.
|
||||
n1 <= n2 # error: [unsupported-operator]
|
||||
n1 >= n2 # error: [unsupported-operator]
|
||||
```
|
||||
|
||||
@@ -61,7 +61,7 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.
|
||||
6 |
|
||||
7 | @final
|
||||
8 | @dataclass(frozen=True)
|
||||
9 | @total_ordering
|
||||
9 | @total_ordering # error: [invalid-total-ordering]
|
||||
10 | class FrozenChild(NotFrozenBase): # error: [invalid-frozen-dataclass-subclass]
|
||||
11 | y: str
|
||||
```
|
||||
@@ -126,6 +126,22 @@ info: rule `invalid-frozen-dataclass-subclass` is enabled by default
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error[invalid-total-ordering]: Class decorated with `@total_ordering` must define at least one ordering method
|
||||
--> src/main.py:9:1
|
||||
|
|
||||
7 | @final
|
||||
8 | @dataclass(frozen=True)
|
||||
9 | @total_ordering # error: [invalid-total-ordering]
|
||||
| ^^^^^^^^^^^^^^^ `FrozenChild` does not define `__lt__`, `__le__`, `__gt__`, or `__ge__`
|
||||
10 | class FrozenChild(NotFrozenBase): # error: [invalid-frozen-dataclass-subclass]
|
||||
11 | y: str
|
||||
|
|
||||
info: The decorator will raise `ValueError` at runtime
|
||||
info: rule `invalid-total-ordering` is enabled by default
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error[invalid-frozen-dataclass-subclass]: Frozen dataclass cannot inherit from non-frozen dataclass
|
||||
--> src/main.py:8:1
|
||||
@@ -133,7 +149,7 @@ error[invalid-frozen-dataclass-subclass]: Frozen dataclass cannot inherit from n
|
||||
7 | @final
|
||||
8 | @dataclass(frozen=True)
|
||||
| ----------------------- `FrozenChild` dataclass parameters
|
||||
9 | @total_ordering
|
||||
9 | @total_ordering # error: [invalid-total-ordering]
|
||||
10 | class FrozenChild(NotFrozenBase): # error: [invalid-frozen-dataclass-subclass]
|
||||
| ^^^^^^^^^^^^-------------^ Subclass `FrozenChild` is frozen but base class `NotFrozenBase` is not
|
||||
11 | y: str
|
||||
|
||||
@@ -125,6 +125,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
|
||||
registry.register_lint(&INVALID_EXPLICIT_OVERRIDE);
|
||||
registry.register_lint(&SUPER_CALL_IN_NAMED_TUPLE_METHOD);
|
||||
registry.register_lint(&INVALID_FROZEN_DATACLASS_SUBCLASS);
|
||||
registry.register_lint(&INVALID_TOTAL_ORDERING);
|
||||
|
||||
// String annotations
|
||||
registry.register_lint(&BYTE_STRING_TYPE_ANNOTATION);
|
||||
@@ -2329,6 +2330,46 @@ declare_lint! {
|
||||
}
|
||||
}
|
||||
|
||||
declare_lint! {
|
||||
/// ## What it does
|
||||
/// Checks for classes decorated with `@functools.total_ordering` that don't
|
||||
/// define any ordering method (`__lt__`, `__le__`, `__gt__`, or `__ge__`).
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// The `@total_ordering` decorator requires the class to define at least one
|
||||
/// ordering method. If none is defined, Python raises a `ValueError` at runtime.
|
||||
///
|
||||
/// ## Example
|
||||
///
|
||||
/// ```python
|
||||
/// from functools import total_ordering
|
||||
///
|
||||
/// @total_ordering
|
||||
/// class MyClass: # Error: no ordering method defined
|
||||
/// def __eq__(self, other: object) -> bool:
|
||||
/// return True
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
///
|
||||
/// ```python
|
||||
/// from functools import total_ordering
|
||||
///
|
||||
/// @total_ordering
|
||||
/// class MyClass:
|
||||
/// def __eq__(self, other: object) -> bool:
|
||||
/// return True
|
||||
///
|
||||
/// def __lt__(self, other: "MyClass") -> bool:
|
||||
/// return True
|
||||
/// ```
|
||||
pub(crate) static INVALID_TOTAL_ORDERING = {
|
||||
summary: "detects `@total_ordering` classes without an ordering method",
|
||||
status: LintStatus::stable("0.0.10"),
|
||||
default_level: Level::Error,
|
||||
}
|
||||
}
|
||||
|
||||
/// A collection of type check diagnostics.
|
||||
#[derive(Default, Eq, PartialEq, get_size2::GetSize)]
|
||||
pub struct TypeCheckDiagnostics {
|
||||
@@ -4618,6 +4659,27 @@ pub(super) fn report_bad_frozen_dataclass_inheritance<'db>(
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn report_invalid_total_ordering(
|
||||
context: &InferContext<'_, '_>,
|
||||
class: ClassLiteral<'_>,
|
||||
decorator: &ast::Decorator,
|
||||
) {
|
||||
let db = context.db();
|
||||
|
||||
let Some(builder) = context.report_lint(&INVALID_TOTAL_ORDERING, decorator) else {
|
||||
return;
|
||||
};
|
||||
|
||||
let mut diagnostic = builder.into_diagnostic(
|
||||
"Class decorated with `@total_ordering` must define at least one ordering method",
|
||||
);
|
||||
diagnostic.set_primary_message(format_args!(
|
||||
"`{}` does not define `__lt__`, `__le__`, `__gt__`, or `__ge__`",
|
||||
class.name(db)
|
||||
));
|
||||
diagnostic.info("The decorator will raise `ValueError` at runtime");
|
||||
}
|
||||
|
||||
/// This function receives an unresolved `from foo import bar` import,
|
||||
/// where `foo` can be resolved to a module but that module does not
|
||||
/// have a `bar` member or submodule.
|
||||
|
||||
@@ -77,7 +77,7 @@ use crate::types::diagnostic::{
|
||||
report_invalid_exception_caught, report_invalid_exception_cause,
|
||||
report_invalid_exception_raised, report_invalid_exception_tuple_caught,
|
||||
report_invalid_generator_function_return_type, report_invalid_key_on_typed_dict,
|
||||
report_invalid_or_unsupported_base, report_invalid_return_type,
|
||||
report_invalid_or_unsupported_base, report_invalid_return_type, report_invalid_total_ordering,
|
||||
report_invalid_type_checking_constant, report_invalid_type_param_order,
|
||||
report_named_tuple_field_with_leading_underscore,
|
||||
report_namedtuple_field_without_default_after_field_with_default, report_not_subscriptable,
|
||||
@@ -852,7 +852,39 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||
}
|
||||
}
|
||||
|
||||
// (5) Check that the class's metaclass can be determined without error.
|
||||
// (5) Check that @total_ordering has a valid ordering method in the MRO
|
||||
if class.total_ordering(self.db()) {
|
||||
let has_ordering_method = class
|
||||
.iter_mro(self.db(), None)
|
||||
.filter_map(super::super::class_base::ClassBase::into_class)
|
||||
.filter(|base_class| {
|
||||
!base_class
|
||||
.class_literal(self.db())
|
||||
.0
|
||||
.is_known(self.db(), KnownClass::Object)
|
||||
})
|
||||
.any(|base_class| {
|
||||
base_class
|
||||
.class_literal(self.db())
|
||||
.0
|
||||
.has_own_ordering_method(self.db())
|
||||
});
|
||||
|
||||
if !has_ordering_method {
|
||||
// Find the @total_ordering decorator to report the diagnostic at its location
|
||||
if let Some(decorator) = class_node.decorator_list.iter().find(|decorator| {
|
||||
self.expression_type(&decorator.expression)
|
||||
.as_function_literal()
|
||||
.is_some_and(|function| {
|
||||
function.is_known(self.db(), KnownFunction::TotalOrdering)
|
||||
})
|
||||
}) {
|
||||
report_invalid_total_ordering(&self.context, class, decorator);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// (6) Check that the class's metaclass can be determined without error.
|
||||
if let Err(metaclass_error) = class.try_metaclass(self.db()) {
|
||||
match metaclass_error.reason() {
|
||||
MetaclassErrorKind::Cycle => {
|
||||
|
||||
Reference in New Issue
Block a user