Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass parent to NeedsParentheses #5708

Merged
merged 1 commit into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion crates/ruff/src/rules/ruff/rules/invalid_index_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ pub(crate) fn invalid_index_type(checker: &mut Checker, expr: &ExprSubscript) {

// The value types supported by this rule should always be checkable
let Some(value_type) = CheckableExprType::try_from(value) else {
debug_assert!(false, "Index value must be a checkable type to generate a violation message.");
debug_assert!(
false,
"Index value must be a checkable type to generate a violation message."
);
return;
};

Expand Down
59 changes: 21 additions & 38 deletions crates/ruff_python_formatter/src/expression/expr_attribute.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use crate::comments::{leading_comments, trailing_comments, Comments};
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use rustpython_parser::ast::{Constant, Expr, ExprAttribute, ExprConstant};

use ruff_formatter::write;
use ruff_python_ast::node::AnyNodeRef;

use crate::comments::{leading_comments, trailing_comments};
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses, Parentheses};
use crate::prelude::*;
use crate::FormatNodeRule;
use ruff_formatter::write;
use rustpython_parser::ast::{Constant, Expr, ExprAttribute, ExprConstant};

#[derive(Default)]
pub struct FormatExprAttribute;
Expand Down Expand Up @@ -35,7 +36,7 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
dangling_comments.split_at(leading_attribute_comments_start);

if needs_parentheses {
value.format().with_options(Parenthesize::Always).fmt(f)?;
value.format().with_options(Parentheses::Always).fmt(f)?;
} else if let Expr::Attribute(expr_attribute) = value.as_ref() {
// We're in a attribute chain (`a.b.c`). The outermost node adds parentheses if
// required, the inner ones don't need them so we skip the `Expr` formatting that
Expand Down Expand Up @@ -71,41 +72,23 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
}
}

/// Checks if there are any own line comments in an attribute chain (a.b.c). This method is
/// recursive up to the innermost expression that the attribute chain starts behind.
fn has_breaking_comments_attribute_chain(
expr_attribute: &ExprAttribute,
comments: &Comments,
) -> bool {
if comments
.dangling_comments(expr_attribute)
.iter()
.any(|comment| comment.line_position().is_own_line())
|| comments.has_trailing_own_line_comments(expr_attribute)
{
return true;
}

if let Expr::Attribute(inner) = expr_attribute.value.as_ref() {
return has_breaking_comments_attribute_chain(inner, comments);
}

return comments.has_trailing_own_line_comments(expr_attribute.value.as_ref());
}

impl NeedsParentheses for ExprAttribute {
fn needs_parentheses(
&self,
parenthesize: Parenthesize,
parent: AnyNodeRef,
context: &PyFormatContext,
) -> Parentheses {
if has_breaking_comments_attribute_chain(self, context.comments()) {
return Parentheses::Always;
}

match default_expression_needs_parentheses(self.into(), parenthesize, context) {
Parentheses::Optional => Parentheses::Never,
parentheses => parentheses,
) -> OptionalParentheses {
// Checks if there are any own line comments in an attribute chain (a.b.c).
if context
.comments()
.dangling_comments(self)
.iter()
.any(|comment| comment.line_position().is_own_line())
|| context.comments().has_trailing_own_line_comments(self)
{
OptionalParentheses::Always
} else {
self.value.needs_parentheses(parent, context)
}
}
}
13 changes: 6 additions & 7 deletions crates/ruff_python_formatter/src/expression/expr_await.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use crate::context::PyFormatContext;
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::{AsFormat, FormatNodeRule, PyFormatter};
use ruff_formatter::prelude::{space, text};
use ruff_formatter::{write, Buffer, FormatResult};
use ruff_python_ast::node::AnyNodeRef;
use rustpython_parser::ast::ExprAwait;

#[derive(Default)]
Expand All @@ -20,9 +19,9 @@ impl FormatNodeRule<ExprAwait> for FormatExprAwait {
impl NeedsParentheses for ExprAwait {
fn needs_parentheses(
&self,
parenthesize: Parenthesize,
context: &PyFormatContext,
) -> Parentheses {
default_expression_needs_parentheses(self.into(), parenthesize, context)
_parent: AnyNodeRef,
_context: &PyFormatContext,
) -> OptionalParentheses {
OptionalParentheses::Multiline
}
}
13 changes: 6 additions & 7 deletions crates/ruff_python_formatter/src/expression/expr_bin_op.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use crate::comments::{trailing_comments, trailing_node_comments};
use crate::expression::parentheses::{
default_expression_needs_parentheses, in_parentheses_only_group, is_expression_parenthesized,
NeedsParentheses, Parenthesize,
in_parentheses_only_group, is_expression_parenthesized, NeedsParentheses, OptionalParentheses,
};
use crate::expression::Parentheses;
use crate::prelude::*;
use crate::FormatNodeRule;
use ruff_formatter::{write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions};
use ruff_python_ast::node::AstNode;
use ruff_python_ast::node::{AnyNodeRef, AstNode};
use rustpython_parser::ast::{
Constant, Expr, ExprAttribute, ExprBinOp, ExprConstant, ExprUnaryOp, Operator, UnaryOp,
};
Expand Down Expand Up @@ -175,9 +174,9 @@ impl FormatRule<Operator, PyFormatContext<'_>> for FormatOperator {
impl NeedsParentheses for ExprBinOp {
fn needs_parentheses(
&self,
parenthesize: Parenthesize,
context: &PyFormatContext,
) -> Parentheses {
default_expression_needs_parentheses(self.into(), parenthesize, context)
_parent: AnyNodeRef,
_context: &PyFormatContext,
) -> OptionalParentheses {
OptionalParentheses::Multiline
}
}
12 changes: 6 additions & 6 deletions crates/ruff_python_formatter/src/expression/expr_bool_op.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::comments::leading_comments;
use crate::expression::parentheses::{
default_expression_needs_parentheses, in_parentheses_only_group, NeedsParentheses, Parentheses,
Parenthesize,
in_parentheses_only_group, NeedsParentheses, OptionalParentheses, Parentheses,
};
use crate::prelude::*;
use ruff_formatter::{write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions};
use ruff_python_ast::node::AnyNodeRef;
use rustpython_parser::ast::{BoolOp, ExprBoolOp};

#[derive(Default)]
Expand Down Expand Up @@ -70,10 +70,10 @@ impl FormatNodeRule<ExprBoolOp> for FormatExprBoolOp {
impl NeedsParentheses for ExprBoolOp {
fn needs_parentheses(
&self,
parenthesize: Parenthesize,
context: &PyFormatContext,
) -> Parentheses {
default_expression_needs_parentheses(self.into(), parenthesize, context)
_parent: AnyNodeRef,
_context: &PyFormatContext,
) -> OptionalParentheses {
OptionalParentheses::Multiline
}
}

Expand Down
41 changes: 23 additions & 18 deletions crates/ruff_python_formatter/src/expression/expr_call.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
use crate::builders::PyFormatterExtensions;
use ruff_text_size::{TextRange, TextSize};
use rustpython_parser::ast::{Expr, ExprCall, Ranged};

use ruff_formatter::write;
use ruff_python_ast::node::AnyNodeRef;

use crate::comments::dangling_comments;
use crate::context::PyFormatContext;

use crate::expression::parentheses::{
default_expression_needs_parentheses, parenthesized, NeedsParentheses, Parentheses,
Parenthesize,
parenthesized, NeedsParentheses, OptionalParentheses, Parentheses,
};
use crate::prelude::*;
use crate::trivia::{SimpleTokenizer, TokenKind};
use crate::{AsFormat, FormatNodeRule, PyFormatter};
use ruff_formatter::prelude::{format_with, group, text};
use ruff_formatter::{write, Buffer, FormatResult};
use ruff_text_size::{TextRange, TextSize};
use rustpython_parser::ast::{Expr, ExprCall, Ranged};
use crate::FormatNodeRule;

#[derive(Default)]
pub struct FormatExprCall;
Expand Down Expand Up @@ -52,14 +53,21 @@ impl FormatNodeRule<ExprCall> for FormatExprCall {
[argument] if keywords.is_empty() => {
let parentheses =
if is_single_argument_parenthesized(argument, item.end(), source) {
Parenthesize::Always
Parentheses::Always
} else {
Parenthesize::Never
Parentheses::Never
};
joiner.entry(argument, &argument.format().with_options(parentheses));
}
arguments => {
joiner.nodes(arguments).nodes(keywords.iter());
joiner
.entries(
// We have the parentheses from the call so the arguments never need any
arguments
.iter()
.map(|arg| (arg, arg.format().with_options(Parentheses::Preserve))),
)
.nodes(keywords.iter());
}
}

Expand Down Expand Up @@ -100,13 +108,10 @@ impl FormatNodeRule<ExprCall> for FormatExprCall {
impl NeedsParentheses for ExprCall {
fn needs_parentheses(
&self,
parenthesize: Parenthesize,
parent: AnyNodeRef,
context: &PyFormatContext,
) -> Parentheses {
match default_expression_needs_parentheses(self.into(), parenthesize, context) {
Parentheses::Optional => Parentheses::Never,
parentheses => parentheses,
}
) -> OptionalParentheses {
self.func.needs_parentheses(parent, context)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never is incorrect. It requires parentheses if the func node requires parentheses (see attribute chain)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep i should have placed a todo comment about missing fluent style support there

}

Expand Down
12 changes: 6 additions & 6 deletions crates/ruff_python_formatter/src/expression/expr_compare.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use crate::comments::leading_comments;
use crate::expression::parentheses::{
default_expression_needs_parentheses, in_parentheses_only_group, NeedsParentheses, Parentheses,
Parenthesize,
in_parentheses_only_group, NeedsParentheses, OptionalParentheses, Parentheses,
};
use crate::prelude::*;
use crate::FormatNodeRule;
use ruff_formatter::{write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions};
use ruff_python_ast::node::AnyNodeRef;
use rustpython_parser::ast::{CmpOp, ExprCompare};

#[derive(Default)]
Expand Down Expand Up @@ -73,10 +73,10 @@ impl FormatNodeRule<ExprCompare> for FormatExprCompare {
impl NeedsParentheses for ExprCompare {
fn needs_parentheses(
&self,
parenthesize: Parenthesize,
context: &PyFormatContext,
) -> Parentheses {
default_expression_needs_parentheses(self.into(), parenthesize, context)
_parent: AnyNodeRef,
_context: &PyFormatContext,
) -> OptionalParentheses {
OptionalParentheses::Multiline
}
}

Expand Down
66 changes: 35 additions & 31 deletions crates/ruff_python_formatter/src/expression/expr_constant.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,17 @@
use crate::expression::parentheses::{
default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
};
use crate::expression::string::{FormatString, StringLayout};
use ruff_text_size::{TextLen, TextRange};
use rustpython_parser::ast::{Constant, ExprConstant, Ranged};

use ruff_formatter::write;
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::str::is_implicit_concatenation;

use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::expression::string::{FormatString, StringPrefix, StringQuotes};
use crate::prelude::*;
use crate::{not_yet_implemented_custom_text, verbatim_text, FormatNodeRule};
use ruff_formatter::{write, FormatRuleWithOptions};
use rustpython_parser::ast::{Constant, ExprConstant};

#[derive(Default)]
pub struct FormatExprConstant {
string_layout: StringLayout,
}

impl FormatRuleWithOptions<ExprConstant, PyFormatContext<'_>> for FormatExprConstant {
type Options = StringLayout;

fn with_options(mut self, options: Self::Options) -> Self {
self.string_layout = options;
self
}
}
pub struct FormatExprConstant;

impl FormatNodeRule<ExprConstant> for FormatExprConstant {
fn fmt_fields(&self, item: &ExprConstant, f: &mut PyFormatter) -> FormatResult<()> {
Expand All @@ -39,7 +31,7 @@ impl FormatNodeRule<ExprConstant> for FormatExprConstant {
Constant::Int(_) | Constant::Float(_) | Constant::Complex { .. } => {
write!(f, [verbatim_text(item)])
}
Constant::Str(_) => FormatString::new(item, self.string_layout).fmt(f),
Constant::Str(_) => FormatString::new(item).fmt(f),
Constant::Bytes(_) => {
not_yet_implemented_custom_text(r#"b"NOT_YET_IMPLEMENTED_BYTE_STRING""#).fmt(f)
}
Expand All @@ -61,20 +53,32 @@ impl FormatNodeRule<ExprConstant> for FormatExprConstant {
impl NeedsParentheses for ExprConstant {
fn needs_parentheses(
&self,
parenthesize: Parenthesize,
_parent: AnyNodeRef,
context: &PyFormatContext,
) -> Parentheses {
match default_expression_needs_parentheses(self.into(), parenthesize, context) {
Parentheses::Optional if self.value.is_str() && parenthesize.is_if_breaks() => {
// Custom handling that only adds parentheses for implicit concatenated strings.
if parenthesize.is_if_breaks() {
Parentheses::Custom
} else {
Parentheses::Optional
}
) -> OptionalParentheses {
if self.value.is_str() {
let contents = context.locator().slice(self.range());
// Don't wrap triple quoted strings
if is_multiline_string(self, context.source()) || !is_implicit_concatenation(contents) {
OptionalParentheses::Never
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is somewhat slow. We should consider extracting the ranges of all multiline strings (similar to Ruff)

} else {
OptionalParentheses::Multiline
}
Parentheses::Optional => Parentheses::Never,
parentheses => parentheses,
} else {
OptionalParentheses::Never
}
}
}

pub(super) fn is_multiline_string(constant: &ExprConstant, source: &str) -> bool {
if constant.value.is_str() {
let contents = &source[constant.range()];
let prefix = StringPrefix::parse(contents);
let quotes =
StringQuotes::parse(&contents[TextRange::new(prefix.text_len(), contents.text_len())]);

quotes.map_or(false, StringQuotes::is_triple) && contents.contains(['\n', '\r'])
} else {
false
}
}
17 changes: 6 additions & 11 deletions crates/ruff_python_formatter/src/expression/expr_dict.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
use crate::comments::{dangling_node_comments, leading_comments};
use crate::expression::parentheses::{
default_expression_needs_parentheses, parenthesized, NeedsParentheses, Parentheses,
Parenthesize,
};
use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses};
use crate::prelude::*;
use crate::FormatNodeRule;
use ruff_formatter::{format_args, write};
use ruff_python_ast::node::AnyNodeRef;
use ruff_text_size::TextRange;
use rustpython_parser::ast::Ranged;
use rustpython_parser::ast::{Expr, ExprDict};
Expand Down Expand Up @@ -99,12 +97,9 @@ impl FormatNodeRule<ExprDict> for FormatExprDict {
impl NeedsParentheses for ExprDict {
fn needs_parentheses(
&self,
parenthesize: Parenthesize,
context: &PyFormatContext,
) -> Parentheses {
match default_expression_needs_parentheses(self.into(), parenthesize, context) {
Parentheses::Optional => Parentheses::Never,
parentheses => parentheses,
}
_parent: AnyNodeRef,
_context: &PyFormatContext,
) -> OptionalParentheses {
OptionalParentheses::Never
}
}
Loading
Loading