From b32d1e8d78a755de254785ad6bbfefa37f8fcbb2 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 2 Jul 2023 20:29:45 -0400 Subject: [PATCH] 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. --- .../test/fixtures/pydocstyle/D410.py | 25 ++++++++ crates/ruff/src/checkers/ast/mod.rs | 1 + crates/ruff/src/docstrings/sections.rs | 57 ++++++++++++------ crates/ruff/src/rules/pydocstyle/mod.rs | 1 + ...ules__pydocstyle__tests__D410_D410.py.snap | 59 +++++++++++++++++++ 5 files changed, 125 insertions(+), 18 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/pydocstyle/D410.py create mode 100644 crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D410_D410.py.snap diff --git a/crates/ruff/resources/test/fixtures/pydocstyle/D410.py b/crates/ruff/resources/test/fixtures/pydocstyle/D410.py new file mode 100644 index 0000000000..b9aec80568 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pydocstyle/D410.py @@ -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 + ------- + """ diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index bd408fb22a..d722ee2a77 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -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, diff --git a/crates/ruff/src/docstrings/sections.rs b/crates/ruff/src/docstrings/sections.rs index 48f38e606d..6bdca985f4 100644 --- a/crates/ruff/src/docstrings/sections.rs +++ b/crates/ruff/src/docstrings/sections.rs @@ -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 = 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 } /// 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; } diff --git a/crates/ruff/src/rules/pydocstyle/mod.rs b/crates/ruff/src/rules/pydocstyle/mod.rs index ca0035e418..41c6a1fc6f 100644 --- a/crates/ruff/src/rules/pydocstyle/mod.rs +++ b/crates/ruff/src/rules/pydocstyle/mod.rs @@ -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"))] diff --git a/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D410_D410.py.snap b/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D410_D410.py.snap new file mode 100644 index 0000000000..0ce1725147 --- /dev/null +++ b/crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D410_D410.py.snap @@ -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 | """ + +