From e83388dceac4281289120c5d9e4089d1b33956d9 Mon Sep 17 00:00:00 2001 From: ukyen Date: Thu, 26 Sep 2024 13:53:21 +0100 Subject: [PATCH] Don't raise `D208` when last line is non-empty (#13372) Co-authored-by: Micha Reiser --- .../test/fixtures/pydocstyle/D208.py | 33 +++++ crates/ruff_linter/src/lib.rs | 39 ++++++ .../src/rules/pydocstyle/rules/indent.rs | 88 +++++++++---- ...ules__pydocstyle__tests__D208_D208.py.snap | 120 ++++++++++++++++++ 4 files changed, 252 insertions(+), 28 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py b/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py index 4e99cf4b7fdf53..6718f461c0830d 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py +++ b/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py @@ -14,3 +14,36 @@ def memory_test(): """ 　　　参数含义：precision:精确到小数点后几位 """ + + + +class Platform: + """Over indented last line + Args: + Returns: + """ + + +class Platform: + """All lines are over indented including the last containing the closing quotes + Args: + Returns: + """ + +class Platform: + """All lines are over indented including the last + Args: + Returns""" + +# OK: This doesn't get flagged because it is accepted when the closing quotes are on a separate line (see next test). Raises D209 +class Platform: + """Over indented last line with content + Args: + Some content on the last line""" + +# OK: +class Platform: + """Over indented last line with content + Args: + Some content on the last line + """ diff --git a/crates/ruff_linter/src/lib.rs b/crates/ruff_linter/src/lib.rs index 4fb40d4a8d215e..20e0638cb30bda 100644 --- a/crates/ruff_linter/src/lib.rs +++ b/crates/ruff_linter/src/lib.rs @@ -46,3 +46,42 @@ pub mod upstream_categories; pub mod test; pub const RUFF_PKG_VERSION: &str = env!("CARGO_PKG_VERSION"); + +#[cfg(test)] +mod tests { + use std::path::Path; + + use ruff_python_ast::PySourceType; + + use crate::codes::Rule; + use crate::settings::LinterSettings; + use crate::source_kind::SourceKind; + use crate::test::{print_messages, test_contents}; + + /// Test for ad-hoc debugging. + #[test] + #[ignore] + fn linter_quick_test() { + let code = r#"class Platform: + """ Remove sampler + Args: +     Returns: + """ +"#; + let source_type = PySourceType::Python; + let rule = Rule::OverIndentation; + + let source_kind = SourceKind::from_source_code(code.to_string(), source_type) + .expect("Source code should be valid") + .expect("Notebook to contain python code"); + + let (diagnostics, fixed) = test_contents( + &source_kind, + Path::new("ruff_linter/rules/quick_test"), + &LinterSettings::for_rule(rule), + ); + + assert_eq!(print_messages(&diagnostics), ""); + assert_eq!(fixed.source_code(), code); + } +} diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs index aaec3c4e83e2d7..63c43b770bdf3a 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs @@ -2,7 +2,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Violation}; use ruff_diagnostics::{Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::docstrings::{clean_space, leading_space}; -use ruff_source_file::NewlineWithTrailingNewline; +use ruff_source_file::{Line, NewlineWithTrailingNewline}; use ruff_text_size::{Ranged, TextSize}; use ruff_text_size::{TextLen, TextRange}; @@ -169,32 +169,49 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { let body = docstring.body(); // Split the docstring into lines. - let lines: Vec<_> = NewlineWithTrailingNewline::with_offset(&body, body.start()).collect(); - if lines.len() <= 1 { + let mut lines = NewlineWithTrailingNewline::with_offset(&body, body.start()).peekable(); + + // The current line being processed + let mut current: Option = lines.next(); + + if lines.peek().is_none() { return; } let mut has_seen_tab = docstring.indentation.contains('\t'); - let mut is_over_indented = true; + let docstring_indent_size = docstring.indentation.chars().count(); + + // Lines, other than the last, that are over indented. let mut over_indented_lines = vec![]; - let mut over_indented_size = usize::MAX; + // The smallest over indent that all docstring lines have in common. None if any line isn't over indented. + let mut smallest_over_indent_size = Some(usize::MAX); + // The last processed line + let mut last = None; - let docstring_indent_size = docstring.indentation.chars().count(); - for i in 0..lines.len() { - // First lines and continuations doesn't need any indentation. - if i == 0 || lines[i - 1].ends_with('\\') { + while let Some(line) = current { + // First lines and continuations don't need any indentation. + if last.is_none() + || last + .as_deref() + .is_some_and(|last: &str| last.ends_with('\\')) + { + last = Some(line); + current = lines.next(); continue; } - let line = &lines[i]; + let is_last = lines.peek().is_none(); + // Omit empty lines, except for the last line, which is non-empty by way of // containing the closing quotation marks. let is_blank = line.trim().is_empty(); - if i < lines.len() - 1 && is_blank { + if !is_last && is_blank { + last = Some(line); + current = lines.next(); continue; } - let line_indent = leading_space(line); + let line_indent = leading_space(&line); let line_indent_size = line_indent.chars().count(); // We only report tab indentation once, so only check if we haven't seen a tab @@ -204,7 +221,7 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { if checker.enabled(Rule::UnderIndentation) { // We report under-indentation on every line. This isn't great, but enables // fix. - if (i == lines.len() - 1 || !is_blank) && line_indent_size < docstring_indent_size { + if (is_last || !is_blank) && line_indent_size < docstring_indent_size { let mut diagnostic = Diagnostic::new(UnderIndentation, TextRange::empty(line.start())); diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( @@ -215,23 +232,35 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { } } + // Only true when the last line is indentation only followed by the closing quotes. + // False when it is not the last line or the last line contains content other than the closing quotes. + // The last line only has special handling when it contains no other content. + let is_last_closing_quotes_only = is_last && is_blank; + // Like pydocstyle, we only report over-indentation if either: (1) every line // (except, optionally, the last line) is over-indented, or (2) the last line // (which contains the closing quotation marks) is // over-indented. We can't know if we've achieved that condition // until we've viewed all the lines, so for now, just track // the over-indentation status of every line. - if i < lines.len() - 1 { - if line_indent_size > docstring_indent_size { - over_indented_lines.push(line); + if !is_last_closing_quotes_only { + smallest_over_indent_size = + smallest_over_indent_size.and_then(|smallest_over_indent_size| { + let over_indent_size = line_indent_size.saturating_sub(docstring_indent_size); - // Track the _smallest_ offset we see, in terms of characters. - over_indented_size = - std::cmp::min(line_indent_size - docstring_indent_size, over_indented_size); - } else { - is_over_indented = false; - } + // `docstring_indent_size < line_indent_size` + if over_indent_size > 0 { + over_indented_lines.push(line.clone()); + // Track the _smallest_ offset we see, in terms of characters. + Some(smallest_over_indent_size.min(over_indent_size)) + } else { + None + } + }); } + + last = Some(line); + current = lines.next(); } if checker.enabled(Rule::IndentWithSpaces) { @@ -244,9 +273,9 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { if checker.enabled(Rule::OverIndentation) { // If every line (except the last) is over-indented... - if is_over_indented { + if let Some(smallest_over_indent_size) = smallest_over_indent_size { for line in over_indented_lines { - let line_indent = leading_space(line); + let line_indent = leading_space(&line); let indent = clean_space(docstring.indentation); // We report over-indentation on every line. This isn't great, but @@ -275,7 +304,7 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { .locator() .after(line.start()) .chars() - .take(docstring.indentation.chars().count() + over_indented_size) + .take(docstring_indent_size + smallest_over_indent_size) .map(TextLen::text_len) .sum::(); let range = TextRange::at(line.start(), offset); @@ -287,10 +316,13 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { } // If the last line is over-indented... - if let Some(last) = lines.last() { - let line_indent = leading_space(last); + if let Some(last) = last { + let line_indent = leading_space(&last); let line_indent_size = line_indent.chars().count(); - if line_indent_size > docstring_indent_size { + let last_line_over_indent = line_indent_size.saturating_sub(docstring_indent_size); + + let is_indent_only = line_indent.len() == last.len(); + if last_line_over_indent > 0 && is_indent_only { let mut diagnostic = Diagnostic::new(OverIndentation, TextRange::empty(last.start())); let indent = clean_space(docstring.indentation); diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap index 517dc3802e1ece..a2b5aad6540b85 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap @@ -78,4 +78,124 @@ D208.py:10:1: D208 [*] Docstring is over-indented 12 12 | 13 13 | def memory_test(): +D208.py:24:1: D208 [*] Docstring is over-indented + | +22 | Args: +23 | Returns: +24 | """ + | D208 + | + = help: Remove over-indentation + +ℹ Safe fix +21 21 | """Over indented last line +22 22 | Args: +23 23 | Returns: +24 |- """ + 24 |+ """ +25 25 | +26 26 | +27 27 | class Platform: + +D208.py:29:1: D208 [*] Docstring is over-indented + | +27 | class Platform: +28 | """All lines are over indented including the last containing the closing quotes +29 | Args: + | D208 +30 | Returns: +31 | """ + | + = help: Remove over-indentation + +ℹ Safe fix +26 26 | +27 27 | class Platform: +28 28 | """All lines are over indented including the last containing the closing quotes +29 |- Args: + 29 |+ Args: +30 30 | Returns: +31 31 | """ +32 32 | + +D208.py:30:1: D208 [*] Docstring is over-indented + | +28 | """All lines are over indented including the last containing the closing quotes +29 | Args: +30 | Returns: + | D208 +31 | """ + | + = help: Remove over-indentation + +ℹ Safe fix +27 27 | class Platform: +28 28 | """All lines are over indented including the last containing the closing quotes +29 29 | Args: +30 |- Returns: + 30 |+ Returns: +31 31 | """ +32 32 | +33 33 | class Platform: +D208.py:31:1: D208 [*] Docstring is over-indented + | +29 | Args: +30 | Returns: +31 | """ + | D208 +32 | +33 | class Platform: + | + = help: Remove over-indentation + +ℹ Safe fix +28 28 | """All lines are over indented including the last containing the closing quotes +29 29 | Args: +30 30 | Returns: +31 |- """ + 31 |+ """ +32 32 | +33 33 | class Platform: +34 34 | """All lines are over indented including the last + +D208.py:35:1: D208 [*] Docstring is over-indented + | +33 | class Platform: +34 | """All lines are over indented including the last +35 | Args: + | D208 +36 | Returns""" + | + = help: Remove over-indentation + +ℹ Safe fix +32 32 | +33 33 | class Platform: +34 34 | """All lines are over indented including the last +35 |- Args: + 35 |+ Args: +36 36 | Returns""" +37 37 | +38 38 | # OK: This doesn't get flagged because it is accepted when the closing quotes are on a separate line (see next test). Raises D209 + +D208.py:36:1: D208 [*] Docstring is over-indented + | +34 | """All lines are over indented including the last +35 | Args: +36 | Returns""" + | D208 +37 | +38 | # OK: This doesn't get flagged because it is accepted when the closing quotes are on a separate line (see next test). Raises D209 + | + = help: Remove over-indentation + +ℹ Safe fix +33 33 | class Platform: +34 34 | """All lines are over indented including the last +35 35 | Args: +36 |- Returns""" + 36 |+ Returns""" +37 37 | +38 38 | # OK: This doesn't get flagged because it is accepted when the closing quotes are on a separate line (see next test). Raises D209 +39 39 | class Platform: