Explain check_docs_formatted.py error message (#6125)

## Summary

This is an error message only change to lead an implementor of a new
rule that has an unformatted or invalid bad example to the
right code.

## Test Plan

n/a

crates/ruff_cli/tests/integration_test.rs

@@ -21,7 +21,9 @@ use path_absolutize::path_dedot;
 #[cfg(unix)]
 use tempfile::TempDir;
 
+#[cfg(unix)]
 use ruff_cli::args::Args;
+#[cfg(unix)]
 use ruff_cli::run;
 
 const BIN_NAME: &str = "ruff";

crates/ruff_formatter/src/printer/mod.rs

@@ -116,7 +116,6 @@ impl<'a> Printer<'a> {
                         self.print_str("\n");
                     }
 
-                    self.state.pending_space = false;
                     self.state.pending_indent = args.indention();
                 }
             }
@@ -744,7 +743,6 @@ struct PrinterState<'a> {
     source_markers: Vec<SourceMarker>,
     source_position: TextSize,
     pending_indent: Indention,
-    pending_space: bool,
     measured_group_fits: bool,
     generated_line: usize,
     generated_column: usize,
@@ -1009,12 +1007,12 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
         let args = self.stack.top();
 
         match element {
-            FormatElement::Space => return Ok(self.fits_text(" ")),
+            FormatElement::Space => return Ok(self.fits_text(" ", args)),
 
             FormatElement::Line(line_mode) => {
                 match args.mode() {
                     PrintMode::Flat => match line_mode {
-                        LineMode::SoftOrSpace => return Ok(self.fits_text(" ")),
+                        LineMode::SoftOrSpace => return Ok(self.fits_text(" ", args)),
                         LineMode::Soft => {}
                         LineMode::Hard | LineMode::Empty => {
                             return Ok(if self.must_be_flat {
@@ -1036,17 +1034,18 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
                             MeasureMode::AllLines => {
                                 // Continue measuring on the next line
                                 self.state.line_width = 0;
+                                self.state.pending_indent = args.indention();
                             }
                         }
                     }
                 }
             }
 
-            FormatElement::StaticText { text } => return Ok(self.fits_text(text)),
-            FormatElement::DynamicText { text, .. } => return Ok(self.fits_text(text)),
+            FormatElement::StaticText { text } => return Ok(self.fits_text(text, args)),
+            FormatElement::DynamicText { text, .. } => return Ok(self.fits_text(text, args)),
             FormatElement::SourceCodeSlice { slice, .. } => {
                 let text = slice.text(self.printer.source_code);
-                return Ok(self.fits_text(text));
+                return Ok(self.fits_text(text, args));
             }
             FormatElement::LineSuffixBoundary => {
                 if self.state.has_line_suffix {
@@ -1255,7 +1254,7 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
         Ok(Fits::Maybe)
     }
 
-    fn fits_text(&mut self, text: &str) -> Fits {
+    fn fits_text(&mut self, text: &str, args: PrintElementArgs) -> Fits {
         let indent = std::mem::take(&mut self.state.pending_indent);
         self.state.line_width += indent.level() as usize * self.options().indent_width() as usize
             + indent.align() as usize;
@@ -1264,10 +1263,17 @@ impl<'a, 'print> FitsMeasurer<'a, 'print> {
             let char_width = match c {
                 '\t' => self.options().tab_width as usize,
                 '\n' => {
-                    return if self.must_be_flat {
-                        Fits::No
+                    if self.must_be_flat {
+                        return Fits::No;
                     } else {
-                        Fits::Yes
+                        match args.measure_mode() {
+                            MeasureMode::FirstLine => return Fits::Yes,
+                            MeasureMode::AllLines => {
+                                self.state.line_width = 0;
+                                self.state.pending_indent = args.indention();
+                                continue;
+                            }
+                        }
                     };
                 }
                 c => c.width().unwrap_or(0),

crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/lambda.py

@@ -66,3 +66,12 @@ a = (
 # formatting
 (lambda:(#
 ),)
+
+# lambda arguments don't have parentheses, so we never add a magic trailing comma ...
+def f(
+    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = lambda x: y,
+):
+    pass
+
+# ...but we do preserve a trailing comma after the arguments
+a = lambda b,: 0

crates/ruff_python_formatter/resources/test/fixtures/ruff/skip_magic_trailing_comma.py

@@ -20,3 +20,10 @@
     "seventh entry",
     "eighth entry",
 )
+
+# Regression test: Respect setting in Arguments formatting
+def f(a): pass
+def g(a,): pass
+
+x1 = lambda y: 1
+x2 = lambda y,: 1

crates/ruff_python_formatter/src/expression/expr_attribute.rs

@@ -75,7 +75,7 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
 impl NeedsParentheses for ExprAttribute {
     fn needs_parentheses(
         &self,
-        parent: AnyNodeRef,
+        _parent: AnyNodeRef,
         context: &PyFormatContext,
     ) -> OptionalParentheses {
         // Checks if there are any own line comments in an attribute chain (a.b.c).
@@ -88,7 +88,7 @@ impl NeedsParentheses for ExprAttribute {
         {
             OptionalParentheses::Always
         } else {
-            self.value.needs_parentheses(parent, context)
+            self.value.needs_parentheses(self.into(), context)
         }
     }
 }

crates/ruff_python_formatter/src/expression/expr_call.rs

@@ -119,10 +119,10 @@ impl FormatNodeRule<ExprCall> for FormatExprCall {
 impl NeedsParentheses for ExprCall {
     fn needs_parentheses(
         &self,
-        parent: AnyNodeRef,
+        _parent: AnyNodeRef,
         context: &PyFormatContext,
     ) -> OptionalParentheses {
-        self.func.needs_parentheses(parent, context)
+        self.func.needs_parentheses(self.into(), context)
     }
 }
 

crates/ruff_python_formatter/src/other/arguments.rs

@@ -5,7 +5,7 @@ use ruff_text_size::{TextRange, TextSize};
 
 use ruff_formatter::{format_args, write, FormatRuleWithOptions};
 use ruff_python_ast::node::{AnyNodeRef, AstNode};
-use ruff_python_trivia::{first_non_trivia_token, SimpleToken, SimpleTokenKind, SimpleTokenizer};
+use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer};
 
 use crate::comments::{
     dangling_comments, leading_comments, leading_node_comments, trailing_comments,
@@ -160,34 +160,31 @@ impl FormatNodeRule<Arguments> for FormatArguments {
 
             joiner.finish()?;
 
-            write!(f, [if_group_breaks(&text(","))])?;
+            // Functions use the regular magic trailing comma logic, lambdas may or may not have
+            // a trailing comma but it's just preserved without any magic.
+            // ```python
+            // # Add magic trailing comma if its expands
+            // def f(a): pass
+            // # Expands if magic trailing comma setting is respect, otherwise remove the comma
+            // def g(a,): pass
+            // # Never expands
+            // x1 = lambda y: 1
+            // # Never expands, the comma is always preserved
+            // x2 = lambda y,: 1
+            // ```
+            if self.parentheses == ArgumentsParentheses::Never {
+                // For lambdas (no parentheses), preserve the trailing comma. It doesn't
+                // behave like a magic trailing comma, it's just preserved
+                if has_trailing_comma(item, last_node, f.context().source()) {
+                    write!(f, [text(",")])?;
+                }
+            } else {
+                write!(f, [if_group_breaks(&text(","))])?;
 
-            // Expand the group if the source has a trailing *magic* comma.
-            if let Some(last_node) = last_node {
-                let ends_with_pos_only_argument_separator = !posonlyargs.is_empty()
-                    && args.is_empty()
-                    && vararg.is_none()
-                    && kwonlyargs.is_empty()
-                    && kwarg.is_none();
-
-                let maybe_comma_token = if ends_with_pos_only_argument_separator {
-                    // `def a(b, c, /): ... `
-                    let mut tokens =
-                        SimpleTokenizer::starts_at(last_node.end(), f.context().source())
-                            .skip_trivia();
-
-                    let comma = tokens.next();
-                    assert!(matches!(comma, Some(SimpleToken { kind: SimpleTokenKind::Comma, .. })), "The last positional only argument must be separated by a `,` from the positional only arguments separator `/` but found '{comma:?}'.");
-
-                    let slash = tokens.next();
-                    assert!(matches!(slash, Some(SimpleToken { kind: SimpleTokenKind::Slash, .. })), "The positional argument separator must be present for a function that has positional only arguments but found '{slash:?}'.");
-
-                    tokens.next()
-                } else {
-                    first_non_trivia_token(last_node.end(), f.context().source())
-                };
-
-                if maybe_comma_token.map_or(false, |token| token.kind() == SimpleTokenKind::Comma) {
+                if f.options().magic_trailing_comma().is_respect()
+                    && has_trailing_comma(item, last_node, f.context().source())
+                {
+                    // Make the magic trailing comma expand the group
                     write!(f, [hard_line_break()])?;
                 }
             }
@@ -591,3 +588,33 @@ pub(crate) enum ArgumentSeparatorCommentLocation {
     StarLeading,
     StarTrailing,
 }
+
+fn has_trailing_comma(arguments: &Arguments, last_node: Option<AnyNodeRef>, source: &str) -> bool {
+    // No nodes, no trailing comma
+    let Some(last_node) = last_node else {
+        return false;
+    };
+
+    let ends_with_pos_only_argument_separator = !arguments.posonlyargs.is_empty()
+        && arguments.args.is_empty()
+        && arguments.vararg.is_none()
+        && arguments.kwonlyargs.is_empty()
+        && arguments.kwarg.is_none();
+
+    let mut tokens = SimpleTokenizer::starts_at(last_node.end(), source).skip_trivia();
+    // `def a(b, c, /): ... `
+    // The slash lacks its own node
+    if ends_with_pos_only_argument_separator {
+        let comma = tokens.next();
+        assert!(matches!(comma, Some(SimpleToken { kind: SimpleTokenKind::Comma, .. })), "The last positional only argument must be separated by a `,` from the positional only arguments separator `/` but found '{comma:?}'.");
+
+        let slash = tokens.next();
+        assert!(matches!(slash, Some(SimpleToken { kind: SimpleTokenKind::Slash, .. })), "The positional argument separator must be present for a function that has positional only arguments but found '{slash:?}'.");
+    }
+
+    tokens
+        .next()
+        .expect("There must be a token after the argument list")
+        .kind()
+        == SimpleTokenKind::Comma
+}

crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition.py.snap

@@ -203,18 +203,41 @@ class C:
                          print(i)
          xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy(
              push_manager=context.request.resource_manager,
-@@ -120,9 +120,7 @@
-             key7: value7,
-             key8: value8,
-             key9: value9,
+@@ -110,19 +110,20 @@
+             value, is_going_to_be="too long to fit in a single line", srsly=True
+         ), "Not what we expected"
+ 
+-        assert {
+-            key1: value1,
+-            key2: value2,
+-            key3: value3,
+-            key4: value4,
+-            key5: value5,
+-            key6: value6,
+-            key7: value7,
+-            key8: value8,
+-            key9: value9,
 -        } == expected, (
 -            "Not what we expected and the message is too long to fit in one line"
 -        )
-+        } == expected, "Not what we expected and the message is too long to fit in one line"
++        assert (
++            {
++                key1: value1,
++                key2: value2,
++                key3: value3,
++                key4: value4,
++                key5: value5,
++                key6: value6,
++                key7: value7,
++                key8: value8,
++                key9: value9,
++            }
++            == expected
++        ), "Not what we expected and the message is too long to fit in one line"
  
          assert expected(
              value, is_going_to_be="too long to fit in a single line", srsly=True
-@@ -153,7 +151,8 @@
+@@ -153,7 +154,8 @@
              " because it's too long"
          )
  
@@ -224,7 +247,7 @@ class C:
          %3d           0 LOAD_FAST                1 (x)
                        2 LOAD_CONST               1 (1)
                        4 COMPARE_OP               2 (==)
-@@ -161,8 +160,8 @@
+@@ -161,8 +163,8 @@
                        8 STORE_ATTR               0 (x)
                       10 LOAD_CONST               0 (None)
                       12 RETURN_VALUE
@@ -352,17 +375,20 @@ class C:
             value, is_going_to_be="too long to fit in a single line", srsly=True
         ), "Not what we expected"
 
-        assert {
-            key1: value1,
-            key2: value2,
-            key3: value3,
-            key4: value4,
-            key5: value5,
-            key6: value6,
-            key7: value7,
-            key8: value8,
-            key9: value9,
-        } == expected, "Not what we expected and the message is too long to fit in one line"
+        assert (
+            {
+                key1: value1,
+                key2: value2,
+                key3: value3,
+                key4: value4,
+                key5: value5,
+                key6: value6,
+                key7: value7,
+                key8: value8,
+                key9: value9,
+            }
+            == expected
+        ), "Not what we expected and the message is too long to fit in one line"
 
         assert expected(
             value, is_going_to_be="too long to fit in a single line", srsly=True

crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__composition_no_trailing_comma.py.snap

@@ -203,18 +203,41 @@ class C:
                          print(i)
          xxxxxxxxxxxxxxxx = Yyyy2YyyyyYyyyyy(
              push_manager=context.request.resource_manager,
-@@ -120,9 +120,7 @@
-             key7: value7,
-             key8: value8,
-             key9: value9,
+@@ -110,19 +110,20 @@
+             value, is_going_to_be="too long to fit in a single line", srsly=True
+         ), "Not what we expected"
+ 
+-        assert {
+-            key1: value1,
+-            key2: value2,
+-            key3: value3,
+-            key4: value4,
+-            key5: value5,
+-            key6: value6,
+-            key7: value7,
+-            key8: value8,
+-            key9: value9,
 -        } == expected, (
 -            "Not what we expected and the message is too long to fit in one line"
 -        )
-+        } == expected, "Not what we expected and the message is too long to fit in one line"
++        assert (
++            {
++                key1: value1,
++                key2: value2,
++                key3: value3,
++                key4: value4,
++                key5: value5,
++                key6: value6,
++                key7: value7,
++                key8: value8,
++                key9: value9,
++            }
++            == expected
++        ), "Not what we expected and the message is too long to fit in one line"
  
          assert expected(
              value, is_going_to_be="too long to fit in a single line", srsly=True
-@@ -153,7 +151,8 @@
+@@ -153,7 +154,8 @@
              " because it's too long"
          )
  
@@ -224,7 +247,7 @@ class C:
          %3d           0 LOAD_FAST                1 (x)
                        2 LOAD_CONST               1 (1)
                        4 COMPARE_OP               2 (==)
-@@ -161,8 +160,8 @@
+@@ -161,8 +163,8 @@
                        8 STORE_ATTR               0 (x)
                       10 LOAD_CONST               0 (None)
                       12 RETURN_VALUE
@@ -352,17 +375,20 @@ class C:
             value, is_going_to_be="too long to fit in a single line", srsly=True
         ), "Not what we expected"
 
-        assert {
-            key1: value1,
-            key2: value2,
-            key3: value3,
-            key4: value4,
-            key5: value5,
-            key6: value6,
-            key7: value7,
-            key8: value8,
-            key9: value9,
-        } == expected, "Not what we expected and the message is too long to fit in one line"
+        assert (
+            {
+                key1: value1,
+                key2: value2,
+                key3: value3,
+                key4: value4,
+                key5: value5,
+                key6: value6,
+                key7: value7,
+                key8: value8,
+                key9: value9,
+            }
+            == expected
+        ), "Not what we expected and the message is too long to fit in one line"
 
         assert expected(
             value, is_going_to_be="too long to fit in a single line", srsly=True

crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap

@@ -72,6 +72,15 @@ a = (
 # formatting
 (lambda:(#
 ),)
+
+# lambda arguments don't have parentheses, so we never add a magic trailing comma ...
+def f(
+    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = lambda x: y,
+):
+    pass
+
+# ...but we do preserve a trailing comma after the arguments
+a = lambda b,: 0
 ```
 
 ## Output
@@ -143,6 +152,17 @@ a = (
     lambda: (  #
     ),
 )
+
+
+# lambda arguments don't have parentheses, so we never add a magic trailing comma ...
+def f(
+    aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = lambda x: y,
+):
+    pass
+
+
+# ...but we do preserve a trailing comma after the arguments
+a = lambda b,: 0
 ```
 
 

crates/ruff_python_formatter/tests/snapshots/format@skip_magic_trailing_comma.py.snap

@@ -26,6 +26,13 @@ input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/skip_magic
     "seventh entry",
     "eighth entry",
 )
+
+# Regression test: Respect setting in Arguments formatting
+def f(a): pass
+def g(a,): pass
+
+x1 = lambda y: 1
+x2 = lambda y,: 1
 ```
 
 ## Outputs
@@ -56,6 +63,21 @@ magic-trailing-comma    = Respect
     "seventh entry",
     "eighth entry",
 )
+
+
+# Regression test: Respect setting in Arguments formatting
+def f(a):
+    pass
+
+
+def g(
+    a,
+):
+    pass
+
+
+x1 = lambda y: 1
+x2 = lambda y,: 1
 ```
 
 
@@ -82,6 +104,19 @@ magic-trailing-comma    = Ignore
     "seventh entry",
     "eighth entry",
 )
+
+
+# Regression test: Respect setting in Arguments formatting
+def f(a):
+    pass
+
+
+def g(a):
+    pass
+
+
+x1 = lambda y: 1
+x2 = lambda y,: 1
 ```
 
 

crates/ruff_python_resolver/src/resolver.rs

@@ -715,37 +715,28 @@ pub(crate) fn resolve_import<Host: host::Host>(
     // importing file's directory, then the parent directory, and so on, until the
     // import root is reached.
     let root = execution_environment.root.as_path();
-    if source_file.starts_with(root) {
-        let mut current = source_file;
-        while let Some(parent) = current.parent() {
-            if parent == root {
-                break;
-            }
-
-            debug!("Resolving absolute import in parent: {}", parent.display());
-
-            let mut result = resolve_absolute_import(
-                parent,
-                module_descriptor,
-                false,
-                false,
-                false,
-                true,
-                false,
-            );
-
-            if result.is_import_found {
-                if let Some(implicit_imports) = result
-                    .implicit_imports
-                    .filter(&module_descriptor.imported_symbols)
-                {
-                    result.implicit_imports = implicit_imports;
-                }
-                return result;
-            }
-
-            current = parent;
+    let mut current = source_file;
+    while let Some(parent) = current.parent() {
+        if !parent.starts_with(root) {
+            break;
         }
+
+        debug!("Resolving absolute import in parent: {}", parent.display());
+
+        let mut result =
+            resolve_absolute_import(parent, module_descriptor, false, false, false, true, false);
+
+        if result.is_import_found {
+            if let Some(implicit_imports) = result
+                .implicit_imports
+                .filter(&module_descriptor.imported_symbols)
+            {
+                result.implicit_imports = implicit_imports;
+            }
+            return result;
+        }
+
+        current = parent;
     }
 
     ImportResult::not_found()

scripts/check_docs_formatted.py

@@ -146,15 +146,19 @@ def format_file(
     if errors and not args.skip_errors and not error_known:
         for error in errors:
             rule_name = file.name.split(".")[0]
-            print(f"Docs parse error for `{rule_name}` docs: {error}")
+            print(
+                f"Docs parse error for `{rule_name}` docs. Either fix or add to "
+                f"`KNOWN_PARSE_ERRORS`. {error}",
+            )
 
         return 2
 
     if contents != new_contents:
         rule_name = file.name.split(".")[0]
         print(
-            f"Rule `{rule_name}` docs are not formatted. The example section "
-            f"should be rewritten to:",
+            f"Rule `{rule_name}` docs are not formatted. Either format the rule or add "
+            f"to `KNOWN_FORMATTING_VIOLATIONS`. The example section should be "
+            f"rewritten to:",
         )
 
         # Add indentation so that snipped can be copied directly to docs