mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-26 11:59:35 +00:00
Avoid parenthesizing multiline strings in binary expressions (#6973)
This commit is contained in:
parent
e2b2b1759f
commit
eb552da8a9
6 changed files with 315 additions and 40 deletions
|
@ -222,3 +222,94 @@ x = (
|
||||||
x = (
|
x = (
|
||||||
() - () #
|
() - () #
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# Avoid unnecessary parentheses around multiline strings.
|
||||||
|
expected_content = """<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
""" % (
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
# Needs parentheses
|
||||||
|
% (
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
%
|
||||||
|
# Needs parentheses
|
||||||
|
(
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
expected_content = """<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
""" + a.call.expression(
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_content = """<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
""" + sssssssssssssssssssssssssssssssssssssssssooooo * looooooooooooooooooooooooooooooongggggggggggg
|
||||||
|
|
||||||
|
call(arg1, arg2, """
|
||||||
|
short
|
||||||
|
""", arg3=True)
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
%
|
||||||
|
(
|
||||||
|
self.base_url
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
%
|
||||||
|
(
|
||||||
|
# Needs parentheses
|
||||||
|
self.base_url
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
|
@ -330,6 +330,17 @@ impl<'a> Comments<'a> {
|
||||||
Self::new(map)
|
Self::new(map)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns `true` if the given `node` has any comments.
|
||||||
|
#[inline]
|
||||||
|
pub(crate) fn has<T>(&self, node: T) -> bool
|
||||||
|
where
|
||||||
|
T: Into<AnyNodeRef<'a>>,
|
||||||
|
{
|
||||||
|
self.data
|
||||||
|
.comments
|
||||||
|
.has(&NodeRefEqualityKey::from_ref(node.into()))
|
||||||
|
}
|
||||||
|
|
||||||
/// Returns `true` if the given `node` has any [leading comments](self#leading-comments).
|
/// Returns `true` if the given `node` has any [leading comments](self#leading-comments).
|
||||||
#[inline]
|
#[inline]
|
||||||
pub(crate) fn has_leading<T>(&self, node: T) -> bool
|
pub(crate) fn has_leading<T>(&self, node: T) -> bool
|
||||||
|
|
|
@ -10,7 +10,8 @@ use ruff_python_ast::{
|
||||||
};
|
};
|
||||||
|
|
||||||
use crate::comments::{trailing_comments, trailing_node_comments, SourceComment};
|
use crate::comments::{trailing_comments, trailing_node_comments, SourceComment};
|
||||||
use crate::expression::expr_constant::ExprConstantLayout;
|
use crate::expression::expr_constant::{is_multiline_string, ExprConstantLayout};
|
||||||
|
use crate::expression::has_parentheses;
|
||||||
use crate::expression::parentheses::{
|
use crate::expression::parentheses::{
|
||||||
in_parentheses_only_group, in_parentheses_only_soft_line_break,
|
in_parentheses_only_group, in_parentheses_only_soft_line_break,
|
||||||
in_parentheses_only_soft_line_break_or_space, is_expression_parenthesized, parenthesized,
|
in_parentheses_only_soft_line_break_or_space, is_expression_parenthesized, parenthesized,
|
||||||
|
@ -263,10 +264,23 @@ impl NeedsParentheses for ExprBinOp {
|
||||||
fn needs_parentheses(
|
fn needs_parentheses(
|
||||||
&self,
|
&self,
|
||||||
parent: AnyNodeRef,
|
parent: AnyNodeRef,
|
||||||
_context: &PyFormatContext,
|
context: &PyFormatContext,
|
||||||
) -> OptionalParentheses {
|
) -> OptionalParentheses {
|
||||||
if parent.is_expr_await() && !self.op.is_pow() {
|
if parent.is_expr_await() && !self.op.is_pow() {
|
||||||
OptionalParentheses::Always
|
OptionalParentheses::Always
|
||||||
|
} else if let Expr::Constant(constant) = self.left.as_ref() {
|
||||||
|
// Multiline strings are guaranteed to never fit, avoid adding unnecessary parentheses
|
||||||
|
if !constant.value.is_implicit_concatenated()
|
||||||
|
&& is_multiline_string(constant, context.source())
|
||||||
|
&& has_parentheses(&self.right, context).is_some()
|
||||||
|
&& !context.comments().has_dangling(self)
|
||||||
|
&& !context.comments().has(self.left.as_ref())
|
||||||
|
&& !context.comments().has(self.right.as_ref())
|
||||||
|
{
|
||||||
|
OptionalParentheses::Never
|
||||||
|
} else {
|
||||||
|
OptionalParentheses::Multiline
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
OptionalParentheses::Multiline
|
OptionalParentheses::Multiline
|
||||||
}
|
}
|
||||||
|
|
|
@ -227,27 +227,17 @@ class C:
|
||||||
|
|
||||||
assert expected(
|
assert expected(
|
||||||
value, is_going_to_be="too long to fit in a single line", srsly=True
|
value, is_going_to_be="too long to fit in a single line", srsly=True
|
||||||
@@ -153,7 +154,8 @@
|
@@ -161,9 +162,7 @@
|
||||||
" because it's too long"
|
|
||||||
)
|
|
||||||
|
|
||||||
- dis_c_instance_method = """\
|
|
||||||
+ dis_c_instance_method = (
|
|
||||||
+ """\
|
|
||||||
%3d 0 LOAD_FAST 1 (x)
|
|
||||||
2 LOAD_CONST 1 (1)
|
|
||||||
4 COMPARE_OP 2 (==)
|
|
||||||
@@ -161,8 +163,8 @@
|
|
||||||
8 STORE_ATTR 0 (x)
|
8 STORE_ATTR 0 (x)
|
||||||
10 LOAD_CONST 0 (None)
|
10 LOAD_CONST 0 (None)
|
||||||
12 RETURN_VALUE
|
12 RETURN_VALUE
|
||||||
- """ % (
|
- """ % (
|
||||||
- _C.__init__.__code__.co_firstlineno + 1,
|
- _C.__init__.__code__.co_firstlineno + 1,
|
||||||
+ """
|
- )
|
||||||
+ % (_C.__init__.__code__.co_firstlineno + 1,)
|
+ """ % (_C.__init__.__code__.co_firstlineno + 1,)
|
||||||
)
|
|
||||||
|
|
||||||
assert (
|
assert (
|
||||||
|
expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect
|
||||||
```
|
```
|
||||||
|
|
||||||
## Ruff Output
|
## Ruff Output
|
||||||
|
@ -409,8 +399,7 @@ class C:
|
||||||
" because it's too long"
|
" because it's too long"
|
||||||
)
|
)
|
||||||
|
|
||||||
dis_c_instance_method = (
|
dis_c_instance_method = """\
|
||||||
"""\
|
|
||||||
%3d 0 LOAD_FAST 1 (x)
|
%3d 0 LOAD_FAST 1 (x)
|
||||||
2 LOAD_CONST 1 (1)
|
2 LOAD_CONST 1 (1)
|
||||||
4 COMPARE_OP 2 (==)
|
4 COMPARE_OP 2 (==)
|
||||||
|
@ -418,9 +407,7 @@ class C:
|
||||||
8 STORE_ATTR 0 (x)
|
8 STORE_ATTR 0 (x)
|
||||||
10 LOAD_CONST 0 (None)
|
10 LOAD_CONST 0 (None)
|
||||||
12 RETURN_VALUE
|
12 RETURN_VALUE
|
||||||
"""
|
""" % (_C.__init__.__code__.co_firstlineno + 1,)
|
||||||
% (_C.__init__.__code__.co_firstlineno + 1,)
|
|
||||||
)
|
|
||||||
|
|
||||||
assert (
|
assert (
|
||||||
expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect
|
expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect
|
||||||
|
|
|
@ -227,27 +227,17 @@ class C:
|
||||||
|
|
||||||
assert expected(
|
assert expected(
|
||||||
value, is_going_to_be="too long to fit in a single line", srsly=True
|
value, is_going_to_be="too long to fit in a single line", srsly=True
|
||||||
@@ -153,7 +154,8 @@
|
@@ -161,9 +162,7 @@
|
||||||
" because it's too long"
|
|
||||||
)
|
|
||||||
|
|
||||||
- dis_c_instance_method = """\
|
|
||||||
+ dis_c_instance_method = (
|
|
||||||
+ """\
|
|
||||||
%3d 0 LOAD_FAST 1 (x)
|
|
||||||
2 LOAD_CONST 1 (1)
|
|
||||||
4 COMPARE_OP 2 (==)
|
|
||||||
@@ -161,8 +163,8 @@
|
|
||||||
8 STORE_ATTR 0 (x)
|
8 STORE_ATTR 0 (x)
|
||||||
10 LOAD_CONST 0 (None)
|
10 LOAD_CONST 0 (None)
|
||||||
12 RETURN_VALUE
|
12 RETURN_VALUE
|
||||||
- """ % (
|
- """ % (
|
||||||
- _C.__init__.__code__.co_firstlineno + 1,
|
- _C.__init__.__code__.co_firstlineno + 1,
|
||||||
+ """
|
- )
|
||||||
+ % (_C.__init__.__code__.co_firstlineno + 1,)
|
+ """ % (_C.__init__.__code__.co_firstlineno + 1,)
|
||||||
)
|
|
||||||
|
|
||||||
assert (
|
assert (
|
||||||
|
expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect
|
||||||
```
|
```
|
||||||
|
|
||||||
## Ruff Output
|
## Ruff Output
|
||||||
|
@ -409,8 +399,7 @@ class C:
|
||||||
" because it's too long"
|
" because it's too long"
|
||||||
)
|
)
|
||||||
|
|
||||||
dis_c_instance_method = (
|
dis_c_instance_method = """\
|
||||||
"""\
|
|
||||||
%3d 0 LOAD_FAST 1 (x)
|
%3d 0 LOAD_FAST 1 (x)
|
||||||
2 LOAD_CONST 1 (1)
|
2 LOAD_CONST 1 (1)
|
||||||
4 COMPARE_OP 2 (==)
|
4 COMPARE_OP 2 (==)
|
||||||
|
@ -418,9 +407,7 @@ class C:
|
||||||
8 STORE_ATTR 0 (x)
|
8 STORE_ATTR 0 (x)
|
||||||
10 LOAD_CONST 0 (None)
|
10 LOAD_CONST 0 (None)
|
||||||
12 RETURN_VALUE
|
12 RETURN_VALUE
|
||||||
"""
|
""" % (_C.__init__.__code__.co_firstlineno + 1,)
|
||||||
% (_C.__init__.__code__.co_firstlineno + 1,)
|
|
||||||
)
|
|
||||||
|
|
||||||
assert (
|
assert (
|
||||||
expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect
|
expectedexpectedexpectedexpectedexpectedexpectedexpectedexpectedexpect
|
||||||
|
|
|
@ -228,6 +228,97 @@ x = (
|
||||||
x = (
|
x = (
|
||||||
() - () #
|
() - () #
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# Avoid unnecessary parentheses around multiline strings.
|
||||||
|
expected_content = """<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
""" % (
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
# Needs parentheses
|
||||||
|
% (
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
%
|
||||||
|
# Needs parentheses
|
||||||
|
(
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
expected_content = """<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
""" + a.call.expression(
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_content = """<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
""" + sssssssssssssssssssssssssssssssssssssssssooooo * looooooooooooooooooooooooooooooongggggggggggg
|
||||||
|
|
||||||
|
call(arg1, arg2, """
|
||||||
|
short
|
||||||
|
""", arg3=True)
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
%
|
||||||
|
(
|
||||||
|
self.base_url
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
%
|
||||||
|
(
|
||||||
|
# Needs parentheses
|
||||||
|
self.base_url
|
||||||
|
)
|
||||||
|
)
|
||||||
```
|
```
|
||||||
|
|
||||||
## Output
|
## Output
|
||||||
|
@ -512,6 +603,100 @@ x = (
|
||||||
x = (
|
x = (
|
||||||
() - () #
|
() - () #
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# Avoid unnecessary parentheses around multiline strings.
|
||||||
|
expected_content = """<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
""" % (
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
# Needs parentheses
|
||||||
|
% (
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
%
|
||||||
|
# Needs parentheses
|
||||||
|
(
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
expected_content = """<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
""" + a.call.expression(
|
||||||
|
self.base_url,
|
||||||
|
date.today(),
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
+ sssssssssssssssssssssssssssssssssssssssssooooo
|
||||||
|
* looooooooooooooooooooooooooooooongggggggggggg
|
||||||
|
)
|
||||||
|
|
||||||
|
call(
|
||||||
|
arg1,
|
||||||
|
arg2,
|
||||||
|
"""
|
||||||
|
short
|
||||||
|
""",
|
||||||
|
arg3=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_content = """<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
""" % (self.base_url)
|
||||||
|
|
||||||
|
|
||||||
|
expected_content = (
|
||||||
|
"""<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
|
||||||
|
<sitemap><loc>%s/simple/sitemap-simple.xml</loc><lastmod>%s</lastmod>
|
||||||
|
</sitemap>
|
||||||
|
</sitemapindex>
|
||||||
|
"""
|
||||||
|
%
|
||||||
|
(
|
||||||
|
# Needs parentheses
|
||||||
|
self.base_url
|
||||||
|
)
|
||||||
|
)
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue