Skip to content

Commit

Permalink
[pylint] add PLR2044 empty_comment
Browse files Browse the repository at this point in the history
  • Loading branch information
mdbernard committed Dec 27, 2023
1 parent 2951339 commit b203d24
Show file tree
Hide file tree
Showing 12 changed files with 492 additions and 1 deletion.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 40 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pylint/empty_comment.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# 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

# Many codebases use multiple `#` characters on a single line to visually
# separate sections of code, so we don't consider these empty comments.

##########################################

# trailing `#` characters are not considered empty comments ###


def foo(): # this comment is OK, the one below is not
pass #


# the lines below have no comments and are OK
def bar():
pass


# "Empty comments" are common in block comments
# to add legibility. For example:
#
# The above line's "empty comment" is likely
# intentional and is considered OK.


# lines in multi-line strings/comments whose last non-whitespace character is a `#`
# do not count as empty comments
"""
The following lines are all fine:
#
#
#
"""
21 changes: 21 additions & 0 deletions crates/ruff_linter/src/checkers/physical_lines.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
//! Lint rules based on checking physical lines.

use std::collections::HashSet;

use ruff_diagnostics::Diagnostic;
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
Expand Down Expand Up @@ -32,9 +35,18 @@ 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();

let block_comment_line_offsets: HashSet<TextSize> = indexer
.comment_ranges()
.block_comments(locator)
.into_iter()
.flatten()
.map(|o| locator.line_start(o))
.collect();

for line in locator.contents().universal_newlines() {
while doc_lines_iter
.next_if(|doc_line_start| line.range().contains_inclusive(**doc_line_start))
Expand Down Expand Up @@ -68,6 +80,15 @@ pub(crate) fn check_physical_lines(
diagnostics.push(diagnostic);
}
}

if enforce_empty_comment
&& !block_comment_line_offsets.contains(&line.start())
&& indexer.comment_ranges().intersects(line.range())
{
if let Some(diagnostic) = pylint::rules::empty_comment(&line) {
diagnostics.push(diagnostic);
}
}
}

if enforce_no_newline_at_end_of_file {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,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)]
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ impl Rule {
Rule::BidirectionalUnicode
| Rule::BlankLineWithWhitespace
| Rule::DocLineTooLong
| Rule::EmptyComment
| Rule::LineTooLong
| Rule::MissingCopyrightNotice
| Rule::MissingNewlineAtEndOfFile
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
88 changes: 88 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/empty_comment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_trivia::is_python_whitespace;
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<String> {
Some(format!("Delete the empty comment"))
}
}

/// PLR2044
pub(crate) fn empty_comment(line: &Line) -> Option<Diagnostic> {
let first_hash_col = line
.as_str()
.char_indices()
.find(|&(_, char)| char == '#')? // indicates the line does not contain a comment
.0;

let last_non_whitespace_col = line
.as_str()
.char_indices()
.rev()
.find(|&(_, char)| !is_python_whitespace(char))
.unwrap() // already verified at least one '#' char is in the line
.0;

if last_non_whitespace_col != first_hash_col {
return None; // the comment is not empty
}

let last = match line
.as_str()
.char_indices()
.rev()
.find(|&(_, c)| !is_python_whitespace(c) && c != '#')
{
#[allow(clippy::cast_possible_truncation)] // TODO: justify safety
Some((i, _second_to_last_non_whitespace_char)) => {
TextSize::new((line.start().to_usize() + i + 1) as u32)
}
None => line.start(),
};

Some(
Diagnostic::new(
EmptyComment,
#[allow(clippy::cast_possible_truncation)] // TODO: justify safety
TextRange::new(
line.start() + TextSize::new(first_hash_col as u32),
line.end(),
),
)
.with_fix(Fix::safe_edit(Edit::deletion(last, line.end()))),
)
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -92,6 +93,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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
---
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 | # this non-empty comment has trailing whitespace and is OK

empty_comment.py:5:9: PLR2044 [*] Line with empty comment
|
3 | #
4 | #
5 | #
| ^^^^^ PLR2044
6 |
7 | # this non-empty comment has trailing whitespace and is OK
|
= 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 |
6 |+
7 7 | # this non-empty comment has trailing whitespace and is OK
8 8 |
9 9 | # Many codebases use multiple `#` characters on a single line to visually

empty_comment.py:18:11: PLR2044 [*] Line with empty comment
|
17 | def foo(): # this comment is OK, the one below is not
18 | pass #
| ^^^^^ PLR2044
|
= help: Delete the empty comment

Safe fix
15 15 |
16 16 |
17 17 | def foo(): # this comment is OK, the one below is not
18 |- pass #
18 |+ pass
19 19 |
20 20 |
21 21 | # the lines below have no comments and are OK


1 change: 1 addition & 0 deletions crates/ruff_python_trivia/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ unicode-ident = { workspace = true }
insta = { workspace = true }
ruff_python_ast = { path = "../ruff_python_ast" }
ruff_python_parser = { path = "../ruff_python_parser" }
ruff_python_index = { path = "../ruff_python_index" }

[lints]
workspace = true
Loading

0 comments on commit b203d24

Please sign in to comment.