Change TODO directive detection to work with multiple pound signs on …

…the same line (#4558)

crates/ruff/resources/test/fixtures/flake8_todos/TD001.py

@@ -6,3 +6,4 @@
 # T001 - errors
 # XXX (evanrittenhouse): this is not fine
 # FIXME (evanrittenhouse): this is not fine
+# foo # XXX: this isn't fine either

crates/ruff/resources/test/fixtures/flake8_todos/TD002.py

@@ -5,3 +5,4 @@
 # TODO: this has no author
 # FIXME: neither does this
 # TODO : and neither does this
+# foo # TODO: this doesn't either

crates/ruff/resources/test/fixtures/flake8_todos/TD003.py

@@ -26,4 +26,6 @@ def foo(x):
 # TODO: followed by a new TODO with an issue link
 # TDO-3870
 
+# foo # TODO: no link!
+
 # TODO: here's a TODO on the last line with no link

crates/ruff/resources/test/fixtures/flake8_todos/TD004.py

@@ -4,3 +4,5 @@
 # TODO this has no colon
 # TODO(evanrittenhouse 😀) this has no colon
 # FIXME add a colon
+# foo # TODO add a colon
+# TODO this has a colon but it doesn't terminate the tag, so this should throw. https://www.google.com

crates/ruff/resources/test/fixtures/flake8_todos/TD005.py

@@ -4,3 +4,4 @@
 # TODO(evanrittenhouse):
 # TODO(evanrittenhouse)
 # FIXME
+# foo # TODO

crates/ruff/resources/test/fixtures/flake8_todos/TD006.py

@@ -3,3 +3,4 @@
 # TDO006 - error
 # ToDo (evanrittenhouse): invalid capitalization
 # todo (evanrittenhouse): another invalid capitalization
+# foo # todo: invalid capitalization

crates/ruff/resources/test/fixtures/flake8_todos/TD007.py

@@ -6,3 +6,5 @@
 # TODO (evanrittenhouse):this doesn't either
 # TODO:neither does this
 # FIXME:and lastly neither does this
+# foo # TODO:this is really the last one
+# TODO this colon doesn't terminate the tag, so don't check it. https://www.google.com

crates/ruff/src/rules/flake8_builtins/rules/rules.rs

@@ -1,3 +1 @@
 
-
-

crates/ruff/src/rules/flake8_todos/rules/todos.rs

@@ -230,33 +230,42 @@ enum Directive {
 impl Directive {
     /// Extract a [`Directive`] from a comment.
     ///
-    /// Returns the offset of the directive within the comment, and the matching directive tag.
+    /// Returns the matching directive tag and its offset within the comment.
     fn from_comment(comment: &str) -> Option<(Directive, TextSize)> {
-        let trimmed = comment.trim_start_matches('#').trim_start();
-        let offset = comment.text_len() - trimmed.text_len();
-        let mut chars = trimmed.chars();
-        match (
-            chars.next(),
-            chars.next(),
-            chars.next(),
-            chars.next(),
-            chars.next(),
-        ) {
-            (
-                Some('F' | 'f'),
-                Some('I' | 'i'),
-                Some('X' | 'x'),
-                Some('M' | 'm'),
-                Some('E' | 'e'),
-            ) => Some((Directive::Fixme, offset)),
-            (Some('T' | 't'), Some('O' | 'o'), Some('D' | 'd'), Some('O' | 'o'), ..) => {
-                Some((Directive::Todo, offset))
-            }
-            (Some('X' | 'x'), Some('X' | 'x'), Some('X' | 'x'), ..) => {
-                Some((Directive::Xxx, offset))
+        let mut subset_opt = Some(comment);
+        let mut total_offset = TextSize::new(0);
+
+        // Loop over the comment to catch cases like `# foo # TODO`.
+        while let Some(subset) = subset_opt {
+            let trimmed = subset.trim_start_matches('#').trim_start().to_lowercase();
+
+            let offset = subset.text_len() - trimmed.text_len();
+            total_offset += offset;
+
+            let directive = if trimmed.starts_with("fixme") {
+                Some((Directive::Fixme, total_offset))
+            } else if trimmed.starts_with("xxx") {
+                Some((Directive::Xxx, total_offset))
+            } else if trimmed.starts_with("todo") {
+                Some((Directive::Todo, total_offset))
+            } else {
+                None
+            };
+
+            if directive.is_some() {
+                return directive;
             }
-            _ => None,
+
+            // Shrink the subset to check for the next phrase starting with "#".
+            subset_opt = if let Some(new_offset) = trimmed.find('#') {
+                total_offset += TextSize::try_from(new_offset).unwrap();
+                subset.get(total_offset.to_usize()..)
+            } else {
+                None
+            };
         }
+
+        None
     }
 
     /// Returns the length of the directive tag.
@@ -412,25 +421,19 @@ fn static_errors(
             TextSize::new(0)
         };
 
-    let post_author = &post_tag[usize::from(author_end)..];
-
-    let post_colon = if let Some((.., after_colon)) = post_author.split_once(':') {
-        if let Some(stripped) = after_colon.strip_prefix(' ') {
-            stripped
-        } else {
-            // TD-007
+    let after_author = &post_tag[usize::from(author_end)..];
+    if let Some(after_colon) = after_author.strip_prefix(':') {
+        if after_colon.is_empty() {
+            diagnostics.push(Diagnostic::new(MissingTodoDescription, tag.range));
+        } else if !after_colon.starts_with(char::is_whitespace) {
             diagnostics.push(Diagnostic::new(MissingSpaceAfterTodoColon, tag.range));
-            after_colon
         }
     } else {
-        // TD-004
         diagnostics.push(Diagnostic::new(MissingTodoColon, tag.range));
-        ""
-    };
 
-    if post_colon.is_empty() {
-        // TD-005
-        diagnostics.push(Diagnostic::new(MissingTodoDescription, tag.range));
+        if after_author.is_empty() {
+            diagnostics.push(Diagnostic::new(MissingTodoDescription, tag.range));
+        }
     }
 }
 
@@ -466,5 +469,17 @@ mod tests {
             range: TextRange::new(TextSize::new(2), TextSize::new(7)),
         };
         assert_eq!(Some(expected), detect_tag(test_comment, TextSize::new(0)));
+        let test_comment = "# noqa # TODO: todo";
+        let expected = Tag {
+            content: "TODO",
+            range: TextRange::new(TextSize::new(9), TextSize::new(13)),
+        };
+        assert_eq!(Some(expected), detect_tag(test_comment, TextSize::new(0)));
+        let test_comment = "# noqa # XXX";
+        let expected = Tag {
+            content: "XXX",
+            range: TextRange::new(TextSize::new(9), TextSize::new(12)),
+        };
+        assert_eq!(Some(expected), detect_tag(test_comment, TextSize::new(0)));
     }
 }

crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__invalid-todo-capitalization_TD006.py.snap

@@ -8,6 +8,7 @@ TD006.py:4:3: TD006 [*] Invalid TODO capitalization: `ToDo` should be `TODO`
 6 | # ToDo (evanrittenhouse): invalid capitalization
   |   ^^^^ TD006
 7 | # todo (evanrittenhouse): another invalid capitalization
+8 | # foo # todo: invalid capitalization
   |
   = help: Replace `ToDo` with `TODO`
 
@@ -18,13 +19,15 @@ TD006.py:4:3: TD006 [*] Invalid TODO capitalization: `ToDo` should be `TODO`
 4   |-# ToDo (evanrittenhouse): invalid capitalization
   4 |+# TODO (evanrittenhouse): invalid capitalization
 5 5 | # todo (evanrittenhouse): another invalid capitalization
+6 6 | # foo # todo: invalid capitalization
 
 TD006.py:5:3: TD006 [*] Invalid TODO capitalization: `todo` should be `TODO`
   |
 5 | # TDO006 - error
 6 | # ToDo (evanrittenhouse): invalid capitalization
 7 | # todo (evanrittenhouse): another invalid capitalization
   |   ^^^^ TD006
+8 | # foo # todo: invalid capitalization
   |
   = help: Replace `todo` with `TODO`
 
@@ -34,5 +37,22 @@ TD006.py:5:3: TD006 [*] Invalid TODO capitalization: `todo` should be `TODO`
 4 4 | # ToDo (evanrittenhouse): invalid capitalization
 5   |-# todo (evanrittenhouse): another invalid capitalization
   5 |+# TODO (evanrittenhouse): another invalid capitalization
+6 6 | # foo # todo: invalid capitalization
+
+TD006.py:6:9: TD006 [*] Invalid TODO capitalization: `todo` should be `TODO`
+  |
+6 | # ToDo (evanrittenhouse): invalid capitalization
+7 | # todo (evanrittenhouse): another invalid capitalization
+8 | # foo # todo: invalid capitalization
+  |         ^^^^ TD006
+  |
+  = help: Replace `todo` with `TODO`
+
+ℹ Fix
+3 3 | # TDO006 - error
+4 4 | # ToDo (evanrittenhouse): invalid capitalization
+5 5 | # todo (evanrittenhouse): another invalid capitalization
+6   |-# foo # todo: invalid capitalization
+  6 |+# foo # TODO: invalid capitalization
 
 

crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__invalid-todo-tag_TD001.py.snap

@@ -2,19 +2,29 @@
 source: crates/ruff/src/rules/flake8_todos/mod.rs
 ---
 TD001.py:7:3: TD001 Invalid TODO tag: `XXX`
-  |
-7 | # T001 - errors
-8 | # XXX (evanrittenhouse): this is not fine
-  |   ^^^ TD001
-9 | # FIXME (evanrittenhouse): this is not fine
-  |
+   |
+ 7 | # T001 - errors
+ 8 | # XXX (evanrittenhouse): this is not fine
+   |   ^^^ TD001
+ 9 | # FIXME (evanrittenhouse): this is not fine
+10 | # foo # XXX: this isn't fine either
+   |
 
 TD001.py:8:3: TD001 Invalid TODO tag: `FIXME`
    |
  8 | # T001 - errors
  9 | # XXX (evanrittenhouse): this is not fine
 10 | # FIXME (evanrittenhouse): this is not fine
    |   ^^^^^ TD001
+11 | # foo # XXX: this isn't fine either
+   |
+
+TD001.py:9:9: TD001 Invalid TODO tag: `XXX`
+   |
+ 9 | # XXX (evanrittenhouse): this is not fine
+10 | # FIXME (evanrittenhouse): this is not fine
+11 | # foo # XXX: this isn't fine either
+   |         ^^^ TD001
    |
 
 

crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-space-after-todo-colon_TD007.py.snap

@@ -28,6 +28,7 @@ TD007.py:7:3: TD007 Missing space after colon in TODO
  9 | # TODO:neither does this
    |   ^^^^ TD007
 10 | # FIXME:and lastly neither does this
+11 | # foo # TODO:this is really the last one
    |
 
 TD007.py:8:3: TD007 Missing space after colon in TODO
@@ -36,6 +37,17 @@ TD007.py:8:3: TD007 Missing space after colon in TODO
  9 | # TODO:neither does this
 10 | # FIXME:and lastly neither does this
    |   ^^^^^ TD007
+11 | # foo # TODO:this is really the last one
+12 | # TODO this colon doesn't terminate the tag, so don't check it. https://www.google.com
+   |
+
+TD007.py:9:9: TD007 Missing space after colon in TODO
+   |
+ 9 | # TODO:neither does this
+10 | # FIXME:and lastly neither does this
+11 | # foo # TODO:this is really the last one
+   |         ^^^^ TD007
+12 | # TODO this colon doesn't terminate the tag, so don't check it. https://www.google.com
    |
 
 

crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-author_TD002.py.snap

@@ -12,20 +12,30 @@ TD002.py:5:3: TD002 Missing author in TODO; try: `# TODO(<author_name>): ...`
   |
 
 TD002.py:6:3: TD002 Missing author in TODO; try: `# TODO(<author_name>): ...`
-  |
-6 | # T002 - errors
-7 | # TODO: this has no author
-8 | # FIXME: neither does this
-  |   ^^^^^ TD002
-9 | # TODO : and neither does this
-  |
+   |
+ 6 | # T002 - errors
+ 7 | # TODO: this has no author
+ 8 | # FIXME: neither does this
+   |   ^^^^^ TD002
+ 9 | # TODO : and neither does this
+10 | # foo # TODO: this doesn't either
+   |
 
 TD002.py:7:3: TD002 Missing author in TODO; try: `# TODO(<author_name>): ...`
-  |
-7 | # TODO: this has no author
-8 | # FIXME: neither does this
-9 | # TODO : and neither does this
-  |   ^^^^ TD002
-  |
+   |
+ 7 | # TODO: this has no author
+ 8 | # FIXME: neither does this
+ 9 | # TODO : and neither does this
+   |   ^^^^ TD002
+10 | # foo # TODO: this doesn't either
+   |
+
+TD002.py:8:9: TD002 Missing author in TODO; try: `# TODO(<author_name>): ...`
+   |
+ 8 | # FIXME: neither does this
+ 9 | # TODO : and neither does this
+10 | # foo # TODO: this doesn't either
+   |         ^^^^ TD002
+   |
 
 

crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-colon_TD004.py.snap

@@ -18,14 +18,34 @@ TD004.py:5:3: TD004 Missing colon in TODO
 7 | # TODO(evanrittenhouse 😀) this has no colon
   |   ^^^^ TD004
 8 | # FIXME add a colon
+9 | # foo # TODO add a colon
   |
 
 TD004.py:6:3: TD004 Missing colon in TODO
-  |
-6 | # TODO this has no colon
-7 | # TODO(evanrittenhouse 😀) this has no colon
-8 | # FIXME add a colon
-  |   ^^^^^ TD004
-  |
+   |
+ 6 | # TODO this has no colon
+ 7 | # TODO(evanrittenhouse 😀) this has no colon
+ 8 | # FIXME add a colon
+   |   ^^^^^ TD004
+ 9 | # foo # TODO add a colon
+10 | # TODO this has a colon but it doesn't terminate the tag, so this should throw. https://www.google.com
+   |
+
+TD004.py:7:9: TD004 Missing colon in TODO
+   |
+ 7 | # TODO(evanrittenhouse 😀) this has no colon
+ 8 | # FIXME add a colon
+ 9 | # foo # TODO add a colon
+   |         ^^^^ TD004
+10 | # TODO this has a colon but it doesn't terminate the tag, so this should throw. https://www.google.com
+   |
+
+TD004.py:8:3: TD004 Missing colon in TODO
+   |
+ 8 | # FIXME add a colon
+ 9 | # foo # TODO add a colon
+10 | # TODO this has a colon but it doesn't terminate the tag, so this should throw. https://www.google.com
+   |   ^^^^ TD004
+   |
 
 

crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-description_TD005.py.snap

@@ -18,6 +18,7 @@ TD005.py:5:3: TD005 Missing issue description after `TODO`
 7 | # TODO(evanrittenhouse)
   |   ^^^^ TD005
 8 | # FIXME
+9 | # foo # TODO
   |
 
 TD005.py:6:3: TD005 Missing issue description after `TODO`
@@ -26,6 +27,15 @@ TD005.py:6:3: TD005 Missing issue description after `TODO`
 7 | # TODO(evanrittenhouse)
 8 | # FIXME
   |   ^^^^^ TD005
+9 | # foo # TODO
+  |
+
+TD005.py:7:9: TD005 Missing issue description after `TODO`
+  |
+7 | # TODO(evanrittenhouse)
+8 | # FIXME
+9 | # foo # TODO
+  |         ^^^^ TD005
   |