Don't raise D208 when last line is non-empty (#13372)

Co-authored-by: Micha Reiser <micha@reiser.io>

crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py

@@ -14,3 +14,36 @@ def memory_test():
     """
 　　　参数含义：precision:精确到小数点后几位
     """
+
+
+
+class Platform:
+    """Over indented last line
+    Args:
+    Returns:
+                """
+
+
+class Platform:
+    """All lines are over indented including the last containing the closing quotes
+        Args:
+        Returns:
+        """
+
+class Platform:
+    """All lines are over indented including the last
+        Args:
+        Returns"""
+
+# OK: This doesn't get flagged because it is accepted when the closing quotes are on a separate line  (see next test). Raises D209
+class Platform:
+    """Over indented last line with content
+    Args:
+        Some content on the last line"""
+
+# OK:
+class Platform:
+    """Over indented last line with content
+    Args:
+        Some content on the last line
+    """

crates/ruff_linter/src/lib.rs

@@ -46,3 +46,42 @@ pub mod upstream_categories;
 pub mod test;
 
 pub const RUFF_PKG_VERSION: &str = env!("CARGO_PKG_VERSION");
+
+#[cfg(test)]
+mod tests {
+    use std::path::Path;
+
+    use ruff_python_ast::PySourceType;
+
+    use crate::codes::Rule;
+    use crate::settings::LinterSettings;
+    use crate::source_kind::SourceKind;
+    use crate::test::{print_messages, test_contents};
+
+    /// Test for ad-hoc debugging.
+    #[test]
+    #[ignore]
+    fn linter_quick_test() {
+        let code = r#"class Platform:
+    """ Remove sampler
+            Args:
+            Returns:
+            """
+"#;
+        let source_type = PySourceType::Python;
+        let rule = Rule::OverIndentation;
+
+        let source_kind = SourceKind::from_source_code(code.to_string(), source_type)
+            .expect("Source code should be valid")
+            .expect("Notebook to contain python code");
+
+        let (diagnostics, fixed) = test_contents(
+            &source_kind,
+            Path::new("ruff_linter/rules/quick_test"),
+            &LinterSettings::for_rule(rule),
+        );
+
+        assert_eq!(print_messages(&diagnostics), "");
+        assert_eq!(fixed.source_code(), code);
+    }
+}

crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs

@@ -2,7 +2,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Violation};
 use ruff_diagnostics::{Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::docstrings::{clean_space, leading_space};
-use ruff_source_file::NewlineWithTrailingNewline;
+use ruff_source_file::{Line, NewlineWithTrailingNewline};
 use ruff_text_size::{Ranged, TextSize};
 use ruff_text_size::{TextLen, TextRange};
 
@@ -169,32 +169,49 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) {
     let body = docstring.body();
 
     // Split the docstring into lines.
-    let lines: Vec<_> = NewlineWithTrailingNewline::with_offset(&body, body.start()).collect();
+    let mut lines = NewlineWithTrailingNewline::with_offset(&body, body.start()).peekable();
-    if lines.len() <= 1 {
+
+    // The current line being processed
+    let mut current: Option<Line> = lines.next();
+
+    if lines.peek().is_none() {
         return;
     }
 
     let mut has_seen_tab = docstring.indentation.contains('\t');
-    let mut is_over_indented = true;
-    let mut over_indented_lines = vec![];
-    let mut over_indented_size = usize::MAX;
-
     let docstring_indent_size = docstring.indentation.chars().count();
-    for i in 0..lines.len() {
+
-        // First lines and continuations doesn't need any indentation.
+    // Lines, other than the last, that are over indented.
-        if i == 0 || lines[i - 1].ends_with('\\') {
+    let mut over_indented_lines = vec![];
+    // The smallest over indent that all docstring lines have in common. None if any line isn't over indented.
+    let mut smallest_over_indent_size = Some(usize::MAX);
+    // The last processed line
+    let mut last = None;
+
+    while let Some(line) = current {
+        // First lines and continuations don't need any indentation.
+        if last.is_none()
+            || last
+                .as_deref()
+                .is_some_and(|last: &str| last.ends_with('\\'))
+        {
+            last = Some(line);
+            current = lines.next();
             continue;
         }
 
-        let line = &lines[i];
+        let is_last = lines.peek().is_none();
+
         // Omit empty lines, except for the last line, which is non-empty by way of
         // containing the closing quotation marks.
         let is_blank = line.trim().is_empty();
-        if i < lines.len() - 1 && is_blank {
+        if !is_last && is_blank {
+            last = Some(line);
+            current = lines.next();
             continue;
         }
 
-        let line_indent = leading_space(line);
+        let line_indent = leading_space(&line);
         let line_indent_size = line_indent.chars().count();
 
         // We only report tab indentation once, so only check if we haven't seen a tab
@@ -204,7 +221,7 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) {
         if checker.enabled(Rule::UnderIndentation) {
             // We report under-indentation on every line. This isn't great, but enables
             // fix.
-            if (i == lines.len() - 1 || !is_blank) && line_indent_size < docstring_indent_size {
+            if (is_last || !is_blank) && line_indent_size < docstring_indent_size {
                 let mut diagnostic =
                     Diagnostic::new(UnderIndentation, TextRange::empty(line.start()));
                 diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
@@ -215,23 +232,35 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) {
             }
         }
 
+        // Only true when the last line is indentation only followed by the closing quotes.
+        // False when it is not the last line or the last line contains content other than the closing quotes.
+        // The last line only has special handling when it contains no other content.
+        let is_last_closing_quotes_only = is_last && is_blank;
+
         // Like pydocstyle, we only report over-indentation if either: (1) every line
         // (except, optionally, the last line) is over-indented, or (2) the last line
         // (which contains the closing quotation marks) is
         // over-indented. We can't know if we've achieved that condition
         // until we've viewed all the lines, so for now, just track
         // the over-indentation status of every line.
-        if i < lines.len() - 1 {
+        if !is_last_closing_quotes_only {
-            if line_indent_size > docstring_indent_size {
+            smallest_over_indent_size =
-                over_indented_lines.push(line);
+                smallest_over_indent_size.and_then(|smallest_over_indent_size| {
+                    let over_indent_size = line_indent_size.saturating_sub(docstring_indent_size);
 
+                    // `docstring_indent_size < line_indent_size`
+                    if over_indent_size > 0 {
+                        over_indented_lines.push(line.clone());
                         // Track the _smallest_ offset we see, in terms of characters.
-                over_indented_size =
+                        Some(smallest_over_indent_size.min(over_indent_size))
-                    std::cmp::min(line_indent_size - docstring_indent_size, over_indented_size);
                     } else {
-                is_over_indented = false;
+                        None
                     }
+                });
         }
+
+        last = Some(line);
+        current = lines.next();
     }
 
     if checker.enabled(Rule::IndentWithSpaces) {
@@ -244,9 +273,9 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) {
 
     if checker.enabled(Rule::OverIndentation) {
         // If every line (except the last) is over-indented...
-        if is_over_indented {
+        if let Some(smallest_over_indent_size) = smallest_over_indent_size {
             for line in over_indented_lines {
-                let line_indent = leading_space(line);
+                let line_indent = leading_space(&line);
                 let indent = clean_space(docstring.indentation);
 
                 // We report over-indentation on every line. This isn't great, but
@@ -275,7 +304,7 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) {
                         .locator()
                         .after(line.start())
                         .chars()
-                        .take(docstring.indentation.chars().count() + over_indented_size)
+                        .take(docstring_indent_size + smallest_over_indent_size)
                         .map(TextLen::text_len)
                         .sum::<TextSize>();
                     let range = TextRange::at(line.start(), offset);
@@ -287,10 +316,13 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) {
         }
 
         // If the last line is over-indented...
-        if let Some(last) = lines.last() {
+        if let Some(last) = last {
-            let line_indent = leading_space(last);
+            let line_indent = leading_space(&last);
             let line_indent_size = line_indent.chars().count();
-            if line_indent_size > docstring_indent_size {
+            let last_line_over_indent = line_indent_size.saturating_sub(docstring_indent_size);
+
+            let is_indent_only = line_indent.len() == last.len();
+            if last_line_over_indent > 0 && is_indent_only {
                 let mut diagnostic =
                     Diagnostic::new(OverIndentation, TextRange::empty(last.start()));
                 let indent = clean_space(docstring.indentation);

crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap

@@ -78,4 +78,124 @@ D208.py:10:1: D208 [*] Docstring is over-indented
 12 12 | 
 13 13 | def memory_test():
 
+D208.py:24:1: D208 [*] Docstring is over-indented
+   |
+22 |     Args:
+23 |     Returns:
+24 |                 """
+   |  D208
+   |
+   = help: Remove over-indentation
 
+ℹ Safe fix
+21 21 |     """Over indented last line
+22 22 |     Args:
+23 23 |     Returns:
+24    |-                """
+   24 |+    """
+25 25 | 
+26 26 | 
+27 27 | class Platform:
+
+D208.py:29:1: D208 [*] Docstring is over-indented
+   |
+27 | class Platform:
+28 |     """All lines are over indented including the last containing the closing quotes
+29 |         Args:
+   |  D208
+30 |         Returns:
+31 |         """
+   |
+   = help: Remove over-indentation
+
+ℹ Safe fix
+26 26 | 
+27 27 | class Platform:
+28 28 |     """All lines are over indented including the last containing the closing quotes
+29    |-        Args:
+   29 |+    Args:
+30 30 |         Returns:
+31 31 |         """
+32 32 | 
+
+D208.py:30:1: D208 [*] Docstring is over-indented
+   |
+28 |     """All lines are over indented including the last containing the closing quotes
+29 |         Args:
+30 |         Returns:
+   |  D208
+31 |         """
+   |
+   = help: Remove over-indentation
+
+ℹ Safe fix
+27 27 | class Platform:
+28 28 |     """All lines are over indented including the last containing the closing quotes
+29 29 |         Args:
+30    |-        Returns:
+   30 |+    Returns:
+31 31 |         """
+32 32 | 
+33 33 | class Platform:
+
+D208.py:31:1: D208 [*] Docstring is over-indented
+   |
+29 |         Args:
+30 |         Returns:
+31 |         """
+   |  D208
+32 | 
+33 | class Platform:
+   |
+   = help: Remove over-indentation
+
+ℹ Safe fix
+28 28 |     """All lines are over indented including the last containing the closing quotes
+29 29 |         Args:
+30 30 |         Returns:
+31    |-        """
+   31 |+    """
+32 32 | 
+33 33 | class Platform:
+34 34 |     """All lines are over indented including the last
+
+D208.py:35:1: D208 [*] Docstring is over-indented
+   |
+33 | class Platform:
+34 |     """All lines are over indented including the last
+35 |         Args:
+   |  D208
+36 |         Returns"""
+   |
+   = help: Remove over-indentation
+
+ℹ Safe fix
+32 32 | 
+33 33 | class Platform:
+34 34 |     """All lines are over indented including the last
+35    |-        Args:
+   35 |+    Args:
+36 36 |         Returns"""
+37 37 | 
+38 38 | # OK: This doesn't get flagged because it is accepted when the closing quotes are on a separate line  (see next test). Raises D209
+
+D208.py:36:1: D208 [*] Docstring is over-indented
+   |
+34 |     """All lines are over indented including the last
+35 |         Args:
+36 |         Returns"""
+   |  D208
+37 | 
+38 | # OK: This doesn't get flagged because it is accepted when the closing quotes are on a separate line  (see next test). Raises D209
+   |
+   = help: Remove over-indentation
+
+ℹ Safe fix
+33 33 | class Platform:
+34 34 |     """All lines are over indented including the last
+35 35 |         Args:
+36    |-        Returns"""
+   36 |+    Returns"""
+37 37 | 
+38 38 | # OK: This doesn't get flagged because it is accepted when the closing quotes are on a separate line  (see next test). Raises D209
+39 39 | class Platform: