From 464a094e062f292eadefb992360559f19314e329 Mon Sep 17 00:00:00 2001 From: Mike Bernard Date: Sun, 17 Dec 2023 18:42:17 -0800 Subject: [PATCH] [pylint] add PLR2044 empty_comment --- .../test/fixtures/pylint/empty_comment.py | 17 ++++ .../src/checkers/physical_lines.rs | 7 ++ crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/registry.rs | 1 + crates/ruff_linter/src/rules/pylint/mod.rs | 1 + .../src/rules/pylint/rules/empty_comment.rs | 75 +++++++++++++++++ .../ruff_linter/src/rules/pylint/rules/mod.rs | 2 + ...lint__tests__PLR2044_empty_comment.py.snap | 83 +++++++++++++++++++ ruff.schema.json | 2 + 9 files changed, 189 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/pylint/empty_comment.py create mode 100644 crates/ruff_linter/src/rules/pylint/rules/empty_comment.rs create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR2044_empty_comment.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/empty_comment.py b/crates/ruff_linter/resources/test/fixtures/pylint/empty_comment.py new file mode 100644 index 00000000000000..a6ea2d531b5bdf --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/empty_comment.py @@ -0,0 +1,17 @@ +# this line has a non-empty comment and is OK + # this line is also OK, but the three following lines are not +# + # + # + + +# this non-empty comment has trailing whitespace and is OK + + +def foo(): # this comment is OK, the one below is not + pass # + + +# the lines below have no comments and are OK +def bar(): + pass diff --git a/crates/ruff_linter/src/checkers/physical_lines.rs b/crates/ruff_linter/src/checkers/physical_lines.rs index a2b8b98b85c343..e09e5ae29f312f 100644 --- a/crates/ruff_linter/src/checkers/physical_lines.rs +++ b/crates/ruff_linter/src/checkers/physical_lines.rs @@ -32,6 +32,7 @@ pub(crate) fn check_physical_lines( let enforce_blank_line_contains_whitespace = settings.rules.enabled(Rule::BlankLineWithWhitespace); let enforce_copyright_notice = settings.rules.enabled(Rule::MissingCopyrightNotice); + let enforce_empty_comment = settings.rules.enabled(Rule::EmptyComment); let mut doc_lines_iter = doc_lines.iter().peekable(); @@ -68,6 +69,12 @@ pub(crate) fn check_physical_lines( diagnostics.push(diagnostic); } } + + if enforce_empty_comment { + if let Some(diagnostic) = pylint::rules::empty_comment(&line) { + diagnostics.push(diagnostic); + } + } } if enforce_no_newline_at_end_of_file { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index a42aa50daa1c33..c60ca333d438dc 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -264,6 +264,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "R1733") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDictIndexLookup), (Pylint, "R1736") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryListIndexLookup), (Pylint, "R2004") => (RuleGroup::Stable, rules::pylint::rules::MagicValueComparison), + (Pylint, "R2044") => (RuleGroup::Stable, rules::pylint::rules::EmptyComment), (Pylint, "R5501") => (RuleGroup::Stable, rules::pylint::rules::CollapsibleElseIf), (Pylint, "R6201") => (RuleGroup::Preview, rules::pylint::rules::LiteralMembership), #[allow(deprecated)] diff --git a/crates/ruff_linter/src/registry.rs b/crates/ruff_linter/src/registry.rs index 2e6fcc199580e4..3fba58247ab056 100644 --- a/crates/ruff_linter/src/registry.rs +++ b/crates/ruff_linter/src/registry.rs @@ -250,6 +250,7 @@ impl Rule { Rule::BidirectionalUnicode | Rule::BlankLineWithWhitespace | Rule::DocLineTooLong + | Rule::EmptyComment | Rule::LineTooLong | Rule::MissingCopyrightNotice | Rule::MissingNewlineAtEndOfFile diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index 3933e07bf9e02f..6f91a77f984701 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -40,6 +40,7 @@ mod tests { )] #[test_case(Rule::ComparisonWithItself, Path::new("comparison_with_itself.py"))] #[test_case(Rule::EqWithoutHash, Path::new("eq_without_hash.py"))] + #[test_case(Rule::EmptyComment, Path::new("empty_comment.py"))] #[test_case(Rule::ManualFromImport, Path::new("import_aliasing.py"))] #[test_case(Rule::SingleStringSlots, Path::new("single_string_slots.py"))] #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_0.py"))] diff --git a/crates/ruff_linter/src/rules/pylint/rules/empty_comment.rs b/crates/ruff_linter/src/rules/pylint/rules/empty_comment.rs new file mode 100644 index 00000000000000..f44eb20f44cd1b --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/empty_comment.rs @@ -0,0 +1,75 @@ +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_source_file::newlines::Line; +use ruff_text_size::{TextRange, TextSize}; + +/// ## What it does +/// Checks for a # symbol appearing on a line not followed by an actual comment. +/// +/// ## Why is this bad? +/// Empty comments don't provide any clarity to the code, and just add clutter. +/// Either add a comment or delete the empty comment. +/// +/// ## Example +/// ```python +/// class Foo: # +/// pass +/// ``` +/// +/// Use instead: +/// ```python +/// class Foo: +/// pass +/// ``` +/// +/// ## References +/// - [Pylint documentation](https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/empty-comment.html) +#[violation] +pub struct EmptyComment; + +impl Violation for EmptyComment { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Always; + + #[derive_message_formats] + fn message(&self) -> String { + format!("Line with empty comment") + } + + fn fix_title(&self) -> Option { + Some(format!("Delete the empty comment")) + } +} + +/// PLR2044 +pub(crate) fn empty_comment(line: &Line) -> Option { + if let Some((column, last_non_whitespace_char)) = line + .as_str() + .char_indices() + .rev() + .find(|&(_, c)| !c.is_whitespace()) + { + if last_non_whitespace_char == '#' { + #[allow(clippy::cast_possible_truncation)] + let start = TextSize::new((line.start().to_usize() + column) as u32); + let last = match line + .as_str() + .char_indices() + .rev() + .find(|&(_, c)| !c.is_whitespace() && c != '#') + { + Some((i, _second_to_last_non_whitespace_char)) => + { + #[allow(clippy::cast_possible_truncation)] + TextSize::new((line.start().to_usize() + i + 1) as u32) + } + None => line.start(), + }; + + return Some( + Diagnostic::new(EmptyComment, TextRange::new(start, line.end())) + .with_fix(Fix::safe_edit(Edit::deletion(last, line.end()))), + ); + } + } + None +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index aaedd82e2f790c..94696ab146fe87 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -14,6 +14,7 @@ pub(crate) use comparison_of_constant::*; pub(crate) use comparison_with_itself::*; pub(crate) use continue_in_finally::*; pub(crate) use duplicate_bases::*; +pub(crate) use empty_comment::*; pub(crate) use eq_without_hash::*; pub(crate) use global_at_module_level::*; pub(crate) use global_statement::*; @@ -91,6 +92,7 @@ mod comparison_of_constant; mod comparison_with_itself; mod continue_in_finally; mod duplicate_bases; +mod empty_comment; mod eq_without_hash; mod global_at_module_level; mod global_statement; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR2044_empty_comment.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR2044_empty_comment.py.snap new file mode 100644 index 00000000000000..9f6ed8090a68b7 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR2044_empty_comment.py.snap @@ -0,0 +1,83 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +empty_comment.py:3:1: PLR2044 [*] Line with empty comment + | +1 | # this line has a non-empty comment and is OK +2 | # this line is also OK, but the three following lines are not +3 | # + | ^ PLR2044 +4 | # +5 | # + | + = help: Delete the empty comment + +ℹ Safe fix +1 1 | # this line has a non-empty comment and is OK +2 2 | # this line is also OK, but the three following lines are not +3 |-# + 3 |+ +4 4 | # +5 5 | # +6 6 | + +empty_comment.py:4:5: PLR2044 [*] Line with empty comment + | +2 | # this line is also OK, but the three following lines are not +3 | # +4 | # + | ^ PLR2044 +5 | # + | + = help: Delete the empty comment + +ℹ Safe fix +1 1 | # this line has a non-empty comment and is OK +2 2 | # this line is also OK, but the three following lines are not +3 3 | # +4 |- # + 4 |+ +5 5 | # +6 6 | +7 7 | + +empty_comment.py:5:5: PLR2044 [*] Line with empty comment + | +3 | # +4 | # +5 | # + | ^^^^^ PLR2044 + | + = help: Delete the empty comment + +ℹ Safe fix +2 2 | # this line is also OK, but the three following lines are not +3 3 | # +4 4 | # +5 |- # +6 5 | +7 6 | + 7 |+ +8 8 | # this non-empty comment has trailing whitespace and is OK +9 9 | +10 10 | + +empty_comment.py:12:11: PLR2044 [*] Line with empty comment + | +11 | def foo(): # this comment is OK, the one below is not +12 | pass # + | ^^^^^ PLR2044 + | + = help: Delete the empty comment + +ℹ Safe fix +9 9 | +10 10 | +11 11 | def foo(): # this comment is OK, the one below is not +12 |- pass # + 12 |+ pass +13 13 | +14 14 | +15 15 | # the lines below have no comments and are OK + + diff --git a/ruff.schema.json b/ruff.schema.json index d7ff44db9d7b0a..c8c37961242429 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3155,6 +3155,8 @@ "PLR20", "PLR200", "PLR2004", + "PLR204", + "PLR2044", "PLR5", "PLR55", "PLR550",