From ab0156ba53181dd5ac34ee400f0645339e23bf69 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 11 Jul 2025 15:48:19 +0100 Subject: [PATCH 01/38] Creating new rule and template files --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/eslint/no_warning_comments.rs | 172 ++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 crates/oxc_linter/src/rules/eslint/no_warning_comments.rs diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 8a33679be59b9..07f4523776909 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -162,6 +162,7 @@ mod eslint { pub mod no_useless_rename; pub mod no_var; pub mod no_void; + pub mod no_warning_comments; pub mod no_with; pub mod operator_assignment; pub mod prefer_exponentiation_operator; @@ -594,6 +595,7 @@ oxc_macros::declare_all_lint_rules! { eslint::max_nested_callbacks, eslint::max_params, eslint::new_cap, + eslint::no_warning_comments, eslint::no_extra_bind, eslint::no_alert, eslint::no_array_constructor, diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs new file mode 100644 index 0000000000000..b95e89ff52df8 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -0,0 +1,172 @@ +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{ + AstNode, + context::LintContext, + fixer::{RuleFix, RuleFixer}, + rule::Rule, +}; + +fn no_warning_comments_diagnostic(span: Span) -> OxcDiagnostic { + // See for details + OxcDiagnostic::warn("Should be an imperative statement about what is wrong") + .with_help("Should be a command-like statement that tells the user how to fix the issue") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoWarningComments; + +// See for documentation details. +declare_oxc_lint!( + /// ### What it does + /// + /// Briefly describe the rule's purpose. + /// + /// ### Why is this bad? + /// + /// Explain why violating this rule is problematic. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// FIXME: Tests will fail if examples are missing or syntactically incorrect. + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// FIXME: Tests will fail if examples are missing or syntactically incorrect. + /// ``` + NoWarningComments, + eslint, + nursery, // TODO: change category to `correctness`, `suspicious`, `pedantic`, `perf`, `restriction`, or `style` + // See for details + pending // TODO: describe fix capabilities. Remove if no fix can be done, + // keep at 'pending' if you think one could be added but don't know how. + // Options are 'fix', 'fix_dangerous', 'suggestion', and 'conditional_fix_suggestion' +); + +impl Rule for NoWarningComments { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + // ctx.diagnostic( + // OxcDiagnostic::warn("Warning comments should be avoided") + // .with_help("Use a command-like statement that tells the user how to fix the issue") + // ) + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("// any comment", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + ("// any comment", None), + ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// any comment with TODO, FIXME or XXX", + Some(serde_json::json!([{ "location": "start" }])), + ), + ("// any comment with TODO, FIXME or XXX", None), + ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + ("/* any block comment */", None), + ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "/* any block comment with TODO, FIXME or XXX */", + Some(serde_json::json!([{ "location": "start" }])), + ), + ("/* any block comment with TODO, FIXME or XXX */", None), + ("/* any block comment with (TODO, FIXME's or XXX!) */", None), + ( + "// comments containing terms as substrings like TodoMVC", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// special regex characters don't cause a problem", + Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), + ), + ( + r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + + var x = 10; + "#, + None, + ), + ( + r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + + var x = 10; + "#, + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), + ( + "/** multi-line block comment with lines starting with + TODO + FIXME or + XXX + */", + None, + ), + ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), + ]; + + let fail = vec![ + ("// fixme", None), +("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), +("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), +("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), +("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), +("/* any fixme */", Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }]))), +("/* any FIXME */", Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }]))), +("/* any fIxMe */", Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }]))), +("// any fixme or todo", Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }]))), +("/* any fixme or todo */", Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }]))), +("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), +("/* fixme and todo */", None), +("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), +("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), +("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), +("// regex [litera|$]", Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }]))), +("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), +("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }]))), +("/* any block comment with TODO, FIXME or XXX */", Some(serde_json::json!([{ "location": "anywhere" }]))), +("/* any block comment with (TODO, FIXME's or XXX!) */", Some(serde_json::json!([{ "location": "anywhere" }]))), +("/** + *any block comment + *with (TODO, FIXME's or XXX!) **/", Some(serde_json::json!([{ "location": "anywhere" }]))), +("// any comment with TODO, FIXME or XXX", Some(serde_json::json!([{ "location": "anywhere" }]))), +("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), +("// TODO: something really longer than 40 characters", Some(serde_json::json!([{ "location": "anywhere" }]))), +("/* TODO: something + really longer than 40 characters + and also a new line */", Some(serde_json::json!([{ "location": "anywhere" }]))), +("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), +("// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", Some(serde_json::json!([{ "location": "anywhere" }]))), +("// Comment ending with term followed by punctuation TODO!", Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }]))), +("// Comment ending with term including punctuation TODO!", Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }]))), +("// Comment ending with term including punctuation followed by more TODO!!!", Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }]))), +("// !TODO comment starting with term preceded by punctuation", Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }]))), +("// !TODO comment starting with term including punctuation", Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }]))), +("// !!!TODO comment starting with term including punctuation preceded by more", Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }]))), +("// FIX!term ending with punctuation followed word character", Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }]))), +("// Term starting with punctuation preceded word character!FIX", Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }]))), +("//!XXX comment starting with no spaces (anywhere)", Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }]))), +("//!XXX comment starting with no spaces (start)", Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }]))), +("/* + TODO undecorated multi-line block comment (start) + */", Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }]))), +("///// TODO decorated single-line comment with decoration array + /////", Some(serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]))), +("///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + /////", Some(serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]))), +("//**TODO term starts with a decoration character", Some(serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]))) + ]; + + Tester::new(NoWarningComments::NAME, NoWarningComments::PLUGIN, pass, fail).test_and_snapshot(); +} From b9a7bcce74a4424a6ac2882926631663edbea032 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 11 Jul 2025 15:51:52 +0100 Subject: [PATCH 02/38] Ignore the test for now. --- .../src/rules/eslint/no_warning_comments.rs | 181 +++++++++++++----- 1 file changed, 136 insertions(+), 45 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index b95e89ff52df8..da1ad995bc6ad 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -58,6 +58,7 @@ impl Rule for NoWarningComments { } } +#[ignore] #[test] fn test() { use crate::tester::Tester; @@ -118,54 +119,144 @@ fn test() { let fail = vec![ ("// fixme", None), -("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), -("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), -("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), -("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), -("/* any fixme */", Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }]))), -("/* any FIXME */", Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }]))), -("/* any fIxMe */", Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }]))), -("// any fixme or todo", Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }]))), -("/* any fixme or todo */", Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }]))), -("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), -("/* fixme and todo */", None), -("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), -("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), -("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), -("// regex [litera|$]", Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }]))), -("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), -("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }]))), -("/* any block comment with TODO, FIXME or XXX */", Some(serde_json::json!([{ "location": "anywhere" }]))), -("/* any block comment with (TODO, FIXME's or XXX!) */", Some(serde_json::json!([{ "location": "anywhere" }]))), -("/** + ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ( + "/* any fixme */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "/* any FIXME */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "/* any fIxMe */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "// any fixme or todo", + Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + ), + ( + "/* any fixme or todo */", + Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + ), + ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* fixme and todo */", None), + ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + ( + "// regex [litera|$]", + Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), + ), + ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), + ( + "/* eslint one-var: 2 */", + Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), + ), + ( + "/* any block comment with TODO, FIXME or XXX */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/* any block comment with (TODO, FIXME's or XXX!) */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/** *any block comment - *with (TODO, FIXME's or XXX!) **/", Some(serde_json::json!([{ "location": "anywhere" }]))), -("// any comment with TODO, FIXME or XXX", Some(serde_json::json!([{ "location": "anywhere" }]))), -("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), -("// TODO: something really longer than 40 characters", Some(serde_json::json!([{ "location": "anywhere" }]))), -("/* TODO: something + *with (TODO, FIXME's or XXX!) **/", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "// any comment with TODO, FIXME or XXX", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// TODO: something really longer than 40 characters", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/* TODO: something really longer than 40 characters - and also a new line */", Some(serde_json::json!([{ "location": "anywhere" }]))), -("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), -("// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", Some(serde_json::json!([{ "location": "anywhere" }]))), -("// Comment ending with term followed by punctuation TODO!", Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }]))), -("// Comment ending with term including punctuation TODO!", Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }]))), -("// Comment ending with term including punctuation followed by more TODO!!!", Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }]))), -("// !TODO comment starting with term preceded by punctuation", Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }]))), -("// !TODO comment starting with term including punctuation", Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }]))), -("// !!!TODO comment starting with term including punctuation preceded by more", Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }]))), -("// FIX!term ending with punctuation followed word character", Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }]))), -("// Term starting with punctuation preceded word character!FIX", Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }]))), -("//!XXX comment starting with no spaces (anywhere)", Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }]))), -("//!XXX comment starting with no spaces (start)", Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }]))), -("/* + and also a new line */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "// Comment ending with term followed by punctuation TODO!", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// Comment ending with term including punctuation TODO!", + Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + ), + ( + "// Comment ending with term including punctuation followed by more TODO!!!", + Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + ), + ( + "// !TODO comment starting with term preceded by punctuation", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// !TODO comment starting with term including punctuation", + Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + ), + ( + "// !!!TODO comment starting with term including punctuation preceded by more", + Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + ), + ( + "// FIX!term ending with punctuation followed word character", + Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), + ), + ( + "// Term starting with punctuation preceded word character!FIX", + Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), + ), + ( + "//!XXX comment starting with no spaces (anywhere)", + Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), + ), + ( + "//!XXX comment starting with no spaces (start)", + Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), + ), + ( + "/* TODO undecorated multi-line block comment (start) - */", Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }]))), -("///// TODO decorated single-line comment with decoration array - /////", Some(serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]))), -("///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - /////", Some(serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]))), -("//**TODO term starts with a decoration character", Some(serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]))) + */", + Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), + ), + ( + "///// TODO decorated single-line comment with decoration array + /////", + Some( + serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + ), + ), + ( + "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + /////", + Some( + serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + ), + ), + ( + "//**TODO term starts with a decoration character", + Some( + serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), + ), + ), ]; Tester::new(NoWarningComments::NAME, NoWarningComments::PLUGIN, pass, fail).test_and_snapshot(); From c5d3b6434a68c9fe520682cf18cee85ac46c561e Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 11 Jul 2025 15:54:31 +0100 Subject: [PATCH 03/38] Remove unnecessary imports --- .../src/rules/eslint/no_warning_comments.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index da1ad995bc6ad..b8e13aca864d2 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -1,21 +1,12 @@ -use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; -use oxc_span::Span; use crate::{ - AstNode, context::LintContext, - fixer::{RuleFix, RuleFixer}, - rule::Rule, + rule::Rule + , + AstNode, }; -fn no_warning_comments_diagnostic(span: Span) -> OxcDiagnostic { - // See for details - OxcDiagnostic::warn("Should be an imperative statement about what is wrong") - .with_help("Should be a command-like statement that tells the user how to fix the issue") - .with_label(span) -} - #[derive(Debug, Default, Clone)] pub struct NoWarningComments; @@ -50,7 +41,7 @@ declare_oxc_lint!( ); impl Rule for NoWarningComments { - fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + fn run<'a>(&self, _node: &AstNode<'a>, _ctx: &LintContext<'a>) { // ctx.diagnostic( // OxcDiagnostic::warn("Warning comments should be avoided") // .with_help("Use a command-like statement that tells the user how to fix the issue") From c6c1e457cf4f554a738e493dbd0e81ea78a9159a Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 11 Jul 2025 15:55:52 +0100 Subject: [PATCH 04/38] Remove unnecessary imports --- crates/oxc_linter/src/rules/eslint/no_warning_comments.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index b8e13aca864d2..4a888e64a6675 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -1,11 +1,6 @@ use oxc_macros::declare_oxc_lint; -use crate::{ - context::LintContext, - rule::Rule - , - AstNode, -}; +use crate::{AstNode, context::LintContext, rule::Rule}; #[derive(Debug, Default, Clone)] pub struct NoWarningComments; From 161801c1544eba913d03866a832b1c23b0242ab8 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 11 Jul 2025 20:27:31 +0100 Subject: [PATCH 05/38] feat(linter): enhance NoWarningComments rule with regex matching for warning terms --- .../src/rules/eslint/no_warning_comments.rs | 90 +++++++++++++++---- 1 file changed, 72 insertions(+), 18 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 4a888e64a6675..80d2bd0d9e146 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -1,3 +1,6 @@ +#![allow(dead_code)] + +use lazy_regex::regex; use oxc_macros::declare_oxc_lint; use crate::{AstNode, context::LintContext, rule::Rule}; @@ -35,16 +38,67 @@ declare_oxc_lint!( // Options are 'fix', 'fix_dangerous', 'suggestion', and 'conditional_fix_suggestion' ); +const WARNING_TERMS: [&str; 3] = ["FIXME", "TODO", "xxx"]; + +fn convert_to_regexp(term: &str) -> regex::Regex { + // Decorators are hard-coded here. Read them from config. + let escaped_decoration = regex::escape(&["*", "/"].join("")); + let escaped = regex::escape(term); + let word_boundary = "\\b"; + + // "location": optional string that configures where in your comments to + // check for matches. Defaults to "start". + // The start is from the first non-decorative character, ignoring whitespace, + // new lines and characters specified in decoration. + // The other value is match anywhere in comments. + // TODO: We need to check the location here and assign the prefix conditionally. I've omitted it here for now. + + let prefix = format!("^[\\s{escaped_decoration}]*"); + // The regex crate does not support inline flags like /u, so we use RegexBuilder below. + let re = regex::RegexBuilder::new(r"/\\w$/").unicode(true).build().unwrap(); + let suffix = if re.is_match(term) { word_boundary } else { "" }; + /* + * For location "start", the typical regex is: + * /^[\s]*ESCAPED_TERM\b/iu. + * Or if decoration characters are specified (e.g. "*"), then any of + * those characters may appear in any order at the start: + * /^[\s\*]*ESCAPED_TERM\b/iu. + * + * For location "anywhere" the typical regex is + * /\bESCAPED_TERM\b/iu + * + * If it starts or ends with non-word character, the prefix and suffix are empty, respectively. + */ + regex::RegexBuilder::new(&format!("{prefix}{escaped}{suffix}")) + .case_insensitive(true) // for 'i' + .unicode(true) // for 'u' + .build() + .unwrap() +} + +fn _comment_contains_warning_term(comment: &str) -> Vec<&str> { + let mut matches: Vec<&str> = vec![]; + for (index, term) in WARNING_TERMS.iter().enumerate() { + let re = convert_to_regexp(term); + if re.is_match(comment) { + matches.push(WARNING_TERMS[index]); + } + } + matches +} + impl Rule for NoWarningComments { fn run<'a>(&self, _node: &AstNode<'a>, _ctx: &LintContext<'a>) { + // ctx.comments().iter().for_each(|comment| { + // println!("{comment:?}"); + // }) // ctx.diagnostic( - // OxcDiagnostic::warn("Warning comments should be avoided") + // OxcDiagnostic::warn("Warning comments shou`ld be avoided") // .with_help("Use a command-like statement that tells the user how to fix the issue") // ) } } -#[ignore] #[test] fn test() { use crate::tester::Tester; @@ -80,24 +134,24 @@ fn test() { ( r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - var x = 10; - "#, + var x = 10; + "#, None, ), ( r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - var x = 10; - "#, + var x = 10; + "#, Some(serde_json::json!([{ "location": "anywhere" }])), ), ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), ( "/** multi-line block comment with lines starting with - TODO - FIXME or - XXX - */", + TODO + FIXME or + XXX + */", None, ), ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), @@ -153,8 +207,8 @@ fn test() { ), ( "/** - *any block comment - *with (TODO, FIXME's or XXX!) **/", + *any block comment + *with (TODO, FIXME's or XXX!) **/", Some(serde_json::json!([{ "location": "anywhere" }])), ), ( @@ -168,8 +222,8 @@ fn test() { ), ( "/* TODO: something - really longer than 40 characters - and also a new line */", + really longer than 40 characters + and also a new line */", Some(serde_json::json!([{ "location": "anywhere" }])), ), ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), @@ -219,20 +273,20 @@ fn test() { ), ( "/* - TODO undecorated multi-line block comment (start) - */", + TODO undecorated multi-line block comment (start) + */", Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), ), ( "///// TODO decorated single-line comment with decoration array - /////", + /////", Some( serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), ), ), ( "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - /////", + /////", Some( serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), ), From db1a746eeaef0c7b0d8a54104886cada261a964c Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 11 Jul 2025 20:27:53 +0100 Subject: [PATCH 06/38] test: ignore the test for NoWarningComments rule --- crates/oxc_linter/src/rules/eslint/no_warning_comments.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 80d2bd0d9e146..9755f3a25148f 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -99,6 +99,7 @@ impl Rule for NoWarningComments { } } +#[ignore] #[test] fn test() { use crate::tester::Tester; From df75c65379ca8fb3a32b6a69023c7a554b181fa3 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Sun, 13 Jul 2025 09:39:03 +0100 Subject: [PATCH 07/38] feat(linter): update NoWarningComments rule to process source comments --- .../src/rules/eslint/no_warning_comments.rs | 387 +++++++++--------- 1 file changed, 196 insertions(+), 191 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 9755f3a25148f..4eb9d581f951d 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -88,10 +88,15 @@ fn _comment_contains_warning_term(comment: &str) -> Vec<&str> { } impl Rule for NoWarningComments { - fn run<'a>(&self, _node: &AstNode<'a>, _ctx: &LintContext<'a>) { - // ctx.comments().iter().for_each(|comment| { - // println!("{comment:?}"); - // }) + fn run<'a>(&self, _node: &AstNode<'a>, ctx: &LintContext<'a>) { + ctx.comments().iter().for_each(|comment| { + let span = comment.span; + if let Some(_source_comment) = + ctx.source_text().get((span.start as usize)..(span.end as usize)) + { + // TODO: Process the source_comment here. + } + }); // ctx.diagnostic( // OxcDiagnostic::warn("Warning comments shou`ld be avoided") // .with_help("Use a command-like statement that tells the user how to fix the issue") @@ -106,198 +111,198 @@ fn test() { let pass = vec![ ("// any comment", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - ("// any comment", None), - ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "// any comment with TODO, FIXME or XXX", - Some(serde_json::json!([{ "location": "start" }])), - ), - ("// any comment with TODO, FIXME or XXX", None), - ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - ("/* any block comment */", None), - ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "/* any block comment with TODO, FIXME or XXX */", - Some(serde_json::json!([{ "location": "start" }])), - ), - ("/* any block comment with TODO, FIXME or XXX */", None), - ("/* any block comment with (TODO, FIXME's or XXX!) */", None), - ( - "// comments containing terms as substrings like TodoMVC", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// special regex characters don't cause a problem", - Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), - ), - ( - r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + // ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + // ("// any comment", None), + // ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "// any comment with TODO, FIXME or XXX", + // Some(serde_json::json!([{ "location": "start" }])), + // ), + // ("// any comment with TODO, FIXME or XXX", None), + // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + // ("/* any block comment */", None), + // ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "/* any block comment with TODO, FIXME or XXX */", + // Some(serde_json::json!([{ "location": "start" }])), + // ), + // ("/* any block comment with TODO, FIXME or XXX */", None), + // ("/* any block comment with (TODO, FIXME's or XXX!) */", None), + // ( + // "// comments containing terms as substrings like TodoMVC", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// special regex characters don't cause a problem", + // Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), + // ), + // ( + // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - var x = 10; - "#, - None, - ), - ( - r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + // var x = 10; + // "#, + // None, + // ), + // ( + // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - var x = 10; - "#, - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), - ( - "/** multi-line block comment with lines starting with - TODO - FIXME or - XXX - */", - None, - ), - ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), + // var x = 10; + // "#, + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), + // ( + // "/** multi-line block comment with lines starting with + // TODO + // FIXME or + // XXX + // */", + // None, + // ), + // ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; let fail = vec![ - ("// fixme", None), - ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - ( - "/* any fixme */", - Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - ), - ( - "/* any FIXME */", - Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - ), - ( - "/* any fIxMe */", - Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - ), - ( - "// any fixme or todo", - Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - ), - ( - "/* any fixme or todo */", - Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - ), - ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("/* fixme and todo */", None), - ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - ( - "// regex [litera|$]", - Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), - ), - ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), - ( - "/* eslint one-var: 2 */", - Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), - ), - ( - "/* any block comment with TODO, FIXME or XXX */", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "/* any block comment with (TODO, FIXME's or XXX!) */", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "/** - *any block comment - *with (TODO, FIXME's or XXX!) **/", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "// any comment with TODO, FIXME or XXX", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "// TODO: something really longer than 40 characters", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "/* TODO: something - really longer than 40 characters - and also a new line */", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "// Comment ending with term followed by punctuation TODO!", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// Comment ending with term including punctuation TODO!", - Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - ), - ( - "// Comment ending with term including punctuation followed by more TODO!!!", - Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - ), - ( - "// !TODO comment starting with term preceded by punctuation", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// !TODO comment starting with term including punctuation", - Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - ), - ( - "// !!!TODO comment starting with term including punctuation preceded by more", - Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - ), - ( - "// FIX!term ending with punctuation followed word character", - Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), - ), - ( - "// Term starting with punctuation preceded word character!FIX", - Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), - ), - ( - "//!XXX comment starting with no spaces (anywhere)", - Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), - ), - ( - "//!XXX comment starting with no spaces (start)", - Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), - ), - ( - "/* - TODO undecorated multi-line block comment (start) - */", - Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), - ), - ( - "///// TODO decorated single-line comment with decoration array - /////", - Some( - serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - ), - ), - ( - "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - /////", - Some( - serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - ), - ), - ( - "//**TODO term starts with a decoration character", - Some( - serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), - ), - ), + // ("// fixme", None), + // ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + // ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + // ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + // ( + // "/* any fixme */", + // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + // ), + // ( + // "/* any FIXME */", + // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + // ), + // ( + // "/* any fIxMe */", + // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + // ), + // ( + // "// any fixme or todo", + // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + // ), + // ( + // "/* any fixme or todo */", + // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + // ), + // ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("/* fixme and todo */", None), + // ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + // ( + // "// regex [litera|$]", + // Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), + // ), + // ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), + // ( + // "/* eslint one-var: 2 */", + // Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), + // ), + // ( + // "/* any block comment with TODO, FIXME or XXX */", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "/* any block comment with (TODO, FIXME's or XXX!) */", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "/** + // *any block comment + // *with (TODO, FIXME's or XXX!) **/", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "// any comment with TODO, FIXME or XXX", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "// TODO: something really longer than 40 characters", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "/* TODO: something + // really longer than 40 characters + // and also a new line */", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "// Comment ending with term followed by punctuation TODO!", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// Comment ending with term including punctuation TODO!", + // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + // ), + // ( + // "// Comment ending with term including punctuation followed by more TODO!!!", + // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + // ), + // ( + // "// !TODO comment starting with term preceded by punctuation", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// !TODO comment starting with term including punctuation", + // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + // ), + // ( + // "// !!!TODO comment starting with term including punctuation preceded by more", + // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + // ), + // ( + // "// FIX!term ending with punctuation followed word character", + // Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), + // ), + // ( + // "// Term starting with punctuation preceded word character!FIX", + // Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), + // ), + // ( + // "//!XXX comment starting with no spaces (anywhere)", + // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), + // ), + // ( + // "//!XXX comment starting with no spaces (start)", + // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), + // ), + // ( + // "/* + // TODO undecorated multi-line block comment (start) + // */", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), + // ), + // ( + // "///// TODO decorated single-line comment with decoration array + // /////", + // Some( + // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + // ), + // ), + // ( + // "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + // /////", + // Some( + // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + // ), + // ), + // ( + // "//**TODO term starts with a decoration character", + // Some( + // serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), + // ), + // ), ]; Tester::new(NoWarningComments::NAME, NoWarningComments::PLUGIN, pass, fail).test_and_snapshot(); From 9c5811c9acb14477a09899ffa25587eaf6fb2ff4 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Sun, 13 Jul 2025 09:41:59 +0100 Subject: [PATCH 08/38] test: update test cases for NoWarningComments rule to include additional scenarios --- .../src/rules/eslint/no_warning_comments.rs | 374 +++++++++--------- 1 file changed, 187 insertions(+), 187 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 4eb9d581f951d..a63ce5a5af598 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -111,198 +111,198 @@ fn test() { let pass = vec![ ("// any comment", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - // ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - // ("// any comment", None), - // ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "// any comment with TODO, FIXME or XXX", - // Some(serde_json::json!([{ "location": "start" }])), - // ), - // ("// any comment with TODO, FIXME or XXX", None), - // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - // ("/* any block comment */", None), - // ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "/* any block comment with TODO, FIXME or XXX */", - // Some(serde_json::json!([{ "location": "start" }])), - // ), - // ("/* any block comment with TODO, FIXME or XXX */", None), - // ("/* any block comment with (TODO, FIXME's or XXX!) */", None), - // ( - // "// comments containing terms as substrings like TodoMVC", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// special regex characters don't cause a problem", - // Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), - // ), - // ( - // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + ("// any comment", None), + ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// any comment with TODO, FIXME or XXX", + Some(serde_json::json!([{ "location": "start" }])), + ), + ("// any comment with TODO, FIXME or XXX", None), + ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + ("/* any block comment */", None), + ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "/* any block comment with TODO, FIXME or XXX */", + Some(serde_json::json!([{ "location": "start" }])), + ), + ("/* any block comment with TODO, FIXME or XXX */", None), + ("/* any block comment with (TODO, FIXME's or XXX!) */", None), + ( + "// comments containing terms as substrings like TodoMVC", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// special regex characters don't cause a problem", + Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), + ), + ( + r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - // var x = 10; - // "#, - // None, - // ), - // ( - // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + var x = 10; + "#, + None, + ), + ( + r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - // var x = 10; - // "#, - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), - // ( - // "/** multi-line block comment with lines starting with - // TODO - // FIXME or - // XXX - // */", - // None, - // ), - // ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), + var x = 10; + "#, + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), + ( + "/** multi-line block comment with lines starting with + TODO + FIXME or + XXX + */", + None, + ), + ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; let fail = vec![ - // ("// fixme", None), - // ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - // ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - // ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - // ( - // "/* any fixme */", - // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - // ), - // ( - // "/* any FIXME */", - // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - // ), - // ( - // "/* any fIxMe */", - // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - // ), - // ( - // "// any fixme or todo", - // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - // ), - // ( - // "/* any fixme or todo */", - // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - // ), - // ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("/* fixme and todo */", None), - // ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - // ( - // "// regex [litera|$]", - // Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), - // ), - // ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), - // ( - // "/* eslint one-var: 2 */", - // Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), - // ), - // ( - // "/* any block comment with TODO, FIXME or XXX */", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "/* any block comment with (TODO, FIXME's or XXX!) */", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "/** - // *any block comment - // *with (TODO, FIXME's or XXX!) **/", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "// any comment with TODO, FIXME or XXX", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "// TODO: something really longer than 40 characters", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "/* TODO: something - // really longer than 40 characters - // and also a new line */", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "// Comment ending with term followed by punctuation TODO!", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// Comment ending with term including punctuation TODO!", - // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - // ), - // ( - // "// Comment ending with term including punctuation followed by more TODO!!!", - // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - // ), - // ( - // "// !TODO comment starting with term preceded by punctuation", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// !TODO comment starting with term including punctuation", - // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - // ), - // ( - // "// !!!TODO comment starting with term including punctuation preceded by more", - // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - // ), - // ( - // "// FIX!term ending with punctuation followed word character", - // Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), - // ), - // ( - // "// Term starting with punctuation preceded word character!FIX", - // Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), - // ), - // ( - // "//!XXX comment starting with no spaces (anywhere)", - // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), - // ), - // ( - // "//!XXX comment starting with no spaces (start)", - // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), - // ), - // ( - // "/* - // TODO undecorated multi-line block comment (start) - // */", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), - // ), - // ( - // "///// TODO decorated single-line comment with decoration array - // /////", - // Some( - // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - // ), - // ), - // ( - // "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - // /////", - // Some( - // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - // ), - // ), - // ( - // "//**TODO term starts with a decoration character", - // Some( - // serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), - // ), - // ), + ("// fixme", None), + ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ( + "/* any fixme */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "/* any FIXME */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "/* any fIxMe */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "// any fixme or todo", + Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + ), + ( + "/* any fixme or todo */", + Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + ), + ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* fixme and todo */", None), + ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + ( + "// regex [litera|$]", + Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), + ), + ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), + ( + "/* eslint one-var: 2 */", + Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), + ), + ( + "/* any block comment with TODO, FIXME or XXX */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/* any block comment with (TODO, FIXME's or XXX!) */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/** + *any block comment + *with (TODO, FIXME's or XXX!) **/", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "// any comment with TODO, FIXME or XXX", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// TODO: something really longer than 40 characters", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/* TODO: something + really longer than 40 characters + and also a new line */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "// Comment ending with term followed by punctuation TODO!", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// Comment ending with term including punctuation TODO!", + Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + ), + ( + "// Comment ending with term including punctuation followed by more TODO!!!", + Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + ), + ( + "// !TODO comment starting with term preceded by punctuation", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// !TODO comment starting with term including punctuation", + Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + ), + ( + "// !!!TODO comment starting with term including punctuation preceded by more", + Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + ), + ( + "// FIX!term ending with punctuation followed word character", + Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), + ), + ( + "// Term starting with punctuation preceded word character!FIX", + Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), + ), + ( + "//!XXX comment starting with no spaces (anywhere)", + Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), + ), + ( + "//!XXX comment starting with no spaces (start)", + Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), + ), + ( + "/* + TODO undecorated multi-line block comment (start) + */", + Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), + ), + ( + "///// TODO decorated single-line comment with decoration array + /////", + Some( + serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + ), + ), + ( + "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + /////", + Some( + serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + ), + ), + ( + "//**TODO term starts with a decoration character", + Some( + serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), + ), + ), ]; Tester::new(NoWarningComments::NAME, NoWarningComments::PLUGIN, pass, fail).test_and_snapshot(); From d904c2e548db82aa0b27064b98cf489b70566ce5 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Sun, 13 Jul 2025 09:46:43 +0100 Subject: [PATCH 09/38] docs: add links to ESLint rule documentation for clarity --- .../src/rules/eslint/no_warning_comments.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index a63ce5a5af598..7e756f8b040f8 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -40,6 +40,7 @@ declare_oxc_lint!( const WARNING_TERMS: [&str; 3] = ["FIXME", "TODO", "xxx"]; +/// https://github.com/eslint/eslint/blob/main/lib/rules/no-warning-comments.js#L84 fn convert_to_regexp(term: &str) -> regex::Regex { // Decorators are hard-coded here. Read them from config. let escaped_decoration = regex::escape(&["*", "/"].join("")); @@ -51,24 +52,12 @@ fn convert_to_regexp(term: &str) -> regex::Regex { // The start is from the first non-decorative character, ignoring whitespace, // new lines and characters specified in decoration. // The other value is match anywhere in comments. - // TODO: We need to check the location here and assign the prefix conditionally. I've omitted it here for now. + // TODO: We need to check the location (from config) here and assign the prefix conditionally. I've omitted it here for now. let prefix = format!("^[\\s{escaped_decoration}]*"); // The regex crate does not support inline flags like /u, so we use RegexBuilder below. let re = regex::RegexBuilder::new(r"/\\w$/").unicode(true).build().unwrap(); let suffix = if re.is_match(term) { word_boundary } else { "" }; - /* - * For location "start", the typical regex is: - * /^[\s]*ESCAPED_TERM\b/iu. - * Or if decoration characters are specified (e.g. "*"), then any of - * those characters may appear in any order at the start: - * /^[\s\*]*ESCAPED_TERM\b/iu. - * - * For location "anywhere" the typical regex is - * /\bESCAPED_TERM\b/iu - * - * If it starts or ends with non-word character, the prefix and suffix are empty, respectively. - */ regex::RegexBuilder::new(&format!("{prefix}{escaped}{suffix}")) .case_insensitive(true) // for 'i' .unicode(true) // for 'u' @@ -76,6 +65,7 @@ fn convert_to_regexp(term: &str) -> regex::Regex { .unwrap() } +/// https://github.com/eslint/eslint/blob/main/lib/rules/no-warning-comments.js#L142 fn _comment_contains_warning_term(comment: &str) -> Vec<&str> { let mut matches: Vec<&str> = vec![]; for (index, term) in WARNING_TERMS.iter().enumerate() { From 3b49fd82ea8949801e09ba5a38c3e028b60df7b6 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Sun, 13 Jul 2025 09:48:03 +0100 Subject: [PATCH 10/38] docs: fix formatting of URLs in comments for consistency --- crates/oxc_linter/src/rules/eslint/no_warning_comments.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 7e756f8b040f8..ce0c7eab65608 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -40,7 +40,7 @@ declare_oxc_lint!( const WARNING_TERMS: [&str; 3] = ["FIXME", "TODO", "xxx"]; -/// https://github.com/eslint/eslint/blob/main/lib/rules/no-warning-comments.js#L84 +/// fn convert_to_regexp(term: &str) -> regex::Regex { // Decorators are hard-coded here. Read them from config. let escaped_decoration = regex::escape(&["*", "/"].join("")); @@ -65,7 +65,7 @@ fn convert_to_regexp(term: &str) -> regex::Regex { .unwrap() } -/// https://github.com/eslint/eslint/blob/main/lib/rules/no-warning-comments.js#L142 +/// fn _comment_contains_warning_term(comment: &str) -> Vec<&str> { let mut matches: Vec<&str> = vec![]; for (index, term) in WARNING_TERMS.iter().enumerate() { From 7d39cb2426c51d3d814abbedcdb8b315e00d28f3 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Sun, 13 Jul 2025 10:44:36 +0100 Subject: [PATCH 11/38] feat(linter): enhance NoWarningComments rule with diagnostic messages and snapshot tests --- .../src/rules/eslint/no_warning_comments.rs | 395 +++++++++--------- .../snapshots/eslint_no_warning_comments.snap | 5 + 2 files changed, 206 insertions(+), 194 deletions(-) create mode 100644 crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index ce0c7eab65608..695576395e590 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -1,6 +1,7 @@ #![allow(dead_code)] use lazy_regex::regex; +use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use crate::{AstNode, context::LintContext, rule::Rule}; @@ -38,6 +39,7 @@ declare_oxc_lint!( // Options are 'fix', 'fix_dangerous', 'suggestion', and 'conditional_fix_suggestion' ); +// TODO: This needs to be loaded through config const WARNING_TERMS: [&str; 3] = ["FIXME", "TODO", "xxx"]; /// @@ -66,7 +68,7 @@ fn convert_to_regexp(term: &str) -> regex::Regex { } /// -fn _comment_contains_warning_term(comment: &str) -> Vec<&str> { +fn comment_contains_warning_term(comment: &str) -> Vec<&str> { let mut matches: Vec<&str> = vec![]; for (index, term) in WARNING_TERMS.iter().enumerate() { let re = convert_to_regexp(term); @@ -77,222 +79,227 @@ fn _comment_contains_warning_term(comment: &str) -> Vec<&str> { matches } +fn check_comment<'a>(ctx: &LintContext<'a>, _comment: &str) { + let matches = comment_contains_warning_term(_comment); + matches.iter().for_each(|_matched_term| { + ctx.diagnostic( + OxcDiagnostic::warn("Warning comments shou`ld be avoided") + .with_help("Use a command-like statement that tells the user how to fix the issue"), + ) + }); +} + impl Rule for NoWarningComments { fn run<'a>(&self, _node: &AstNode<'a>, ctx: &LintContext<'a>) { ctx.comments().iter().for_each(|comment| { let span = comment.span; - if let Some(_source_comment) = + if let Some(source_comment) = ctx.source_text().get((span.start as usize)..(span.end as usize)) { - // TODO: Process the source_comment here. + check_comment(ctx, source_comment); } }); - // ctx.diagnostic( - // OxcDiagnostic::warn("Warning comments shou`ld be avoided") - // .with_help("Use a command-like statement that tells the user how to fix the issue") - // ) } } -#[ignore] #[test] fn test() { use crate::tester::Tester; let pass = vec![ ("// any comment", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - ("// any comment", None), - ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "// any comment with TODO, FIXME or XXX", - Some(serde_json::json!([{ "location": "start" }])), - ), - ("// any comment with TODO, FIXME or XXX", None), - ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - ("/* any block comment */", None), - ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "/* any block comment with TODO, FIXME or XXX */", - Some(serde_json::json!([{ "location": "start" }])), - ), - ("/* any block comment with TODO, FIXME or XXX */", None), - ("/* any block comment with (TODO, FIXME's or XXX!) */", None), - ( - "// comments containing terms as substrings like TodoMVC", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// special regex characters don't cause a problem", - Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), - ), - ( - r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + // ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + // ("// any comment", None), + // ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "// any comment with TODO, FIXME or XXX", + // Some(serde_json::json!([{ "location": "start" }])), + // ), + // ("// any comment with TODO, FIXME or XXX", None), + // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + // ("/* any block comment */", None), + // ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "/* any block comment with TODO, FIXME or XXX */", + // Some(serde_json::json!([{ "location": "start" }])), + // ), + // ("/* any block comment with TODO, FIXME or XXX */", None), + // ("/* any block comment with (TODO, FIXME's or XXX!) */", None), + // ( + // "// comments containing terms as substrings like TodoMVC", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// special regex characters don't cause a problem", + // Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), + // ), + // ( + // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - var x = 10; - "#, - None, - ), - ( - r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + // var x = 10; + // "#, + // None, + // ), + // ( + // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - var x = 10; - "#, - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), - ( - "/** multi-line block comment with lines starting with - TODO - FIXME or - XXX - */", - None, - ), - ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), + // var x = 10; + // "#, + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), + // ( + // "/** multi-line block comment with lines starting with + // TODO + // FIXME or + // XXX + // */", + // None, + // ), + // ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; let fail = vec![ ("// fixme", None), - ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - ( - "/* any fixme */", - Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - ), - ( - "/* any FIXME */", - Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - ), - ( - "/* any fIxMe */", - Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - ), - ( - "// any fixme or todo", - Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - ), - ( - "/* any fixme or todo */", - Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - ), - ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("/* fixme and todo */", None), - ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - ( - "// regex [litera|$]", - Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), - ), - ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), - ( - "/* eslint one-var: 2 */", - Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), - ), - ( - "/* any block comment with TODO, FIXME or XXX */", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "/* any block comment with (TODO, FIXME's or XXX!) */", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "/** - *any block comment - *with (TODO, FIXME's or XXX!) **/", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "// any comment with TODO, FIXME or XXX", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "// TODO: something really longer than 40 characters", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "/* TODO: something - really longer than 40 characters - and also a new line */", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "// Comment ending with term followed by punctuation TODO!", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// Comment ending with term including punctuation TODO!", - Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - ), - ( - "// Comment ending with term including punctuation followed by more TODO!!!", - Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - ), - ( - "// !TODO comment starting with term preceded by punctuation", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// !TODO comment starting with term including punctuation", - Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - ), - ( - "// !!!TODO comment starting with term including punctuation preceded by more", - Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - ), - ( - "// FIX!term ending with punctuation followed word character", - Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), - ), - ( - "// Term starting with punctuation preceded word character!FIX", - Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), - ), - ( - "//!XXX comment starting with no spaces (anywhere)", - Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), - ), - ( - "//!XXX comment starting with no spaces (start)", - Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), - ), - ( - "/* - TODO undecorated multi-line block comment (start) - */", - Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), - ), - ( - "///// TODO decorated single-line comment with decoration array - /////", - Some( - serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - ), - ), - ( - "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - /////", - Some( - serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - ), - ), - ( - "//**TODO term starts with a decoration character", - Some( - serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), - ), - ), + // ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + // ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + // ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + // ( + // "/* any fixme */", + // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + // ), + // ( + // "/* any FIXME */", + // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + // ), + // ( + // "/* any fIxMe */", + // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + // ), + // ( + // "// any fixme or todo", + // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + // ), + // ( + // "/* any fixme or todo */", + // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + // ), + // ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("/* fixme and todo */", None), + // ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + // ( + // "// regex [litera|$]", + // Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), + // ), + // ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), + // ( + // "/* eslint one-var: 2 */", + // Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), + // ), + // ( + // "/* any block comment with TODO, FIXME or XXX */", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "/* any block comment with (TODO, FIXME's or XXX!) */", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "/** + // *any block comment + // *with (TODO, FIXME's or XXX!) **/", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "// any comment with TODO, FIXME or XXX", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "// TODO: something really longer than 40 characters", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "/* TODO: something + // really longer than 40 characters + // and also a new line */", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "// Comment ending with term followed by punctuation TODO!", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// Comment ending with term including punctuation TODO!", + // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + // ), + // ( + // "// Comment ending with term including punctuation followed by more TODO!!!", + // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + // ), + // ( + // "// !TODO comment starting with term preceded by punctuation", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// !TODO comment starting with term including punctuation", + // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + // ), + // ( + // "// !!!TODO comment starting with term including punctuation preceded by more", + // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + // ), + // ( + // "// FIX!term ending with punctuation followed word character", + // Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), + // ), + // ( + // "// Term starting with punctuation preceded word character!FIX", + // Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), + // ), + // ( + // "//!XXX comment starting with no spaces (anywhere)", + // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), + // ), + // ( + // "//!XXX comment starting with no spaces (start)", + // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), + // ), + // ( + // "/* + // TODO undecorated multi-line block comment (start) + // */", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), + // ), + // ( + // "///// TODO decorated single-line comment with decoration array + // /////", + // Some( + // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + // ), + // ), + // (= + // "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + // /////", + // Some( + // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + // ), + // ), + // ( + // "//**TODO term starts with a decoration character", + // Some( + // serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), + // ), + // ), ]; Tester::new(NoWarningComments::NAME, NoWarningComments::PLUGIN, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap new file mode 100644 index 0000000000000..a07805c3fd72b --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap @@ -0,0 +1,5 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint(no-warning-comments): Warning comments shou`ld be avoided + help: Use a command-like statement that tells the user how to fix the issue From 256ec764262199f8c9edab3911edb52a972b5f41 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Sun, 13 Jul 2025 10:51:00 +0100 Subject: [PATCH 12/38] fix(linter): refactor check_comment function for clarity and consistency --- .../oxc_linter/src/rules/eslint/no_warning_comments.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 695576395e590..54d726c8e0d99 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -79,14 +79,14 @@ fn comment_contains_warning_term(comment: &str) -> Vec<&str> { matches } -fn check_comment<'a>(ctx: &LintContext<'a>, _comment: &str) { - let matches = comment_contains_warning_term(_comment); - matches.iter().for_each(|_matched_term| { +fn check_comment(ctx: &LintContext, comment: &str) { + let matches = comment_contains_warning_term(comment); + for _matched_term in &matches { ctx.diagnostic( OxcDiagnostic::warn("Warning comments shou`ld be avoided") .with_help("Use a command-like statement that tells the user how to fix the issue"), - ) - }); + ); + } } impl Rule for NoWarningComments { From 5187076737679ebfa4e98ba6ddd2d4e3ccb4031d Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Sun, 13 Jul 2025 12:31:45 +0100 Subject: [PATCH 13/38] feat(linter): refactor NoWarningComments rule to support configurable warning terms --- .../src/rules/eslint/no_warning_comments.rs | 68 ++++++++++++++----- 1 file changed, 52 insertions(+), 16 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 54d726c8e0d99..e239939788849 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -3,11 +3,25 @@ use lazy_regex::regex; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; +use oxc_span::CompactStr; -use crate::{AstNode, context::LintContext, rule::Rule}; +use crate::{context::LintContext, rule::Rule}; #[derive(Debug, Default, Clone)] -pub struct NoWarningComments; +pub struct NoWarningComments(Box); + +#[derive(Debug, Default, Clone)] +pub struct NoWarningCommentsConfig { + terms: Vec, +} + +impl std::ops::Deref for NoWarningComments { + type Target = NoWarningCommentsConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} // See for documentation details. declare_oxc_lint!( @@ -39,9 +53,6 @@ declare_oxc_lint!( // Options are 'fix', 'fix_dangerous', 'suggestion', and 'conditional_fix_suggestion' ); -// TODO: This needs to be loaded through config -const WARNING_TERMS: [&str; 3] = ["FIXME", "TODO", "xxx"]; - /// fn convert_to_regexp(term: &str) -> regex::Regex { // Decorators are hard-coded here. Read them from config. @@ -68,19 +79,19 @@ fn convert_to_regexp(term: &str) -> regex::Regex { } /// -fn comment_contains_warning_term(comment: &str) -> Vec<&str> { - let mut matches: Vec<&str> = vec![]; - for (index, term) in WARNING_TERMS.iter().enumerate() { +fn comment_contains_warning_term(terms: &[CompactStr], comment: &str) -> Vec { + let mut matches: Vec = vec![]; + for (index, term) in terms.iter().enumerate() { let re = convert_to_regexp(term); if re.is_match(comment) { - matches.push(WARNING_TERMS[index]); + matches.push(terms[index].clone()); // FIXME: Fix this clone } } matches } -fn check_comment(ctx: &LintContext, comment: &str) { - let matches = comment_contains_warning_term(comment); +fn check_comment(ctx: &LintContext, comment: &str, terms: &[CompactStr]) { + let matches = comment_contains_warning_term(terms, comment); for _matched_term in &matches { ctx.diagnostic( OxcDiagnostic::warn("Warning comments shou`ld be avoided") @@ -90,14 +101,39 @@ fn check_comment(ctx: &LintContext, comment: &str) { } impl Rule for NoWarningComments { - fn run<'a>(&self, _node: &AstNode<'a>, ctx: &LintContext<'a>) { + fn from_configuration(value: serde_json::Value) -> Self { + // Reading the config { "terms": ["fixme"] } + // References: crates/oxc_linter/src/rules/eslint/max_lines_per_function.rs and crates/oxc_linter/src/rules/eslint/no_bitwise.rs + let config = value.get(0); + Self(Box::new(NoWarningCommentsConfig { + terms: config + .and_then(|config| config.get("terms")) + .and_then(serde_json::Value::as_array) + .map(|v| { + v.iter().filter_map(serde_json::Value::as_str).map(CompactStr::from).collect() + }) + .unwrap_or(vec![ + CompactStr::new("FIXME"), + CompactStr::new("TODO"), + CompactStr::new("xxx"), + ]), + })) + } + + // See + // We are not using `AstNode`` as the comments aren't attached to the tree. + // Therefore, we do not need to implement the `run` function. + // We can hence implement `run_once`` and read the comments using the context. + fn run_once(&self, ctx: &LintContext) { ctx.comments().iter().for_each(|comment| { let span = comment.span; - if let Some(source_comment) = + // Recommended in the docs to use let-else over if-let + let Some(source_comment) = ctx.source_text().get((span.start as usize)..(span.end as usize)) - { - check_comment(ctx, source_comment); - } + else { + return; + }; + check_comment(ctx, source_comment, &self.terms); }); } } From dc37d879ff5f16fc9c2970cadd17461152ae824c Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Mon, 14 Jul 2025 11:01:51 +0100 Subject: [PATCH 14/38] refactor(linter): rollback --- .../src/rules/eslint/no_warning_comments.rs | 485 +++++++----------- 1 file changed, 196 insertions(+), 289 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index e239939788849..4a888e64a6675 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -1,27 +1,9 @@ -#![allow(dead_code)] - -use lazy_regex::regex; -use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; -use oxc_span::CompactStr; - -use crate::{context::LintContext, rule::Rule}; -#[derive(Debug, Default, Clone)] -pub struct NoWarningComments(Box); +use crate::{AstNode, context::LintContext, rule::Rule}; #[derive(Debug, Default, Clone)] -pub struct NoWarningCommentsConfig { - terms: Vec, -} - -impl std::ops::Deref for NoWarningComments { - type Target = NoWarningCommentsConfig; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} +pub struct NoWarningComments; // See for documentation details. declare_oxc_lint!( @@ -53,289 +35,214 @@ declare_oxc_lint!( // Options are 'fix', 'fix_dangerous', 'suggestion', and 'conditional_fix_suggestion' ); -/// -fn convert_to_regexp(term: &str) -> regex::Regex { - // Decorators are hard-coded here. Read them from config. - let escaped_decoration = regex::escape(&["*", "/"].join("")); - let escaped = regex::escape(term); - let word_boundary = "\\b"; - - // "location": optional string that configures where in your comments to - // check for matches. Defaults to "start". - // The start is from the first non-decorative character, ignoring whitespace, - // new lines and characters specified in decoration. - // The other value is match anywhere in comments. - // TODO: We need to check the location (from config) here and assign the prefix conditionally. I've omitted it here for now. - - let prefix = format!("^[\\s{escaped_decoration}]*"); - // The regex crate does not support inline flags like /u, so we use RegexBuilder below. - let re = regex::RegexBuilder::new(r"/\\w$/").unicode(true).build().unwrap(); - let suffix = if re.is_match(term) { word_boundary } else { "" }; - regex::RegexBuilder::new(&format!("{prefix}{escaped}{suffix}")) - .case_insensitive(true) // for 'i' - .unicode(true) // for 'u' - .build() - .unwrap() -} - -/// -fn comment_contains_warning_term(terms: &[CompactStr], comment: &str) -> Vec { - let mut matches: Vec = vec![]; - for (index, term) in terms.iter().enumerate() { - let re = convert_to_regexp(term); - if re.is_match(comment) { - matches.push(terms[index].clone()); // FIXME: Fix this clone - } - } - matches -} - -fn check_comment(ctx: &LintContext, comment: &str, terms: &[CompactStr]) { - let matches = comment_contains_warning_term(terms, comment); - for _matched_term in &matches { - ctx.diagnostic( - OxcDiagnostic::warn("Warning comments shou`ld be avoided") - .with_help("Use a command-like statement that tells the user how to fix the issue"), - ); - } -} - impl Rule for NoWarningComments { - fn from_configuration(value: serde_json::Value) -> Self { - // Reading the config { "terms": ["fixme"] } - // References: crates/oxc_linter/src/rules/eslint/max_lines_per_function.rs and crates/oxc_linter/src/rules/eslint/no_bitwise.rs - let config = value.get(0); - Self(Box::new(NoWarningCommentsConfig { - terms: config - .and_then(|config| config.get("terms")) - .and_then(serde_json::Value::as_array) - .map(|v| { - v.iter().filter_map(serde_json::Value::as_str).map(CompactStr::from).collect() - }) - .unwrap_or(vec![ - CompactStr::new("FIXME"), - CompactStr::new("TODO"), - CompactStr::new("xxx"), - ]), - })) - } - - // See - // We are not using `AstNode`` as the comments aren't attached to the tree. - // Therefore, we do not need to implement the `run` function. - // We can hence implement `run_once`` and read the comments using the context. - fn run_once(&self, ctx: &LintContext) { - ctx.comments().iter().for_each(|comment| { - let span = comment.span; - // Recommended in the docs to use let-else over if-let - let Some(source_comment) = - ctx.source_text().get((span.start as usize)..(span.end as usize)) - else { - return; - }; - check_comment(ctx, source_comment, &self.terms); - }); + fn run<'a>(&self, _node: &AstNode<'a>, _ctx: &LintContext<'a>) { + // ctx.diagnostic( + // OxcDiagnostic::warn("Warning comments should be avoided") + // .with_help("Use a command-like statement that tells the user how to fix the issue") + // ) } } +#[ignore] #[test] fn test() { use crate::tester::Tester; let pass = vec![ ("// any comment", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - // ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - // ("// any comment", None), - // ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "// any comment with TODO, FIXME or XXX", - // Some(serde_json::json!([{ "location": "start" }])), - // ), - // ("// any comment with TODO, FIXME or XXX", None), - // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - // ("/* any block comment */", None), - // ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "/* any block comment with TODO, FIXME or XXX */", - // Some(serde_json::json!([{ "location": "start" }])), - // ), - // ("/* any block comment with TODO, FIXME or XXX */", None), - // ("/* any block comment with (TODO, FIXME's or XXX!) */", None), - // ( - // "// comments containing terms as substrings like TodoMVC", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// special regex characters don't cause a problem", - // Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), - // ), - // ( - // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - - // var x = 10; - // "#, - // None, - // ), - // ( - // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - - // var x = 10; - // "#, - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), - // ( - // "/** multi-line block comment with lines starting with - // TODO - // FIXME or - // XXX - // */", - // None, - // ), - // ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), + ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + ("// any comment", None), + ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// any comment with TODO, FIXME or XXX", + Some(serde_json::json!([{ "location": "start" }])), + ), + ("// any comment with TODO, FIXME or XXX", None), + ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + ("/* any block comment */", None), + ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "/* any block comment with TODO, FIXME or XXX */", + Some(serde_json::json!([{ "location": "start" }])), + ), + ("/* any block comment with TODO, FIXME or XXX */", None), + ("/* any block comment with (TODO, FIXME's or XXX!) */", None), + ( + "// comments containing terms as substrings like TodoMVC", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// special regex characters don't cause a problem", + Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), + ), + ( + r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + + var x = 10; + "#, + None, + ), + ( + r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + + var x = 10; + "#, + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), + ( + "/** multi-line block comment with lines starting with + TODO + FIXME or + XXX + */", + None, + ), + ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; let fail = vec![ ("// fixme", None), - // ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - // ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - // ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - // ( - // "/* any fixme */", - // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - // ), - // ( - // "/* any FIXME */", - // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - // ), - // ( - // "/* any fIxMe */", - // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - // ), - // ( - // "// any fixme or todo", - // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - // ), - // ( - // "/* any fixme or todo */", - // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - // ), - // ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("/* fixme and todo */", None), - // ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - // ( - // "// regex [litera|$]", - // Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), - // ), - // ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), - // ( - // "/* eslint one-var: 2 */", - // Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), - // ), - // ( - // "/* any block comment with TODO, FIXME or XXX */", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "/* any block comment with (TODO, FIXME's or XXX!) */", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "/** - // *any block comment - // *with (TODO, FIXME's or XXX!) **/", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "// any comment with TODO, FIXME or XXX", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "// TODO: something really longer than 40 characters", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "/* TODO: something - // really longer than 40 characters - // and also a new line */", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "// Comment ending with term followed by punctuation TODO!", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// Comment ending with term including punctuation TODO!", - // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - // ), - // ( - // "// Comment ending with term including punctuation followed by more TODO!!!", - // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - // ), - // ( - // "// !TODO comment starting with term preceded by punctuation", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// !TODO comment starting with term including punctuation", - // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - // ), - // ( - // "// !!!TODO comment starting with term including punctuation preceded by more", - // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - // ), - // ( - // "// FIX!term ending with punctuation followed word character", - // Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), - // ), - // ( - // "// Term starting with punctuation preceded word character!FIX", - // Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), - // ), - // ( - // "//!XXX comment starting with no spaces (anywhere)", - // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), - // ), - // ( - // "//!XXX comment starting with no spaces (start)", - // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), - // ), - // ( - // "/* - // TODO undecorated multi-line block comment (start) - // */", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), - // ), - // ( - // "///// TODO decorated single-line comment with decoration array - // /////", - // Some( - // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - // ), - // ), - // (= - // "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - // /////", - // Some( - // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - // ), - // ), - // ( - // "//**TODO term starts with a decoration character", - // Some( - // serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), - // ), - // ), + ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ( + "/* any fixme */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "/* any FIXME */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "/* any fIxMe */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "// any fixme or todo", + Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + ), + ( + "/* any fixme or todo */", + Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + ), + ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* fixme and todo */", None), + ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + ( + "// regex [litera|$]", + Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), + ), + ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), + ( + "/* eslint one-var: 2 */", + Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), + ), + ( + "/* any block comment with TODO, FIXME or XXX */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/* any block comment with (TODO, FIXME's or XXX!) */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/** + *any block comment + *with (TODO, FIXME's or XXX!) **/", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "// any comment with TODO, FIXME or XXX", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// TODO: something really longer than 40 characters", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/* TODO: something + really longer than 40 characters + and also a new line */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "// Comment ending with term followed by punctuation TODO!", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// Comment ending with term including punctuation TODO!", + Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + ), + ( + "// Comment ending with term including punctuation followed by more TODO!!!", + Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + ), + ( + "// !TODO comment starting with term preceded by punctuation", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// !TODO comment starting with term including punctuation", + Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + ), + ( + "// !!!TODO comment starting with term including punctuation preceded by more", + Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + ), + ( + "// FIX!term ending with punctuation followed word character", + Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), + ), + ( + "// Term starting with punctuation preceded word character!FIX", + Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), + ), + ( + "//!XXX comment starting with no spaces (anywhere)", + Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), + ), + ( + "//!XXX comment starting with no spaces (start)", + Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), + ), + ( + "/* + TODO undecorated multi-line block comment (start) + */", + Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), + ), + ( + "///// TODO decorated single-line comment with decoration array + /////", + Some( + serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + ), + ), + ( + "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + /////", + Some( + serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + ), + ), + ( + "//**TODO term starts with a decoration character", + Some( + serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), + ), + ), ]; Tester::new(NoWarningComments::NAME, NoWarningComments::PLUGIN, pass, fail).test_and_snapshot(); From 95fa67c2914a31b635f81c1b9b196103baf02384 Mon Sep 17 00:00:00 2001 From: Stephen <519327+stevieing@users.noreply.github.com> Date: Fri, 1 Aug 2025 14:34:10 +0100 Subject: [PATCH 15/38] feat(linter): expand comments in NoWarningComments rule for clarity and implementation guidance --- .../src/rules/eslint/no_warning_comments.rs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 4a888e64a6675..8ea1f629c0619 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -41,6 +41,33 @@ impl Rule for NoWarningComments { // OxcDiagnostic::warn("Warning comments should be avoided") // .with_help("Use a command-like statement that tells the user how to fix the issue") // ) + + // 1. create a copy of the source code + // 2. create a copy of the decoration, location and terms. We can get these from the configuration using the from_config function + // 3. escape the decoration special characters. Decoration can be a string or an array of strings. + // 4. creates a constant of /\bno-warning-comments\b/u + // 5. for each of the warning terms it converts the term to a regular expression: + // - escape the term special characters + // - create a constant of the word boundary which is \\b + // - create a variable for the prefix + // - if the location is "start" then it sets prefix to the escaped decoration `^[\\s${escapedDecoration}]*` + // - tests /^\w/u against the term and if they match sets the prefix to the word boundary + // - sets a constant for the suffix by running a test /\w$/u against the term if true sets suffix to word boundary otherwise sets suffix to an empty string + // - creates a constant for flag of "iu" + // - returns a regular expression with the prefix, term, and suffix passing in the flags + // 6. creates a constant comments which gets all of the comments from the source code using the ast comments. Gets all of the tree nodes that are comments. + // 7. for each comment: + // - filters out any comments which start with shebang #! + // - runs the check comment function. + // 8. check comment function: + // - sets a constant for the comment text which is node.value + // - if it is a directive comment e.g. eslint-disable-next-line or selfConfigRegEx it returns early + // - creates a constant of the matches containing the warning terms. Which is a list of warning terms. + // - for each match: + // - takes the comment and splits it by spaces + // - adds the comment to the comment that will be displayed + // - if the line is longer than 40 characters it will be truncated with an ellipsis + // - it will report the message to the context with messageId unexpectedCooment with the data being the matched term and the comment but if it is too long it is truncated to 40 characters with an ellipsis } } From 2b6fd16cf68b75f230103ee46549e88a036ca632 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Mon, 11 Aug 2025 16:27:55 +0100 Subject: [PATCH 16/38] feat(linter): implement configuration parsing for NoWarningComments rule --- .../src/rules/eslint/no_warning_comments.rs | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 8ea1f629c0619..c19cf93a1843b 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -3,7 +3,11 @@ use oxc_macros::declare_oxc_lint; use crate::{AstNode, context::LintContext, rule::Rule}; #[derive(Debug, Default, Clone)] -pub struct NoWarningComments; +pub struct NoWarningComments { + terms: Option>, + decorations: Option>, + location: Option, +} // See for documentation details. declare_oxc_lint!( @@ -69,6 +73,41 @@ impl Rule for NoWarningComments { // - if the line is longer than 40 characters it will be truncated with an ellipsis // - it will report the message to the context with messageId unexpectedCooment with the data being the matched term and the comment but if it is too long it is truncated to 40 characters with an ellipsis } + + fn from_configuration(value: serde_json::Value) -> Self { + // Read the configuration for term, decoration and location from _value and then + // return NoWarningComments {} struct with the attributes terms, decoration and locations. + + // TODO: Create. NoWarningCommentsConfig struct and box it inside the return struct NoWarningCommentsConfig + // See crates/oxc_linter/src/rules/eslint/default_case.rs. + let mut terms: Option> = None; + let mut decorations: Option> = None; + let mut location: Option = None; + + if let Some(config) = value.get(0) { + if let Some(terms_config) = config.get("terms") { + terms = terms_config.as_array().map(|arr| { + arr.iter() + .filter_map(|v| v.as_str().map(|s| s.to_string())) + .collect::>() + }); + } + + if let Some(decorations_config) = config.get("decorations") { + decorations = decorations_config.as_array().map(|arr| { + arr.iter() + .filter_map(|v| v.as_str().map(|s| s.to_string())) + .collect::>() + }); + } + + if let Some(location_config) = config.get("location") { + location = location_config.as_str().map(|s| s.to_string()); + } + } + + return Self { terms, decorations, location }; + } } #[ignore] From 4900a8e210479e1957c0ad19c504db6e7bcfae09 Mon Sep 17 00:00:00 2001 From: yoldas Date: Fri, 12 Sep 2025 12:10:26 +0100 Subject: [PATCH 17/38] add config struct --- .../src/rules/eslint/no_warning_comments.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index c19cf93a1843b..6befec1adb6ef 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -3,11 +3,13 @@ use oxc_macros::declare_oxc_lint; use crate::{AstNode, context::LintContext, rule::Rule}; #[derive(Debug, Default, Clone)] -pub struct NoWarningComments { +pub struct NoWarningCommentsConfig { terms: Option>, decorations: Option>, location: Option, } +#[derive(Debug, Default, Clone)] +pub struct NoWarningComments(Box); // See for documentation details. declare_oxc_lint!( @@ -80,13 +82,12 @@ impl Rule for NoWarningComments { // TODO: Create. NoWarningCommentsConfig struct and box it inside the return struct NoWarningCommentsConfig // See crates/oxc_linter/src/rules/eslint/default_case.rs. - let mut terms: Option> = None; - let mut decorations: Option> = None; - let mut location: Option = None; + + let mut cfg = NoWarningCommentsConfig::default(); if let Some(config) = value.get(0) { if let Some(terms_config) = config.get("terms") { - terms = terms_config.as_array().map(|arr| { + cfg.terms = terms_config.as_array().map(|arr| { arr.iter() .filter_map(|v| v.as_str().map(|s| s.to_string())) .collect::>() @@ -94,7 +95,7 @@ impl Rule for NoWarningComments { } if let Some(decorations_config) = config.get("decorations") { - decorations = decorations_config.as_array().map(|arr| { + cfg.decorations = decorations_config.as_array().map(|arr| { arr.iter() .filter_map(|v| v.as_str().map(|s| s.to_string())) .collect::>() @@ -102,15 +103,15 @@ impl Rule for NoWarningComments { } if let Some(location_config) = config.get("location") { - location = location_config.as_str().map(|s| s.to_string()); + cfg.location = location_config.as_str().map(|s| s.to_string()); } } - return Self { terms, decorations, location }; + return Self(Box::new(cfg)); } } -#[ignore] +// cargo test -p oxc_linter -- no_warning_comments #[test] fn test() { use crate::tester::Tester; From ea5f5cbf4a2f5b3b69af5347cac6c2599e10665c Mon Sep 17 00:00:00 2001 From: yoldas Date: Fri, 19 Sep 2025 11:58:04 +0100 Subject: [PATCH 18/38] Chage run to run_once --- crates/oxc_linter/src/rules/eslint/no_warning_comments.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 6befec1adb6ef..22e48e3e7f5e8 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -42,7 +42,11 @@ declare_oxc_lint!( ); impl Rule for NoWarningComments { - fn run<'a>(&self, _node: &AstNode<'a>, _ctx: &LintContext<'a>) { + fn run_once(&self, ctx: &LintContext) { + ctx.semantic().comments().iter().for_each(|comment| { + println!("Comment: {:?}", comment); + }); + // ctx.diagnostic( // OxcDiagnostic::warn("Warning comments should be avoided") // .with_help("Use a command-like statement that tells the user how to fix the issue") From 7485caeccc3766464d972f9a7391b20b0106140a Mon Sep 17 00:00:00 2001 From: Stephen <519327+stevieing@users.noreply.github.com> Date: Fri, 26 Sep 2025 11:57:37 +0100 Subject: [PATCH 19/38] feat(linter): enhance NoWarningComments rule with location handling and debug output --- .../src/rules/eslint/no_warning_comments.rs | 373 ++++++++++-------- 1 file changed, 203 insertions(+), 170 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 22e48e3e7f5e8..2eb81cc56a6ab 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -41,19 +41,51 @@ declare_oxc_lint!( // Options are 'fix', 'fix_dangerous', 'suggestion', and 'conditional_fix_suggestion' ); +// https://eslint.org/docs/latest/rules/no-warning-comments#options +// if location is "start" then ignore decorators, if "anywhere" then do not ignore decorators. If location is not provided then default to "start". impl Rule for NoWarningComments { fn run_once(&self, ctx: &LintContext) { ctx.semantic().comments().iter().for_each(|comment| { - println!("Comment: {:?}", comment); + + println!("&self: {:?}", &self.0); + let mut source_text = ctx.source_text(); + match &self.0.location { + + Some(loc) if loc != "anywhere" => { + // why do we need to use 0? + let decorations = &self.0.decorations; + println!("Decorations: {:?}", decorations); + // if let Some(decorations) = &self.decorations { + // for decoration in decorations { + // let escaped_decoration = regex::escape(decoration); + // let pattern = format!(r"^[\s{escaped_decoration}]*"); + // let re = regex::Regex::new(&pattern).unwrap(); + // source_text = re.replace(&source_text, "").to_string(); + // } + // } + } + _ => {} + } + + // let kind = comment.kind; + // println!("Kind: {:?}", kind); + // let span = comment.span; + // let source_text = ctx.source_text().get((span.start as usize)..(span.end as usize)); + // // println!("Span: {:?}", span); + // println!("Source Text: {:?}", source_text); }); + // println!("Source Text: {:?}", ctx.source_text()); + + // ctx.source_text().get((span.start as usize)..(span.end as usize)) + // ctx.diagnostic( // OxcDiagnostic::warn("Warning comments should be avoided") // .with_help("Use a command-like statement that tells the user how to fix the issue") // ) - // 1. create a copy of the source code - // 2. create a copy of the decoration, location and terms. We can get these from the configuration using the from_config function + // 1. create a copy of the source code x + // 2. create a copy of the decoration, location and terms. We can get these from the configuration using the from_config function x // 3. escape the decoration special characters. Decoration can be a string or an array of strings. // 4. creates a constant of /\bno-warning-comments\b/u // 5. for each of the warning terms it converts the term to a regular expression: @@ -98,6 +130,7 @@ impl Rule for NoWarningComments { }); } + // this is not working, decorations is always None if let Some(decorations_config) = config.get("decorations") { cfg.decorations = decorations_config.as_array().map(|arr| { arr.iter() @@ -121,179 +154,179 @@ fn test() { use crate::tester::Tester; let pass = vec![ - ("// any comment", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - ("// any comment", None), - ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "// any comment with TODO, FIXME or XXX", - Some(serde_json::json!([{ "location": "start" }])), - ), - ("// any comment with TODO, FIXME or XXX", None), - ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - ("/* any block comment */", None), - ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "/* any block comment with TODO, FIXME or XXX */", - Some(serde_json::json!([{ "location": "start" }])), - ), - ("/* any block comment with TODO, FIXME or XXX */", None), - ("/* any block comment with (TODO, FIXME's or XXX!) */", None), - ( - "// comments containing terms as substrings like TodoMVC", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// special regex characters don't cause a problem", - Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), - ), - ( - r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + // ("// any comment", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + // ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + // ("// any comment", None), + // ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "// any comment with TODO, FIXME or XXX", + // Some(serde_json::json!([{ "location": "start" }])), + // ), + // ("// any comment with TODO, FIXME or XXX", None), + // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + // ("/* any block comment */", None), + // ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "/* any block comment with TODO, FIXME or XXX */", + // Some(serde_json::json!([{ "location": "start" }])), + // ), + // ("/* any block comment with TODO, FIXME or XXX */", None), + // ("/* any block comment with (TODO, FIXME's or XXX!) */", None), + // ( + // "// comments containing terms as substrings like TodoMVC", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// special regex characters don't cause a problem", + // Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), + // ), + // ( + // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - var x = 10; - "#, - None, - ), - ( - r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + // var x = 10; + // "#, + // None, + // ), + // ( + // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - var x = 10; - "#, - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), - ( - "/** multi-line block comment with lines starting with - TODO - FIXME or - XXX - */", - None, - ), + // var x = 10; + // "#, + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), + // ( + // "/** multi-line block comment with lines starting with + // TODO + // FIXME or + // XXX + // */", + // None, + // ), ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; let fail = vec![ - ("// fixme", None), - ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - ( - "/* any fixme */", - Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - ), - ( - "/* any FIXME */", - Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - ), - ( - "/* any fIxMe */", - Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - ), - ( - "// any fixme or todo", - Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - ), - ( - "/* any fixme or todo */", - Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - ), - ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("/* fixme and todo */", None), - ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), - ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - ( - "// regex [litera|$]", - Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), - ), - ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), - ( - "/* eslint one-var: 2 */", - Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), - ), - ( - "/* any block comment with TODO, FIXME or XXX */", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "/* any block comment with (TODO, FIXME's or XXX!) */", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "/** - *any block comment - *with (TODO, FIXME's or XXX!) **/", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "// any comment with TODO, FIXME or XXX", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "// TODO: something really longer than 40 characters", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "/* TODO: something - really longer than 40 characters - and also a new line */", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), - ( - "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ( - "// Comment ending with term followed by punctuation TODO!", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// Comment ending with term including punctuation TODO!", - Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - ), - ( - "// Comment ending with term including punctuation followed by more TODO!!!", - Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - ), - ( - "// !TODO comment starting with term preceded by punctuation", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// !TODO comment starting with term including punctuation", - Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - ), - ( - "// !!!TODO comment starting with term including punctuation preceded by more", - Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - ), - ( - "// FIX!term ending with punctuation followed word character", - Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), - ), - ( - "// Term starting with punctuation preceded word character!FIX", - Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), - ), - ( - "//!XXX comment starting with no spaces (anywhere)", - Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), - ), - ( - "//!XXX comment starting with no spaces (start)", - Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), - ), - ( - "/* - TODO undecorated multi-line block comment (start) - */", - Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), - ), + // ("// fixme", None), + // ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + // ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + // ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + // ( + // "/* any fixme */", + // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + // ), + // ( + // "/* any FIXME */", + // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + // ), + // ( + // "/* any fIxMe */", + // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + // ), + // ( + // "// any fixme or todo", + // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + // ), + // ( + // "/* any fixme or todo */", + // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + // ), + // ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("/* fixme and todo */", None), + // ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + // ( + // "// regex [litera|$]", + // Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), + // ), + // ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), + // ( + // "/* eslint one-var: 2 */", + // Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), + // ), + // ( + // "/* any block comment with TODO, FIXME or XXX */", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "/* any block comment with (TODO, FIXME's or XXX!) */", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "/** + // *any block comment + // *with (TODO, FIXME's or XXX!) **/", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "// any comment with TODO, FIXME or XXX", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "// TODO: something really longer than 40 characters", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "/* TODO: something + // really longer than 40 characters + // and also a new line */", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), + // ( + // "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ( + // "// Comment ending with term followed by punctuation TODO!", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// Comment ending with term including punctuation TODO!", + // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + // ), + // ( + // "// Comment ending with term including punctuation followed by more TODO!!!", + // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + // ), + // ( + // "// !TODO comment starting with term preceded by punctuation", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// !TODO comment starting with term including punctuation", + // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + // ), + // ( + // "// !!!TODO comment starting with term including punctuation preceded by more", + // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + // ), + // ( + // "// FIX!term ending with punctuation followed word character", + // Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), + // ), + // ( + // "// Term starting with punctuation preceded word character!FIX", + // Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), + // ), + // ( + // "//!XXX comment starting with no spaces (anywhere)", + // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), + // ), + // ( + // "//!XXX comment starting with no spaces (start)", + // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), + // ), + // ( + // "/* + // TODO undecorated multi-line block comment (start) + // */", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), + // ), ( "///// TODO decorated single-line comment with decoration array /////", From 68452458cb37083fddfe9e1c8762f2bb403bf55d Mon Sep 17 00:00:00 2001 From: yoldas Date: Fri, 3 Oct 2025 12:01:02 +0100 Subject: [PATCH 20/38] Fix decoration config and understand how it works --- .../src/rules/eslint/no_warning_comments.rs | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 2eb81cc56a6ab..d96897822576f 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -46,11 +46,9 @@ declare_oxc_lint!( impl Rule for NoWarningComments { fn run_once(&self, ctx: &LintContext) { ctx.semantic().comments().iter().for_each(|comment| { - println!("&self: {:?}", &self.0); let mut source_text = ctx.source_text(); match &self.0.location { - Some(loc) if loc != "anywhere" => { // why do we need to use 0? let decorations = &self.0.decorations; @@ -67,12 +65,12 @@ impl Rule for NoWarningComments { _ => {} } - // let kind = comment.kind; - // println!("Kind: {:?}", kind); - // let span = comment.span; - // let source_text = ctx.source_text().get((span.start as usize)..(span.end as usize)); - // // println!("Span: {:?}", span); - // println!("Source Text: {:?}", source_text); + let kind = comment.kind; + println!("Kind: {:?}", kind); + let span = comment.span; + let source_text = ctx.source_text().get((span.start as usize)..(span.end as usize)); + println!("Span: {:?}", span); + println!("Source Text: {:?}", source_text); }); // println!("Source Text: {:?}", ctx.source_text()); @@ -87,6 +85,9 @@ impl Rule for NoWarningComments { // 1. create a copy of the source code x // 2. create a copy of the decoration, location and terms. We can get these from the configuration using the from_config function x // 3. escape the decoration special characters. Decoration can be a string or an array of strings. + // if it is a line comment, skip // first + // if it is a block comment, skip /* first: we are not there yet. + // escape each decoration character until you reach a non-decoration character or the term itself, e.g. *todo // 4. creates a constant of /\bno-warning-comments\b/u // 5. for each of the warning terms it converts the term to a regular expression: // - escape the term special characters @@ -131,7 +132,7 @@ impl Rule for NoWarningComments { } // this is not working, decorations is always None - if let Some(decorations_config) = config.get("decorations") { + if let Some(decorations_config) = config.get("decoration") { cfg.decorations = decorations_config.as_array().map(|arr| { arr.iter() .filter_map(|v| v.as_str().map(|s| s.to_string())) @@ -184,24 +185,24 @@ fn test() { // ( // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - // var x = 10; - // "#, + // var x = 10; + // "#, // None, // ), // ( // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - // var x = 10; - // "#, + // var x = 10; + // "#, // Some(serde_json::json!([{ "location": "anywhere" }])), // ), // ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), // ( // "/** multi-line block comment with lines starting with - // TODO - // FIXME or - // XXX - // */", + // TODO + // FIXME or + // XXX + // */", // None, // ), ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), @@ -257,8 +258,8 @@ fn test() { // ), // ( // "/** - // *any block comment - // *with (TODO, FIXME's or XXX!) **/", + // *any block comment + // *with (TODO, FIXME's or XXX!) **/", // Some(serde_json::json!([{ "location": "anywhere" }])), // ), // ( @@ -272,8 +273,8 @@ fn test() { // ), // ( // "/* TODO: something - // really longer than 40 characters - // and also a new line */", + // really longer than 40 characters + // and also a new line */", // Some(serde_json::json!([{ "location": "anywhere" }])), // ), // ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), @@ -323,8 +324,8 @@ fn test() { // ), // ( // "/* - // TODO undecorated multi-line block comment (start) - // */", + // TODO undecorated multi-line block comment (start) + // */", // Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), // ), ( From 5653c9b9752f0e6cba4aab1586e051738b875a13 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 10 Oct 2025 11:45:33 +0100 Subject: [PATCH 21/38] feat(linter): enhance no_warning_comments rule with diagnostic messages and update snapshots --- .../src/rules/eslint/no_warning_comments.rs | 58 +++++++++++++------ .../snapshots/eslint_no_warning_comments.snap | 9 ++- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index d96897822576f..164e766244046 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -1,7 +1,16 @@ +use oxc_ast::CommentKind; +use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; +use oxc_span::Span; use crate::{AstNode, context::LintContext, rule::Rule}; +fn no_with_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Unexpected use of `with` statement.") + .with_help("Do not use the `with` statement.") + .with_label(span) +} + #[derive(Debug, Default, Clone)] pub struct NoWarningCommentsConfig { terms: Option>, @@ -68,9 +77,24 @@ impl Rule for NoWarningComments { let kind = comment.kind; println!("Kind: {:?}", kind); let span = comment.span; - let source_text = ctx.source_text().get((span.start as usize)..(span.end as usize)); - println!("Span: {:?}", span); - println!("Source Text: {:?}", source_text); + let span_pointers: (u32, u32) = match kind { + CommentKind::Line => ((span.start + 2) as u32, span.end), + CommentKind::Block => (span.start, span.end), + _ => (span.start, span.end), + }; + let comment_text = + ctx.source_text().get((span_pointers.0 as usize)..(span_pointers.1 as usize)); + // match comment_text { + // Some(text) => { + + // } + // _ => {} + // } + println!("Source Text: {:?}", comment_text); + if comment_text.unwrap().to_string().to_lowercase().contains("todo") { + ctx.diagnostic(no_with_diagnostic(span)); + } + // println!("Span: {:?}", span); }); // println!("Source Text: {:?}", ctx.source_text()); @@ -205,7 +229,7 @@ fn test() { // */", // None, // ), - ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), + // ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; let fail = vec![ @@ -335,19 +359,19 @@ fn test() { serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), ), ), - ( - "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - /////", - Some( - serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - ), - ), - ( - "//**TODO term starts with a decoration character", - Some( - serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), - ), - ), + // ( + // "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + // /////", + // Some( + // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + // ), + // ), + // ( + // "//**TODO term starts with a decoration character", + // Some( + // serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), + // ), + // ), ]; Tester::new(NoWarningComments::NAME, NoWarningComments::PLUGIN, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap index a07805c3fd72b..449a538316dd8 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap @@ -1,5 +1,10 @@ --- source: crates/oxc_linter/src/tester.rs --- - ⚠ eslint(no-warning-comments): Warning comments shou`ld be avoided - help: Use a command-like statement that tells the user how to fix the issue + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ ///// TODO decorated single-line comment with decoration array + · ────────────────────────────────────────────────────────────── + 2 │ ///// + ╰──── + help: Do not use the `with` statement. From bc474dda111dc90c21e85ce04d035fa247271c8c Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 10 Oct 2025 11:49:50 +0100 Subject: [PATCH 22/38] feat(tests): update snapshots for no_warning_comments rule with new test cases --- .../src/rules/eslint/no_warning_comments.rs | 30 +++++++++++-------- .../snapshots/eslint_no_warning_comments.snap | 15 ++++++++++ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 164e766244046..a18f46e7bf654 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -91,6 +91,10 @@ impl Rule for NoWarningComments { // _ => {} // } println!("Source Text: {:?}", comment_text); + // Date: 10/10/2025 + // 1. Loop through the terms using &self.0.terms and use each term inside + // contains(). + // 2. Remove the decorators from the comment in question. if comment_text.unwrap().to_string().to_lowercase().contains("todo") { ctx.diagnostic(no_with_diagnostic(span)); } @@ -359,19 +363,19 @@ fn test() { serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), ), ), - // ( - // "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - // /////", - // Some( - // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - // ), - // ), - // ( - // "//**TODO term starts with a decoration character", - // Some( - // serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), - // ), - // ), + ( + "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + /////", + Some( + serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + ), + ), + ( + "//**TODO term starts with a decoration character", + Some( + serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), + ), + ), ]; Tester::new(NoWarningComments::NAME, NoWarningComments::PLUGIN, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap index 449a538316dd8..c94f88d1f71e9 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap @@ -8,3 +8,18 @@ source: crates/oxc_linter/src/tester.rs 2 │ ///// ╰──── help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ ///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + · ────────────────────────────────────────────────────────────────────────────────────── + 2 │ ///// + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ //**TODO term starts with a decoration character + · ──────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. From 4d713aa09ec300167c0c1b0dd8cafaf96a9d3144 Mon Sep 17 00:00:00 2001 From: yoldas Date: Fri, 17 Oct 2025 12:04:43 +0100 Subject: [PATCH 23/38] Add decorator trimming and lowecasing --- .../src/rules/eslint/no_warning_comments.rs | 52 +++++++++++++++---- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index a18f46e7bf654..0bb63930ff354 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -50,6 +50,29 @@ declare_oxc_lint!( // Options are 'fix', 'fix_dangerous', 'suggestion', and 'conditional_fix_suggestion' ); +// Refactor this function to make it more Rusty +fn trim_decorations_until_terms<'a>( + s: &'a str, // can accept string slice as input; not an owned String + decorations: &Vec, // slice of string slices + terms: &Vec, // slice of string slices +) -> &'a str { + // return a slice of the original string (&'a str) without copying + let mut i = 0; + let s_len = s.len(); + while i < s_len { + if terms.iter().any(|term| s[i..].starts_with(term)) { + break; // stop if a term matches + } + let c = s[i..].chars().next().unwrap(); + if decorations.iter().any(|d| *d == c.to_string()) { + i += c.len_utf8(); // keep going with the loop + } else { + break; // stop if non-decoration + } + } + &s[i..] // new slice from position i +} + // https://eslint.org/docs/latest/rules/no-warning-comments#options // if location is "start" then ignore decorators, if "anywhere" then do not ignore decorators. If location is not provided then default to "start". impl Rule for NoWarningComments { @@ -82,8 +105,11 @@ impl Rule for NoWarningComments { CommentKind::Block => (span.start, span.end), _ => (span.start, span.end), }; - let comment_text = - ctx.source_text().get((span_pointers.0 as usize)..(span_pointers.1 as usize)); + let comment_text = ctx + .source_text() + .get((span_pointers.0 as usize)..(span_pointers.1 as usize)) + .unwrap() + .to_lowercase(); // match comment_text { // Some(text) => { @@ -91,11 +117,18 @@ impl Rule for NoWarningComments { // _ => {} // } println!("Source Text: {:?}", comment_text); - // Date: 10/10/2025 - // 1. Loop through the terms using &self.0.terms and use each term inside - // contains(). - // 2. Remove the decorators from the comment in question. - if comment_text.unwrap().to_string().to_lowercase().contains("todo") { + + let cleaned_text = trim_decorations_until_terms( + &comment_text, + &self.0.decorations.as_ref().unwrap(), + &self.0.terms.as_ref().unwrap(), + ); + println!("Cleaned Text: {:?}", cleaned_text); + + // Date: 17/10/2025 + // Next time, uncomment the test from the bottom and work from there. + // Understand uniform handling of strings in Rust (Patterns). + if comment_text.to_string().to_lowercase().contains("todo") { ctx.diagnostic(no_with_diagnostic(span)); } // println!("Span: {:?}", span); @@ -177,7 +210,8 @@ impl Rule for NoWarningComments { } } -// cargo test -p oxc_linter -- no_warning_comments +// cargo insta accept +// cargo test -p oxc_linter -- --nocapture no_warning_comments #[test] fn test() { use crate::tester::Tester; @@ -365,7 +399,7 @@ fn test() { ), ( "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - /////", + /////", Some( serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), ), From ec6b4b1fbb8e87a2f798c085fcf5298b755a58cc Mon Sep 17 00:00:00 2001 From: Stephen <519327+stevieing@users.noreply.github.com> Date: Fri, 24 Oct 2025 11:44:24 +0100 Subject: [PATCH 24/38] feat(linter): improve handling of decorations and terms in no_warning_comments rule all fail tests are now working as expected. --- .../src/rules/eslint/no_warning_comments.rs | 295 +++++++------- .../snapshots/eslint_no_warning_comments.snap | 362 ++++++++++++++++++ 2 files changed, 512 insertions(+), 145 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 0bb63930ff354..955ef342ea39e 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -110,42 +110,47 @@ impl Rule for NoWarningComments { .get((span_pointers.0 as usize)..(span_pointers.1 as usize)) .unwrap() .to_lowercase(); - // match comment_text { - // Some(text) => { - // } - // _ => {} - // } println!("Source Text: {:?}", comment_text); - let cleaned_text = trim_decorations_until_terms( - &comment_text, - &self.0.decorations.as_ref().unwrap(), - &self.0.terms.as_ref().unwrap(), - ); + // If there are no decorations it returns none so we need to match it otherwise it panics + let cleaned_text = match &self.0.decorations { + Some(decorations) => trim_decorations_until_terms( + &comment_text, + decorations, + self.0.terms.as_ref().unwrap(), + ), + None => &comment_text, + }; + println!("Cleaned Text: {:?}", cleaned_text); - // Date: 17/10/2025 - // Next time, uncomment the test from the bottom and work from there. - // Understand uniform handling of strings in Rust (Patterns). - if comment_text.to_string().to_lowercase().contains("todo") { - ctx.diagnostic(no_with_diagnostic(span)); + // performance might be an issue here with nested loops. Look at refactoring not with regex. + + // if the terms exist in the comment text then report a diagnostic + // if there are no terms then use default terms + match &self.0.terms { + Some(terms) => { + for term in terms { + if cleaned_text.to_lowercase().contains(&term.to_lowercase()) { + ctx.diagnostic(no_with_diagnostic(span)); + } + } + } + None => { + let default_terms = vec!["todo", "fixme", "xxx"]; + for term in default_terms { + if cleaned_text.to_lowercase().contains(&term) { + ctx.diagnostic(no_with_diagnostic(span)); + } + } + } } - // println!("Span: {:?}", span); }); - // println!("Source Text: {:?}", ctx.source_text()); - - // ctx.source_text().get((span.start as usize)..(span.end as usize)) - - // ctx.diagnostic( - // OxcDiagnostic::warn("Warning comments should be avoided") - // .with_help("Use a command-like statement that tells the user how to fix the issue") - // ) - // 1. create a copy of the source code x // 2. create a copy of the decoration, location and terms. We can get these from the configuration using the from_config function x - // 3. escape the decoration special characters. Decoration can be a string or an array of strings. + // 3. escape the decoration special characters. Decoration can be a string or an array of strings. x // if it is a line comment, skip // first // if it is a block comment, skip /* first: we are not there yet. // escape each decoration character until you reach a non-decoration character or the term itself, e.g. *todo @@ -271,125 +276,125 @@ fn test() { ]; let fail = vec![ - // ("// fixme", None), - // ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - // ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - // ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), - // ( - // "/* any fixme */", - // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - // ), - // ( - // "/* any FIXME */", - // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - // ), - // ( - // "/* any fIxMe */", - // Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), - // ), - // ( - // "// any fixme or todo", - // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - // ), - // ( - // "/* any fixme or todo */", - // Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), - // ), - // ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("/* fixme and todo */", None), - // ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - // ( - // "// regex [litera|$]", - // Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), - // ), - // ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), - // ( - // "/* eslint one-var: 2 */", - // Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), - // ), - // ( - // "/* any block comment with TODO, FIXME or XXX */", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "/* any block comment with (TODO, FIXME's or XXX!) */", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "/** - // *any block comment - // *with (TODO, FIXME's or XXX!) **/", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "// any comment with TODO, FIXME or XXX", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "// TODO: something really longer than 40 characters", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "/* TODO: something - // really longer than 40 characters - // and also a new line */", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ( - // "// Comment ending with term followed by punctuation TODO!", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// Comment ending with term including punctuation TODO!", - // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - // ), - // ( - // "// Comment ending with term including punctuation followed by more TODO!!!", - // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - // ), - // ( - // "// !TODO comment starting with term preceded by punctuation", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// !TODO comment starting with term including punctuation", - // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - // ), - // ( - // "// !!!TODO comment starting with term including punctuation preceded by more", - // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - // ), - // ( - // "// FIX!term ending with punctuation followed word character", - // Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), - // ), - // ( - // "// Term starting with punctuation preceded word character!FIX", - // Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), - // ), - // ( - // "//!XXX comment starting with no spaces (anywhere)", - // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), - // ), - // ( - // "//!XXX comment starting with no spaces (start)", - // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), - // ), - // ( - // "/* - // TODO undecorated multi-line block comment (start) - // */", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), - // ), + ("// fixme", None), + ("// any fixme", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("// any fixme", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ("// any FIXME", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ("// any fIxMe", Some(serde_json::json!([{ "terms": ["fixme"], "location": "anywhere" }]))), + ( + "/* any fixme */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "/* any FIXME */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "/* any fIxMe */", + Some(serde_json::json!([{ "terms": ["FIXME"], "location": "anywhere" }])), + ), + ( + "// any fixme or todo", + Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + ), + ( + "/* any fixme or todo */", + Some(serde_json::json!([{ "terms": ["fixme", "todo"], "location": "anywhere" }])), + ), + ("/* any fixme or todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* fixme and todo */", None), + ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + ( + "// regex [litera|$]", + Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), + ), + ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), + ( + "/* eslint one-var: 2 */", + Some(serde_json::json!([{ "terms": ["one"], "location": "anywhere" }])), + ), + ( + "/* any block comment with TODO, FIXME or XXX */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/* any block comment with (TODO, FIXME's or XXX!) */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/** + *any block comment + *with (TODO, FIXME's or XXX!) **/", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "// any comment with TODO, FIXME or XXX", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// TODO: something small", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// TODO: something really longer than 40 characters", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "/* TODO: something + really longer than 40 characters + and also a new line */", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// TODO: small", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ( + "// Comment ending with term followed by punctuation TODO!", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// Comment ending with term including punctuation TODO!", + Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + ), + ( + "// Comment ending with term including punctuation followed by more TODO!!!", + Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + ), + ( + "// !TODO comment starting with term preceded by punctuation", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// !TODO comment starting with term including punctuation", + Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + ), + ( + "// !!!TODO comment starting with term including punctuation preceded by more", + Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + ), + ( + "// FIX!term ending with punctuation followed word character", + Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), + ), + ( + "// Term starting with punctuation preceded word character!FIX", + Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), + ), + ( + "//!XXX comment starting with no spaces (anywhere)", + Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), + ), + ( + "//!XXX comment starting with no spaces (start)", + Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), + ), + ( + "/* + TODO undecorated multi-line block comment (start) + */", + Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), + ), ( "///// TODO decorated single-line comment with decoration array /////", diff --git a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap index c94f88d1f71e9..3be30aa2b531e 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap @@ -1,6 +1,368 @@ --- source: crates/oxc_linter/src/tester.rs --- + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // fixme + · ──────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // any fixme + · ──────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // any fixme + · ──────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // any FIXME + · ──────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // any fIxMe + · ──────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any fixme */ + · ─────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any FIXME */ + · ─────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any fIxMe */ + · ─────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // any fixme or todo + · ──────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // any fixme or todo + · ──────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any fixme or todo */ + · ─────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any fixme or todo */ + · ─────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any fixme or todo */ + · ─────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any fixme or todo */ + · ─────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* fixme and todo */ + · ──────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* fixme and todo */ + · ──────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* fixme and todo */ + · ──────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* fixme and todo */ + · ──────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any fixme */ + · ─────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* fixme! */ + · ──────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // regex [litera|$] + · ─────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* eslint one-var: 2 */ + · ─────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* eslint one-var: 2 */ + · ─────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any block comment with TODO, FIXME or XXX */ + · ─────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any block comment with TODO, FIXME or XXX */ + · ─────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any block comment with TODO, FIXME or XXX */ + · ─────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any block comment with (TODO, FIXME's or XXX!) */ + · ──────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any block comment with (TODO, FIXME's or XXX!) */ + · ──────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ /* any block comment with (TODO, FIXME's or XXX!) */ + · ──────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ ╭─▶ /** + 2 │ │ *any block comment + 3 │ ╰─▶ *with (TODO, FIXME's or XXX!) **/ + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ ╭─▶ /** + 2 │ │ *any block comment + 3 │ ╰─▶ *with (TODO, FIXME's or XXX!) **/ + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ ╭─▶ /** + 2 │ │ *any block comment + 3 │ ╰─▶ *with (TODO, FIXME's or XXX!) **/ + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // any comment with TODO, FIXME or XXX + · ────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // any comment with TODO, FIXME or XXX + · ────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // any comment with TODO, FIXME or XXX + · ────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // TODO: something small + · ──────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // TODO: something really longer than 40 characters + · ─────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ ╭─▶ /* TODO: something + 2 │ │ really longer than 40 characters + 3 │ ╰─▶ and also a new line */ + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // TODO: small + · ────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO + · ───────────────────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // Comment ending with term followed by punctuation TODO! + · ───────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // Comment ending with term including punctuation TODO! + · ─────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // Comment ending with term including punctuation followed by more TODO!!! + · ────────────────────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // !TODO comment starting with term preceded by punctuation + · ─────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // !TODO comment starting with term including punctuation + · ───────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // !!!TODO comment starting with term including punctuation preceded by more + · ──────────────────────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // FIX!term ending with punctuation followed word character + · ─────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // Term starting with punctuation preceded word character!FIX + · ───────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ //!XXX comment starting with no spaces (anywhere) + · ───────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ //!XXX comment starting with no spaces (start) + · ────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ ╭─▶ /* + 2 │ │ TODO undecorated multi-line block comment (start) + 3 │ ╰─▶ */ + ╰──── + help: Do not use the `with` statement. + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. ╭─[no_warning_comments.tsx:1:1] 1 │ ///// TODO decorated single-line comment with decoration array From 149b4ec1816c9d10bc040d4add736215b8f3edeb Mon Sep 17 00:00:00 2001 From: Stephen <519327+stevieing@users.noreply.github.com> Date: Fri, 24 Oct 2025 11:45:57 +0100 Subject: [PATCH 25/38] refactor(no_warning_comments): clean up commented-out decoration handling code --- .../oxc_linter/src/rules/eslint/no_warning_comments.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 955ef342ea39e..aa7c76c2f7f8f 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -85,14 +85,6 @@ impl Rule for NoWarningComments { // why do we need to use 0? let decorations = &self.0.decorations; println!("Decorations: {:?}", decorations); - // if let Some(decorations) = &self.decorations { - // for decoration in decorations { - // let escaped_decoration = regex::escape(decoration); - // let pattern = format!(r"^[\s{escaped_decoration}]*"); - // let re = regex::Regex::new(&pattern).unwrap(); - // source_text = re.replace(&source_text, "").to_string(); - // } - // } } _ => {} } @@ -179,6 +171,7 @@ impl Rule for NoWarningComments { // - it will report the message to the context with messageId unexpectedCooment with the data being the matched term and the comment but if it is too long it is truncated to 40 characters with an ellipsis } + // this could do with a tidy up. fn from_configuration(value: serde_json::Value) -> Self { // Read the configuration for term, decoration and location from _value and then // return NoWarningComments {} struct with the attributes terms, decoration and locations. @@ -197,7 +190,6 @@ impl Rule for NoWarningComments { }); } - // this is not working, decorations is always None if let Some(decorations_config) = config.get("decoration") { cfg.decorations = decorations_config.as_array().map(|arr| { arr.iter() From 2388677d3108f2d622f5107e8b5f9248f32d28dc Mon Sep 17 00:00:00 2001 From: Stephen <519327+stevieing@users.noreply.github.com> Date: Fri, 24 Oct 2025 11:55:50 +0100 Subject: [PATCH 26/38] fix(no_warning_comments): clarify location handling in comments and adjust test case --- crates/oxc_linter/src/rules/eslint/no_warning_comments.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index aa7c76c2f7f8f..0b701a78d9d00 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -138,6 +138,10 @@ impl Rule for NoWarningComments { } } } + + // 24/10/25 there is an issue with the location + // if you do not have location set to anywhere it will use the default of start + // if the location is start we should start with rather than contains }); // 1. create a copy of the source code x @@ -264,7 +268,7 @@ fn test() { // */", // None, // ), - // ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), + ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; let fail = vec![ From cd30ea5cea257f605543d7042f82254b1ebe9901 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 31 Oct 2025 17:44:38 +0000 Subject: [PATCH 27/38] Adding a small comment on the reference implementation --- crates/oxc_linter/src/rules/eslint/no_warning_comments.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 0b701a78d9d00..4f601c15036b5 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -92,6 +92,8 @@ impl Rule for NoWarningComments { let kind = comment.kind; println!("Kind: {:?}", kind); let span = comment.span; + + // ctx.source_range(comment.content_span()); let span_pointers: (u32, u32) = match kind { CommentKind::Line => ((span.start + 2) as u32, span.end), CommentKind::Block => (span.start, span.end), From 0e19a627b8767017289bd12777579562c826c55d Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 7 Nov 2025 11:45:08 +0000 Subject: [PATCH 28/38] fix(no_warning_comments): handle empty terms and adjust comment span calculations --- .../src/rules/eslint/no_warning_comments.rs | 37 +++++++++++++++---- .../snapshots/eslint_no_warning_comments.snap | 7 ---- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 4f601c15036b5..757c2ae3107c4 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -60,6 +60,9 @@ fn trim_decorations_until_terms<'a>( let mut i = 0; let s_len = s.len(); while i < s_len { + if terms.len() == 0 { + break; + } if terms.iter().any(|term| s[i..].starts_with(term)) { break; // stop if a term matches } @@ -96,7 +99,7 @@ impl Rule for NoWarningComments { // ctx.source_range(comment.content_span()); let span_pointers: (u32, u32) = match kind { CommentKind::Line => ((span.start + 2) as u32, span.end), - CommentKind::Block => (span.start, span.end), + CommentKind::Block => (span.start + 2, span.end), _ => (span.start, span.end), }; let comment_text = ctx @@ -109,11 +112,14 @@ impl Rule for NoWarningComments { // If there are no decorations it returns none so we need to match it otherwise it panics let cleaned_text = match &self.0.decorations { - Some(decorations) => trim_decorations_until_terms( - &comment_text, - decorations, - self.0.terms.as_ref().unwrap(), - ), + Some(decorations) => { + let empty_vec: Vec = Vec::new(); + trim_decorations_until_terms( + &comment_text, + decorations, + self.0.terms.as_ref().unwrap_or(&empty_vec), + ) + } None => &comment_text, }; @@ -134,8 +140,23 @@ impl Rule for NoWarningComments { None => { let default_terms = vec!["todo", "fixme", "xxx"]; for term in default_terms { - if cleaned_text.to_lowercase().contains(&term) { - ctx.diagnostic(no_with_diagnostic(span)); + match &self.0.location { + Some(location) => { + if location == "start" { + if !cleaned_text.to_lowercase().starts_with(&term) {} + } else { + if cleaned_text.to_lowercase().contains(&term) { + ctx.diagnostic(no_with_diagnostic(span)); + } + } + } + None => { + // If location=None that means, location="start" + if cleaned_text.to_lowercase().trim().starts_with(&term) { + ctx.diagnostic(no_with_diagnostic(span)); + } else { + } + } } } } diff --git a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap index 3be30aa2b531e..44133248f50a8 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap @@ -120,13 +120,6 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Do not use the `with` statement. - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ /* fixme and todo */ - · ──────────────────── - ╰──── - help: Do not use the `with` statement. - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. ╭─[no_warning_comments.tsx:1:1] 1 │ /* any fixme */ From f47f424bd04a1bd6033e2f84c412267de99736a3 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 7 Nov 2025 11:48:05 +0000 Subject: [PATCH 29/38] fix(tests): update test cases for no_warning_comments rule with clearer examples --- .../src/rules/eslint/no_warning_comments.rs | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 757c2ae3107c4..21a85b9a83917 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -275,22 +275,23 @@ fn test() { // "#, // None, // ), - // ( - // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + // TODO: 7/11/12 (Stopped here; we need to fix this in the next session) + ( + r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - // var x = 10; - // "#, - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), - // ( - // "/** multi-line block comment with lines starting with - // TODO - // FIXME or - // XXX - // */", - // None, - // ), + var x = 10; + "#, + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), + ( + "/** multi-line block comment with lines starting with + TODO + FIXME or + XXX + */", + None, + ), ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; From e43e6b78a16ddae95ba5852280c8255885378a55 Mon Sep 17 00:00:00 2001 From: Stephen <519327+stevieing@users.noreply.github.com> Date: Fri, 14 Nov 2025 12:00:11 +0000 Subject: [PATCH 30/38] fix(no_warning_comments): improve comment handling and optimize term matching logic --- .../src/rules/eslint/no_warning_comments.rs | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 21a85b9a83917..70392dabb2d86 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -96,7 +96,6 @@ impl Rule for NoWarningComments { println!("Kind: {:?}", kind); let span = comment.span; - // ctx.source_range(comment.content_span()); let span_pointers: (u32, u32) = match kind { CommentKind::Line => ((span.start + 2) as u32, span.end), CommentKind::Block => (span.start + 2, span.end), @@ -108,6 +107,11 @@ impl Rule for NoWarningComments { .unwrap() .to_lowercase(); + // it would be better to strip comments with no-warning-comments + if comment_text.contains("no-warning-comments") { + return; + } + println!("Source Text: {:?}", comment_text); // If there are no decorations it returns none so we need to match it otherwise it panics @@ -125,6 +129,12 @@ impl Rule for NoWarningComments { println!("Cleaned Text: {:?}", cleaned_text); + let words = cleaned_text + .split(|c: char| !c.is_alphanumeric()) + .filter(|w| !w.is_empty()) + .map(|w| w.to_lowercase()) + .collect::>(); + // performance might be an issue here with nested loops. Look at refactoring not with regex. // if the terms exist in the comment text then report a diagnostic @@ -132,7 +142,7 @@ impl Rule for NoWarningComments { match &self.0.terms { Some(terms) => { for term in terms { - if cleaned_text.to_lowercase().contains(&term.to_lowercase()) { + if words.contains(&term.to_lowercase()) { ctx.diagnostic(no_with_diagnostic(span)); } } @@ -143,16 +153,15 @@ impl Rule for NoWarningComments { match &self.0.location { Some(location) => { if location == "start" { - if !cleaned_text.to_lowercase().starts_with(&term) {} + if !(words[0] == term.to_lowercase()) {} } else { - if cleaned_text.to_lowercase().contains(&term) { + if words.contains(&term.to_lowercase()) { ctx.diagnostic(no_with_diagnostic(span)); } } } None => { - // If location=None that means, location="start" - if cleaned_text.to_lowercase().trim().starts_with(&term) { + if (words[0] == term.to_lowercase()) { ctx.diagnostic(no_with_diagnostic(span)); } else { } @@ -260,22 +269,21 @@ fn test() { // ), // ("/* any block comment with TODO, FIXME or XXX */", None), // ("/* any block comment with (TODO, FIXME's or XXX!) */", None), - // ( - // "// comments containing terms as substrings like TodoMVC", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// special regex characters don't cause a problem", - // Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), - // ), - // ( - // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + ( + "// comments containing terms as substrings like TodoMVC", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// special regex characters don't cause a problem", + Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), + ), + ( + r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - // var x = 10; - // "#, - // None, - // ), - // TODO: 7/11/12 (Stopped here; we need to fix this in the next session) + var x = 10; + "#, + None, + ), ( r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ @@ -292,6 +300,7 @@ fn test() { */", None, ), + // This test is now failing ... ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; From ecd9011a188189e0dc28618ba7c2f5d68ebc8861 Mon Sep 17 00:00:00 2001 From: yoldas Date: Fri, 19 Dec 2025 11:57:26 +0000 Subject: [PATCH 31/38] Default to "start" if location is not provided --- .../oxc_linter/src/rules/eslint/no_warning_comments.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 70392dabb2d86..5a1df409633da 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -129,6 +129,8 @@ impl Rule for NoWarningComments { println!("Cleaned Text: {:?}", cleaned_text); + // We have handled ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))) + // But it made the following to fail: Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), let words = cleaned_text .split(|c: char| !c.is_alphanumeric()) .filter(|w| !w.is_empty()) @@ -139,9 +141,13 @@ impl Rule for NoWarningComments { // if the terms exist in the comment text then report a diagnostic // if there are no terms then use default terms + println!("Terms: {:?}", self.0.terms); + println!("Location: {:?}", self.0.location); match &self.0.terms { Some(terms) => { for term in terms { + println!("Condition: {:?}", words.contains(&term.to_lowercase())); + println!("Words: {:?}", words); if words.contains(&term.to_lowercase()) { ctx.diagnostic(no_with_diagnostic(span)); } @@ -163,7 +169,6 @@ impl Rule for NoWarningComments { None => { if (words[0] == term.to_lowercase()) { ctx.diagnostic(no_with_diagnostic(span)); - } else { } } } @@ -236,6 +241,9 @@ impl Rule for NoWarningComments { if let Some(location_config) = config.get("location") { cfg.location = location_config.as_str().map(|s| s.to_string()); + } else { + // Default to "start" if location is not provided + cfg.location = Some("start".to_string()); } } From 5a1c755db5c834fb256518d5e979117957f01ca2 Mon Sep 17 00:00:00 2001 From: Stephen <519327+stevieing@users.noreply.github.com> Date: Fri, 9 Jan 2026 11:53:40 +0000 Subject: [PATCH 32/38] fix(no_warning_comments): add term matching function and update comment handling logic This function is not working correctly so we need to decide on the various scenarios. --- .../src/rules/eslint/no_warning_comments.rs | 163 ++++++++++-------- .../snapshots/eslint_no_warning_comments.snap | 101 ----------- 2 files changed, 95 insertions(+), 169 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 5a1df409633da..d254e93fbc47d 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -76,6 +76,29 @@ fn trim_decorations_until_terms<'a>( &s[i..] // new slice from position i } +/// Checks if any word matches the term, including non-alphanumeric characters. +/// if term is "todo", it matches "todo", "todo!", "(todo)", etc. +/// This is done by checking if the term is a substring of any word. +/// if word is todoMVC and term is todo it will not match. +/// This function is not working correctly so we need to go through the various options. +fn any_word_matches_term(words: &[String], term: &str) -> bool { + let term_lower = term.to_lowercase(); + let is_term_alnum = term_lower.chars().all(|c| c.is_alphanumeric()); + words.iter().any(|word| { + if is_term_alnum { + // If the word starts with the term and the rest is non-alphanumeric or empty, it's a match + if word.starts_with(&term_lower) { + let rest = &word[term_lower.len()..]; + rest.chars().all(|c| !c.is_alphanumeric()) + } else { + false + } + } else { + word == &term_lower + } + }) +} + // https://eslint.org/docs/latest/rules/no-warning-comments#options // if location is "start" then ignore decorators, if "anywhere" then do not ignore decorators. If location is not provided then default to "start". impl Rule for NoWarningComments { @@ -132,7 +155,8 @@ impl Rule for NoWarningComments { // We have handled ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))) // But it made the following to fail: Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), let words = cleaned_text - .split(|c: char| !c.is_alphanumeric()) + .split_whitespace() + // .split(|c: char| !c.is_alphanumeric()) .filter(|w| !w.is_empty()) .map(|w| w.to_lowercase()) .collect::>(); @@ -141,14 +165,15 @@ impl Rule for NoWarningComments { // if the terms exist in the comment text then report a diagnostic // if there are no terms then use default terms - println!("Terms: {:?}", self.0.terms); - println!("Location: {:?}", self.0.location); + // println!("Terms: {:?}", self.0.terms); + // println!("Location: {:?}", self.0.location); + println!("Words: {:?}", words); match &self.0.terms { Some(terms) => { for term in terms { - println!("Condition: {:?}", words.contains(&term.to_lowercase())); + println!("Condition: {:?}", any_word_matches_term(&words, term)); println!("Words: {:?}", words); - if words.contains(&term.to_lowercase()) { + if any_word_matches_term(&words, term) { ctx.diagnostic(no_with_diagnostic(span)); } } @@ -161,7 +186,7 @@ impl Rule for NoWarningComments { if location == "start" { if !(words[0] == term.to_lowercase()) {} } else { - if words.contains(&term.to_lowercase()) { + if any_word_matches_term(&words, term) { ctx.diagnostic(no_with_diagnostic(span)); } } @@ -343,10 +368,11 @@ fn test() { ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - ( - "// regex [litera|$]", - Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), - ), + // this test is now failing ... + // ( + // "// regex [litera|$]", + // Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), + // ), ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), ( "/* eslint one-var: 2 */", @@ -390,68 +416,69 @@ fn test() { "// Comment ending with term followed by punctuation TODO!", Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), ), + // this test is now failing ... ( "// Comment ending with term including punctuation TODO!", Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), ), - ( - "// Comment ending with term including punctuation followed by more TODO!!!", - Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - ), - ( - "// !TODO comment starting with term preceded by punctuation", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// !TODO comment starting with term including punctuation", - Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - ), - ( - "// !!!TODO comment starting with term including punctuation preceded by more", - Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - ), - ( - "// FIX!term ending with punctuation followed word character", - Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), - ), - ( - "// Term starting with punctuation preceded word character!FIX", - Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), - ), - ( - "//!XXX comment starting with no spaces (anywhere)", - Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), - ), - ( - "//!XXX comment starting with no spaces (start)", - Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), - ), - ( - "/* - TODO undecorated multi-line block comment (start) - */", - Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), - ), - ( - "///// TODO decorated single-line comment with decoration array - /////", - Some( - serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - ), - ), - ( - "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - /////", - Some( - serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - ), - ), - ( - "//**TODO term starts with a decoration character", - Some( - serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), - ), - ), + // ( + // "// Comment ending with term including punctuation followed by more TODO!!!", + // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + // ), + // ( + // "// !TODO comment starting with term preceded by punctuation", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// !TODO comment starting with term including punctuation", + // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + // ), + // ( + // "// !!!TODO comment starting with term including punctuation preceded by more", + // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + // ), + // ( + // "// FIX!term ending with punctuation followed word character", + // Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), + // ), + // ( + // "// Term starting with punctuation preceded word character!FIX", + // Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), + // ), + // ( + // "//!XXX comment starting with no spaces (anywhere)", + // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), + // ), + // ( + // "//!XXX comment starting with no spaces (start)", + // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), + // ), + // ( + // "/* + // TODO undecorated multi-line block comment (start) + // */", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), + // ), + // ( + // "///// TODO decorated single-line comment with decoration array + // /////", + // Some( + // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + // ), + // ), + // ( + // "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + // /////", + // Some( + // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + // ), + // ), + // ( + // "//**TODO term starts with a decoration character", + // Some( + // serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), + // ), + // ), ]; Tester::new(NoWarningComments::NAME, NoWarningComments::PLUGIN, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap index 44133248f50a8..280008216d77e 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap @@ -134,13 +134,6 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Do not use the `with` statement. - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ // regex [litera|$] - · ─────────────────── - ╰──── - help: Do not use the `with` statement. - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. ╭─[no_warning_comments.tsx:1:1] 1 │ /* eslint one-var: 2 */ @@ -284,97 +277,3 @@ source: crates/oxc_linter/src/tester.rs · ───────────────────────────────────────────────────────── ╰──── help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ // Comment ending with term including punctuation TODO! - · ─────────────────────────────────────────────────────── - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ // Comment ending with term including punctuation followed by more TODO!!! - · ────────────────────────────────────────────────────────────────────────── - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ // !TODO comment starting with term preceded by punctuation - · ─────────────────────────────────────────────────────────── - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ // !TODO comment starting with term including punctuation - · ───────────────────────────────────────────────────────── - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ // !!!TODO comment starting with term including punctuation preceded by more - · ──────────────────────────────────────────────────────────────────────────── - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ // FIX!term ending with punctuation followed word character - · ─────────────────────────────────────────────────────────── - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ // Term starting with punctuation preceded word character!FIX - · ───────────────────────────────────────────────────────────── - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ //!XXX comment starting with no spaces (anywhere) - · ───────────────────────────────────────────────── - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ //!XXX comment starting with no spaces (start) - · ────────────────────────────────────────────── - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ ╭─▶ /* - 2 │ │ TODO undecorated multi-line block comment (start) - 3 │ ╰─▶ */ - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ ///// TODO decorated single-line comment with decoration array - · ────────────────────────────────────────────────────────────── - 2 │ ///// - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ ///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - · ────────────────────────────────────────────────────────────────────────────────────── - 2 │ ///// - ╰──── - help: Do not use the `with` statement. - - ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. - ╭─[no_warning_comments.tsx:1:1] - 1 │ //**TODO term starts with a decoration character - · ──────────────────────────────────────────────── - ╰──── - help: Do not use the `with` statement. From 2a4d2070ac47a9dec666b4075882e947857ea833 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 16 Jan 2026 11:58:03 +0000 Subject: [PATCH 33/38] fix(no_warning_comments): enhance term matching logic and update snapshot tests --- .../src/rules/eslint/no_warning_comments.rs | 208 +++++++++--------- .../snapshots/eslint_no_warning_comments.snap | 101 +++++++++ 2 files changed, 204 insertions(+), 105 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index d254e93fbc47d..a9580d3fdde96 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -81,22 +81,21 @@ fn trim_decorations_until_terms<'a>( /// This is done by checking if the term is a substring of any word. /// if word is todoMVC and term is todo it will not match. /// This function is not working correctly so we need to go through the various options. +/// --- "/* eslint one-var: 2 */" --- fn any_word_matches_term(words: &[String], term: &str) -> bool { let term_lower = term.to_lowercase(); let is_term_alnum = term_lower.chars().all(|c| c.is_alphanumeric()); - words.iter().any(|word| { + let x = words.iter().any(|word| { if is_term_alnum { // If the word starts with the term and the rest is non-alphanumeric or empty, it's a match - if word.starts_with(&term_lower) { - let rest = &word[term_lower.len()..]; - rest.chars().all(|c| !c.is_alphanumeric()) - } else { - false - } + word.contains(&term_lower) } else { - word == &term_lower + word.contains(&term_lower) + // word == &term_lower } - }) + }); + println!("x = {:?}", x); + x } // https://eslint.org/docs/latest/rules/no-warning-comments#options @@ -192,7 +191,7 @@ impl Rule for NoWarningComments { } } None => { - if (words[0] == term.to_lowercase()) { + if words[0] == term.to_lowercase() { ctx.diagnostic(no_with_diagnostic(span)); } } @@ -302,39 +301,39 @@ fn test() { // ), // ("/* any block comment with TODO, FIXME or XXX */", None), // ("/* any block comment with (TODO, FIXME's or XXX!) */", None), - ( - "// comments containing terms as substrings like TodoMVC", - Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - ), - ( - "// special regex characters don't cause a problem", - Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), - ), - ( - r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + // ( + // "// comments containing terms as substrings like TodoMVC", + // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + // ), + // ( + // "// special regex characters don't cause a problem", + // Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), + // ), + // ( + // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - var x = 10; - "#, - None, - ), - ( - r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + // var x = 10; + // "#, + // None, + // ), + // ( + // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - var x = 10; - "#, - Some(serde_json::json!([{ "location": "anywhere" }])), - ), - ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), - ( - "/** multi-line block comment with lines starting with - TODO - FIXME or - XXX - */", - None, - ), - // This test is now failing ... - ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), + // var x = 10; + // "#, + // Some(serde_json::json!([{ "location": "anywhere" }])), + // ), + // ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), + // ( + // "/** multi-line block comment with lines starting with + // TODO + // FIXME or + // XXX + // */", + // None, + // ), + // // This test is now failing ... + // ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; let fail = vec![ @@ -368,11 +367,10 @@ fn test() { ("/* fixme and todo */", Some(serde_json::json!([{ "location": "anywhere" }]))), ("/* any fixme */", Some(serde_json::json!([{ "location": "anywhere" }]))), ("/* fixme! */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - // this test is now failing ... - // ( - // "// regex [litera|$]", - // Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), - // ), + ( + "// regex [litera|$]", + Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), + ), ("/* eslint one-var: 2 */", Some(serde_json::json!([{ "terms": ["eslint"] }]))), ( "/* eslint one-var: 2 */", @@ -416,69 +414,69 @@ fn test() { "// Comment ending with term followed by punctuation TODO!", Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), ), - // this test is now failing ... + // // this test is now failing ... ( "// Comment ending with term including punctuation TODO!", Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), ), - // ( - // "// Comment ending with term including punctuation followed by more TODO!!!", - // Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), - // ), - // ( - // "// !TODO comment starting with term preceded by punctuation", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// !TODO comment starting with term including punctuation", - // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - // ), - // ( - // "// !!!TODO comment starting with term including punctuation preceded by more", - // Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), - // ), - // ( - // "// FIX!term ending with punctuation followed word character", - // Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), - // ), - // ( - // "// Term starting with punctuation preceded word character!FIX", - // Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), - // ), - // ( - // "//!XXX comment starting with no spaces (anywhere)", - // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), - // ), - // ( - // "//!XXX comment starting with no spaces (start)", - // Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), - // ), - // ( - // "/* - // TODO undecorated multi-line block comment (start) - // */", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), - // ), - // ( - // "///// TODO decorated single-line comment with decoration array - // /////", - // Some( - // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - // ), - // ), - // ( - // "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) - // /////", - // Some( - // serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), - // ), - // ), - // ( - // "//**TODO term starts with a decoration character", - // Some( - // serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), - // ), - // ), + ( + "// Comment ending with term including punctuation followed by more TODO!!!", + Some(serde_json::json!([{ "terms": ["todo!"], "location": "anywhere" }])), + ), + ( + "// !TODO comment starting with term preceded by punctuation", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// !TODO comment starting with term including punctuation", + Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + ), + ( + "// !!!TODO comment starting with term including punctuation preceded by more", + Some(serde_json::json!([{ "terms": ["!todo"], "location": "anywhere" }])), + ), + ( + "// FIX!term ending with punctuation followed word character", + Some(serde_json::json!([{ "terms": ["FIX!"], "location": "anywhere" }])), + ), + ( + "// Term starting with punctuation preceded word character!FIX", + Some(serde_json::json!([{ "terms": ["!FIX"], "location": "anywhere" }])), + ), + ( + "//!XXX comment starting with no spaces (anywhere)", + Some(serde_json::json!([{ "terms": ["!xxx"], "location": "anywhere" }])), + ), + ( + "//!XXX comment starting with no spaces (start)", + Some(serde_json::json!([{ "terms": ["!xxx"], "location": "start" }])), + ), + ( + "/* + TODO undecorated multi-line block comment (start) + */", + Some(serde_json::json!([{ "terms": ["todo"], "location": "start" }])), + ), + ( + "///// TODO decorated single-line comment with decoration array + /////", + Some( + serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + ), + ), + ( + "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + /////", + Some( + serde_json::json!([ { "terms": ["todo"], "location": "start", "decoration": ["*", "/"] }, ]), + ), + ), + ( + "//**TODO term starts with a decoration character", + Some( + serde_json::json!([ { "terms": ["*todo"], "location": "start", "decoration": ["*"] }, ]), + ), + ), ]; Tester::new(NoWarningComments::NAME, NoWarningComments::PLUGIN, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap index 280008216d77e..44133248f50a8 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_warning_comments.snap @@ -134,6 +134,13 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Do not use the `with` statement. + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // regex [litera|$] + · ─────────────────── + ╰──── + help: Do not use the `with` statement. + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. ╭─[no_warning_comments.tsx:1:1] 1 │ /* eslint one-var: 2 */ @@ -277,3 +284,97 @@ source: crates/oxc_linter/src/tester.rs · ───────────────────────────────────────────────────────── ╰──── help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // Comment ending with term including punctuation TODO! + · ─────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // Comment ending with term including punctuation followed by more TODO!!! + · ────────────────────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // !TODO comment starting with term preceded by punctuation + · ─────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // !TODO comment starting with term including punctuation + · ───────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // !!!TODO comment starting with term including punctuation preceded by more + · ──────────────────────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // FIX!term ending with punctuation followed word character + · ─────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ // Term starting with punctuation preceded word character!FIX + · ───────────────────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ //!XXX comment starting with no spaces (anywhere) + · ───────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ //!XXX comment starting with no spaces (start) + · ────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ ╭─▶ /* + 2 │ │ TODO undecorated multi-line block comment (start) + 3 │ ╰─▶ */ + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ ///// TODO decorated single-line comment with decoration array + · ────────────────────────────────────────────────────────────── + 2 │ ///// + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ ///*/*/ TODO decorated single-line comment with multiple decoration characters (start) + · ────────────────────────────────────────────────────────────────────────────────────── + 2 │ ///// + ╰──── + help: Do not use the `with` statement. + + ⚠ eslint(no-warning-comments): Unexpected use of `with` statement. + ╭─[no_warning_comments.tsx:1:1] + 1 │ //**TODO term starts with a decoration character + · ──────────────────────────────────────────────── + ╰──── + help: Do not use the `with` statement. From 8321fb67728aa0a20e15a01c2a4a52276eda8149 Mon Sep 17 00:00:00 2001 From: yoldas Date: Fri, 23 Jan 2026 11:32:38 +0000 Subject: [PATCH 34/38] Check match based on whether the word is alphanumeric --- .gitignore | 2 + .../src/rules/eslint/no_warning_comments.rs | 118 +++++++++--------- 2 files changed, 60 insertions(+), 60 deletions(-) diff --git a/.gitignore b/.gitignore index e8224a0b34300..4bdb445584bc1 100644 --- a/.gitignore +++ b/.gitignore @@ -27,6 +27,8 @@ target/ /editors/vscode/test_workspace_second/ /editors/vscode/*.test.code-workspace +.vscode/settings.json + # Cloned conformance repos tasks/coverage/babel/ tasks/coverage/test262/ diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index a9580d3fdde96..ad6a5324089c1 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -84,15 +84,14 @@ fn trim_decorations_until_terms<'a>( /// --- "/* eslint one-var: 2 */" --- fn any_word_matches_term(words: &[String], term: &str) -> bool { let term_lower = term.to_lowercase(); - let is_term_alnum = term_lower.chars().all(|c| c.is_alphanumeric()); + // let is_term_alnum = term_lower.chars().all(|c| c.is_alphanumeric()); let x = words.iter().any(|word| { - if is_term_alnum { - // If the word starts with the term and the rest is non-alphanumeric or empty, it's a match - word.contains(&term_lower) - } else { - word.contains(&term_lower) - // word == &term_lower - } + // term is alphanumeric, word is alphanumeric, check for exact match ; todo && todoMVC => todo == todoMVC + // term is alphanumeric, word is not alphanumeric, check for contains ; todo && todo! => todo! contains todo + + let word_lower = word.to_lowercase(); + let is_word_alnum = word_lower.chars().all(|c| c.is_alphanumeric()); + if is_word_alnum { word_lower == term_lower } else { word_lower.contains(&term_lower) } }); println!("x = {:?}", x); x @@ -282,58 +281,57 @@ fn test() { use crate::tester::Tester; let pass = vec![ - // ("// any comment", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - // ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - // ("// any comment", None), - // ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "// any comment with TODO, FIXME or XXX", - // Some(serde_json::json!([{ "location": "start" }])), - // ), - // ("// any comment with TODO, FIXME or XXX", None), - // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), - // ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), - // ("/* any block comment */", None), - // ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), - // ( - // "/* any block comment with TODO, FIXME or XXX */", - // Some(serde_json::json!([{ "location": "start" }])), - // ), - // ("/* any block comment with TODO, FIXME or XXX */", None), - // ("/* any block comment with (TODO, FIXME's or XXX!) */", None), - // ( - // "// comments containing terms as substrings like TodoMVC", - // Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), - // ), - // ( - // "// special regex characters don't cause a problem", - // Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), - // ), - // ( - // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - - // var x = 10; - // "#, - // None, - // ), - // ( - // r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ - - // var x = 10; - // "#, - // Some(serde_json::json!([{ "location": "anywhere" }])), - // ), - // ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), - // ( - // "/** multi-line block comment with lines starting with - // TODO - // FIXME or - // XXX - // */", - // None, - // ), - // // This test is now failing ... - // ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), + ("// any comment", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + ("// any comment", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + ("// any comment", None), + ("// any comment", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "// any comment with TODO, FIXME or XXX", + Some(serde_json::json!([{ "location": "start" }])), + ), + ("// any comment with TODO, FIXME or XXX", None), + ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme"] }]))), + ("/* any block comment */", Some(serde_json::json!([{ "terms": ["fixme", "todo"] }]))), + ("/* any block comment */", None), + ("/* any block comment */", Some(serde_json::json!([{ "location": "anywhere" }]))), + ( + "/* any block comment with TODO, FIXME or XXX */", + Some(serde_json::json!([{ "location": "start" }])), + ), + ("/* any block comment with TODO, FIXME or XXX */", None), + ("/* any block comment with (TODO, FIXME's or XXX!) */", None), + ( + "// comments containing terms as substrings like TodoMVC", + Some(serde_json::json!([{ "terms": ["todo"], "location": "anywhere" }])), + ), + ( + "// special regex characters don't cause a problem", + Some(serde_json::json!([{ "terms": ["[aeiou]"], "location": "anywhere" }])), + ), + ( + r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + + var x = 10; + "#, + None, + ), + ( + r#"/*eslint no-warning-comments: [2, { "terms": ["todo", "fixme", "any other term"], "location": "anywhere" }]*/ + + var x = 10; + "#, + Some(serde_json::json!([{ "location": "anywhere" }])), + ), + ("// foo", Some(serde_json::json!([{ "terms": ["foo-bar"] }]))), + ( + "/** multi-line block comment with lines starting with + TODO + FIXME or + XXX + */", + None, + ), + ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))), ]; let fail = vec![ From 9d611f7419142aae47c5d44d4384e59312596d33 Mon Sep 17 00:00:00 2001 From: yoldas Date: Fri, 23 Jan 2026 11:35:22 +0000 Subject: [PATCH 35/38] Remove prints --- .../src/rules/eslint/no_warning_comments.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index ad6a5324089c1..0eb1023994c3d 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -85,16 +85,14 @@ fn trim_decorations_until_terms<'a>( fn any_word_matches_term(words: &[String], term: &str) -> bool { let term_lower = term.to_lowercase(); // let is_term_alnum = term_lower.chars().all(|c| c.is_alphanumeric()); - let x = words.iter().any(|word| { + words.iter().any(|word| { // term is alphanumeric, word is alphanumeric, check for exact match ; todo && todoMVC => todo == todoMVC // term is alphanumeric, word is not alphanumeric, check for contains ; todo && todo! => todo! contains todo let word_lower = word.to_lowercase(); let is_word_alnum = word_lower.chars().all(|c| c.is_alphanumeric()); if is_word_alnum { word_lower == term_lower } else { word_lower.contains(&term_lower) } - }); - println!("x = {:?}", x); - x + }) } // https://eslint.org/docs/latest/rules/no-warning-comments#options @@ -102,19 +100,16 @@ fn any_word_matches_term(words: &[String], term: &str) -> bool { impl Rule for NoWarningComments { fn run_once(&self, ctx: &LintContext) { ctx.semantic().comments().iter().for_each(|comment| { - println!("&self: {:?}", &self.0); let mut source_text = ctx.source_text(); match &self.0.location { Some(loc) if loc != "anywhere" => { // why do we need to use 0? let decorations = &self.0.decorations; - println!("Decorations: {:?}", decorations); } _ => {} } let kind = comment.kind; - println!("Kind: {:?}", kind); let span = comment.span; let span_pointers: (u32, u32) = match kind { @@ -133,8 +128,6 @@ impl Rule for NoWarningComments { return; } - println!("Source Text: {:?}", comment_text); - // If there are no decorations it returns none so we need to match it otherwise it panics let cleaned_text = match &self.0.decorations { Some(decorations) => { @@ -148,8 +141,6 @@ impl Rule for NoWarningComments { None => &comment_text, }; - println!("Cleaned Text: {:?}", cleaned_text); - // We have handled ("//!TODO ", Some(serde_json::json!([{ "decoration": ["*"] }]))) // But it made the following to fail: Some(serde_json::json!([{ "terms": ["[litera|$]"], "location": "anywhere" }])), let words = cleaned_text @@ -165,12 +156,9 @@ impl Rule for NoWarningComments { // if there are no terms then use default terms // println!("Terms: {:?}", self.0.terms); // println!("Location: {:?}", self.0.location); - println!("Words: {:?}", words); match &self.0.terms { Some(terms) => { for term in terms { - println!("Condition: {:?}", any_word_matches_term(&words, term)); - println!("Words: {:?}", words); if any_word_matches_term(&words, term) { ctx.diagnostic(no_with_diagnostic(span)); } From 461dd4b12243b7a272ce818a813fedbbcfb059f3 Mon Sep 17 00:00:00 2001 From: yoldas Date: Fri, 23 Jan 2026 11:39:15 +0000 Subject: [PATCH 36/38] Fix warning --- crates/oxc_linter/src/rules/eslint/no_warning_comments.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 0eb1023994c3d..ab80de9823d22 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -100,7 +100,7 @@ fn any_word_matches_term(words: &[String], term: &str) -> bool { impl Rule for NoWarningComments { fn run_once(&self, ctx: &LintContext) { ctx.semantic().comments().iter().for_each(|comment| { - let mut source_text = ctx.source_text(); + let source_text = ctx.source_text(); match &self.0.location { Some(loc) if loc != "anywhere" => { // why do we need to use 0? @@ -154,8 +154,6 @@ impl Rule for NoWarningComments { // if the terms exist in the comment text then report a diagnostic // if there are no terms then use default terms - // println!("Terms: {:?}", self.0.terms); - // println!("Location: {:?}", self.0.location); match &self.0.terms { Some(terms) => { for term in terms { From 6a74e0be675e158256b94f11110b258220a07fe2 Mon Sep 17 00:00:00 2001 From: yoldas Date: Fri, 23 Jan 2026 11:45:19 +0000 Subject: [PATCH 37/38] Fix warnings --- .../src/rules/eslint/no_warning_comments.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index ab80de9823d22..0d642d301fa58 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -100,22 +100,13 @@ fn any_word_matches_term(words: &[String], term: &str) -> bool { impl Rule for NoWarningComments { fn run_once(&self, ctx: &LintContext) { ctx.semantic().comments().iter().for_each(|comment| { - let source_text = ctx.source_text(); - match &self.0.location { - Some(loc) if loc != "anywhere" => { - // why do we need to use 0? - let decorations = &self.0.decorations; - } - _ => {} - } - let kind = comment.kind; let span = comment.span; let span_pointers: (u32, u32) = match kind { CommentKind::Line => ((span.start + 2) as u32, span.end), CommentKind::Block => (span.start + 2, span.end), - _ => (span.start, span.end), + // _ => (span.start, span.end), }; let comment_text = ctx .source_text() From 5376caf9a2f72cda46a3765c066a419952981ca7 Mon Sep 17 00:00:00 2001 From: Dasun Pubudumal Date: Fri, 30 Jan 2026 11:38:40 +0000 Subject: [PATCH 38/38] Update clippy fixes --- .../src/rules/eslint/no_warning_comments.rs | 79 ++++++++----------- 1 file changed, 34 insertions(+), 45 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs index 0d642d301fa58..197852aa0dc14 100644 --- a/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs +++ b/crates/oxc_linter/src/rules/eslint/no_warning_comments.rs @@ -1,9 +1,9 @@ -use oxc_ast::CommentKind; +use cow_utils::CowUtils; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_span::Span; -use crate::{AstNode, context::LintContext, rule::Rule}; +use crate::{context::LintContext, rule::Rule}; fn no_with_diagnostic(span: Span) -> OxcDiagnostic { OxcDiagnostic::warn("Unexpected use of `with` statement.") @@ -52,15 +52,15 @@ declare_oxc_lint!( // Refactor this function to make it more Rusty fn trim_decorations_until_terms<'a>( - s: &'a str, // can accept string slice as input; not an owned String - decorations: &Vec, // slice of string slices - terms: &Vec, // slice of string slices + s: &'a str, // can accept string slice as input; not an owned String + decorations: &[String], // slice of string slices + terms: &[String], // slice of string slices ) -> &'a str { // return a slice of the original string (&'a str) without copying let mut i = 0; let s_len = s.len(); while i < s_len { - if terms.len() == 0 { + if terms.is_empty() { break; } if terms.iter().any(|term| s[i..].starts_with(term)) { @@ -83,15 +83,15 @@ fn trim_decorations_until_terms<'a>( /// This function is not working correctly so we need to go through the various options. /// --- "/* eslint one-var: 2 */" --- fn any_word_matches_term(words: &[String], term: &str) -> bool { - let term_lower = term.to_lowercase(); + let term_lower = term.cow_to_lowercase(); // let is_term_alnum = term_lower.chars().all(|c| c.is_alphanumeric()); words.iter().any(|word| { // term is alphanumeric, word is alphanumeric, check for exact match ; todo && todoMVC => todo == todoMVC // term is alphanumeric, word is not alphanumeric, check for contains ; todo && todo! => todo! contains todo - let word_lower = word.to_lowercase(); - let is_word_alnum = word_lower.chars().all(|c| c.is_alphanumeric()); - if is_word_alnum { word_lower == term_lower } else { word_lower.contains(&term_lower) } + let word_lower = word.cow_to_lowercase(); + let is_word_alnum = word_lower.chars().all(char::is_alphanumeric); + if is_word_alnum { word_lower == term_lower } else { word_lower.contains(&*term_lower) } }) } @@ -100,19 +100,14 @@ fn any_word_matches_term(words: &[String], term: &str) -> bool { impl Rule for NoWarningComments { fn run_once(&self, ctx: &LintContext) { ctx.semantic().comments().iter().for_each(|comment| { - let kind = comment.kind; let span = comment.span; - let span_pointers: (u32, u32) = match kind { - CommentKind::Line => ((span.start + 2) as u32, span.end), - CommentKind::Block => (span.start + 2, span.end), - // _ => (span.start, span.end), - }; + let span_pointers: (u32, u32) = (span.start + 2, span.end); let comment_text = ctx .source_text() .get((span_pointers.0 as usize)..(span_pointers.1 as usize)) .unwrap() - .to_lowercase(); + .cow_to_lowercase(); // it would be better to strip comments with no-warning-comments if comment_text.contains("no-warning-comments") { @@ -138,44 +133,38 @@ impl Rule for NoWarningComments { .split_whitespace() // .split(|c: char| !c.is_alphanumeric()) .filter(|w| !w.is_empty()) - .map(|w| w.to_lowercase()) + .map(|w| cow_utils::CowUtils::cow_to_lowercase(w).into_owned()) .collect::>(); // performance might be an issue here with nested loops. Look at refactoring not with regex. // if the terms exist in the comment text then report a diagnostic // if there are no terms then use default terms - match &self.0.terms { - Some(terms) => { - for term in terms { - if any_word_matches_term(&words, term) { - ctx.diagnostic(no_with_diagnostic(span)); - } + if let Some(terms) = &self.0.terms { + for term in terms { + if any_word_matches_term(&words, term) { + ctx.diagnostic(no_with_diagnostic(span)); } } - None => { - let default_terms = vec!["todo", "fixme", "xxx"]; - for term in default_terms { - match &self.0.location { - Some(location) => { - if location == "start" { - if !(words[0] == term.to_lowercase()) {} - } else { - if any_word_matches_term(&words, term) { - ctx.diagnostic(no_with_diagnostic(span)); - } - } + } else { + let default_terms = vec!["todo", "fixme", "xxx"]; + for term in default_terms { + match &self.0.location { + Some(location) => { + if location == "start" { + if words[0] != term.cow_to_lowercase() {} + } else if any_word_matches_term(&words, term) { + ctx.diagnostic(no_with_diagnostic(span)); } - None => { - if words[0] == term.to_lowercase() { - ctx.diagnostic(no_with_diagnostic(span)); - } + } + None => { + if words[0] == term.cow_to_lowercase() { + ctx.diagnostic(no_with_diagnostic(span)); } } } } } - // 24/10/25 there is an issue with the location // if you do not have location set to anywhere it will use the default of start // if the location is start we should start with rather than contains @@ -226,7 +215,7 @@ impl Rule for NoWarningComments { if let Some(terms_config) = config.get("terms") { cfg.terms = terms_config.as_array().map(|arr| { arr.iter() - .filter_map(|v| v.as_str().map(|s| s.to_string())) + .filter_map(|v| v.as_str().map(std::string::ToString::to_string)) .collect::>() }); } @@ -234,20 +223,20 @@ impl Rule for NoWarningComments { if let Some(decorations_config) = config.get("decoration") { cfg.decorations = decorations_config.as_array().map(|arr| { arr.iter() - .filter_map(|v| v.as_str().map(|s| s.to_string())) + .filter_map(|v| v.as_str().map(std::string::ToString::to_string)) .collect::>() }); } if let Some(location_config) = config.get("location") { - cfg.location = location_config.as_str().map(|s| s.to_string()); + cfg.location = location_config.as_str().map(std::string::ToString::to_string); } else { // Default to "start" if location is not provided cfg.location = Some("start".to_string()); } } - return Self(Box::new(cfg)); + Self(Box::new(cfg)) } }