mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-26 11:59:10 +00:00
Avoid extra parentheses in yield
expressions (#7444)
## Summary This is equivalent to https://github.com/astral-sh/ruff/pull/7424, but for `yield` and `yield from` expressions. Specifically, we want to avoid adding unnecessary extra parentheses for `yield expr` when `expr` itself does not require parentheses. ## Test Plan `cargo test` No change in any of the similarity metrics. Before: | project | similarity index | total files | changed files | |--------------|------------------:|------------------:|------------------:| | cpython | 0.76083 | 1789 | 1632 | | django | 0.99982 | 2760 | 37 | | transformers | 0.99957 | 2587 | 399 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99983 | 3496 | 18 | | warehouse | 0.99929 | 648 | 16 | | zulip | 0.99962 | 1437 | 22 | After: | project | similarity index | total files | changed files | |--------------|------------------:|------------------:|------------------:| | cpython | 0.76083 | 1789 | 1632 | | django | 0.99982 | 2760 | 37 | | transformers | 0.99957 | 2587 | 399 | | twine | 1.00000 | 33 | 0 | | typeshed | 0.99983 | 3496 | 18 | | warehouse | 0.99929 | 648 | 16 | | zulip | 0.99962 | 1437 | 22 |
This commit is contained in:
parent
8d0a5e01bd
commit
422ff82f4a
3 changed files with 89 additions and 4 deletions
|
@ -104,3 +104,20 @@ yield (
|
||||||
yield "# * Make sure each ForeignKey and OneToOneField has `on_delete` set " "to the desired behavior"
|
yield "# * Make sure each ForeignKey and OneToOneField has `on_delete` set " "to the desired behavior"
|
||||||
yield "# * Remove `managed = False` lines if you wish to allow " "Django to create, modify, and delete the table"
|
yield "# * Remove `managed = False` lines if you wish to allow " "Django to create, modify, and delete the table"
|
||||||
yield "# Feel free to rename the models, but don't rename db_table values or " "field names."
|
yield "# Feel free to rename the models, but don't rename db_table values or " "field names."
|
||||||
|
|
||||||
|
# Regression test for: https://github.com/astral-sh/ruff/issues/7420
|
||||||
|
result = yield self.request(
|
||||||
|
f"/applications/{int(application_id)}/guilds/{int(scope)}/commands/{int(command_id)}/permissions"
|
||||||
|
)
|
||||||
|
|
||||||
|
result = yield (self.request(
|
||||||
|
f"/applications/{int(application_id)}/guilds/{int(scope)}/commands/{int(command_id)}/permissions"
|
||||||
|
))
|
||||||
|
|
||||||
|
result = yield
|
||||||
|
|
||||||
|
result = yield (1 + f(1, 2, 3,))
|
||||||
|
|
||||||
|
result = (yield (1 + f(1, 2, 3,)))
|
||||||
|
|
||||||
|
print((yield x))
|
||||||
|
|
|
@ -4,7 +4,9 @@ use ruff_python_ast::{Expr, ExprYield, ExprYieldFrom};
|
||||||
use ruff_text_size::{Ranged, TextRange};
|
use ruff_text_size::{Ranged, TextRange};
|
||||||
|
|
||||||
use crate::expression::maybe_parenthesize_expression;
|
use crate::expression::maybe_parenthesize_expression;
|
||||||
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses, Parenthesize};
|
use crate::expression::parentheses::{
|
||||||
|
is_expression_parenthesized, NeedsParentheses, OptionalParentheses, Parenthesize,
|
||||||
|
};
|
||||||
use crate::prelude::*;
|
use crate::prelude::*;
|
||||||
|
|
||||||
pub(super) enum AnyExpressionYield<'a> {
|
pub(super) enum AnyExpressionYield<'a> {
|
||||||
|
@ -38,7 +40,7 @@ impl NeedsParentheses for AnyExpressionYield<'_> {
|
||||||
fn needs_parentheses(
|
fn needs_parentheses(
|
||||||
&self,
|
&self,
|
||||||
parent: AnyNodeRef,
|
parent: AnyNodeRef,
|
||||||
_context: &PyFormatContext,
|
context: &PyFormatContext,
|
||||||
) -> OptionalParentheses {
|
) -> OptionalParentheses {
|
||||||
// According to https://docs.python.org/3/reference/grammar.html There are two situations
|
// According to https://docs.python.org/3/reference/grammar.html There are two situations
|
||||||
// where we do not want to always parenthesize a yield expression:
|
// where we do not want to always parenthesize a yield expression:
|
||||||
|
@ -47,8 +49,24 @@ impl NeedsParentheses for AnyExpressionYield<'_> {
|
||||||
// We catch situation 1 below. Situation 2 does not need to be handled here as
|
// We catch situation 1 below. Situation 2 does not need to be handled here as
|
||||||
// FormatStmtExpr, does not add parenthesis
|
// FormatStmtExpr, does not add parenthesis
|
||||||
if parent.is_stmt_assign() || parent.is_stmt_ann_assign() || parent.is_stmt_aug_assign() {
|
if parent.is_stmt_assign() || parent.is_stmt_ann_assign() || parent.is_stmt_aug_assign() {
|
||||||
OptionalParentheses::Multiline
|
if let Some(value) = self.value() {
|
||||||
|
if is_expression_parenthesized(
|
||||||
|
value.into(),
|
||||||
|
context.comments().ranges(),
|
||||||
|
context.source(),
|
||||||
|
) {
|
||||||
|
// Ex) `x = yield (1)`
|
||||||
|
OptionalParentheses::Never
|
||||||
|
} else {
|
||||||
|
// Ex) `x = yield f(1, 2, 3)`
|
||||||
|
value.needs_parentheses(self.into(), context)
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// Ex) `x = yield`
|
||||||
|
OptionalParentheses::Never
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
|
// Ex) `print((yield))`
|
||||||
OptionalParentheses::Always
|
OptionalParentheses::Always
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -94,7 +112,7 @@ impl Format<PyFormatContext<'_>> for AnyExpressionYield<'_> {
|
||||||
)?;
|
)?;
|
||||||
} else {
|
} else {
|
||||||
// ExprYieldFrom always has Some(value) so we should never get a bare `yield from`
|
// ExprYieldFrom always has Some(value) so we should never get a bare `yield from`
|
||||||
write!(f, [&token(keyword)])?;
|
write!(f, [token(keyword)])?;
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
|
@ -110,6 +110,23 @@ yield (
|
||||||
yield "# * Make sure each ForeignKey and OneToOneField has `on_delete` set " "to the desired behavior"
|
yield "# * Make sure each ForeignKey and OneToOneField has `on_delete` set " "to the desired behavior"
|
||||||
yield "# * Remove `managed = False` lines if you wish to allow " "Django to create, modify, and delete the table"
|
yield "# * Remove `managed = False` lines if you wish to allow " "Django to create, modify, and delete the table"
|
||||||
yield "# Feel free to rename the models, but don't rename db_table values or " "field names."
|
yield "# Feel free to rename the models, but don't rename db_table values or " "field names."
|
||||||
|
|
||||||
|
# Regression test for: https://github.com/astral-sh/ruff/issues/7420
|
||||||
|
result = yield self.request(
|
||||||
|
f"/applications/{int(application_id)}/guilds/{int(scope)}/commands/{int(command_id)}/permissions"
|
||||||
|
)
|
||||||
|
|
||||||
|
result = yield (self.request(
|
||||||
|
f"/applications/{int(application_id)}/guilds/{int(scope)}/commands/{int(command_id)}/permissions"
|
||||||
|
))
|
||||||
|
|
||||||
|
result = yield
|
||||||
|
|
||||||
|
result = yield (1 + f(1, 2, 3,))
|
||||||
|
|
||||||
|
result = (yield (1 + f(1, 2, 3,)))
|
||||||
|
|
||||||
|
print((yield x))
|
||||||
```
|
```
|
||||||
|
|
||||||
## Output
|
## Output
|
||||||
|
@ -230,6 +247,39 @@ yield (
|
||||||
"# Feel free to rename the models, but don't rename db_table values or "
|
"# Feel free to rename the models, but don't rename db_table values or "
|
||||||
"field names."
|
"field names."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Regression test for: https://github.com/astral-sh/ruff/issues/7420
|
||||||
|
result = yield self.request(
|
||||||
|
f"/applications/{int(application_id)}/guilds/{int(scope)}/commands/{int(command_id)}/permissions"
|
||||||
|
)
|
||||||
|
|
||||||
|
result = yield (
|
||||||
|
self.request(
|
||||||
|
f"/applications/{int(application_id)}/guilds/{int(scope)}/commands/{int(command_id)}/permissions"
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
result = yield
|
||||||
|
|
||||||
|
result = yield (
|
||||||
|
1
|
||||||
|
+ f(
|
||||||
|
1,
|
||||||
|
2,
|
||||||
|
3,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
result = yield (
|
||||||
|
1
|
||||||
|
+ f(
|
||||||
|
1,
|
||||||
|
2,
|
||||||
|
3,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
print((yield x))
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue