Implement D404 and D418 for pydocstyle (#409)

README.md

@@ -217,7 +217,7 @@ ruff also implements some of the most popular Flake8 plugins natively, including
 - [`flake8-print`](https://pypi.org/project/flake8-print/)
 - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) (12/16)
 - [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (3/32)
-- [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/) (25/48)
+- [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/) (27/48)
 - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (8/34)
 
 Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis Flake8:
@@ -323,7 +323,9 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
 | 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` |  |  |
 | D415 | EndsInPunctuation | First line should end with a period, question mark, or exclamation point |  |  |
+| D418 | SkipDocstring | Function decorated with @overload shouldn't contain a docstring |  |  |
 | D419 | NonEmpty | Docstring is empty |  |  |
 | D201 | NoBlankLineBeforeFunction | No blank lines allowed before function docstring (found 1) |  |  |
 | D202 | NoBlankLineAfterFunction | No blank lines allowed after function docstring (found 1) |  |  |

src/check_ast.rs

@@ -1955,9 +1955,15 @@ impl<'a> Checker<'a> {
             if self.settings.enabled.contains(&CheckCode::D403) {
                 docstrings::capitalized(self, &docstring);
             }
+            if self.settings.enabled.contains(&CheckCode::D404) {
+                docstrings::starts_with_this(self, &docstring);
+            }
             if self.settings.enabled.contains(&CheckCode::D415) {
                 docstrings::ends_with_punctuation(self, &docstring);
             }
+            if self.settings.enabled.contains(&CheckCode::D418) {
+                docstrings::if_needed(self, &docstring);
+            }
         }
     }
 

src/checks.rs

@@ -170,7 +170,9 @@ pub enum CheckCode {
     D400,
     D402,
     D403,
+    D404,
     D415,
+    D418,
     D419,
     D201,
     D202,
@@ -279,27 +281,29 @@ pub enum CheckKind {
     EndsInPunctuation,
     FirstLineCapitalized,
     FitsOnOneLine,
+    MagicMethod,
     MultiLineSummaryFirstLine,
     MultiLineSummarySecondLine,
     NewLineAfterLastParagraph,
-    NoBlankLineAfterSummary,
-    NoSurroundingWhitespace,
-    NonEmpty,
-    UsesTripleQuotes,
-    NoSignature,
-    NoBlankLineBeforeFunction(usize),
     NoBlankLineAfterFunction(usize),
+    NoBlankLineAfterSummary,
     NoBlankLineBeforeClass(usize),
-    OneBlankLineBeforeClass(usize),
+    NoBlankLineBeforeFunction(usize),
+    NoSignature,
+    NoSurroundingWhitespace,
+    NoThisPrefix,
+    NonEmpty,
     OneBlankLineAfterClass(usize),
-    PublicModule,
+    OneBlankLineBeforeClass(usize),
     PublicClass,
-    PublicMethod,
     PublicFunction,
-    PublicPackage,
-    MagicMethod,
-    PublicNestedClass,
     PublicInit,
+    PublicMethod,
+    PublicModule,
+    PublicNestedClass,
+    PublicPackage,
+    SkipDocstring,
+    UsesTripleQuotes,
     // Meta
     UnusedNOQA(Option<Vec<String>>),
 }
@@ -422,22 +426,24 @@ impl CheckCode {
             CheckCode::D106 => CheckKind::PublicNestedClass,
             CheckCode::D107 => CheckKind::PublicInit,
             CheckCode::D200 => CheckKind::FitsOnOneLine,
+            CheckCode::D201 => CheckKind::NoBlankLineBeforeFunction(1),
+            CheckCode::D202 => CheckKind::NoBlankLineAfterFunction(1),
+            CheckCode::D203 => CheckKind::OneBlankLineBeforeClass(0),
+            CheckCode::D204 => CheckKind::OneBlankLineAfterClass(0),
             CheckCode::D205 => CheckKind::NoBlankLineAfterSummary,
             CheckCode::D209 => CheckKind::NewLineAfterLastParagraph,
             CheckCode::D210 => CheckKind::NoSurroundingWhitespace,
-            CheckCode::D400 => CheckKind::EndsInPeriod,
-            CheckCode::D419 => CheckKind::NonEmpty,
+            CheckCode::D211 => CheckKind::NoBlankLineBeforeClass(1),
             CheckCode::D212 => CheckKind::MultiLineSummaryFirstLine,
             CheckCode::D213 => CheckKind::MultiLineSummarySecondLine,
             CheckCode::D300 => CheckKind::UsesTripleQuotes,
+            CheckCode::D400 => CheckKind::EndsInPeriod,
             CheckCode::D402 => CheckKind::NoSignature,
             CheckCode::D403 => CheckKind::FirstLineCapitalized,
+            CheckCode::D404 => CheckKind::NoThisPrefix,
             CheckCode::D415 => CheckKind::EndsInPunctuation,
-            CheckCode::D201 => CheckKind::NoBlankLineBeforeFunction(1),
-            CheckCode::D202 => CheckKind::NoBlankLineAfterFunction(1),
-            CheckCode::D211 => CheckKind::NoBlankLineBeforeClass(1),
-            CheckCode::D203 => CheckKind::OneBlankLineBeforeClass(0),
-            CheckCode::D204 => CheckKind::OneBlankLineAfterClass(0),
+            CheckCode::D418 => CheckKind::SkipDocstring,
+            CheckCode::D419 => CheckKind::NonEmpty,
             // Meta
             CheckCode::M001 => CheckKind::UnusedNOQA(None),
         }
@@ -527,31 +533,33 @@ impl CheckKind {
             CheckKind::UselessObjectInheritance(_) => &CheckCode::U004,
             CheckKind::SuperCallWithParameters => &CheckCode::U008,
             // pydocstyle
-            CheckKind::PublicModule => &CheckCode::D100,
-            CheckKind::PublicClass => &CheckCode::D101,
-            CheckKind::PublicMethod => &CheckCode::D102,
-            CheckKind::PublicFunction => &CheckCode::D103,
-            CheckKind::PublicPackage => &CheckCode::D104,
-            CheckKind::MagicMethod => &CheckCode::D105,
-            CheckKind::PublicNestedClass => &CheckCode::D106,
-            CheckKind::PublicInit => &CheckCode::D107,
-            CheckKind::FitsOnOneLine => &CheckCode::D200,
-            CheckKind::NoBlankLineAfterSummary => &CheckCode::D205,
-            CheckKind::NewLineAfterLastParagraph => &CheckCode::D209,
-            CheckKind::NoSurroundingWhitespace => &CheckCode::D210,
             CheckKind::EndsInPeriod => &CheckCode::D400,
-            CheckKind::NonEmpty => &CheckCode::D419,
+            CheckKind::EndsInPunctuation => &CheckCode::D415,
+            CheckKind::FirstLineCapitalized => &CheckCode::D403,
+            CheckKind::FitsOnOneLine => &CheckCode::D200,
+            CheckKind::MagicMethod => &CheckCode::D105,
             CheckKind::MultiLineSummaryFirstLine => &CheckCode::D212,
             CheckKind::MultiLineSummarySecondLine => &CheckCode::D213,
-            CheckKind::UsesTripleQuotes => &CheckCode::D300,
-            CheckKind::NoSignature => &CheckCode::D402,
-            CheckKind::FirstLineCapitalized => &CheckCode::D403,
-            CheckKind::EndsInPunctuation => &CheckCode::D415,
-            CheckKind::NoBlankLineBeforeFunction(_) => &CheckCode::D201,
+            CheckKind::NewLineAfterLastParagraph => &CheckCode::D209,
             CheckKind::NoBlankLineAfterFunction(_) => &CheckCode::D202,
+            CheckKind::NoBlankLineAfterSummary => &CheckCode::D205,
             CheckKind::NoBlankLineBeforeClass(_) => &CheckCode::D211,
-            CheckKind::OneBlankLineBeforeClass(_) => &CheckCode::D203,
+            CheckKind::NoBlankLineBeforeFunction(_) => &CheckCode::D201,
+            CheckKind::NoSignature => &CheckCode::D402,
+            CheckKind::NoSurroundingWhitespace => &CheckCode::D210,
+            CheckKind::NoThisPrefix => &CheckCode::D404,
+            CheckKind::NonEmpty => &CheckCode::D419,
             CheckKind::OneBlankLineAfterClass(_) => &CheckCode::D204,
+            CheckKind::OneBlankLineBeforeClass(_) => &CheckCode::D203,
+            CheckKind::PublicClass => &CheckCode::D101,
+            CheckKind::PublicFunction => &CheckCode::D103,
+            CheckKind::PublicInit => &CheckCode::D107,
+            CheckKind::PublicMethod => &CheckCode::D102,
+            CheckKind::PublicModule => &CheckCode::D100,
+            CheckKind::PublicNestedClass => &CheckCode::D106,
+            CheckKind::PublicPackage => &CheckCode::D104,
+            CheckKind::SkipDocstring => &CheckCode::D418,
+            CheckKind::UsesTripleQuotes => &CheckCode::D300,
             // Meta
             CheckKind::UnusedNOQA(_) => &CheckCode::M001,
         }
@@ -851,6 +859,12 @@ impl CheckKind {
             CheckKind::MagicMethod => "Missing docstring in magic method".to_string(),
             CheckKind::PublicNestedClass => "Missing docstring in public nested class".to_string(),
             CheckKind::PublicInit => "Missing docstring in __init__".to_string(),
+            CheckKind::NoThisPrefix => {
+                "First word of the docstring should not be `This`".to_string()
+            }
+            CheckKind::SkipDocstring => {
+                "Function decorated with @overload shouldn't contain a docstring".to_string()
+            }
             // Meta
             CheckKind::UnusedNOQA(codes) => match codes {
                 None => "Unused `noqa` directive".to_string(),

src/docstrings.rs

@@ -1,3 +1,5 @@
+//! Abstractions for tracking and validationg docstrings in Python code.
+
 use once_cell::sync::Lazy;
 use regex::Regex;
 use rustpython_ast::{Constant, Expr, ExprKind, Location, Stmt, StmtKind};
@@ -593,6 +595,32 @@ pub fn capitalized(checker: &mut Checker, definition: &Definition) {
     }
 }
 
+/// D404
+pub fn starts_with_this(checker: &mut Checker, definition: &Definition) {
+    if let Some(docstring) = definition.docstring {
+        if let ExprKind::Constant {
+            value: Constant::Str(string),
+            ..
+        } = &docstring.node
+        {
+            let trimmed = string.trim();
+            if trimmed.is_empty() {
+                return;
+            }
+
+            if let Some(first_word) = string.split(' ').next() {
+                if first_word
+                    .replace(|c: char| !c.is_alphanumeric(), "")
+                    .to_lowercase()
+                    == "this"
+                {
+                    checker.add_check(Check::new(CheckKind::NoThisPrefix, range_for(docstring)));
+                }
+            }
+        }
+    }
+}
+
 /// D415
 pub fn ends_with_punctuation(checker: &mut Checker, definition: &Definition) {
     if let Some(docstring) = definition.docstring {
@@ -613,6 +641,23 @@ pub fn ends_with_punctuation(checker: &mut Checker, definition: &Definition) {
     }
 }
 
+/// D418
+pub fn if_needed(checker: &mut Checker, definition: &Definition) {
+    if definition.docstring.is_some() {
+        if let DefinitionKind::Function(stmt)
+        | DefinitionKind::NestedFunction(stmt)
+        | DefinitionKind::Method(stmt) = definition.kind
+        {
+            if is_overload(stmt) {
+                checker.add_check(Check::new(
+                    CheckKind::SkipDocstring,
+                    Range::from_located(stmt),
+                ));
+            }
+        }
+    }
+}
+
 /// D419
 pub fn not_empty(checker: &mut Checker, definition: &Definition) -> bool {
     if let Some(docstring) = definition.docstring {

src/linter.rs

@@ -1284,6 +1284,18 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn d404() -> Result<()> {
+        let mut checks = check_path(
+            Path::new("./resources/test/fixtures/D.py"),
+            &settings::Settings::for_rule(CheckCode::D404),
+            &fixer::Mode::Generate,
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(checks);
+        Ok(())
+    }
+
     #[test]
     fn d415() -> Result<()> {
         let mut checks = check_path(
@@ -1296,6 +1308,18 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn d418() -> Result<()> {
+        let mut checks = check_path(
+            Path::new("./resources/test/fixtures/D.py"),
+            &settings::Settings::for_rule(CheckCode::D418),
+            &fixer::Mode::Generate,
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(checks);
+        Ok(())
+    }
+
     #[test]
     fn d419() -> Result<()> {
         let mut checks = check_path(

src/snapshots/ruff__linter__tests__d404.snap

@@ -0,0 +1,6 @@
+---
+source: src/linter.rs
+expression: checks
+---
+[]
+

src/snapshots/ruff__linter__tests__d418.snap

@@ -0,0 +1,29 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind: SkipDocstring
+  location:
+    row: 33
+    column: 5
+  end_location:
+    row: 37
+    column: 5
+  fix: ~
+- kind: SkipDocstring
+  location:
+    row: 85
+    column: 5
+  end_location:
+    row: 89
+    column: 5
+  fix: ~
+- kind: SkipDocstring
+  location:
+    row: 105
+    column: 1
+  end_location:
+    row: 110
+    column: 1
+  fix: ~
+

src/visibility.rs

@@ -1,3 +1,5 @@
+//! Abstractions for tracking public and private visibility across modules, classes, and functions.
+
 use std::path::Path;
 
 use crate::ast::helpers::match_name_or_attr;
@@ -24,6 +26,7 @@ pub struct VisibleScope {
     pub visibility: Visibility,
 }
 
+/// Returns `true` if a function definition is an `@overload`.
 pub fn is_overload(stmt: &Stmt) -> bool {
     match &stmt.node {
         StmtKind::FunctionDef { decorator_list, .. }
@@ -34,6 +37,7 @@ pub fn is_overload(stmt: &Stmt) -> bool {
     }
 }
 
+/// Returns `true` if a function is a "magic method".
 pub fn is_magic(stmt: &Stmt) -> bool {
     match &stmt.node {
         StmtKind::FunctionDef { name, .. } | StmtKind::AsyncFunctionDef { name, .. } => {
@@ -47,6 +51,7 @@ pub fn is_magic(stmt: &Stmt) -> bool {
     }
 }
 
+/// Returns `true` if a function is an `__init__`.
 pub fn is_init(stmt: &Stmt) -> bool {
     match &stmt.node {
         StmtKind::FunctionDef { name, .. } | StmtKind::AsyncFunctionDef { name, .. } => {
@@ -56,13 +61,14 @@ pub fn is_init(stmt: &Stmt) -> bool {
     }
 }
 
-fn is_private_name(module_name: &str) -> bool {
+/// Returns `true` if a module name indicates private visibility.
+fn is_private_module(module_name: &str) -> bool {
     module_name.starts_with('_') || (module_name.starts_with("__") && module_name.ends_with("__"))
 }
 
 pub fn module_visibility(path: &Path) -> Visibility {
     for component in path.iter().rev() {
-        if is_private_name(&component.to_string_lossy()) {
+        if is_private_module(&component.to_string_lossy()) {
             return Visibility::Private;
         }
     }
@@ -115,6 +121,9 @@ fn class_visibility(stmt: &Stmt) -> Visibility {
 }
 
 /// Transition a `VisibleScope` based on a new `Documentable` definition.
+///
+/// `scope` is the current `VisibleScope`, while `Documentable` and `Stmt` describe the current
+/// node used to modify visibility.
 pub fn transition_scope(scope: &VisibleScope, stmt: &Stmt, kind: &Documentable) -> VisibleScope {
     match kind {
         Documentable::Function => VisibleScope {