From 2f53781a7761872cd0d8774376f257d98ca5d8f8 Mon Sep 17 00:00:00 2001 From: Evan Rittenhouse Date: Sat, 13 May 2023 09:19:06 -0500 Subject: [PATCH] Implement `flake8_todos` (#3921) --- LICENSE | 31 ++ README.md | 1 + .../test/fixtures/flake8_todos/TD001.py | 8 + .../test/fixtures/flake8_todos/TD002.py | 7 + .../test/fixtures/flake8_todos/TD003.py | 29 ++ .../test/fixtures/flake8_todos/TD004.py | 6 + .../test/fixtures/flake8_todos/TD005.py | 6 + .../test/fixtures/flake8_todos/TD006.py | 5 + .../test/fixtures/flake8_todos/TD007.py | 8 + crates/ruff/src/checkers/tokens.rs | 22 +- crates/ruff/src/codes.rs | 8 + crates/ruff/src/registry.rs | 20 +- crates/ruff/src/rules/flake8_todos/mod.rs | 31 ++ crates/ruff/src/rules/flake8_todos/rules.rs | 458 ++++++++++++++++++ ..._invalid-todo-capitalization_TD006.py.snap | 38 ++ ...dos__tests__invalid-todo-tag_TD001.py.snap | 20 + ...ssing-space-after-todo-colon_TD007.py.snap | 41 ++ ...__tests__missing-todo-author_TD002.py.snap | 31 ++ ...s__tests__missing-todo-colon_TD004.py.snap | 31 ++ ...ts__missing-todo-description_TD005.py.snap | 31 ++ ...os__tests__missing-todo-link_TD003.py.snap | 40 ++ crates/ruff/src/rules/mod.rs | 1 + docs/faq.md | 2 + ruff.schema.json | 10 + 24 files changed, 882 insertions(+), 3 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/flake8_todos/TD001.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_todos/TD002.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_todos/TD003.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_todos/TD004.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_todos/TD005.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_todos/TD006.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_todos/TD007.py create mode 100644 crates/ruff/src/rules/flake8_todos/mod.rs create mode 100644 crates/ruff/src/rules/flake8_todos/rules.rs create mode 100644 crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__invalid-todo-capitalization_TD006.py.snap create mode 100644 crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__invalid-todo-tag_TD001.py.snap create mode 100644 crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-space-after-todo-colon_TD007.py.snap create mode 100644 crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-author_TD002.py.snap create mode 100644 crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-colon_TD004.py.snap create mode 100644 crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-description_TD005.py.snap create mode 100644 crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-link_TD003.py.snap diff --git a/LICENSE b/LICENSE index 6a01371bb5..b98ccc9c78 100644 --- a/LICENSE +++ b/LICENSE @@ -354,6 +354,37 @@ are: SOFTWARE. """ +- flake8-todos, licensed as follows: + """ + Copyright (c) 2019 EclecticIQ. All rights reserved. + Copyright (c) 2020 Gram . All rights reserved. + + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions are met: + + 1. Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + + 2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + + 3. Neither the name of the copyright holder nor the names of its + contributors may be used to endorse or promote products derived from this + software without specific prior written permission. + + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE + FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + """ + - flake8-unused-arguments, licensed as follows: """ MIT License diff --git a/README.md b/README.md index e8a3e5721c..0579d377ea 100644 --- a/README.md +++ b/README.md @@ -279,6 +279,7 @@ quality tools, including: - [flake8-simplify](https://pypi.org/project/flake8-simplify/) - [flake8-super](https://pypi.org/project/flake8-super/) - [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports/) +- [flake8-todos](https://pypi.org/project/flake8-todos/) - [flake8-type-checking](https://pypi.org/project/flake8-type-checking/) - [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/) - [flynt](https://pypi.org/project/flynt/) ([#2102](https://github.com/charliermarsh/ruff/issues/2102)) diff --git a/crates/ruff/resources/test/fixtures/flake8_todos/TD001.py b/crates/ruff/resources/test/fixtures/flake8_todos/TD001.py new file mode 100644 index 0000000000..542db85c72 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_todos/TD001.py @@ -0,0 +1,8 @@ +# T001 - accepted +# TODO (evanrittenhouse): this is a valid TODO +# SOME_OTHER_TAG: this is impossible to determine +# this is not a TODO + +# T001 - errors +# XXX (evanrittenhouse): this is not fine +# FIXME (evanrittenhouse): this is not fine diff --git a/crates/ruff/resources/test/fixtures/flake8_todos/TD002.py b/crates/ruff/resources/test/fixtures/flake8_todos/TD002.py new file mode 100644 index 0000000000..bab0117611 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_todos/TD002.py @@ -0,0 +1,7 @@ +# T002 - accepted +# TODO (evanrittenhouse): this has an author +# TODO(evanrittenhouse): this also has an author +# T002 - errors +# TODO: this has no author +# FIXME: neither does this +# TODO : and neither does this diff --git a/crates/ruff/resources/test/fixtures/flake8_todos/TD003.py b/crates/ruff/resources/test/fixtures/flake8_todos/TD003.py new file mode 100644 index 0000000000..af00d6c82a --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_todos/TD003.py @@ -0,0 +1,29 @@ +# TDO003 - accepted +# TODO: this comment has a link +# https://github.com/charliermarsh/ruff/issues/3870 + +# TODO: this comment has an issue +# TDO-3870 + +# TDO003 - errors +# TODO: this comment has no +# link after it + +# TODO: here's a TODO with no link after it +def foo(x): + return x + +# TODO: here's a TODO on the last line with no link +# Here's more content. +# TDO-3870 + +# TODO: here's a TODO on the last line with no link +# Here's more content, with a space. + +# TDO-3870 + +# TODO: here's a TODO without an issue link +# TODO: followed by a new TODO with an issue link +# TDO-3870 + +# TODO: here's a TODO on the last line with no link diff --git a/crates/ruff/resources/test/fixtures/flake8_todos/TD004.py b/crates/ruff/resources/test/fixtures/flake8_todos/TD004.py new file mode 100644 index 0000000000..50dd9a78ad --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_todos/TD004.py @@ -0,0 +1,6 @@ +# T004 - accepted +# TODO(evanrittenhouse): this has a colon +# T004 - errors +# TODO this has no colon +# TODO(evanrittenhouse 😀) this has no colon +# FIXME add a colon diff --git a/crates/ruff/resources/test/fixtures/flake8_todos/TD005.py b/crates/ruff/resources/test/fixtures/flake8_todos/TD005.py new file mode 100644 index 0000000000..a16bb51de6 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_todos/TD005.py @@ -0,0 +1,6 @@ +# T005 - accepted +# TODO(evanrittenhouse): this has text, while the errors do not +# T005 - errors +# TODO(evanrittenhouse): +# TODO(evanrittenhouse) +# FIXME diff --git a/crates/ruff/resources/test/fixtures/flake8_todos/TD006.py b/crates/ruff/resources/test/fixtures/flake8_todos/TD006.py new file mode 100644 index 0000000000..3288e23f56 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_todos/TD006.py @@ -0,0 +1,5 @@ +# TDO006 - accepted +# TODO (evanrittenhouse): this is a valid TODO +# TDO006 - error +# ToDo (evanrittenhouse): invalid capitalization +# todo (evanrittenhouse): another invalid capitalization diff --git a/crates/ruff/resources/test/fixtures/flake8_todos/TD007.py b/crates/ruff/resources/test/fixtures/flake8_todos/TD007.py new file mode 100644 index 0000000000..7283dbb60a --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_todos/TD007.py @@ -0,0 +1,8 @@ +# T007 - accepted +# TODO(evanrittenhouse): this has a space after a colon +# TODO: so does this +# T007 - errors +# TODO(evanrittenhouse):this has no space after a colon +# TODO (evanrittenhouse):this doesn't either +# TODO:neither does this +# FIXME:and lastly neither does this diff --git a/crates/ruff/src/checkers/tokens.rs b/crates/ruff/src/checkers/tokens.rs index bf52d716ff..e935f9cc18 100644 --- a/crates/ruff/src/checkers/tokens.rs +++ b/crates/ruff/src/checkers/tokens.rs @@ -7,8 +7,8 @@ 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, pycodestyle, - pylint, pyupgrade, ruff, + eradicate, flake8_commas, flake8_implicit_str_concat, flake8_pyi, flake8_quotes, flake8_todos, + pycodestyle, pylint, pyupgrade, ruff, }; use crate::settings::Settings; use ruff_diagnostics::Diagnostic; @@ -59,6 +59,15 @@ 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); + let enforce_todos = settings.rules.any_enabled(&[ + Rule::InvalidTodoTag, + Rule::MissingTodoAuthor, + Rule::MissingTodoLink, + Rule::MissingTodoColon, + Rule::MissingTodoDescription, + Rule::InvalidTodoCapitalization, + Rule::MissingSpaceAfterTodoColon, + ]); // RUF001, RUF002, RUF003 if enforce_ambiguous_unicode_character { @@ -179,5 +188,14 @@ pub(crate) fn check_tokens( diagnostics.extend(flake8_pyi::rules::type_comment_in_stub(tokens)); } + // TD001, TD002, TD003, TD004, TD005, TD006, TD007 + if enforce_todos { + diagnostics.extend( + flake8_todos::rules::todos(tokens, settings) + .into_iter() + .filter(|diagnostic| settings.rules.enabled(diagnostic.kind.rule())), + ); + } + diagnostics } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 042aa9851d..3e0e17b352 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -743,6 +743,14 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { // Reserved: (Flynt, "001") => Rule::StringConcatenationToFString, (Flynt, "002") => Rule::StaticJoinToFString, + // flake8-todo + (Flake8Todo, "001") => Rule::InvalidTodoTag, + (Flake8Todo, "002") => Rule::MissingTodoAuthor, + (Flake8Todo, "003") => Rule::MissingTodoLink, + (Flake8Todo, "004") => Rule::MissingTodoColon, + (Flake8Todo, "005") => Rule::MissingTodoDescription, + (Flake8Todo, "006") => Rule::InvalidTodoCapitalization, + (Flake8Todo, "007") => Rule::MissingSpaceAfterTodoColon, _ => return None, }) } diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index cdacca52b1..7447f1c485 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -674,6 +674,14 @@ ruff_macros::register_rules!( rules::flake8_django::rules::DjangoNonLeadingReceiverDecorator, // flynt rules::flynt::rules::StaticJoinToFString, + // flake8-todo + rules::flake8_todos::rules::InvalidTodoTag, + rules::flake8_todos::rules::MissingTodoAuthor, + rules::flake8_todos::rules::MissingTodoLink, + rules::flake8_todos::rules::MissingTodoColon, + rules::flake8_todos::rules::MissingTodoDescription, + rules::flake8_todos::rules::InvalidTodoCapitalization, + rules::flake8_todos::rules::MissingSpaceAfterTodoColon, ); pub trait AsRule { @@ -814,6 +822,9 @@ pub enum Linter { /// [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/) #[prefix = "PTH"] Flake8UsePathlib, + /// [flake8-todos](https://github.com/orsinium-labs/flake8-todos/) + #[prefix = "TD"] + Flake8Todo, /// [eradicate](https://pypi.org/project/eradicate/) #[prefix = "ERA"] Eradicate, @@ -938,7 +949,14 @@ impl Rule { | Rule::UselessSemicolon | Rule::MultipleStatementsOnOneLineSemicolon | Rule::ProhibitedTrailingComma - | Rule::TypeCommentInStub => LintSource::Tokens, + | Rule::TypeCommentInStub + | Rule::InvalidTodoTag + | Rule::MissingTodoAuthor + | Rule::MissingTodoLink + | Rule::MissingTodoColon + | Rule::MissingTodoDescription + | Rule::InvalidTodoCapitalization + | Rule::MissingSpaceAfterTodoColon => LintSource::Tokens, Rule::IOError => LintSource::Io, Rule::UnsortedImports | Rule::MissingRequiredImport => LintSource::Imports, Rule::ImplicitNamespacePackage | Rule::InvalidModuleName => LintSource::Filesystem, diff --git a/crates/ruff/src/rules/flake8_todos/mod.rs b/crates/ruff/src/rules/flake8_todos/mod.rs new file mode 100644 index 0000000000..d95100d4ec --- /dev/null +++ b/crates/ruff/src/rules/flake8_todos/mod.rs @@ -0,0 +1,31 @@ +pub(crate) mod rules; + +#[cfg(test)] +mod tests { + use std::convert::AsRef; + 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::InvalidTodoTag, Path::new("TD001.py"); "TD001")] + #[test_case(Rule::MissingTodoAuthor, Path::new("TD002.py"); "TD002")] + #[test_case(Rule::MissingTodoLink, Path::new("TD003.py"); "TD003")] + #[test_case(Rule::MissingTodoColon, Path::new("TD004.py"); "TD004")] + #[test_case(Rule::MissingTodoDescription, Path::new("TD005.py"); "TD005")] + #[test_case(Rule::InvalidTodoCapitalization, Path::new("TD006.py"); "TD006")] + #[test_case(Rule::MissingSpaceAfterTodoColon, Path::new("TD007.py"); "TD007")] + fn rules(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy()); + let diagnostics = test_path( + Path::new("flake8_todos").join(path).as_path(), + &settings::Settings::for_rule(rule_code), + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } +} diff --git a/crates/ruff/src/rules/flake8_todos/rules.rs b/crates/ruff/src/rules/flake8_todos/rules.rs new file mode 100644 index 0000000000..e18ab476fe --- /dev/null +++ b/crates/ruff/src/rules/flake8_todos/rules.rs @@ -0,0 +1,458 @@ +use itertools::Itertools; +use once_cell::sync::Lazy; +use regex::RegexSet; +use ruff_text_size::{TextLen, TextRange, TextSize}; +use rustpython_parser::lexer::LexResult; +use rustpython_parser::Tok; + +use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::{registry::Rule, settings::Settings}; + +/// ## What it does +/// Checks that a TODO comment is labelled with "TODO". +/// +/// ## Why is this bad? +/// Ambiguous tags reduce code visibility and can lead to dangling TODOs. +/// For example, if a comment is tagged with "FIXME" rather than "TODO", it may +/// be overlooked by future readers. +/// +/// Note that this rule will only flag "FIXME" and "XXX" tags as incorrect. +/// +/// ## Example +/// ```python +/// # FIXME(ruff): this should get fixed! +/// ``` +/// +/// Use instead: +/// ```python +/// # TODO(ruff): this is now fixed! +/// ``` +#[violation] +pub struct InvalidTodoTag { + pub tag: String, +} + +impl Violation for InvalidTodoTag { + #[derive_message_formats] + fn message(&self) -> String { + let InvalidTodoTag { tag } = self; + format!("Invalid TODO tag: `{tag}`") + } +} + +/// ## What it does +/// Checks that a TODO comment includes an author. +/// +/// ## Why is this bad? +/// Including an author on a TODO provides future readers with context around +/// the issue. While the TODO author is not always considered responsible for +/// fixing the issue, they are typically the individual with the most context. +/// +/// ## Example +/// ```python +/// # TODO: should assign an author here +/// ``` +/// +/// Use instead +/// ```python +/// # TODO(charlie): now an author is assigned +/// ``` +#[violation] +pub struct MissingTodoAuthor; + +impl Violation for MissingTodoAuthor { + #[derive_message_formats] + fn message(&self) -> String { + format!("Missing author in TODO; try: `# TODO(): ...`") + } +} + +/// ## What it does +/// Checks that a TODO comment is associated with a link to a relevant issue +/// or ticket. +/// +/// ## Why is this bad? +/// Including an issue link near a TODO makes it easier for resolvers +/// to get context around the issue. +/// +/// ## Example +/// ```python +/// # TODO: this link has no issue +/// ``` +/// +/// Use one of these instead: +/// ```python +/// # TODO(charlie): this comment has an issue link +/// # https://github.com/charliermarsh/ruff/issues/3870 +/// +/// # TODO(charlie): this comment has a 3-digit issue code +/// # 003 +/// +/// # TODO(charlie): this comment has an issue code of (up to) 6 characters, then digits +/// # SIXCHR-003 +/// ``` +#[violation] +pub struct MissingTodoLink; + +impl Violation for MissingTodoLink { + #[derive_message_formats] + fn message(&self) -> String { + format!("Missing issue link on the line following this TODO") + } +} + +/// ## What it does +/// Checks that a "TODO" tag is followed by a colon. +/// +/// ## Why is this bad? +/// "TODO" tags are typically followed by a parenthesized author name, a colon, +/// a space, and a description of the issue, in that order. +/// +/// Deviating from this pattern can lead to inconsistent and non-idiomatic +/// comments. +/// +/// ## Example +/// ```python +/// # TODO(charlie) fix this colon +/// ``` +/// +/// Used instead: +/// ```python +/// # TODO(charlie): colon fixed +/// ``` +#[violation] +pub struct MissingTodoColon; + +impl Violation for MissingTodoColon { + #[derive_message_formats] + fn message(&self) -> String { + format!("Missing colon in TODO") + } +} + +/// ## What it does +/// Checks that a "TODO" tag contains a description of the issue following the +/// tag itself. +/// +/// ## Why is this bad? +/// TODO comments should include a description of the issue to provide context +/// for future readers. +/// +/// ## Example +/// ```python +/// # TODO(charlie) +/// ``` +/// +/// Use instead: +/// ```python +/// # TODO(charlie): fix some issue +/// ``` +#[violation] +pub struct MissingTodoDescription; + +impl Violation for MissingTodoDescription { + #[derive_message_formats] + fn message(&self) -> String { + format!("Missing issue description after `TODO`") + } +} + +/// ## What it does +/// Checks that a "TODO" tag is properly capitalized (i.e., that the tag is +/// uppercase). +/// +/// ## Why is this bad? +/// Capitalizing the "TODO" in a TODO comment is a convention that makes it +/// easier for future readers to identify TODOs. +/// +/// ## Example +/// ```python +/// # todo(charlie): capitalize this +/// ``` +/// +/// Use instead: +/// ```python +/// # TODO(charlie): this is capitalized +/// ``` +#[violation] +pub struct InvalidTodoCapitalization { + tag: String, +} + +impl AlwaysAutofixableViolation for InvalidTodoCapitalization { + #[derive_message_formats] + fn message(&self) -> String { + let InvalidTodoCapitalization { tag } = self; + format!("Invalid TODO capitalization: `{tag}` should be `TODO`") + } + + fn autofix_title(&self) -> String { + let InvalidTodoCapitalization { tag } = self; + format!("Replace `{tag}` with `TODO`") + } +} + +/// ## What it does +/// Checks that the colon after a "TODO" tag is followed by a space. +/// +/// ## Why is this bad? +/// "TODO" tags are typically followed by a parenthesized author name, a colon, +/// a space, and a description of the issue, in that order. +/// +/// Deviating from this pattern can lead to inconsistent and non-idiomatic +/// comments. +/// +/// ## Example +/// ```python +/// # TODO(charlie):fix this +/// ``` +/// +/// Use instead: +/// ```python +/// # TODO(charlie): fix this +/// ``` +#[violation] +pub struct MissingSpaceAfterTodoColon; + +impl Violation for MissingSpaceAfterTodoColon { + #[derive_message_formats] + fn message(&self) -> String { + format!("Missing space after colon in TODO") + } +} + +static TODO_REGEX_SET: Lazy = Lazy::new(|| { + RegexSet::new([ + r#"^#\s*(?i)(TODO).*$"#, + r#"^#\s*(?i)(FIXME).*$"#, + r#"^#\s*(?i)(XXX).*$"#, + ]) + .unwrap() +}); + +// Maps the index of a particular Regex (specified by its index in the above TODO_REGEX_SET slice) +// to the length of the tag that we're trying to capture. +static PATTERN_TAG_LENGTH: &[usize; 3] = &["TODO".len(), "FIXME".len(), "XXX".len()]; + +static ISSUE_LINK_REGEX_SET: Lazy = Lazy::new(|| { + let patterns: [&str; 3] = [ + r#"^#\s*(http|https)://.*"#, // issue link + r#"^#\s*\d+$"#, // issue code - like "003" + r#"^#\s*[A-Z]{1,6}\-?\d+$"#, // issue code - like "TD003" or "TD-003" + ]; + RegexSet::new(patterns).unwrap() +}); + +// If this struct ever gets pushed outside of this module, it may be worth creating an enum for +// the different tag types + other convenience methods. +/// Represents a TODO tag or any of its variants - FIXME, XXX, TODO. +#[derive(Debug, PartialEq, Eq)] +struct Tag<'a> { + range: TextRange, + content: &'a str, +} + +pub(crate) fn todos(tokens: &[LexResult], settings: &Settings) -> Vec { + let mut diagnostics: Vec = vec![]; + + let mut iter = tokens.iter().flatten().multipeek(); + while let Some((token, token_range)) = iter.next() { + let Tok::Comment(comment) = token else { + continue; + }; + + // Check that the comment is a TODO (properly formed or not). + let Some(tag) = detect_tag(comment, token_range) else { + continue; + }; + + tag_errors(&tag, &mut diagnostics, settings); + static_errors(&mut diagnostics, comment, *token_range, &tag); + + // TD003 + let mut has_issue_link = false; + while let Some((token, token_range)) = iter.peek() { + if let Tok::Comment(comment) = token { + if detect_tag(comment, token_range).is_some() { + break; + } + if ISSUE_LINK_REGEX_SET.is_match(comment) { + has_issue_link = true; + break; + } + } else { + break; + } + } + if !has_issue_link { + diagnostics.push(Diagnostic::new(MissingTodoLink, tag.range)); + } + } + + diagnostics +} + +/// Returns the tag pulled out of a given comment, if it exists. +fn detect_tag<'a>(comment: &'a str, comment_range: &'a TextRange) -> Option> { + let Some(regex_index) = TODO_REGEX_SET.matches(comment).into_iter().next() else { + return None; + }; + + let tag_length = PATTERN_TAG_LENGTH[regex_index]; + + let mut tag_start_offset = 0usize; + for (i, char) in comment.chars().enumerate() { + // Regex ensures that the first letter in the comment is the first letter of the tag. + if char.is_alphabetic() { + tag_start_offset = i; + break; + } + } + + Some(Tag { + content: &comment[tag_start_offset..tag_start_offset + tag_length], + range: TextRange::at( + comment_range.start() + TextSize::try_from(tag_start_offset).ok().unwrap(), + TextSize::try_from(tag_length).ok().unwrap(), + ), + }) +} + +/// Check that the tag itself is valid. This function modifies `diagnostics` in-place. +fn tag_errors(tag: &Tag, diagnostics: &mut Vec, settings: &Settings) { + if tag.content == "TODO" { + return; + } + + if tag.content.to_uppercase() == "TODO" { + // TD006 + let mut diagnostic = Diagnostic::new( + InvalidTodoCapitalization { + tag: tag.content.to_string(), + }, + tag.range, + ); + + if settings.rules.should_fix(Rule::InvalidTodoCapitalization) { + diagnostic.set_fix(Fix::automatic(Edit::range_replacement( + "TODO".to_string(), + tag.range, + ))); + } + + diagnostics.push(diagnostic); + } else { + // TD001 + diagnostics.push(Diagnostic::new( + InvalidTodoTag { + tag: tag.content.to_string(), + }, + tag.range, + )); + } +} + +/// Checks for "static" errors in the comment: missing colon, missing author, etc. This function +/// modifies `diagnostics` in-place. +fn static_errors( + diagnostics: &mut Vec, + comment: &str, + comment_range: TextRange, + tag: &Tag, +) { + let post_tag = &comment[usize::from(tag.range.end() - comment_range.start())..]; + let trimmed = post_tag.trim_start(); + let content_offset = post_tag.text_len() - trimmed.text_len(); + + let author_end = content_offset + + if trimmed.starts_with('(') { + if let Some(end_index) = trimmed.find(')') { + TextSize::try_from(end_index + 1).unwrap() + } else { + trimmed.text_len() + } + } else { + diagnostics.push(Diagnostic::new(MissingTodoAuthor, tag.range)); + + 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 { + diagnostics.push(Diagnostic::new(MissingSpaceAfterTodoColon, tag.range)); + after_colon + } + } else { + diagnostics.push(Diagnostic::new(MissingTodoColon, tag.range)); + "" + }; + + if post_colon.is_empty() { + diagnostics.push(Diagnostic::new(MissingTodoDescription, tag.range)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_detect_tag() { + let test_comment = "# TODO: todo tag"; + let expected = Tag { + content: "TODO", + range: TextRange::new(TextSize::new(2), TextSize::new(6)), + }; + assert_eq!( + Some(expected), + detect_tag( + test_comment, + &TextRange::new(TextSize::new(0), TextSize::new(15)), + ) + ); + + let test_comment = "#TODO: todo tag"; + let expected = Tag { + content: "TODO", + range: TextRange::new(TextSize::new(1), TextSize::new(5)), + }; + assert_eq!( + Some(expected), + detect_tag( + test_comment, + &TextRange::new(TextSize::new(0), TextSize::new(15)), + ) + ); + + let test_comment = "# todo: todo tag"; + let expected = Tag { + content: "todo", + range: TextRange::new(TextSize::new(2), TextSize::new(6)), + }; + assert_eq!( + Some(expected), + detect_tag( + test_comment, + &TextRange::new(TextSize::new(0), TextSize::new(15)), + ) + ); + let test_comment = "# fixme: fixme tag"; + let expected = Tag { + content: "fixme", + range: TextRange::new(TextSize::new(2), TextSize::new(7)), + }; + assert_eq!( + Some(expected), + detect_tag( + test_comment, + &TextRange::new(TextSize::new(0), TextSize::new(17)), + ) + ); + } +} diff --git a/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__invalid-todo-capitalization_TD006.py.snap b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__invalid-todo-capitalization_TD006.py.snap new file mode 100644 index 0000000000..7c87a41fc4 --- /dev/null +++ b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__invalid-todo-capitalization_TD006.py.snap @@ -0,0 +1,38 @@ +--- +source: crates/ruff/src/rules/flake8_todos/mod.rs +--- +TD006.py:4:3: TD006 [*] Invalid TODO capitalization: `ToDo` should be `TODO` + | +4 | # TODO (evanrittenhouse): this is a valid TODO +5 | # TDO006 - error +6 | # ToDo (evanrittenhouse): invalid capitalization + | ^^^^ TD006 +7 | # todo (evanrittenhouse): another invalid capitalization + | + = help: Replace `ToDo` with `TODO` + +ℹ 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 + +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 + | + = help: Replace `todo` with `TODO` + +ℹ 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 + + diff --git a/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__invalid-todo-tag_TD001.py.snap b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__invalid-todo-tag_TD001.py.snap new file mode 100644 index 0000000000..8631a8f2c7 --- /dev/null +++ b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__invalid-todo-tag_TD001.py.snap @@ -0,0 +1,20 @@ +--- +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 + | + +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 + | + + diff --git a/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-space-after-todo-colon_TD007.py.snap b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-space-after-todo-colon_TD007.py.snap new file mode 100644 index 0000000000..c569e2561b --- /dev/null +++ b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-space-after-todo-colon_TD007.py.snap @@ -0,0 +1,41 @@ +--- +source: crates/ruff/src/rules/flake8_todos/mod.rs +--- +TD007.py:5:3: TD007 Missing space after colon in TODO + | +5 | # TODO: so does this +6 | # T007 - errors +7 | # TODO(evanrittenhouse):this has no space after a colon + | ^^^^ TD007 +8 | # TODO (evanrittenhouse):this doesn't either +9 | # TODO:neither does this + | + +TD007.py:6:3: TD007 Missing space after colon in TODO + | + 6 | # T007 - errors + 7 | # TODO(evanrittenhouse):this has no space after a colon + 8 | # TODO (evanrittenhouse):this doesn't either + | ^^^^ TD007 + 9 | # TODO:neither does this +10 | # FIXME:and lastly neither does this + | + +TD007.py:7:3: TD007 Missing space after colon in TODO + | + 7 | # TODO(evanrittenhouse):this has no space after a colon + 8 | # TODO (evanrittenhouse):this doesn't either + 9 | # TODO:neither does this + | ^^^^ TD007 +10 | # FIXME:and lastly neither does this + | + +TD007.py:8:3: TD007 Missing space after colon in TODO + | + 8 | # TODO (evanrittenhouse):this doesn't either + 9 | # TODO:neither does this +10 | # FIXME:and lastly neither does this + | ^^^^^ TD007 + | + + diff --git a/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-author_TD002.py.snap b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-author_TD002.py.snap new file mode 100644 index 0000000000..4a97aebc03 --- /dev/null +++ b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-author_TD002.py.snap @@ -0,0 +1,31 @@ +--- +source: crates/ruff/src/rules/flake8_todos/mod.rs +--- +TD002.py:5:3: TD002 Missing author in TODO; try: `# TODO(): ...` + | +5 | # TODO(evanrittenhouse): this also has an author +6 | # T002 - errors +7 | # TODO: this has no author + | ^^^^ TD002 +8 | # FIXME: neither does this +9 | # TODO : and neither does this + | + +TD002.py:6:3: TD002 Missing author in TODO; try: `# TODO(): ...` + | +6 | # T002 - errors +7 | # TODO: this has no author +8 | # FIXME: neither does this + | ^^^^^ TD002 +9 | # TODO : and neither does this + | + +TD002.py:7:3: TD002 Missing author in TODO; try: `# TODO(): ...` + | +7 | # TODO: this has no author +8 | # FIXME: neither does this +9 | # TODO : and neither does this + | ^^^^ TD002 + | + + diff --git a/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-colon_TD004.py.snap b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-colon_TD004.py.snap new file mode 100644 index 0000000000..d470269deb --- /dev/null +++ b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-colon_TD004.py.snap @@ -0,0 +1,31 @@ +--- +source: crates/ruff/src/rules/flake8_todos/mod.rs +--- +TD004.py:4:3: TD004 Missing colon in TODO + | +4 | # TODO(evanrittenhouse): this has a colon +5 | # T004 - errors +6 | # TODO this has no colon + | ^^^^ TD004 +7 | # TODO(evanrittenhouse 😀) this has no colon +8 | # FIXME add a colon + | + +TD004.py:5:3: TD004 Missing colon in TODO + | +5 | # T004 - errors +6 | # TODO this has no colon +7 | # TODO(evanrittenhouse 😀) this has no colon + | ^^^^ TD004 +8 | # FIXME 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 + | + + diff --git a/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-description_TD005.py.snap b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-description_TD005.py.snap new file mode 100644 index 0000000000..c6bd1eb87a --- /dev/null +++ b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-description_TD005.py.snap @@ -0,0 +1,31 @@ +--- +source: crates/ruff/src/rules/flake8_todos/mod.rs +--- +TD005.py:4:3: TD005 Missing issue description after `TODO` + | +4 | # TODO(evanrittenhouse): this has text, while the errors do not +5 | # T005 - errors +6 | # TODO(evanrittenhouse): + | ^^^^ TD005 +7 | # TODO(evanrittenhouse) +8 | # FIXME + | + +TD005.py:5:3: TD005 Missing issue description after `TODO` + | +5 | # T005 - errors +6 | # TODO(evanrittenhouse): +7 | # TODO(evanrittenhouse) + | ^^^^ TD005 +8 | # FIXME + | + +TD005.py:6:3: TD005 Missing issue description after `TODO` + | +6 | # TODO(evanrittenhouse): +7 | # TODO(evanrittenhouse) +8 | # FIXME + | ^^^^^ TD005 + | + + diff --git a/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-link_TD003.py.snap b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-link_TD003.py.snap new file mode 100644 index 0000000000..421a9f9c01 --- /dev/null +++ b/crates/ruff/src/rules/flake8_todos/snapshots/ruff__rules__flake8_todos__tests__missing-todo-link_TD003.py.snap @@ -0,0 +1,40 @@ +--- +source: crates/ruff/src/rules/flake8_todos/mod.rs +--- +TD003.py:9:3: TD003 Missing issue link on the line following this TODO + | + 9 | # TDO003 - errors +10 | # TODO: this comment has no + | ^^^^ TD003 +11 | # link after it + | + +TD003.py:12:3: TD003 Missing issue link on the line following this TODO + | +12 | # link after it +13 | +14 | # TODO: here's a TODO with no link after it + | ^^^^ TD003 +15 | def foo(x): +16 | return x + | + +TD003.py:25:3: TD003 Missing issue link on the line following this TODO + | +25 | # TDO-3870 +26 | +27 | # TODO: here's a TODO without an issue link + | ^^^^ TD003 +28 | # TODO: followed by a new TODO with an issue link +29 | # TDO-3870 + | + +TD003.py:29:3: TD003 Missing issue link on the line following this TODO + | +29 | # TDO-3870 +30 | +31 | # TODO: here's a TODO on the last line with no link + | ^^^^ TD003 + | + + diff --git a/crates/ruff/src/rules/mod.rs b/crates/ruff/src/rules/mod.rs index bda3d832d4..fd268d02b9 100644 --- a/crates/ruff/src/rules/mod.rs +++ b/crates/ruff/src/rules/mod.rs @@ -29,6 +29,7 @@ pub mod flake8_return; pub mod flake8_self; pub mod flake8_simplify; pub mod flake8_tidy_imports; +pub mod flake8_todos; pub mod flake8_type_checking; pub mod flake8_unused_arguments; pub mod flake8_use_pathlib; diff --git a/docs/faq.md b/docs/faq.md index 475cffc3c5..19caa945b5 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -63,6 +63,7 @@ natively, including: - [flake8-simplify](https://pypi.org/project/flake8-simplify/) - [flake8-super](https://pypi.org/project/flake8-super/) - [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports/) +- [flake8-todos](https://pypi.org/project/flake8-todos/) - [flake8-type-checking](https://pypi.org/project/flake8-type-checking/) - [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/) - [flynt](https://pypi.org/project/flynt/) ([#2102](https://github.com/charliermarsh/ruff/issues/2102)) @@ -161,6 +162,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl - [flake8-simplify](https://pypi.org/project/flake8-simplify/) - [flake8-super](https://pypi.org/project/flake8-super/) - [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports/) +- [flake8-todos](https://pypi.org/project/flake8-todos/) - [flake8-type-checking](https://pypi.org/project/flake8-type-checking/) - [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/) - [flynt](https://pypi.org/project/flynt/) ([#2102](https://github.com/charliermarsh/ruff/issues/2102)) diff --git a/ruff.schema.json b/ruff.schema.json index c03f29a043..70f253b309 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2296,6 +2296,16 @@ "TCH003", "TCH004", "TCH005", + "TD", + "TD0", + "TD00", + "TD001", + "TD002", + "TD003", + "TD004", + "TD005", + "TD006", + "TD007", "TID", "TID2", "TID25",