Implement flake8_fixme and refactor TodoDirective (#4681)

crates/ruff/resources/test/fixtures/flake8_fixme/T00.py

@@ -0,0 +1,8 @@
+# TODO: todo
+# todo: todo
+# XXX: xxx
+# xxx: xxx
+# HACK: hack
+# hack: hack
+# FIXME: fixme
+# fixme: fixme

crates/ruff/src/checkers/tokens.rs

@@ -3,12 +3,13 @@
 use rustpython_parser::lexer::LexResult;
 use rustpython_parser::Tok;
 
+use crate::directives::TodoComment;
 use crate::lex::docstring_detection::StateMachine;
 use crate::registry::{AsRule, Rule};
 use crate::rules::ruff::rules::Context;
 use crate::rules::{
-    eradicate, flake8_commas, flake8_implicit_str_concat, flake8_pyi, flake8_quotes, flake8_todos,
-    pycodestyle, pylint, pyupgrade, ruff,
+    eradicate, flake8_commas, flake8_fixme, flake8_implicit_str_concat, flake8_pyi, flake8_quotes,
+    flake8_todos, pycodestyle, pylint, pyupgrade, ruff,
 };
 use crate::settings::Settings;
 use ruff_diagnostics::Diagnostic;
@@ -60,6 +61,8 @@ pub(crate) fn check_tokens(
     ]);
     let enforce_extraneous_parenthesis = settings.rules.enabled(Rule::ExtraneousParentheses);
     let enforce_type_comment_in_stub = settings.rules.enabled(Rule::TypeCommentInStub);
+
+    // Combine flake8_todos and flake8_fixme so that we can reuse detected [`TodoDirective`]s.
     let enforce_todos = settings.rules.any_enabled(&[
         Rule::InvalidTodoTag,
         Rule::MissingTodoAuthor,
@@ -68,6 +71,10 @@ pub(crate) fn check_tokens(
         Rule::MissingTodoDescription,
         Rule::InvalidTodoCapitalization,
         Rule::MissingSpaceAfterTodoColon,
+        Rule::LineContainsFixme,
+        Rule::LineContainsXxx,
+        Rule::LineContainsTodo,
+        Rule::LineContainsHack,
     ]);
 
     // RUF001, RUF002, RUF003
@@ -184,9 +191,26 @@ pub(crate) fn check_tokens(
     }
 
     // TD001, TD002, TD003, TD004, TD005, TD006, TD007
+    // T001, T002, T003, T004
     if enforce_todos {
+        let todo_comments: Vec<TodoComment> = indexer
+            .comment_ranges()
+            .iter()
+            .enumerate()
+            .filter_map(|(i, comment_range)| {
+                let comment = locator.slice(*comment_range);
+                TodoComment::from_comment(comment, *comment_range, i)
+            })
+            .collect();
+
+        diagnostics.extend(
+            flake8_todos::rules::todos(&todo_comments, indexer, locator, settings)
+                .into_iter()
+                .filter(|diagnostic| settings.rules.enabled(diagnostic.kind.rule())),
+        );
+
         diagnostics.extend(
-            flake8_todos::rules::todos(indexer, locator, settings)
+            flake8_fixme::rules::todos(&todo_comments)
                 .into_iter()
                 .filter(|diagnostic| settings.rules.enabled(diagnostic.kind.rule())),
         );

crates/ruff/src/codes.rs

@@ -760,6 +760,12 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         // airflow
         (Airflow, "001") => (RuleGroup::Unspecified, Rule::AirflowVariableNameTaskIdMismatch),
 
+        // flake8-fixme
+        (Flake8Fixme, "001") => (RuleGroup::Unspecified, Rule::LineContainsFixme),
+        (Flake8Fixme, "002") => (RuleGroup::Unspecified, Rule::LineContainsTodo),
+        (Flake8Fixme, "003") => (RuleGroup::Unspecified, Rule::LineContainsXxx),
+        (Flake8Fixme, "004") => (RuleGroup::Unspecified, Rule::LineContainsHack),
+
         _ => return None,
     })
 }

crates/ruff/src/directives.rs

@@ -1,4 +1,6 @@
-//! Extract `# noqa` and `# isort: skip` directives from tokenized source.
+//! Extract `# noqa`, `# isort: skip`, and `# TODO` directives from tokenized source.
+
+use std::str::FromStr;
 
 use bitflags::bitflags;
 use ruff_text_size::{TextLen, TextRange, TextSize};
@@ -217,6 +219,133 @@ fn extract_isort_directives(lxr: &[LexResult], locator: &Locator) -> IsortDirect
     }
 }
 
+/// A comment that contains a [`TodoDirective`]
+pub(crate) struct TodoComment<'a> {
+    /// The comment's text
+    pub(crate) content: &'a str,
+    /// The directive found within the comment.
+    pub(crate) directive: TodoDirective<'a>,
+    /// The comment's actual [`TextRange`].
+    pub(crate) range: TextRange,
+    /// The comment range's position in [`Indexer`].comment_ranges()
+    pub(crate) range_index: usize,
+}
+
+impl<'a> TodoComment<'a> {
+    /// Attempt to transform a normal comment into a [`TodoComment`].
+    pub(crate) fn from_comment(
+        content: &'a str,
+        range: TextRange,
+        range_index: usize,
+    ) -> Option<Self> {
+        TodoDirective::from_comment(content, range).map(|directive| Self {
+            content,
+            directive,
+            range,
+            range_index,
+        })
+    }
+}
+
+#[derive(Debug, PartialEq)]
+pub(crate) struct TodoDirective<'a> {
+    /// The actual directive
+    pub(crate) content: &'a str,
+    /// The directive's [`TextRange`] in the file.
+    pub(crate) range: TextRange,
+    /// The directive's kind: HACK, XXX, FIXME, or TODO.
+    pub(crate) kind: TodoDirectiveKind,
+}
+
+impl<'a> TodoDirective<'a> {
+    /// Extract a [`TodoDirective`] from a comment.
+    pub(crate) fn from_comment(comment: &'a str, comment_range: TextRange) -> Option<Self> {
+        // The directive's offset from the start of the comment.
+        let mut relative_offset = TextSize::new(0);
+        let mut subset_opt = Some(comment);
+
+        // Loop over `#`-delimited sections of the comment to check for directives. This will
+        // correctly handle cases like `# foo # TODO`.
+        while let Some(subset) = subset_opt {
+            let trimmed = subset.trim_start_matches('#').trim_start();
+
+            let offset = subset.text_len() - trimmed.text_len();
+            relative_offset += offset;
+
+            // If we detect a TodoDirectiveKind variant substring in the comment, construct and
+            // return the appropriate TodoDirective
+            if let Ok(directive_kind) = trimmed.parse::<TodoDirectiveKind>() {
+                let len = directive_kind.len();
+
+                return Some(Self {
+                    content: &comment[TextRange::at(relative_offset, len)],
+                    range: TextRange::at(comment_range.start() + relative_offset, len),
+                    kind: directive_kind,
+                });
+            }
+
+            // Shrink the subset to check for the next phrase starting with "#".
+            subset_opt = if let Some(new_offset) = trimmed.find('#') {
+                relative_offset += TextSize::try_from(new_offset).unwrap();
+                subset.get(relative_offset.to_usize()..)
+            } else {
+                None
+            };
+        }
+
+        None
+    }
+}
+
+#[derive(Debug, PartialEq)]
+pub(crate) enum TodoDirectiveKind {
+    Todo,
+    Fixme,
+    Xxx,
+    Hack,
+}
+
+impl FromStr for TodoDirectiveKind {
+    type Err = ();
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        // The lengths of the respective variant strings: TODO, FIXME, HACK, XXX
+        for length in [3, 4, 5] {
+            let Some(substr) = s.get(..length) else {
+                break;
+            };
+
+            match substr.to_lowercase().as_str() {
+                "fixme" => {
+                    return Ok(TodoDirectiveKind::Fixme);
+                }
+                "hack" => {
+                    return Ok(TodoDirectiveKind::Hack);
+                }
+                "todo" => {
+                    return Ok(TodoDirectiveKind::Todo);
+                }
+                "xxx" => {
+                    return Ok(TodoDirectiveKind::Xxx);
+                }
+                _ => continue,
+            }
+        }
+
+        Err(())
+    }
+}
+
+impl TodoDirectiveKind {
+    fn len(&self) -> TextSize {
+        match self {
+            TodoDirectiveKind::Xxx => TextSize::new(3),
+            TodoDirectiveKind::Hack | TodoDirectiveKind::Todo => TextSize::new(4),
+            TodoDirectiveKind::Fixme => TextSize::new(5),
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use ruff_text_size::{TextLen, TextRange, TextSize};
@@ -225,7 +354,9 @@ mod tests {
 
     use ruff_python_ast::source_code::{Indexer, Locator};
 
-    use crate::directives::{extract_isort_directives, extract_noqa_line_for};
+    use crate::directives::{
+        extract_isort_directives, extract_noqa_line_for, TodoDirective, TodoDirectiveKind,
+    };
     use crate::noqa::NoqaMapping;
 
     fn noqa_mappings(contents: &str) -> NoqaMapping {
@@ -428,4 +559,62 @@ z = x + 1";
             vec![TextSize::from(13)]
         );
     }
+
+    #[test]
+    fn todo_directives() {
+        let test_comment = "# TODO: todo tag";
+        let test_comment_range = TextRange::at(TextSize::new(0), test_comment.text_len());
+        let expected = TodoDirective {
+            content: "TODO",
+            range: TextRange::new(TextSize::new(2), TextSize::new(6)),
+            kind: TodoDirectiveKind::Todo,
+        };
+        assert_eq!(
+            expected,
+            TodoDirective::from_comment(test_comment, test_comment_range).unwrap()
+        );
+
+        let test_comment = "#TODO: todo tag";
+        let test_comment_range = TextRange::at(TextSize::new(0), test_comment.text_len());
+        let expected = TodoDirective {
+            content: "TODO",
+            range: TextRange::new(TextSize::new(1), TextSize::new(5)),
+            kind: TodoDirectiveKind::Todo,
+        };
+        assert_eq!(
+            expected,
+            TodoDirective::from_comment(test_comment, test_comment_range).unwrap()
+        );
+
+        let test_comment = "# fixme: fixme tag";
+        let test_comment_range = TextRange::at(TextSize::new(0), test_comment.text_len());
+        let expected = TodoDirective {
+            content: "fixme",
+            range: TextRange::new(TextSize::new(2), TextSize::new(7)),
+            kind: TodoDirectiveKind::Fixme,
+        };
+        assert_eq!(
+            expected,
+            TodoDirective::from_comment(test_comment, test_comment_range).unwrap()
+        );
+
+        let test_comment = "# noqa # TODO: todo";
+        let test_comment_range = TextRange::at(TextSize::new(0), test_comment.text_len());
+        let expected = TodoDirective {
+            content: "TODO",
+            range: TextRange::new(TextSize::new(9), TextSize::new(13)),
+            kind: TodoDirectiveKind::Todo,
+        };
+        assert_eq!(
+            expected,
+            TodoDirective::from_comment(test_comment, test_comment_range).unwrap()
+        );
+
+        let test_comment = "# no directive";
+        let test_comment_range = TextRange::at(TextSize::new(0), test_comment.text_len());
+        assert_eq!(
+            None,
+            TodoDirective::from_comment(test_comment, test_comment_range)
+        );
+    }
 }

crates/ruff/src/registry.rs

@@ -678,6 +678,11 @@ ruff_macros::register_rules!(
     rules::flake8_todos::rules::MissingSpaceAfterTodoColon,
     // airflow
     rules::airflow::rules::AirflowVariableNameTaskIdMismatch,
+    // flake8-fixme
+    rules::flake8_fixme::rules::LineContainsTodo,
+    rules::flake8_fixme::rules::LineContainsHack,
+    rules::flake8_fixme::rules::LineContainsXxx,
+    rules::flake8_fixme::rules::LineContainsFixme,
 );
 
 pub trait AsRule {
@@ -827,6 +832,9 @@ pub enum Linter {
     /// [flake8-todos](https://github.com/orsinium-labs/flake8-todos/)
     #[prefix = "TD"]
     Flake8Todos,
+    /// [flake8-fixme](https://github.com/tommilligan/flake8-fixme)
+    #[prefix = "T"]
+    Flake8Fixme,
     /// [eradicate](https://pypi.org/project/eradicate/)
     #[prefix = "ERA"]
     Eradicate,
@@ -961,7 +969,11 @@ impl Rule {
             | Rule::MissingTodoColon
             | Rule::MissingTodoDescription
             | Rule::InvalidTodoCapitalization
-            | Rule::MissingSpaceAfterTodoColon => LintSource::Tokens,
+            | Rule::MissingSpaceAfterTodoColon
+            | Rule::LineContainsFixme
+            | Rule::LineContainsHack
+            | Rule::LineContainsTodo
+            | Rule::LineContainsXxx => LintSource::Tokens,
             Rule::IOError => LintSource::Io,
             Rule::UnsortedImports | Rule::MissingRequiredImport => LintSource::Imports,
             Rule::ImplicitNamespacePackage | Rule::InvalidModuleName => LintSource::Filesystem,

crates/ruff/src/rules/flake8_fixme/mod.rs

@@ -0,0 +1,27 @@
+pub mod rules;
+
+#[cfg(test)]
+mod tests {
+    use std::path::Path;
+
+    use anyhow::Result;
+    use test_case::test_case;
+
+    use crate::registry::Rule;
+    use crate::test::test_path;
+    use crate::{assert_messages, settings};
+
+    #[test_case(Rule::LineContainsFixme; "T001")]
+    #[test_case(Rule::LineContainsHack; "T002")]
+    #[test_case(Rule::LineContainsTodo; "T003")]
+    #[test_case(Rule::LineContainsXxx; "T004")]
+    fn rules(rule_code: Rule) -> Result<()> {
+        let snapshot = format!("{}_T00.py", rule_code.as_ref());
+        let diagnostics = test_path(
+            Path::new("flake8_fixme/T00.py"),
+            &settings::Settings::for_rule(rule_code),
+        )?;
+        assert_messages!(snapshot, diagnostics);
+        Ok(())
+    }
+}