From 9bb4722ebf554aab472cae0c8bf29e8908902df7 Mon Sep 17 00:00:00 2001 From: Sid <27780930+autinerd@users.noreply.github.com> Date: Mon, 14 Oct 2024 12:21:45 +0200 Subject: [PATCH] [`flake8-todos`] Allow words starting with todo (#13640) Co-authored-by: Micha Reiser --- .../test/fixtures/flake8_todos/TD006.py | 1 + crates/ruff_linter/src/directives.rs | 45 +++++-------- ..._invalid-todo-capitalization_TD006.py.snap | 66 +++++++++---------- 3 files changed, 49 insertions(+), 63 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_todos/TD006.py b/crates/ruff_linter/resources/test/fixtures/flake8_todos/TD006.py index 90fbbe387b..b811ee3a51 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_todos/TD006.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_todos/TD006.py @@ -1,5 +1,6 @@ # TDO006 - accepted # TODO (evanrittenhouse): this is a valid TODO +# Todoism is a word which starts with todo, but is not a todo # TDO006 - error # ToDo (evanrittenhouse): invalid capitalization # todo (evanrittenhouse): another invalid capitalization diff --git a/crates/ruff_linter/src/directives.rs b/crates/ruff_linter/src/directives.rs index 2972a3fe0e..43c28df171 100644 --- a/crates/ruff_linter/src/directives.rs +++ b/crates/ruff_linter/src/directives.rs @@ -287,19 +287,23 @@ impl<'a> TodoDirective<'a> { pub(crate) fn from_comment(comment: &'a str, comment_range: TextRange) -> Option { // The directive's offset from the start of the comment. let mut relative_offset = TextSize::new(0); - let mut subset_opt = Some(comment); + let mut subset = 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 { + loop { let trimmed = subset.trim_start_matches('#').trim_start(); let offset = subset.text_len() - trimmed.text_len(); relative_offset += offset; + // Find the first word. Don't use split by whitespace because that would include the `:` character + // in `TODO:` + let first_word = trimmed.split(|c: char| !c.is_alphanumeric()).next()?; + // If we detect a TodoDirectiveKind variant substring in the comment, construct and // return the appropriate TodoDirective - if let Ok(directive_kind) = trimmed.parse::() { + if let Ok(directive_kind) = first_word.parse::() { let len = directive_kind.len(); return Some(Self { @@ -310,11 +314,11 @@ impl<'a> TodoDirective<'a> { } // Shrink the subset to check for the next phrase starting with "#". - subset_opt = if let Some(new_offset) = trimmed.find('#') { + if let Some(new_offset) = trimmed.find('#') { relative_offset += TextSize::try_from(new_offset).unwrap(); - subset.get(relative_offset.to_usize()..) + subset = &subset[relative_offset.to_usize()..]; } else { - None + break; }; } @@ -334,30 +338,13 @@ impl FromStr for TodoDirectiveKind { type Err = (); fn from_str(s: &str) -> Result { - // 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, - } + match s.to_lowercase().as_str() { + "fixme" => Ok(TodoDirectiveKind::Fixme), + "hack" => Ok(TodoDirectiveKind::Hack), + "todo" => Ok(TodoDirectiveKind::Todo), + "xxx" => Ok(TodoDirectiveKind::Xxx), + _ => Err(()), } - - Err(()) } } diff --git a/crates/ruff_linter/src/rules/flake8_todos/snapshots/ruff_linter__rules__flake8_todos__tests__invalid-todo-capitalization_TD006.py.snap b/crates/ruff_linter/src/rules/flake8_todos/snapshots/ruff_linter__rules__flake8_todos__tests__invalid-todo-capitalization_TD006.py.snap index 32cbcb436d..7e5c3c1482 100644 --- a/crates/ruff_linter/src/rules/flake8_todos/snapshots/ruff_linter__rules__flake8_todos__tests__invalid-todo-capitalization_TD006.py.snap +++ b/crates/ruff_linter/src/rules/flake8_todos/snapshots/ruff_linter__rules__flake8_todos__tests__invalid-todo-capitalization_TD006.py.snap @@ -1,58 +1,56 @@ --- source: crates/ruff_linter/src/rules/flake8_todos/mod.rs --- -TD006.py:4:3: TD006 [*] Invalid TODO capitalization: `ToDo` should be `TODO` +TD006.py:5:3: TD006 [*] Invalid TODO capitalization: `ToDo` should be `TODO` | -2 | # TODO (evanrittenhouse): this is a valid TODO -3 | # TDO006 - error -4 | # ToDo (evanrittenhouse): invalid capitalization +3 | # Todoism is a word which starts with todo, but is not a todo +4 | # TDO006 - error +5 | # ToDo (evanrittenhouse): invalid capitalization | ^^^^ TD006 -5 | # todo (evanrittenhouse): another invalid capitalization -6 | # foo # todo: invalid capitalization +6 | # todo (evanrittenhouse): another invalid capitalization +7 | # foo # todo: invalid capitalization | = help: Replace `ToDo` with `TODO` ℹ Safe fix -1 1 | # TDO006 - accepted 2 2 | # TODO (evanrittenhouse): this is a valid TODO -3 3 | # TDO006 - error -4 |-# ToDo (evanrittenhouse): invalid capitalization - 4 |+# TODO (evanrittenhouse): invalid capitalization -5 5 | # todo (evanrittenhouse): another invalid capitalization -6 6 | # foo # todo: invalid capitalization +3 3 | # Todoism is a word which starts with todo, but is not a todo +4 4 | # TDO006 - error +5 |-# ToDo (evanrittenhouse): invalid capitalization + 5 |+# TODO (evanrittenhouse): invalid capitalization +6 6 | # todo (evanrittenhouse): another invalid capitalization +7 7 | # foo # todo: invalid capitalization -TD006.py:5:3: TD006 [*] Invalid TODO capitalization: `todo` should be `TODO` +TD006.py:6:3: TD006 [*] Invalid TODO capitalization: `todo` should be `TODO` | -3 | # TDO006 - error -4 | # ToDo (evanrittenhouse): invalid capitalization -5 | # todo (evanrittenhouse): another invalid capitalization +4 | # TDO006 - error +5 | # ToDo (evanrittenhouse): invalid capitalization +6 | # todo (evanrittenhouse): another invalid capitalization | ^^^^ TD006 -6 | # foo # todo: invalid capitalization +7 | # foo # todo: invalid capitalization | = help: Replace `todo` with `TODO` ℹ Safe fix -2 2 | # TODO (evanrittenhouse): this is a valid TODO -3 3 | # TDO006 - error -4 4 | # ToDo (evanrittenhouse): invalid capitalization -5 |-# todo (evanrittenhouse): another invalid capitalization - 5 |+# TODO (evanrittenhouse): another invalid capitalization -6 6 | # foo # todo: invalid capitalization +3 3 | # Todoism is a word which starts with todo, but is not a todo +4 4 | # TDO006 - error +5 5 | # ToDo (evanrittenhouse): invalid capitalization +6 |-# todo (evanrittenhouse): another invalid capitalization + 6 |+# TODO (evanrittenhouse): another invalid capitalization +7 7 | # foo # todo: invalid capitalization -TD006.py:6:9: TD006 [*] Invalid TODO capitalization: `todo` should be `TODO` +TD006.py:7:9: TD006 [*] Invalid TODO capitalization: `todo` should be `TODO` | -4 | # ToDo (evanrittenhouse): invalid capitalization -5 | # todo (evanrittenhouse): another invalid capitalization -6 | # foo # todo: invalid capitalization +5 | # ToDo (evanrittenhouse): invalid capitalization +6 | # todo (evanrittenhouse): another invalid capitalization +7 | # foo # todo: invalid capitalization | ^^^^ TD006 | = help: Replace `todo` with `TODO` ℹ Safe 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 - - +4 4 | # TDO006 - error +5 5 | # ToDo (evanrittenhouse): invalid capitalization +6 6 | # todo (evanrittenhouse): another invalid capitalization +7 |-# foo # todo: invalid capitalization + 7 |+# foo # TODO: invalid capitalization