[ty] Reachability analysis for isinstance(…) branches (#19503)
Some checks are pending
CI / mkdocs (push) Waiting to run
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 / formatter instabilities and black similarity (push) Blocked by required conditions
CI / cargo build (msrv) (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / cargo fuzz build (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 / pre-commit (push) Waiting to run
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

## Summary

Add more precise type inference for a limited set of `isinstance(…)`
calls, i.e. return `Literal[True]` if we can be sure that this is the
correct result. This improves exhaustiveness checking / reachability
analysis for if-elif-else chains with `isinstance` checks. For example:

```py
def is_number(x: int | str) -> bool:  # no "can implicitly return `None` error here anymore
    if isinstance(x, int):
        return True
    elif isinstance(x, str):
        return False

    # code here is now detected as being unreachable
```

This PR also adds a new test suite for exhaustiveness checking.

## Test Plan

New Markdown tests

### Ecosystem analysis

The removed diagnostics look good. There's [one
case](f52c4f1afd/torchvision/io/video_reader.py (L125-L143))
where a "true positive" is removed in unreachable code. `src` is
annotated as being of type `str`, but there is an `elif isinstance(src,
bytes)` branch, which we now detect as unreachable. And so the
diagnostic inside that branch is silenced. I don't think this is a
problem, especially once we have a "graying out" feature, or a lint that
warns about unreachable code.
This commit is contained in:
David Peter 2025-07-23 13:06:30 +02:00 committed by GitHub
parent b605c3e232
commit 905b9d7f51
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 404 additions and 14 deletions

View file

@ -76,8 +76,9 @@ use crate::types::narrow::ClassInfoConstraintFunction;
use crate::types::signatures::{CallableSignature, Signature};
use crate::types::visitor::any_over_type;
use crate::types::{
BoundMethodType, CallableType, DeprecatedInstance, DynamicType, KnownClass, Type, TypeMapping,
TypeRelation, TypeTransformer, TypeVarInstance, UnionBuilder, walk_type_mapping,
BoundMethodType, CallableType, ClassLiteral, ClassType, DeprecatedInstance, DynamicType,
KnownClass, Truthiness, Type, TypeMapping, TypeRelation, TypeTransformer, TypeVarInstance,
UnionBuilder, walk_type_mapping,
};
use crate::{Db, FxOrderSet, ModuleName, resolve_module};
@ -882,6 +883,96 @@ impl<'db> FunctionType<'db> {
}
}
/// Evaluate an `isinstance` call. Return `Truthiness::AlwaysTrue` if we can definitely infer that
/// this will return `True` at runtime, `Truthiness::AlwaysFalse` if we can definitely infer
/// that this will return `False` at runtime, or `Truthiness::Ambiguous` if we should infer `bool`
/// instead.
fn is_instance_truthiness<'db>(
db: &'db dyn Db,
ty: Type<'db>,
class: ClassLiteral<'db>,
) -> Truthiness {
let is_instance = |ty: &Type<'_>| {
if let Type::NominalInstance(instance) = ty {
if instance
.class
.is_subclass_of(db, ClassType::NonGeneric(class))
{
return true;
}
}
false
};
let always_true_if = |test: bool| {
if test {
Truthiness::AlwaysTrue
} else {
Truthiness::Ambiguous
}
};
match ty {
Type::Union(..) => {
// We do not handle unions specifically here, because something like `A | SubclassOfA` would
// have been simplified to `A` anyway
Truthiness::Ambiguous
}
Type::Intersection(intersection) => always_true_if(
intersection
.positive(db)
.iter()
.any(|element| is_instance_truthiness(db, *element, class).is_always_true()),
),
Type::NominalInstance(..) => always_true_if(is_instance(&ty)),
Type::BooleanLiteral(..)
| Type::BytesLiteral(..)
| Type::IntLiteral(..)
| Type::StringLiteral(..)
| Type::LiteralString
| Type::ModuleLiteral(..)
| Type::EnumLiteral(..) => always_true_if(
ty.literal_fallback_instance(db)
.as_ref()
.is_some_and(is_instance),
),
Type::Tuple(..) => always_true_if(class.is_known(db, KnownClass::Tuple)),
Type::FunctionLiteral(..) => {
always_true_if(is_instance(&KnownClass::FunctionType.to_instance(db)))
}
Type::ClassLiteral(..) => always_true_if(is_instance(&KnownClass::Type.to_instance(db))),
Type::BoundMethod(..)
| Type::MethodWrapper(..)
| Type::WrapperDescriptor(..)
| Type::DataclassDecorator(..)
| Type::DataclassTransformer(..)
| Type::GenericAlias(..)
| Type::SubclassOf(..)
| Type::ProtocolInstance(..)
| Type::SpecialForm(..)
| Type::KnownInstance(..)
| Type::PropertyInstance(..)
| Type::AlwaysTruthy
| Type::AlwaysFalsy
| Type::TypeVar(..)
| Type::BoundSuper(..)
| Type::TypeIs(..)
| Type::Callable(..)
| Type::Dynamic(..)
| Type::Never => {
// We could probably try to infer more precise types in some of these cases, but it's unclear
// if it's worth the effort.
Truthiness::Ambiguous
}
}
}
fn signature_cycle_recover<'db>(
_db: &'db dyn Db,
_value: &CallableSignature<'db>,
@ -1228,21 +1319,26 @@ impl KnownFunction {
}
KnownFunction::IsInstance | KnownFunction::IsSubclass => {
let [_, Some(Type::ClassLiteral(class))] = parameter_types else {
let [Some(first_arg), Some(Type::ClassLiteral(class))] = parameter_types else {
return;
};
let Some(protocol_class) = class.into_protocol_class(db) else {
return;
};
if protocol_class.is_runtime_checkable(db) {
return;
if let Some(protocol_class) = class.into_protocol_class(db) {
if !protocol_class.is_runtime_checkable(db) {
report_runtime_check_against_non_runtime_checkable_protocol(
context,
call_expression,
protocol_class,
self,
);
}
}
if self == KnownFunction::IsInstance {
overload.set_return_type(
is_instance_truthiness(db, *first_arg, *class).into_type(db),
);
}
report_runtime_check_against_non_runtime_checkable_protocol(
context,
call_expression,
protocol_class,
self,
);
}
known @ (KnownFunction::DunderImport | KnownFunction::ImportModule) => {