From e9054232c062417532bacee8fbe8b8c67c74ad2a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 7 Feb 2024 13:48:28 -0800 Subject: [PATCH] Fix blank-line docstring rules for module-level docstrings (#9878) ## Summary Given: ```python """Make a summary line. Note: ---- Per the code comment the next two lines are blank. "// The first blank line is the line containing the closing triple quotes, so we need at least two." """ ``` It turns out we excluded the line ending in `"""`, because it's empty (unlike for functions, where it consists of the indent). This PR changes the `following_lines` iterator to always include the trailing newline, which gives us correct and consistent handling between function and module-level docstrings. Closes https://github.com/astral-sh/ruff/issues/9877. --- crates/ruff_linter/src/docstrings/sections.rs | 11 +++++--- .../src/rules/pydocstyle/rules/sections.rs | 27 ++++++++++--------- ...ules__pydocstyle__tests__D413_D413.py.snap | 7 +++-- crates/ruff_source_file/src/newlines.rs | 9 ++++++- 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/crates/ruff_linter/src/docstrings/sections.rs b/crates/ruff_linter/src/docstrings/sections.rs index 1ef49ea614604b..04dfb08e214eb9 100644 --- a/crates/ruff_linter/src/docstrings/sections.rs +++ b/crates/ruff_linter/src/docstrings/sections.rs @@ -5,7 +5,7 @@ use ruff_python_ast::docstrings::{leading_space, leading_words}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use strum_macros::EnumIter; -use ruff_source_file::{Line, UniversalNewlineIterator, UniversalNewlines}; +use ruff_source_file::{Line, NewlineWithTrailingNewline, UniversalNewlines}; use crate::docstrings::styles::SectionStyle; use crate::docstrings::{Docstring, DocstringBody}; @@ -356,13 +356,16 @@ impl<'a> SectionContext<'a> { pub(crate) fn previous_line(&self) -> Option<&'a str> { let previous = &self.docstring_body.as_str()[TextRange::up_to(self.range_relative().start())]; - previous.universal_newlines().last().map(|l| l.as_str()) + previous + .universal_newlines() + .last() + .map(|line| line.as_str()) } /// Returns the lines belonging to this section after the summary line. - pub(crate) fn following_lines(&self) -> UniversalNewlineIterator<'a> { + pub(crate) fn following_lines(&self) -> NewlineWithTrailingNewline<'a> { let lines = self.following_lines_str(); - UniversalNewlineIterator::with_offset(lines, self.offset() + self.data.summary_full_end) + NewlineWithTrailingNewline::with_offset(lines, self.offset() + self.data.summary_full_end) } fn following_lines_str(&self) -> &'a str { diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs index 5724ab8e00af98..7275cff37fd5a3 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs @@ -1634,12 +1634,13 @@ fn common_section( let line_end = checker.stylist().line_ending().as_str(); if let Some(next) = next { - if context - .following_lines() - .last() - .map_or(true, |line| !line.trim().is_empty()) - { - if checker.enabled(Rule::NoBlankLineAfterSection) { + if checker.enabled(Rule::NoBlankLineAfterSection) { + let num_blank_lines = context + .following_lines() + .rev() + .take_while(|line| line.trim().is_empty()) + .count(); + if num_blank_lines < 2 { let mut diagnostic = Diagnostic::new( NoBlankLineAfterSection { name: context.section_name().to_string(), @@ -1657,13 +1658,13 @@ fn common_section( } else { // The first blank line is the line containing the closing triple quotes, so we need at // least two. - let num_blank_lines = context - .following_lines() - .rev() - .take_while(|line| line.trim().is_empty()) - .count(); - if num_blank_lines < 2 { - if checker.enabled(Rule::BlankLineAfterLastSection) { + if checker.enabled(Rule::BlankLineAfterLastSection) { + let num_blank_lines = context + .following_lines() + .rev() + .take_while(|line| line.trim().is_empty()) + .count(); + if num_blank_lines < 2 { let mut diagnostic = Diagnostic::new( BlankLineAfterLastSection { name: context.section_name().to_string(), diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D413_D413.py.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D413_D413.py.snap index eda7d334cfaada..ae996b1e45c0b4 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D413_D413.py.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D413_D413.py.snap @@ -21,10 +21,9 @@ D413.py:1:1: D413 [*] Missing blank line after last section ("Returns") 7 7 | Returns: 8 8 | the value 9 |+ - 10 |+ -9 11 | """ -10 12 | -11 13 | +9 10 | """ +10 11 | +11 12 | D413.py:13:5: D413 [*] Missing blank line after last section ("Returns") | diff --git a/crates/ruff_source_file/src/newlines.rs b/crates/ruff_source_file/src/newlines.rs index 4e4d4e09a4a3ed..deb6d8469031a7 100644 --- a/crates/ruff_source_file/src/newlines.rs +++ b/crates/ruff_source_file/src/newlines.rs @@ -184,11 +184,18 @@ impl<'a> Iterator for NewlineWithTrailingNewline<'a> { type Item = Line<'a>; #[inline] - fn next(&mut self) -> Option> { + fn next(&mut self) -> Option { self.underlying.next().or_else(|| self.trailing.take()) } } +impl DoubleEndedIterator for NewlineWithTrailingNewline<'_> { + #[inline] + fn next_back(&mut self) -> Option { + self.trailing.take().or_else(|| self.underlying.next_back()) + } +} + #[derive(Debug, Clone, Eq, PartialEq)] pub struct Line<'a> { text: &'a str,