Skip to content

Commit

Permalink
Ignore same-line docstrings for lines-before and lines-after rules (#…
Browse files Browse the repository at this point in the history 
…6344)

These rules assume that the docstring is on its own line. pydocstyle
treats them inconsistently, so I'm just going to disable them in this
case.

Closes #6329.
  • Loading branch information
@charliermarsh
charliermarsh authored Aug 4, 2023
1 parent 08dd87e commit fa5c9cc
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 36 deletions.
5 changes: 5 additions & 0 deletions crates/ruff/resources/test/fixtures/pydocstyle/D.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -634,3 +634,8 @@ def starts_with_this():
@expect('D404: First word of the docstring should not be "This"')
def starts_with_space_then_this():
""" This is a docstring that starts with a space.""" # noqa: D210


class SameLine: """This is a docstring on the same line"""

def same_line(): """This is a docstring on the same line"""
62 changes: 26 additions & 36 deletions crates/ruff/src/rules/pydocstyle/rules/blank_before_after_class.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ use crate::registry::{AsRule, Rule};
///
/// [D211]: https://beta.ruff.rs/docs/rules/blank-line-before-class
#[violation]
pub struct OneBlankLineBeforeClass {
lines: usize,
}
pub struct OneBlankLineBeforeClass;

impl AlwaysAutofixableViolation for OneBlankLineBeforeClass {
#[derive_message_formats]
Expand Down Expand Up @@ -97,9 +95,7 @@ impl AlwaysAutofixableViolation for OneBlankLineBeforeClass {
///
/// [PEP 257]: https://peps.python.org/pep-0257/
#[violation]
pub struct OneBlankLineAfterClass {
lines: usize,
}
pub struct OneBlankLineAfterClass;

impl AlwaysAutofixableViolation for OneBlankLineAfterClass {
#[derive_message_formats]
Expand Down Expand Up @@ -144,9 +140,7 @@ impl AlwaysAutofixableViolation for OneBlankLineAfterClass {
///
/// [D203]: https://beta.ruff.rs/docs/rules/one-blank-line-before-class
#[violation]
pub struct BlankLineBeforeClass {
lines: usize,
}
pub struct BlankLineBeforeClass;

impl AlwaysAutofixableViolation for BlankLineBeforeClass {
#[derive_message_formats]
Expand All @@ -170,14 +164,24 @@ pub(crate) fn blank_before_after_class(checker: &mut Checker, docstring: &Docstr
return;
};

// Special-case: the docstring is on the same line as the class. For example:
// ```python
// class PhotoMetadata: """Metadata about a photo."""
// ```
let between_range = TextRange::new(stmt.start(), docstring.start());
if !checker.locator().contains_line_break(between_range) {
return;
}

if checker.enabled(Rule::OneBlankLineBeforeClass) || checker.enabled(Rule::BlankLineBeforeClass)
{
let before = checker
.locator()
.slice(TextRange::new(stmt.start(), docstring.start()));
let mut lines = UniversalNewlineIterator::with_offset(
checker.locator().slice(between_range),
between_range.start(),
)
.rev();

let mut blank_lines_before = 0usize;
let mut lines = UniversalNewlineIterator::with_offset(before, stmt.start()).rev();
let mut blank_lines_start = lines.next().map(|line| line.start()).unwrap_or_default();

for line in lines {
Expand All @@ -191,12 +195,7 @@ pub(crate) fn blank_before_after_class(checker: &mut Checker, docstring: &Docstr

if checker.enabled(Rule::BlankLineBeforeClass) {
if blank_lines_before != 0 {
let mut diagnostic = Diagnostic::new(
BlankLineBeforeClass {
lines: blank_lines_before,
},
docstring.range(),
);
let mut diagnostic = Diagnostic::new(BlankLineBeforeClass, docstring.range());
if checker.patch(diagnostic.kind.rule()) {
// Delete the blank line before the class.
diagnostic.set_fix(Fix::automatic(Edit::deletion(
Expand All @@ -209,12 +208,7 @@ pub(crate) fn blank_before_after_class(checker: &mut Checker, docstring: &Docstr
}
if checker.enabled(Rule::OneBlankLineBeforeClass) {
if blank_lines_before != 1 {
let mut diagnostic = Diagnostic::new(
OneBlankLineBeforeClass {
lines: blank_lines_before,
},
docstring.range(),
);
let mut diagnostic = Diagnostic::new(OneBlankLineBeforeClass, docstring.range());
if checker.patch(diagnostic.kind.rule()) {
// Insert one blank line before the class.
diagnostic.set_fix(Fix::automatic(Edit::replacement(
Expand All @@ -229,9 +223,9 @@ pub(crate) fn blank_before_after_class(checker: &mut Checker, docstring: &Docstr
}

if checker.enabled(Rule::OneBlankLineAfterClass) {
let after = checker
.locator()
.slice(TextRange::new(docstring.end(), stmt.end()));
let after_range = TextRange::new(docstring.end(), stmt.end());

let after = checker.locator().slice(after_range);

let all_blank_after = after.universal_newlines().skip(1).all(|line| {
line.trim_whitespace().is_empty() || line.trim_whitespace_start().starts_with('#')
Expand All @@ -240,9 +234,10 @@ pub(crate) fn blank_before_after_class(checker: &mut Checker, docstring: &Docstr
return;
}

let mut blank_lines_after = 0usize;
let mut lines = UniversalNewlineIterator::with_offset(after, docstring.end());
let mut lines = UniversalNewlineIterator::with_offset(after, after_range.start());

let first_line_start = lines.next().map(|l| l.start()).unwrap_or_default();
let mut blank_lines_after = 0usize;
let mut blank_lines_end = docstring.end();

for line in lines {
Expand All @@ -255,12 +250,7 @@ pub(crate) fn blank_before_after_class(checker: &mut Checker, docstring: &Docstr
}

if blank_lines_after != 1 {
let mut diagnostic = Diagnostic::new(
OneBlankLineAfterClass {
lines: blank_lines_after,
},
docstring.range(),
);
let mut diagnostic = Diagnostic::new(OneBlankLineAfterClass, docstring.range());
if checker.patch(diagnostic.kind.rule()) {
// Insert a blank line before the class (replacing any existing lines).
diagnostic.set_fix(Fix::automatic(Edit::replacement(
Expand Down
34 changes: 34 additions & 0 deletions crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D400_D.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,4 +272,38 @@ D.py:615:5: D400 [*] First line should end with a period
617 617 | """
618 618 |

D.py:639:17: D400 [*] First line should end with a period
|
639 | class SameLine: """This is a docstring on the same line"""
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ D400
640 |
641 | def same_line(): """This is a docstring on the same line"""
|
= help: Add period

ℹ Suggested fix
636 636 | """ This is a docstring that starts with a space.""" # noqa: D210
637 637 |
638 638 |
639 |-class SameLine: """This is a docstring on the same line"""
639 |+class SameLine: """This is a docstring on the same line."""
640 640 |
641 641 | def same_line(): """This is a docstring on the same line"""

D.py:641:18: D400 [*] First line should end with a period
|
639 | class SameLine: """This is a docstring on the same line"""
640 |
641 | def same_line(): """This is a docstring on the same line"""
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ D400
|
= help: Add period

ℹ Suggested fix
638 638 |
639 639 | class SameLine: """This is a docstring on the same line"""
640 640 |
641 |-def same_line(): """This is a docstring on the same line"""
641 |+def same_line(): """This is a docstring on the same line."""


16 changes: 16 additions & 0 deletions crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D404_D.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,20 @@ D.py:636:5: D404 First word of the docstring should not be "This"
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ D404
|

D.py:639:17: D404 First word of the docstring should not be "This"
|
639 | class SameLine: """This is a docstring on the same line"""
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ D404
640 |
641 | def same_line(): """This is a docstring on the same line"""
|

D.py:641:18: D404 First word of the docstring should not be "This"
|
639 | class SameLine: """This is a docstring on the same line"""
640 |
641 | def same_line(): """This is a docstring on the same line"""
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ D404
|


34 changes: 34 additions & 0 deletions crates/ruff/src/rules/pydocstyle/snapshots/ruff__rules__pydocstyle__tests__D415_D.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,4 +254,38 @@ D.py:615:5: D415 [*] First line should end with a period, question mark, or excl
617 617 | """
618 618 |

D.py:639:17: D415 [*] First line should end with a period, question mark, or exclamation point
|
639 | class SameLine: """This is a docstring on the same line"""
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ D415
640 |
641 | def same_line(): """This is a docstring on the same line"""
|
= help: Add closing punctuation

ℹ Suggested fix
636 636 | """ This is a docstring that starts with a space.""" # noqa: D210
637 637 |
638 638 |
639 |-class SameLine: """This is a docstring on the same line"""
639 |+class SameLine: """This is a docstring on the same line."""
640 640 |
641 641 | def same_line(): """This is a docstring on the same line"""

D.py:641:18: D415 [*] First line should end with a period, question mark, or exclamation point
|
639 | class SameLine: """This is a docstring on the same line"""
640 |
641 | def same_line(): """This is a docstring on the same line"""
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ D415
|
= help: Add closing punctuation

ℹ Suggested fix
638 638 |
639 639 | class SameLine: """This is a docstring on the same line"""
640 640 |
641 |-def same_line(): """This is a docstring on the same line"""
641 |+def same_line(): """This is a docstring on the same line."""


0 comments on commit fa5c9cc

Please sign in to comment.