Implement D407, D408, D409, D412, and D414 (#413)

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/) (32/48)
+- [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/) (37/48)
 - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (8/34)
 
 Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis Flake8:
@@ -337,6 +337,11 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
 | D410 | BlankLineAfterSection | Missing blank line after section ("Returns") |  |  |
 | D411 | BlankLineBeforeSection | Missing blank line before section ("Returns") |  |  |
 | D406 | NewLineAfterSectionName | Section name should end with a newline ("Returns") |  |  |
+| D407 | DashedUnderlineAfterSection | Missing dashed underline after section ("Returns") |  |  |
+| D408 | SectionUnderlineAfterName | Section underline should be in the line following the section's name ("Returns") |  |  |
+| D409 | SectionUnderlineMatchesSectionLength | Section underline should match the length of its name ("Returns") |  |  |
+| D412 | NoBlankLinesBetweenHeaderAndContent | No blank lines allowed between a section header and its content ("Returns") |  |  |
+| D414 | NonEmptySection | Section has no content ("Returns") |  |  |
 | M001 | UnusedNOQA | Unused `noqa` directive |  | 🛠 |
 
 ## Integrations

src/checks.rs

@@ -161,9 +161,14 @@ pub enum CheckCode {
     D106,
     D107,
     D200,
+    D201,
+    D202,
+    D203,
+    D204,
     D205,
     D209,
     D210,
+    D211,
     D212,
     D213,
     D300,
@@ -171,19 +176,19 @@ pub enum CheckCode {
     D402,
     D403,
     D404,
+    D405,
+    D406,
+    D407,
+    D408,
+    D409,
+    D410,
+    D411,
+    D412,
+    D413,
+    D414,
     D415,
     D418,
     D419,
-    D201,
-    D202,
-    D211,
-    D203,
-    D204,
-    D405,
-    D413,
-    D410,
-    D411,
-    D406,
     // Meta
     M001,
 }
@@ -282,6 +287,11 @@ pub enum CheckKind {
     UsePEP604Annotation,
     SuperCallWithParameters,
     // pydocstyle
+    BlankLineAfterLastSection(String),
+    BlankLineAfterSection(String),
+    BlankLineBeforeSection(String),
+    CapitalizeSectionName(String),
+    DashedUnderlineAfterSection(String),
     EndsInPeriod,
     EndsInPunctuation,
     FirstLineCapitalized,
@@ -290,14 +300,17 @@ pub enum CheckKind {
     MultiLineSummaryFirstLine,
     MultiLineSummarySecondLine,
     NewLineAfterLastParagraph,
+    NewLineAfterSectionName(String),
     NoBlankLineAfterFunction(usize),
     NoBlankLineAfterSummary,
     NoBlankLineBeforeClass(usize),
     NoBlankLineBeforeFunction(usize),
+    NoBlankLinesBetweenHeaderAndContent(String),
     NoSignature,
     NoSurroundingWhitespace,
     NoThisPrefix,
     NonEmpty,
+    NonEmptySection(String),
     OneBlankLineAfterClass(usize),
     OneBlankLineBeforeClass(usize),
     PublicClass,
@@ -307,13 +320,10 @@ pub enum CheckKind {
     PublicModule,
     PublicNestedClass,
     PublicPackage,
+    SectionUnderlineAfterName(String),
+    SectionUnderlineMatchesSectionLength(String),
     SkipDocstring,
     UsesTripleQuotes,
-    CapitalizeSectionName(String),
-    BlankLineAfterLastSection(String),
-    BlankLineAfterSection(String),
-    BlankLineBeforeSection(String),
-    NewLineAfterSectionName(String),
     // Meta
     UnusedNOQA(Option<Vec<String>>),
 }
@@ -451,14 +461,23 @@ impl CheckCode {
             CheckCode::D402 => CheckKind::NoSignature,
             CheckCode::D403 => CheckKind::FirstLineCapitalized,
             CheckCode::D404 => CheckKind::NoThisPrefix,
+            CheckCode::D405 => CheckKind::CapitalizeSectionName("returns".to_string()),
+            CheckCode::D406 => CheckKind::NewLineAfterSectionName("Returns".to_string()),
+            CheckCode::D407 => CheckKind::DashedUnderlineAfterSection("Returns".to_string()),
+            CheckCode::D408 => CheckKind::SectionUnderlineAfterName("Returns".to_string()),
+            CheckCode::D409 => {
+                CheckKind::SectionUnderlineMatchesSectionLength("Returns".to_string())
+            }
+            CheckCode::D410 => CheckKind::BlankLineAfterSection("Returns".to_string()),
+            CheckCode::D411 => CheckKind::BlankLineBeforeSection("Returns".to_string()),
+            CheckCode::D412 => {
+                CheckKind::NoBlankLinesBetweenHeaderAndContent("Returns".to_string())
+            }
+            CheckCode::D413 => CheckKind::BlankLineAfterLastSection("Returns".to_string()),
+            CheckCode::D414 => CheckKind::NonEmptySection("Returns".to_string()),
             CheckCode::D415 => CheckKind::EndsInPunctuation,
             CheckCode::D418 => CheckKind::SkipDocstring,
             CheckCode::D419 => CheckKind::NonEmpty,
-            CheckCode::D405 => CheckKind::CapitalizeSectionName("returns".to_string()),
-            CheckCode::D413 => CheckKind::BlankLineAfterLastSection("Returns".to_string()),
-            CheckCode::D410 => CheckKind::BlankLineAfterSection("Returns".to_string()),
-            CheckCode::D411 => CheckKind::BlankLineBeforeSection("Returns".to_string()),
-            CheckCode::D406 => CheckKind::NewLineAfterSectionName("Returns".to_string()),
             // Meta
             CheckCode::M001 => CheckKind::UnusedNOQA(None),
         }
@@ -548,6 +567,11 @@ impl CheckKind {
             CheckKind::UselessObjectInheritance(_) => &CheckCode::U004,
             CheckKind::SuperCallWithParameters => &CheckCode::U008,
             // pydocstyle
+            CheckKind::BlankLineAfterLastSection(_) => &CheckCode::D413,
+            CheckKind::BlankLineAfterSection(_) => &CheckCode::D410,
+            CheckKind::BlankLineBeforeSection(_) => &CheckCode::D411,
+            CheckKind::CapitalizeSectionName(_) => &CheckCode::D405,
+            CheckKind::DashedUnderlineAfterSection(_) => &CheckCode::D407,
             CheckKind::EndsInPeriod => &CheckCode::D400,
             CheckKind::EndsInPunctuation => &CheckCode::D415,
             CheckKind::FirstLineCapitalized => &CheckCode::D403,
@@ -556,14 +580,17 @@ impl CheckKind {
             CheckKind::MultiLineSummaryFirstLine => &CheckCode::D212,
             CheckKind::MultiLineSummarySecondLine => &CheckCode::D213,
             CheckKind::NewLineAfterLastParagraph => &CheckCode::D209,
+            CheckKind::NewLineAfterSectionName(_) => &CheckCode::D406,
             CheckKind::NoBlankLineAfterFunction(_) => &CheckCode::D202,
             CheckKind::NoBlankLineAfterSummary => &CheckCode::D205,
             CheckKind::NoBlankLineBeforeClass(_) => &CheckCode::D211,
             CheckKind::NoBlankLineBeforeFunction(_) => &CheckCode::D201,
+            CheckKind::NoBlankLinesBetweenHeaderAndContent(_) => &CheckCode::D412,
             CheckKind::NoSignature => &CheckCode::D402,
             CheckKind::NoSurroundingWhitespace => &CheckCode::D210,
             CheckKind::NoThisPrefix => &CheckCode::D404,
             CheckKind::NonEmpty => &CheckCode::D419,
+            CheckKind::NonEmptySection(_) => &CheckCode::D414,
             CheckKind::OneBlankLineAfterClass(_) => &CheckCode::D204,
             CheckKind::OneBlankLineBeforeClass(_) => &CheckCode::D203,
             CheckKind::PublicClass => &CheckCode::D101,
@@ -573,13 +600,10 @@ impl CheckKind {
             CheckKind::PublicModule => &CheckCode::D100,
             CheckKind::PublicNestedClass => &CheckCode::D106,
             CheckKind::PublicPackage => &CheckCode::D104,
+            CheckKind::SectionUnderlineAfterName(_) => &CheckCode::D408,
+            CheckKind::SectionUnderlineMatchesSectionLength(_) => &CheckCode::D409,
             CheckKind::SkipDocstring => &CheckCode::D418,
             CheckKind::UsesTripleQuotes => &CheckCode::D300,
-            CheckKind::CapitalizeSectionName(_) => &CheckCode::D405,
-            CheckKind::BlankLineAfterLastSection(_) => &CheckCode::D413,
-            CheckKind::BlankLineAfterSection(_) => &CheckCode::D410,
-            CheckKind::BlankLineBeforeSection(_) => &CheckCode::D411,
-            CheckKind::NewLineAfterSectionName(_) => &CheckCode::D406,
             // Meta
             CheckKind::UnusedNOQA(_) => &CheckCode::M001,
         }
@@ -900,6 +924,21 @@ impl CheckKind {
             CheckKind::NewLineAfterSectionName(name) => {
                 format!("Section name should end with a newline (\"{name}\")")
             }
+            CheckKind::DashedUnderlineAfterSection(name) => {
+                format!("Missing dashed underline after section (\"{name}\")")
+            }
+            CheckKind::SectionUnderlineAfterName(name) => {
+                format!("Section underline should be in the line following the section's name (\"{name}\")")
+            }
+            CheckKind::SectionUnderlineMatchesSectionLength(name) => {
+                format!("Section underline should match the length of its name (\"{name}\")")
+            }
+            CheckKind::NoBlankLinesBetweenHeaderAndContent(name) => {
+                format!(
+                    "No blank lines allowed between a section header and its content (\"{name}\")"
+                )
+            }
+            CheckKind::NonEmptySection(name) => format!("Section has no content (\"{name}\")"),
             // Meta
             CheckKind::UnusedNOQA(codes) => match codes {
                 None => "Unused `noqa` directive".to_string(),

src/docstrings/sections.rs

@@ -146,7 +146,7 @@ pub fn section_contexts<'a>(lines: &'a [&'a str]) -> Vec<SectionContext<'a>> {
             section_name: get_leading_words(lines[lineno]),
             previous_line: lines[lineno - 1],
             line: lines[lineno],
-            following_lines: &lines[lineno..],
+            following_lines: &lines[lineno + 1..],
             original_index: lineno,
             is_last_section: false,
         };
@@ -164,7 +164,7 @@ pub fn section_contexts<'a>(lines: &'a [&'a str]) -> Vec<SectionContext<'a>> {
             previous_line: context.previous_line,
             line: context.line,
             following_lines: if let Some(end) = end {
-                &lines[context.original_index..end]
+                &lines[context.original_index + 1..end]
             } else {
                 context.following_lines
             },
@@ -177,9 +177,137 @@ pub fn section_contexts<'a>(lines: &'a [&'a str]) -> Vec<SectionContext<'a>> {
     truncated_contexts
 }
 
+fn check_blanks_and_section_underline(
+    checker: &mut Checker,
+    definition: &Definition,
+    context: &SectionContext,
+) {
+    let docstring = definition
+        .docstring
+        .expect("Sections are only available for docstrings.");
+
+    let mut blank_lines_after_header = 0;
+    for line in context.following_lines {
+        if !line.trim().is_empty() {
+            break;
+        }
+        blank_lines_after_header += 1;
+    }
+
+    // Nothing but blank lines after the section header.
+    if blank_lines_after_header == context.following_lines.len() {
+        // D407
+        if checker.settings.enabled.contains(&CheckCode::D407) {
+            checker.add_check(Check::new(
+                CheckKind::DashedUnderlineAfterSection(context.section_name.to_string()),
+                range_for(docstring),
+            ));
+        }
+        // D414
+        if checker.settings.enabled.contains(&CheckCode::D414) {
+            checker.add_check(Check::new(
+                CheckKind::NonEmptySection(context.section_name.to_string()),
+                range_for(docstring),
+            ));
+        }
+        return;
+    }
+
+    let non_empty_line = context.following_lines[blank_lines_after_header];
+    let dash_line_found = non_empty_line
+        .chars()
+        .all(|char| char.is_whitespace() || char == '-');
+
+    if !dash_line_found {
+        // D407
+        if checker.settings.enabled.contains(&CheckCode::D407) {
+            checker.add_check(Check::new(
+                CheckKind::DashedUnderlineAfterSection(context.section_name.to_string()),
+                range_for(docstring),
+            ));
+        }
+        if blank_lines_after_header > 0 {
+            // D212
+            if checker.settings.enabled.contains(&CheckCode::D212) {
+                checker.add_check(Check::new(
+                    CheckKind::NoBlankLinesBetweenHeaderAndContent(
+                        context.section_name.to_string(),
+                    ),
+                    range_for(docstring),
+                ));
+            }
+        }
+    } else {
+        if blank_lines_after_header > 0 {
+            // D408
+            if checker.settings.enabled.contains(&CheckCode::D408) {
+                checker.add_check(Check::new(
+                    CheckKind::SectionUnderlineAfterName(context.section_name.to_string()),
+                    range_for(docstring),
+                ));
+            }
+        }
+
+        if non_empty_line
+            .trim()
+            .chars()
+            .filter(|char| *char == '-')
+            .count()
+            != context.section_name.len()
+        {
+            // D409
+            if checker.settings.enabled.contains(&CheckCode::D409) {
+                checker.add_check(Check::new(
+                    CheckKind::SectionUnderlineMatchesSectionLength(
+                        context.section_name.to_string(),
+                    ),
+                    range_for(docstring),
+                ));
+            }
+        }
+
+        // TODO(charlie): Implement D215, which requires indentation and leading space tracking.
+        let line_after_dashes_index = blank_lines_after_header + 1;
+
+        if line_after_dashes_index < context.following_lines.len() {
+            let line_after_dashes = context.following_lines[line_after_dashes_index];
+
+            if line_after_dashes.trim().is_empty() {
+                let rest_of_lines = &context.following_lines[line_after_dashes_index..];
+                if rest_of_lines.iter().all(|line| line.trim().is_empty()) {
+                    // D414
+                    if checker.settings.enabled.contains(&CheckCode::D414) {
+                        checker.add_check(Check::new(
+                            CheckKind::NonEmptySection(context.section_name.to_string()),
+                            range_for(docstring),
+                        ));
+                    }
+                } else {
+                    // 412
+                    if checker.settings.enabled.contains(&CheckCode::D412) {
+                        checker.add_check(Check::new(
+                            CheckKind::NoBlankLinesBetweenHeaderAndContent(
+                                context.section_name.to_string(),
+                            ),
+                            range_for(docstring),
+                        ));
+                    }
+                }
+            }
+        } else {
+            // D414
+            if checker.settings.enabled.contains(&CheckCode::D414) {
+                checker.add_check(Check::new(
+                    CheckKind::NonEmptySection(context.section_name.to_string()),
+                    range_for(docstring),
+                ));
+            }
+        }
+    }
+}
+
 fn check_common_section(checker: &mut Checker, definition: &Definition, context: &SectionContext) {
-    // TODO(charlie): Implement D214.
+    // TODO(charlie): Implement D214, which requires indentation and leading space tracking.
-    // TODO(charlie): Implement `_check_blanks_and_section_underline`.
     let docstring = definition
         .docstring
         .expect("Sections are only available for docstrings.");
@@ -235,6 +363,7 @@ pub fn check_numpy_section(
 ) {
     // TODO(charlie): Implement `_check_parameters_section`.
     check_common_section(checker, definition, context);
+    check_blanks_and_section_underline(checker, definition, context);
 
     if checker.settings.enabled.contains(&CheckCode::D406) {
         let suffix = context

src/linter.rs

@@ -1320,6 +1320,42 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn d407() -> Result<()> {
+        let mut checks = check_path(
+            Path::new("./resources/test/fixtures/sections.py"),
+            &settings::Settings::for_rule(CheckCode::D407),
+            &fixer::Mode::Generate,
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(checks);
+        Ok(())
+    }
+
+    #[test]
+    fn d408() -> Result<()> {
+        let mut checks = check_path(
+            Path::new("./resources/test/fixtures/sections.py"),
+            &settings::Settings::for_rule(CheckCode::D408),
+            &fixer::Mode::Generate,
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(checks);
+        Ok(())
+    }
+
+    #[test]
+    fn d409() -> Result<()> {
+        let mut checks = check_path(
+            Path::new("./resources/test/fixtures/sections.py"),
+            &settings::Settings::for_rule(CheckCode::D409),
+            &fixer::Mode::Generate,
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(checks);
+        Ok(())
+    }
+
     #[test]
     fn d410() -> Result<()> {
         let mut checks = check_path(
@@ -1344,6 +1380,18 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn d412() -> Result<()> {
+        let mut checks = check_path(
+            Path::new("./resources/test/fixtures/sections.py"),
+            &settings::Settings::for_rule(CheckCode::D412),
+            &fixer::Mode::Generate,
+        )?;
+        checks.sort_by_key(|check| check.location);
+        insta::assert_yaml_snapshot!(checks);
+        Ok(())
+    }
+
     #[test]
     fn d413() -> Result<()> {
         let mut checks = check_path(
@@ -1356,6 +1404,18 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn d414() -> Result<()> {
+        let mut checks = check_path(
+            Path::new("./resources/test/fixtures/sections.py"),
+            &settings::Settings::for_rule(CheckCode::D414),
+            &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(

src/snapshots/ruff__linter__tests__d407.snap

@@ -0,0 +1,50 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind:
+    DashedUnderlineAfterSection: Returns
+  location:
+    row: 42
+    column: 5
+  end_location:
+    row: 47
+    column: 8
+  fix: ~
+- kind:
+    DashedUnderlineAfterSection: Returns
+  location:
+    row: 54
+    column: 5
+  end_location:
+    row: 58
+    column: 8
+  fix: ~
+- kind:
+    DashedUnderlineAfterSection: Raises
+  location:
+    row: 207
+    column: 5
+  end_location:
+    row: 221
+    column: 8
+  fix: ~
+- kind:
+    DashedUnderlineAfterSection: Returns
+  location:
+    row: 252
+    column: 5
+  end_location:
+    row: 262
+    column: 8
+  fix: ~
+- kind:
+    DashedUnderlineAfterSection: Raises
+  location:
+    row: 252
+    column: 5
+  end_location:
+    row: 262
+    column: 8
+  fix: ~
+

src/snapshots/ruff__linter__tests__d408.snap

@@ -0,0 +1,14 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind:
+    SectionUnderlineAfterName: Returns
+  location:
+    row: 85
+    column: 5
+  end_location:
+    row: 92
+    column: 8
+  fix: ~
+

src/snapshots/ruff__linter__tests__d409.snap

@@ -0,0 +1,23 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind:
+    SectionUnderlineMatchesSectionLength: Returns
+  location:
+    row: 99
+    column: 5
+  end_location:
+    row: 105
+    column: 8
+  fix: ~
+- kind:
+    SectionUnderlineMatchesSectionLength: Returns
+  location:
+    row: 207
+    column: 5
+  end_location:
+    row: 221
+    column: 8
+  fix: ~
+

src/snapshots/ruff__linter__tests__d412.snap

@@ -0,0 +1,14 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind:
+    NoBlankLinesBetweenHeaderAndContent: Short summary
+  location:
+    row: 207
+    column: 5
+  end_location:
+    row: 221
+    column: 8
+  fix: ~
+

src/snapshots/ruff__linter__tests__d414.snap

@@ -0,0 +1,50 @@
+---
+source: src/linter.rs
+expression: checks
+---
+- kind:
+    NonEmptySection: Returns
+  location:
+    row: 54
+    column: 5
+  end_location:
+    row: 58
+    column: 8
+  fix: ~
+- kind:
+    NonEmptySection: Returns
+  location:
+    row: 67
+    column: 5
+  end_location:
+    row: 78
+    column: 8
+  fix: ~
+- kind:
+    NonEmptySection: Yields
+  location:
+    row: 67
+    column: 5
+  end_location:
+    row: 78
+    column: 8
+  fix: ~
+- kind:
+    NonEmptySection: Returns
+  location:
+    row: 161
+    column: 5
+  end_location:
+    row: 165
+    column: 8
+  fix: ~
+- kind:
+    NonEmptySection: Returns
+  location:
+    row: 252
+    column: 5
+  end_location:
+    row: 262
+    column: 8
+  fix: ~
+