Detect consecutive, non-newline-delimited NumPy sections (#5467)
## Summary
Given a docstring like:
```py
def f(a: int, b: int) -> int:
"""Showcase function.
Parameters
----------
a : int
_description_
b : int
_description_
Returns
-------
int
_description
"""
```
We were failing to identify `Returns` as a section, because the previous
line was neither empty nor ended with punctuation. This was causing a
false negative, where by we weren't flagging a missing line before
`Returns`. So, the very reason for the rule (no blank line) was causing
us to fail to catch it.
Note that, we did have a test case for this, which was working properly:
```py
def f() -> int:
"""Showcase function.
Parameters
----------
Returns
-------
"""
```
...because the line before `Returns` "ends in a punctuation mark" (`-`).
Closes #5442.
This commit is contained in:
25
crates/ruff/resources/test/fixtures/pydocstyle/D410.py
vendored
Normal file
25
crates/ruff/resources/test/fixtures/pydocstyle/D410.py
vendored
Normal file
@@ -0,0 +1,25 @@
|
||||
def f(a: int, b: int) -> int:
|
||||
"""Showcase function.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
a : int
|
||||
_description_
|
||||
b : int
|
||||
_description_
|
||||
Returns
|
||||
-------
|
||||
int
|
||||
_description
|
||||
"""
|
||||
return b - a
|
||||
|
||||
|
||||
def f() -> int:
|
||||
"""Showcase function.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
Returns
|
||||
-------
|
||||
"""
|
||||
@@ -36,6 +36,7 @@ use crate::importer::Importer;
|
||||
use crate::noqa::NoqaMapping;
|
||||
use crate::registry::Rule;
|
||||
use crate::rules::flake8_builtins::helpers::AnyShadowing;
|
||||
|
||||
use crate::rules::{
|
||||
airflow, flake8_2020, flake8_annotations, flake8_async, flake8_bandit, flake8_blind_except,
|
||||
flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_datetimez,
|
||||
|
||||
@@ -5,7 +5,7 @@ use ruff_python_ast::docstrings::{leading_space, leading_words};
|
||||
use ruff_text_size::{TextLen, TextRange, TextSize};
|
||||
use strum_macros::EnumIter;
|
||||
|
||||
use ruff_python_whitespace::{UniversalNewlineIterator, UniversalNewlines};
|
||||
use ruff_python_whitespace::{Line, UniversalNewlineIterator, UniversalNewlines};
|
||||
|
||||
use crate::docstrings::styles::SectionStyle;
|
||||
use crate::docstrings::{Docstring, DocstringBody};
|
||||
@@ -144,15 +144,13 @@ impl<'a> SectionContexts<'a> {
|
||||
|
||||
let mut contexts = Vec::new();
|
||||
let mut last: Option<SectionContextData> = None;
|
||||
let mut previous_line = None;
|
||||
|
||||
for line in contents.universal_newlines() {
|
||||
if previous_line.is_none() {
|
||||
// skip the first line
|
||||
previous_line = Some(line.as_str());
|
||||
continue;
|
||||
}
|
||||
let mut lines = contents.universal_newlines().peekable();
|
||||
|
||||
// Skip the first line, which is the summary.
|
||||
let mut previous_line = lines.next();
|
||||
|
||||
while let Some(line) = lines.next() {
|
||||
if let Some(section_kind) = suspected_as_section(&line, style) {
|
||||
let indent = leading_space(&line);
|
||||
let section_name = leading_words(&line);
|
||||
@@ -162,7 +160,8 @@ impl<'a> SectionContexts<'a> {
|
||||
if is_docstring_section(
|
||||
&line,
|
||||
section_name_range,
|
||||
previous_line.unwrap_or_default(),
|
||||
previous_line.as_ref(),
|
||||
lines.peek(),
|
||||
) {
|
||||
if let Some(mut last) = last.take() {
|
||||
last.range = TextRange::new(last.range.start(), line.start());
|
||||
@@ -178,7 +177,7 @@ impl<'a> SectionContexts<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
previous_line = Some(line.as_str());
|
||||
previous_line = Some(line);
|
||||
}
|
||||
|
||||
if let Some(mut last) = last.take() {
|
||||
@@ -388,7 +387,13 @@ fn suspected_as_section(line: &str, style: SectionStyle) -> Option<SectionKind>
|
||||
}
|
||||
|
||||
/// Check if the suspected context is really a section header.
|
||||
fn is_docstring_section(line: &str, section_name_range: TextRange, previous_lines: &str) -> bool {
|
||||
fn is_docstring_section(
|
||||
line: &Line,
|
||||
section_name_range: TextRange,
|
||||
previous_line: Option<&Line>,
|
||||
next_line: Option<&Line>,
|
||||
) -> bool {
|
||||
// Determine whether the current line looks like a section header, e.g., "Args:".
|
||||
let section_name_suffix = line[usize::from(section_name_range.end())..].trim();
|
||||
let this_looks_like_a_section_name =
|
||||
section_name_suffix == ":" || section_name_suffix.is_empty();
|
||||
@@ -396,13 +401,29 @@ fn is_docstring_section(line: &str, section_name_range: TextRange, previous_line
|
||||
return false;
|
||||
}
|
||||
|
||||
let prev_line = previous_lines.trim();
|
||||
let prev_line_ends_with_punctuation = [',', ';', '.', '-', '\\', '/', ']', '}', ')']
|
||||
.into_iter()
|
||||
.any(|char| prev_line.ends_with(char));
|
||||
let prev_line_looks_like_end_of_paragraph =
|
||||
prev_line_ends_with_punctuation || prev_line.is_empty();
|
||||
if !prev_line_looks_like_end_of_paragraph {
|
||||
// Determine whether the next line is an underline, e.g., "-----".
|
||||
let next_line_is_underline = next_line.map_or(false, |next_line| {
|
||||
let next_line = next_line.trim();
|
||||
if next_line.is_empty() {
|
||||
false
|
||||
} else {
|
||||
let next_line_is_underline = next_line.chars().all(|char| matches!(char, '-' | '='));
|
||||
next_line_is_underline
|
||||
}
|
||||
});
|
||||
if next_line_is_underline {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Determine whether the previous line looks like the end of a paragraph.
|
||||
let previous_line_looks_like_end_of_paragraph = previous_line.map_or(true, |previous_line| {
|
||||
let previous_line = previous_line.trim();
|
||||
let previous_line_ends_with_punctuation = [',', ';', '.', '-', '\\', '/', ']', '}', ')']
|
||||
.into_iter()
|
||||
.any(|char| previous_line.ends_with(char));
|
||||
previous_line_ends_with_punctuation || previous_line.is_empty()
|
||||
});
|
||||
if !previous_line_looks_like_end_of_paragraph {
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
@@ -51,6 +51,7 @@ mod tests {
|
||||
#[test_case(Rule::EmptyDocstring, Path::new("D.py"))]
|
||||
#[test_case(Rule::EmptyDocstringSection, Path::new("sections.py"))]
|
||||
#[test_case(Rule::NonImperativeMood, Path::new("D401.py"))]
|
||||
#[test_case(Rule::NoBlankLineAfterSection, Path::new("D410.py"))]
|
||||
#[test_case(Rule::OneBlankLineAfterClass, Path::new("D.py"))]
|
||||
#[test_case(Rule::OneBlankLineBeforeClass, Path::new("D.py"))]
|
||||
#[test_case(Rule::UndocumentedPublicClass, Path::new("D.py"))]
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
---
|
||||
source: crates/ruff/src/rules/pydocstyle/mod.rs
|
||||
---
|
||||
D410.py:2:5: D410 [*] Missing blank line after section ("Parameters")
|
||||
|
|
||||
1 | def f(a: int, b: int) -> int:
|
||||
2 | """Showcase function.
|
||||
| _____^
|
||||
3 | |
|
||||
4 | | Parameters
|
||||
5 | | ----------
|
||||
6 | | a : int
|
||||
7 | | _description_
|
||||
8 | | b : int
|
||||
9 | | _description_
|
||||
10 | | Returns
|
||||
11 | | -------
|
||||
12 | | int
|
||||
13 | | _description
|
||||
14 | | """
|
||||
| |_______^ D410
|
||||
15 | return b - a
|
||||
|
|
||||
= help: Add blank line after "Parameters"
|
||||
|
||||
ℹ Fix
|
||||
7 7 | _description_
|
||||
8 8 | b : int
|
||||
9 9 | _description_
|
||||
10 |+
|
||||
10 11 | Returns
|
||||
11 12 | -------
|
||||
12 13 | int
|
||||
|
||||
D410.py:19:5: D410 [*] Missing blank line after section ("Parameters")
|
||||
|
|
||||
18 | def f() -> int:
|
||||
19 | """Showcase function.
|
||||
| _____^
|
||||
20 | |
|
||||
21 | | Parameters
|
||||
22 | | ----------
|
||||
23 | | Returns
|
||||
24 | | -------
|
||||
25 | | """
|
||||
| |_______^ D410
|
||||
|
|
||||
= help: Add blank line after "Parameters"
|
||||
|
||||
ℹ Fix
|
||||
20 20 |
|
||||
21 21 | Parameters
|
||||
22 22 | ----------
|
||||
23 |+
|
||||
23 24 | Returns
|
||||
24 25 | -------
|
||||
25 26 | """
|
||||
|
||||
|
||||
Reference in New Issue
Block a user