From 04f9949711ef7048d93da6bde00e9cb9dea4d3f5 Mon Sep 17 00:00:00 2001 From: Bhuminjay Soni Date: Mon, 15 Dec 2025 01:05:37 +0530 Subject: [PATCH 1/9] [ty] Emit diagnostic when a type variable with a default is followed by one without a default (#21787) Co-authored-by: Alex Waygood --- crates/ty/docs/rules.md | 103 +++++----- .../invalid_type_parameter_order.md | 43 ++++ .../mdtest/generics/legacy/paramspec.md | 3 +- ...nvalid_Order_of_Leg…_(eaa359e8d6b3031d).snap | 190 ++++++++++++++++++ .../src/types/diagnostic.rs | 95 ++++++++- .../src/types/infer/builder.rs | 75 +++++-- ty.schema.json | 2 +- 7 files changed, 438 insertions(+), 73 deletions(-) create mode 100644 crates/ty_python_semantic/resources/mdtest/diagnostics/invalid_type_parameter_order.md create mode 100644 crates/ty_python_semantic/resources/mdtest/snapshots/invalid_type_paramet…_-_Invalid_Order_of_Leg…_(eaa359e8d6b3031d).snap diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md index c2b450ae10..8b35be52a6 100644 --- a/crates/ty/docs/rules.md +++ b/crates/ty/docs/rules.md @@ -557,7 +557,7 @@ a: int = '' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -751,7 +751,7 @@ except ZeroDivisionError: Default level: error · Added in 0.0.1-alpha.28 · Related issues · -View source +View source @@ -793,7 +793,7 @@ class D(A): Default level: error · Added in 0.0.1-alpha.35 · Related issues · -View source +View source @@ -848,16 +848,21 @@ Checks for the creation of invalid generic classes **Why is this bad?** There are several requirements that you must follow when defining a generic class. +Many of these result in `TypeError` being raised at runtime if they are violated. **Examples** ```python -from typing import Generic, TypeVar +from typing_extensions import Generic, TypeVar -T = TypeVar("T") # okay +T = TypeVar("T") +U = TypeVar("U", default=int) # error: class uses both PEP-695 syntax and legacy syntax class C[U](Generic[T]): ... + +# error: type parameter with default comes before type parameter without default +class D(Generic[U, T]): ... ``` **References** @@ -909,7 +914,7 @@ carol = Person(name="Carol", age=25) # typo! Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -944,7 +949,7 @@ def f(t: TypeVar("U")): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -978,7 +983,7 @@ class B(metaclass=f): ... Default level: error · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -1139,7 +1144,7 @@ AttributeError: Cannot overwrite NamedTuple attribute _asdict Default level: error · Preview (since 1.0.0) · Related issues · -View source +View source @@ -1169,7 +1174,7 @@ Baz = NewType("Baz", int | str) # error: invalid base for `typing.NewType` Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1219,7 +1224,7 @@ def foo(x: int) -> int: ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1245,7 +1250,7 @@ def f(a: int = ''): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1310,7 +1315,7 @@ TypeError: Protocols can only inherit from other protocols, got Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1384,7 +1389,7 @@ def func() -> int: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1442,7 +1447,7 @@ TODO #14889 Default level: error · Added in 0.0.1-alpha.6 · Related issues · -View source +View source @@ -1469,7 +1474,7 @@ NewAlias = TypeAliasType(get_name(), int) # error: TypeAliasType name mus Default level: error · Added in 0.0.1-alpha.29 · Related issues · -View source +View source @@ -1516,7 +1521,7 @@ Bar[int] # error: too few arguments Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1546,7 +1551,7 @@ TYPE_CHECKING = '' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1576,7 +1581,7 @@ b: Annotated[int] # `Annotated` expects at least two arguments Default level: error · Added in 0.0.1-alpha.11 · Related issues · -View source +View source @@ -1610,7 +1615,7 @@ f(10) # Error Default level: error · Added in 0.0.1-alpha.11 · Related issues · -View source +View source @@ -1644,7 +1649,7 @@ class C: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1679,7 +1684,7 @@ T = TypeVar('T', bound=str) # valid bound TypeVar Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1704,7 +1709,7 @@ func() # TypeError: func() missing 1 required positional argument: 'x' Default level: error · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -1737,7 +1742,7 @@ alice["age"] # KeyError Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1766,7 +1771,7 @@ func("string") # error: [no-matching-overload] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1790,7 +1795,7 @@ Subscripting an object that does not support it will raise a `TypeError` at runt Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1816,7 +1821,7 @@ for i in 34: # TypeError: 'int' object is not iterable Default level: error · Added in 0.0.1-alpha.29 · Related issues · -View source +View source @@ -1849,7 +1854,7 @@ class B(A): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1876,7 +1881,7 @@ f(1, x=2) # Error raised here Default level: error · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -1934,7 +1939,7 @@ def test(): -> "int": Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1964,7 +1969,7 @@ static_assert(int(2.0 * 3.0) == 6) # error: does not have a statically known tr Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1993,7 +1998,7 @@ class B(A): ... # Error raised here Default level: error · Preview (since 0.0.1-alpha.30) · Related issues · -View source +View source @@ -2027,7 +2032,7 @@ class F(NamedTuple): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2054,7 +2059,7 @@ f("foo") # Error raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2082,7 +2087,7 @@ def _(x: int): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2128,7 +2133,7 @@ class A: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2155,7 +2160,7 @@ f(x=1, y=2) # Error raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2183,7 +2188,7 @@ A().foo # AttributeError: 'A' object has no attribute 'foo' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2208,7 +2213,7 @@ import foo # ModuleNotFoundError: No module named 'foo' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2233,7 +2238,7 @@ print(x) # NameError: name 'x' is not defined Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2270,7 +2275,7 @@ b1 < b2 < b1 # exception raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2298,7 +2303,7 @@ A() + A() # TypeError: unsupported operand type(s) for +: 'A' and 'A' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2452,7 +2457,7 @@ a = 20 / 0 # type: ignore Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2512,7 +2517,7 @@ A()[0] # TypeError: 'A' object is not subscriptable Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2544,7 +2549,7 @@ from module import a # ImportError: cannot import name 'a' from 'module' Default level: warn · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2571,7 +2576,7 @@ cast(int, f()) # Redundant Default level: warn · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2595,7 +2600,7 @@ reveal_type(1) # NameError: name 'reveal_type' is not defined Default level: warn · Added in 0.0.1-alpha.15 · Related issues · -View source +View source @@ -2692,7 +2697,7 @@ class D(C): ... # error: [unsupported-base] Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2779,7 +2784,7 @@ Dividing by zero raises a `ZeroDivisionError` at runtime. Default level: ignore · Added in 0.0.1-alpha.1 · Related issues · -View source +View source diff --git a/crates/ty_python_semantic/resources/mdtest/diagnostics/invalid_type_parameter_order.md b/crates/ty_python_semantic/resources/mdtest/diagnostics/invalid_type_parameter_order.md new file mode 100644 index 0000000000..705ceae6b6 --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/diagnostics/invalid_type_parameter_order.md @@ -0,0 +1,43 @@ +# Invalid Order of Legacy Type Parameters + + + +```toml +[environment] +python-version = "3.13" +``` + +```py +from typing import TypeVar, Generic, Protocol + +T1 = TypeVar("T1", default=int) + +T2 = TypeVar("T2") +T3 = TypeVar("T3") + +DefaultStrT = TypeVar("DefaultStrT", default=str) + +class SubclassMe(Generic[T1, DefaultStrT]): + x: DefaultStrT + +class Baz(SubclassMe[int, DefaultStrT]): + pass + +# error: [invalid-generic-class] "Type parameter `T2` without a default cannot follow earlier parameter `T1` with a default" +class Foo(Generic[T1, T2]): + pass + +class Bar(Generic[T2, T1, T3]): # error: [invalid-generic-class] + pass + +class Spam(Generic[T1, T2, DefaultStrT, T3]): # error: [invalid-generic-class] + pass + +class Ham(Protocol[T1, T2, DefaultStrT, T3]): # error: [invalid-generic-class] + pass + +class VeryBad( + Protocol[T1, T2, DefaultStrT, T3], # error: [invalid-generic-class] + Generic[T1, T2, DefaultStrT, T3], +): ... +``` diff --git a/crates/ty_python_semantic/resources/mdtest/generics/legacy/paramspec.md b/crates/ty_python_semantic/resources/mdtest/generics/legacy/paramspec.md index 7a365b6405..5e3cbe3888 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/legacy/paramspec.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/legacy/paramspec.md @@ -424,9 +424,8 @@ p3 = ParamSpecWithDefault4[[int], [str]]() reveal_type(p3.attr1) # revealed: (int, /) -> None reveal_type(p3.attr2) # revealed: (str, /) -> None -# TODO: error # Un-ordered type variables as the default of `PAnother` is `P` -class ParamSpecWithDefault5(Generic[PAnother, P]): +class ParamSpecWithDefault5(Generic[PAnother, P]): # error: [invalid-generic-class] attr: Callable[PAnother, None] # TODO: error diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/invalid_type_paramet…_-_Invalid_Order_of_Leg…_(eaa359e8d6b3031d).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/invalid_type_paramet…_-_Invalid_Order_of_Leg…_(eaa359e8d6b3031d).snap new file mode 100644 index 0000000000..290fbb0ae0 --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/invalid_type_paramet…_-_Invalid_Order_of_Leg…_(eaa359e8d6b3031d).snap @@ -0,0 +1,190 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: invalid_type_parameter_order.md - Invalid Order of Legacy Type Parameters +mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/invalid_type_parameter_order.md +--- + +# Python source files + +## mdtest_snippet.py + +``` + 1 | from typing import TypeVar, Generic, Protocol + 2 | + 3 | T1 = TypeVar("T1", default=int) + 4 | + 5 | T2 = TypeVar("T2") + 6 | T3 = TypeVar("T3") + 7 | + 8 | DefaultStrT = TypeVar("DefaultStrT", default=str) + 9 | +10 | class SubclassMe(Generic[T1, DefaultStrT]): +11 | x: DefaultStrT +12 | +13 | class Baz(SubclassMe[int, DefaultStrT]): +14 | pass +15 | +16 | # error: [invalid-generic-class] "Type parameter `T2` without a default cannot follow earlier parameter `T1` with a default" +17 | class Foo(Generic[T1, T2]): +18 | pass +19 | +20 | class Bar(Generic[T2, T1, T3]): # error: [invalid-generic-class] +21 | pass +22 | +23 | class Spam(Generic[T1, T2, DefaultStrT, T3]): # error: [invalid-generic-class] +24 | pass +25 | +26 | class Ham(Protocol[T1, T2, DefaultStrT, T3]): # error: [invalid-generic-class] +27 | pass +28 | +29 | class VeryBad( +30 | Protocol[T1, T2, DefaultStrT, T3], # error: [invalid-generic-class] +31 | Generic[T1, T2, DefaultStrT, T3], +32 | ): ... +``` + +# Diagnostics + +``` +error[invalid-generic-class]: Type parameters without defaults cannot follow type parameters with defaults + --> src/mdtest_snippet.py:17:19 + | +16 | # error: [invalid-generic-class] "Type parameter `T2` without a default cannot follow earlier parameter `T1` with a default" +17 | class Foo(Generic[T1, T2]): + | ^^^^^^ + | | + | Type variable `T2` does not have a default + | Earlier TypeVar `T1` does +18 | pass + | + ::: src/mdtest_snippet.py:3:1 + | + 1 | from typing import TypeVar, Generic, Protocol + 2 | + 3 | T1 = TypeVar("T1", default=int) + | ------------------------------- `T1` defined here + 4 | + 5 | T2 = TypeVar("T2") + | ------------------ `T2` defined here + 6 | T3 = TypeVar("T3") + | +info: rule `invalid-generic-class` is enabled by default + +``` + +``` +error[invalid-generic-class]: Type parameters without defaults cannot follow type parameters with defaults + --> src/mdtest_snippet.py:20:19 + | +18 | pass +19 | +20 | class Bar(Generic[T2, T1, T3]): # error: [invalid-generic-class] + | ^^^^^^^^^^ + | | + | Type variable `T3` does not have a default + | Earlier TypeVar `T1` does +21 | pass + | + ::: src/mdtest_snippet.py:3:1 + | + 1 | from typing import TypeVar, Generic, Protocol + 2 | + 3 | T1 = TypeVar("T1", default=int) + | ------------------------------- `T1` defined here + 4 | + 5 | T2 = TypeVar("T2") + 6 | T3 = TypeVar("T3") + | ------------------ `T3` defined here + 7 | + 8 | DefaultStrT = TypeVar("DefaultStrT", default=str) + | +info: rule `invalid-generic-class` is enabled by default + +``` + +``` +error[invalid-generic-class]: Type parameters without defaults cannot follow type parameters with defaults + --> src/mdtest_snippet.py:23:20 + | +21 | pass +22 | +23 | class Spam(Generic[T1, T2, DefaultStrT, T3]): # error: [invalid-generic-class] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | | + | Type variables `T2` and `T3` do not have defaults + | Earlier TypeVar `T1` does +24 | pass + | + ::: src/mdtest_snippet.py:3:1 + | + 1 | from typing import TypeVar, Generic, Protocol + 2 | + 3 | T1 = TypeVar("T1", default=int) + | ------------------------------- `T1` defined here + 4 | + 5 | T2 = TypeVar("T2") + | ------------------ `T2` defined here + 6 | T3 = TypeVar("T3") + | +info: rule `invalid-generic-class` is enabled by default + +``` + +``` +error[invalid-generic-class]: Type parameters without defaults cannot follow type parameters with defaults + --> src/mdtest_snippet.py:26:20 + | +24 | pass +25 | +26 | class Ham(Protocol[T1, T2, DefaultStrT, T3]): # error: [invalid-generic-class] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | | + | Type variables `T2` and `T3` do not have defaults + | Earlier TypeVar `T1` does +27 | pass + | + ::: src/mdtest_snippet.py:3:1 + | + 1 | from typing import TypeVar, Generic, Protocol + 2 | + 3 | T1 = TypeVar("T1", default=int) + | ------------------------------- `T1` defined here + 4 | + 5 | T2 = TypeVar("T2") + | ------------------ `T2` defined here + 6 | T3 = TypeVar("T3") + | +info: rule `invalid-generic-class` is enabled by default + +``` + +``` +error[invalid-generic-class]: Type parameters without defaults cannot follow type parameters with defaults + --> src/mdtest_snippet.py:30:14 + | +29 | class VeryBad( +30 | Protocol[T1, T2, DefaultStrT, T3], # error: [invalid-generic-class] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | | + | Type variables `T2` and `T3` do not have defaults + | Earlier TypeVar `T1` does +31 | Generic[T1, T2, DefaultStrT, T3], +32 | ): ... + | + ::: src/mdtest_snippet.py:3:1 + | + 1 | from typing import TypeVar, Generic, Protocol + 2 | + 3 | T1 = TypeVar("T1", default=int) + | ------------------------------- `T1` defined here + 4 | + 5 | T2 = TypeVar("T2") + | ------------------ `T2` defined here + 6 | T3 = TypeVar("T3") + | +info: rule `invalid-generic-class` is enabled by default + +``` diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 3acd7b0a64..e136057d45 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -30,7 +30,7 @@ use crate::types::{ ProtocolInstanceType, SpecialFormType, SubclassOfInner, Type, TypeContext, binding_type, protocol_class::ProtocolClass, }; -use crate::types::{DataclassFlags, KnownInstanceType, MemberLookupPolicy}; +use crate::types::{DataclassFlags, KnownInstanceType, MemberLookupPolicy, TypeVarInstance}; use crate::{Db, DisplaySettings, FxIndexMap, Module, ModuleName, Program, declare_lint}; use itertools::Itertools; use ruff_db::{ @@ -894,15 +894,20 @@ declare_lint! { /// /// ## Why is this bad? /// There are several requirements that you must follow when defining a generic class. + /// Many of these result in `TypeError` being raised at runtime if they are violated. /// /// ## Examples /// ```python - /// from typing import Generic, TypeVar + /// from typing_extensions import Generic, TypeVar /// - /// T = TypeVar("T") # okay + /// T = TypeVar("T") + /// U = TypeVar("U", default=int) /// /// # error: class uses both PEP-695 syntax and legacy syntax /// class C[U](Generic[T]): ... + /// + /// # error: type parameter with default comes before type parameter without default + /// class D(Generic[U, T]): ... /// ``` /// /// ## References @@ -3695,6 +3700,90 @@ pub(crate) fn report_cannot_pop_required_field_on_typed_dict<'db>( } } +pub(crate) fn report_invalid_type_param_order<'db>( + context: &InferContext<'db, '_>, + class: ClassLiteral<'db>, + node: &ast::StmtClassDef, + typevar_with_default: TypeVarInstance<'db>, + invalid_later_typevars: &[TypeVarInstance<'db>], +) { + let db = context.db(); + + let base_index = class + .explicit_bases(db) + .iter() + .position(|base| { + matches!( + base, + Type::KnownInstance( + KnownInstanceType::SubscriptedProtocol(_) + | KnownInstanceType::SubscriptedGeneric(_) + ) + ) + }) + .expect( + "It should not be possible for a class to have a legacy generic context \ + if it does not inherit from `Protocol[]` or `Generic[]`", + ); + + let base_node = &node.bases()[base_index]; + + let primary_diagnostic_range = base_node + .as_subscript_expr() + .map(|subscript| &*subscript.slice) + .unwrap_or(base_node) + .range(); + + let Some(builder) = context.report_lint(&INVALID_GENERIC_CLASS, primary_diagnostic_range) + else { + return; + }; + + let mut diagnostic = builder.into_diagnostic( + "Type parameters without defaults cannot follow type parameters with defaults", + ); + + diagnostic.set_concise_message(format_args!( + "Type parameter `{}` without a default cannot follow earlier parameter `{}` with a default", + invalid_later_typevars[0].name(db), + typevar_with_default.name(db), + )); + + if let [single_typevar] = invalid_later_typevars { + diagnostic.set_primary_message(format_args!( + "Type variable `{}` does not have a default", + single_typevar.name(db), + )); + } else { + let later_typevars = + format_enumeration(invalid_later_typevars.iter().map(|tv| tv.name(db))); + diagnostic.set_primary_message(format_args!( + "Type variables {later_typevars} do not have defaults", + )); + } + + diagnostic.annotate( + Annotation::primary(Span::from(context.file()).with_range(primary_diagnostic_range)) + .message(format_args!( + "Earlier TypeVar `{}` does", + typevar_with_default.name(db) + )), + ); + + for tvar in [typevar_with_default, invalid_later_typevars[0]] { + let Some(definition) = tvar.definition(db) else { + continue; + }; + let file = definition.file(db); + diagnostic.annotate( + Annotation::secondary(Span::from( + definition.full_range(db, &parsed_module(db, file).load(db)), + )) + .message(format_args!("`{}` defined here", tvar.name(db))), + ); + } +} + pub(crate) fn report_rebound_typevar<'db>( context: &InferContext<'db, '_>, typevar_name: &ast::name::Name, diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 9ae325a9ae..723e89519c 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -78,7 +78,7 @@ use crate::types::diagnostic::{ report_invalid_exception_tuple_caught, report_invalid_generator_function_return_type, report_invalid_key_on_typed_dict, report_invalid_or_unsupported_base, report_invalid_return_type, report_invalid_type_checking_constant, - report_named_tuple_field_with_leading_underscore, + report_invalid_type_param_order, report_named_tuple_field_with_leading_underscore, report_namedtuple_field_without_default_after_field_with_default, report_non_subscriptable, report_possibly_missing_attribute, report_possibly_unresolved_reference, report_rebound_typevar, report_slice_step_size_zero, report_unsupported_augmented_assignment, @@ -949,23 +949,62 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } } - let scope = class.body_scope(self.db()).scope(self.db()); - if self.context.is_lint_enabled(&INVALID_GENERIC_CLASS) - && let Some(parent) = scope.parent() - { - for self_typevar in class.typevars_referenced_in_definition(self.db()) { - let self_typevar_name = self_typevar.typevar(self.db()).name(self.db()); - for enclosing in enclosing_generic_contexts(self.db(), self.index, parent) { - if let Some(other_typevar) = - enclosing.binds_named_typevar(self.db(), self_typevar_name) - { - report_rebound_typevar( - &self.context, - self_typevar_name, - class, - class_node, - other_typevar, - ); + if self.context.is_lint_enabled(&INVALID_GENERIC_CLASS) { + if !class.has_pep_695_type_params(self.db()) + && let Some(generic_context) = class.legacy_generic_context(self.db()) + { + struct State<'db> { + typevar_with_default: TypeVarInstance<'db>, + invalid_later_tvars: Vec>, + } + + let mut state: Option> = None; + + for bound_typevar in generic_context.variables(self.db()) { + let typevar = bound_typevar.typevar(self.db()); + let has_default = typevar.default_type(self.db()).is_some(); + + if let Some(state) = state.as_mut() { + if !has_default { + state.invalid_later_tvars.push(typevar); + } + } else if has_default { + state = Some(State { + typevar_with_default: typevar, + invalid_later_tvars: vec![], + }); + } + } + + if let Some(state) = state + && !state.invalid_later_tvars.is_empty() + { + report_invalid_type_param_order( + &self.context, + class, + class_node, + state.typevar_with_default, + &state.invalid_later_tvars, + ); + } + } + + let scope = class.body_scope(self.db()).scope(self.db()); + if let Some(parent) = scope.parent() { + for self_typevar in class.typevars_referenced_in_definition(self.db()) { + let self_typevar_name = self_typevar.typevar(self.db()).name(self.db()); + for enclosing in enclosing_generic_contexts(self.db(), self.index, parent) { + if let Some(other_typevar) = + enclosing.binds_named_typevar(self.db(), self_typevar_name) + { + report_rebound_typevar( + &self.context, + self_typevar_name, + class, + class_node, + other_typevar, + ); + } } } } diff --git a/ty.schema.json b/ty.schema.json index 87feeb2507..74c142ec4c 100644 --- a/ty.schema.json +++ b/ty.schema.json @@ -595,7 +595,7 @@ }, "invalid-generic-class": { "title": "detects invalid generic classes", - "description": "## What it does\nChecks for the creation of invalid generic classes\n\n## Why is this bad?\nThere are several requirements that you must follow when defining a generic class.\n\n## Examples\n```python\nfrom typing import Generic, TypeVar\n\nT = TypeVar(\"T\") # okay\n\n# error: class uses both PEP-695 syntax and legacy syntax\nclass C[U](Generic[T]): ...\n```\n\n## References\n- [Typing spec: Generics](https://typing.python.org/en/latest/spec/generics.html#introduction)", + "description": "## What it does\nChecks for the creation of invalid generic classes\n\n## Why is this bad?\nThere are several requirements that you must follow when defining a generic class.\nMany of these result in `TypeError` being raised at runtime if they are violated.\n\n## Examples\n```python\nfrom typing_extensions import Generic, TypeVar\n\nT = TypeVar(\"T\")\nU = TypeVar(\"U\", default=int)\n\n# error: class uses both PEP-695 syntax and legacy syntax\nclass C[U](Generic[T]): ...\n\n# error: type parameter with default comes before type parameter without default\nclass D(Generic[U, T]): ...\n```\n\n## References\n- [Typing spec: Generics](https://typing.python.org/en/latest/spec/generics.html#introduction)", "default": "error", "oneOf": [ { From ba47349c2e874677ddc996f7c817c7b978e07a2f Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Mon, 15 Dec 2025 11:04:28 +0530 Subject: [PATCH 2/9] [ty] Use `ParamSpec` without the attr for inferable check (#21934) ## Summary fixes: https://github.com/astral-sh/ty/issues/1820 ## Test Plan Add new mdtests. Ecosystem changes removes all false positives. --- .../mdtest/generics/pep695/paramspec.md | 56 +++++++++++++++++++ .../src/types/signatures.rs | 23 ++++++++ 2 files changed, 79 insertions(+) diff --git a/crates/ty_python_semantic/resources/mdtest/generics/pep695/paramspec.md b/crates/ty_python_semantic/resources/mdtest/generics/pep695/paramspec.md index 354521288e..e6e6acd35c 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/paramspec.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/paramspec.md @@ -670,3 +670,59 @@ reveal_type(with_parameters(int_int, 1)) # revealed: Overload[(x: int) -> str, # error: [invalid-argument-type] reveal_type(with_parameters(int_int, "a")) # revealed: Overload[(x: int) -> str, (x: str) -> str] ``` + +## ParamSpec attribute assignability + +When comparing signatures with `ParamSpec` attributes (`P.args` and `P.kwargs`), two different +inferable `ParamSpec` attributes with the same kind are assignable to each other. This enables +method overrides where both methods have their own `ParamSpec`. + +### Same attribute kind, both inferable + +```py +from typing import Callable + +class Parent: + def method[**P](self, callback: Callable[P, None]) -> Callable[P, None]: + return callback + +class Child1(Parent): + # This is a valid override: Q.args matches P.args, Q.kwargs matches P.kwargs + def method[**Q](self, callback: Callable[Q, None]) -> Callable[Q, None]: + return callback + +# Both signatures use ParamSpec, so they should be compatible +def outer[**P](f: Callable[P, int]) -> Callable[P, int]: + def inner[**Q](g: Callable[Q, int]) -> Callable[Q, int]: + return g + return inner(f) +``` + +We can explicitly mark it as an override using the `@override` decorator. + +```py +from typing import override + +class Child2(Parent): + @override + def method[**Q](self, callback: Callable[Q, None]) -> Callable[Q, None]: + return callback +``` + +### One `ParamSpec` not inferable + +Here, `P` is in a non-inferable position while `Q` is inferable. So, they are not considered +assignable. + +```py +from typing import Callable + +class Container[**P]: + def method(self, f: Callable[P, None]) -> Callable[P, None]: + return f + + def try_assign[**Q](self, f: Callable[Q, None]) -> Callable[Q, None]: + # error: [invalid-return-type] "Return type does not match returned value: expected `(**Q@try_assign) -> None`, found `(**P@Container) -> None`" + # error: [invalid-argument-type] "Argument to bound method `method` is incorrect: Expected `(**P@Container) -> None`, found `(**Q@try_assign) -> None`" + return self.method(f) +``` diff --git a/crates/ty_python_semantic/src/types/signatures.rs b/crates/ty_python_semantic/src/types/signatures.rs index 45c3f81de2..76fe3a35d4 100644 --- a/crates/ty_python_semantic/src/types/signatures.rs +++ b/crates/ty_python_semantic/src/types/signatures.rs @@ -1072,6 +1072,29 @@ impl<'db> Signature<'db> { let mut check_types = |type1: Option>, type2: Option>| { let type1 = type1.unwrap_or(Type::unknown()); let type2 = type2.unwrap_or(Type::unknown()); + + match (type1, type2) { + // This is a special case where the _same_ components of two different `ParamSpec` + // type variables are assignable to each other when they're both in an inferable + // position. + // + // `ParamSpec` type variables can only occur in parameter lists so this special case + // is present here instead of in `Type::has_relation_to_impl`. + (Type::TypeVar(typevar1), Type::TypeVar(typevar2)) + if typevar1.paramspec_attr(db).is_some() + && typevar1.paramspec_attr(db) == typevar2.paramspec_attr(db) + && typevar1 + .without_paramspec_attr(db) + .is_inferable(db, inferable) + && typevar2 + .without_paramspec_attr(db) + .is_inferable(db, inferable) => + { + return true; + } + _ => {} + } + !result .intersect( db, From 9838f81bafd521019ac715437cafa13a84e1eb19 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 15 Dec 2025 06:52:52 +0000 Subject: [PATCH 3/9] Update actions/checkout digest to 8e8c483 (#21982) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Micha Reiser --- .github/workflows/release.yml | 8 ++++---- dist-workspace.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6cec5a828f..abee17bfc1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -60,7 +60,7 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: - - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 with: persist-credentials: false submodules: recursive @@ -123,7 +123,7 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} BUILD_MANIFEST_NAME: target/distrib/global-dist-manifest.json steps: - - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 with: persist-credentials: false submodules: recursive @@ -174,7 +174,7 @@ jobs: outputs: val: ${{ steps.host.outputs.manifest }} steps: - - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 with: persist-credentials: false submodules: recursive @@ -250,7 +250,7 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: - - uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 with: persist-credentials: false submodules: recursive diff --git a/dist-workspace.toml b/dist-workspace.toml index 809beefa73..10bec6d868 100644 --- a/dist-workspace.toml +++ b/dist-workspace.toml @@ -66,7 +66,7 @@ install-path = ["$XDG_BIN_HOME/", "$XDG_DATA_HOME/../bin", "~/.local/bin"] global = "depot-ubuntu-latest-4" [dist.github-action-commits] -"actions/checkout" = "1af3b93b6815bc44a9784bd300feb67ff0d1eeb3" # v6.0.0 +"actions/checkout" = "8e8c483db84b4bee98b60c0593521ed34d9990e8" # v6.0.1 "actions/upload-artifact" = "330a01c490aca151604b8cf639adc76d48f6c5d4" # v5.0.0 "actions/download-artifact" = "018cc2cf5baa6db3ef3c5f8a56943fffe632ef53" # v6.0.0 "actions/attest-build-provenance" = "c074443f1aee8d4aeeae555aebba3282517141b2" #v2.2.3 From 0b918ae4d53e68ce4b1ab34be87054597f845d0c Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 15 Dec 2025 08:56:35 +0000 Subject: [PATCH 4/9] [ty] Improve check enforcing that an overloaded function must have an implementation (#21978) ## Summary - Treat `if TYPE_CHECKING` blocks the same as stub files (the feature requested in https://github.com/astral-sh/ty/issues/1216) - We currently only allow `@abstractmethod`-decorated methods to omit the implementation if they're methods in classes that have _exactly_ `ABCMeta` as their metaclass. That seems wrong -- `@abstractmethod` has the same semantics if a class has a subclass of `ABCMeta` as its metaclass. This PR fixes that too. (I'm actually not _totally_ sure we should care what the class's metaclass is at all -- see discussion in https://github.com/astral-sh/ty/issues/1877#issue-3725937441... but the change this PR is making seems less wrong than what we have currently, anyway.) Fixes https://github.com/astral-sh/ty/issues/1216 ## Test Plan Mdtests and snapshots --- .../resources/mdtest/overloads.md | 58 +++++++++++++++++++ ...€¦_-_Regular_modules_(5c8e81664d1c7470).snap | 12 +++- crates/ty_python_semantic/src/types/class.rs | 7 --- .../src/types/infer/builder.rs | 23 ++++++-- 4 files changed, 87 insertions(+), 13 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/overloads.md b/crates/ty_python_semantic/resources/mdtest/overloads.md index f74cafb80b..f2fd9b7595 100644 --- a/crates/ty_python_semantic/resources/mdtest/overloads.md +++ b/crates/ty_python_semantic/resources/mdtest/overloads.md @@ -418,6 +418,18 @@ Using the `@abstractmethod` decorator requires that the class's metaclass is `AB from it. ```py +from abc import ABCMeta + +class CustomAbstractMetaclass(ABCMeta): ... + +class Fine(metaclass=CustomAbstractMetaclass): + @overload + @abstractmethod + def f(self, x: int) -> int: ... + @overload + @abstractmethod + def f(self, x: str) -> str: ... + class Foo: @overload @abstractmethod @@ -448,6 +460,52 @@ class PartialFoo(ABC): def f(self, x: str) -> str: ... ``` +#### `TYPE_CHECKING` blocks + +As in other areas of ty, we treat `TYPE_CHECKING` blocks the same as "inline stub files", so we +permit overloaded functions to exist without an implementation if all overloads are defined inside +an `if TYPE_CHECKING` block: + +```py +from typing import overload, TYPE_CHECKING + +if TYPE_CHECKING: + @overload + def a() -> str: ... + @overload + def a(x: int) -> int: ... + + class F: + @overload + def method(self) -> None: ... + @overload + def method(self, x: int) -> int: ... + +class G: + if TYPE_CHECKING: + @overload + def method(self) -> None: ... + @overload + def method(self, x: int) -> int: ... + +if TYPE_CHECKING: + @overload + def b() -> str: ... + +if TYPE_CHECKING: + @overload + def b(x: int) -> int: ... + +if TYPE_CHECKING: + @overload + def c() -> None: ... + +# not all overloads are in a `TYPE_CHECKING` block, so this is an error +@overload +# error: [invalid-overload] +def c(x: int) -> int: ... +``` + ### `@overload`-decorated functions with non-stub bodies diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/overloads.md_-_Overloads_-_Invalid_-_Overload_without_an_…_-_Regular_modules_(5c8e81664d1c7470).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/overloads.md_-_Overloads_-_Invalid_-_Overload_without_an_…_-_Regular_modules_(5c8e81664d1c7470).snap index 21cc311cd8..257cc54b78 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/overloads.md_-_Overloads_-_Invalid_-_Overload_without_an_…_-_Regular_modules_(5c8e81664d1c7470).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/overloads.md_-_Overloads_-_Invalid_-_Overload_without_an_…_-_Regular_modules_(5c8e81664d1c7470).snap @@ -42,7 +42,11 @@ error[invalid-overload]: Overloads for function `func` must be followed by a non 9 | class Foo: | info: Attempting to call `func` will raise `TypeError` at runtime -info: Overloaded functions without implementations are only permitted in stub files, on protocols, or for abstract methods +info: Overloaded functions without implementations are only permitted: +info: - in stub files +info: - in `if TYPE_CHECKING` blocks +info: - as methods on protocol classes +info: - or as `@abstractmethod`-decorated methods on abstract classes info: See https://docs.python.org/3/library/typing.html#typing.overload for more details info: rule `invalid-overload` is enabled by default @@ -58,7 +62,11 @@ error[invalid-overload]: Overloads for function `method` must be followed by a n | ^^^^^^ | info: Attempting to call `method` will raise `TypeError` at runtime -info: Overloaded functions without implementations are only permitted in stub files, on protocols, or for abstract methods +info: Overloaded functions without implementations are only permitted: +info: - in stub files +info: - in `if TYPE_CHECKING` blocks +info: - as methods on protocol classes +info: - or as `@abstractmethod`-decorated methods on abstract classes info: See https://docs.python.org/3/library/typing.html#typing.overload for more details info: rule `invalid-overload` is enabled by default diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 022e63fe33..61ee82e030 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -1815,13 +1815,6 @@ impl<'db> ClassLiteral<'db> { }) } - /// Determine if this is an abstract class. - pub(super) fn is_abstract(self, db: &'db dyn Db) -> bool { - self.metaclass(db) - .as_class_literal() - .is_some_and(|metaclass| metaclass.is_known(db, KnownClass::ABCMeta)) - } - /// Return the types of the decorators on this class #[salsa::tracked(returns(deref), cycle_initial=decorators_cycle_initial, heap_size=ruff_memory_usage::heap_size)] fn decorators(self, db: &'db dyn Db) -> Box<[Type<'db>]> { diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 723e89519c..d691b46cf9 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -1143,7 +1143,16 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { if implementation.is_none() && !self.in_stub() { let mut implementation_required = true; - if let NodeWithScopeKind::Class(class_node_ref) = scope { + if function + .iter_overloads_and_implementation(self.db()) + .all(|f| { + f.body_scope(self.db()) + .scope(self.db()) + .in_type_checking_block() + }) + { + implementation_required = false; + } else if let NodeWithScopeKind::Class(class_node_ref) = scope { let class = binding_type( self.db(), self.index @@ -1152,7 +1161,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { .expect_class_literal(); if class.is_protocol(self.db()) - || (class.is_abstract(self.db()) + || (Type::ClassLiteral(class) + .is_subtype_of(self.db(), KnownClass::ABCMeta.to_instance(self.db())) && overloads.iter().all(|overload| { overload.has_known_decorator( self.db(), @@ -1179,8 +1189,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { &function_node.name )); diagnostic.info( - "Overloaded functions without implementations are only permitted \ - in stub files, on protocols, or for abstract methods", + "Overloaded functions without implementations are only permitted:", + ); + diagnostic.info(" - in stub files"); + diagnostic.info(" - in `if TYPE_CHECKING` blocks"); + diagnostic.info(" - as methods on protocol classes"); + diagnostic.info( + " - or as `@abstractmethod`-decorated methods on abstract classes", ); diagnostic.info( "See https://docs.python.org/3/library/typing.html#typing.overload \ From d08e41417971f1d05b9daa75f794536a1dd4bedf Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 15 Dec 2025 14:29:11 +0100 Subject: [PATCH 5/9] Update MSRV to 1.90 (#21987) --- Cargo.toml | 2 +- crates/ruff/src/args.rs | 2 +- crates/ruff/src/lib.rs | 2 +- crates/ruff_formatter/src/macros.rs | 4 ++-- crates/ruff_linter/src/fix/edits.rs | 7 +------ .../src/rules/flake8_simplify/rules/yoda_conditions.rs | 2 +- crates/ruff_python_codegen/src/generator.rs | 1 + crates/ruff_python_formatter/src/cli.rs | 2 +- crates/ty_ide/src/completion.rs | 3 +-- crates/ty_project/src/lib.rs | 1 - crates/ty_python_semantic/src/types.rs | 1 - rust-toolchain.toml | 2 +- 12 files changed, 11 insertions(+), 18 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dbd9808fdd..bd06571cd3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ resolver = "2" [workspace.package] # Please update rustfmt.toml when bumping the Rust edition edition = "2024" -rust-version = "1.89" +rust-version = "1.90" homepage = "https://docs.astral.sh/ruff" documentation = "https://docs.astral.sh/ruff" repository = "https://github.com/astral-sh/ruff" diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index 370186a0b4..da6d3833b7 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -10,7 +10,7 @@ use anyhow::bail; use clap::builder::Styles; use clap::builder::styling::{AnsiColor, Effects}; use clap::builder::{TypedValueParser, ValueParserFactory}; -use clap::{Parser, Subcommand, command}; +use clap::{Parser, Subcommand}; use colored::Colorize; use itertools::Itertools; use path_absolutize::path_dedot; diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index 3ea0d94fad..3ecefd6dde 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -9,7 +9,7 @@ use std::sync::mpsc::channel; use anyhow::Result; use clap::CommandFactory; use colored::Colorize; -use log::{error, warn}; +use log::error; use notify::{RecursiveMode, Watcher, recommended_watcher}; use args::{GlobalConfigArgs, ServerCommand}; diff --git a/crates/ruff_formatter/src/macros.rs b/crates/ruff_formatter/src/macros.rs index 4d0d3ef234..8090d41397 100644 --- a/crates/ruff_formatter/src/macros.rs +++ b/crates/ruff_formatter/src/macros.rs @@ -337,7 +337,7 @@ macro_rules! best_fitting { #[cfg(test)] mod tests { use crate::prelude::*; - use crate::{FormatState, SimpleFormatOptions, VecBuffer, write}; + use crate::{FormatState, SimpleFormatOptions, VecBuffer}; struct TestFormat; @@ -385,8 +385,8 @@ mod tests { #[test] fn best_fitting_variants_print_as_lists() { + use crate::Formatted; use crate::prelude::*; - use crate::{Formatted, format, format_args}; // The second variant below should be selected when printing at a width of 30 let formatted_best_fitting = format!( diff --git a/crates/ruff_linter/src/fix/edits.rs b/crates/ruff_linter/src/fix/edits.rs index 20f50d6e10..b5420e4ee1 100644 --- a/crates/ruff_linter/src/fix/edits.rs +++ b/crates/ruff_linter/src/fix/edits.rs @@ -286,12 +286,7 @@ pub(crate) fn add_argument(argument: &str, arguments: &Arguments, tokens: &Token /// Generic function to add a (regular) parameter to a function definition. pub(crate) fn add_parameter(parameter: &str, parameters: &Parameters, source: &str) -> Edit { - if let Some(last) = parameters - .args - .iter() - .filter(|arg| arg.default.is_none()) - .next_back() - { + if let Some(last) = parameters.args.iter().rfind(|arg| arg.default.is_none()) { // Case 1: at least one regular parameter, so append after the last one. Edit::insertion(format!(", {parameter}"), last.end()) } else if !parameters.args.is_empty() { diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs index 687729c20c..84aff5ce5e 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs @@ -146,7 +146,7 @@ fn reverse_comparison(expr: &Expr, locator: &Locator, stylist: &Stylist) -> Resu let left = (*comparison.left).clone(); // Copy the right side to the left side. - comparison.left = Box::new(comparison.comparisons[0].comparator.clone()); + *comparison.left = comparison.comparisons[0].comparator.clone(); // Copy the left side to the right side. comparison.comparisons[0].comparator = left; diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index 710e295f62..362d00d235 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1247,6 +1247,7 @@ impl<'a> Generator<'a> { self.p_bytes_repr(&bytes_literal.value, bytes_literal.flags); } } + #[expect(clippy::eq_op)] Expr::NumberLiteral(ast::ExprNumberLiteral { value, .. }) => { static INF_STR: &str = "1e309"; assert_eq!(f64::MAX_10_EXP, 308); diff --git a/crates/ruff_python_formatter/src/cli.rs b/crates/ruff_python_formatter/src/cli.rs index 96da64e86e..df289f08d8 100644 --- a/crates/ruff_python_formatter/src/cli.rs +++ b/crates/ruff_python_formatter/src/cli.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; -use clap::{Parser, ValueEnum, command}; +use clap::{Parser, ValueEnum}; use ruff_formatter::SourceCode; use ruff_python_ast::{PySourceType, PythonVersion}; diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index ca2305df0c..4e5051ddcc 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -4711,8 +4711,7 @@ from os. let last_nonunderscore = test .completions() .iter() - .filter(|c| !c.name.starts_with('_')) - .next_back() + .rfind(|c| !c.name.starts_with('_')) .unwrap(); assert_eq!(&last_nonunderscore.name, "type_check_only"); diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index fe034b0a07..2bcb287b40 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -27,7 +27,6 @@ use std::iter::FusedIterator; use std::panic::{AssertUnwindSafe, UnwindSafe}; use std::sync::Arc; use thiserror::Error; -use tracing::error; use ty_python_semantic::add_inferred_python_version_hint_to_diagnostic; use ty_python_semantic::lint::RuleSelection; use ty_python_semantic::types::check_types; diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 726decfc07..b318638742 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -10691,7 +10691,6 @@ pub struct UnionTypeInstance<'db> { /// ``. For `Union[int, str]`, this field is `None`, as we infer /// the elements as type expressions. Use `value_expression_types` to get the /// corresponding value expression types. - #[expect(clippy::ref_option)] #[returns(ref)] _value_expr_types: Option]>>, diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 1a35d66439..50b3f5d474 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,2 +1,2 @@ [toolchain] -channel = "1.91" +channel = "1.92" From 5372bb344054135054205743c60a4c6b8646df8e Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 15 Dec 2025 16:04:34 +0100 Subject: [PATCH 6/9] [ty] Use jemalloc on linux (#21975) Co-authored-by: Claude --- Cargo.lock | 1 + crates/ty/Cargo.toml | 6 ++++++ crates/ty/src/main.rs | 16 ++++++++++++++++ scripts/ty_benchmark/src/benchmark/run.py | 7 ++++++- 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 9c5a084a58..cda8a8261d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4390,6 +4390,7 @@ dependencies = [ "ruff_python_trivia", "salsa", "tempfile", + "tikv-jemallocator", "toml", "tracing", "tracing-flame", diff --git a/crates/ty/Cargo.toml b/crates/ty/Cargo.toml index 4189758bb1..53a5bb7b15 100644 --- a/crates/ty/Cargo.toml +++ b/crates/ty/Cargo.toml @@ -51,5 +51,11 @@ regex = { workspace = true } tempfile = { workspace = true } toml = { workspace = true } +[features] +default = [] + +[target.'cfg(all(not(target_os = "macos"), not(target_os = "windows"), not(target_os = "openbsd"), not(target_os = "aix"), not(target_os = "android"), any(target_arch = "x86_64", target_arch = "aarch64", target_arch = "powerpc64", target_arch = "riscv64")))'.dependencies] +tikv-jemallocator = { workspace = true } + [lints] workspace = true diff --git a/crates/ty/src/main.rs b/crates/ty/src/main.rs index 6dbae583fe..102ec184e3 100644 --- a/crates/ty/src/main.rs +++ b/crates/ty/src/main.rs @@ -2,6 +2,22 @@ use colored::Colorize; use std::io; use ty::{ExitStatus, run}; +#[cfg(all( + not(target_os = "macos"), + not(target_os = "windows"), + not(target_os = "openbsd"), + not(target_os = "aix"), + not(target_os = "android"), + any( + target_arch = "x86_64", + target_arch = "aarch64", + target_arch = "powerpc64", + target_arch = "riscv64" + ) +))] +#[global_allocator] +static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; + pub fn main() -> ExitStatus { run().unwrap_or_else(|error| { use io::Write; diff --git a/scripts/ty_benchmark/src/benchmark/run.py b/scripts/ty_benchmark/src/benchmark/run.py index 10e412dcd8..0c0cb34791 100644 --- a/scripts/ty_benchmark/src/benchmark/run.py +++ b/scripts/ty_benchmark/src/benchmark/run.py @@ -56,6 +56,7 @@ def main() -> None: parser.add_argument( "--ty-path", + action="append", type=Path, help="Path to the ty binary to benchmark.", ) @@ -108,7 +109,11 @@ def main() -> None: for tool_name in args.tool or TOOL_CHOICES: match tool_name: case "ty": - suites.append(Ty(path=args.ty_path)) + if args.ty_path: + for path in args.ty_path: + suites.append(Ty(path=path)) + else: + suites.append(Ty()) case "pyrefly": suites.append(Pyrefly()) case "pyright": From 8f530a7ab0643028dbbb44b7a5a1e42ffe67679d Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Mon, 15 Dec 2025 10:14:36 -0500 Subject: [PATCH 7/9] [ty] Add "qualify ..." code fix for undefined references (#21968) ## Summary If `import warnings` exists in the file, we will suggest an edit of `deprecated -> warnings.deprecated` as "qualify warnings.deprecated" ## Test Plan Should test more cases... --- crates/ty_ide/src/code_action.rs | 462 +++++++++++++----- crates/ty_ide/src/completion.rs | 48 +- ...n_existing_import_undefined_decorator.snap | 45 ++ 3 files changed, 428 insertions(+), 127 deletions(-) diff --git a/crates/ty_ide/src/code_action.rs b/crates/ty_ide/src/code_action.rs index 2b6703afac..5c4a769dd7 100644 --- a/crates/ty_ide/src/code_action.rs +++ b/crates/ty_ide/src/code_action.rs @@ -29,12 +29,11 @@ pub fn code_actions( let mut actions = Vec::new(); - // Suggest imports for unresolved references (often ideal) - // TODO: suggest qualifying with an already imported symbol + // Suggest imports/qualifications for unresolved references (often ideal) let is_unresolved_reference = lint_id == LintId::of(&UNRESOLVED_REFERENCE) || lint_id == LintId::of(&UNDEFINED_REVEAL); if is_unresolved_reference - && let Some(import_quick_fix) = create_import_symbol_quick_fix(db, file, diagnostic_range) + && let Some(import_quick_fix) = unresolved_fixes(db, file, diagnostic_range) { actions.extend(import_quick_fix); } @@ -49,7 +48,7 @@ pub fn code_actions( actions } -fn create_import_symbol_quick_fix( +fn unresolved_fixes( db: &dyn Db, file: File, diagnostic_range: TextRange, @@ -59,7 +58,7 @@ fn create_import_symbol_quick_fix( let symbol = &node.expr_name()?.id; Some( - completion::missing_imports(db, file, &parsed, symbol, node) + completion::unresolved_fixes(db, file, &parsed, symbol, node) .into_iter() .map(|import| QuickFix { title: import.label, @@ -84,6 +83,7 @@ mod tests { system::{DbWithWritableSystem, SystemPathBuf}, }; use ruff_diagnostics::Fix; + use ruff_python_trivia::textwrap::dedent; use ruff_text_size::{TextRange, TextSize}; use ty_project::ProjectMetadata; use ty_python_semantic::{ @@ -149,15 +149,14 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:2:17 + --> main.py:2:5 | - 2 | b = a / 0 # ty:ignore[division-by-zero] - | ^ + 2 | b = a / 0 # ty:ignore[division-by-zero] + | ^ | 1 | - - b = a / 0 # ty:ignore[division-by-zero] - 2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference] - 3 | + - b = a / 0 # ty:ignore[division-by-zero] + 2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference] "); } @@ -171,15 +170,14 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:2:17 + --> main.py:2:5 | - 2 | b = a / 0 # ty:ignore[division-by-zero,] - | ^ + 2 | b = a / 0 # ty:ignore[division-by-zero,] + | ^ | 1 | - - b = a / 0 # ty:ignore[division-by-zero,] - 2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference] - 3 | + - b = a / 0 # ty:ignore[division-by-zero,] + 2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference] "); } @@ -193,15 +191,14 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:2:17 + --> main.py:2:5 | - 2 | b = a / 0 # ty:ignore[division-by-zero ] - | ^ + 2 | b = a / 0 # ty:ignore[division-by-zero ] + | ^ | 1 | - - b = a / 0 # ty:ignore[division-by-zero ] - 2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference ] - 3 | + - b = a / 0 # ty:ignore[division-by-zero ] + 2 + b = a / 0 # ty:ignore[division-by-zero, unresolved-reference ] "); } @@ -215,15 +212,14 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:2:17 + --> main.py:2:5 | - 2 | b = a / 0 # ty:ignore[division-by-zero] some explanation - | ^ + 2 | b = a / 0 # ty:ignore[division-by-zero] some explanation + | ^ | 1 | - - b = a / 0 # ty:ignore[division-by-zero] some explanation - 2 + b = a / 0 # ty:ignore[division-by-zero] some explanation # ty:ignore[unresolved-reference] - 3 | + - b = a / 0 # ty:ignore[division-by-zero] some explanation + 2 + b = a / 0 # ty:ignore[division-by-zero] some explanation # ty:ignore[unresolved-reference] "); } @@ -241,22 +237,22 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:3:21 + --> main.py:3:9 | - 2 | b = ( - 3 | / a # ty:ignore[division-by-zero] - 4 | | / - 5 | | 0 - | |_____________________^ - 6 | ) + 2 | b = ( + 3 | / a # ty:ignore[division-by-zero] + 4 | | / + 5 | | 0 + | |_________^ + 6 | ) | 1 | - 2 | b = ( - - a # ty:ignore[division-by-zero] - 3 + a # ty:ignore[division-by-zero, unresolved-reference] - 4 | / - 5 | 0 - 6 | ) + 2 | b = ( + - a # ty:ignore[division-by-zero] + 3 + a # ty:ignore[division-by-zero, unresolved-reference] + 4 | / + 5 | 0 + 6 | ) "); } @@ -274,22 +270,21 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:3:21 + --> main.py:3:9 | - 2 | b = ( - 3 | / a - 4 | | / - 5 | | 0 # ty:ignore[division-by-zero] - | |_____________________^ - 6 | ) + 2 | b = ( + 3 | / a + 4 | | / + 5 | | 0 # ty:ignore[division-by-zero] + | |_________^ + 6 | ) | - 2 | b = ( - 3 | a - 4 | / - - 0 # ty:ignore[division-by-zero] - 5 + 0 # ty:ignore[division-by-zero, unresolved-reference] - 6 | ) - 7 | + 2 | b = ( + 3 | a + 4 | / + - 0 # ty:ignore[division-by-zero] + 5 + 0 # ty:ignore[division-by-zero, unresolved-reference] + 6 | ) "); } @@ -307,22 +302,22 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:3:21 + --> main.py:3:9 | - 2 | b = ( - 3 | / a # ty:ignore[division-by-zero] - 4 | | / - 5 | | 0 # ty:ignore[division-by-zero] - | |_____________________^ - 6 | ) + 2 | b = ( + 3 | / a # ty:ignore[division-by-zero] + 4 | | / + 5 | | 0 # ty:ignore[division-by-zero] + | |_________^ + 6 | ) | 1 | - 2 | b = ( - - a # ty:ignore[division-by-zero] - 3 + a # ty:ignore[division-by-zero, unresolved-reference] - 4 | / - 5 | 0 # ty:ignore[division-by-zero] - 6 | ) + 2 | b = ( + - a # ty:ignore[division-by-zero] + 3 + a # ty:ignore[division-by-zero, unresolved-reference] + 4 | / + 5 | 0 # ty:ignore[division-by-zero] + 6 | ) "); } @@ -339,20 +334,19 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:3:18 + --> main.py:3:6 | - 2 | b = f""" - 3 | {a} - | ^ - 4 | more text - 5 | """ + 2 | b = f""" + 3 | {a} + | ^ + 4 | more text + 5 | """ | - 2 | b = f""" - 3 | {a} - 4 | more text - - """ - 5 + """ # ty:ignore[unresolved-reference] - 6 | + 2 | b = f""" + 3 | {a} + 4 | more text + - """ + 5 + """ # ty:ignore[unresolved-reference] "#); } @@ -371,23 +365,23 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:4:17 + --> main.py:4:5 | - 2 | b = f""" - 3 | { - 4 | a - | ^ - 5 | } - 6 | more text + 2 | b = f""" + 3 | { + 4 | a + | ^ + 5 | } + 6 | more text | 1 | - 2 | b = f""" - 3 | { - - a - 4 + a # ty:ignore[unresolved-reference] - 5 | } - 6 | more text - 7 | """ + 2 | b = f""" + 3 | { + - a + 4 + a # ty:ignore[unresolved-reference] + 5 | } + 6 | more text + 7 | """ "#); } @@ -403,19 +397,18 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:2:17 + --> main.py:2:5 | - 2 | b = a + """ - | ^ - 3 | more text - 4 | """ + 2 | b = a + """ + | ^ + 3 | more text + 4 | """ | 1 | - 2 | b = a + """ - 3 | more text - - """ - 4 + """ # ty:ignore[unresolved-reference] - 5 | + 2 | b = a + """ + 3 | more text + - """ + 4 + """ # ty:ignore[unresolved-reference] "#); } @@ -430,17 +423,16 @@ mod tests { assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" info[code-action]: Ignore 'unresolved-reference' for this line - --> main.py:2:17 + --> main.py:2:5 | - 2 | b = a \ - | ^ - 3 | + "test" + 2 | b = a \ + | ^ + 3 | + "test" | 1 | - 2 | b = a \ - - + "test" - 3 + + "test" # ty:ignore[unresolved-reference] - 4 | + 2 | b = a \ + - + "test" + 3 + + "test" # ty:ignore[unresolved-reference] "#); } @@ -454,27 +446,249 @@ mod tests { assert_snapshot!(test.code_actions(&UNDEFINED_REVEAL), @r" info[code-action]: import typing.reveal_type - --> main.py:2:13 + --> main.py:2:1 | - 2 | reveal_type(1) - | ^^^^^^^^^^^ + 2 | reveal_type(1) + | ^^^^^^^^^^^ | help: This is a preferred code action 1 + from typing import reveal_type 2 | - 3 | reveal_type(1) - 4 | + 3 | reveal_type(1) info[code-action]: Ignore 'undefined-reveal' for this line - --> main.py:2:13 + --> main.py:2:1 | - 2 | reveal_type(1) - | ^^^^^^^^^^^ + 2 | reveal_type(1) + | ^^^^^^^^^^^ | 1 | - - reveal_type(1) - 2 + reveal_type(1) # ty:ignore[undefined-reveal] + - reveal_type(1) + 2 + reveal_type(1) # ty:ignore[undefined-reveal] + "); + } + + #[test] + fn unresolved_deprecated() { + let test = CodeActionTest::with_source( + r#" + @deprecated("do not use") + def my_func(): ... + "#, + ); + + assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" + info[code-action]: import warnings.deprecated + --> main.py:2:2 + | + 2 | @deprecated("do not use") + | ^^^^^^^^^^ + 3 | def my_func(): ... + | + help: This is a preferred code action + 1 + from warnings import deprecated + 2 | + 3 | @deprecated("do not use") + 4 | def my_func(): ... + + info[code-action]: Ignore 'unresolved-reference' for this line + --> main.py:2:2 + | + 2 | @deprecated("do not use") + | ^^^^^^^^^^ + 3 | def my_func(): ... + | + 1 | + - @deprecated("do not use") + 2 + @deprecated("do not use") # ty:ignore[unresolved-reference] + 3 | def my_func(): ... + "#); + } + + #[test] + fn unresolved_deprecated_warnings_imported() { + let test = CodeActionTest::with_source( + r#" + import warnings + + @deprecated("do not use") + def my_func(): ... + "#, + ); + + assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r#" + info[code-action]: import warnings.deprecated + --> main.py:4:2 + | + 2 | import warnings 3 | + 4 | @deprecated("do not use") + | ^^^^^^^^^^ + 5 | def my_func(): ... + | + help: This is a preferred code action + 1 + from warnings import deprecated + 2 | + 3 | import warnings + 4 | + + info[code-action]: qualify warnings.deprecated + --> main.py:4:2 + | + 2 | import warnings + 3 | + 4 | @deprecated("do not use") + | ^^^^^^^^^^ + 5 | def my_func(): ... + | + help: This is a preferred code action + 1 | + 2 | import warnings + 3 | + - @deprecated("do not use") + 4 + @warnings.deprecated("do not use") + 5 | def my_func(): ... + + info[code-action]: Ignore 'unresolved-reference' for this line + --> main.py:4:2 + | + 2 | import warnings + 3 | + 4 | @deprecated("do not use") + | ^^^^^^^^^^ + 5 | def my_func(): ... + | + 1 | + 2 | import warnings + 3 | + - @deprecated("do not use") + 4 + @deprecated("do not use") # ty:ignore[unresolved-reference] + 5 | def my_func(): ... + "#); + } + + // using `importlib.abc.ExecutionLoader` when no imports are in scope + #[test] + fn unresolved_loader() { + let test = CodeActionTest::with_source( + r#" + ExecutionLoader + "#, + ); + + assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" + info[code-action]: import importlib.abc.ExecutionLoader + --> main.py:2:1 + | + 2 | ExecutionLoader + | ^^^^^^^^^^^^^^^ + | + help: This is a preferred code action + 1 + from importlib.abc import ExecutionLoader + 2 | + 3 | ExecutionLoader + + info[code-action]: Ignore 'unresolved-reference' for this line + --> main.py:2:1 + | + 2 | ExecutionLoader + | ^^^^^^^^^^^^^^^ + | + 1 | + - ExecutionLoader + 2 + ExecutionLoader # ty:ignore[unresolved-reference] + "); + } + + // using `importlib.abc.ExecutionLoader` when `import importlib` is in scope + // + // TODO: `importlib.abc` is available whenever `importlib` is, so qualifying + // `importlib.abc.ExecutionLoader` without adding imports is actually legal here! + #[test] + fn unresolved_loader_importlib_imported() { + let test = CodeActionTest::with_source( + r#" + import importlib + ExecutionLoader + "#, + ); + + assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" + info[code-action]: import importlib.abc.ExecutionLoader + --> main.py:3:1 + | + 2 | import importlib + 3 | ExecutionLoader + | ^^^^^^^^^^^^^^^ + | + help: This is a preferred code action + 1 + from importlib.abc import ExecutionLoader + 2 | + 3 | import importlib + 4 | ExecutionLoader + + info[code-action]: Ignore 'unresolved-reference' for this line + --> main.py:3:1 + | + 2 | import importlib + 3 | ExecutionLoader + | ^^^^^^^^^^^^^^^ + | + 1 | + 2 | import importlib + - ExecutionLoader + 3 + ExecutionLoader # ty:ignore[unresolved-reference] + "); + } + + // Using `importlib.abc.ExecutionLoader` when `import importlib.abc` is in scope + #[test] + fn unresolved_loader_abc_imported() { + let test = CodeActionTest::with_source( + r#" + import importlib.abc + ExecutionLoader + "#, + ); + + assert_snapshot!(test.code_actions(&UNRESOLVED_REFERENCE), @r" + info[code-action]: import importlib.abc.ExecutionLoader + --> main.py:3:1 + | + 2 | import importlib.abc + 3 | ExecutionLoader + | ^^^^^^^^^^^^^^^ + | + help: This is a preferred code action + 1 + from importlib.abc import ExecutionLoader + 2 | + 3 | import importlib.abc + 4 | ExecutionLoader + + info[code-action]: qualify importlib.abc.ExecutionLoader + --> main.py:3:1 + | + 2 | import importlib.abc + 3 | ExecutionLoader + | ^^^^^^^^^^^^^^^ + | + help: This is a preferred code action + 1 | + 2 | import importlib.abc + - ExecutionLoader + 3 + importlib.abc.ExecutionLoader + + info[code-action]: Ignore 'unresolved-reference' for this line + --> main.py:3:1 + | + 2 | import importlib.abc + 3 | ExecutionLoader + | ^^^^^^^^^^^^^^^ + | + 1 | + 2 | import importlib.abc + - ExecutionLoader + 3 + ExecutionLoader # ty:ignore[unresolved-reference] "); } @@ -493,7 +707,7 @@ mod tests { db.init_program().unwrap(); - let mut cleansed = source.to_string(); + let mut cleansed = dedent(source).to_string(); let start = cleansed .find("") diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 4e5051ddcc..a33f7600eb 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -67,6 +67,7 @@ impl<'db> Completions<'db> { self.items } + // Convert this collection into a list of "import..." fixes fn into_imports(mut self) -> Vec { self.items.sort_by(compare_suggestions); self.items @@ -82,6 +83,28 @@ impl<'db> Completions<'db> { .collect() } + // Convert this collection into a list of "qualify..." fixes + fn into_qualifications(mut self, range: TextRange) -> Vec { + self.items.sort_by(compare_suggestions); + self.items + .dedup_by(|c1, c2| (&c1.name, c1.module_name) == (&c2.name, c2.module_name)); + self.items + .into_iter() + .filter_map(|item| { + // If we would have to actually import something, don't suggest the qualification + // (we could, maybe we should, but for now, we don't) + if item.import.is_some() { + return None; + } + + Some(ImportEdit { + label: format!("qualify {}", item.insert.as_ref()?), + edit: Edit::replacement(item.insert?.into_string(), range.start(), range.end()), + }) + }) + .collect() + } + /// Attempts to adds the given completion to this collection. /// /// When added, `true` is returned. @@ -555,15 +578,19 @@ pub(crate) struct ImportEdit { pub edit: Edit, } -pub(crate) fn missing_imports( +/// Get fixes that would resolve an unresolved reference +pub(crate) fn unresolved_fixes( db: &dyn Db, file: File, parsed: &ParsedModuleRef, symbol: &str, node: AnyNodeRef, ) -> Vec { - let mut completions = Completions::exactly(db, symbol); + let mut results = Vec::new(); let scoped = ScopedTarget { node }; + + // Request imports we could add to put the symbol in scope + let mut completions = Completions::exactly(db, symbol); add_unimported_completions( db, file, @@ -574,8 +601,23 @@ pub(crate) fn missing_imports( }, &mut completions, ); + results.extend(completions.into_imports()); - completions.into_imports() + // Request qualifications we could apply to the symbol to make it resolve + let mut completions = Completions::exactly(db, symbol); + add_unimported_completions( + db, + file, + parsed, + scoped, + |module_name: &ModuleName, symbol: &str| { + ImportRequest::import(module_name.as_str(), symbol).force() + }, + &mut completions, + ); + results.extend(completions.into_qualifications(node.range())); + + results } /// Adds completions derived from keywords. diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__code_actions__code_action_existing_import_undefined_decorator.snap b/crates/ty_server/tests/e2e/snapshots/e2e__code_actions__code_action_existing_import_undefined_decorator.snap index f5e5a4f565..ceaf6ad94a 100644 --- a/crates/ty_server/tests/e2e/snapshots/e2e__code_actions__code_action_existing_import_undefined_decorator.snap +++ b/crates/ty_server/tests/e2e/snapshots/e2e__code_actions__code_action_existing_import_undefined_decorator.snap @@ -48,6 +48,51 @@ expression: code_actions }, "isPreferred": true }, + { + "title": "qualify warnings.deprecated", + "kind": "quickfix", + "diagnostics": [ + { + "range": { + "start": { + "line": 3, + "character": 1 + }, + "end": { + "line": 3, + "character": 11 + } + }, + "severity": 1, + "code": "unresolved-reference", + "codeDescription": { + "href": "https://ty.dev/rules#unresolved-reference" + }, + "source": "ty", + "message": "Name `deprecated` used when not defined" + } + ], + "edit": { + "changes": { + "file:///src/foo.py": [ + { + "range": { + "start": { + "line": 3, + "character": 1 + }, + "end": { + "line": 3, + "character": 11 + } + }, + "newText": "warnings.deprecated" + } + ] + } + }, + "isPreferred": true + }, { "title": "Ignore 'unresolved-reference' for this line", "kind": "quickfix", From cbfecfaf41c62f3f4b88e1e88c0b0f5946814ea3 Mon Sep 17 00:00:00 2001 From: Douglas Creager Date: Mon, 15 Dec 2025 10:25:33 -0500 Subject: [PATCH 8/9] [ty] Avoid stack overflow when calculating inferable typevars (#21971) When we calculate which typevars are inferable in a generic context, the result might include more than the typevars bound by the generic context. The canonical example is a generic method of a generic class: ```py class C[A]: def method[T](self, t: T): ... ``` Here, the inferable typevar set of `method` contains `Self` and `T`, as you'd expect. (Those are the typevars bound by the method.) But it also contains `A@C`, since the implicit `Self` typevar is defined as `Self: C[A]`. That means when we call `method`, we need to mark `A@C` as inferable, so that we can determine the correct mapping for `A@C` at the call site. Fixes https://github.com/astral-sh/ty/issues/1874 --- .../mdtest/generics/pep695/classes.md | 23 +++++++++++++++++++ .../ty_python_semantic/src/types/generics.rs | 21 ++++++++++++++--- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md b/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md index 71e05171e8..06680d2168 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md @@ -800,6 +800,29 @@ def func(x: D): ... func(G()) # error: [invalid-argument-type] ``` +### Self-referential protocol with different specialization + +This is a minimal reproduction for [ty#1874](https://github.com/astral-sh/ty/issues/1874). + +```py +from __future__ import annotations +from typing import Protocol +from ty_extensions import generic_context + +class A[S, R](Protocol): + def get(self, s: S) -> R: ... + def set(self, s: S, r: R) -> S: ... + def merge[R2](self, other: A[S, R2]) -> A[S, tuple[R, R2]]: ... + +class Impl[S, R](A[S, R]): + def foo(self, s: S) -> S: + return self.set(s, self.get(s)) + +reveal_type(generic_context(A.get)) # revealed: ty_extensions.GenericContext[Self@get] +reveal_type(generic_context(A.merge)) # revealed: ty_extensions.GenericContext[Self@merge, R2@merge] +reveal_type(generic_context(Impl.foo)) # revealed: ty_extensions.GenericContext[Self@foo] +``` + ## Tuple as a PEP-695 generic class Our special handling for `tuple` does not break if `tuple` is defined as a PEP-695 generic class in diff --git a/crates/ty_python_semantic/src/types/generics.rs b/crates/ty_python_semantic/src/types/generics.rs index 58ccd28ca0..d4e6c7a446 100644 --- a/crates/ty_python_semantic/src/types/generics.rs +++ b/crates/ty_python_semantic/src/types/generics.rs @@ -23,7 +23,7 @@ use crate::types::{ KnownClass, KnownInstanceType, MaterializationKind, NormalizedVisitor, Signature, Type, TypeContext, TypeMapping, TypeRelation, TypeVarBoundOrConstraints, TypeVarIdentity, TypeVarInstance, TypeVarKind, TypeVarVariance, UnionType, declaration_type, - walk_bound_type_var_type, + walk_type_var_bounds, }; use crate::{Db, FxOrderMap, FxOrderSet}; @@ -290,6 +290,18 @@ impl<'db> GenericContext<'db> { ) } + /// Returns the typevars that are inferable in this generic context. This set might include + /// more typevars than the ones directly bound by the generic context. For instance, consider a + /// method of a generic class: + /// + /// ```py + /// class C[A]: + /// def method[T](self, t: T): + /// ``` + /// + /// In this example, `method`'s generic context binds `Self` and `T`, but its inferable set + /// also includes `A@C`. This is needed because at each call site, we need to infer the + /// specialized class instance type whose method is being invoked. pub(crate) fn inferable_typevars(self, db: &'db dyn Db) -> InferableTypeVars<'db, 'db> { #[derive(Default)] struct CollectTypeVars<'db> { @@ -299,7 +311,7 @@ impl<'db> GenericContext<'db> { impl<'db> TypeVisitor<'db> for CollectTypeVars<'db> { fn should_visit_lazy_type_attributes(&self) -> bool { - true + false } fn visit_bound_type_var_type( @@ -310,7 +322,10 @@ impl<'db> GenericContext<'db> { self.typevars .borrow_mut() .insert(bound_typevar.identity(db)); - walk_bound_type_var_type(db, bound_typevar, self); + let typevar = bound_typevar.typevar(db); + if let Some(bound_or_constraints) = typevar.bound_or_constraints(db) { + walk_type_var_bounds(db, bound_or_constraints, self); + } } fn visit_type(&self, db: &'db dyn Db, ty: Type<'db>) { From 4e1cf5747ae597459e97b3b620ce8af39aa4545d Mon Sep 17 00:00:00 2001 From: Dylan Date: Mon, 15 Dec 2025 09:29:50 -0600 Subject: [PATCH 9/9] Fluent formatting of method chains (#21369) This PR implements a modification (in preview) to fluent formatting for method chains: We break _at_ the first call instead of _after_. For example, we have the following diff between `main` and this PR (with `line-length=8` so I don't have to stretch out the text): ```diff x = ( - df.merge() + df + .merge() .groupby() .agg() .filter() ) ``` ## Explanation of current implementation Recall that we traverse the AST to apply formatting. A method chain, while read left-to-right, is stored in the AST "in reverse". So if we start with something like ```python a.b.c.d().e.f() ``` then the first syntax node we meet is essentially `.f()`. So we have to peek ahead. And we actually _already_ do this in our current fluent formatting logic: we peek ahead to count how many calls we have in the chain to see whether we should be using fluent formatting or now. In this implementation, we actually _record_ this number inside the enum for `CallChainLayout`. That is, we make the variant `Fluent` hold an `AttributeState`. This state can either be: - The number of call-like attributes preceding the current attribute - The state `FirstCallOrSubscript` which means we are at the first call-like attribute in the chain (reading from left to right) - The state `BeforeFirstCallOrSubscript` which means we are in the "first group" of attributes, preceding that first call. In our example, here's what it looks like at each attribute: ``` a.b.c.d().e.f @ Fluent(CallsOrSubscriptsPreceding(1)) a.b.c.d().e @ Fluent(CallsOrSubscriptsPreceding(1)) a.b.c.d @ Fluent(FirstCallOrSubscript) a.b.c @ Fluent(BeforeFirstCallOrSubscript) a.b @ Fluent(BeforeFirstCallOrSubscript) ``` Now, as we descend down from the parent expression, we pass along this little piece of state and modify it as we go to track where we are. This state doesn't do anything except when we are in `FirstCallOrSubscript`, in which case we add a soft line break. Closes #8598 --------- Co-authored-by: Brent Westbrook <36778786+ntBre@users.noreply.github.com> --- .../test/fixtures/ruff/fluent.options.json | 1 + .../resources/test/fixtures/ruff/fluent.py | 35 ++ .../fixtures/ruff/parentheses/call_chains.py | 66 ++++ .../src/expression/expr_attribute.rs | 48 ++- .../src/expression/expr_call.rs | 12 +- .../src/expression/expr_lambda.rs | 3 +- .../src/expression/expr_subscript.rs | 12 +- .../src/expression/mod.rs | 229 +++++++++++-- crates/ruff_python_formatter/src/preview.rs | 7 + ...ty@cases__preview_long_dict_values.py.snap | 10 +- .../format@expression__await.py.snap | 18 +- .../snapshots/format@expression__call.py.snap | 18 +- .../format@expression__lambda.py.snap | 35 +- ...t@expression__split_empty_brackets.py.snap | 1 - .../tests/snapshots/format@fluent.py.snap | 163 ++++++++++ .../format@parentheses__call_chains.py.snap | 300 +++++++++++++++++- docs/formatter.md | 42 +++ 17 files changed, 939 insertions(+), 61 deletions(-) create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/ruff/fluent.options.json create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/ruff/fluent.py create mode 100644 crates/ruff_python_formatter/tests/snapshots/format@fluent.py.snap diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/fluent.options.json b/crates/ruff_python_formatter/resources/test/fixtures/ruff/fluent.options.json new file mode 100644 index 0000000000..f69dec6066 --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/fluent.options.json @@ -0,0 +1 @@ +[{"line_width":8}] diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/fluent.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/fluent.py new file mode 100644 index 0000000000..0b0b76f1dd --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/fluent.py @@ -0,0 +1,35 @@ +# Fixtures for fluent formatting of call chains +# Note that `fluent.options.json` sets line width to 8 + + +x = a.b() + +x = a.b().c() + +x = a.b().c().d + +x = a.b.c.d().e() + +x = a.b.c().d.e().f.g() + +# Consecutive calls/subscripts are grouped together +# for the purposes of fluent formatting (though, as 2025.12.15, +# there may be a break inside of one of these +# calls/subscripts, but that is unrelated to the fluent format.) + +x = a()[0]().b().c() + +x = a.b()[0].c.d()[1]().e + +# Parentheses affect both where the root of the call +# chain is and how many calls we require before applying +# fluent formatting (just 1, in the presence of a parenthesized +# root, as of 2025.12.15.) + +x = (a).b() + +x = (a()).b() + +x = (a.b()).d.e() + +x = (a.b().d).e() diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/call_chains.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/call_chains.py index 0b49a1dd11..d221f4e1ed 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/call_chains.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/call_chains.py @@ -216,3 +216,69 @@ max_message_id = ( .baz() ) +# Note in preview we split at `pl` which some +# folks may dislike. (Similarly with common +# `np` and `pd` invocations). +# +# This is because we cannot reliably predict, +# just from syntax, whether a short identifier +# is being used as a 'namespace' or as an 'object'. +# +# As of 2025.12.15, we do not indent methods in +# fluent formatting. If we ever decide to do so, +# it may make sense to special case call chain roots +# that are shorter than the indent-width (like Prettier does). +# This would have the benefit of handling these common +# two-letter aliases for libraries. + + +expr = ( + pl.scan_parquet("/data/pypi-parquet/*.parquet") + .filter( + [ + pl.col("path").str.contains( + r"\.(asm|c|cc|cpp|cxx|h|hpp|rs|[Ff][0-9]{0,2}(?:or)?|go)$" + ), + ~pl.col("path").str.contains(r"(^|/)test(|s|ing)"), + ~pl.col("path").str.contains("/site-packages/", literal=True), + ] + ) + .with_columns( + month=pl.col("uploaded_on").dt.truncate("1mo"), + ext=pl.col("path") + .str.extract(pattern=r"\.([a-z0-9]+)$", group_index=1) + .str.replace_all(pattern=r"cxx|cpp|cc|c|hpp|h", value="C/C++") + .str.replace_all(pattern="^f.*$", value="Fortran") + .str.replace("rs", "Rust", literal=True) + .str.replace("go", "Go", literal=True) + .str.replace("asm", "Assembly", literal=True) + .replace({"": None}), + ) + .group_by(["month", "ext"]) + .agg(project_count=pl.col("project_name").n_unique()) + .drop_nulls(["ext"]) + .sort(["month", "project_count"], descending=True) +) + +def indentation_matching_for_loop_in_preview(): + if make_this: + if more_nested_because_line_length: + identical_hidden_layer_sizes = all( + current_hidden_layer_sizes == first_hidden_layer_sizes + for current_hidden_layer_sizes in self.component_config[ + HIDDEN_LAYERS_SIZES + ].values().attr + ) + +def indentation_matching_walrus_in_preview(): + if make_this: + if more_nested_because_line_length: + with self.read_ctx(book_type) as cursor: + if (entry_count := len(names := cursor.execute( + 'SELECT name FROM address_book WHERE address=?', + (address,), + ).fetchall().some_attr)) == 0 or len(set(names)) > 1: + return + +# behavior with parenthesized roots +x = (aaaaaaaaaaaaaaaaaaaaaa).bbbbbbbbbbbbbbbbbbb.cccccccccccccccccccccccc().dddddddddddddddddddddddd().eeeeeeeeeeee diff --git a/crates/ruff_python_formatter/src/expression/expr_attribute.rs b/crates/ruff_python_formatter/src/expression/expr_attribute.rs index 781b8b4601..ca88313b19 100644 --- a/crates/ruff_python_formatter/src/expression/expr_attribute.rs +++ b/crates/ruff_python_formatter/src/expression/expr_attribute.rs @@ -10,6 +10,7 @@ use crate::expression::parentheses::{ NeedsParentheses, OptionalParentheses, Parentheses, is_expression_parenthesized, }; use crate::prelude::*; +use crate::preview::is_fluent_layout_split_first_call_enabled; #[derive(Default)] pub struct FormatExprAttribute { @@ -47,20 +48,26 @@ impl FormatNodeRule for FormatExprAttribute { ) }; - if call_chain_layout == CallChainLayout::Fluent { + if call_chain_layout.is_fluent() { if parenthesize_value { // Don't propagate the call chain layout. value.format().with_options(Parentheses::Always).fmt(f)?; } else { match value.as_ref() { Expr::Attribute(expr) => { - expr.format().with_options(call_chain_layout).fmt(f)?; + expr.format() + .with_options(call_chain_layout.transition_after_attribute()) + .fmt(f)?; } Expr::Call(expr) => { - expr.format().with_options(call_chain_layout).fmt(f)?; + expr.format() + .with_options(call_chain_layout.transition_after_attribute()) + .fmt(f)?; } Expr::Subscript(expr) => { - expr.format().with_options(call_chain_layout).fmt(f)?; + expr.format() + .with_options(call_chain_layout.transition_after_attribute()) + .fmt(f)?; } _ => { value.format().with_options(Parentheses::Never).fmt(f)?; @@ -105,8 +112,30 @@ impl FormatNodeRule for FormatExprAttribute { // Allow the `.` on its own line if this is a fluent call chain // and the value either requires parenthesizing or is a call or subscript expression // (it's a fluent chain but not the first element). - else if call_chain_layout == CallChainLayout::Fluent { - if parenthesize_value || value.is_call_expr() || value.is_subscript_expr() { + // + // In preview we also break _at_ the first call in the chain. + // For example: + // + // ```diff + // # stable formatting vs. preview + // x = ( + // - df.merge() + // + df + // + .merge() + // .groupby() + // .agg() + // .filter() + // ) + // ``` + else if call_chain_layout.is_fluent() { + if parenthesize_value + || value.is_call_expr() + || value.is_subscript_expr() + // Remember to update the doc-comment above when + // stabilizing this behavior. + || (is_fluent_layout_split_first_call_enabled(f.context()) + && call_chain_layout.is_first_call_like()) + { soft_line_break().fmt(f)?; } } @@ -148,8 +177,8 @@ impl FormatNodeRule for FormatExprAttribute { ) }); - let is_call_chain_root = self.call_chain_layout == CallChainLayout::Default - && call_chain_layout == CallChainLayout::Fluent; + let is_call_chain_root = + self.call_chain_layout == CallChainLayout::Default && call_chain_layout.is_fluent(); if is_call_chain_root { write!(f, [group(&format_inner)]) } else { @@ -169,7 +198,8 @@ impl NeedsParentheses for ExprAttribute { self.into(), context.comments().ranges(), context.source(), - ) == CallChainLayout::Fluent + ) + .is_fluent() { OptionalParentheses::Multiline } else if context.comments().has_dangling(self) { diff --git a/crates/ruff_python_formatter/src/expression/expr_call.rs b/crates/ruff_python_formatter/src/expression/expr_call.rs index a5c3227a7d..135ade2266 100644 --- a/crates/ruff_python_formatter/src/expression/expr_call.rs +++ b/crates/ruff_python_formatter/src/expression/expr_call.rs @@ -47,7 +47,10 @@ impl FormatNodeRule for FormatExprCall { func.format().with_options(Parentheses::Always).fmt(f) } else { match func.as_ref() { - Expr::Attribute(expr) => expr.format().with_options(call_chain_layout).fmt(f), + Expr::Attribute(expr) => expr + .format() + .with_options(call_chain_layout.decrement_call_like_count()) + .fmt(f), Expr::Call(expr) => expr.format().with_options(call_chain_layout).fmt(f), Expr::Subscript(expr) => expr.format().with_options(call_chain_layout).fmt(f), _ => func.format().with_options(Parentheses::Never).fmt(f), @@ -67,9 +70,7 @@ impl FormatNodeRule for FormatExprCall { // queryset.distinct().order_by(field.name).values_list(field_name_flat_long_long=True) // ) // ``` - if call_chain_layout == CallChainLayout::Fluent - && self.call_chain_layout == CallChainLayout::Default - { + if call_chain_layout.is_fluent() && self.call_chain_layout == CallChainLayout::Default { group(&fmt_func).fmt(f) } else { fmt_func.fmt(f) @@ -87,7 +88,8 @@ impl NeedsParentheses for ExprCall { self.into(), context.comments().ranges(), context.source(), - ) == CallChainLayout::Fluent + ) + .is_fluent() { OptionalParentheses::Multiline } else if context.comments().has_dangling(self) { diff --git a/crates/ruff_python_formatter/src/expression/expr_lambda.rs b/crates/ruff_python_formatter/src/expression/expr_lambda.rs index faab6ef8c5..63e8aa6d97 100644 --- a/crates/ruff_python_formatter/src/expression/expr_lambda.rs +++ b/crates/ruff_python_formatter/src/expression/expr_lambda.rs @@ -397,7 +397,8 @@ impl Format> for FormatBody<'_> { body.into(), comments.ranges(), f.context().source(), - ) == CallChainLayout::Fluent + ) + .is_fluent() { parenthesize_if_expands(&unparenthesized).fmt(f) } else { diff --git a/crates/ruff_python_formatter/src/expression/expr_subscript.rs b/crates/ruff_python_formatter/src/expression/expr_subscript.rs index ce3aaf1f69..c9595c4a26 100644 --- a/crates/ruff_python_formatter/src/expression/expr_subscript.rs +++ b/crates/ruff_python_formatter/src/expression/expr_subscript.rs @@ -51,7 +51,10 @@ impl FormatNodeRule for FormatExprSubscript { value.format().with_options(Parentheses::Always).fmt(f) } else { match value.as_ref() { - Expr::Attribute(expr) => expr.format().with_options(call_chain_layout).fmt(f), + Expr::Attribute(expr) => expr + .format() + .with_options(call_chain_layout.decrement_call_like_count()) + .fmt(f), Expr::Call(expr) => expr.format().with_options(call_chain_layout).fmt(f), Expr::Subscript(expr) => expr.format().with_options(call_chain_layout).fmt(f), _ => value.format().with_options(Parentheses::Never).fmt(f), @@ -71,8 +74,8 @@ impl FormatNodeRule for FormatExprSubscript { .fmt(f) }); - let is_call_chain_root = self.call_chain_layout == CallChainLayout::Default - && call_chain_layout == CallChainLayout::Fluent; + let is_call_chain_root = + self.call_chain_layout == CallChainLayout::Default && call_chain_layout.is_fluent(); if is_call_chain_root { write!(f, [group(&format_inner)]) } else { @@ -92,7 +95,8 @@ impl NeedsParentheses for ExprSubscript { self.into(), context.comments().ranges(), context.source(), - ) == CallChainLayout::Fluent + ) + .is_fluent() { OptionalParentheses::Multiline } else if is_expression_parenthesized( diff --git a/crates/ruff_python_formatter/src/expression/mod.rs b/crates/ruff_python_formatter/src/expression/mod.rs index a320a1edf5..4b6c159fe2 100644 --- a/crates/ruff_python_formatter/src/expression/mod.rs +++ b/crates/ruff_python_formatter/src/expression/mod.rs @@ -876,6 +876,22 @@ impl<'a> First<'a> { /// ) /// ).all() /// ``` +/// +/// In [`preview`](crate::preview::is_fluent_layout_split_first_call_enabled), we also track the position of the leftmost call or +/// subscript on an attribute in the chain and break just before the dot. +/// +/// So, for example, the right-hand summand in the above expression +/// would get formatted as: +/// ```python +/// Blog.objects +/// .filter( +/// entry__headline__contains="McCartney", +/// ) +/// .limit_results[:10] +/// .filter( +/// entry__pub_date__year=2010, +/// ) +/// ``` #[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] pub enum CallChainLayout { /// The root of a call chain @@ -883,19 +899,149 @@ pub enum CallChainLayout { Default, /// A nested call chain element that uses fluent style. - Fluent, + Fluent(AttributeState), /// A nested call chain element not using fluent style. NonFluent, } +/// Records information about the current position within +/// a call chain. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AttributeState { + /// Stores the number of calls or subscripts + /// to the left of the current position in a chain. + /// + /// Consecutive calls/subscripts on a single + /// object only count once. For example, if we are at + /// `c` in `a.b()[0]()().c()` then this number would be 1. + /// + /// Caveat: If the root of the chain is parenthesized, + /// it contributes +1 to this count, even if it is not + /// a call or subscript. But the name + /// `CallLikeOrParenthesizedRootPreceding` + /// is a tad unwieldy, and this also rarely occurs. + CallLikePreceding(u32), + /// Indicates that we are at the first called or + /// subscripted object in the chain + /// + /// For example, if we are at `b` in `a.b()[0]()().c()` + FirstCallLike, + /// Indicates that we are to the left of the first + /// called or subscripted object in the chain, and therefore + /// need not break. + /// + /// For example, if we are at `a` in `a.b()[0]()().c()` + BeforeFirstCallLike, +} + impl CallChainLayout { + /// Returns new state decreasing count of remaining calls/subscripts + /// to traverse, or the state `FirstCallOrSubscript`, as appropriate. + #[must_use] + pub(crate) fn decrement_call_like_count(self) -> Self { + match self { + Self::Fluent(AttributeState::CallLikePreceding(x)) => { + if x > 1 { + // Recall that we traverse call chains from right to + // left. So after moving from a call/subscript into + // an attribute, we _decrease_ the count of + // _remaining_ calls or subscripts to the left of our + // current position. + Self::Fluent(AttributeState::CallLikePreceding(x - 1)) + } else { + Self::Fluent(AttributeState::FirstCallLike) + } + } + _ => self, + } + } + + /// Returns with state change + /// `FirstCallOrSubscript` -> `BeforeFirstCallOrSubscript` + /// and otherwise returns unchanged. + #[must_use] + pub(crate) fn transition_after_attribute(self) -> Self { + match self { + Self::Fluent(AttributeState::FirstCallLike) => { + Self::Fluent(AttributeState::BeforeFirstCallLike) + } + _ => self, + } + } + + pub(crate) fn is_first_call_like(self) -> bool { + matches!(self, Self::Fluent(AttributeState::FirstCallLike)) + } + + /// Returns either `Fluent` or `NonFluent` depending on a + /// heuristic computed for the whole chain. + /// + /// Explicitly, the criterion to return `Fluent` is + /// as follows: + /// + /// 1. Beginning from the right (i.e. the `expr` itself), + /// traverse inwards past calls, subscripts, and attribute + /// expressions until we meet the first expression that is + /// either none of these or else is parenthesized. This will + /// be the _root_ of the call chain. + /// 2. Count the number of _attribute values_ that are _called + /// or subscripted_ in the chain (note that this includes the + /// root but excludes the rightmost attribute in the chain since + /// it is not the _value_ of some attribute). + /// 3. If the root is parenthesized, add 1 to that value. + /// 4. If the total is at least 2, return `Fluent`. Otherwise + /// return `NonFluent` pub(crate) fn from_expression( mut expr: ExprRef, comment_ranges: &CommentRanges, source: &str, ) -> Self { - let mut attributes_after_parentheses = 0; + // TODO(dylan): Once the fluent layout preview style is + // stabilized, see if it is possible to simplify some of + // the logic around parenthesized roots. (While supporting + // both styles it is more difficult to do this.) + + // Count of attribute _values_ which are called or + // subscripted, after the leftmost parenthesized + // value. + // + // Examples: + // ``` + // # Count of 3 - notice that .d() + // # does not contribute + // a().b().c[0]()().d() + // # Count of 2 - notice that a() + // # does not contribute + // (a()).b().c[0].d + // ``` + let mut computed_attribute_values_after_parentheses = 0; + + // Similar to the above, but instead looks at all calls + // and subscripts rather than looking only at those on + // _attribute values_. So this count can differ from the + // above. + // + // Examples of `computed_attribute_values_after_parentheses` vs + // `call_like_count`: + // + // a().b ---> 1 vs 1 + // a.b().c --> 1 vs 1 + // a.b() ---> 0 vs 1 + let mut call_like_count = 0; + + // Going from right to left, we traverse calls, subscripts, + // and attributes until we get to an expression of a different + // kind _or_ to a parenthesized expression. This records + // the case where we end the traversal at a parenthesized expression. + // + // In these cases, the inferred semantics of the chain are different. + // We interpret this as the user indicating: + // "this parenthesized value is the object of interest and we are + // doing transformations on it". This increases our confidence that + // this should be fluently formatted, and also means we should make + // our first break after this value. + let mut root_value_parenthesized = false; loop { match expr { ExprRef::Attribute(ast::ExprAttribute { value, .. }) => { @@ -907,10 +1053,10 @@ impl CallChainLayout { // ``` if is_expression_parenthesized(value.into(), comment_ranges, source) { // `(a).b`. We preserve these parentheses so don't recurse - attributes_after_parentheses += 1; + root_value_parenthesized = true; break; } else if matches!(value.as_ref(), Expr::Call(_) | Expr::Subscript(_)) { - attributes_after_parentheses += 1; + computed_attribute_values_after_parentheses += 1; } expr = ExprRef::from(value.as_ref()); @@ -925,31 +1071,68 @@ impl CallChainLayout { // ``` ExprRef::Call(ast::ExprCall { func: inner, .. }) | ExprRef::Subscript(ast::ExprSubscript { value: inner, .. }) => { + // We preserve these parentheses so don't recurse + // e.g. (a)[0].x().y().z() + // ^stop here + if is_expression_parenthesized(inner.into(), comment_ranges, source) { + break; + } + + // Accumulate the `call_like_count`, but we only + // want to count things like `a()[0]()()` once. + if !inner.is_call_expr() && !inner.is_subscript_expr() { + call_like_count += 1; + } + expr = ExprRef::from(inner.as_ref()); } _ => { - // We to format the following in fluent style: - // ``` - // f2 = (a).w().t(1,) - // ^ expr - // ``` - if is_expression_parenthesized(expr, comment_ranges, source) { - attributes_after_parentheses += 1; - } - break; } } - - // We preserve these parentheses so don't recurse - if is_expression_parenthesized(expr, comment_ranges, source) { - break; - } } - if attributes_after_parentheses < 2 { + + if computed_attribute_values_after_parentheses + u32::from(root_value_parenthesized) < 2 { CallChainLayout::NonFluent } else { - CallChainLayout::Fluent + CallChainLayout::Fluent(AttributeState::CallLikePreceding( + // We count a parenthesized root value as an extra + // call for the purposes of tracking state. + // + // The reason is that, in this case, we want the first + // "special" break to happen right after the root, as + // opposed to right after the first called/subscripted + // attribute. + // + // For example: + // + // ``` + // (object_of_interest) + // .data.filter() + // .agg() + // .etc() + // ``` + // + // instead of (in preview): + // + // ``` + // (object_of_interest) + // .data + // .filter() + // .etc() + // ``` + // + // For comparison, if we didn't have parentheses around + // the root, we want (and get, in preview): + // + // ``` + // object_of_interest.data + // .filter() + // .agg() + // .etc() + // ``` + call_like_count + u32::from(root_value_parenthesized), + )) } } @@ -972,9 +1155,13 @@ impl CallChainLayout { CallChainLayout::NonFluent } } - layout @ (CallChainLayout::Fluent | CallChainLayout::NonFluent) => layout, + layout @ (CallChainLayout::Fluent(_) | CallChainLayout::NonFluent) => layout, } } + + pub(crate) fn is_fluent(self) -> bool { + matches!(self, CallChainLayout::Fluent(_)) + } } #[derive(Debug, Copy, Clone, PartialEq, Eq)] diff --git a/crates/ruff_python_formatter/src/preview.rs b/crates/ruff_python_formatter/src/preview.rs index 62b6b90033..ff94f66081 100644 --- a/crates/ruff_python_formatter/src/preview.rs +++ b/crates/ruff_python_formatter/src/preview.rs @@ -59,3 +59,10 @@ pub(crate) const fn is_avoid_parens_for_long_as_captures_enabled( pub(crate) const fn is_parenthesize_lambda_bodies_enabled(context: &PyFormatContext) -> bool { context.is_preview() } + +/// Returns `true` if the +/// [`fluent_layout_split_first_call`](https://github.com/astral-sh/ruff/pull/21369) preview +/// style is enabled. +pub(crate) const fn is_fluent_layout_split_first_call_enabled(context: &PyFormatContext) -> bool { + context.is_preview() +} diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_dict_values.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_dict_values.py.snap index 93f28b8669..f021bad61c 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_dict_values.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_long_dict_values.py.snap @@ -192,7 +192,7 @@ class Random: } x = { "foobar": (123) + 456, -@@ -97,24 +94,20 @@ +@@ -97,24 +94,21 @@ my_dict = { @@ -221,13 +221,14 @@ class Random: - .second_call() - .third_call(some_args="some value") - ) -+ "a key in my dict": MyClass.some_attribute.first_call() ++ "a key in my dict": MyClass.some_attribute ++ .first_call() + .second_call() + .third_call(some_args="some value") } { -@@ -139,17 +132,17 @@ +@@ -139,17 +133,17 @@ class Random: def func(): @@ -363,7 +364,8 @@ my_dict = { / 100000.0 } my_dict = { - "a key in my dict": MyClass.some_attribute.first_call() + "a key in my dict": MyClass.some_attribute + .first_call() .second_call() .third_call(some_args="some value") } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__await.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__await.py.snap index de396cccfb..63b8890e04 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__await.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__await.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/await.py -snapshot_kind: text --- ## Input ```python @@ -142,3 +141,20 @@ test_data = await ( .to_list() ) ``` + + +## Preview changes +```diff +--- Stable ++++ Preview +@@ -65,7 +65,8 @@ + + # https://github.com/astral-sh/ruff/issues/8644 + test_data = await ( +- Stream.from_async(async_data) ++ Stream ++ .from_async(async_data) + .flat_map_async() + .map() + .filter_async(is_valid_data) +``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap index 6b5d7bfa99..6feb06a6ac 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py -snapshot_kind: text --- ## Input ```python @@ -557,3 +556,20 @@ result = ( result = (object[complicate_caller])("argument").a["b"].test(argument) ``` + + +## Preview changes +```diff +--- Stable ++++ Preview +@@ -57,7 +57,8 @@ + + # Call chains/fluent interface (https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#call-chains) + result = ( +- session.query(models.Customer.id) ++ session ++ .query(models.Customer.id) + .filter( + models.Customer.account_id == 10000, + models.Customer.email == "user@example.org", +``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap index 82ee1639d7..393995f523 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap @@ -2155,7 +2155,7 @@ transform = ( ), param( lambda left, right: ( -@@ -471,9 +463,9 @@ +@@ -471,15 +463,16 @@ ), param(lambda left, right: ibis.timestamp("2017-04-01").cast(dt.date)), param( @@ -2168,7 +2168,15 @@ transform = ( ), # This is too long on one line in the lambda body and gets wrapped # inside the body. -@@ -507,16 +499,18 @@ + param( + lambda left, right: ( +- ibis.timestamp("2017-04-01") ++ ibis ++ .timestamp("2017-04-01") + .cast(dt.date) + .between(left, right) + .between(left, right) +@@ -507,16 +500,18 @@ ] # adds parentheses around the body @@ -2190,7 +2198,7 @@ transform = ( lambda x, y, z: ( x + y + z -@@ -527,7 +521,7 @@ +@@ -527,7 +522,7 @@ x + y + z # trailing eol body ) @@ -2199,7 +2207,7 @@ transform = ( lambda x, y, z: ( # leading body -@@ -539,21 +533,23 @@ +@@ -539,21 +534,23 @@ ) ( @@ -2233,7 +2241,7 @@ transform = ( # dangling header comment source_bucket if name == source_bucket_name -@@ -561,8 +557,7 @@ +@@ -561,8 +558,7 @@ ) ( @@ -2243,7 +2251,7 @@ transform = ( source_bucket if name == source_bucket_name else storage.Bucket(mock_service, destination_bucket_name) -@@ -570,61 +565,70 @@ +@@ -570,61 +566,71 @@ ) ( @@ -2293,7 +2301,8 @@ transform = ( - .cast(dt.date) - .between(left, right) + lambda left, right: ( -+ ibis.timestamp("2017-04-01") # comment ++ ibis ++ .timestamp("2017-04-01") # comment + .cast(dt.date) + .between(left, right) + ) @@ -2346,7 +2355,7 @@ transform = ( ) ( -@@ -637,27 +641,31 @@ +@@ -637,27 +643,31 @@ ( lambda # comment @@ -2386,7 +2395,7 @@ transform = ( ) ( -@@ -672,25 +680,28 @@ +@@ -672,25 +682,28 @@ ) ( @@ -2427,7 +2436,7 @@ transform = ( ) ( -@@ -698,9 +709,9 @@ +@@ -698,9 +711,9 @@ # comment 1 *ergs, # comment 2 @@ -2440,7 +2449,7 @@ transform = ( ) ( -@@ -708,19 +719,20 @@ +@@ -708,19 +721,20 @@ # 2 left, # 3 # 4 @@ -2471,7 +2480,7 @@ transform = ( ) ) ) -@@ -738,48 +750,52 @@ +@@ -738,48 +752,52 @@ foo( lambda from_ts, # but still wrap the body if it gets too long to_ts, @@ -2548,7 +2557,7 @@ transform = ( ) ( -@@ -828,8 +844,7 @@ +@@ -828,8 +846,7 @@ ) ( diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__split_empty_brackets.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__split_empty_brackets.py.snap index 65fa97cca4..ccb7c95aa6 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__split_empty_brackets.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__split_empty_brackets.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/split_empty_brackets.py -snapshot_kind: text --- ## Input ```python diff --git a/crates/ruff_python_formatter/tests/snapshots/format@fluent.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@fluent.py.snap new file mode 100644 index 0000000000..73213398d5 --- /dev/null +++ b/crates/ruff_python_formatter/tests/snapshots/format@fluent.py.snap @@ -0,0 +1,163 @@ +--- +source: crates/ruff_python_formatter/tests/fixtures.rs +input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/fluent.py +--- +## Input +```python +# Fixtures for fluent formatting of call chains +# Note that `fluent.options.json` sets line width to 8 + + +x = a.b() + +x = a.b().c() + +x = a.b().c().d + +x = a.b.c.d().e() + +x = a.b.c().d.e().f.g() + +# Consecutive calls/subscripts are grouped together +# for the purposes of fluent formatting (though, as 2025.12.15, +# there may be a break inside of one of these +# calls/subscripts, but that is unrelated to the fluent format.) + +x = a()[0]().b().c() + +x = a.b()[0].c.d()[1]().e + +# Parentheses affect both where the root of the call +# chain is and how many calls we require before applying +# fluent formatting (just 1, in the presence of a parenthesized +# root, as of 2025.12.15.) + +x = (a).b() + +x = (a()).b() + +x = (a.b()).d.e() + +x = (a.b().d).e() +``` + +## Outputs +### Output 1 +``` +indent-style = space +line-width = 8 +indent-width = 4 +quote-style = Double +line-ending = LineFeed +magic-trailing-comma = Respect +docstring-code = Disabled +docstring-code-line-width = "dynamic" +preview = Disabled +target_version = 3.10 +source_type = Python +``` + +```python +# Fixtures for fluent formatting of call chains +# Note that `fluent.options.json` sets line width to 8 + + +x = a.b() + +x = a.b().c() + +x = ( + a.b() + .c() + .d +) + +x = a.b.c.d().e() + +x = ( + a.b.c() + .d.e() + .f.g() +) + +# Consecutive calls/subscripts are grouped together +# for the purposes of fluent formatting (though, as 2025.12.15, +# there may be a break inside of one of these +# calls/subscripts, but that is unrelated to the fluent format.) + +x = ( + a()[ + 0 + ]() + .b() + .c() +) + +x = ( + a.b()[ + 0 + ] + .c.d()[ + 1 + ]() + .e +) + +# Parentheses affect both where the root of the call +# chain is and how many calls we require before applying +# fluent formatting (just 1, in the presence of a parenthesized +# root, as of 2025.12.15.) + +x = ( + a +).b() + +x = ( + a() +).b() + +x = ( + a.b() +).d.e() + +x = ( + a.b().d +).e() +``` + + +#### Preview changes +```diff +--- Stable ++++ Preview +@@ -7,7 +7,8 @@ + x = a.b().c() + + x = ( +- a.b() ++ a ++ .b() + .c() + .d + ) +@@ -15,7 +16,8 @@ + x = a.b.c.d().e() + + x = ( +- a.b.c() ++ a.b ++ .c() + .d.e() + .f.g() + ) +@@ -34,7 +36,8 @@ + ) + + x = ( +- a.b()[ ++ a ++ .b()[ + 0 + ] + .c.d()[ +``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__call_chains.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__call_chains.py.snap index 4da30d3632..bbbb6de9df 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__call_chains.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__call_chains.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/call_chains.py -snapshot_kind: text --- ## Input ```python @@ -223,6 +222,72 @@ max_message_id = ( .baz() ) +# Note in preview we split at `pl` which some +# folks may dislike. (Similarly with common +# `np` and `pd` invocations). +# +# This is because we cannot reliably predict, +# just from syntax, whether a short identifier +# is being used as a 'namespace' or as an 'object'. +# +# As of 2025.12.15, we do not indent methods in +# fluent formatting. If we ever decide to do so, +# it may make sense to special case call chain roots +# that are shorter than the indent-width (like Prettier does). +# This would have the benefit of handling these common +# two-letter aliases for libraries. + + +expr = ( + pl.scan_parquet("/data/pypi-parquet/*.parquet") + .filter( + [ + pl.col("path").str.contains( + r"\.(asm|c|cc|cpp|cxx|h|hpp|rs|[Ff][0-9]{0,2}(?:or)?|go)$" + ), + ~pl.col("path").str.contains(r"(^|/)test(|s|ing)"), + ~pl.col("path").str.contains("/site-packages/", literal=True), + ] + ) + .with_columns( + month=pl.col("uploaded_on").dt.truncate("1mo"), + ext=pl.col("path") + .str.extract(pattern=r"\.([a-z0-9]+)$", group_index=1) + .str.replace_all(pattern=r"cxx|cpp|cc|c|hpp|h", value="C/C++") + .str.replace_all(pattern="^f.*$", value="Fortran") + .str.replace("rs", "Rust", literal=True) + .str.replace("go", "Go", literal=True) + .str.replace("asm", "Assembly", literal=True) + .replace({"": None}), + ) + .group_by(["month", "ext"]) + .agg(project_count=pl.col("project_name").n_unique()) + .drop_nulls(["ext"]) + .sort(["month", "project_count"], descending=True) +) + +def indentation_matching_for_loop_in_preview(): + if make_this: + if more_nested_because_line_length: + identical_hidden_layer_sizes = all( + current_hidden_layer_sizes == first_hidden_layer_sizes + for current_hidden_layer_sizes in self.component_config[ + HIDDEN_LAYERS_SIZES + ].values().attr + ) + +def indentation_matching_walrus_in_preview(): + if make_this: + if more_nested_because_line_length: + with self.read_ctx(book_type) as cursor: + if (entry_count := len(names := cursor.execute( + 'SELECT name FROM address_book WHERE address=?', + (address,), + ).fetchall().some_attr)) == 0 or len(set(names)) > 1: + return + +# behavior with parenthesized roots +x = (aaaaaaaaaaaaaaaaaaaaaa).bbbbbbbbbbbbbbbbbbb.cccccccccccccccccccccccc().dddddddddddddddddddddddd().eeeeeeeeeeee ``` ## Output @@ -466,4 +531,237 @@ max_message_id = ( .sum() .baz() ) + +# Note in preview we split at `pl` which some +# folks may dislike. (Similarly with common +# `np` and `pd` invocations). +# +# This is because we cannot reliably predict, +# just from syntax, whether a short identifier +# is being used as a 'namespace' or as an 'object'. +# +# As of 2025.12.15, we do not indent methods in +# fluent formatting. If we ever decide to do so, +# it may make sense to special case call chain roots +# that are shorter than the indent-width (like Prettier does). +# This would have the benefit of handling these common +# two-letter aliases for libraries. + + +expr = ( + pl.scan_parquet("/data/pypi-parquet/*.parquet") + .filter( + [ + pl.col("path").str.contains( + r"\.(asm|c|cc|cpp|cxx|h|hpp|rs|[Ff][0-9]{0,2}(?:or)?|go)$" + ), + ~pl.col("path").str.contains(r"(^|/)test(|s|ing)"), + ~pl.col("path").str.contains("/site-packages/", literal=True), + ] + ) + .with_columns( + month=pl.col("uploaded_on").dt.truncate("1mo"), + ext=pl.col("path") + .str.extract(pattern=r"\.([a-z0-9]+)$", group_index=1) + .str.replace_all(pattern=r"cxx|cpp|cc|c|hpp|h", value="C/C++") + .str.replace_all(pattern="^f.*$", value="Fortran") + .str.replace("rs", "Rust", literal=True) + .str.replace("go", "Go", literal=True) + .str.replace("asm", "Assembly", literal=True) + .replace({"": None}), + ) + .group_by(["month", "ext"]) + .agg(project_count=pl.col("project_name").n_unique()) + .drop_nulls(["ext"]) + .sort(["month", "project_count"], descending=True) +) + + +def indentation_matching_for_loop_in_preview(): + if make_this: + if more_nested_because_line_length: + identical_hidden_layer_sizes = all( + current_hidden_layer_sizes == first_hidden_layer_sizes + for current_hidden_layer_sizes in self.component_config[ + HIDDEN_LAYERS_SIZES + ] + .values() + .attr + ) + + +def indentation_matching_walrus_in_preview(): + if make_this: + if more_nested_because_line_length: + with self.read_ctx(book_type) as cursor: + if ( + entry_count := len( + names := cursor.execute( + "SELECT name FROM address_book WHERE address=?", + (address,), + ) + .fetchall() + .some_attr + ) + ) == 0 or len(set(names)) > 1: + return + + +# behavior with parenthesized roots +x = ( + (aaaaaaaaaaaaaaaaaaaaaa) + .bbbbbbbbbbbbbbbbbbb.cccccccccccccccccccccccc() + .dddddddddddddddddddddddd() + .eeeeeeeeeeee +) +``` + + +## Preview changes +```diff +--- Stable ++++ Preview +@@ -21,7 +21,8 @@ + ) + + raise OsError("") from ( +- Blog.objects.filter( ++ Blog.objects ++ .filter( + entry__headline__contains="Lennon", + ) + .filter( +@@ -33,7 +34,8 @@ + ) + + raise OsError("sökdjffffsldkfjlhsakfjhalsökafhsöfdahsödfjösaaksjdllllllllllllll") from ( +- Blog.objects.filter( ++ Blog.objects ++ .filter( + entry__headline__contains="Lennon", + ) + .filter( +@@ -46,7 +48,8 @@ + + # Break only after calls and indexing + b1 = ( +- session.query(models.Customer.id) ++ session ++ .query(models.Customer.id) + .filter( + models.Customer.account_id == account_id, models.Customer.email == email_address + ) +@@ -54,7 +57,8 @@ + ) + + b2 = ( +- Blog.objects.filter( ++ Blog.objects ++ .filter( + entry__headline__contains="Lennon", + ) + .limit_results[:10] +@@ -70,7 +74,8 @@ + ).filter( + entry__pub_date__year=2008, + ) +- + Blog.objects.filter( ++ + Blog.objects ++ .filter( + entry__headline__contains="McCartney", + ) + .limit_results[:10] +@@ -89,7 +94,8 @@ + d11 = x.e().e().e() # + d12 = x.e().e().e() # + d13 = ( +- x.e() # ++ x ++ .e() # + .e() + .e() + ) +@@ -101,7 +107,8 @@ + + # Doesn't fit, fluent style + d3 = ( +- x.e() # ++ x ++ .e() # + .esadjkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk() + .esadjkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk() + ) +@@ -218,7 +225,8 @@ + + ( + ( +- df1_aaaaaaaaaaaa.merge() ++ df1_aaaaaaaaaaaa ++ .merge() + .groupby( + 1, + ) +@@ -228,7 +236,8 @@ + + ( + ( +- df1_aaaaaaaaaaaa.merge() ++ df1_aaaaaaaaaaaa ++ .merge() + .groupby( + 1, + ) +@@ -255,19 +264,19 @@ + + + expr = ( +- pl.scan_parquet("/data/pypi-parquet/*.parquet") +- .filter( +- [ +- pl.col("path").str.contains( +- r"\.(asm|c|cc|cpp|cxx|h|hpp|rs|[Ff][0-9]{0,2}(?:or)?|go)$" +- ), +- ~pl.col("path").str.contains(r"(^|/)test(|s|ing)"), +- ~pl.col("path").str.contains("/site-packages/", literal=True), +- ] +- ) ++ pl ++ .scan_parquet("/data/pypi-parquet/*.parquet") ++ .filter([ ++ pl.col("path").str.contains( ++ r"\.(asm|c|cc|cpp|cxx|h|hpp|rs|[Ff][0-9]{0,2}(?:or)?|go)$" ++ ), ++ ~pl.col("path").str.contains(r"(^|/)test(|s|ing)"), ++ ~pl.col("path").str.contains("/site-packages/", literal=True), ++ ]) + .with_columns( + month=pl.col("uploaded_on").dt.truncate("1mo"), +- ext=pl.col("path") ++ ext=pl ++ .col("path") + .str.extract(pattern=r"\.([a-z0-9]+)$", group_index=1) + .str.replace_all(pattern=r"cxx|cpp|cc|c|hpp|h", value="C/C++") + .str.replace_all(pattern="^f.*$", value="Fortran") +@@ -288,9 +297,8 @@ + if more_nested_because_line_length: + identical_hidden_layer_sizes = all( + current_hidden_layer_sizes == first_hidden_layer_sizes +- for current_hidden_layer_sizes in self.component_config[ +- HIDDEN_LAYERS_SIZES +- ] ++ for current_hidden_layer_sizes in self ++ .component_config[HIDDEN_LAYERS_SIZES] + .values() + .attr + ) +@@ -302,7 +310,8 @@ + with self.read_ctx(book_type) as cursor: + if ( + entry_count := len( +- names := cursor.execute( ++ names := cursor ++ .execute( + "SELECT name FROM address_book WHERE address=?", + (address,), + ) ``` diff --git a/docs/formatter.md b/docs/formatter.md index a80028d09e..f4c39edabb 100644 --- a/docs/formatter.md +++ b/docs/formatter.md @@ -501,6 +501,48 @@ If you want Ruff to split an f-string across multiple lines, ensure there's a li [self-documenting f-string]: https://realpython.com/python-f-strings/#self-documenting-expressions-for-debugging [configured quote style]: settings.md/#format_quote-style +#### Fluent layout for method chains + +At times, when developers write long chains of methods on an object, such as + +```python +x = df.filter(cond).agg(func).merge(other) +``` + +the intent is to perform a sequence of transformations or operations +on a fixed object of interest - in this example, the object `df`. +Assuming the assigned expression exceeds the `line-length`, this preview +style will format the above as: + +```python +x = ( + df + .filter(cond) + .agg(func) + .merge(other) +) +``` + +This deviates from the stable formatting, and also from Black, both +of which would produce: + + +```python +x = ( + df.filter(cond) + .agg(func) + .merge(other) +) +``` + +Both the stable and preview formatting are variants of something +called a **fluent layout**. + +In general, this preview style differs from the stable style +only at the first attribute that precedes +a call or subscript. The preview formatting breaks _before_ this attribute, +while the stable formatting breaks _after_ the call or subscript. + ## Sorting imports Currently, the Ruff formatter does not sort imports. In order to both sort imports and format,