From 436aeed20a1186afcdcaad5d5e55e3c054540930 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 5 Dec 2022 20:24:56 -0500 Subject: [PATCH] Implement autofix for D400 and D415 (#1094) --- README.md | 4 +- resources/test/fixtures/pydocstyle/D400.py | 73 +++++++ src/checks.rs | 2 + src/linter.rs | 2 +- src/pydocstyle/helpers.rs | 43 ++++ src/pydocstyle/mod.rs | 4 +- src/pydocstyle/plugins.rs | 116 ++++++----- .../ruff__pydocstyle__tests__D400_D.py.snap | 126 ++++++++++-- ...ruff__pydocstyle__tests__D400_D400.py.snap | 185 ++++++++++++++++++ 9 files changed, 486 insertions(+), 69 deletions(-) create mode 100644 resources/test/fixtures/pydocstyle/D400.py create mode 100644 src/pydocstyle/helpers.rs create mode 100644 src/pydocstyle/snapshots/ruff__pydocstyle__tests__D400_D400.py.snap diff --git a/README.md b/README.md index 067db65474..5a59653800 100644 --- a/README.md +++ b/README.md @@ -507,7 +507,7 @@ For more, see [pydocstyle](https://pypi.org/project/pydocstyle/6.1.1/) on PyPI. | D214 | SectionNotOverIndented | Section is over-indented ("Returns") | 🛠 | | D215 | SectionUnderlineNotOverIndented | Section underline is over-indented ("Returns") | 🛠 | | D300 | UsesTripleQuotes | Use """triple double quotes""" | | -| D400 | EndsInPeriod | First line should end with a period | | +| D400 | EndsInPeriod | First line should end with a period | 🛠 | | D402 | NoSignature | First line should not be the function's signature | | | D403 | FirstLineCapitalized | First word of the first line should be properly capitalized | | | D404 | NoThisPrefix | First word of the docstring should not be "This" | | @@ -521,7 +521,7 @@ For more, see [pydocstyle](https://pypi.org/project/pydocstyle/6.1.1/) on PyPI. | D412 | NoBlankLinesBetweenHeaderAndContent | No blank lines allowed between a section header and its content ("Returns") | 🛠 | | D413 | BlankLineAfterLastSection | Missing blank line after last section ("Returns") | 🛠 | | D414 | NonEmptySection | Section has no content ("Returns") | | -| D415 | EndsInPunctuation | First line should end with a period, question mark, or exclamation point | | +| D415 | EndsInPunctuation | First line should end with a period, question mark, or exclamation point | 🛠 | | D416 | SectionNameEndsInColon | Section name should end with a colon ("Returns") | 🛠 | | D417 | DocumentAllArguments | Missing argument descriptions in the docstring: `x`, `y` | | | D418 | SkipDocstring | Function decorated with `@overload` shouldn't contain a docstring | | diff --git a/resources/test/fixtures/pydocstyle/D400.py b/resources/test/fixtures/pydocstyle/D400.py new file mode 100644 index 0000000000..aff12aac01 --- /dev/null +++ b/resources/test/fixtures/pydocstyle/D400.py @@ -0,0 +1,73 @@ +def f(): + "Here's a line without a period" + ... + + +def f(): + """Here's a line without a period""" + ... + + +def f(): + """ + Here's a line without a period, + but here's the next line + """ + ... + + +def f(): + """Here's a line without a period""" + ... + + +def f(): + """ + Here's a line without a period, + but here's the next line""" + ... + + +def f(): + """ + Here's a line without a period, + but here's the next line with trailing space """ + ... + + + +def f(): + r"Here's a line without a period" + ... + + +def f(): + r"""Here's a line without a period""" + ... + + +def f(): + r""" + Here's a line without a period, + but here's the next line + """ + ... + + +def f(): + r"""Here's a line without a period""" + ... + + +def f(): + r""" + Here's a line without a period, + but here's the next line""" + ... + + +def f(): + r""" + Here's a line without a period, + but here's the next line with trailing space """ + ... diff --git a/src/checks.rs b/src/checks.rs index 1bdf14113f..58e5fa8bad 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -2497,6 +2497,8 @@ impl CheckKind { | CheckKind::DoNotAssertFalse | CheckKind::DoNotAssignLambda | CheckKind::DuplicateHandlerException(..) + | CheckKind::EndsInPeriod + | CheckKind::EndsInPunctuation | CheckKind::GetAttrWithConstant | CheckKind::ImplicitReturn | CheckKind::ImplicitReturnValue diff --git a/src/linter.rs b/src/linter.rs index 2c4527e543..51236c113d 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -133,7 +133,7 @@ pub(crate) fn check_path( Ok(checks) } -const MAX_ITERATIONS: usize = 100; +const MAX_ITERATIONS: usize = 1; /// Lint the source code at the given `Path`. pub fn lint_path( diff --git a/src/pydocstyle/helpers.rs b/src/pydocstyle/helpers.rs new file mode 100644 index 0000000000..f8c2b980f3 --- /dev/null +++ b/src/pydocstyle/helpers.rs @@ -0,0 +1,43 @@ +use rustpython_ast::Expr; + +use crate::ast::types::Range; +use crate::docstrings::constants; +use crate::SourceCodeLocator; + +/// Return the leading quote string for a docstring (e.g., `"""`). +pub fn leading_quote<'a>(docstring: &Expr, locator: &'a SourceCodeLocator) -> Option<&'a str> { + if let Some(first_line) = locator + .slice_source_code_range(&Range::from_located(docstring)) + .lines() + .next() + .map(str::to_lowercase) + { + for pattern in constants::TRIPLE_QUOTE_PREFIXES + .iter() + .chain(constants::SINGLE_QUOTE_PREFIXES) + { + if first_line.starts_with(pattern) { + return Some(pattern); + } + } + } + None +} + +/// Return the index of the first logical line in a string. +pub fn logical_line(content: &str) -> Option { + // Find the first logical line. + let mut logical_line = None; + for (i, line) in content.lines().enumerate() { + if line.trim().is_empty() { + // Empty line. If this is the line _after_ the first logical line, stop. + if logical_line.is_some() { + break; + } + } else { + // Non-empty line. Store the index. + logical_line = Some(i); + } + } + logical_line +} diff --git a/src/pydocstyle/mod.rs b/src/pydocstyle/mod.rs index e7dc86fb73..972c2654b5 100644 --- a/src/pydocstyle/mod.rs +++ b/src/pydocstyle/mod.rs @@ -1,3 +1,4 @@ +mod helpers; pub mod plugins; #[cfg(test)] @@ -36,7 +37,8 @@ mod tests { #[test_case(CheckCode::D214, Path::new("sections.py"); "D214")] #[test_case(CheckCode::D215, Path::new("sections.py"); "D215")] #[test_case(CheckCode::D300, Path::new("D.py"); "D300")] - #[test_case(CheckCode::D400, Path::new("D.py"); "D400")] + #[test_case(CheckCode::D400, Path::new("D.py"); "D400_0")] + #[test_case(CheckCode::D400, Path::new("D400.py"); "D400_1")] #[test_case(CheckCode::D402, Path::new("D.py"); "D402")] #[test_case(CheckCode::D403, Path::new("D.py"); "D403")] #[test_case(CheckCode::D404, Path::new("D.py"); "D404")] diff --git a/src/pydocstyle/plugins.rs b/src/pydocstyle/plugins.rs index 394a05f774..67deefb816 100644 --- a/src/pydocstyle/plugins.rs +++ b/src/pydocstyle/plugins.rs @@ -16,6 +16,7 @@ use crate::docstrings::constants; use crate::docstrings::definition::{Definition, DefinitionKind}; use crate::docstrings::sections::{section_contexts, SectionContext}; use crate::docstrings::styles::SectionStyle; +use crate::pydocstyle::helpers::{leading_quote, logical_line}; use crate::visibility::{is_init, is_magic, is_overload, is_override, is_staticmethod, Visibility}; /// D100, D101, D102, D103, D104, D105, D106, D107 @@ -621,38 +622,22 @@ pub fn no_surrounding_whitespace(checker: &mut Checker, definition: &Definition) Range::from_located(docstring), ); if checker.patch(check.kind.code()) { - if let Some(first_line) = checker - .locator - .slice_source_code_range(&Range::from_located(docstring)) - .lines() - .next() - .map(str::to_lowercase) - { - for pattern in constants::TRIPLE_QUOTE_PREFIXES - .iter() - .chain(constants::SINGLE_QUOTE_PREFIXES) - { - if first_line.starts_with(pattern) { - if let Some(quote) = pattern.chars().last() { - // If removing whitespace would lead to an invalid string of quote - // characters, avoid applying the fix. - if !trimmed.ends_with(quote) { - check.amend(Fix::replacement( - trimmed.to_string(), - Location::new( - docstring.location.row(), - docstring.location.column() + pattern.len(), - ), - Location::new( - docstring.location.row(), - docstring.location.column() - + pattern.len() - + line.chars().count(), - ), - )); - } - } - break; + if let Some(pattern) = leading_quote(docstring, checker.locator) { + if let Some(quote) = pattern.chars().last() { + // If removing whitespace would lead to an invalid string of quote + // characters, avoid applying the fix. + if !trimmed.ends_with(quote) { + check.amend(Fix::replacement( + trimmed.to_string(), + Location::new( + docstring.location.row(), + docstring.location.column() + pattern.len(), + ), + Location::new( + docstring.location.row(), + docstring.location.column() + pattern.len() + line.chars().count(), + ), + )); } } } @@ -751,16 +736,30 @@ pub fn ends_with_period(checker: &mut Checker, definition: &Definition) { } = &docstring.node else { return; }; - let Some(string) = string.trim().lines().next() else { - return; - }; - if string.ends_with('.') { - return; - }; - checker.add_check(Check::new( - CheckKind::EndsInPeriod, - Range::from_located(docstring), - )); + if let Some(index) = logical_line(string) { + let line = string.lines().nth(index).unwrap(); + let trimmed = line.trim_end(); + if !trimmed.ends_with('.') { + let mut check = Check::new(CheckKind::EndsInPeriod, Range::from_located(docstring)); + // Best-effort autofix: avoid adding a period after other punctuation marks. + if checker.patch(&CheckCode::D400) && !trimmed.ends_with(':') && !trimmed.ends_with(';') + { + if let Some((row, column)) = if index == 0 { + leading_quote(docstring, checker.locator).map(|pattern| { + ( + docstring.location.row(), + docstring.location.column() + pattern.len() + trimmed.chars().count(), + ) + }) + } else { + Some((docstring.location.row() + index, trimmed.chars().count())) + } { + check.amend(Fix::insertion(".".to_string(), Location::new(row, column))); + } + } + checker.add_check(check); + }; + } } /// D402 @@ -878,16 +877,31 @@ pub fn ends_with_punctuation(checker: &mut Checker, definition: &Definition) { } = &docstring.node else { return }; - let Some(string) = string.trim().lines().next() else { - return - }; - if string.ends_with('.') || string.ends_with('!') || string.ends_with('?') { - return; + if let Some(index) = logical_line(string) { + let line = string.lines().nth(index).unwrap(); + let trimmed = line.trim_end(); + if !(trimmed.ends_with('.') || trimmed.ends_with('!') || trimmed.ends_with('?')) { + let mut check = + Check::new(CheckKind::EndsInPunctuation, Range::from_located(docstring)); + // Best-effort autofix: avoid adding a period after other punctuation marks. + if checker.patch(&CheckCode::D415) && !trimmed.ends_with(':') && !trimmed.ends_with(';') + { + if let Some((row, column)) = if index == 0 { + leading_quote(docstring, checker.locator).map(|pattern| { + ( + docstring.location.row(), + docstring.location.column() + pattern.len() + trimmed.chars().count(), + ) + }) + } else { + Some((docstring.location.row() + index, trimmed.chars().count())) + } { + check.amend(Fix::insertion(".".to_string(), Location::new(row, column))); + } + } + checker.add_check(check); + }; } - checker.add_check(Check::new( - CheckKind::EndsInPunctuation, - Range::from_located(docstring), - )); } /// D418 diff --git a/src/pydocstyle/snapshots/ruff__pydocstyle__tests__D400_D.py.snap b/src/pydocstyle/snapshots/ruff__pydocstyle__tests__D400_D.py.snap index af29dfeb26..fde719cf37 100644 --- a/src/pydocstyle/snapshots/ruff__pydocstyle__tests__D400_D.py.snap +++ b/src/pydocstyle/snapshots/ruff__pydocstyle__tests__D400_D.py.snap @@ -9,7 +9,14 @@ expression: checks end_location: row: 355 column: 17 - fix: ~ + fix: + content: "." + location: + row: 355 + column: 14 + end_location: + row: 355 + column: 14 - kind: EndsInPeriod location: row: 406 @@ -17,7 +24,14 @@ expression: checks end_location: row: 406 column: 39 - fix: ~ + fix: + content: "." + location: + row: 406 + column: 36 + end_location: + row: 406 + column: 36 - kind: EndsInPeriod location: row: 410 @@ -25,7 +39,14 @@ expression: checks end_location: row: 410 column: 24 - fix: ~ + fix: + content: "." + location: + row: 410 + column: 21 + end_location: + row: 410 + column: 21 - kind: EndsInPeriod location: row: 416 @@ -33,7 +54,14 @@ expression: checks end_location: row: 416 column: 24 - fix: ~ + fix: + content: "." + location: + row: 416 + column: 21 + end_location: + row: 416 + column: 21 - kind: EndsInPeriod location: row: 422 @@ -41,7 +69,14 @@ expression: checks end_location: row: 422 column: 49 - fix: ~ + fix: + content: "." + location: + row: 422 + column: 46 + end_location: + row: 422 + column: 46 - kind: EndsInPeriod location: row: 429 @@ -49,7 +84,14 @@ expression: checks end_location: row: 429 column: 63 - fix: ~ + fix: + content: "." + location: + row: 429 + column: 60 + end_location: + row: 429 + column: 60 - kind: EndsInPeriod location: row: 470 @@ -57,7 +99,14 @@ expression: checks end_location: row: 470 column: 24 - fix: ~ + fix: + content: "." + location: + row: 470 + column: 21 + end_location: + row: 470 + column: 21 - kind: EndsInPeriod location: row: 475 @@ -65,7 +114,14 @@ expression: checks end_location: row: 475 column: 24 - fix: ~ + fix: + content: "." + location: + row: 475 + column: 21 + end_location: + row: 475 + column: 21 - kind: EndsInPeriod location: row: 480 @@ -73,7 +129,14 @@ expression: checks end_location: row: 480 column: 24 - fix: ~ + fix: + content: "." + location: + row: 480 + column: 21 + end_location: + row: 480 + column: 21 - kind: EndsInPeriod location: row: 487 @@ -81,7 +144,14 @@ expression: checks end_location: row: 487 column: 24 - fix: ~ + fix: + content: "." + location: + row: 487 + column: 21 + end_location: + row: 487 + column: 21 - kind: EndsInPeriod location: row: 509 @@ -89,7 +159,14 @@ expression: checks end_location: row: 509 column: 34 - fix: ~ + fix: + content: "." + location: + row: 509 + column: 31 + end_location: + row: 509 + column: 31 - kind: EndsInPeriod location: row: 514 @@ -97,7 +174,14 @@ expression: checks end_location: row: 514 column: 33 - fix: ~ + fix: + content: "." + location: + row: 514 + column: 30 + end_location: + row: 514 + column: 30 - kind: EndsInPeriod location: row: 520 @@ -105,7 +189,14 @@ expression: checks end_location: row: 520 column: 32 - fix: ~ + fix: + content: "." + location: + row: 520 + column: 29 + end_location: + row: 520 + column: 29 - kind: EndsInPeriod location: row: 581 @@ -113,5 +204,12 @@ expression: checks end_location: row: 581 column: 51 - fix: ~ + fix: + content: "." + location: + row: 581 + column: 47 + end_location: + row: 581 + column: 47 diff --git a/src/pydocstyle/snapshots/ruff__pydocstyle__tests__D400_D400.py.snap b/src/pydocstyle/snapshots/ruff__pydocstyle__tests__D400_D400.py.snap new file mode 100644 index 0000000000..127b4dbaff --- /dev/null +++ b/src/pydocstyle/snapshots/ruff__pydocstyle__tests__D400_D400.py.snap @@ -0,0 +1,185 @@ +--- +source: src/pydocstyle/mod.rs +expression: checks +--- +- kind: EndsInPeriod + location: + row: 2 + column: 4 + end_location: + row: 2 + column: 36 + fix: + content: "." + location: + row: 2 + column: 35 + end_location: + row: 2 + column: 35 +- kind: EndsInPeriod + location: + row: 7 + column: 4 + end_location: + row: 7 + column: 40 + fix: + content: "." + location: + row: 7 + column: 37 + end_location: + row: 7 + column: 37 +- kind: EndsInPeriod + location: + row: 12 + column: 4 + end_location: + row: 15 + column: 7 + fix: + content: "." + location: + row: 14 + column: 28 + end_location: + row: 14 + column: 28 +- kind: EndsInPeriod + location: + row: 20 + column: 4 + end_location: + row: 20 + column: 40 + fix: + content: "." + location: + row: 20 + column: 37 + end_location: + row: 20 + column: 37 +- kind: EndsInPeriod + location: + row: 25 + column: 4 + end_location: + row: 27 + column: 31 + fix: + content: "." + location: + row: 27 + column: 28 + end_location: + row: 27 + column: 28 +- kind: EndsInPeriod + location: + row: 32 + column: 4 + end_location: + row: 34 + column: 52 + fix: + content: "." + location: + row: 34 + column: 48 + end_location: + row: 34 + column: 48 +- kind: EndsInPeriod + location: + row: 40 + column: 4 + end_location: + row: 40 + column: 37 + fix: + content: "." + location: + row: 40 + column: 36 + end_location: + row: 40 + column: 36 +- kind: EndsInPeriod + location: + row: 45 + column: 4 + end_location: + row: 45 + column: 41 + fix: + content: "." + location: + row: 45 + column: 38 + end_location: + row: 45 + column: 38 +- kind: EndsInPeriod + location: + row: 50 + column: 4 + end_location: + row: 53 + column: 7 + fix: + content: "." + location: + row: 52 + column: 28 + end_location: + row: 52 + column: 28 +- kind: EndsInPeriod + location: + row: 58 + column: 4 + end_location: + row: 58 + column: 41 + fix: + content: "." + location: + row: 58 + column: 38 + end_location: + row: 58 + column: 38 +- kind: EndsInPeriod + location: + row: 63 + column: 4 + end_location: + row: 65 + column: 31 + fix: + content: "." + location: + row: 65 + column: 28 + end_location: + row: 65 + column: 28 +- kind: EndsInPeriod + location: + row: 70 + column: 4 + end_location: + row: 72 + column: 52 + fix: + content: "." + location: + row: 72 + column: 48 + end_location: + row: 72 + column: 48 +