wip fixing ruff rules

astral-sh · Aug 10, 2023 · e917a61 · e917a61
1 parent f84a3de
commit e917a61
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 43 deletions.
diff --git a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs b/crates/ruff/src/rules/flake8_comprehensions/fixes.rs
@@ -1003,7 +1003,7 @@ pub(crate) fn fix_unnecessary_map(
         // If the expression is embedded in an f-string, surround it with spaces to avoid
         // syntax errors.
         if matches!(object_type, ObjectType::Set | ObjectType::Dict) {
-            if parent.is_some_and(Expr::is_formatted_value_expr) {
+            if parent.is_some_and(Expr::is_f_string_expr) {
                 content = format!(" {content} ");
             }
         }

diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/helpers.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/helpers.rs
@@ -64,12 +64,21 @@ pub(super) fn is_empty_or_null_string(expr: &Expr) -> bool {
         }) => string.is_empty(),
         Expr::Constant(constant) if constant.value.is_none() => true,
         Expr::FString(ast::ExprFString { values, range: _ }) => {
-            values.iter().all(is_empty_or_null_string)
+            values.iter().all(is_empty_or_null_string_part)
         }
         _ => false,
     }
 }
 
+fn is_empty_or_null_string_part(part: &ast::FStringPart) -> bool {
+    match part {
+        ast::FStringPart::String(ast::StringTodoName { value, .. }) => value.is_empty(),
+        ast::FStringPart::FormattedValue(ast::FormattedValue { expression, .. }) => {
+            is_empty_or_null_string(expression)
+        }
+    }
+}
+
 pub(super) fn split_names(names: &str) -> Vec<&str> {
     // Match the following pytest code:
     //    [x.strip() for x in argnames.split(",") if x.strip()]

diff --git a/crates/ruff/src/rules/flynt/helpers.rs b/crates/ruff/src/rules/flynt/helpers.rs
@@ -2,25 +2,22 @@ use ruff_python_ast::{self as ast, Arguments, Constant, ConversionFlag, Expr};
 use ruff_text_size::TextRange;
 
 /// Wrap an expression in a `FormattedValue` with no special formatting.
-fn to_formatted_value_expr(inner: &Expr) -> Expr {
-    let node = ast::ExprFormattedValue {
-        value: Box::new(inner.clone()),
+fn to_formatted_value_expr(inner: &Expr) -> ast::FStringPart {
+    ast::FStringPart::FormattedValue(ast::FormattedValue {
+        expression: Box::new(inner.clone()),
         debug_text: None,
         conversion: ConversionFlag::None,
-        format_spec: None,
+        format_spec: vec![],
         range: TextRange::default(),
-    };
-    node.into()
+    })
 }
 
 /// Convert a string to a constant string expression.
-pub(super) fn to_constant_string(s: &str) -> Expr {
-    let node = ast::ExprConstant {
-        value: Constant::Str(s.to_owned()),
-        kind: None,
+pub(super) fn to_constant_string(s: &str) -> ast::FStringPart {
+    ast::FStringPart::String(ast::StringTodoName {
+        value: s.to_owned(),
         range: TextRange::default(),
-    };
-    node.into()
+    })
 }
 
 /// Figure out if `expr` represents a "simple" call
@@ -52,15 +49,20 @@ fn is_simple_callee(func: &Expr) -> bool {
 }
 
 /// Convert an expression to a f-string element (if it looks like a good idea).
-pub(super) fn to_f_string_element(expr: &Expr) -> Option<Expr> {
+pub(super) fn to_fstring_element(expr: &Expr) -> Option<ast::FStringPart> {
     match expr {
         // These are directly handled by `unparse_f_string_element`:
         Expr::Constant(ast::ExprConstant {
-            value: Constant::Str(_),
+            value: Constant::Str(value),
+            range,
             ..
-        })
-        | Expr::FString(_)
-        | Expr::FormattedValue(_) => Some(expr.clone()),
+        }) => Some(ast::FStringPart::String(ast::StringTodoName {
+            value: value.to_string(),
+            range: *range,
+        })),
+        // TODO (but "we don't know how to handle these right now" when called so not needed?)
+        // | Expr::FString(_)
+
         // These should be pretty safe to wrap in a formatted value.
         Expr::Constant(ast::ExprConstant {
             value:

diff --git a/crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs b/crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs
@@ -95,7 +95,7 @@ fn build_fstring(joiner: &str, joinees: &[Expr]) -> Option<Expr> {
         if !std::mem::take(&mut first) {
             fstring_elems.push(helpers::to_constant_string(joiner));
         }
-        fstring_elems.push(helpers::to_f_string_element(expr)?);
+        fstring_elems.push(helpers::to_fstring_element(expr)?);
     }
 
     let node = ast::ExprFString {

diff --git a/crates/ruff/src/rules/pyflakes/rules/f_string_missing_placeholders.rs b/crates/ruff/src/rules/pyflakes/rules/f_string_missing_placeholders.rs
@@ -1,4 +1,4 @@
-use ruff_python_ast::{Expr, PySourceType, Ranged};
+use ruff_python_ast::{Expr, FStringPart, PySourceType, Ranged};
 use ruff_python_parser::{lexer, AsMode, StringKind, Tok};
 use ruff_text_size::{TextRange, TextSize};
 
@@ -81,10 +81,14 @@ fn find_useless_f_strings<'a>(
 }
 
 /// F541
-pub(crate) fn f_string_missing_placeholders(expr: &Expr, values: &[Expr], checker: &mut Checker) {
+pub(crate) fn f_string_missing_placeholders(
+    expr: &Expr,
+    values: &[FStringPart],
+    checker: &mut Checker,
+) {
     if !values
         .iter()
-        .any(|value| matches!(value, Expr::FormattedValue(_)))
+        .any(|value| matches!(value, FStringPart::FormattedValue(_)))
     {
         for (prefix_range, tok_range) in
             find_useless_f_strings(expr, checker.locator(), checker.source_type)

diff --git a/crates/ruff/src/rules/pylint/rules/assert_on_string_literal.rs b/crates/ruff/src/rules/pylint/rules/assert_on_string_literal.rs
@@ -75,21 +75,17 @@ pub(crate) fn assert_on_string_literal(checker: &mut Checker, test: &Expr) {
             checker.diagnostics.push(Diagnostic::new(
                 AssertOnStringLiteral {
                     kind: if values.iter().all(|value| match value {
-                        Expr::Constant(ast::ExprConstant { value, .. }) => match value {
-                            Constant::Str(value, ..) => value.is_empty(),
-                            Constant::Bytes(value) => value.is_empty(),
-                            _ => false,
-                        },
-                        _ => false,
+                        ast::FStringPart::String(ast::StringTodoName { value, .. }) => {
+                            value.is_empty()
+                        }
+                        ast::FStringPart::FormattedValue(_) => false,
                     }) {
                         Kind::Empty
                     } else if values.iter().any(|value| match value {
-                        Expr::Constant(ast::ExprConstant { value, .. }) => match value {
-                            Constant::Str(value, ..) => !value.is_empty(),
-                            Constant::Bytes(value) => !value.is_empty(),
-                            _ => false,
-                        },
-                        _ => false,
+                        ast::FStringPart::String(ast::StringTodoName { value, .. }) => {
+                            !value.is_empty()
+                        }
+                        ast::FStringPart::FormattedValue(_) => false,
                     }) {
                         Kind::NonEmpty
                     } else {

diff --git a/crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs b/crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs
@@ -56,21 +56,23 @@ impl AlwaysAutofixableViolation for ExplicitFStringTypeConversion {
 pub(crate) fn explicit_f_string_type_conversion(
     checker: &mut Checker,
     expr: &Expr,
-    values: &[Expr],
+    values: &[ast::FStringPart],
 ) {
     for (index, formatted_value) in values
         .iter()
         .filter_map(|expr| {
-            if let Expr::FormattedValue(expr) = &expr {
+            if let ast::FStringPart::FormattedValue(expr) = &expr {
                 Some(expr)
             } else {
                 None
             }
         })
         .enumerate()
     {
-        let ast::ExprFormattedValue {
-            value, conversion, ..
+        let ast::FormattedValue {
+            expression,
+            conversion,
+            ..
         } = formatted_value;
 
         // Skip if there's already a conversion flag.
@@ -87,7 +89,7 @@ pub(crate) fn explicit_f_string_type_conversion(
                     range: _,
                 },
             ..
-        }) = value.as_ref()
+        }) = expression.as_ref()
         else {
             continue;
         };
@@ -122,7 +124,7 @@ pub(crate) fn explicit_f_string_type_conversion(
             continue;
         }
 
-        let mut diagnostic = Diagnostic::new(ExplicitFStringTypeConversion, value.range());
+        let mut diagnostic = Diagnostic::new(ExplicitFStringTypeConversion, expression.range());
         if checker.patch(diagnostic.kind.rule()) {
             diagnostic.try_set_fix(|| {
                 convert_call_to_conversion_flag(expr, index, checker.locator(), checker.stylist())

diff --git a/crates/ruff/src/rules/tryceratops/rules/raise_vanilla_args.rs b/crates/ruff/src/rules/tryceratops/rules/raise_vanilla_args.rs
@@ -56,8 +56,10 @@ where
     match expr {
         Expr::FString(ast::ExprFString { values, range: _ }) => {
             for value in values {
-                if any_string(value, predicate) {
-                    return true;
+                if let ast::FStringPart::String(ast::StringTodoName { value, .. }) = value {
+                    if predicate(value.as_str()) {
+                        return true;
+                    }
                 }
             }
         }