-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[ty] support kw_only=True
for dataclass()
and field()
#19677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -12,6 +12,7 @@ use ruff_db::parsed::parsed_module; | |||||||||||
use smallvec::{SmallVec, smallvec, smallvec_inline}; | ||||||||||||
|
||||||||||||
use super::{Argument, CallArguments, CallError, CallErrorKind, InferContext, Signature, Type}; | ||||||||||||
use crate::Program; | ||||||||||||
use crate::db::Db; | ||||||||||||
use crate::dunder_all::dunder_all_names; | ||||||||||||
use crate::place::{Boundness, Place}; | ||||||||||||
|
@@ -33,7 +34,7 @@ use crate::types::{ | |||||||||||
WrapperDescriptorKind, enums, ide_support, todo_type, | ||||||||||||
}; | ||||||||||||
use ruff_db::diagnostic::{Annotation, Diagnostic, SubDiagnostic, SubDiagnosticSeverity}; | ||||||||||||
use ruff_python_ast as ast; | ||||||||||||
use ruff_python_ast::{self as ast, PythonVersion}; | ||||||||||||
|
||||||||||||
/// Binding information for a possible union of callables. At a call site, the arguments must be | ||||||||||||
/// compatible with _all_ of the types in the union for the call to be valid. | ||||||||||||
|
@@ -860,7 +861,11 @@ impl<'db> Bindings<'db> { | |||||||||||
params |= DataclassParams::MATCH_ARGS; | ||||||||||||
} | ||||||||||||
if to_bool(kw_only, false) { | ||||||||||||
params |= DataclassParams::KW_ONLY; | ||||||||||||
if Program::get(db).python_version(db) >= PythonVersion::PY310 { | ||||||||||||
params |= DataclassParams::KW_ONLY; | ||||||||||||
} else { | ||||||||||||
// TODO: emit diagnostic | ||||||||||||
} | ||||||||||||
} | ||||||||||||
if to_bool(slots, false) { | ||||||||||||
params |= DataclassParams::SLOTS; | ||||||||||||
|
@@ -919,7 +924,9 @@ impl<'db> Bindings<'db> { | |||||||||||
} | ||||||||||||
|
||||||||||||
Some(KnownFunction::Field) => { | ||||||||||||
if let [default, default_factory, init, ..] = overload.parameter_types() | ||||||||||||
// TODO this will break on Python 3.14 -- we should match by parameter name instead | ||||||||||||
if let [default, default_factory, init, .., kw_only] = | ||||||||||||
overload.parameter_types() | ||||||||||||
Comment on lines
+928
to
+929
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not awesome how we're hardcoding a correspondence with the ordering of keyword-only parameters in typeshed here, rather than actually matching by parameter name. But that's orthogonal to this PR. But this makes it even less robust. Python 3.14 added a new We can't just list all the parameters from the front, because then this I think we should modify this code to do something a bit more intelligent here. Ideally that more intelligent thing would be to actually match by parameter name. That should be very doable, but it is a bit more involved; it will require adding new API to So for now perhaps we just add a TODO comment here:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO added yeah - i remember thinking this felt a little brittle while i was working on it. if you want to expand a bit on the vision for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basically a |
||||||||||||
{ | ||||||||||||
let default_ty = match (default, default_factory) { | ||||||||||||
(Some(default_ty), _) => *default_ty, | ||||||||||||
|
@@ -933,6 +940,14 @@ impl<'db> Bindings<'db> { | |||||||||||
.map(|init| !init.bool(db).is_always_false()) | ||||||||||||
.unwrap_or(true); | ||||||||||||
|
||||||||||||
let kw_only = if Program::get(db).python_version(db) | ||||||||||||
>= PythonVersion::PY310 | ||||||||||||
{ | ||||||||||||
kw_only.map(|kw_only| !kw_only.bool(db).is_always_false()) | ||||||||||||
} else { | ||||||||||||
None | ||||||||||||
}; | ||||||||||||
|
||||||||||||
// `typeshed` pretends that `dataclasses.field()` returns the type of the | ||||||||||||
// default value directly. At runtime, however, this function returns an | ||||||||||||
// instance of `dataclasses.Field`. We also model it this way and return | ||||||||||||
|
@@ -942,7 +957,7 @@ impl<'db> Bindings<'db> { | |||||||||||
// to `T`. Otherwise, we would error on `name: str = field(default="")`. | ||||||||||||
overload.set_return_type(Type::KnownInstance( | ||||||||||||
KnownInstanceType::Field(FieldInstance::new( | ||||||||||||
db, default_ty, init, | ||||||||||||
db, default_ty, init, kw_only, | ||||||||||||
)), | ||||||||||||
)); | ||||||||||||
} | ||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i believe this should cover the
dataclasses_transform_func
false positive