mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-29 13:24:57 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[ty] Todo-types for os.fdopen, NamedTemporaryFile, and Path.open (#20549)

Browse source 
## Summary

This applies the trick that we use for `builtins.open` to similar
functions that have the same problem. The reason is that the problem
would otherwise become even more pronounced once we add understanding of
the implicit type of `self` parameters, because then something like
`(base_path / "test.bin").open("rb")` also leads to a wrong return type
and can result in false positives.

## Test Plan

New Markdown tests
This commit is contained in:
David Peter 2025-09-24 15:43:58 +02:00 • committed by GitHub
parent eea87e24e3
commit fcc76bb7b2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 240 additions and 100 deletions
Download patch file Download diff file Expand all files Collapse all files

19
crates/ty_python_semantic/resources/mdtest/call/builtins.md
View file

@ -162,22 +162,3 @@ def _(x: A | B, y: list[int]):
reveal_type(x) # revealed: B & ~A
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)
```

68
crates/ty_python_semantic/resources/mdtest/call/open.md Normal file
View file

@ -0,0 +1,68 @@
# Calls to `open()`
## `builtins.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)
```
## `os.fdopen`
The same is true for `os.fdopen()`:
```py
import pickle
import os
reveal_type(os.fdopen(0)) # revealed: TextIOWrapper[_WrappedBuffer]
reveal_type(os.fdopen(0, "r")) # revealed: TextIOWrapper[_WrappedBuffer]
reveal_type(os.fdopen(0, "rb")) # revealed: @Todo(`os.fdopen` return type)
with os.fdopen(0, "rb") as f:
x = pickle.load(f) # fine
```
## `Path.open`
And similarly for `Path.open()`:
```py
from pathlib import Path
import pickle
reveal_type(Path("").open()) # revealed: @Todo(`Path.open` return type)
reveal_type(Path("").open("r")) # revealed: @Todo(`Path.open` return type)
reveal_type(Path("").open("rb")) # revealed: @Todo(`Path.open` return type)
with Path("foo.pickle").open("rb") as f:
x = pickle.load(f) # fine
```
## `NamedTemporaryFile`
And similarly for `tempfile.NamedTemporaryFile()`:
```py
from tempfile import NamedTemporaryFile
import pickle
reveal_type(NamedTemporaryFile()) # revealed: _TemporaryFileWrapper[bytes]
reveal_type(NamedTemporaryFile("r")) # revealed: _TemporaryFileWrapper[str]
reveal_type(NamedTemporaryFile("rb")) # revealed: @Todo(`tempfile.NamedTemporaryFile` return type)
with NamedTemporaryFile("rb") as f:
x = pickle.load(f) # fine
```

6
crates/ty_python_semantic/src/module_resolver/module.rs
View file

@ -318,6 +318,9 @@ pub enum KnownModule {
TypingExtensions,
Typing,
Sys,
Os,
Tempfile,
Pathlib,
Abc,
Dataclasses,
Collections,
@ -347,6 +350,9 @@ impl KnownModule {
Self::Typeshed => "_typeshed",
Self::TypingExtensions => "typing_extensions",
Self::Sys => "sys",
Self::Os => "os",
Self::Tempfile => "tempfile",
Self::Pathlib => "pathlib",
Self::Abc => "abc",
Self::Dataclasses => "dataclasses",
Self::Collections => "collections",

40
crates/ty_python_semantic/src/types.rs
View file

@ -3447,6 +3447,11 @@ impl<'db> Type<'db> {
Type::KnownBoundMethod(KnownBoundMethodType::StrStartswith(literal)),
)
.into(),
Type::NominalInstance(instance)
if instance.has_known_class(db, KnownClass::Path) && name == "open" =>
{
Place::bound(Type::KnownBoundMethod(KnownBoundMethodType::PathOpen)).into()
}
Type::ClassLiteral(class)
if name == "__get__" && class.is_known(db, KnownClass::FunctionType) =>
@ -6209,7 +6214,7 @@ impl<'db> Type<'db> {
| Type::AlwaysTruthy
| Type::AlwaysFalsy
| Type::WrapperDescriptor(_)
| Type::KnownBoundMethod(KnownBoundMethodType::StrStartswith(_))
| Type::KnownBoundMethod(KnownBoundMethodType::StrStartswith(_) | KnownBoundMethodType::PathOpen)
| Type::DataclassDecorator(_)
| Type::DataclassTransformer(_)
// A non-generic class never needs to be specialized. A generic class is specialized
@ -6354,7 +6359,9 @@ impl<'db> Type<'db> {
| Type::AlwaysTruthy
| Type::AlwaysFalsy
| Type::WrapperDescriptor(_)
| Type::KnownBoundMethod(KnownBoundMethodType::StrStartswith(_))
| Type::KnownBoundMethod(
KnownBoundMethodType::StrStartswith(_) | KnownBoundMethodType::PathOpen,
)
| Type::DataclassDecorator(_)
| Type::DataclassTransformer(_)
| Type::ModuleLiteral(_)
@ -9435,6 +9442,8 @@ pub enum KnownBoundMethodType<'db> {
/// this allows us to understand statically known branches for common tests such as
/// `if sys.platform.startswith("freebsd")`.
StrStartswith(StringLiteralType<'db>),
/// Method wrapper for `Path.open`,
PathOpen,
}
pub(super) fn walk_method_wrapper_type<'db, V: visitor::TypeVisitor<'db> + ?Sized>(
@ -9458,6 +9467,7 @@ pub(super) fn walk_method_wrapper_type<'db, V: visitor::TypeVisitor<'db> + ?Size
KnownBoundMethodType::StrStartswith(string_literal) => {
visitor.visit_type(db, Type::StringLiteral(string_literal));
}
KnownBoundMethodType::PathOpen => {}
}
}
@ -9493,17 +9503,23 @@ impl<'db> KnownBoundMethodType<'db> {
ConstraintSet::from(self == other)
}
(KnownBoundMethodType::PathOpen, KnownBoundMethodType::PathOpen) => {
ConstraintSet::from(true)
}
(
KnownBoundMethodType::FunctionTypeDunderGet(_)
| KnownBoundMethodType::FunctionTypeDunderCall(_)
| KnownBoundMethodType::PropertyDunderGet(_)
| KnownBoundMethodType::PropertyDunderSet(_)
| KnownBoundMethodType::StrStartswith(_),
| KnownBoundMethodType::StrStartswith(_)
| KnownBoundMethodType::PathOpen,
KnownBoundMethodType::FunctionTypeDunderGet(_)
| KnownBoundMethodType::FunctionTypeDunderCall(_)
| KnownBoundMethodType::PropertyDunderGet(_)
| KnownBoundMethodType::PropertyDunderSet(_)
| KnownBoundMethodType::StrStartswith(_),
| KnownBoundMethodType::StrStartswith(_)
| KnownBoundMethodType::PathOpen,
) => ConstraintSet::from(false),
}
}
@ -9538,17 +9554,23 @@ impl<'db> KnownBoundMethodType<'db> {
ConstraintSet::from(self == other)
}
(KnownBoundMethodType::PathOpen, KnownBoundMethodType::PathOpen) => {
ConstraintSet::from(true)
}
(
KnownBoundMethodType::FunctionTypeDunderGet(_)
| KnownBoundMethodType::FunctionTypeDunderCall(_)
| KnownBoundMethodType::PropertyDunderGet(_)
| KnownBoundMethodType::PropertyDunderSet(_)
| KnownBoundMethodType::StrStartswith(_),
| KnownBoundMethodType::StrStartswith(_)
| KnownBoundMethodType::PathOpen,
KnownBoundMethodType::FunctionTypeDunderGet(_)
| KnownBoundMethodType::FunctionTypeDunderCall(_)
| KnownBoundMethodType::PropertyDunderGet(_)
| KnownBoundMethodType::PropertyDunderSet(_)
| KnownBoundMethodType::StrStartswith(_),
| KnownBoundMethodType::StrStartswith(_)
| KnownBoundMethodType::PathOpen,
) => ConstraintSet::from(false),
}
}
@ -9567,7 +9589,7 @@ impl<'db> KnownBoundMethodType<'db> {
KnownBoundMethodType::PropertyDunderSet(property) => {
KnownBoundMethodType::PropertyDunderSet(property.normalized_impl(db, visitor))
}
KnownBoundMethodType::StrStartswith(_) => self,
KnownBoundMethodType::StrStartswith(_) | KnownBoundMethodType::PathOpen => self,
}
}
@ -9579,6 +9601,7 @@ impl<'db> KnownBoundMethodType<'db> {
| KnownBoundMethodType::PropertyDunderGet(_)
| KnownBoundMethodType::PropertyDunderSet(_) => KnownClass::MethodWrapperType,
KnownBoundMethodType::StrStartswith(_) => KnownClass::BuiltinFunctionType,
KnownBoundMethodType::PathOpen => KnownClass::MethodType,
}
}
@ -9675,6 +9698,9 @@ impl<'db> KnownBoundMethodType<'db> {
Some(KnownClass::Bool.to_instance(db)),
)))
}
KnownBoundMethodType::PathOpen => {
Either::Right(std::iter::once(Signature::todo("`Path.open` return type")))
}
}
}
}

29
crates/ty_python_semantic/src/types/class.rs
View file

@ -3721,6 +3721,8 @@ pub enum KnownClass {
TypedDictFallback,
// string.templatelib
Template,
// pathlib
Path,
// ty_extensions
ConstraintSet,
}
@ -3767,7 +3769,8 @@ impl KnownClass {
| Self::MethodWrapperType
| Self::CoroutineType
| Self::BuiltinFunctionType
| Self::Template => Some(Truthiness::AlwaysTrue),
| Self::Template
| Self::Path => Some(Truthiness::AlwaysTrue),
Self::NoneType => Some(Truthiness::AlwaysFalse),
@ -3909,7 +3912,8 @@ impl KnownClass {
| KnownClass::TypedDictFallback
| KnownClass::BuiltinFunctionType
| KnownClass::ProtocolMeta
| KnownClass::Template => false,
| KnownClass::Template
| KnownClass::Path => false,
}
}
@ -3990,7 +3994,8 @@ impl KnownClass {
| KnownClass::TypedDictFallback
| KnownClass::BuiltinFunctionType
| KnownClass::ProtocolMeta
| KnownClass::Template => false,
| KnownClass::Template
| KnownClass::Path => false,
}
}
@ -4070,7 +4075,8 @@ impl KnownClass {
| KnownClass::ConstraintSet
| KnownClass::BuiltinFunctionType
| KnownClass::ProtocolMeta
| KnownClass::Template => false,
| KnownClass::Template
| KnownClass::Path => false,
}
}
@ -4163,7 +4169,8 @@ impl KnownClass {
| Self::TypedDictFallback
| Self::BuiltinFunctionType
| Self::ProtocolMeta
| Self::Template => false,
| Self::Template
| KnownClass::Path => false,
}
}
@ -4263,6 +4270,7 @@ impl KnownClass {
Self::ConstraintSet => "ConstraintSet",
Self::TypedDictFallback => "TypedDictFallback",
Self::Template => "Template",
Self::Path => "Path",
Self::ProtocolMeta => "_ProtocolMeta",
}
}
@ -4534,6 +4542,7 @@ impl KnownClass {
Self::NamedTupleFallback | Self::TypedDictFallback => KnownModule::TypeCheckerInternals,
Self::NamedTupleLike | Self::ConstraintSet => KnownModule::TyExtensions,
Self::Template => KnownModule::Templatelib,
Self::Path => KnownModule::Pathlib,
}
}
@ -4616,7 +4625,8 @@ impl KnownClass {
| Self::TypedDictFallback
| Self::BuiltinFunctionType
| Self::ProtocolMeta
| Self::Template => Some(false),
| Self::Template
| Self::Path => Some(false),
Self::Tuple => None,
}
@ -4702,7 +4712,8 @@ impl KnownClass {
| Self::TypedDictFallback
| Self::BuiltinFunctionType
| Self::ProtocolMeta
| Self::Template => false,
| Self::Template
| Self::Path => false,
}
}
@ -4798,6 +4809,7 @@ impl KnownClass {
"ConstraintSet" => Self::ConstraintSet,
"TypedDictFallback" => Self::TypedDictFallback,
"Template" => Self::Template,
"Path" => Self::Path,
"_ProtocolMeta" => Self::ProtocolMeta,
_ => return None,
};
@ -4869,7 +4881,8 @@ impl KnownClass {
| Self::ConstraintSet
| Self::Awaitable
| Self::Generator
| Self::Template => module == self.canonical_module(db),
| Self::Template
| Self::Path => module == self.canonical_module(db),
Self::NoneType => matches!(module, KnownModule::Typeshed | KnownModule::Types),
Self::SpecialForm
| Self::TypeVar

3
crates/ty_python_semantic/src/types/display.rs
View file

@ -387,6 +387,9 @@ impl Display for DisplayRepresentation<'_> {
Type::KnownBoundMethod(KnownBoundMethodType::StrStartswith(_)) => {
f.write_str("<method-wrapper `startswith` of `str` object>")
}
Type::KnownBoundMethod(KnownBoundMethodType::PathOpen) => {
f.write_str("bound method `Path.open`")
}
Type::WrapperDescriptor(kind) => {
let (method, object) = match kind {
WrapperDescriptorKind::FunctionTypeDunderGet => ("__get__", "function"),

175
crates/ty_python_semantic/src/types/function.rs
View file

@ -1120,6 +1120,70 @@ fn is_instance_truthiness<'db>(
}
}
/// Return true, if the type passed as `mode` would require us to pick a non-trivial overload of
/// `builtins.open` / `os.fdopen` / `Path.open`.
fn is_mode_with_nontrivial_return_type<'db>(db: &'db dyn Db, mode: Type<'db>) -> bool {
// Return true for any mode that doesn't match typeshed's
// `OpenTextMode` type alias (<https://github.com/python/typeshed/blob/6937a9b193bfc2f0696452d58aad96d7627aa29a/stdlib/_typeshed/__init__.pyi#L220>).
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"
)
})
}
fn signature_cycle_recover<'db>(
_db: &'db dyn Db,
_value: &CallableSignature<'db>,
@ -1188,8 +1252,16 @@ pub enum KnownFunction {
DunderImport,
/// `importlib.import_module`, which returns the submodule.
ImportModule,
/// `builtins.open`
Open,
/// `os.fdopen`
Fdopen,
/// `tempfile.NamedTemporaryFile`
#[strum(serialize = "NamedTemporaryFile")]
NamedTemporaryFile,
/// `typing(_extensions).final`
Final,
/// `typing(_extensions).disjoint_base`
@ -1308,6 +1380,12 @@ impl KnownFunction {
Self::AbstractMethod => {
matches!(module, KnownModule::Abc)
}
Self::Fdopen => {
matches!(module, KnownModule::Os)
}
Self::NamedTemporaryFile => {
matches!(module, KnownModule::Tempfile)
}
Self::Dataclass | Self::Field => {
matches!(module, KnownModule::Dataclasses)
}
@ -1656,72 +1734,33 @@ 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"));
}
// TODO: Temporary special-casing for `builtins.open` to avoid an excessive number of
// false positives in lieu of proper support for PEP-613 type aliases.
if let [_, Some(mode), ..] = parameter_types
&& is_mode_with_nontrivial_return_type(db, *mode)
{
overload.set_return_type(todo_type!("`builtins.open` return type"));
}
}
KnownFunction::Fdopen => {
// TODO: Temporary special-casing for `os.fdopen` to avoid an excessive number of
// false positives in lieu of proper support for PEP-613 type aliases.
if let [_, Some(mode), ..] = parameter_types
&& is_mode_with_nontrivial_return_type(db, *mode)
{
overload.set_return_type(todo_type!("`os.fdopen` return type"));
}
}
KnownFunction::NamedTemporaryFile => {
// TODO: Temporary special-casing for `tempfile.NamedTemporaryFile` to avoid an excessive number of
// false positives in lieu of proper support for PEP-613 type aliases.
if let [Some(mode), ..] = parameter_types
&& is_mode_with_nontrivial_return_type(db, *mode)
{
overload
.set_return_type(todo_type!("`tempfile.NamedTemporaryFile` return type"));
}
}
@ -1756,6 +1795,10 @@ pub(crate) mod tests {
KnownFunction::AbstractMethod => KnownModule::Abc,
KnownFunction::Fdopen => KnownModule::Os,
KnownFunction::NamedTemporaryFile => KnownModule::Tempfile,
KnownFunction::Dataclass | KnownFunction::Field => KnownModule::Dataclasses,
KnownFunction::GetattrStatic => KnownModule::Inspect,