From 1de3518046c9728236ff180602481da0fd9a1213 Mon Sep 17 00:00:00 2001 From: Wenzhe Wang Date: Sun, 28 Jan 2024 11:48:23 +0800 Subject: [PATCH] refactor(linter): remove Regex and change error position (#2188) --- crates/oxc_ast/src/span.rs | 11 ++ .../src/rules/react/jsx_no_target_blank.rs | 35 +++-- .../src/snapshots/jsx_no_target_blank.snap | 132 +++++++++--------- 3 files changed, 97 insertions(+), 81 deletions(-) diff --git a/crates/oxc_ast/src/span.rs b/crates/oxc_ast/src/span.rs index edd9efbafbd1b..904b6bb72795b 100644 --- a/crates/oxc_ast/src/span.rs +++ b/crates/oxc_ast/src/span.rs @@ -423,6 +423,17 @@ impl<'a> GetSpan for JSXAttributeName<'a> { } } +impl<'a> GetSpan for JSXAttributeValue<'a> { + fn span(&self) -> Span { + match &self { + JSXAttributeValue::StringLiteral(literal) => literal.span, + JSXAttributeValue::ExpressionContainer(container) => container.span, + JSXAttributeValue::Fragment(fragment) => fragment.span, + JSXAttributeValue::Element(element) => element.span, + } + } +} + impl<'a> GetSpan for JSXMemberExpressionObject<'a> { fn span(&self) -> Span { match &self { diff --git a/crates/oxc_linter/src/rules/react/jsx_no_target_blank.rs b/crates/oxc_linter/src/rules/react/jsx_no_target_blank.rs index b39189cc3d741..ea257492e6a4b 100644 --- a/crates/oxc_linter/src/rules/react/jsx_no_target_blank.rs +++ b/crates/oxc_linter/src/rules/react/jsx_no_target_blank.rs @@ -1,4 +1,3 @@ -use lazy_static::lazy_static; use oxc_ast::{ ast::{ Expression, JSXAttributeItem, JSXAttributeName, JSXAttributeValue, JSXElementName, @@ -11,8 +10,7 @@ use oxc_diagnostics::{ thiserror::Error, }; use oxc_macros::declare_oxc_lint; -use oxc_span::{Atom, Span}; -use regex::Regex; +use oxc_span::{Atom, GetSpan, Span}; use std::ops::Deref; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -21,11 +19,15 @@ use crate::{context::LintContext, rule::Rule, AstNode}; enum JsxNoTargetBlankDiagnostic { #[error("eslint-plugin-react(jsx-no-target-blank): Using target=`_blank` without rel=`noreferrer` (which implies rel=`noopener`) is a security risk in older browsers: see https://mathiasbynens.github.io/rel-noopener/#recommendations")] #[diagnostic(severity(warning), help("add rel=`noreferrer` to the element"))] - NoTargetBlankWithoutNoreferrer(#[label] Span), + TargetBlankWithoutNoreferrer(#[label] Span), #[error("eslint-plugin-react(jsx-no-target-blank): Using target=`_blank` without rel=`noreferrer` or rel=`noopener` (the former implies the latter and is preferred due to wider support) is a security risk: see https://mathiasbynens.github.io/rel-noopener/#recommendations")] #[diagnostic(severity(warning), help("add rel=`noreferrer` or rel=`noopener` to the element"))] - NoTargetBlankWithoutNoopener(#[label] Span), + TargetBlankWithoutNoopener(#[label] Span), + + #[error("eslint-plugin-react(jsx-no-target-blank): all spread attributes are treated as if they contain an unsafe combination of props, unless specifically overridden by props after the last spread attribute prop.")] + #[diagnostic(severity(warning), help("add rel=`noreferrer` to the element"))] + ExplicitPropsInSpreadAttributes(#[label] Span), } #[derive(Debug, Clone)] @@ -58,9 +60,9 @@ impl Default for JsxNoTargetBlank { impl JsxNoTargetBlank { fn diagnostic(&self, span: Span, ctx: &LintContext) { if self.allow_referrer { - ctx.diagnostic(JsxNoTargetBlankDiagnostic::NoTargetBlankWithoutNoopener(span)); + ctx.diagnostic(JsxNoTargetBlankDiagnostic::TargetBlankWithoutNoopener(span)); } else { - ctx.diagnostic(JsxNoTargetBlankDiagnostic::NoTargetBlankWithoutNoreferrer(span)); + ctx.diagnostic(JsxNoTargetBlankDiagnostic::TargetBlankWithoutNoreferrer(span)); } } } @@ -113,6 +115,8 @@ impl Rule for JsxNoTargetBlank { let mut is_href_valid = true; let mut has_href_value = false; let mut is_warn_on_spread_attributes = false; + let mut target_span = None; + let mut spread_span = Span::default(); jsx_ele.attributes.iter().for_each(|attribute| match attribute { JSXAttributeItem::Attribute(attribute) => { @@ -120,6 +124,7 @@ impl Rule for JsxNoTargetBlank { if identifier.name.as_str() == "target" { if let Some(val) = attribute.deref().value.as_ref() { target_blank_tuple = check_target(val); + target_span = attribute.value.as_ref().map(GetSpan::span); } } else if identifier.name.as_str() == "href" || identifier.name.as_str() == "to" @@ -139,6 +144,7 @@ impl Rule for JsxNoTargetBlank { JSXAttributeItem::SpreadAttribute(_) => { if self.warn_on_spread_attributes { is_warn_on_spread_attributes = true; + spread_span = attribute.span(); target_blank_tuple = (false, "", false, false); rel_valid_tuple = (false, "", false, false); is_href_valid = false; @@ -151,22 +157,25 @@ impl Rule for JsxNoTargetBlank { if (has_href_value && is_href_valid) || rel_valid_tuple.0 { return; } - self.diagnostic(jsx_ele.span, ctx); + ctx.diagnostic(JsxNoTargetBlankDiagnostic::ExplicitPropsInSpreadAttributes( + spread_span, + )); return; } + let span = target_span.unwrap_or(jsx_ele.span); if !is_href_valid { if !target_blank_tuple.1.is_empty() && target_blank_tuple.1 == rel_valid_tuple.1 { if (target_blank_tuple.2 && !rel_valid_tuple.2) || (target_blank_tuple.3 && !rel_valid_tuple.3) { - self.diagnostic(jsx_ele.span, ctx); + self.diagnostic(span, ctx); } return; } if target_blank_tuple.0 && !rel_valid_tuple.0 { - self.diagnostic(jsx_ele.span, ctx); + self.diagnostic(span, ctx); } } } @@ -204,11 +213,7 @@ impl Rule for JsxNoTargetBlank { } fn check_is_external_link(link: &Atom) -> bool { - // TODO It may hurt performance. replace Regex with something else more efficient - lazy_static! { - static ref CTL_PAT: Regex = Regex::new(r"(?:\w+:|\/\/)",).unwrap(); - } - CTL_PAT.is_match(link.as_str()) + link.as_str().contains("//") } fn match_href_expression( diff --git a/crates/oxc_linter/src/snapshots/jsx_no_target_blank.snap b/crates/oxc_linter/src/snapshots/jsx_no_target_blank.snap index e36e9a4d41927..2a54e65516bfc 100644 --- a/crates/oxc_linter/src/snapshots/jsx_no_target_blank.snap +++ b/crates/oxc_linter/src/snapshots/jsx_no_target_blank.snap @@ -6,7 +6,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -14,7 +14,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ─────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -22,7 +22,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -30,7 +30,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -38,7 +38,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -46,7 +46,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ─────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -54,7 +54,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -62,7 +62,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -70,7 +70,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -78,7 +78,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -86,7 +86,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -94,7 +94,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ─────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -102,7 +102,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -110,7 +110,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -118,7 +118,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -126,7 +126,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────────────────────────────────────── + · ────────── ╰──── help: add rel=`noreferrer` to the element @@ -134,7 +134,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────────────────────────────────────────────────────────────────────────────── + · ────────── ╰──── help: add rel=`noreferrer` to the element @@ -142,7 +142,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ─────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -150,7 +150,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -158,7 +158,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────────── + · ────────── ╰──── help: add rel=`noreferrer` to the element @@ -166,7 +166,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────────── + · ────────── ╰──── help: add rel=`noreferrer` to the element @@ -174,7 +174,7 @@ expression: jsx_no_target_blank │ risk: see https://mathiasbynens.github.io/rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` or rel=`noopener` to the element @@ -182,7 +182,7 @@ expression: jsx_no_target_blank │ risk: see https://mathiasbynens.github.io/rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` or rel=`noopener` to the element @@ -190,47 +190,47 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element - ⚠ eslint-plugin-react(jsx-no-target-blank): Using target=`_blank` without rel=`noreferrer` (which implies rel=`noopener`) is a security risk in older browsers: see https://mathiasbynens.github.io/ - │ rel-noopener/#recommendations + ⚠ eslint-plugin-react(jsx-no-target-blank): all spread attributes are treated as if they contain an unsafe combination of props, unless specifically overridden by props after the last spread + │ attribute prop. ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ─────────────────── + · ─────────────── ╰──── help: add rel=`noreferrer` to the element - ⚠ eslint-plugin-react(jsx-no-target-blank): Using target=`_blank` without rel=`noreferrer` (which implies rel=`noopener`) is a security risk in older browsers: see https://mathiasbynens.github.io/ - │ rel-noopener/#recommendations + ⚠ eslint-plugin-react(jsx-no-target-blank): all spread attributes are treated as if they contain an unsafe combination of props, unless specifically overridden by props after the last spread + │ attribute prop. ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ─────────────────────────────────── + · ─────────────── ╰──── help: add rel=`noreferrer` to the element - ⚠ eslint-plugin-react(jsx-no-target-blank): Using target=`_blank` without rel=`noreferrer` (which implies rel=`noopener`) is a security risk in older browsers: see https://mathiasbynens.github.io/ - │ rel-noopener/#recommendations + ⚠ eslint-plugin-react(jsx-no-target-blank): all spread attributes are treated as if they contain an unsafe combination of props, unless specifically overridden by props after the last spread + │ attribute prop. ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────────────── + · ─────────────── ╰──── help: add rel=`noreferrer` to the element - ⚠ eslint-plugin-react(jsx-no-target-blank): Using target=`_blank` without rel=`noreferrer` (which implies rel=`noopener`) is a security risk in older browsers: see https://mathiasbynens.github.io/ - │ rel-noopener/#recommendations + ⚠ eslint-plugin-react(jsx-no-target-blank): all spread attributes are treated as if they contain an unsafe combination of props, unless specifically overridden by props after the last spread + │ attribute prop. ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────────────────────────────── + · ─────────────── ╰──── help: add rel=`noreferrer` to the element - ⚠ eslint-plugin-react(jsx-no-target-blank): Using target=`_blank` without rel=`noreferrer` (which implies rel=`noopener`) is a security risk in older browsers: see https://mathiasbynens.github.io/ - │ rel-noopener/#recommendations + ⚠ eslint-plugin-react(jsx-no-target-blank): all spread attributes are treated as if they contain an unsafe combination of props, unless specifically overridden by props after the last spread + │ attribute prop. ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────────────── + · ─────────────── ╰──── help: add rel=`noreferrer` to the element @@ -238,7 +238,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ─────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -246,23 +246,23 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element - ⚠ eslint-plugin-react(jsx-no-target-blank): Using target=`_blank` without rel=`noreferrer` (which implies rel=`noopener`) is a security risk in older browsers: see https://mathiasbynens.github.io/ - │ rel-noopener/#recommendations + ⚠ eslint-plugin-react(jsx-no-target-blank): all spread attributes are treated as if they contain an unsafe combination of props, unless specifically overridden by props after the last spread + │ attribute prop. ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ─────────────────────────────────────────────────────────────────── + · ─────────────── ╰──── help: add rel=`noreferrer` to the element - ⚠ eslint-plugin-react(jsx-no-target-blank): Using target=`_blank` without rel=`noreferrer` (which implies rel=`noopener`) is a security risk in older browsers: see https://mathiasbynens.github.io/ - │ rel-noopener/#recommendations + ⚠ eslint-plugin-react(jsx-no-target-blank): all spread attributes are treated as if they contain an unsafe combination of props, unless specifically overridden by props after the last spread + │ attribute prop. ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ─────────────────────────────────────────────────────────────────── + · ─────────────── ╰──── help: add rel=`noreferrer` to the element @@ -270,7 +270,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -278,7 +278,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -286,7 +286,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -294,7 +294,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │
- · ───────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -302,7 +302,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │
- · ──────────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -310,7 +310,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │
- · ────────────────────────────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -318,7 +318,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │
- · ────────────────────────────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -326,7 +326,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -334,7 +334,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ─────────────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -342,7 +342,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -350,7 +350,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────────────────────────────────────────────────────────────────────────────── + · ─────────────────────────────────── ╰──── help: add rel=`noreferrer` to the element @@ -358,7 +358,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ────────────────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -366,7 +366,7 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element @@ -374,7 +374,7 @@ expression: jsx_no_target_blank │ risk: see https://mathiasbynens.github.io/rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ───────────────────────────────────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` or rel=`noopener` to the element @@ -382,7 +382,7 @@ expression: jsx_no_target_blank │ risk: see https://mathiasbynens.github.io/rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │
- · ──────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` or rel=`noopener` to the element @@ -390,15 +390,15 @@ expression: jsx_no_target_blank │ rel-noopener/#recommendations ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────────── + · ──────── ╰──── help: add rel=`noreferrer` to the element - ⚠ eslint-plugin-react(jsx-no-target-blank): Using target=`_blank` without rel=`noreferrer` (which implies rel=`noopener`) is a security risk in older browsers: see https://mathiasbynens.github.io/ - │ rel-noopener/#recommendations + ⚠ eslint-plugin-react(jsx-no-target-blank): all spread attributes are treated as if they contain an unsafe combination of props, unless specifically overridden by props after the last spread + │ attribute prop. ╭─[jsx_no_target_blank.tsx:1:1] 1 │ - · ──────────────────────────────────── + · ─────────── ╰──── help: add rel=`noreferrer` to the element