Skip to content

Commit 9ddc579

Browse files
committed
Parenthesize long type annotations in annotated assignment statements
1 parent ef4bd8d commit 9ddc579

File tree

8 files changed

+777
-47
lines changed

8 files changed

+777
-47
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
x1: A[b] | EventHandler | EventSpec | list[EventHandler | EventSpec] | Other | More | AndMore | None = None
2+
3+
x2: "VeryLongClassNameWithAwkwardGenericSubtype[int] |" "VeryLongClassNameWithAwkwardGenericSubtype[str]"
4+
5+
x6: VeryLongClassNameWithAwkwardGenericSubtype[
6+
integeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeer,
7+
VeryLongClassNameWithAwkwardGenericSubtype,
8+
str
9+
] = True
10+
11+
12+
x7: CustomTrainingJob | CustomContainerTrainingJob | CustomPythonPackageTrainingJob
13+
x8: (
14+
None
15+
| datasets.ImageDataset
16+
| datasets.TabularDataset
17+
| datasets.TextDataset
18+
| datasets.VideoDataset
19+
) = None
20+
21+
x9: None | (
22+
datasets.ImageDataset
23+
| datasets.TabularDataset
24+
| datasets.TextDataset
25+
| datasets.VideoDataset
26+
) = None
27+
28+
29+
x10: (
30+
aaaaaaaaaaaaaaaaaaaaaaaa[
31+
bbbbbbbbbbb,
32+
Subscript
33+
| None
34+
| datasets.ImageDataset
35+
| datasets.TabularDataset
36+
| datasets.TextDataset
37+
| datasets.VideoDataset,
38+
],
39+
bbb[other],
40+
) = None
41+
42+
x11: None | [
43+
datasets.ImageDataset,
44+
datasets.TabularDataset,
45+
datasets.TextDataset,
46+
datasets.VideoDataset,
47+
] = None
48+
49+
x12: None | [
50+
datasets.ImageDataset,
51+
datasets.TabularDataset,
52+
datasets.TextDataset,
53+
datasets.VideoDataset,
54+
] | Other = None
55+
56+
57+
x13: [
58+
datasets.ImageDataset,
59+
datasets.TabularDataset,
60+
datasets.TextDataset,
61+
datasets.VideoDataset,
62+
] | Other = None
63+
64+
x14: [
65+
datasets.ImageDataset,
66+
datasets.TabularDataset,
67+
datasets.TextDataset,
68+
datasets.VideoDataset,
69+
] | [
70+
datasets.ImageDataset,
71+
datasets.TabularDataset,
72+
datasets.TextDataset,
73+
datasets.VideoDataset,
74+
] = None
75+
76+
x15: [
77+
datasets.ImageDataset,
78+
datasets.TabularDataset,
79+
datasets.TextDataset,
80+
datasets.VideoDataset,
81+
] | [
82+
datasets.ImageDataset,
83+
datasets.TabularDataset,
84+
datasets.TextDataset,
85+
datasets.VideoDataset,
86+
] | Other = None
87+
88+
x16: None | Literal[
89+
"split",
90+
"a bit longer",
91+
"records",
92+
"index",
93+
"table",
94+
"columns",
95+
"values",
96+
] = None
97+
98+
x17: None | [
99+
datasets.ImageDataset,
100+
datasets.TabularDataset,
101+
datasets.TextDataset,
102+
datasets.VideoDataset,
103+
]
104+
105+
106+
class Test:
107+
safe_age: Decimal # the user's age, used to determine if it's safe for them to use ruff
108+
applied_fixes: int # the number of fixes that this user applied. Used for ranking the users with the most applied fixes.
109+
string_annotation: "Test" # a long comment after a quoted, runtime-only type annotation
110+
111+
112+
##########
113+
# Comments
114+
115+
leading: (
116+
# Leading comment
117+
None | dataset.ImageDataset
118+
)
119+
120+
leading_with_value: (
121+
# Leading comment
122+
None
123+
| dataset.ImageDataset
124+
) = None
125+
126+
leading_open_parentheses: ( # Leading comment
127+
None
128+
| dataset.ImageDataset
129+
)
130+
131+
leading_open_parentheses_with_value: ( # Leading comment
132+
None
133+
| dataset.ImageDataset
134+
) = None
135+
136+
trailing: (
137+
None | dataset.ImageDataset # trailing comment
138+
)
139+
140+
trailing_with_value: (
141+
None | dataset.ImageDataset # trailing comment
142+
) = None
143+
144+
trailing_own_line: (
145+
None | dataset.ImageDataset
146+
# trailing own line
147+
)
148+
149+
trailing_own_line_with_value: (
150+
None | dataset.ImageDataset
151+
# trailing own line
152+
) = None
153+
154+
nested_comment: None | [
155+
# a list of strings
156+
str
157+
] = None

crates/ruff_python_formatter/src/expression/mod.rs

+73-1
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ impl<'ast> IntoFormat<PyFormatContext<'ast>> for Expr {
529529
///
530530
/// This mimics Black's [`_maybe_split_omitting_optional_parens`](https://github.com/psf/black/blob/d1248ca9beaf0ba526d265f4108836d89cf551b7/src/black/linegen.py#L746-L820)
531531
#[allow(clippy::if_same_then_else)]
532-
fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool {
532+
pub(crate) fn can_omit_optional_parentheses(expr: &Expr, context: &PyFormatContext) -> bool {
533533
let mut visitor = CanOmitOptionalParenthesesVisitor::new(context);
534534
visitor.visit_subexpression(expr);
535535

@@ -1195,3 +1195,75 @@ impl From<Operator> for OperatorPrecedence {
11951195
}
11961196
}
11971197
}
1198+
1199+
/// Returns `true` if `expr` is an expression that can be split into multiple lines.
1200+
///
1201+
/// Returns `false` for expressions that are guaranteed to never split.
1202+
pub(crate) fn is_splittable_expression(expr: &Expr, context: &PyFormatContext) -> bool {
1203+
match expr {
1204+
// Single token expressions. They never have any split points.
1205+
Expr::NamedExpr(_)
1206+
| Expr::Name(_)
1207+
| Expr::NumberLiteral(_)
1208+
| Expr::BooleanLiteral(_)
1209+
| Expr::NoneLiteral(_)
1210+
| Expr::EllipsisLiteral(_)
1211+
| Expr::Slice(_)
1212+
| Expr::IpyEscapeCommand(_) => false,
1213+
1214+
// Expressions that insert split points when parenthesized.
1215+
Expr::Compare(_)
1216+
| Expr::BinOp(_)
1217+
| Expr::BoolOp(_)
1218+
| Expr::IfExp(_)
1219+
| Expr::GeneratorExp(_)
1220+
| Expr::Subscript(_)
1221+
| Expr::Await(_)
1222+
| Expr::ListComp(_)
1223+
| Expr::SetComp(_)
1224+
| Expr::DictComp(_)
1225+
| Expr::YieldFrom(_) => true,
1226+
1227+
// Sequence types can split if they contain at least one element.
1228+
Expr::Tuple(tuple) => !tuple.elts.is_empty(),
1229+
Expr::Dict(dict) => !dict.values.is_empty(),
1230+
Expr::Set(set) => !set.elts.is_empty(),
1231+
Expr::List(list) => !list.elts.is_empty(),
1232+
1233+
Expr::UnaryOp(unary) => is_splittable_expression(unary.operand.as_ref(), context),
1234+
Expr::Yield(ast::ExprYield { value, .. }) => value.is_some(),
1235+
1236+
Expr::Call(ast::ExprCall {
1237+
arguments, func, ..
1238+
}) => {
1239+
!arguments.is_empty()
1240+
|| is_expression_parenthesized(
1241+
func.as_ref().into(),
1242+
context.comments().ranges(),
1243+
context.source(),
1244+
)
1245+
}
1246+
1247+
// String like literals can expand if they are implicit concatenated.
1248+
Expr::FString(fstring) => fstring.value.is_implicit_concatenated(),
1249+
Expr::StringLiteral(string) => string.value.is_implicit_concatenated(),
1250+
Expr::BytesLiteral(bytes) => bytes.value.is_implicit_concatenated(),
1251+
1252+
// Expressions that have no split points per se, but they contain nested sub expressions that might expand.
1253+
Expr::Lambda(ast::ExprLambda {
1254+
body: expression, ..
1255+
})
1256+
| Expr::Starred(ast::ExprStarred {
1257+
value: expression, ..
1258+
})
1259+
| Expr::Attribute(ast::ExprAttribute {
1260+
value: expression, ..
1261+
}) => {
1262+
is_expression_parenthesized(
1263+
expression.into(),
1264+
context.comments().ranges(),
1265+
context.source(),
1266+
) || is_splittable_expression(expression.as_ref(), context)
1267+
}
1268+
}
1269+
}

crates/ruff_python_formatter/src/preview.rs

+5
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ pub(crate) const fn is_prefer_splitting_right_hand_side_of_assignments_enabled(
2525
context.is_preview()
2626
}
2727

28+
/// Returns `true` if the [`parenthesize_long_type_hints`](https://github.com/astral-sh/ruff/issues/8894) preview style is enabled.
29+
pub(crate) const fn is_parenthesize_long_type_hints_enabled(context: &PyFormatContext) -> bool {
30+
context.is_preview()
31+
}
32+
2833
/// Returns `true` if the [`no_blank_line_before_class_docstring`] preview style is enabled.
2934
///
3035
/// [`no_blank_line_before_class_docstring`]: https://github.com/astral-sh/ruff/issues/8888

crates/ruff_python_formatter/src/statement/stmt_ann_assign.rs

+43-5
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@ use ruff_formatter::write;
22
use ruff_python_ast::StmtAnnAssign;
33

44
use crate::comments::{SourceComment, SuppressionKind};
5-
use crate::expression::has_parentheses;
5+
use crate::expression::parentheses::Parentheses;
6+
use crate::expression::{has_parentheses, is_splittable_expression};
67
use crate::prelude::*;
7-
use crate::preview::is_prefer_splitting_right_hand_side_of_assignments_enabled;
8+
use crate::preview::{
9+
is_parenthesize_long_type_hints_enabled,
10+
is_prefer_splitting_right_hand_side_of_assignments_enabled,
11+
};
812
use crate::statement::stmt_assign::{
913
AnyAssignmentOperator, AnyBeforeOperator, FormatStatementsLastExpression,
1014
};
@@ -27,7 +31,11 @@ impl FormatNodeRule<StmtAnnAssign> for FormatStmtAnnAssign {
2731

2832
if let Some(value) = value {
2933
if is_prefer_splitting_right_hand_side_of_assignments_enabled(f.context())
30-
&& has_parentheses(annotation, f.context()).is_some()
34+
// The `has_parentheses` check can be removed when stabilizing `is_parenthesize_long_type_hints`.
35+
// because `is_splittable_expression` covers both.
36+
&& (has_parentheses(annotation, f.context()).is_some()
37+
|| (is_parenthesize_long_type_hints_enabled(f.context())
38+
&& is_splittable_expression(annotation, f.context())))
3139
{
3240
FormatStatementsLastExpression::RightToLeft {
3341
before_operator: AnyBeforeOperator::Expression(annotation),
@@ -37,10 +45,28 @@ impl FormatNodeRule<StmtAnnAssign> for FormatStmtAnnAssign {
3745
}
3846
.fmt(f)?;
3947
} else {
48+
// Remove unnecessary parentheses around the annotation if the parenthesize long type hints preview style is enabled.
49+
// Ensure we keep the parentheses if the annotation has any comments.
50+
if is_parenthesize_long_type_hints_enabled(f.context()) {
51+
if f.context().comments().has_leading(annotation.as_ref())
52+
|| f.context().comments().has_trailing(annotation.as_ref())
53+
{
54+
annotation
55+
.format()
56+
.with_options(Parentheses::Always)
57+
.fmt(f)?;
58+
} else {
59+
annotation
60+
.format()
61+
.with_options(Parentheses::Never)
62+
.fmt(f)?;
63+
}
64+
} else {
65+
annotation.format().fmt(f)?;
66+
}
4067
write!(
4168
f,
4269
[
43-
annotation.format(),
4470
space(),
4571
token("="),
4672
space(),
@@ -49,7 +75,19 @@ impl FormatNodeRule<StmtAnnAssign> for FormatStmtAnnAssign {
4975
)?;
5076
}
5177
} else {
52-
annotation.format().fmt(f)?;
78+
// Parenthesize the value and inline the comment if it is a "simple" type annotation, similar
79+
// to what we do with the value.
80+
// ```python
81+
// class Test:
82+
// safe_age: (
83+
// Decimal # the user's age, used to determine if it's safe for them to use ruff
84+
// )
85+
// ```
86+
if is_parenthesize_long_type_hints_enabled(f.context()) {
87+
FormatStatementsLastExpression::left_to_right(annotation, item).fmt(f)?;
88+
} else {
89+
annotation.format().fmt(f)?;
90+
}
5391
}
5492

5593
if f.options().source_type().is_ipynb()

crates/ruff_python_formatter/src/statement/stmt_assign.rs

+20-4
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,18 @@ use crate::comments::{
99
};
1010
use crate::context::{NodeLevel, WithNodeLevel};
1111
use crate::expression::parentheses::{
12-
is_expression_parenthesized, NeedsParentheses, OptionalParentheses, Parentheses, Parenthesize,
12+
is_expression_parenthesized, optional_parentheses, NeedsParentheses, OptionalParentheses,
13+
Parentheses, Parenthesize,
14+
};
15+
use crate::expression::{
16+
can_omit_optional_parentheses, has_own_parentheses, has_parentheses,
17+
maybe_parenthesize_expression,
1318
};
14-
use crate::expression::{has_own_parentheses, has_parentheses, maybe_parenthesize_expression};
1519
use crate::prelude::*;
16-
use crate::preview::is_prefer_splitting_right_hand_side_of_assignments_enabled;
20+
use crate::preview::{
21+
is_parenthesize_long_type_hints_enabled,
22+
is_prefer_splitting_right_hand_side_of_assignments_enabled,
23+
};
1724
use crate::statement::trailing_semicolon;
1825

1926
#[derive(Default)]
@@ -686,8 +693,17 @@ impl Format<PyFormatContext<'_>> for AnyBeforeOperator<'_> {
686693
}
687694
// Never parenthesize targets that come with their own parentheses, e.g. don't parenthesize lists or dictionary literals.
688695
else if should_parenthesize_target(expression, f.context()) {
689-
parenthesize_if_expands(&expression.format().with_options(Parentheses::Never))
696+
if is_parenthesize_long_type_hints_enabled(f.context())
697+
&& can_omit_optional_parentheses(expression, f.context())
698+
{
699+
optional_parentheses(&expression.format().with_options(Parentheses::Never))
700+
.fmt(f)
701+
} else {
702+
parenthesize_if_expands(
703+
&expression.format().with_options(Parentheses::Never),
704+
)
690705
.fmt(f)
706+
}
691707
} else {
692708
expression.format().with_options(Parentheses::Never).fmt(f)
693709
}

0 commit comments

Comments
 (0)