Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ignore same-line docstrings for lines-before and lines-after rules #6344

Merged
merged 1 commit into from
Aug 4, 2023
Merged

Ignore same-line docstrings for lines-before and lines-after rules #6344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/ruff/resources/test/fixtures/pydocstyle/D.py
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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."""


Loading