[ty] Temporary hack to reduce false positives around builtins.open() (#20367)

## Summary

https://github.com/astral-sh/ruff/pull/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
This commit is contained in:
Alex Waygood 2025-09-12 22:20:38 +01:00 committed by GitHub
parent 98708976e4
commit 1745554809
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 93 additions and 1 deletions

View file

@ -162,3 +162,22 @@ def _(x: A | B, y: list[int]):
reveal_type(x) # revealed: B & ~A reveal_type(x) # revealed: B & ~A
reveal_type(isinstance(x, B)) # revealed: Literal[True] reveal_type(isinstance(x, B)) # revealed: Literal[True]
``` ```
## Calls to `open()`
We do not fully understand typeshed's overloads for `open()` yet, due to missing support for PEP-613
type aliases. However, we also do not emit false-positive diagnostics on common calls to `open()`:
```py
import pickle
reveal_type(open("")) # revealed: TextIOWrapper[_WrappedBuffer]
reveal_type(open("", "r")) # revealed: TextIOWrapper[_WrappedBuffer]
reveal_type(open("", "rb")) # revealed: @Todo(`builtins.open` return type)
with open("foo.pickle", "rb") as f:
x = pickle.load(f) # fine
def _(mode: str):
reveal_type(open("", mode)) # revealed: @Todo(`builtins.open` return type)
```

View file

@ -81,7 +81,7 @@ use crate::types::{
DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor, HasRelationToVisitor, DeprecatedInstance, DynamicType, FindLegacyTypeVarsVisitor, HasRelationToVisitor,
IsEquivalentVisitor, KnownClass, KnownInstanceType, NormalizedVisitor, SpecialFormType, IsEquivalentVisitor, KnownClass, KnownInstanceType, NormalizedVisitor, SpecialFormType,
TrackedConstraintSet, Truthiness, Type, TypeMapping, TypeRelation, UnionBuilder, all_members, TrackedConstraintSet, Truthiness, Type, TypeMapping, TypeRelation, UnionBuilder, all_members,
binding_type, walk_type_mapping, binding_type, todo_type, walk_type_mapping,
}; };
use crate::{Db, FxOrderSet, ModuleName, resolve_module}; use crate::{Db, FxOrderSet, ModuleName, resolve_module};
@ -1191,6 +1191,7 @@ pub enum KnownFunction {
DunderImport, DunderImport,
/// `importlib.import_module`, which returns the submodule. /// `importlib.import_module`, which returns the submodule.
ImportModule, ImportModule,
Open,
/// `typing(_extensions).final` /// `typing(_extensions).final`
Final, Final,
@ -1292,6 +1293,7 @@ impl KnownFunction {
| Self::HasAttr | Self::HasAttr
| Self::Len | Self::Len
| Self::Repr | Self::Repr
| Self::Open
| Self::DunderImport => module.is_builtins(), | Self::DunderImport => module.is_builtins(),
Self::AssertType Self::AssertType
| Self::AssertNever | Self::AssertNever
@ -1703,6 +1705,76 @@ impl KnownFunction {
))); )));
} }
KnownFunction::Open => {
// Temporary special-casing for `builtins.open` to avoid an excessive number of false positives
// in lieu of proper support for PEP-614 type aliases.
if let [_, Some(mode), ..] = parameter_types {
// Infer `Todo` for any argument that doesn't match typeshed's
// `OpenTextMode` type alias (<https://github.com/python/typeshed/blob/6937a9b193bfc2f0696452d58aad96d7627aa29a/stdlib/_typeshed/__init__.pyi#L220>).
// Without this special-casing, we'd just always select the first overload in our current state,
// which leads to lots of false positives.
if mode.into_string_literal().is_none_or(|mode| {
!matches!(
mode.value(db),
"r+" | "+r"
| "rt+"
| "r+t"
| "+rt"
| "tr+"
| "t+r"
| "+tr"
| "w+"
| "+w"
| "wt+"
| "w+t"
| "+wt"
| "tw+"
| "t+w"
| "+tw"
| "a+"
| "+a"
| "at+"
| "a+t"
| "+at"
| "ta+"
| "t+a"
| "+ta"
| "x+"
| "+x"
| "xt+"
| "x+t"
| "+xt"
| "tx+"
| "t+x"
| "+tx"
| "w"
| "wt"
| "tw"
| "a"
| "at"
| "ta"
| "x"
| "xt"
| "tx"
| "r"
| "rt"
| "tr"
| "U"
| "rU"
| "Ur"
| "rtU"
| "rUt"
| "Urt"
| "trU"
| "tUr"
| "Utr"
)
}) {
overload.set_return_type(todo_type!("`builtins.open` return type"));
}
}
}
_ => {} _ => {}
} }
} }
@ -1729,6 +1801,7 @@ pub(crate) mod tests {
| KnownFunction::IsInstance | KnownFunction::IsInstance
| KnownFunction::HasAttr | KnownFunction::HasAttr
| KnownFunction::IsSubclass | KnownFunction::IsSubclass
| KnownFunction::Open
| KnownFunction::DunderImport => KnownModule::Builtins, | KnownFunction::DunderImport => KnownModule::Builtins,
KnownFunction::AbstractMethod => KnownModule::Abc, KnownFunction::AbstractMethod => KnownModule::Abc,