[pylint] Implement empty-comment (PLR2044)

[mdbernard]

Summary

Part of #970.

This adds Pylint's R0244 empty_comment lint as well as an always-safe fix.

Test Plan

The included snapshot verifies the following:

• A line containing only a non-empty comment is not changed
• A line containing leading whitespace before a non-empty comment is not changed
• A line containing only an empty comment has its content deleted
• A line containing only leading whitespace before an empty comment has its content deleted
• A line containing only leading and trailing whitespace on an empty comment has its content deleted
• A line containing trailing whitespace after a non-empty comment is not changed
• A line containing only a single newline character (i.e. a blank line) is not changed
• A line containing code followed by a non-empty comment is not changed
• A line containing code followed by an empty comment has its content deleted after the last non-whitespace character
• Lines containing code and no comments are not changed
• Empty comment lines within block comments are ignored
• Empty comments within triple-quoted sections are ignored

Comparison to Pylint

Running Ruff and Pylint 3.0.3 with Python 3.12.0 against the empty_comment.py file added in this PR, we see the following:

• Identical behavior:

  • empty_comment.py:3:0: R2044: Line with empty comment (empty-comment)
  • empty_comment.py:4:0: R2044: Line with empty comment (empty-comment)
  • empty_comment.py:5:0: R2044: Line with empty comment (empty-comment)
  • empty_comment.py:18:0: R2044: Line with empty comment (empty-comment)

• Differing behavior:

  • Pylint doesn't ignore empty comments in block comments commonly used for visual spacing; I decided these were fine in this implementation since many projects use these and likely do not want them removed.
    • empty_comment.py:28:0: R2044: Line with empty comment (empty-comment)
  • Pylint detects "empty comments" within the triple-quoted section at the bottom of the file, which is arguably a bug in the Pylint implementation since these are not truly comments. These are ignored by this implementation.
    • empty_comment.py:37:0: R2044: Line with empty comment (empty-comment)
    • empty_comment.py:38:0: R2044: Line with empty comment (empty-comment)
    • empty_comment.py:39:0: R2044: Line with empty comment (empty-comment)

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+27 -2 violations, +0 -0 fixes in 6 projects; 35 projects unchanged)

apache/airflow (+8 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview --select ALL

+ docs/exts/docroles.py:20:1: PLR2044 [*] Line with empty comment
+ docs/exts/docroles.py:21:1: PLR2044 [*] Line with empty comment
+ tests/providers/common/sql/hooks/test_sql.py:18:1: PLR2044 [*] Line with empty comment
+ tests/providers/databricks/hooks/test_databricks_sql.py:18:1: PLR2044 [*] Line with empty comment
+ tests/providers/exasol/hooks/test_sql.py:18:1: PLR2044 [*] Line with empty comment
+ tests/providers/google/suite/transfers/test_gcs_to_gdrive.py:117:5: PLR2044 [*] Line with empty comment
+ tests/providers/snowflake/hooks/test_sql.py:18:1: PLR2044 [*] Line with empty comment
+ tests/system/providers/google/cloud/storage_transfer/example_cloud_storage_transfer_service_aws.py:121:5: PLR2044 [*] Line with empty comment

aws/aws-sam-cli (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ samcli/local/docker/lambda_container.py:149:9: PLR2044 [*] Line with empty comment

milvus-io/pymilvus (+4 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ pymilvus/client/abstract.py:114:9: PLR2044 [*] Line with empty comment
+ pymilvus/client/abstract.py:18:9: PLR2044 [*] Line with empty comment
+ pymilvus/client/abstract.py:99:9: PLR2044 [*] Line with empty comment
+ pymilvus/client/types.py:133:5: PLR2044 [*] Line with empty comment

pandas-dev/pandas (+2 -2 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ pandas/tests/extension/decimal/array.py:175:9: PLC0415 `import` should be at the top-level of a file
- pandas/tests/extension/decimal/array.py:176:9: PLC0415 `import` should be at the top-level of a file
+ pandas/tests/extension/decimal/array.py:233:9: PLR6301 Method `_formatter` could be a function, class method, or static method
- pandas/tests/extension/decimal/array.py:234:9: PLR6301 Method `_formatter` could be a function, class method, or static method

rotki/rotki (+10 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ rotkehlchen/tests/db/test_db.py:752:48: PLR2044 [*] Line with empty comment
+ rotkehlchen/tests/db/test_db.py:757:48: PLR2044 [*] Line with empty comment
+ rotkehlchen/tests/db/test_db.py:795:52: PLR2044 [*] Line with empty comment
+ rotkehlchen/tests/db/test_db.py:797:52: PLR2044 [*] Line with empty comment
+ rotkehlchen/tests/db/test_db.py:801:52: PLR2044 [*] Line with empty comment
+ rotkehlchen/tests/db/test_db.py:834:52: PLR2044 [*] Line with empty comment
+ rotkehlchen/tests/db/test_db.py:837:52: PLR2044 [*] Line with empty comment
+ rotkehlchen/tests/db/test_db.py:843:52: PLR2044 [*] Line with empty comment
+ rotkehlchen/tests/db/test_db.py:844:52: PLR2044 [*] Line with empty comment
+ rotkehlchen/tests/db/test_db.py:845:52: PLR2044 [*] Line with empty comment

zulip/zulip (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview --select ALL

+ zerver/lib/export.py:842:5: PLR2044 [*] Line with empty comment
+ zerver/lib/export.py:867:5: PLR2044 [*] Line with empty comment

Changes by rule (3 rules affected)

code	total	+ violation	- violation	+ fix	- fix
PLR2044	25	25	0	0	0
PLC0415	2	1	1	0	0
PLR6301	2	1	1	0	0

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

[mdbernard]

Looks like the ecosystem checks found some edge cases not currently covered:

• "empty comment" lines within block comments, which add vertical whitespace for clarity, especially common as header comments for files
• lines containing a trailing # character in multi-line comments, which should not be considered empty comments

[charliermarsh]

Nice, thanks for kicking this off! Agreed on the edge cases described above.

[mdbernard]

• Add test for "empty comment" lines in multi-line strings/comments
• Add test for "empty comment" lines in block comments

[mdbernard]

• Account for comment style with trailing # at end of comment, e.g. https://github.com/rotki/rotki/blob/63302ca6d67ab928fd6cac7b6137c41df416416c/rotkehlchen/chain/ethereum/modules/l2/loopring.py#L527:

# -- Methods following the EthereumModule interface -- #

[mdbernard]

• Account for comment style with multiple # symbols on a single line (tying in with the above comment, probably best to just lint when a comment consists of only a single # symbol followed by only whitespace through the end of the line.

[mdbernard]

• Rebase

[charliermarsh]

This looks great, I really like what I'm seeing in the ecosystem checks too -- thank you!

I'm going to explore making this a token-based rule rather than a physical line-based rule, just because we want to avoid using physical lines when possible, but it'll be a mostly mechanical change...

[charliermarsh]

Pylint doesn't ignore empty comments in block comments commonly used for visual spacing; I decided these were fine in this implementation since many projects use these and likely do not want them removed.

I agree, we shouldn't flag these.

Pylint detects "empty comments" within the triple-quoted section at the bottom of the file, which is arguably a bug in the Pylint implementation since these are not truly comments. These are ignored by this implementation.

I agree, flagging empty comments within a multi-line string seems like a bug to me.

[mdbernard]

• Noting that I might've missed an edge case where two empty comments exist on consecutive lines forming a "block" comment.

[mdbernard]

we want to avoid using physical lines when possible

Out of curiosity, why?

[charliermarsh]

Out of curiosity, why?

Primarily because physical line-based approaches tend to be slower, since we need to run the rule over every line. This rule is actually a good example: we already know the locations of all comments in the file (via CommentRanges), so iterating over every line, then checking if the line intersects with the comment ranges, is wasteful as compared to merely iterating over the comment ranges directly. In general, we often have information we've already computed and/or can exploit to limit the search space vs. iterating over the physical lines.

[charliermarsh]

This is great work! I ended up moving to the token checker, and added support for the case in which we have multiple consecutive empty comments.