Skip to content

Commit

Permalink
Fix blank-line docstring rules for module-level docstrings (#9878)
Browse files Browse the repository at this point in the history
## 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 #9877.
  • Loading branch information
charliermarsh committed Feb 7, 2024
1 parent 533dcfb commit 4593742
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 22 deletions.
11 changes: 7 additions & 4 deletions crates/ruff_linter/src/docstrings/sections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down
27 changes: 14 additions & 13 deletions crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
|
Expand Down
9 changes: 8 additions & 1 deletion crates/ruff_source_file/src/newlines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,18 @@ impl<'a> Iterator for NewlineWithTrailingNewline<'a> {
type Item = Line<'a>;

#[inline]
fn next(&mut self) -> Option<Line<'a>> {
fn next(&mut self) -> Option<Self::Item> {
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)]
pub struct Line<'a> {
text: &'a str,
Expand Down

0 comments on commit 4593742

Please sign in to comment.