From 3aa7e35a4c3f04e79338db3f800f12d68441b899 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 30 May 2024 13:29:20 -0400 Subject: [PATCH] Avoid removing newlines between docstring headers and rST blocks (#11609) Given: ```python def func(): """ Example: .. code-block:: python import foo """ ``` Removing the newline after the `Example:` header breaks Sphinx rendering. See: https://github.com/astral-sh/ruff/issues/11577 --- .../test/fixtures/pydocstyle/sphinx.py | 67 +++++++++ .../ruff_linter/src/rules/pydocstyle/mod.rs | 1 + .../src/rules/pydocstyle/rules/sections.rs | 134 ++++++++++++++---- ...es__pydocstyle__tests__D412_sphinx.py.snap | 78 ++++++++++ 4 files changed, 254 insertions(+), 26 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pydocstyle/sphinx.py create mode 100644 crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D412_sphinx.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pydocstyle/sphinx.py b/crates/ruff_linter/resources/test/fixtures/pydocstyle/sphinx.py new file mode 100644 index 0000000000..6f595c2e70 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pydocstyle/sphinx.py @@ -0,0 +1,67 @@ +def func(): + """ + Example: + + .. code-block:: python + + import foo + """ + + +def func(): + """ + Example: + + + .. code-block:: python + + import foo + """ + + +def func(): + """ + Example: + + + + .. code-block:: python + + import foo + """ + + +def func(): + """ + Example + ------- + + .. code-block:: python + + import foo + """ + + +def func(): + """ + Example + ------- + + + .. code-block:: python + + import foo + """ + + +def func(): + """ + Example + ------- + + + + .. code-block:: python + + import foo + """ diff --git a/crates/ruff_linter/src/rules/pydocstyle/mod.rs b/crates/ruff_linter/src/rules/pydocstyle/mod.rs index 577df34dca..5b2d237766 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/mod.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/mod.rs @@ -46,6 +46,7 @@ mod tests { #[test_case(Rule::BlankLineBeforeClass, Path::new("D.py"))] #[test_case(Rule::NoBlankLineBeforeFunction, Path::new("D.py"))] #[test_case(Rule::BlankLinesBetweenHeaderAndContent, Path::new("sections.py"))] + #[test_case(Rule::BlankLinesBetweenHeaderAndContent, Path::new("sphinx.py"))] #[test_case(Rule::OverIndentation, Path::new("D.py"))] #[test_case(Rule::OverIndentation, Path::new("D208.py"))] #[test_case(Rule::NoSignature, Path::new("D.py"))] diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs index b381fcca2b..bbf6c2227a 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs @@ -1393,14 +1393,14 @@ fn blanks_and_section_underline( docstring: &Docstring, context: &SectionContext, ) { - let mut blank_lines_after_header = 0; + let mut num_blank_lines_after_header = 0u32; let mut blank_lines_end = context.following_range().start(); let mut following_lines = context.following_lines().peekable(); while let Some(line) = following_lines.peek() { if line.trim().is_empty() { blank_lines_end = line.full_end(); - blank_lines_after_header += 1; + num_blank_lines_after_header += 1; following_lines.next(); } else { break; @@ -1409,7 +1409,7 @@ fn blanks_and_section_underline( if let Some(non_blank_line) = following_lines.next() { if let Some(dashed_line) = find_underline(&non_blank_line, '-') { - if blank_lines_after_header > 0 { + if num_blank_lines_after_header > 0 { if checker.enabled(Rule::SectionUnderlineAfterName) { let mut diagnostic = Diagnostic::new( SectionUnderlineAfterName { @@ -1475,10 +1475,12 @@ fn blanks_and_section_underline( if let Some(line_after_dashes) = following_lines.next() { if line_after_dashes.trim().is_empty() { + let mut num_blank_lines_dashes_end = 1u32; let mut blank_lines_after_dashes_end = line_after_dashes.full_end(); while let Some(line) = following_lines.peek() { if line.trim().is_empty() { blank_lines_after_dashes_end = line.full_end(); + num_blank_lines_dashes_end += 1; following_lines.next(); } else { break; @@ -1495,18 +1497,57 @@ fn blanks_and_section_underline( )); } } else if checker.enabled(Rule::BlankLinesBetweenHeaderAndContent) { - let mut diagnostic = Diagnostic::new( - BlankLinesBetweenHeaderAndContent { - name: context.section_name().to_string(), - }, - context.section_name_range(), - ); - // Delete any blank lines between the header and content. - diagnostic.set_fix(Fix::safe_edit(Edit::deletion( - line_after_dashes.start(), - blank_lines_after_dashes_end, - ))); - checker.diagnostics.push(diagnostic); + // If the section is followed by exactly one line, and then a + // reStructuredText directive, the blank lines should be preserved, as in: + // + // ```python + // """ + // Example + // ------- + // + // .. code-block:: python + // + // import foo + // """ + // ``` + // + // Otherwise, documentation generators will not recognize the directive. + let is_sphinx = checker + .locator() + .line(blank_lines_after_dashes_end) + .trim_start() + .starts_with(".. "); + + if is_sphinx { + if num_blank_lines_dashes_end > 1 { + let mut diagnostic = Diagnostic::new( + BlankLinesBetweenHeaderAndContent { + name: context.section_name().to_string(), + }, + context.section_name_range(), + ); + // Preserve a single blank line between the header and content. + diagnostic.set_fix(Fix::safe_edit(Edit::replacement( + checker.stylist().line_ending().to_string(), + line_after_dashes.start(), + blank_lines_after_dashes_end, + ))); + checker.diagnostics.push(diagnostic); + } + } else { + let mut diagnostic = Diagnostic::new( + BlankLinesBetweenHeaderAndContent { + name: context.section_name().to_string(), + }, + context.section_name_range(), + ); + // Delete any blank lines between the header and content. + diagnostic.set_fix(Fix::safe_edit(Edit::deletion( + line_after_dashes.start(), + blank_lines_after_dashes_end, + ))); + checker.diagnostics.push(diagnostic); + } } } } else { @@ -1560,18 +1601,59 @@ fn blanks_and_section_underline( checker.diagnostics.push(diagnostic); } } - if blank_lines_after_header > 0 { + if num_blank_lines_after_header > 0 { if checker.enabled(Rule::BlankLinesBetweenHeaderAndContent) { - let mut diagnostic = Diagnostic::new( - BlankLinesBetweenHeaderAndContent { - name: context.section_name().to_string(), - }, - context.section_name_range(), - ); - let range = TextRange::new(context.following_range().start(), blank_lines_end); - // Delete any blank lines between the header and content. - diagnostic.set_fix(Fix::safe_edit(Edit::range_deletion(range))); - checker.diagnostics.push(diagnostic); + // If the section is followed by exactly one line, and then a + // reStructuredText directive, the blank lines should be preserved, as in: + // + // ```python + // """ + // Example: + // + // .. code-block:: python + // + // import foo + // """ + // ``` + // + // Otherwise, documentation generators will not recognize the directive. + let is_sphinx = checker + .locator() + .line(blank_lines_end) + .trim_start() + .starts_with(".. "); + + if is_sphinx { + if num_blank_lines_after_header > 1 { + let mut diagnostic = Diagnostic::new( + BlankLinesBetweenHeaderAndContent { + name: context.section_name().to_string(), + }, + context.section_name_range(), + ); + + // Preserve a single blank line between the header and content. + diagnostic.set_fix(Fix::safe_edit(Edit::replacement( + checker.stylist().line_ending().to_string(), + context.following_range().start(), + blank_lines_end, + ))); + checker.diagnostics.push(diagnostic); + } + } else { + let mut diagnostic = Diagnostic::new( + BlankLinesBetweenHeaderAndContent { + name: context.section_name().to_string(), + }, + context.section_name_range(), + ); + + let range = + TextRange::new(context.following_range().start(), blank_lines_end); + // Delete any blank lines between the header and content. + diagnostic.set_fix(Fix::safe_edit(Edit::range_deletion(range))); + checker.diagnostics.push(diagnostic); + } } } } diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D412_sphinx.py.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D412_sphinx.py.snap new file mode 100644 index 0000000000..8c9f7c45dc --- /dev/null +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D412_sphinx.py.snap @@ -0,0 +1,78 @@ +--- +source: crates/ruff_linter/src/rules/pydocstyle/mod.rs +--- +sphinx.py:13:5: D412 [*] No blank lines allowed between a section header and its content ("Example") + | +11 | def func(): +12 | """ +13 | Example: + | ^^^^^^^ D412 + | + = help: Remove blank line(s) + +ℹ Safe fix +12 12 | """ +13 13 | Example: +14 14 | +15 |- +16 15 | .. code-block:: python +17 16 | +18 17 | import foo + +sphinx.py:24:5: D412 [*] No blank lines allowed between a section header and its content ("Example") + | +22 | def func(): +23 | """ +24 | Example: + | ^^^^^^^ D412 + | + = help: Remove blank line(s) + +ℹ Safe fix +23 23 | """ +24 24 | Example: +25 25 | +26 |- +27 |- +28 26 | .. code-block:: python +29 27 | +30 28 | import foo + +sphinx.py:47:5: D412 [*] No blank lines allowed between a section header and its content ("Example") + | +45 | def func(): +46 | """ +47 | Example + | ^^^^^^^ D412 +48 | ------- + | + = help: Remove blank line(s) + +ℹ Safe fix +47 47 | Example +48 48 | ------- +49 49 | +50 |- +51 50 | .. code-block:: python +52 51 | +53 52 | import foo + +sphinx.py:59:5: D412 [*] No blank lines allowed between a section header and its content ("Example") + | +57 | def func(): +58 | """ +59 | Example + | ^^^^^^^ D412 +60 | ------- + | + = help: Remove blank line(s) + +ℹ Safe fix +59 59 | Example +60 60 | ------- +61 61 | +62 |- +63 |- +64 62 | .. code-block:: python +65 63 | +66 64 | import foo