Implement autofix for D400 and D415 (#1094)

README.md

@@ -507,7 +507,7 @@ For more, see [pydocstyle](https://pypi.org/project/pydocstyle/6.1.1/) on PyPI.
 | D214 | SectionNotOverIndented | Section is over-indented ("Returns") | 🛠 |
 | D215 | SectionUnderlineNotOverIndented | Section underline is over-indented ("Returns") | 🛠 |
 | D300 | UsesTripleQuotes | Use """triple double quotes""" |  |
-| D400 | EndsInPeriod | First line should end with a period |  |
+| D400 | EndsInPeriod | First line should end with a period | 🛠 |
 | D402 | NoSignature | First line should not be the function's signature |  |
 | D403 | FirstLineCapitalized | First word of the first line should be properly capitalized |  |
 | D404 | NoThisPrefix | First word of the docstring should not be "This" |  |
@@ -521,7 +521,7 @@ For more, see [pydocstyle](https://pypi.org/project/pydocstyle/6.1.1/) on PyPI.
 | D412 | NoBlankLinesBetweenHeaderAndContent | No blank lines allowed between a section header and its content ("Returns") | 🛠 |
 | D413 | BlankLineAfterLastSection | Missing blank line after last section ("Returns") | 🛠 |
 | D414 | NonEmptySection | Section has no content ("Returns") |  |
-| D415 | EndsInPunctuation | First line should end with a period, question mark, or exclamation point |  |
+| D415 | EndsInPunctuation | First line should end with a period, question mark, or exclamation point | 🛠 |
 | D416 | SectionNameEndsInColon | Section name should end with a colon ("Returns") | 🛠 |
 | D417 | DocumentAllArguments | Missing argument descriptions in the docstring: `x`, `y` |  |
 | D418 | SkipDocstring | Function decorated with `@overload` shouldn't contain a docstring |  |

resources/test/fixtures/pydocstyle/D400.py

@@ -0,0 +1,73 @@
+def f():
+    "Here's a line without a period"
+    ...
+
+
+def f():
+    """Here's a line without a period"""
+    ...
+
+
+def f():
+    """
+    Here's a line without a period,
+    but here's the next line
+    """
+    ...
+
+
+def f():
+    """Here's a line without a period"""
+    ...
+
+
+def f():
+    """
+    Here's a line without a period,
+    but here's the next line"""
+    ...
+
+
+def f():
+    """
+    Here's a line without a period,
+    but here's the next line with trailing space """
+    ...
+
+
+
+def f():
+    r"Here's a line without a period"
+    ...
+
+
+def f():
+    r"""Here's a line without a period"""
+    ...
+
+
+def f():
+    r"""
+    Here's a line without a period,
+    but here's the next line
+    """
+    ...
+
+
+def f():
+    r"""Here's a line without a period"""
+    ...
+
+
+def f():
+    r"""
+    Here's a line without a period,
+    but here's the next line"""
+    ...
+
+
+def f():
+    r"""
+    Here's a line without a period,
+    but here's the next line with trailing space """
+    ...

src/checks.rs

@@ -2497,6 +2497,8 @@ impl CheckKind {
                 | CheckKind::DoNotAssertFalse
                 | CheckKind::DoNotAssignLambda
                 | CheckKind::DuplicateHandlerException(..)
+                | CheckKind::EndsInPeriod
+                | CheckKind::EndsInPunctuation
                 | CheckKind::GetAttrWithConstant
                 | CheckKind::ImplicitReturn
                 | CheckKind::ImplicitReturnValue

src/linter.rs

@@ -133,7 +133,7 @@ pub(crate) fn check_path(
     Ok(checks)
 }
 
-const MAX_ITERATIONS: usize = 100;
+const MAX_ITERATIONS: usize = 1;
 
 /// Lint the source code at the given `Path`.
 pub fn lint_path(

src/pydocstyle/helpers.rs

@@ -0,0 +1,43 @@
+use rustpython_ast::Expr;
+
+use crate::ast::types::Range;
+use crate::docstrings::constants;
+use crate::SourceCodeLocator;
+
+/// Return the leading quote string for a docstring (e.g., `"""`).
+pub fn leading_quote<'a>(docstring: &Expr, locator: &'a SourceCodeLocator) -> Option<&'a str> {
+    if let Some(first_line) = locator
+        .slice_source_code_range(&Range::from_located(docstring))
+        .lines()
+        .next()
+        .map(str::to_lowercase)
+    {
+        for pattern in constants::TRIPLE_QUOTE_PREFIXES
+            .iter()
+            .chain(constants::SINGLE_QUOTE_PREFIXES)
+        {
+            if first_line.starts_with(pattern) {
+                return Some(pattern);
+            }
+        }
+    }
+    None
+}
+
+/// Return the index of the first logical line in a string.
+pub fn logical_line(content: &str) -> Option<usize> {
+    // Find the first logical line.
+    let mut logical_line = None;
+    for (i, line) in content.lines().enumerate() {
+        if line.trim().is_empty() {
+            // Empty line. If this is the line _after_ the first logical line, stop.
+            if logical_line.is_some() {
+                break;
+            }
+        } else {
+            // Non-empty line. Store the index.
+            logical_line = Some(i);
+        }
+    }
+    logical_line
+}

src/pydocstyle/mod.rs

@@ -1,3 +1,4 @@
+mod helpers;
 pub mod plugins;
 
 #[cfg(test)]
@@ -36,7 +37,8 @@ mod tests {
     #[test_case(CheckCode::D214, Path::new("sections.py"); "D214")]
     #[test_case(CheckCode::D215, Path::new("sections.py"); "D215")]
     #[test_case(CheckCode::D300, Path::new("D.py"); "D300")]
-    #[test_case(CheckCode::D400, Path::new("D.py"); "D400")]
+    #[test_case(CheckCode::D400, Path::new("D.py"); "D400_0")]
+    #[test_case(CheckCode::D400, Path::new("D400.py"); "D400_1")]
     #[test_case(CheckCode::D402, Path::new("D.py"); "D402")]
     #[test_case(CheckCode::D403, Path::new("D.py"); "D403")]
     #[test_case(CheckCode::D404, Path::new("D.py"); "D404")]

src/pydocstyle/plugins.rs

@@ -16,6 +16,7 @@ use crate::docstrings::constants;
 use crate::docstrings::definition::{Definition, DefinitionKind};
 use crate::docstrings::sections::{section_contexts, SectionContext};
 use crate::docstrings::styles::SectionStyle;
+use crate::pydocstyle::helpers::{leading_quote, logical_line};
 use crate::visibility::{is_init, is_magic, is_overload, is_override, is_staticmethod, Visibility};
 
 /// D100, D101, D102, D103, D104, D105, D106, D107
@@ -621,18 +622,7 @@ pub fn no_surrounding_whitespace(checker: &mut Checker, definition: &Definition)
         Range::from_located(docstring),
     );
     if checker.patch(check.kind.code()) {
-        if let Some(first_line) = checker
+        if let Some(pattern) = leading_quote(docstring, checker.locator) {
-            .locator
-            .slice_source_code_range(&Range::from_located(docstring))
-            .lines()
-            .next()
-            .map(str::to_lowercase)
-        {
-            for pattern in constants::TRIPLE_QUOTE_PREFIXES
-                .iter()
-                .chain(constants::SINGLE_QUOTE_PREFIXES)
-            {
-                if first_line.starts_with(pattern) {
             if let Some(quote) = pattern.chars().last() {
                 // If removing whitespace would lead to an invalid string of quote
                 // characters, avoid applying the fix.
@@ -645,16 +635,11 @@ pub fn no_surrounding_whitespace(checker: &mut Checker, definition: &Definition)
                         ),
                         Location::new(
                             docstring.location.row(),
-                                    docstring.location.column()
+                            docstring.location.column() + pattern.len() + line.chars().count(),
-                                        + pattern.len()
-                                        + line.chars().count(),
                         ),
                     ));
                 }
             }
-                    break;
-                }
-            }
         }
     }
     checker.add_check(check);
@@ -751,16 +736,30 @@ pub fn ends_with_period(checker: &mut Checker, definition: &Definition) {
     } = &docstring.node else {
         return;
     };
-    let Some(string) = string.trim().lines().next() else {
+    if let Some(index) = logical_line(string) {
-        return;
+        let line = string.lines().nth(index).unwrap();
+        let trimmed = line.trim_end();
+        if !trimmed.ends_with('.') {
+            let mut check = Check::new(CheckKind::EndsInPeriod, Range::from_located(docstring));
+            // Best-effort autofix: avoid adding a period after other punctuation marks.
+            if checker.patch(&CheckCode::D400) && !trimmed.ends_with(':') && !trimmed.ends_with(';')
+            {
+                if let Some((row, column)) = if index == 0 {
+                    leading_quote(docstring, checker.locator).map(|pattern| {
+                        (
+                            docstring.location.row(),
+                            docstring.location.column() + pattern.len() + trimmed.chars().count(),
+                        )
+                    })
+                } else {
+                    Some((docstring.location.row() + index, trimmed.chars().count()))
+                } {
+                    check.amend(Fix::insertion(".".to_string(), Location::new(row, column)));
+                }
+            }
+            checker.add_check(check);
         };
-    if string.ends_with('.') {
+    }
-        return;
-    };
-    checker.add_check(Check::new(
-        CheckKind::EndsInPeriod,
-        Range::from_located(docstring),
-    ));
 }
 
 /// D402
@@ -878,16 +877,31 @@ pub fn ends_with_punctuation(checker: &mut Checker, definition: &Definition) {
     } = &docstring.node else {
         return
     };
-    let Some(string) = string.trim().lines().next() else {
+    if let Some(index) = logical_line(string) {
-        return
+        let line = string.lines().nth(index).unwrap();
-    };
+        let trimmed = line.trim_end();
-    if string.ends_with('.') || string.ends_with('!') || string.ends_with('?') {
+        if !(trimmed.ends_with('.') || trimmed.ends_with('!') || trimmed.ends_with('?')) {
-        return;
+            let mut check =
+                Check::new(CheckKind::EndsInPunctuation, Range::from_located(docstring));
+            // Best-effort autofix: avoid adding a period after other punctuation marks.
+            if checker.patch(&CheckCode::D415) && !trimmed.ends_with(':') && !trimmed.ends_with(';')
+            {
+                if let Some((row, column)) = if index == 0 {
+                    leading_quote(docstring, checker.locator).map(|pattern| {
+                        (
+                            docstring.location.row(),
+                            docstring.location.column() + pattern.len() + trimmed.chars().count(),
+                        )
+                    })
+                } else {
+                    Some((docstring.location.row() + index, trimmed.chars().count()))
+                } {
+                    check.amend(Fix::insertion(".".to_string(), Location::new(row, column)));
+                }
+            }
+            checker.add_check(check);
+        };
     }
-    checker.add_check(Check::new(
-        CheckKind::EndsInPunctuation,
-        Range::from_located(docstring),
-    ));
 }
 
 /// D418

src/pydocstyle/snapshots/ruff__pydocstyle__tests__D400_D.py.snap

@@ -9,7 +9,14 @@ expression: checks
   end_location:
     row: 355
     column: 17
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 355
+      column: 14
+    end_location:
+      row: 355
+      column: 14
 - kind: EndsInPeriod
   location:
     row: 406
@@ -17,7 +24,14 @@ expression: checks
   end_location:
     row: 406
     column: 39
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 406
+      column: 36
+    end_location:
+      row: 406
+      column: 36
 - kind: EndsInPeriod
   location:
     row: 410
@@ -25,7 +39,14 @@ expression: checks
   end_location:
     row: 410
     column: 24
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 410
+      column: 21
+    end_location:
+      row: 410
+      column: 21
 - kind: EndsInPeriod
   location:
     row: 416
@@ -33,7 +54,14 @@ expression: checks
   end_location:
     row: 416
     column: 24
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 416
+      column: 21
+    end_location:
+      row: 416
+      column: 21
 - kind: EndsInPeriod
   location:
     row: 422
@@ -41,7 +69,14 @@ expression: checks
   end_location:
     row: 422
     column: 49
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 422
+      column: 46
+    end_location:
+      row: 422
+      column: 46
 - kind: EndsInPeriod
   location:
     row: 429
@@ -49,7 +84,14 @@ expression: checks
   end_location:
     row: 429
     column: 63
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 429
+      column: 60
+    end_location:
+      row: 429
+      column: 60
 - kind: EndsInPeriod
   location:
     row: 470
@@ -57,7 +99,14 @@ expression: checks
   end_location:
     row: 470
     column: 24
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 470
+      column: 21
+    end_location:
+      row: 470
+      column: 21
 - kind: EndsInPeriod
   location:
     row: 475
@@ -65,7 +114,14 @@ expression: checks
   end_location:
     row: 475
     column: 24
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 475
+      column: 21
+    end_location:
+      row: 475
+      column: 21
 - kind: EndsInPeriod
   location:
     row: 480
@@ -73,7 +129,14 @@ expression: checks
   end_location:
     row: 480
     column: 24
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 480
+      column: 21
+    end_location:
+      row: 480
+      column: 21
 - kind: EndsInPeriod
   location:
     row: 487
@@ -81,7 +144,14 @@ expression: checks
   end_location:
     row: 487
     column: 24
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 487
+      column: 21
+    end_location:
+      row: 487
+      column: 21
 - kind: EndsInPeriod
   location:
     row: 509
@@ -89,7 +159,14 @@ expression: checks
   end_location:
     row: 509
     column: 34
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 509
+      column: 31
+    end_location:
+      row: 509
+      column: 31
 - kind: EndsInPeriod
   location:
     row: 514
@@ -97,7 +174,14 @@ expression: checks
   end_location:
     row: 514
     column: 33
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 514
+      column: 30
+    end_location:
+      row: 514
+      column: 30
 - kind: EndsInPeriod
   location:
     row: 520
@@ -105,7 +189,14 @@ expression: checks
   end_location:
     row: 520
     column: 32
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 520
+      column: 29
+    end_location:
+      row: 520
+      column: 29
 - kind: EndsInPeriod
   location:
     row: 581
@@ -113,5 +204,12 @@ expression: checks
   end_location:
     row: 581
     column: 51
-  fix: ~
+  fix:
+    content: "."
+    location:
+      row: 581
+      column: 47
+    end_location:
+      row: 581
+      column: 47
 

src/pydocstyle/snapshots/ruff__pydocstyle__tests__D400_D400.py.snap

@@ -0,0 +1,185 @@
+---
+source: src/pydocstyle/mod.rs
+expression: checks
+---
+- kind: EndsInPeriod
+  location:
+    row: 2
+    column: 4
+  end_location:
+    row: 2
+    column: 36
+  fix:
+    content: "."
+    location:
+      row: 2
+      column: 35
+    end_location:
+      row: 2
+      column: 35
+- kind: EndsInPeriod
+  location:
+    row: 7
+    column: 4
+  end_location:
+    row: 7
+    column: 40
+  fix:
+    content: "."
+    location:
+      row: 7
+      column: 37
+    end_location:
+      row: 7
+      column: 37
+- kind: EndsInPeriod
+  location:
+    row: 12
+    column: 4
+  end_location:
+    row: 15
+    column: 7
+  fix:
+    content: "."
+    location:
+      row: 14
+      column: 28
+    end_location:
+      row: 14
+      column: 28
+- kind: EndsInPeriod
+  location:
+    row: 20
+    column: 4
+  end_location:
+    row: 20
+    column: 40
+  fix:
+    content: "."
+    location:
+      row: 20
+      column: 37
+    end_location:
+      row: 20
+      column: 37
+- kind: EndsInPeriod
+  location:
+    row: 25
+    column: 4
+  end_location:
+    row: 27
+    column: 31
+  fix:
+    content: "."
+    location:
+      row: 27
+      column: 28
+    end_location:
+      row: 27
+      column: 28
+- kind: EndsInPeriod
+  location:
+    row: 32
+    column: 4
+  end_location:
+    row: 34
+    column: 52
+  fix:
+    content: "."
+    location:
+      row: 34
+      column: 48
+    end_location:
+      row: 34
+      column: 48
+- kind: EndsInPeriod
+  location:
+    row: 40
+    column: 4
+  end_location:
+    row: 40
+    column: 37
+  fix:
+    content: "."
+    location:
+      row: 40
+      column: 36
+    end_location:
+      row: 40
+      column: 36
+- kind: EndsInPeriod
+  location:
+    row: 45
+    column: 4
+  end_location:
+    row: 45
+    column: 41
+  fix:
+    content: "."
+    location:
+      row: 45
+      column: 38
+    end_location:
+      row: 45
+      column: 38
+- kind: EndsInPeriod
+  location:
+    row: 50
+    column: 4
+  end_location:
+    row: 53
+    column: 7
+  fix:
+    content: "."
+    location:
+      row: 52
+      column: 28
+    end_location:
+      row: 52
+      column: 28
+- kind: EndsInPeriod
+  location:
+    row: 58
+    column: 4
+  end_location:
+    row: 58
+    column: 41
+  fix:
+    content: "."
+    location:
+      row: 58
+      column: 38
+    end_location:
+      row: 58
+      column: 38
+- kind: EndsInPeriod
+  location:
+    row: 63
+    column: 4
+  end_location:
+    row: 65
+    column: 31
+  fix:
+    content: "."
+    location:
+      row: 65
+      column: 28
+    end_location:
+      row: 65
+      column: 28
+- kind: EndsInPeriod
+  location:
+    row: 70
+    column: 4
+  end_location:
+    row: 72
+    column: 52
+  fix:
+    content: "."
+    location:
+      row: 72
+      column: 48
+    end_location:
+      row: 72
+      column: 48
+