mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-01 06:11:21 +00:00
[ty] equality narrowing on enums that don't override __eq__
or __ne__
(#20285)
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run
Add equality narrowing for enums, if they don't override `__eq__` or `__ne__` in an unsafe way. Follow-up to PR https://github.com/astral-sh/ruff/pull/20164 Fixes https://github.com/astral-sh/ty/issues/939
This commit is contained in:
parent
08a561fc05
commit
61f906d8e7
3 changed files with 50 additions and 38 deletions
|
@ -168,11 +168,9 @@ class Color(Enum):
|
||||||
|
|
||||||
def _(x: Color):
|
def _(x: Color):
|
||||||
if x in (Color.RED, Color.GREEN):
|
if x in (Color.RED, Color.GREEN):
|
||||||
# TODO should be `Literal[Color.RED, Color.GREEN]`
|
reveal_type(x) # revealed: Literal[Color.RED, Color.GREEN]
|
||||||
reveal_type(x) # revealed: Color
|
|
||||||
else:
|
else:
|
||||||
# TODO should be `Literal[Color.BLUE]`
|
reveal_type(x) # revealed: Literal[Color.BLUE]
|
||||||
reveal_type(x) # revealed: Color
|
|
||||||
```
|
```
|
||||||
|
|
||||||
## Union with enum and `int`
|
## Union with enum and `int`
|
||||||
|
@ -187,11 +185,9 @@ class Status(Enum):
|
||||||
|
|
||||||
def test(x: Status | int):
|
def test(x: Status | int):
|
||||||
if x in (Status.PENDING, Status.APPROVED):
|
if x in (Status.PENDING, Status.APPROVED):
|
||||||
# TODO should be `Literal[Status.PENDING, Status.APPROVED] | int`
|
|
||||||
# int is included because custom __eq__ methods could make
|
# int is included because custom __eq__ methods could make
|
||||||
# an int equal to Status.PENDING or Status.APPROVED, so we can't eliminate it
|
# an int equal to Status.PENDING or Status.APPROVED, so we can't eliminate it
|
||||||
reveal_type(x) # revealed: Status | int
|
reveal_type(x) # revealed: Literal[Status.PENDING, Status.APPROVED] | int
|
||||||
else:
|
else:
|
||||||
# TODO should be `Literal[Status.REJECTED] | int`
|
reveal_type(x) # revealed: Literal[Status.REJECTED] | int
|
||||||
reveal_type(x) # revealed: Status | int
|
|
||||||
```
|
```
|
||||||
|
|
|
@ -746,6 +746,35 @@ impl<'db> Type<'db> {
|
||||||
.is_some_and(|instance| instance.class(db).is_known(db, KnownClass::Bool))
|
.is_some_and(|instance| instance.class(db).is_known(db, KnownClass::Bool))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn is_enum(&self, db: &'db dyn Db) -> bool {
|
||||||
|
self.into_nominal_instance().is_some_and(|instance| {
|
||||||
|
crate::types::enums::enum_metadata(db, instance.class(db).class_literal(db).0).is_some()
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Return true if this type overrides __eq__ or __ne__ methods
|
||||||
|
fn overrides_equality(&self, db: &'db dyn Db) -> bool {
|
||||||
|
let check_dunder = |dunder_name, allowed_return_value| {
|
||||||
|
// Note that we do explicitly exclude dunder methods on `object`, `int` and `str` here.
|
||||||
|
// The reason for this is that we know that these dunder methods behave in a predictable way.
|
||||||
|
// Only custom dunder methods need to be examined here, as they might break single-valuedness
|
||||||
|
// by always returning `False`, for example.
|
||||||
|
let call_result = self.try_call_dunder_with_policy(
|
||||||
|
db,
|
||||||
|
dunder_name,
|
||||||
|
&mut CallArguments::positional([Type::unknown()]),
|
||||||
|
MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK
|
||||||
|
| MemberLookupPolicy::MRO_NO_INT_OR_STR_LOOKUP,
|
||||||
|
);
|
||||||
|
let call_result = call_result.as_ref();
|
||||||
|
call_result.is_ok_and(|bindings| {
|
||||||
|
bindings.return_type(db) == Type::BooleanLiteral(allowed_return_value)
|
||||||
|
}) || call_result.is_err_and(|err| matches!(err, CallDunderError::MethodNotAvailable))
|
||||||
|
};
|
||||||
|
|
||||||
|
!(check_dunder("__eq__", true) && check_dunder("__ne__", false))
|
||||||
|
}
|
||||||
|
|
||||||
pub(crate) fn is_notimplemented(&self, db: &'db dyn Db) -> bool {
|
pub(crate) fn is_notimplemented(&self, db: &'db dyn Db) -> bool {
|
||||||
self.into_nominal_instance().is_some_and(|instance| {
|
self.into_nominal_instance().is_some_and(|instance| {
|
||||||
instance
|
instance
|
||||||
|
@ -980,22 +1009,28 @@ impl<'db> Type<'db> {
|
||||||
|
|
||||||
pub(crate) fn is_union_of_single_valued(&self, db: &'db dyn Db) -> bool {
|
pub(crate) fn is_union_of_single_valued(&self, db: &'db dyn Db) -> bool {
|
||||||
self.into_union().is_some_and(|union| {
|
self.into_union().is_some_and(|union| {
|
||||||
union
|
union.elements(db).iter().all(|ty| {
|
||||||
.elements(db)
|
ty.is_single_valued(db)
|
||||||
.iter()
|
|| ty.is_bool(db)
|
||||||
.all(|ty| ty.is_single_valued(db) || ty.is_bool(db) || ty.is_literal_string())
|
|| ty.is_literal_string()
|
||||||
|
|| (ty.is_enum(db) && !ty.overrides_equality(db))
|
||||||
|
})
|
||||||
}) || self.is_bool(db)
|
}) || self.is_bool(db)
|
||||||
|| self.is_literal_string()
|
|| self.is_literal_string()
|
||||||
|
|| (self.is_enum(db) && !self.overrides_equality(db))
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn is_union_with_single_valued(&self, db: &'db dyn Db) -> bool {
|
pub(crate) fn is_union_with_single_valued(&self, db: &'db dyn Db) -> bool {
|
||||||
self.into_union().is_some_and(|union| {
|
self.into_union().is_some_and(|union| {
|
||||||
union
|
union.elements(db).iter().any(|ty| {
|
||||||
.elements(db)
|
ty.is_single_valued(db)
|
||||||
.iter()
|
|| ty.is_bool(db)
|
||||||
.any(|ty| ty.is_single_valued(db) || ty.is_bool(db) || ty.is_literal_string())
|
|| ty.is_literal_string()
|
||||||
|
|| (ty.is_enum(db) && !ty.overrides_equality(db))
|
||||||
|
})
|
||||||
}) || self.is_bool(db)
|
}) || self.is_bool(db)
|
||||||
|| self.is_literal_string()
|
|| self.is_literal_string()
|
||||||
|
|| (self.is_enum(db) && !self.overrides_equality(db))
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn into_string_literal(self) -> Option<StringLiteralType<'db>> {
|
pub(crate) fn into_string_literal(self) -> Option<StringLiteralType<'db>> {
|
||||||
|
@ -2574,28 +2609,7 @@ impl<'db> Type<'db> {
|
||||||
| Type::SpecialForm(..)
|
| Type::SpecialForm(..)
|
||||||
| Type::KnownInstance(..) => true,
|
| Type::KnownInstance(..) => true,
|
||||||
|
|
||||||
Type::EnumLiteral(_) => {
|
Type::EnumLiteral(_) => !self.overrides_equality(db),
|
||||||
let check_dunder = |dunder_name, allowed_return_value| {
|
|
||||||
// Note that we do explicitly exclude dunder methods on `object`, `int` and `str` here.
|
|
||||||
// The reason for this is that we know that these dunder methods behave in a predictable way.
|
|
||||||
// Only custom dunder methods need to be examined here, as they might break single-valuedness
|
|
||||||
// by always returning `False`, for example.
|
|
||||||
let call_result = self.try_call_dunder_with_policy(
|
|
||||||
db,
|
|
||||||
dunder_name,
|
|
||||||
&mut CallArguments::positional([Type::unknown()]),
|
|
||||||
MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK
|
|
||||||
| MemberLookupPolicy::MRO_NO_INT_OR_STR_LOOKUP,
|
|
||||||
);
|
|
||||||
let call_result = call_result.as_ref();
|
|
||||||
call_result.is_ok_and(|bindings| {
|
|
||||||
bindings.return_type(db) == Type::BooleanLiteral(allowed_return_value)
|
|
||||||
}) || call_result
|
|
||||||
.is_err_and(|err| matches!(err, CallDunderError::MethodNotAvailable))
|
|
||||||
};
|
|
||||||
|
|
||||||
check_dunder("__eq__", true) && check_dunder("__ne__", false)
|
|
||||||
}
|
|
||||||
|
|
||||||
Type::ProtocolInstance(..) => {
|
Type::ProtocolInstance(..) => {
|
||||||
// See comment in the `Type::ProtocolInstance` branch for `Type::is_singleton`.
|
// See comment in the `Type::ProtocolInstance` branch for `Type::is_singleton`.
|
||||||
|
|
|
@ -640,6 +640,7 @@ impl<'db, 'ast> NarrowingConstraintsBuilder<'db, 'ast> {
|
||||||
if !element.is_single_valued(self.db)
|
if !element.is_single_valued(self.db)
|
||||||
&& !element.is_literal_string()
|
&& !element.is_literal_string()
|
||||||
&& !element.is_bool(self.db)
|
&& !element.is_bool(self.db)
|
||||||
|
&& (!element.is_enum(self.db) || element.overrides_equality(self.db))
|
||||||
{
|
{
|
||||||
builder = builder.add(*element);
|
builder = builder.add(*element);
|
||||||
}
|
}
|
||||||
|
@ -675,6 +676,7 @@ impl<'db, 'ast> NarrowingConstraintsBuilder<'db, 'ast> {
|
||||||
if element.is_single_valued(self.db)
|
if element.is_single_valued(self.db)
|
||||||
|| element.is_literal_string()
|
|| element.is_literal_string()
|
||||||
|| element.is_bool(self.db)
|
|| element.is_bool(self.db)
|
||||||
|
|| (element.is_enum(self.db) && !element.overrides_equality(self.db))
|
||||||
{
|
{
|
||||||
single_builder = single_builder.add(*element);
|
single_builder = single_builder.add(*element);
|
||||||
} else {
|
} else {
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue