Skip to content

Commit 1745554

Browse files
authored
[ty] Temporary hack to reduce false positives around builtins.open() (#20367)
## Summary #20165 added a lot of false positives around calls to `builtins.open()`, because our missing support for PEP-613 type aliases means that we don't understand typeshed's overloads for `builtins.open()` at all yet, and therefore always select the first overload. This didn't use to matter very much, but now that we have a much stricter implementation of protocol assignability/subtyping it matters a lot, because most of the stdlib functions dealing with I/O (`pickle`, `marshal`, `io`, `json`, etc.) are annotated in typeshed as taking in protocols of some kind. In lieu of full PEP-613 support, which is blocked on various things and might not land in time for our next alpha release, this PR adds some temporary special-casing for `builtins.open()` to avoid the false positives. We just infer `Todo` for anything that isn't meant to match typeshed's first `open()` overload. This should be easy to rip out again once we have proper support for PEP-613 type aliases, which hopefully should be pretty soon! ## Test Plan Added an mdtest
1 parent 9870897 commit 1745554

File tree

2 files changed

+93
-1
lines changed

2 files changed

+93
-1
lines changed

crates/ty_python_semantic/resources/mdtest/call/builtins.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,3 +162,22 @@ def _(x: A | B, y: list[int]):
162162
reveal_type(x) # revealed: B & ~A
163163
reveal_type(isinstance(x, B)) # revealed: Literal[True]
164164
```
165+
166+
## Calls to `open()`
167+
168+
We do not fully understand typeshed's overloads for `open()` yet, due to missing support for PEP-613
169+
type aliases. However, we also do not emit false-positive diagnostics on common calls to `open()`:
170+
171+
```py
172+
import pickle
173+
174+
reveal_type(open("")) # revealed: TextIOWrapper[_WrappedBuffer]
175+
reveal_type(open("", "r")) # revealed: TextIOWrapper[_WrappedBuffer]
176+
reveal_type(open("", "rb")) # revealed: @Todo(`builtins.open` return type)
177+
178+
with open("foo.pickle", "rb") as f:
179+
x = pickle.load(f) # fine
180+
181+
def _(mode: str):
182+
reveal_type(open("", mode)) # revealed: @Todo(`builtins.open` return type)
183+
```

crates/ty_python_semantic/src/types/function.rs

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ use crate::types::{
8181
DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor, HasRelationToVisitor,
8282
IsEquivalentVisitor, KnownClass, KnownInstanceType, NormalizedVisitor, SpecialFormType,
8383
TrackedConstraintSet, Truthiness, Type, TypeMapping, TypeRelation, UnionBuilder, all_members,
84-
binding_type, walk_type_mapping,
84+
binding_type, todo_type, walk_type_mapping,
8585
};
8686
use crate::{Db, FxOrderSet, ModuleName, resolve_module};
8787

@@ -1191,6 +1191,7 @@ pub enum KnownFunction {
11911191
DunderImport,
11921192
/// `importlib.import_module`, which returns the submodule.
11931193
ImportModule,
1194+
Open,
11941195

11951196
/// `typing(_extensions).final`
11961197
Final,
@@ -1292,6 +1293,7 @@ impl KnownFunction {
12921293
| Self::HasAttr
12931294
| Self::Len
12941295
| Self::Repr
1296+
| Self::Open
12951297
| Self::DunderImport => module.is_builtins(),
12961298
Self::AssertType
12971299
| Self::AssertNever
@@ -1703,6 +1705,76 @@ impl KnownFunction {
17031705
)));
17041706
}
17051707

1708+
KnownFunction::Open => {
1709+
// Temporary special-casing for `builtins.open` to avoid an excessive number of false positives
1710+
// in lieu of proper support for PEP-614 type aliases.
1711+
if let [_, Some(mode), ..] = parameter_types {
1712+
// Infer `Todo` for any argument that doesn't match typeshed's
1713+
// `OpenTextMode` type alias (<https://github.com/python/typeshed/blob/6937a9b193bfc2f0696452d58aad96d7627aa29a/stdlib/_typeshed/__init__.pyi#L220>).
1714+
// Without this special-casing, we'd just always select the first overload in our current state,
1715+
// which leads to lots of false positives.
1716+
if mode.into_string_literal().is_none_or(|mode| {
1717+
!matches!(
1718+
mode.value(db),
1719+
"r+" | "+r"
1720+
| "rt+"
1721+
| "r+t"
1722+
| "+rt"
1723+
| "tr+"
1724+
| "t+r"
1725+
| "+tr"
1726+
| "w+"
1727+
| "+w"
1728+
| "wt+"
1729+
| "w+t"
1730+
| "+wt"
1731+
| "tw+"
1732+
| "t+w"
1733+
| "+tw"
1734+
| "a+"
1735+
| "+a"
1736+
| "at+"
1737+
| "a+t"
1738+
| "+at"
1739+
| "ta+"
1740+
| "t+a"
1741+
| "+ta"
1742+
| "x+"
1743+
| "+x"
1744+
| "xt+"
1745+
| "x+t"
1746+
| "+xt"
1747+
| "tx+"
1748+
| "t+x"
1749+
| "+tx"
1750+
| "w"
1751+
| "wt"
1752+
| "tw"
1753+
| "a"
1754+
| "at"
1755+
| "ta"
1756+
| "x"
1757+
| "xt"
1758+
| "tx"
1759+
| "r"
1760+
| "rt"
1761+
| "tr"
1762+
| "U"
1763+
| "rU"
1764+
| "Ur"
1765+
| "rtU"
1766+
| "rUt"
1767+
| "Urt"
1768+
| "trU"
1769+
| "tUr"
1770+
| "Utr"
1771+
)
1772+
}) {
1773+
overload.set_return_type(todo_type!("`builtins.open` return type"));
1774+
}
1775+
}
1776+
}
1777+
17061778
_ => {}
17071779
}
17081780
}
@@ -1729,6 +1801,7 @@ pub(crate) mod tests {
17291801
| KnownFunction::IsInstance
17301802
| KnownFunction::HasAttr
17311803
| KnownFunction::IsSubclass
1804+
| KnownFunction::Open
17321805
| KnownFunction::DunderImport => KnownModule::Builtins,
17331806

17341807
KnownFunction::AbstractMethod => KnownModule::Abc,

0 commit comments

Comments
 (0)