fix: issue D401 only for non-test/property functions and methods (#2071)

Extend test fixture to verify the targeting.

Includes two "attribute docstrings" which per PEP 257 are not recognized by the Python bytecode compiler or available as runtime object attributes. They are not available for us either at time of writing, but include them for completeness anyway in case they one day are.

resources/test/fixtures/pydocstyle/D401.py

@@ -1,3 +1,7 @@
+"""This module docstring does not need to be written in imperative mood."""
+
+from functools import cached_property
+
 # Bad examples
 
 def bad_liouiwnlkjl():
@@ -18,7 +22,11 @@ def bad_sdgfsdg23245777():
 
 def bad_run_something():
     """Runs something"""
-    pass
+
+    def bad_nested():
+        """Runs other things, nested"""
+
+    bad_nested()
 
 
 def multi_line():
@@ -32,6 +40,11 @@ def multi_line():
 def good_run_something():
     """Run away."""
 
+    def good_nested():
+        """Run to the hills."""
+
+    good_nested()
+
 
 def good_construct():
     """Construct a beautiful house."""
@@ -41,3 +54,48 @@ def good_multi_line():
     """Write a logical line that
     extends to two physical lines.
     """
+
+
+good_top_level_var = False
+"""This top level assignment attribute docstring does not need to be written in imperative mood."""
+
+
+# Classes and their members
+
+class Thingy:
+    """This class docstring does not need to be written in imperative mood."""
+
+    _beep = "boop"
+    """This class attribute docstring does not need to be written in imperative mood."""
+
+    def bad_method(self):
+        """This method docstring should be written in imperative mood."""
+
+    @property
+    def good_property(self):
+        """This property method docstring does not need to be written in imperative mood."""
+        return self._beep
+
+    @cached_property
+    def good_cached_property(self):
+        """This property method docstring does not need to be written in imperative mood."""
+        return 42 * 42
+
+    class NestedThingy:
+        """This nested class docstring does not need to be written in imperative mood."""
+
+
+# Test functions
+
+def test_something():
+    """This test function does not need to be written in imperative mood.
+
+    pydocstyle's rationale:
+    We exclude tests from the imperative mood check, because to phrase
+    their docstring in the imperative mood, they would have to start with
+    a highly redundant "Test that ..."
+    """
+
+
+def runTest():
+    """This test function does not need to be written in imperative mood, either."""

src/rules/pydocstyle/rules/non_imperative_mood.rs

@@ -2,18 +2,31 @@ use imperative::Mood;
 use once_cell::sync::Lazy;
 use ruff_macros::derive_message_formats;
 
+use crate::ast::cast;
 use crate::ast::types::Range;
 use crate::checkers::ast::Checker;
 use crate::define_violation;
-use crate::docstrings::definition::Docstring;
+use crate::docstrings::definition::{DefinitionKind, Docstring};
 use crate::registry::Diagnostic;
 use crate::rules::pydocstyle::helpers::normalize_word;
 use crate::violation::Violation;
+use crate::visibility::{is_property, is_test};
 
 static MOOD: Lazy<Mood> = Lazy::new(Mood::new);
 
 /// D401
 pub fn non_imperative_mood(checker: &mut Checker, docstring: &Docstring) {
+    let (
+        DefinitionKind::Function(parent)
+        | DefinitionKind::NestedFunction(parent)
+        | DefinitionKind::Method(parent)
+    ) = &docstring.kind else {
+        return;
+    };
+    if is_test(cast::name(parent)) || is_property(checker, cast::decorator_list(parent)) {
+        return;
+    }
+
     let body = docstring.body;
 
     // Find first line, disregarding whitespace.

src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D401_D401.py.snap

@@ -5,51 +5,71 @@ expression: diagnostics
 - kind:
     NonImperativeMood: Returns foo.
   location:
-    row: 4
+    row: 8
     column: 4
   end_location:
-    row: 4
+    row: 8
     column: 22
   fix: ~
   parent: ~
 - kind:
     NonImperativeMood: Constructor for a foo.
   location:
-    row: 8
+    row: 12
     column: 4
   end_location:
-    row: 8
+    row: 12
     column: 32
   fix: ~
   parent: ~
 - kind:
     NonImperativeMood: Constructor for a boa.
   location:
-    row: 12
+    row: 16
     column: 4
   end_location:
-    row: 16
+    row: 20
     column: 7
   fix: ~
   parent: ~
 - kind:
     NonImperativeMood: Runs something
   location:
-    row: 20
+    row: 24
     column: 4
   end_location:
-    row: 20
+    row: 24
     column: 24
   fix: ~
   parent: ~
+- kind:
+    NonImperativeMood: "Runs other things, nested"
+  location:
+    row: 27
+    column: 8
+  end_location:
+    row: 27
+    column: 39
+  fix: ~
+  parent: ~
 - kind:
     NonImperativeMood: Writes a logical line that
   location:
-    row: 25
+    row: 33
     column: 4
   end_location:
-    row: 27
+    row: 35
     column: 7
   fix: ~
   parent: ~
+- kind:
+    NonImperativeMood: This method docstring should be written in imperative mood.
+  location:
+    row: 72
+    column: 8
+  end_location:
+    row: 72
+    column: 73
+  fix: ~
+  parent: ~
 

src/visibility.rs

@@ -70,6 +70,15 @@ pub fn is_abstract(checker: &Checker, decorator_list: &[Expr]) -> bool {
     })
 }
 
+/// Returns `true` if a function definition is a `@property`.
+pub fn is_property(checker: &Checker, decorator_list: &[Expr]) -> bool {
+    decorator_list.iter().any(|expr| {
+        checker.resolve_call_path(expr).map_or(false, |call_path| {
+            call_path.as_slice() == ["", "property"]
+                || call_path.as_slice() == ["functools", "cached_property"]
+        })
+    })
+}
 /// Returns `true` if a function is a "magic method".
 pub fn is_magic(name: &str) -> bool {
     name.starts_with("__") && name.ends_with("__")
@@ -90,6 +99,11 @@ pub fn is_call(name: &str) -> bool {
     name == "__call__"
 }
 
+/// Returns `true` if a function is a test one.
+pub fn is_test(name: &str) -> bool {
+    name == "runTest" || name.starts_with("test")
+}
+
 /// Returns `true` if a module name indicates public visibility.
 fn is_public_module(module_name: &str) -> bool {
     !module_name.starts_with('_') || (module_name.starts_with("__") && module_name.ends_with("__"))