From a4302bda7d5ce4af3d7233eecb174f425c5a5689 Mon Sep 17 00:00:00 2001 From: "Rebecca Chen (Python)" Date: Mon, 13 Jan 2025 16:03:11 -0800 Subject: [PATCH] Narrow on `isinstance` calls 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 --- pyre2/conformance/third_party/conformance.exp | 20 +++++++-------- .../third_party/conformance.result | 4 +-- pyre2/conformance/third_party/results.json | 6 ++--- pyre2/pyre2/bin/alt/answers.rs | 21 ++++++++++++++++ pyre2/pyre2/bin/binding/narrow.rs | 25 +++++++++++++++++++ pyre2/pyre2/bin/test/narrow.rs | 12 +++++++++ pyre2/pyre2/bin/test/stdlib.rs | 2 ++ 7 files changed, 74 insertions(+), 16 deletions(-) diff --git a/pyre2/conformance/third_party/conformance.exp b/pyre2/conformance/third_party/conformance.exp index 6173a234141..c358f597d88 100644 --- a/pyre2/conformance/third_party/conformance.exp +++ b/pyre2/conformance/third_party/conformance.exp @@ -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, @@ -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": [ diff --git a/pyre2/conformance/third_party/conformance.result b/pyre2/conformance/third_party/conformance.result index de4c81c72bc..b212f29cf25 100644 --- a/pyre2/conformance/third_party/conformance.result +++ b/pyre2/conformance/third_party/conformance.result @@ -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", @@ -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", diff --git a/pyre2/conformance/third_party/results.json b/pyre2/conformance/third_party/results.json index 078aa618b92..aba4044c520 100644 --- a/pyre2/conformance/third_party/results.json +++ b/pyre2/conformance/third_party/results.json @@ -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", @@ -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, @@ -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, diff --git a/pyre2/pyre2/bin/alt/answers.rs b/pyre2/pyre2/bin/alt/answers.rs index 683f0bf2e58..70e458a4931 100644 --- a/pyre2/pyre2/bin/alt/answers.rs +++ b/pyre2/pyre2/bin/alt/answers.rs @@ -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; @@ -1064,6 +1065,15 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { }) } + fn resolve_narrowing_call(&self, func: &Expr, args: &Arguments) -> Option { + // 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) => { @@ -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() + } + } } } diff --git a/pyre2/pyre2/bin/binding/narrow.rs b/pyre2/pyre2/bin/binding/narrow.rs index 9fc86686e04..1f6bc783246 100644 --- a/pyre2/pyre2/bin/binding/narrow.rs +++ b/pyre2/pyre2/bin/binding/narrow.rs @@ -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; @@ -34,6 +36,9 @@ pub enum NarrowOp { Or(Vec), IsInstance(Box), IsNotInstance(Box), + /// (func, args) for a function call that may narrow the type of its first argument. + Call(Box, Arguments), + NotCall(Box, Arguments), } impl NarrowOp { @@ -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()), } } @@ -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) { diff --git a/pyre2/pyre2/bin/test/narrow.rs b/pyre2/pyre2/bin/test/narrow.rs index d30eadfd979..1f2aa41e991 100644 --- a/pyre2/pyre2/bin/test/narrow.rs +++ b/pyre2/pyre2/bin/test/narrow.rs @@ -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) + "#, +); diff --git a/pyre2/pyre2/bin/test/stdlib.rs b/pyre2/pyre2/bin/test/stdlib.rs index 63743dd60d2..630791320ea 100644 --- a/pyre2/pyre2/bin/test/stdlib.rs +++ b/pyre2/pyre2/bin/test/stdlib.rs @@ -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#"