From 685a5900301c980d852f231ad82afecec542f472 Mon Sep 17 00:00:00 2001 From: DonIsaac <22823424+DonIsaac@users.noreply.github.com> Date: Sun, 13 Oct 2024 19:34:19 +0000 Subject: [PATCH] fix(linter/no-control-regex): better diagnostic messages (#6530) 1. Handle plural/singular cases in message and help text 2. Shrink spans in `RegExp` constructors to only cover the regular expression itself. --- .../src/rules/eslint/no_control_regex.rs | 30 ++++++++++++++---- .../src/snapshots/no_control_regex.snap | Bin 4513 -> 3941 bytes ..._control_regex@capture-group-indexing.snap | 16 +++++----- 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_control_regex.rs b/crates/oxc_linter/src/rules/eslint/no_control_regex.rs index 0e0b0bba20028..20bc9442388ed 100644 --- a/crates/oxc_linter/src/rules/eslint/no_control_regex.rs +++ b/crates/oxc_linter/src/rules/eslint/no_control_regex.rs @@ -12,10 +12,15 @@ use oxc_span::{GetSpan, Span}; use crate::{ast_util::extract_regex_flags, context::LintContext, rule::Rule, AstNode}; -fn no_control_regex_diagnostic(regex: &str, span: Span) -> OxcDiagnostic { - OxcDiagnostic::warn("Unexpected control character(s)") - .with_help(format!("Unexpected control character(s) in regular expression: \"{regex}\"")) - .with_label(span) +fn no_control_regex_diagnostic(count: usize, regex: &str, span: Span) -> OxcDiagnostic { + debug_assert!(count > 0); + let (message, help) = if count == 1 { + ("Unexpected control character", format!("'{regex}' is not a valid control character.")) + } else { + ("Unexpected control characters", format!("'{regex}' are not valid control characters.")) + }; + + OxcDiagnostic::warn(message).with_help(help).with_label(span) } #[derive(Debug, Default, Clone)] @@ -89,7 +94,12 @@ impl Rule for NoControlRegex { if let Argument::StringLiteral(pattern) = &expr.arguments[0] { // get pattern from arguments. Missing or non-string arguments // will be runtime errors, but are not covered by this rule. - parse_and_check_regex(context, &pattern.value, &expr.arguments, expr.span); + parse_and_check_regex( + context, + &pattern.value, + &expr.arguments, + pattern.span, + ); } } } @@ -107,7 +117,12 @@ impl Rule for NoControlRegex { if let Argument::StringLiteral(pattern) = &expr.arguments[0] { // get pattern from arguments. Missing or non-string arguments // will be runtime errors, but are not covered by this rule. - parse_and_check_regex(context, &pattern.value, &expr.arguments, expr.span); + parse_and_check_regex( + context, + &pattern.value, + &expr.arguments, + pattern.span, + ); } } } @@ -143,8 +158,9 @@ fn check_pattern(context: &LintContext, pattern: &Pattern, span: Span) { finder.visit_pattern(pattern); if !finder.control_chars.is_empty() { + let num_control_chars = finder.control_chars.len(); let violations = finder.control_chars.into_iter().map(|c| c.to_string()).join(", "); - context.diagnostic(no_control_regex_diagnostic(&violations, span)); + context.diagnostic(no_control_regex_diagnostic(num_control_chars, &violations, span)); } } diff --git a/crates/oxc_linter/src/snapshots/no_control_regex.snap b/crates/oxc_linter/src/snapshots/no_control_regex.snap index f931a8ab5d353991fcbe35d8af4f924f40c1fc1e..c7241ca0d72c14712bf9a524d8e798ab0185f746 100644 GIT binary patch literal 3941 zcmds(!A`EB{A_NhBRd0^jeFMVy3nf2Loxl%i&{rvONnAGJ#wYQBI3el4cnF-HO>N9joD9j!dop^u^@H|! zxSh_dhAcJ>(>T2=fC0(q8SZTg=@49i){2d;#;1RM;$w2*QZskC$%P3{O+)RU*vYz9QYSw`k4CuCA8nU}C)wghKoO2vPtw1S!Rg=U&Ik_?N` zDgt?F?D0ukx;(cVym6G*J!7QOapisf&p`F9=+WrZ&zxbSTIL+q#UM&yKF9^3nsCIA zBZcF^Yk0zc#w?`*O0>us^P-n#azRBv(KrZes6~9+c(h97$%RF_z8{vDm*#rQ#bvs4 WB2iM))^MF#J{aAu_eU3RN%=Q|gCCaw literal 4513 zcmds(!AiqG5QaVHA?(mg5^0(Qy%@Bf%(WnRvQk32rh$-@-9!Tt5TBu5Joy5GH}5`5 zp2XRt#M(eBEo(F(EX(X}cK?t0C#z|i#G-_{!~&N(F=2X?y5qnPV?uRCT^&Q%?NcUc zI2Ry)egVSx1SO0#Hwt4K1sWwDNmUCjLz2#j8d&uO%=cpFhH_L!lUgQ_x4Ld86~?AMes@ZLB}*S=C2H(ChSIzrlL7N{?q9x>FFq z3aKEHdw+yvL9w(jI$1PK(NsGPQeAfTjvR<!UzIzRLM*i>ESHz4NchRrmI9)t^(tCp+MmS6l9ZoK%jh0}v5u2UMMdv@<>7 ta_|nahLjM&iDCSb1FJl$Bzyj5(It