From c2a9cf8ae59d40be2abaebd3830367fd2a015625 Mon Sep 17 00:00:00 2001
From: Tom Kuson <mail@tjkuson.me>
Date: Fri, 29 Sep 2023 00:13:11 +0100
Subject: [PATCH] Ignore TODO tags in `commented-out-code` (#7523)

## Summary

Extend the `task-tags` checking logic to ignore TODO tags (with or
without parentheses). For example,

```python
# TODO(tjkuson): Rewrite in Rust
```

is no longer flagged as commented-out code.

Closes #7031.

I also updated the documentation to inform users that the rule is prone
to false positives like this!

EDIT: Accidentally linked to the wrong issue when first opening this PR,
now corrected.

## Test Plan

`cargo test`
---
 .../src/rules/eradicate/detection.rs          | 56 ++++++++++++++++---
 .../eradicate/rules/commented_out_code.rs     |  8 ++-
 2 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/crates/ruff_linter/src/rules/eradicate/detection.rs b/crates/ruff_linter/src/rules/eradicate/detection.rs
index 77be87c048..ac9c6e0e4c 100644
--- a/crates/ruff_linter/src/rules/eradicate/detection.rs
+++ b/crates/ruff_linter/src/rules/eradicate/detection.rs
@@ -6,7 +6,7 @@ use ruff_python_parser::parse_suite;
 
 static ALLOWLIST_REGEX: Lazy<Regex> = Lazy::new(|| {
     Regex::new(
-        r"^(?i)(?:pylint|pyright|noqa|nosec|region|endregion|type:\s*ignore|fmt:\s*(on|off)|isort:\s*(on|off|skip|skip_file|split|dont-add-imports(:\s*\[.*?])?)|mypy:|SPDX-License-Identifier:)"
+        r"^(?i)(?:pylint|pyright|noqa|nosec|region|endregion|type:\s*ignore|fmt:\s*(on|off)|isort:\s*(on|off|skip|skip_file|split|dont-add-imports(:\s*\[.*?])?)|mypy:|SPDX-License-Identifier:)",
     ).unwrap()
 });
 static BRACKET_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"^[()\[\]{}\s]+$").unwrap());
@@ -39,6 +39,15 @@ pub(crate) fn comment_contains_code(line: &str, task_tags: &[String]) -> bool {
         return false;
     };
 
+    // Ignore task tag comments (e.g., "# TODO(tom): Refactor").
+    if line
+        .split(&[' ', ':', '('])
+        .next()
+        .is_some_and(|first| task_tags.iter().any(|tag| tag == first))
+    {
+        return false;
+    }
+
     // Ignore non-comment related hashes (e.g., "# Issue #999").
     if HASH_NUMBER.is_match(line) {
         return false;
@@ -49,12 +58,6 @@ pub(crate) fn comment_contains_code(line: &str, task_tags: &[String]) -> bool {
         return false;
     }
 
-    if let Some(first) = line.split(&[' ', ':']).next() {
-        if task_tags.iter().any(|tag| tag == first) {
-            return false;
-        }
-    }
-
     if CODING_COMMENT_REGEX.is_match(line) {
         return false;
     }
@@ -102,6 +105,7 @@ fn multiline_case(line: &str) -> bool {
 #[cfg(test)]
 mod tests {
     use super::comment_contains_code;
+    use crate::settings::TASK_TAGS;
 
     #[test]
     fn comment_contains_code_basic() {
@@ -279,4 +283,42 @@ mod tests {
             &["XXX".to_string()]
         ));
     }
+
+    #[test]
+    fn comment_contains_todo() {
+        let task_tags = TASK_TAGS
+            .iter()
+            .map(ToString::to_string)
+            .collect::<Vec<_>>();
+
+        assert!(!comment_contains_code(
+            "# TODO(tom): Rewrite in Rust",
+            &task_tags
+        ));
+        assert!(!comment_contains_code(
+            "# TODO: Rewrite in Rust",
+            &task_tags
+        ));
+        assert!(!comment_contains_code("# TODO:Rewrite in Rust", &task_tags));
+
+        assert!(!comment_contains_code(
+            "# FIXME(tom): Rewrite in Rust",
+            &task_tags
+        ));
+        assert!(!comment_contains_code(
+            "# FIXME: Rewrite in Rust",
+            &task_tags
+        ));
+        assert!(!comment_contains_code(
+            "# FIXME:Rewrite in Rust",
+            &task_tags
+        ));
+
+        assert!(!comment_contains_code(
+            "# XXX(tom): Rewrite in Rust",
+            &task_tags
+        ));
+        assert!(!comment_contains_code("# XXX: Rewrite in Rust", &task_tags));
+        assert!(!comment_contains_code("# XXX:Rewrite in Rust", &task_tags));
+    }
 }
diff --git a/crates/ruff_linter/src/rules/eradicate/rules/commented_out_code.rs b/crates/ruff_linter/src/rules/eradicate/rules/commented_out_code.rs
index fd6288fd75..4bb0612001 100644
--- a/crates/ruff_linter/src/rules/eradicate/rules/commented_out_code.rs
+++ b/crates/ruff_linter/src/rules/eradicate/rules/commented_out_code.rs
@@ -15,13 +15,19 @@ use super::super::detection::comment_contains_code;
 /// Commented-out code is dead code, and is often included inadvertently.
 /// It should be removed.
 ///
+/// ## Known problems
+/// Prone to false positives when checking comments that resemble Python code,
+/// but are not actually Python code ([#4845]).
+///
 /// ## Example
 /// ```python
-/// # print('foo')
+/// # print("Hello, world!")
 /// ```
 ///
 /// ## Options
 /// - `task-tags`
+///
+/// [#4845]: https://github.com/astral-sh/ruff/issues/4845
 #[violation]
 pub struct CommentedOutCode;