diff --git a/crates/ruff_linter/src/rules/eradicate/detection.rs b/crates/ruff_linter/src/rules/eradicate/detection.rs index 77be87c048f1b..ac9c6e0e4cddd 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 = 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 = 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::>(); + + 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 fd6288fd75dfb..4bb0612001e53 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;