Skip to content

Commit

Permalink
Narrow on isinstance calls
Browse files Browse the repository at this point in the history
Summary: Narrows the type of the first argument to an `isinstance` call. Defers the decision of whether or not to narrow on a function call until the answers phase, so we have the opportunity to use type information in the decision. For now, this diff just checks whether the name of the function is `isinstance`, but I'll make the check smarter in a follow-up.

Reviewed By: stroxler

Differential Revision: D68118407

fbshipit-source-id: 54b9b8ba05ddc3cb6e8b2c3b89046cc69fdd4b2f
  • Loading branch information
rchen152 authored and facebook-github-bot committed Jan 14, 2025
1 parent 0e652f0 commit a4302bd
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 16 deletions.
20 changes: 10 additions & 10 deletions pyre2/conformance/third_party/conformance.exp
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,16 @@
"stop_column": 30,
"stop_line": 23
},
{
"code": -2,
"column": 22,
"concise_description": "Expected type form, got ScopedTypeAlias[GoodAlias1, type[int]]",
"description": "Expected type form, got ScopedTypeAlias[GoodAlias1, type[int]]",
"line": 31,
"name": "PyreError",
"stop_column": 32,
"stop_line": 31
},
{
"code": -2,
"column": 27,
Expand Down Expand Up @@ -19484,16 +19494,6 @@
"name": "PyreError",
"stop_column": 16,
"stop_line": 13
},
{
"code": -2,
"column": 9,
"concise_description": "Object of class `float` has no attribute `numerator`",
"description": "Object of class `float` has no attribute `numerator`",
"line": 16,
"name": "PyreError",
"stop_column": 20,
"stop_line": 16
}
],
"specialtypes_type.py": [
Expand Down
4 changes: 1 addition & 3 deletions pyre2/conformance/third_party/conformance.result
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
],
"aliases_type_statement.py": [
"Line 26: Expected 1 errors",
"Line 31: Expected 1 errors",
"Line 42: Expected 1 errors",
"Line 44: Expected 1 errors",
"Line 48: Expected 1 errors",
Expand Down Expand Up @@ -1411,8 +1410,7 @@
"specialtypes_promotions.py": [
"Line 7: Unexpected errors ['EXPECTED Literal[1] <: float']",
"Line 8: Unexpected errors ['EXPECTED Literal[1.2] <: complex']",
"Line 9: Unexpected errors ['EXPECTED Literal[1] <: complex']",
"Line 16: Unexpected errors ['Object of class `float` has no attribute `numerator`']"
"Line 9: Unexpected errors ['EXPECTED Literal[1] <: complex']"
],
"specialtypes_type.py": [
"Line 70: Expected 1 errors",
Expand Down
6 changes: 3 additions & 3 deletions pyre2/conformance/third_party/results.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"pass": 9,
"fail": 124,
"pass_rate": 0.07,
"differences": 1419,
"differences": 1417,
"passing": [
"annotations_coroutines.py",
"directives_no_type_check.py",
Expand All @@ -20,7 +20,7 @@
"aliases_implicit.py": 22,
"aliases_newtype.py": 10,
"aliases_recursive.py": 19,
"aliases_type_statement.py": 16,
"aliases_type_statement.py": 15,
"aliases_typealiastype.py": 25,
"aliases_variance.py": 3,
"annotations_forward_refs.py": 10,
Expand Down Expand Up @@ -123,7 +123,7 @@
"specialtypes_any.py": 8,
"specialtypes_never.py": 5,
"specialtypes_none.py": 3,
"specialtypes_promotions.py": 4,
"specialtypes_promotions.py": 3,
"specialtypes_type.py": 31,
"tuples_type_compat.py": 34,
"tuples_unpacked.py": 14,
Expand Down
21 changes: 21 additions & 0 deletions pyre2/pyre2/bin/alt/answers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::sync::Arc;

use dupe::Dupe;
use ruff_python_ast::name::Name;
use ruff_python_ast::Arguments;
use ruff_python_ast::Expr;
use ruff_python_ast::Keyword;
use ruff_python_ast::TypeParam;
Expand Down Expand Up @@ -1064,6 +1065,15 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
})
}

fn resolve_narrowing_call(&self, func: &Expr, args: &Arguments) -> Option<NarrowOp> {
// TODO: matching on the name is a bad way to detect an `isinstance` call.
if matches!(func, Expr::Name(name) if name.id == "isinstance") && args.args.len() > 1 {
Some(NarrowOp::IsInstance(Box::new(args.args[1].clone())))
} else {
None
}
}

fn narrow(&self, ty: &Type, op: &NarrowOp) -> Type {
match op {
NarrowOp::Is(e) => {
Expand Down Expand Up @@ -1171,6 +1181,17 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
}
}
NarrowOp::Or(ops) => self.unions(&ops.map(|op| self.narrow(ty, op))),
NarrowOp::Call(func, args) | NarrowOp::NotCall(func, args) => {
if let Some(resolved_op) = self.resolve_narrowing_call(func, args) {
if matches!(op, NarrowOp::Call(..)) {
self.narrow(ty, &resolved_op)
} else {
self.narrow(ty, &resolved_op.negate())
}
} else {
ty.clone()
}
}
}
}

Expand Down
25 changes: 25 additions & 0 deletions pyre2/pyre2/bin/binding/narrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
*/

use ruff_python_ast::name::Name;
use ruff_python_ast::Arguments;
use ruff_python_ast::BoolOp;
use ruff_python_ast::CmpOp;
use ruff_python_ast::Expr;
use ruff_python_ast::ExprBoolOp;
use ruff_python_ast::ExprCall;
use ruff_python_ast::ExprCompare;
use ruff_python_ast::ExprName;
use ruff_python_ast::ExprNamed;
Expand All @@ -34,6 +36,9 @@ pub enum NarrowOp {
Or(Vec<NarrowOp>),
IsInstance(Box<Expr>),
IsNotInstance(Box<Expr>),
/// (func, args) for a function call that may narrow the type of its first argument.
Call(Box<Expr>, Arguments),
NotCall(Box<Expr>, Arguments),
}

impl NarrowOp {
Expand All @@ -49,6 +54,8 @@ impl NarrowOp {
Self::Falsy => Self::Truthy,
Self::And(ops) => Self::Or(ops.map(|op| op.negate())),
Self::Or(ops) => Self::And(ops.map(|op| op.negate())),
Self::Call(f, args) => Self::NotCall(f.clone(), args.clone()),
Self::NotCall(f, args) => Self::Call(f.clone(), args.clone()),
}
}

Expand Down Expand Up @@ -182,6 +189,24 @@ impl NarrowOps {
op: UnaryOp::Not,
operand: box e,
})) => Self::from_expr(Some(e)).negate(),
Some(Expr::Call(ExprCall {
range,
func,
arguments:
ref args @ Arguments {
range: _,
args: ref posargs,
keywords: _,
},
})) if !posargs.is_empty() => {
// This may be a function call that narrows the type of its first argument. Record
// it as a possible narrowing operation that we'll resolve in the answers phase.
let mut narrow_ops = Self::new();
for name in expr_to_names(&posargs[0]) {
narrow_ops.and(name.id, NarrowOp::Call(func.clone(), args.clone()), range);
}
narrow_ops
}
Some(e) => {
let mut narrow_ops = Self::new();
for name in expr_to_names(&e) {
Expand Down
12 changes: 12 additions & 0 deletions pyre2/pyre2/bin/test/narrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,3 +431,15 @@ def f(x: Literal[E.X], y: E):
assert_type(x, Literal[E.X])
"#,
);

testcase!(
test_isinstance,
r#"
from typing import assert_type
def f(x: str | int):
if isinstance(x, str):
assert_type(x, str)
else:
assert_type(x, int)
"#,
);
2 changes: 2 additions & 0 deletions pyre2/pyre2/bin/test/stdlib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ class type:
# TODO: overload for slice, tuple should be Sequence[T]
class tuple[T](Iterable[T]):
def __getitem__(self, index: int) -> T: ...
def isinstance(obj: object, class_or_tuple: object, /) -> bool: ...
"#;

static TYPING: &str = r#"
Expand Down

0 comments on commit a4302bd

Please sign in to comment.