diff --git a/crates/ruff/src/rules/pycodestyle/helpers.rs b/crates/ruff/src/rules/pycodestyle/helpers.rs index ff7c837a22333..46b728b224533 100644 --- a/crates/ruff/src/rules/pycodestyle/helpers.rs +++ b/crates/ruff/src/rules/pycodestyle/helpers.rs @@ -1,7 +1,7 @@ use unicode_width::UnicodeWidthStr; use ruff_python_ast::node::AnyNodeRef; -use ruff_python_ast::parenthesize::ParenthesizedExpression; +use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{CmpOp, Expr, Ranged}; use ruff_source_file::{Line, Locator}; use ruff_text_size::{TextLen, TextRange}; @@ -25,7 +25,7 @@ pub(super) fn generate_comparison( // Add the left side of the comparison. contents.push_str(locator.slice( - ParenthesizedExpression::from_expr(left.into(), parent, locator.contents()).range(), + parenthesized_range(left.into(), parent, locator.contents()).unwrap_or(left.range()), )); for (op, comparator) in ops.iter().zip(comparators) { @@ -46,8 +46,8 @@ pub(super) fn generate_comparison( // Add the right side of the comparison. contents.push_str( locator.slice( - ParenthesizedExpression::from_expr(comparator.into(), parent, locator.contents()) - .range(), + parenthesized_range(comparator.into(), parent, locator.contents()) + .unwrap_or(comparator.range()), ), ); } diff --git a/crates/ruff/src/rules/pycodestyle/rules/not_tests.rs b/crates/ruff/src/rules/pycodestyle/rules/not_tests.rs index d5a0c3d5898e7..7aba04d41aa0e 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/not_tests.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/not_tests.rs @@ -83,7 +83,8 @@ pub(crate) fn not_tests(checker: &mut Checker, unary_op: &ast::ExprUnaryOp) { ops, comparators, range: _, - }) = unary_op.operand.as_ref() else { + }) = unary_op.operand.as_ref() + else { return; }; diff --git a/crates/ruff_python_ast/src/parenthesize.rs b/crates/ruff_python_ast/src/parenthesize.rs index 9949c9c2e9d8a..e7b4866fcbdbf 100644 --- a/crates/ruff_python_ast/src/parenthesize.rs +++ b/crates/ruff_python_ast/src/parenthesize.rs @@ -1,48 +1,29 @@ use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{TextRange, TextSize}; -use std::ops::Sub; use crate::node::AnyNodeRef; -use crate::Ranged; - -/// A wrapper around an expression that may be parenthesized. -#[derive(Debug)] -pub struct ParenthesizedExpression<'a> { - /// The underlying AST node. - expr: AnyNodeRef<'a>, - /// The range of the expression including parentheses, if the expression is parenthesized; - /// or `None`, if the expression is not parenthesized. - range: Option, -} - -impl<'a> ParenthesizedExpression<'a> { - /// Given an expression and its parent, returns a parenthesized expression. - pub fn from_expr(expr: AnyNodeRef<'a>, parent: AnyNodeRef<'a>, contents: &str) -> Self { - Self { - expr, - range: parenthesized_range(expr, parent, contents), - } - } - - /// Returns `true` if the expression is parenthesized. - pub fn is_parenthesized(&self) -> bool { - self.range.is_some() - } -} - -impl Ranged for ParenthesizedExpression<'_> { - fn range(&self) -> TextRange { - self.range.unwrap_or_else(|| self.expr.range()) - } -} +use crate::{ExpressionRef, Ranged}; /// Returns the [`TextRange`] of a given expression including parentheses, if the expression is /// parenthesized; or `None`, if the expression is not parenthesized. -fn parenthesized_range(expr: AnyNodeRef, parent: AnyNodeRef, contents: &str) -> Option { - // If the parent is an `arguments` node, then the range of the expression includes the closing - // parenthesis, so exclude it from our test range. +pub fn parenthesized_range( + expr: ExpressionRef, + parent: AnyNodeRef, + contents: &str, +) -> Option { + // If the parent is a node that brings its own parentheses, exclude the closing parenthesis + // from our search range. Otherwise, we risk matching on calls, like `func(x)`, for which + // the open and close parentheses are part of the `Arguments` node. + // + // There are a few other nodes that may have their own parentheses, but are fine to exclude: + // - `Parameters`: The parameters to a function definition. Any expressions would represent + // default arguments, and so must be preceded by _at least_ the parameter name. As such, + // we won't mistake any parentheses for the opening and closing parentheses on the + // `Parameters` node itself. + // - `Tuple`: The elements of a tuple. The only risk is a single-element tuple (e.g., `(x,)`), + // which must have a trailing comma anyway. let exclusive_parent_end = if parent.is_arguments() { - parent.end().sub(TextSize::new(1)) + parent.end() - TextSize::new(1) } else { parent.end() }; diff --git a/crates/ruff_python_ast/tests/parenthesize.rs b/crates/ruff_python_ast/tests/parenthesize.rs index 82f4489c102dd..eb1ef3850de7a 100644 --- a/crates/ruff_python_ast/tests/parenthesize.rs +++ b/crates/ruff_python_ast/tests/parenthesize.rs @@ -1,4 +1,4 @@ -use ruff_python_ast::parenthesize::ParenthesizedExpression; +use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_parser::parse_expression; #[test] @@ -9,20 +9,20 @@ fn test_parenthesized_name() { let bin_op = expr.as_bin_op_expr().unwrap(); let name = bin_op.left.as_ref(); - let parenthesized = ParenthesizedExpression::from_expr(name.into(), bin_op.into(), source_code); - assert!(parenthesized.is_parenthesized()); + let parenthesized = parenthesized_range(name.into(), bin_op.into(), source_code); + assert!(parenthesized.is_some()); } #[test] -fn test_un_parenthesized_name() { +fn test_non_parenthesized_name() { let source_code = r#"x + 1"#; let expr = parse_expression(source_code, "").unwrap(); let bin_op = expr.as_bin_op_expr().unwrap(); let name = bin_op.left.as_ref(); - let parenthesized = ParenthesizedExpression::from_expr(name.into(), bin_op.into(), source_code); - assert!(!parenthesized.is_parenthesized()); + let parenthesized = parenthesized_range(name.into(), bin_op.into(), source_code); + assert!(parenthesized.is_none()); } #[test] @@ -34,13 +34,12 @@ fn test_parenthesized_argument() { let arguments = &call.arguments; let argument = arguments.args.first().unwrap(); - let parenthesized = - ParenthesizedExpression::from_expr(argument.into(), arguments.into(), source_code); - assert!(parenthesized.is_parenthesized()); + let parenthesized = parenthesized_range(argument.into(), arguments.into(), source_code); + assert!(parenthesized.is_some()); } #[test] -fn test_unparenthesized_argument() { +fn test_non_parenthesized_argument() { let source_code = r#"f(a)"#; let expr = parse_expression(source_code, "").unwrap(); @@ -48,9 +47,8 @@ fn test_unparenthesized_argument() { let arguments = &call.arguments; let argument = arguments.args.first().unwrap(); - let parenthesized = - ParenthesizedExpression::from_expr(argument.into(), arguments.into(), source_code); - assert!(!parenthesized.is_parenthesized()); + let parenthesized = parenthesized_range(argument.into(), arguments.into(), source_code); + assert!(parenthesized.is_none()); } #[test] @@ -61,20 +59,18 @@ fn test_parenthesized_tuple_member() { let tuple = expr.as_tuple_expr().unwrap(); let member = tuple.elts.last().unwrap(); - let parenthesized = - ParenthesizedExpression::from_expr(member.into(), tuple.into(), source_code); - assert!(parenthesized.is_parenthesized()); + let parenthesized = parenthesized_range(member.into(), tuple.into(), source_code); + assert!(parenthesized.is_some()); } #[test] -fn test_unparenthesized_tuple_member() { +fn test_non_parenthesized_tuple_member() { let source_code = r#"(a, b)"#; let expr = parse_expression(source_code, "").unwrap(); let tuple = expr.as_tuple_expr().unwrap(); let member = tuple.elts.last().unwrap(); - let parenthesized = - ParenthesizedExpression::from_expr(member.into(), tuple.into(), source_code); - assert!(!parenthesized.is_parenthesized()); + let parenthesized = parenthesized_range(member.into(), tuple.into(), source_code); + assert!(parenthesized.is_none()); }