WPS237: forbid complex f-string format specifiers (#3608)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: j2cry

CHANGELOG.md

@@ -25,6 +25,7 @@ Semantic versioning in our case means:
 - Allows walrus operator in `WPS332`, #3505
 - Forbids symmetric bitwise operations in `WPS345`, #3593
 - Adds `WPS366`: forbid meaningless boolean operations, #3593
+- Forbids complex f-string format specifiers in `WPS237`, #3491
 
 ### Bugfixes
 

tests/test_visitors/test_ast/test_builtins/test_strings/test_formatted_string.py

@@ -22,6 +22,7 @@
 # Allowed
 f_single_chained_attr = "f'{attr1.attr2}'"
 f_variable_lookup = "f'smth {value}'"
+f_multi_variable_lookup = "f'smth {value1} {value2} {value3}'"
 f_dict_lookup_str_key = 'f\'smth {dict_value["key"]}\''
 f_list_index_lookup = "f'smth {list_value[0]}'"
 f_function_empty_args = "f'smth {user.get_full_name()}'"
@@ -33,6 +34,32 @@
 f_function_with_single_arg = "f'smth {func(arg)}'"
 f_function_with_three_args = "f'{func(arg1, arg2, arg3)}'"
 f_method_with_three_args = "f'{obj.method(arg1, arg2, arg3)}'"
+f_assign = "f'{value=}'"
+f_assign_attr = "f'{value.attr=}'"
+f_assign_call = "f'{value()=}'"
+
+# Allowed format specifiers
+f_format_aligned = "f'{value:<5}'"
+f_format_str = "f'{value!s}'"
+f_format_repr = "f'{value!r}'"
+f_format_code = "f'{value!a}'"
+f_format_hex_lower_short = "f'{value:x}'"
+f_format_hex_upper_short = "f'{value:X}'"
+f_format_hex_lower_long = "f'{value:#x}'"
+f_format_hex_upper_long = "f'{value:#X}'"
+f_format_char = "f'{value:c}'"
+f_format_rounded = "f'{value:.123456f}'"
+f_format_scientific = "f'{value:.456789e}'"
+f_format_var_single = "f'{value:{fmt}}'"
+f_format_var_single2 = "f'{value1:{fmt1}} {value2:{fmt2}}'"
+f_format_var_single_index = "f'{value:{fmt[0]}}'"
+f_format_var_single_call = "f'{value:{fmt()}}'"
+f_format_convertions = "f'{value1!r} {value2!s} {value3!a}'"
+f_format_assign = "f'{value=:<8}'"
+f_format_assign_conversion = "f'{value=!r}'"
+f_format_assign_attr = "f'{value.attr=:.456e}'"
+f_format_assign_var_single = "f'{value=:{fmt}}'"
+f_format_assign_var_single2 = "f'{value1=:{fmt1}} {value2=:{fmt2}}'"
 
 # Disallowed
 f_string = "f'x + y = {2 + 2}'"
@@ -53,6 +80,36 @@
 f_single_chained_functions = "f'{f1().f2()}'"
 f_function_with_four_args = "f'{func(arg1, arg2, arg3, arg4)}'"
 f_method_with_four_args = "f'{obj.meth(arg1, arg2, arg3, arg4)}-post'"
+f_nested_string = 'f\'{f"{value}"}\''
+# Disallowed format specifiers
+f_format_var_multi = "f'pre {value:{fmt1}{fmt2}}'"
+f_format_var_chain = "f'{value:{fmt.attr.attr}}'"
+f_format_var_before1 = "f'{value:{fmt}10}'"
+f_format_var_before2 = "f'{value:{fmt}.4f}'"
+f_format_var_after1 = "f'{value:_{fmt}}'"
+f_format_var_after2 = "f'{value:_^{fmt}}'"
+f_format_var_between1 = "f'{value:_{fmt}10}'"
+f_format_var_between2 = "f'{value:.{precision}f}'"
+f_format_var_around = "f'{value:{fmt1}^{fmt2}}'"
+f_format_str_const = "f'{value!s:10}'"
+f_format_repr_const = "f'{value!r:10}'"
+f_format_code_const = "f'{value!a:10}'"
+f_format_str_var = "f'{value!s:{fmt}}'"
+f_format_repr_var = "f'{value!r:{fmt}}'"
+f_format_code_var = "f'{value!a:{fmt}}'"
+f_format_hex_lower_short_const = "f'{value:10x}'"
+f_format_hex_upper_short_const = "f'{value:10X}'"
+f_format_hex_lower_long_const = "f'{value:#10x}'"
+f_format_hex_upper_long_const = "f'{value:#10X}'"
+f_format_char_const = "f'{value:10c}'"
+f_format_round_const = "f'{value:10.4f}'"
+f_format_scientific_const = "f'{value:10.7e}'"
+f_format_useless1 = "f'{value:_}'"
+f_format_useless2 = "f'{value:_<}'"
+f_format_assign_const = "f'{value=:_^8.2f}'"
+f_format_assign_conversion_const = "f'{value=!r:_>11}'"
+f_format_assign_var_chain = "f'{value=:{fmt.attr.attr}}'"
+f_format_assign_var_multi = "f'{value=:{fmt1}{fmt2}}'"
 
 # regression 1921
 f_string_comma_format = 'f"Count={count:,}"'
@@ -103,6 +160,37 @@ def test_string_normal(
         f_calling_returned_function,
         f_single_chained_functions,
         f_function_with_four_args,
+        f_method_with_four_args,
+        f_nested_string,
+        # format specifiers
+        f_format_var_multi,
+        f_format_var_chain,
+        f_format_var_before1,
+        f_format_var_before2,
+        f_format_var_after1,
+        f_format_var_after2,
+        f_format_var_between1,
+        f_format_var_between2,
+        f_format_var_around,
+        f_format_str_const,
+        f_format_repr_const,
+        f_format_code_const,
+        f_format_str_var,
+        f_format_repr_var,
+        f_format_code_var,
+        f_format_hex_lower_short_const,
+        f_format_hex_upper_short_const,
+        f_format_hex_lower_long_const,
+        f_format_hex_upper_long_const,
+        f_format_char_const,
+        f_format_round_const,
+        f_format_scientific_const,
+        f_format_useless1,
+        f_format_useless2,
+        f_format_assign_const,
+        f_format_assign_conversion_const,
+        f_format_assign_var_chain,
+        f_format_assign_var_multi,
     ],
 )
 def test_complex_f_string(assert_errors, parse_ast_tree, code, default_options):
@@ -125,6 +213,7 @@ def test_complex_f_string(assert_errors, parse_ast_tree, code, default_options):
         f_function_empty_args,
         f_list_index_lookup,
         f_variable_lookup,
+        f_multi_variable_lookup,
         f_single_chained_attr,
         f_attr_on_function,
         f_true_index,
@@ -135,6 +224,31 @@ def test_complex_f_string(assert_errors, parse_ast_tree, code, default_options):
         f_function_with_single_arg,
         f_function_with_three_args,
         f_method_with_three_args,
+        f_assign,
+        f_assign_attr,
+        f_assign_call,
+        # format specifiers
+        f_format_aligned,
+        f_format_str,
+        f_format_repr,
+        f_format_code,
+        f_format_hex_lower_short,
+        f_format_hex_upper_short,
+        f_format_hex_lower_long,
+        f_format_hex_upper_long,
+        f_format_char,
+        f_format_rounded,
+        f_format_scientific,
+        f_format_var_single,
+        f_format_var_single2,
+        f_format_var_single_index,
+        f_format_var_single_call,
+        f_format_convertions,
+        f_format_assign,
+        f_format_assign_conversion,
+        f_format_assign_attr,
+        f_format_assign_var_single,
+        f_format_assign_var_single2,
     ],
 )
 def test_simple_f_string(assert_errors, parse_ast_tree, code, default_options):

wemake_python_styleguide/logic/tree/strings.py

@@ -1,5 +1,7 @@
 import ast
 
+from wemake_python_styleguide.logic.walk import get_closest_parent
+
 
 def is_doc_string(node: ast.AST) -> bool:
     """
@@ -14,3 +16,14 @@ def is_doc_string(node: ast.AST) -> bool:
         node.value.value,
         str,
     )
+
+
+def has_fstring_conversion(component: ast.AST) -> bool:
+    """Checks whether f-string with the component has a conversion specifier."""
+    formatted_component = (
+        get_closest_parent(component, ast.FormattedValue) or component
+    )
+    return (
+        isinstance(formatted_component, ast.FormattedValue)
+        and formatted_component.conversion != -1
+    )

wemake_python_styleguide/violations/complexity.py

@@ -1149,6 +1149,9 @@ class TooComplexFormattedStringViolation(ASTViolation):
       string as the key
     - the return value of a function / method call with 3 arguments maximum
 
+    Using format specifiers also increases complexity, so you should avoid
+    using more than one specifier for a single value.
+
     Reasoning:
         Complex ``f`` strings are often difficult to understand,
         making the code less readable. Generally we don't allow
@@ -1164,12 +1167,17 @@ class TooComplexFormattedStringViolation(ASTViolation):
         # Correct:
         f'smth {user.get_full_name()}'
         f'smth {math_func(1, 2, 3)}'
+        f'smth {value=:.2f}
+        f'smth {value=:{formatspec}}
 
         # Wrong:
         f'{reverse("url-name")}?{"&".join("user=" + uid for uid in user_ids)}'
         f'smth {math_func(1, 2, 3, 4)}'
+        f'smth {value=:{filler}^{padding}.{precision}f}'
 
     .. versionadded:: 0.15.0
+    .. versionchanged:: 1.6.0
+        Complex format specifiers are forbidden.
 
     """
 

wemake_python_styleguide/visitors/ast/builtins.py

@@ -1,4 +1,5 @@
 import ast
+import re
 import string
 from collections.abc import Sequence
 from typing import ClassVar, Final, TypeAlias, final
@@ -8,13 +9,14 @@
     AssignNodesWithWalrus,
     FunctionNodes,
 )
-from wemake_python_styleguide.logic import nodes, source, walk
+from wemake_python_styleguide.logic import source, walk
 from wemake_python_styleguide.logic.complexity.annotations import (
     check_is_node_in_specific_annotation,
 )
 from wemake_python_styleguide.logic.tree import (
     attributes,
     operators,
+    strings,
     variables,
 )
 from wemake_python_styleguide.types import (
@@ -101,24 +103,52 @@ class WrongFormatStringVisitor(base.BaseNodeVisitor):
 
     def visit_JoinedStr(self, node: ast.JoinedStr) -> None:
         """Forbids use of ``f`` strings and too complex ``f`` strings."""
-        if not isinstance(nodes.get_parent(node), ast.FormattedValue):
-            # We need this condition to make sure that this
-            # is not a part of complex string format like `f"Count={count:,}"`:
-            self._check_complex_formatted_string(node)
+        self._check_complex_formatted_string(node)
         self.generic_visit(node)
 
     def _check_complex_formatted_string(self, node: ast.JoinedStr) -> None:
         """Allows all simple uses of `f` strings."""
+        parent = walk.get_closest_parent(node, ast.JoinedStr)
         for string_component in node.values:
+            # check component complexity
+            is_component_complex = False
             if isinstance(string_component, ast.FormattedValue):
-                # Test if possible chaining is invalid
-                if self._is_valid_formatted_value(string_component.value):
-                    continue
-                self.add_violation(  # Everything else is too complex:
-                    complexity.TooComplexFormattedStringViolation(node),
+                is_component_complex = not self._is_valid_formatted_value(
+                    string_component.value
+                )
+            # check format complexity for nested JoinedStr
+            is_format_complex = parent and (
+                strings.has_fstring_conversion(string_component)
+                or len(node.values) > 1
+                or not self._is_const_format_valid(string_component, parent)
+            )
+
+            if is_component_complex or is_format_complex:
+                self.add_violation(
+                    complexity.TooComplexFormattedStringViolation(
+                        parent or node
+                    ),
                 )
                 return
 
+    def _is_const_format_valid(
+        self,
+        string_component: ast.AST,
+        parent: ast.AST | None,
+    ) -> bool:
+        if (
+            parent
+            and isinstance(string_component, ast.Constant)
+            and isinstance(string_component.value, str)
+        ):
+            # check if a string component contains a valid format pattern
+            matched = re.match(
+                r'^(\.\d+(?:f|e)|[<^>]\d+|#?[x|X]|[c,])$',
+                string_component.value,
+            )
+            return bool(matched)
+        return True
+
     def _is_valid_formatted_value(self, format_value: ast.AST) -> bool:
         if isinstance(
             format_value,