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.
This commit is contained in:
Charlie Marsh 2024-02-07 13:48:28 -08:00 committed by GitHub
parent 533dcfb114
commit 45937426c7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 32 additions and 22 deletions

View file

@ -5,7 +5,7 @@ use ruff_python_ast::docstrings::{leading_space, leading_words};
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
use strum_macros::EnumIter; 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::styles::SectionStyle;
use crate::docstrings::{Docstring, DocstringBody}; use crate::docstrings::{Docstring, DocstringBody};
@ -356,13 +356,16 @@ impl<'a> SectionContext<'a> {
pub(crate) fn previous_line(&self) -> Option<&'a str> { pub(crate) fn previous_line(&self) -> Option<&'a str> {
let previous = let previous =
&self.docstring_body.as_str()[TextRange::up_to(self.range_relative().start())]; &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. /// 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(); 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 { fn following_lines_str(&self) -> &'a str {

View file

@ -1634,12 +1634,13 @@ fn common_section(
let line_end = checker.stylist().line_ending().as_str(); let line_end = checker.stylist().line_ending().as_str();
if let Some(next) = next { 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( let mut diagnostic = Diagnostic::new(
NoBlankLineAfterSection { NoBlankLineAfterSection {
name: context.section_name().to_string(), name: context.section_name().to_string(),
@ -1657,13 +1658,13 @@ fn common_section(
} else { } else {
// The first blank line is the line containing the closing triple quotes, so we need at // The first blank line is the line containing the closing triple quotes, so we need at
// least two. // least two.
if checker.enabled(Rule::BlankLineAfterLastSection) {
let num_blank_lines = context let num_blank_lines = context
.following_lines() .following_lines()
.rev() .rev()
.take_while(|line| line.trim().is_empty()) .take_while(|line| line.trim().is_empty())
.count(); .count();
if num_blank_lines < 2 { if num_blank_lines < 2 {
if checker.enabled(Rule::BlankLineAfterLastSection) {
let mut diagnostic = Diagnostic::new( let mut diagnostic = Diagnostic::new(
BlankLineAfterLastSection { BlankLineAfterLastSection {
name: context.section_name().to_string(), name: context.section_name().to_string(),

View file

@ -21,10 +21,9 @@ D413.py:1:1: D413 [*] Missing blank line after last section ("Returns")
7 7 | Returns: 7 7 | Returns:
8 8 | the value 8 8 | the value
9 |+ 9 |+
10 |+ 9 10 | """
9 11 | """ 10 11 |
10 12 | 11 12 |
11 13 |
D413.py:13:5: D413 [*] Missing blank line after last section ("Returns") D413.py:13:5: D413 [*] Missing blank line after last section ("Returns")
| |

View file

@ -184,11 +184,18 @@ impl<'a> Iterator for NewlineWithTrailingNewline<'a> {
type Item = Line<'a>; type Item = Line<'a>;
#[inline] #[inline]
fn next(&mut self) -> Option<Line<'a>> { fn next(&mut self) -> Option<Self::Item> {
self.underlying.next().or_else(|| self.trailing.take()) self.underlying.next().or_else(|| self.trailing.take())
} }
} }
impl DoubleEndedIterator for NewlineWithTrailingNewline<'_> {
#[inline]
fn next_back(&mut self) -> Option<Self::Item> {
self.trailing.take().or_else(|| self.underlying.next_back())
}
}
#[derive(Debug, Clone, Eq, PartialEq)] #[derive(Debug, Clone, Eq, PartialEq)]
pub struct Line<'a> { pub struct Line<'a> {
text: &'a str, text: &'a str,