From 33a6ecafe05f001e1c3349b64dd3430ccf66a4aa Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Mon, 3 Nov 2025 05:46:15 +0900 Subject: [PATCH 1/5] fix --- crates/pyrefly_types/src/annotation.rs | 11 ++- crates/pyrefly_types/src/display.rs | 3 + crates/pyrefly_types/src/keywords.rs | 18 ++++ crates/pyrefly_types/src/types.rs | 15 ++- pyrefly/lib/alt/class/class_field.rs | 123 +++++++++++++++++++++-- pyrefly/lib/alt/class/dataclass.rs | 3 + pyrefly/lib/alt/solve.rs | 132 ++++++++++++++++++++++--- pyrefly/lib/binding/pydantic.rs | 1 + pyrefly/lib/test/pydantic/field.rs | 30 ++++++ 9 files changed, 314 insertions(+), 22 deletions(-) diff --git a/crates/pyrefly_types/src/annotation.rs b/crates/pyrefly_types/src/annotation.rs index 1da2d4654d..ce7fd96134 100644 --- a/crates/pyrefly_types/src/annotation.rs +++ b/crates/pyrefly_types/src/annotation.rs @@ -16,6 +16,7 @@ use pyrefly_derive::TypeEq; use pyrefly_derive::VisitMut; use pyrefly_util::display::intersperse_iter; +use crate::keywords::RangeConstraints; use crate::types::AnyStyle; use crate::types::Substitution; use crate::types::Type; @@ -24,6 +25,7 @@ use crate::types::Type; pub struct Annotation { pub qualifiers: Vec, pub ty: Option, + pub range_constraints: RangeConstraints, } impl Display for Annotation { @@ -48,6 +50,7 @@ impl Annotation { Self { qualifiers: Vec::new(), ty: Some(ty), + range_constraints: RangeConstraints::default(), } } @@ -75,6 +78,7 @@ impl Annotation { Self { qualifiers: self.qualifiers, ty: self.ty.map(|ty| substitution.substitute_into(ty)), + range_constraints: self.range_constraints, } } } @@ -100,7 +104,8 @@ mod tests { assert_eq!( Annotation { qualifiers: Vec::new(), - ty: Some(Type::None) + ty: Some(Type::None), + range_constraints: RangeConstraints::default(), } .to_string(), "None" @@ -108,7 +113,8 @@ mod tests { assert_eq!( Annotation { qualifiers: vec![Qualifier::Required, Qualifier::ReadOnly], - ty: None + ty: None, + range_constraints: RangeConstraints::default(), } .to_string(), "Required[ReadOnly]" @@ -117,6 +123,7 @@ mod tests { Annotation { qualifiers: vec![Qualifier::Required, Qualifier::ReadOnly], ty: Some(Type::LiteralString), + range_constraints: RangeConstraints::default(), } .to_string(), "Required[ReadOnly[LiteralString]]" diff --git a/crates/pyrefly_types/src/display.rs b/crates/pyrefly_types/src/display.rs index ce0ef4418b..d455b86169 100644 --- a/crates/pyrefly_types/src/display.rs +++ b/crates/pyrefly_types/src/display.rs @@ -24,6 +24,8 @@ use starlark_map::smallmap; use crate::callable::Function; use crate::class::Class; +#[cfg(test)] +use crate::keywords::RangeConstraints; use crate::literal::Lit; use crate::tuple::Tuple; use crate::types::AnyStyle; @@ -1113,6 +1115,7 @@ pub mod tests { Type::None, TypeAliasStyle::LegacyImplicit, Vec::new(), + RangeConstraints::default(), ))); let wrapped = Type::tuple(vec![alias.clone()]); let type_of = Type::type_form(alias.clone()); diff --git a/crates/pyrefly_types/src/keywords.rs b/crates/pyrefly_types/src/keywords.rs index 2cf7e81608..814d0e3cec 100644 --- a/crates/pyrefly_types/src/keywords.rs +++ b/crates/pyrefly_types/src/keywords.rs @@ -74,6 +74,22 @@ impl KwCall { } } +/// Numeric range constraints extracted from metadata such as `annotated_types.Gt`. +#[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Visit, VisitMut, TypeEq)] +pub struct RangeConstraints { + pub lt: Option, + pub gt: Option, + pub ge: Option, + pub le: Option, +} + +impl RangeConstraints { + pub fn is_empty(&self) -> bool { + self.lt.is_none() && self.gt.is_none() && self.ge.is_none() && self.le.is_none() + } +} + /// Parameters to `typing.dataclass_transform`. /// See https://typing.python.org/en/latest/spec/dataclasses.html#dataclass-transform-parameters. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -132,6 +148,7 @@ pub struct DataclassFieldKeywords { pub lt: Option, pub gt: Option, pub ge: Option, + pub le: Option, /// Whether we should strictly evaluate the type of the field pub strict: Option, /// If a converter callable is passed in, its first positional parameter @@ -160,6 +177,7 @@ impl DataclassFieldKeywords { lt: None, gt: None, ge: None, + le: None, converter_param: None, strict: None, } diff --git a/crates/pyrefly_types/src/types.rs b/crates/pyrefly_types/src/types.rs index d1affc049d..b8e5fcceb3 100644 --- a/crates/pyrefly_types/src/types.rs +++ b/crates/pyrefly_types/src/types.rs @@ -40,6 +40,7 @@ use crate::class::ClassKind; use crate::class::ClassType; use crate::keywords::DataclassTransformKeywords; use crate::keywords::KwCall; +use crate::keywords::RangeConstraints; use crate::literal::Lit; use crate::module::ModuleType; use crate::param_spec::ParamSpec; @@ -325,15 +326,23 @@ pub struct TypeAlias { ty: Box, pub style: TypeAliasStyle, annotated_metadata: Box<[Type]>, + range_constraints: RangeConstraints, } impl TypeAlias { - pub fn new(name: Name, ty: Type, style: TypeAliasStyle, annotated_metadata: Vec) -> Self { + pub fn new( + name: Name, + ty: Type, + style: TypeAliasStyle, + annotated_metadata: Vec, + range_constraints: RangeConstraints, + ) -> Self { Self { name: Box::new(name), ty: Box::new(ty), style, annotated_metadata: annotated_metadata.into_boxed_slice(), + range_constraints, } } @@ -341,6 +350,10 @@ impl TypeAlias { &self.annotated_metadata } + pub fn range_constraints(&self) -> &RangeConstraints { + &self.range_constraints + } + /// Gets the type contained within the type alias for use in a value /// position - for example, for a function call or attribute access. pub fn as_value(&self, stdlib: &Stdlib) -> Type { diff --git a/pyrefly/lib/alt/class/class_field.rs b/pyrefly/lib/alt/class/class_field.rs index bc4c55f491..6b825e5b68 100644 --- a/pyrefly/lib/alt/class/class_field.rs +++ b/pyrefly/lib/alt/class/class_field.rs @@ -28,6 +28,7 @@ use pyrefly_util::visit::VisitMut; use ruff_python_ast::Expr; use ruff_python_ast::ExprCall; use ruff_python_ast::name::Name; +use ruff_text_size::Ranged; use ruff_text_size::TextRange; use starlark_map::small_map::SmallMap; use starlark_map::small_set::SmallSet; @@ -64,6 +65,8 @@ use crate::types::callable::Required; use crate::types::class::Class; use crate::types::class::ClassType; use crate::types::keywords::DataclassFieldKeywords; +use crate::types::keywords::RangeConstraints; +use crate::types::lit_int::LitInt; use crate::types::literal::Lit; use crate::types::quantified::Quantified; use crate::types::read_only::ReadOnlyReason; @@ -80,6 +83,13 @@ use crate::types::types::SuperObj; use crate::types::types::TArgs; use crate::types::types::Type; +fn int_literal_from_type(ty: &Type) -> Option { + match ty { + Type::Literal(Lit::Int(lit)) => Some(lit.clone()), + _ => None, + } +} + /// The result of looking up an attribute access on a class (either as an instance or a /// class access, and possibly through a special case lookup such as a type var with a bound). #[derive(Debug, Clone)] @@ -583,6 +593,7 @@ impl ClassField { Some(Annotation { ty: Some(ty), qualifiers, + range_constraints: _, }), .. } => Some(TypedDictField { @@ -961,6 +972,86 @@ pub enum DataclassMember { } impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { + fn merge_range_constraints_into_keywords( + &self, + keywords: &mut DataclassFieldKeywords, + constraints: &RangeConstraints, + ) { + if keywords.gt.is_none() { + if let Some(gt) = &constraints.gt { + keywords.gt = Some(gt.clone()); + } + } + if keywords.ge.is_none() { + if let Some(ge) = &constraints.ge { + keywords.ge = Some(ge.clone()); + } + } + if keywords.lt.is_none() { + if let Some(lt) = &constraints.lt { + keywords.lt = Some(lt.clone()); + } + } + if keywords.le.is_none() { + if let Some(le) = &constraints.le { + keywords.le = Some(le.clone()); + } + } + } + + fn check_pydantic_range_default( + &self, + field_name: &Name, + expr: &Expr, + value_ty: &Type, + keywords: &DataclassFieldKeywords, + errors: &ErrorCollector, + ) { + let Some(value_lit) = int_literal_from_type(value_ty) else { + return; + }; + let emit_violation = |label: &str, constraint_ty: &Type| { + let Some(constraint_lit) = int_literal_from_type(constraint_ty) else { + return; + }; + let comparison = value_lit.cmp(&constraint_lit); + let violates = match label { + "gt" => !matches!(comparison, std::cmp::Ordering::Greater), + "ge" => matches!(comparison, std::cmp::Ordering::Less), + "lt" => !matches!(comparison, std::cmp::Ordering::Less), + "le" => matches!(comparison, std::cmp::Ordering::Greater), + _ => false, + }; + if violates { + self.error( + errors, + expr.range(), + ErrorInfo::Kind(ErrorKind::BadArgumentType), + format!( + "Default value `{}` violates Pydantic `{}` constraint `{}` for field `{}`", + self.for_display(value_ty.clone()), + label, + self.for_display(constraint_ty.clone()), + field_name + ), + ); + } + }; + + if let Some(gt) = &keywords.gt { + emit_violation("gt", gt); + } + if let Some(ge) = &keywords.ge { + emit_violation("ge", ge); + } + if let Some(lt) = &keywords.lt { + emit_violation("lt", lt); + } + if let Some(le) = &keywords.le { + emit_violation("le", le); + } + } + pub fn calculate_class_field( &self, class: &Class, @@ -1113,24 +1204,34 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { .as_ref() .is_some_and(|annot| annot.has_qualifier(&Qualifier::ClassVar)) }; - let initialization = + let mut initialization = self.get_class_field_initialization(&metadata, initial_value, magically_initialized); + if metadata.is_pydantic_base_model() { + if let Some(annot) = &direct_annotation { + if !annot.range_constraints.is_empty() { + let mut maybe_keywords = match &initialization { + ClassFieldInitialization::ClassBody(Some(k)) => Some(k.clone()), + _ => None, + }; + let keywords = maybe_keywords.get_or_insert_with(DataclassFieldKeywords::new); + self.merge_range_constraints_into_keywords(keywords, &annot.range_constraints); + initialization = ClassFieldInitialization::ClassBody(Some(keywords.clone())); + } + } + } + // Note: the subset check here is too conservative when it comes to modeling runtime behavior // we want to check if the bound_val is coercible to the annotation type at runtime. // statically, this could be a challenge, which is why we go with this more conservative approach for now. if metadata.is_pydantic_base_model() && let Some(annot) = &direct_annotation - && let ClassFieldInitialization::ClassBody(Some(DataclassFieldKeywords { - gt, - lt, - ge, - .. - })) = &initialization + && let ClassFieldInitialization::ClassBody(Some(field_flags)) = &initialization { let field_ty = annot.get_type(); - for (bound_val, label) in [(gt, "gt"), (lt, "lt"), (ge, "ge")] { + let DataclassFieldKeywords { gt, lt, ge, le, .. } = field_flags; + for (bound_val, label) in [(gt, "gt"), (lt, "lt"), (ge, "ge"), (le, "le")] { let Some(val) = bound_val else { continue }; if !self.is_subset_eq(val, field_ty) { self.error( @@ -1160,6 +1261,12 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { format!("TypedDict item `{name}` may not be initialized"), ); } + if metadata.is_pydantic_base_model() + && let ClassFieldInitialization::ClassBody(Some(keywords)) = &initialization + && let RawClassFieldInitialization::ClassBody(Some(expr)) = initial_value + { + self.check_pydantic_range_default(name, expr, &value_ty, keywords, errors); + } if metadata.is_typed_dict() || metadata .named_tuple_metadata() diff --git a/pyrefly/lib/alt/class/dataclass.rs b/pyrefly/lib/alt/class/dataclass.rs index a06ff1833e..013a19c404 100644 --- a/pyrefly/lib/alt/class/dataclass.rs +++ b/pyrefly/lib/alt/class/dataclass.rs @@ -31,6 +31,7 @@ use crate::alt::types::class_metadata::ClassSynthesizedFields; use crate::alt::types::class_metadata::DataclassMetadata; use crate::binding::pydantic::GE; use crate::binding::pydantic::GT; +use crate::binding::pydantic::LE; use crate::binding::pydantic::LT; use crate::binding::pydantic::STRICT; use crate::config::error_kind::ErrorKind; @@ -245,6 +246,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { let gt = map.0.get(>).cloned(); let lt = map.0.get(<).cloned(); let ge = map.0.get(&GE).cloned(); + let le = map.0.get(&LE).cloned(); let strict: Option = map.0.get(&STRICT).and_then(|v| v.as_bool()); @@ -279,6 +281,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { lt, gt, ge, + le, strict, converter_param, } diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index f8230803fb..afe52d4938 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -14,6 +14,7 @@ use pyrefly_python::ast::Ast; use pyrefly_python::dunder; use pyrefly_python::short_identifier::ShortIdentifier; use pyrefly_types::facet::FacetKind; +use pyrefly_types::keywords::RangeConstraints; use pyrefly_types::type_info::JoinStyle; use pyrefly_types::typed_dict::ExtraItems; use pyrefly_types::typed_dict::TypedDict; @@ -134,6 +135,26 @@ use crate::types::types::TypeAlias; use crate::types::types::TypeAliasStyle; use crate::types::types::Var; +#[derive(Debug, Clone, Copy)] +enum RangeConstraintKind { + Gt, + Ge, + Lt, + Le, +} + +fn expr_qualified_name(expr: &Expr) -> Option> { + match expr { + Expr::Name(name) => Some(vec![name.id.to_string()]), + Expr::Attribute(attr) => { + let mut base = expr_qualified_name(&attr.value)?; + base.push(attr.attr.id.to_string()); + Some(base) + } + _ => None, + } +} + #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum TypeFormContext { /// Expression in a base class list @@ -213,6 +234,29 @@ pub enum Iterable { } impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { + fn parse_range_constraint_metadata(&self, expr: &Expr) -> Option<(RangeConstraintKind, Type)> { + let call = expr.as_call_expr()?; + let qual_name = expr_qualified_name(&call.func)?; + let last = qual_name.last()?.as_str(); + let kind = match last { + "Gt" => RangeConstraintKind::Gt, + "Ge" => RangeConstraintKind::Ge, + "Lt" => RangeConstraintKind::Lt, + "Le" => RangeConstraintKind::Le, + _ => return None, + }; + let arg_expr = call.arguments.args.first()?; + if call.arguments.args.len() != 1 { + return None; + } + // Ignore keyword arguments for now; we only handle positional constraints. + if !call.arguments.keywords.is_empty() { + return None; + } + let ty = self.expr_infer(arg_expr, &self.error_swallower()); + Some((kind, ty)) + } + pub fn solve_legacy_tparam( &self, binding: &BindingLegacyTypeParam, @@ -418,6 +462,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { qualifiers: vec![qualifier], ty: Some(self.expr_infer(&x.slice, errors)), + range_constraints: RangeConstraints::default(), } } _ => Annotation::new_type(self.expr_infer(x, errors)), @@ -593,6 +638,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { qualifiers: vec![qualifier], ty: None, + range_constraints: RangeConstraints::default(), } } Expr::Subscript(x) @@ -624,6 +670,28 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ); } let mut ann = self.expr_annotation(&unpacked_slice[0], type_form_context, errors); + if qualifier == Qualifier::Annotated { + for meta in unpacked_slice.iter().skip(1) { + if let Some((kind, constraint_ty)) = + self.parse_range_constraint_metadata(meta) + { + match kind { + RangeConstraintKind::Gt => { + ann.range_constraints.gt = Some(constraint_ty.clone()) + } + RangeConstraintKind::Ge => { + ann.range_constraints.ge = Some(constraint_ty.clone()) + } + RangeConstraintKind::Lt => { + ann.range_constraints.lt = Some(constraint_ty.clone()) + } + RangeConstraintKind::Le => { + ann.range_constraints.le = Some(constraint_ty.clone()) + } + } + } + } + } if qualifier == Qualifier::ClassVar && ann.get_type().any(|x| x.is_type_variable()) { self.error( @@ -667,8 +735,27 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ann } _ => { - let ann_ty = self.expr_untype(x, type_form_context, errors); - if let Type::SpecialForm(special_form) = ann_ty + let inferred_ty = self.expr_infer(x, errors); + if type_form_context == TypeFormContext::BaseClassList + && let Type::TypeAlias(ta) = &inferred_ty + && ta.style == TypeAliasStyle::Scoped + { + self.error( + errors, + x.range(), + ErrorInfo::Kind(ErrorKind::InvalidInheritance), + format!( + "Cannot use scoped type alias `{}` as a base class. Use a legacy type alias instead: `{}: TypeAlias = {}`", + ta.name, + ta.name, + self.for_display(ta.as_type()) + ), + ); + return Annotation::new_type(Type::any_error()); + } + let ann_ty = self.untype(inferred_ty.clone(), x.range(), errors); + let ann_ty = self.validate_type_form(ann_ty, x.range(), type_form_context, errors); + if let Type::SpecialForm(special_form) = ann_ty.clone() && !type_form_context.is_valid_unparameterized_annotation(special_form) { if special_form.can_be_subscripted() { @@ -687,7 +774,11 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ); } } - Annotation::new_type(ann_ty) + let mut annotation = Annotation::new_type(ann_ty); + if let Type::TypeAlias(ta) = inferred_ty { + annotation.range_constraints = ta.range_constraints().clone(); + } + annotation } } } @@ -1164,23 +1255,38 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { } } // Extract Annotated metadata; skip the first element since that's the type and collect the rest of the vector - let annotated_metadata = match expr { + let (annotated_metadata, range_constraints) = match expr { Expr::Subscript(s) if matches!( self.expr_qualifier(&s.value, TypeFormContext::TypeAlias, errors), Some(Qualifier::Annotated) ) => { - Ast::unpack_slice(&s.slice) - .iter() - .skip(1) - .map(|e| self.expr_infer(e, &self.error_swallower())) - .collect() + let mut metadata = Vec::new(); + let mut constraints = RangeConstraints::default(); + for e in Ast::unpack_slice(&s.slice).iter().skip(1) { + if let Some((kind, ty)) = self.parse_range_constraint_metadata(e) { + match kind { + RangeConstraintKind::Gt => constraints.gt = Some(ty.clone()), + RangeConstraintKind::Ge => constraints.ge = Some(ty.clone()), + RangeConstraintKind::Lt => constraints.lt = Some(ty.clone()), + RangeConstraintKind::Le => constraints.le = Some(ty.clone()), + } + } + metadata.push(self.expr_infer(e, &self.error_swallower())); + } + (metadata, constraints) } - _ => Vec::new(), + _ => (Vec::new(), RangeConstraints::default()), }; - let ta = TypeAlias::new(name.clone(), Type::type_form(ty), style, annotated_metadata); + let ta = TypeAlias::new( + name.clone(), + Type::type_form(ty), + style, + annotated_metadata, + range_constraints, + ); Forallable::TypeAlias(ta).forall(self.validated_tparams( range, @@ -2733,6 +2839,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { ty: Some(want), qualifiers: _, + range_constraints: _, }, } = &*self.get_idx(*k) { @@ -2752,6 +2859,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { ty: Some(want), qualifiers: _, + range_constraints: _, }, } = &*self.get_idx(*k) { @@ -2773,6 +2881,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { ty: Some(want), qualifiers: _, + range_constraints: _, }, } = &*self.get_idx(*k) { @@ -3295,6 +3404,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { ty: Some(want), qualifiers: _, + range_constraints: _, }, } = &*self.get_idx(*k) { diff --git a/pyrefly/lib/binding/pydantic.rs b/pyrefly/lib/binding/pydantic.rs index 45934d838c..adf77d84ac 100644 --- a/pyrefly/lib/binding/pydantic.rs +++ b/pyrefly/lib/binding/pydantic.rs @@ -25,6 +25,7 @@ pub const VALIDATE_BY_ALIAS: Name = Name::new_static("validate_by_alias"); pub const GT: Name = Name::new_static("gt"); pub const LT: Name = Name::new_static("lt"); pub const GE: Name = Name::new_static("ge"); +pub const LE: Name = Name::new_static("le"); pub const ROOT: Name = Name::new_static("root"); pub const STRICT: Name = Name::new_static("strict"); pub const STRICT_DEFAULT: bool = false; diff --git a/pyrefly/lib/test/pydantic/field.rs b/pyrefly/lib/test/pydantic/field.rs index 4241afe150..e72c1ff27f 100644 --- a/pyrefly/lib/test/pydantic/field.rs +++ b/pyrefly/lib/test/pydantic/field.rs @@ -76,6 +76,36 @@ A() # E: Missing argument `x` "#, ); +pydantic_testcase!( + test_positive_int_default_violation, + r#" +from pydantic import BaseModel, PositiveInt + +class Model(BaseModel): + value: PositiveInt = -1 # E: Default value `Literal[-1]` violates Pydantic `gt` constraint `Literal[0]` for field `value` + "#, +); + +pydantic_testcase!( + test_positive_int_default_ok, + r#" +from pydantic import BaseModel, PositiveInt + +class Model(BaseModel): + value: PositiveInt = 1 + "#, +); + +pydantic_testcase!( + test_non_negative_int_default_violation, + r#" +from pydantic import BaseModel, NonNegativeInt + +class Model(BaseModel): + value: NonNegativeInt = -1 # E: Default value `Literal[-1]` violates Pydantic `ge` constraint `Literal[0]` for field `value` + "#, +); + fn pydantic_env_3_10() -> TestEnv { let env = pydantic_env(); env.with_version(PythonVersion::new(3, 10, 0)) From 07274ebc54f053e473e5a7b6bb536dfb9245ced7 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Tue, 4 Nov 2025 01:36:27 +0900 Subject: [PATCH 2/5] add comment --- pyrefly/lib/alt/class/class_field.rs | 10 ++++++++++ pyrefly/lib/alt/solve.rs | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/pyrefly/lib/alt/class/class_field.rs b/pyrefly/lib/alt/class/class_field.rs index 6b825e5b68..1149993c5e 100644 --- a/pyrefly/lib/alt/class/class_field.rs +++ b/pyrefly/lib/alt/class/class_field.rs @@ -84,6 +84,8 @@ use crate::types::types::TArgs; use crate::types::types::Type; fn int_literal_from_type(ty: &Type) -> Option { + // We only currently enforce range constraints for literal defaults, so carve out + // the `Literal[int]` case and ignore everything else. match ty { Type::Literal(Lit::Int(lit)) => Some(lit.clone()), _ => None, @@ -977,6 +979,9 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { keywords: &mut DataclassFieldKeywords, constraints: &RangeConstraints, ) { + // `DataclassFieldKeywords` already carries any `Field(gt=...)` metadata. When a type alias + // such as `PositiveInt` supplies additional bounds, merge them in so that the analysis for + // class-body defaults sees the tightest possible range. if keywords.gt.is_none() { if let Some(gt) = &constraints.gt { keywords.gt = Some(gt.clone()); @@ -1007,6 +1012,11 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { keywords: &DataclassFieldKeywords, errors: &ErrorCollector, ) { + // This is the connective tissue that turns the static range information into actionable + // diagnostics. Whenever a field has a class-body default, we compute the literal value + // and ensure it satisfies every bound coming from `Field(...)` keywords as well as from + // type aliases layered on the annotation. If the metadata disagrees with the default we + // surface a precise `BadArgumentType` error that mirrors the runtime Pydantic failure. let Some(value_lit) = int_literal_from_type(value_ty) else { return; }; diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index afe52d4938..297f5b9dd4 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -144,6 +144,8 @@ enum RangeConstraintKind { } fn expr_qualified_name(expr: &Expr) -> Option> { + /// Walks attribute/name expressions like `annotated_types.Gt` + /// so we can match against the exact helper that produced a Pydantic metadata value. match expr { Expr::Name(name) => Some(vec![name.id.to_string()]), Expr::Attribute(attr) => { @@ -235,6 +237,10 @@ pub enum Iterable { impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { fn parse_range_constraint_metadata(&self, expr: &Expr) -> Option<(RangeConstraintKind, Type)> { + // Annotated metadata can contain arbitrary expressions; we only care about the small set + // of `annotated_types.Gt/Ge/Lt/Le` helpers. When we see one, capture the literal argument + // as a Type so downstream consumers (Pydantic field synthesis) can reason about + // the numeric bound without having to re-parse the AST. let call = expr.as_call_expr()?; let qual_name = expr_qualified_name(&call.func)?; let last = qual_name.last()?.as_str(); From 016263fb9ff2eaf1c5c1fe92457c91061dfe1a57 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Tue, 4 Nov 2025 01:54:50 +0900 Subject: [PATCH 3/5] add test case --- pyrefly/lib/alt/class/class_field.rs | 42 ++++++++++++++++++++++++++-- pyrefly/lib/alt/solve.rs | 4 +-- pyrefly/lib/test/pydantic/field.rs | 20 +++++++++++++ 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/pyrefly/lib/alt/class/class_field.rs b/pyrefly/lib/alt/class/class_field.rs index 1149993c5e..f9e33ac2fb 100644 --- a/pyrefly/lib/alt/class/class_field.rs +++ b/pyrefly/lib/alt/class/class_field.rs @@ -92,6 +92,18 @@ fn int_literal_from_type(ty: &Type) -> Option { } } +fn expr_qualified_name(expr: &Expr) -> Option> { + match expr { + Expr::Name(name) => Some(vec![name.id.to_string()]), + Expr::Attribute(attr) => { + let mut base = expr_qualified_name(&attr.value)?; + base.push(attr.attr.id.to_string()); + Some(base) + } + _ => None, + } +} + /// The result of looking up an attribute access on a class (either as an instance or a /// class access, and possibly through a special case lookup such as a type var with a bound). #[derive(Debug, Clone)] @@ -1004,6 +1016,31 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { } } + fn class_field_default_type( + &self, + expr: &Expr, + value_ty: &Type, + errors: &ErrorCollector, + ) -> Type { + if let Some(call) = expr.as_call_expr() + && let Some(parts) = expr_qualified_name(&call.func) + && parts.last().map(|s| s.as_str()) == Some("Field") + { + if let Some(arg0) = call.arguments.args.first() { + return self.expr_infer(arg0, errors); + } + if let Some(keyword) = call + .arguments + .keywords + .iter() + .find(|kw| kw.arg.as_ref().map(|n| n.id.as_str()) == Some("default")) + { + return self.expr_infer(&keyword.value, errors); + } + } + value_ty.clone() + } + fn check_pydantic_range_default( &self, field_name: &Name, @@ -1017,7 +1054,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { // and ensure it satisfies every bound coming from `Field(...)` keywords as well as from // type aliases layered on the annotation. If the metadata disagrees with the default we // surface a precise `BadArgumentType` error that mirrors the runtime Pydantic failure. - let Some(value_lit) = int_literal_from_type(value_ty) else { + let default_ty = self.class_field_default_type(expr, value_ty, errors); + let Some(value_lit) = int_literal_from_type(&default_ty) else { return; }; let emit_violation = |label: &str, constraint_ty: &Type| { @@ -1039,7 +1077,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ErrorInfo::Kind(ErrorKind::BadArgumentType), format!( "Default value `{}` violates Pydantic `{}` constraint `{}` for field `{}`", - self.for_display(value_ty.clone()), + self.for_display(default_ty.clone()), label, self.for_display(constraint_ty.clone()), field_name diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index 297f5b9dd4..ba127cace6 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -144,8 +144,8 @@ enum RangeConstraintKind { } fn expr_qualified_name(expr: &Expr) -> Option> { - /// Walks attribute/name expressions like `annotated_types.Gt` - /// so we can match against the exact helper that produced a Pydantic metadata value. + // Walk attribute/name expressions like `annotated_types.Gt` so we can + // match against the exact helper that produced a Pydantic metadata value. match expr { Expr::Name(name) => Some(vec![name.id.to_string()]), Expr::Attribute(attr) => { diff --git a/pyrefly/lib/test/pydantic/field.rs b/pyrefly/lib/test/pydantic/field.rs index e72c1ff27f..83eec20c4d 100644 --- a/pyrefly/lib/test/pydantic/field.rs +++ b/pyrefly/lib/test/pydantic/field.rs @@ -106,6 +106,26 @@ class Model(BaseModel): "#, ); +pydantic_testcase!( + test_field_default_gt_violation, + r#" +from pydantic import BaseModel, Field + +class Model(BaseModel): + value: int = Field(0, gt=0) # E: Default value `Literal[0]` violates Pydantic `gt` constraint `Literal[0]` for field `value` + "#, +); + +pydantic_testcase!( + test_field_default_gt_ok, + r#" +from pydantic import BaseModel, Field + +class Model(BaseModel): + value: int = Field(1, gt=0) + "#, +); + fn pydantic_env_3_10() -> TestEnv { let env = pydantic_env(); env.with_version(PythonVersion::new(3, 10, 0)) From 47c162d2fa616304edb08c84621d56577c3890f7 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Tue, 4 Nov 2025 02:32:41 +0900 Subject: [PATCH 4/5] add comment --- crates/pyrefly_types/src/keywords.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/pyrefly_types/src/keywords.rs b/crates/pyrefly_types/src/keywords.rs index 814d0e3cec..a9b08e4b9e 100644 --- a/crates/pyrefly_types/src/keywords.rs +++ b/crates/pyrefly_types/src/keywords.rs @@ -75,6 +75,19 @@ impl KwCall { } /// Numeric range constraints extracted from metadata such as `annotated_types.Gt`. +/// +/// These bounds are intentionally conservative: +/// * Only constraints that can be recovered statically (e.g. literal arguments to +/// `annotated_types.Gt/Ge/Lt/Le` or equivalent Field keywords) are recorded here. +/// * The information is used primarily to vet class-body defaults; call sites that +/// pass dynamic values are still enforced at runtime by Pydantic. +/// * When the metadata cannot be evaluated to a literal (for example, it depends on +/// a value computed at runtime), the corresponding entry remains `None` and the +/// solver defers to runtime validation. +/// +/// In other words, `RangeConstraints` lets us catch the obvious mismatches during +/// static analysis without attempting to replicate every bit of Pydantic’s runtime +/// behaviour. #[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] #[derive(Visit, VisitMut, TypeEq)] pub struct RangeConstraints { From 504fda7a0f4017ee89d5534a76736619ff9cd76d Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Tue, 4 Nov 2025 16:18:59 +0900 Subject: [PATCH 5/5] don't change the Annotation struct --- crates/pyrefly_types/src/annotation.rs | 7 --- pyrefly/lib/alt/class/class_field.rs | 26 ++++++----- pyrefly/lib/alt/solve.rs | 63 +++++++++++++++----------- pyrefly/lib/alt/traits.rs | 2 + pyrefly/lib/binding/binding.rs | 2 + 5 files changed, 56 insertions(+), 44 deletions(-) diff --git a/crates/pyrefly_types/src/annotation.rs b/crates/pyrefly_types/src/annotation.rs index ce7fd96134..7971ce3f5f 100644 --- a/crates/pyrefly_types/src/annotation.rs +++ b/crates/pyrefly_types/src/annotation.rs @@ -16,7 +16,6 @@ use pyrefly_derive::TypeEq; use pyrefly_derive::VisitMut; use pyrefly_util::display::intersperse_iter; -use crate::keywords::RangeConstraints; use crate::types::AnyStyle; use crate::types::Substitution; use crate::types::Type; @@ -25,7 +24,6 @@ use crate::types::Type; pub struct Annotation { pub qualifiers: Vec, pub ty: Option, - pub range_constraints: RangeConstraints, } impl Display for Annotation { @@ -50,7 +48,6 @@ impl Annotation { Self { qualifiers: Vec::new(), ty: Some(ty), - range_constraints: RangeConstraints::default(), } } @@ -78,7 +75,6 @@ impl Annotation { Self { qualifiers: self.qualifiers, ty: self.ty.map(|ty| substitution.substitute_into(ty)), - range_constraints: self.range_constraints, } } } @@ -105,7 +101,6 @@ mod tests { Annotation { qualifiers: Vec::new(), ty: Some(Type::None), - range_constraints: RangeConstraints::default(), } .to_string(), "None" @@ -114,7 +109,6 @@ mod tests { Annotation { qualifiers: vec![Qualifier::Required, Qualifier::ReadOnly], ty: None, - range_constraints: RangeConstraints::default(), } .to_string(), "Required[ReadOnly]" @@ -123,7 +117,6 @@ mod tests { Annotation { qualifiers: vec![Qualifier::Required, Qualifier::ReadOnly], ty: Some(Type::LiteralString), - range_constraints: RangeConstraints::default(), } .to_string(), "Required[ReadOnly[LiteralString]]" diff --git a/pyrefly/lib/alt/class/class_field.rs b/pyrefly/lib/alt/class/class_field.rs index f9e33ac2fb..08b8115e6e 100644 --- a/pyrefly/lib/alt/class/class_field.rs +++ b/pyrefly/lib/alt/class/class_field.rs @@ -607,7 +607,6 @@ impl ClassField { Some(Annotation { ty: Some(ty), qualifiers, - range_constraints: _, }), .. } => Some(TypedDictField { @@ -1115,10 +1114,10 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { // which requires us having a place to store synthesized dummy values until we've refactored more. let value_storage = Owner::new(); let initial_value_storage = Owner::new(); - let (value, direct_annotation, initial_value, is_function_without_return_annotation) = + let (value, direct_annotation_entry, initial_value, is_function_without_return_annotation) = match field_definition { ClassFieldDefinition::DeclaredByAnnotation { annotation } => { - let annotation = self.get_idx(*annotation).as_ref().annotation.clone(); + let annotation = self.get_idx(*annotation).as_ref().clone(); ( value_storage .push(ExprOrBinding::Binding(Binding::Type(Type::any_implicit()))), @@ -1136,8 +1135,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ClassFieldDefinition::AssignedInBody { value, annotation } => { let annotation = annotation .map(|a| self.get_idx(a)) - .as_deref() - .map(|annot| annot.annotation.clone()); + .map(|annot| annot.as_ref().clone()); ( value, annotation, @@ -1172,8 +1170,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { } => { let annotation = annotation .map(|a| self.get_idx(a)) - .as_deref() - .map(|annot| annot.annotation.clone()); + .map(|annot| annot.as_ref().clone()); ( value, annotation, @@ -1184,6 +1181,11 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { } }; + let (direct_annotation, direct_range_constraints) = match direct_annotation_entry { + Some(annot) => (Some(annot.annotation), Some(annot.range_constraints)), + None => (None, None), + }; + // Optimisation. If we can determine that the name definitely doesn't exist in the inheritance // then we can avoid a bunch of work with checking for override errors. let mut is_inherited = IsInherited::Maybe; @@ -1214,7 +1216,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { } else if let Some(annot) = &inherited_annot { let ctx: &dyn Fn() -> TypeCheckContext = &|| TypeCheckContext::of_kind(TypeCheckKind::Attribute(name.clone())); - let hint = Some((annot.get_type(), ctx)); + let hint: Option<(&Type, &dyn Fn() -> TypeCheckContext)> = + Some((annot.get_type(), ctx)); self.expr(e, hint, errors) } else { self.expr_infer(e, errors) @@ -1228,6 +1231,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { None, ), }; + let metadata = self.get_metadata_for_class(class); if let Some(named_tuple_metadata) = metadata.named_tuple_metadata() @@ -1256,14 +1260,14 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { self.get_class_field_initialization(&metadata, initial_value, magically_initialized); if metadata.is_pydantic_base_model() { - if let Some(annot) = &direct_annotation { - if !annot.range_constraints.is_empty() { + if let Some(range_constraints) = &direct_range_constraints { + if !range_constraints.is_empty() { let mut maybe_keywords = match &initialization { ClassFieldInitialization::ClassBody(Some(k)) => Some(k.clone()), _ => None, }; let keywords = maybe_keywords.get_or_insert_with(DataclassFieldKeywords::new); - self.merge_range_constraints_into_keywords(keywords, &annot.range_constraints); + self.merge_range_constraints_into_keywords(keywords, range_constraints); initialization = ClassFieldInitialization::ClassBody(Some(keywords.clone())); } } diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index ba127cace6..66175a45dd 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -416,9 +416,10 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { match binding { BindingAnnotation::AnnotateExpr(target, x, class_key) => { let type_form_context = target.type_form_context(); - let mut ann = self.expr_annotation(x, type_form_context, errors); + let (mut annotation, range_constraints) = + self.expr_annotation(x, type_form_context, errors); if let Some(class_key) = class_key - && let Some(ty) = &mut ann.ty + && let Some(ty) = &mut annotation.ty { let class = &*self.get_idx(*class_key); if let Some(cls) = &class.0 { @@ -429,12 +430,14 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { } Arc::new(AnnotationWithTarget { target: target.clone(), - annotation: ann, + annotation, + range_constraints, }) } BindingAnnotation::Type(target, x) => Arc::new(AnnotationWithTarget { target: target.clone(), annotation: Annotation::new_type(x.clone()), + range_constraints: RangeConstraints::default(), }), } } @@ -468,7 +471,6 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { qualifiers: vec![qualifier], ty: Some(self.expr_infer(&x.slice, errors)), - range_constraints: RangeConstraints::default(), } } _ => Annotation::new_type(self.expr_infer(x, errors)), @@ -617,9 +619,12 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { x: &Expr, type_form_context: TypeFormContext, errors: &ErrorCollector, - ) -> Annotation { + ) -> (Annotation, RangeConstraints) { if !self.has_valid_annotation_syntax(x, errors) { - return Annotation::new_type(Type::any_error()); + return ( + Annotation::new_type(Type::any_error()), + RangeConstraints::default(), + ); } match x { _ if let Some(qualifier) = self.expr_qualifier(x, type_form_context, errors) => { @@ -641,11 +646,13 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ); } } - Annotation { - qualifiers: vec![qualifier], - ty: None, - range_constraints: RangeConstraints::default(), - } + ( + Annotation { + qualifiers: vec![qualifier], + ty: None, + }, + RangeConstraints::default(), + ) } Expr::Subscript(x) if let unpacked_slice = Ast::unpack_slice(&x.slice) @@ -675,7 +682,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ), ); } - let mut ann = self.expr_annotation(&unpacked_slice[0], type_form_context, errors); + let (mut ann, mut range_constraints) = + self.expr_annotation(&unpacked_slice[0], type_form_context, errors); if qualifier == Qualifier::Annotated { for meta in unpacked_slice.iter().skip(1) { if let Some((kind, constraint_ty)) = @@ -683,16 +691,16 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { { match kind { RangeConstraintKind::Gt => { - ann.range_constraints.gt = Some(constraint_ty.clone()) + range_constraints.gt = Some(constraint_ty.clone()) } RangeConstraintKind::Ge => { - ann.range_constraints.ge = Some(constraint_ty.clone()) + range_constraints.ge = Some(constraint_ty.clone()) } RangeConstraintKind::Lt => { - ann.range_constraints.lt = Some(constraint_ty.clone()) + range_constraints.lt = Some(constraint_ty.clone()) } RangeConstraintKind::Le => { - ann.range_constraints.le = Some(constraint_ty.clone()) + range_constraints.le = Some(constraint_ty.clone()) } } } @@ -738,7 +746,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { } else { ann.qualifiers.insert(0, qualifier); } - ann + (ann, range_constraints) } _ => { let inferred_ty = self.expr_infer(x, errors); @@ -757,7 +765,10 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { self.for_display(ta.as_type()) ), ); - return Annotation::new_type(Type::any_error()); + return ( + Annotation::new_type(Type::any_error()), + RangeConstraints::default(), + ); } let ann_ty = self.untype(inferred_ty.clone(), x.range(), errors); let ann_ty = self.validate_type_form(ann_ty, x.range(), type_form_context, errors); @@ -780,11 +791,11 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ); } } - let mut annotation = Annotation::new_type(ann_ty); - if let Type::TypeAlias(ta) = inferred_ty { - annotation.range_constraints = ta.range_constraints().clone(); + let mut range_constraints = RangeConstraints::default(); + if let Type::TypeAlias(ta) = inferred_ty.clone() { + range_constraints = ta.range_constraints().clone(); } - annotation + (Annotation::new_type(ann_ty), range_constraints) } } } @@ -2845,8 +2856,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { ty: Some(want), qualifiers: _, - range_constraints: _, }, + .. } = &*self.get_idx(*k) { self.check_and_return_type(ty, want, x.range, errors, &|| { @@ -2865,8 +2876,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { ty: Some(want), qualifiers: _, - range_constraints: _, }, + .. } = &*self.get_idx(*k) { self.check_and_return_type(ty, want, x.range, errors, &|| { @@ -2887,8 +2898,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { ty: Some(want), qualifiers: _, - range_constraints: _, }, + .. } = &*self.get_idx(*k) { self.check_and_return_type(ty, want, x.range, errors, &|| { @@ -3410,8 +3421,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { Annotation { ty: Some(want), qualifiers: _, - range_constraints: _, }, + .. } = &*self.get_idx(*k) { self.check_and_return_type(ta.clone(), want, x.range, errors, &|| { diff --git a/pyrefly/lib/alt/traits.rs b/pyrefly/lib/alt/traits.rs index 6bdc54596b..db210cfd84 100644 --- a/pyrefly/lib/alt/traits.rs +++ b/pyrefly/lib/alt/traits.rs @@ -70,6 +70,7 @@ use crate::binding::binding::NoneIfRecursive; use crate::error::collector::ErrorCollector; use crate::types::annotation::Annotation; use crate::types::class::Class; +use crate::types::keywords::RangeConstraints; use crate::types::type_info::TypeInfo; use crate::types::types::TParams; use crate::types::types::Type; @@ -321,6 +322,7 @@ impl Solve for KeyAnnotation { AnnotationWithTarget { target: AnnotationTarget::Assign(Name::default(), AnnAssignHasValue::Yes), annotation: Annotation::default(), + range_constraints: RangeConstraints::default(), } } } diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index 4db26c4acd..1dbc8124f3 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -69,6 +69,7 @@ use crate::types::class::ClassDefIndex; use crate::types::class::ClassFieldProperties; use crate::types::equality::TypeEq; use crate::types::globals::ImplicitGlobal; +use crate::types::keywords::RangeConstraints; use crate::types::quantified::QuantifiedKind; use crate::types::stdlib::Stdlib; use crate::types::tuple::Tuple; @@ -1703,6 +1704,7 @@ pub enum AnnAssignHasValue { pub struct AnnotationWithTarget { pub target: AnnotationTarget, pub annotation: Annotation, + pub range_constraints: RangeConstraints, } impl AnnotationWithTarget {