diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 73b0b8663d348..49f1dce591a84 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -240,6 +240,7 @@ mod react { pub mod jsx_props_no_spread_multi; pub mod no_children_prop; pub mod no_danger; + pub mod no_danger_with_children; pub mod no_direct_mutation_state; pub mod no_find_dom_node; pub mod no_is_mounted; @@ -765,6 +766,7 @@ oxc_macros::declare_all_lint_rules! { react::jsx_props_no_spread_multi, react::no_children_prop, react::no_danger, + react::no_danger_with_children, react::no_direct_mutation_state, react::no_find_dom_node, react::no_is_mounted, diff --git a/crates/oxc_linter/src/rules/react/no_danger_with_children.rs b/crates/oxc_linter/src/rules/react/no_danger_with_children.rs new file mode 100644 index 0000000000000..65941cce9772a --- /dev/null +++ b/crates/oxc_linter/src/rules/react/no_danger_with_children.rs @@ -0,0 +1,343 @@ +use oxc_ast::{ + ast::{Argument, Expression, JSXAttributeItem, JSXAttributeName, JSXChild, ObjectPropertyKind}, + AstKind, +}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +fn no_danger_with_children_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Only set one of `children` or `props.dangerouslySetInnerHTML`") + .with_help("`dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime.") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoDangerWithChildren; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallows when a DOM element is using both `children` and `dangerouslySetInnerHTML` properties. + /// + /// ### Why is this bad? + /// + /// React will throw a warning if this rule is ignored and both `children` and `dangerouslySetInnerHTML` are used. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```jsx + ///
Children
+ /// React.createElement( + /// "div", + /// { dangerouslySetInnerHTML: { __html: "HTML" } }, + /// "Children" + /// ); + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```jsx + ///
Children
+ ///
+ /// ``` + NoDangerWithChildren, + correctness +); + +impl Rule for NoDangerWithChildren { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match node.kind() { + AstKind::JSXElement(jsx) => { + // Either children are passed in as a prop like `children={}` or they are nested between the tags. + let has_children = has_jsx_prop(ctx, node, "children") + || (!jsx.children.is_empty() && !is_line_break(&jsx.children[0])); + if !has_children { + return; + } + + let has_danger_prop = has_jsx_prop(ctx, node, "dangerouslySetInnerHTML"); + if has_danger_prop { + ctx.diagnostic(no_danger_with_children_diagnostic(jsx.span)); + } + } + AstKind::CallExpression(call_expr) => { + // Calls with zero or one arguments are safe since they are not proper createElement calls. + if call_expr.arguments.len() <= 1 { + return; + } + let Expression::StaticMemberExpression(callee) = &call_expr.callee else { + return; + }; + + // Only accept calls like `.createElement(...)` + if callee.property.name != "createElement" { + return; + } + + let Some(props) = call_expr.arguments.get(1).and_then(Argument::as_expression) + else { + return; + }; + + // If there are three arguments, then it is a JSX element with children. + // If it's just two arguments, it only has children if the props object has a children property. + let has_children = if call_expr.arguments.len() == 2 { + match props { + Expression::ObjectExpression(obj_expr) => { + is_object_with_prop_name(&obj_expr.properties, "children") + } + Expression::Identifier(ident) => { + does_object_var_have_prop_name(ctx, node, &ident.name, "children") + } + _ => false, + } + } else { + true + }; + + if !has_children { + return; + } + + let has_danger_prop = match props { + Expression::ObjectExpression(obj_expr) => { + is_object_with_prop_name(&obj_expr.properties, "dangerouslySetInnerHTML") + } + Expression::Identifier(ident) => does_object_var_have_prop_name( + ctx, + node, + &ident.name, + "dangerouslySetInnerHTML", + ), + _ => false, + }; + + if has_danger_prop && has_children { + ctx.diagnostic(no_danger_with_children_diagnostic(call_expr.span)); + } + } + _ => (), + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "
Children
", + "
", + r#"
"#, + r#"
"#, + r#" + const props = { dangerouslySetInnerHTML: { __html: "HTML" } }; +
+ "#, + r#" + const moreProps = { className: "eslint" }; + const props = { children: "Children", ...moreProps }; +
+ "#, + r#" + const otherProps = { children: "Children" }; + const { a, b, ...props } = otherProps; +
+ "#, + "Children", + r#""#, + r#" + + + "#, + r#"React.createElement("div", { dangerouslySetInnerHTML: { __html: "HTML" } });"#, + r#"React.createElement("div", {}, "Children");"#, + r#"React.createElement("Hello", { dangerouslySetInnerHTML: { __html: "HTML" } });"#, + r#"React.createElement("Hello", {}, "Children");"#, + "Children", + r#"React.createElement("Hello", undefined, "Children")"#, + " + const props = {...props, scratch: {mode: 'edit'}}; + const component = shallow(); + ", + ]; + + let fail = vec![ + r#" +
+ Children +
+ "#, + r#"
"#, + r#" + const props = { dangerouslySetInnerHTML: { __html: "HTML" } }; +
Children
+ "#, + r#" + const props = { children: "Children", dangerouslySetInnerHTML: { __html: "HTML" } }; +
+ "#, + r#" + + Children + + "#, + r#""#, + r#" "#, + r#" + React.createElement( + "div", + { dangerouslySetInnerHTML: { __html: "HTML" } }, + "Children" + ); + "#, + r#" + React.createElement( + "div", + { + dangerouslySetInnerHTML: { __html: "HTML" }, + children: "Children", + } + ); + "#, + r#" + React.createElement( + "Hello", + { dangerouslySetInnerHTML: { __html: "HTML" } }, + "Children" + ); + "#, + r#" + React.createElement( + "Hello", + { + dangerouslySetInnerHTML: { __html: "HTML" }, + children: "Children", + } + ); + "#, + r#" + const props = { dangerouslySetInnerHTML: { __html: "HTML" } }; + React.createElement("div", props, "Children"); + "#, + r#" + const props = { children: "Children", dangerouslySetInnerHTML: { __html: "HTML" } }; + React.createElement("div", props); + "#, + r#" + const moreProps = { children: "Children" }; + const otherProps = { ...moreProps }; + const props = { ...otherProps, dangerouslySetInnerHTML: { __html: "HTML" } }; + React.createElement("div", props); + "#, + ]; + + Tester::new(NoDangerWithChildren::NAME, pass, fail).test_and_snapshot(); +} + +fn is_whitespace(s: &str) -> bool { + s.chars().all(char::is_whitespace) +} + +fn is_line_break(child: &JSXChild) -> bool { + let JSXChild::Text(text) = child else { + return false; + }; + let is_multi_line = text.value.contains('\n'); + is_multi_line && is_whitespace(text.value.as_str()) +} + +/// Given a JSX element, find the JSXAttributeItem with the given name. +/// If there are spread props, it will search within those as well. +fn has_jsx_prop(ctx: &LintContext, node: &AstNode, prop_name: &'static str) -> bool { + let AstKind::JSXElement(jsx) = node.kind() else { + return false; + }; + + jsx.opening_element.attributes.iter().any(|attr| match attr { + JSXAttributeItem::Attribute(attr) => { + let JSXAttributeName::Identifier(ident) = &attr.name else { + return false; + }; + ident.name == prop_name + } + JSXAttributeItem::SpreadAttribute(attr) => { + let Some(ident) = attr.argument.get_identifier_reference() else { + return false; + }; + does_object_var_have_prop_name(ctx, node, ident.name.as_str(), prop_name) + } + }) +} + +/// Given a variable name, finds the variable and checks if it is an object that has a property +/// by the given name, either by directly being set or by being spread into the object. +fn does_object_var_have_prop_name( + ctx: &LintContext, + node: &AstNode, + name: &str, + prop_name: &str, +) -> bool { + let Some(symbol) = &find_var_in_scope(ctx, node, name) else { + return false; + }; + + let AstKind::VariableDeclarator(var_decl) = symbol.kind() else { + return false; + }; + + let Some(init) = &var_decl.init else { + return false; + }; + + let Expression::ObjectExpression(obj_expr) = init else { + return false; + }; + + obj_expr.properties.iter().any(|prop| match prop { + ObjectPropertyKind::ObjectProperty(obj_prop) => { + obj_prop.key.static_name().is_some_and(|key| key == prop_name) + } + ObjectPropertyKind::SpreadProperty(spread_prop) => { + let Some(ident) = spread_prop.argument.get_identifier_reference() else { + return false; + }; + // If the next symbol is the same as the current symbol, then there is a cycle, + // for example: `const props = {...props}`, so we will stop searching. + if let Some(next_symbol) = find_var_in_scope(ctx, node, ident.name.as_str()) { + if next_symbol.id() == symbol.id() { + return false; + } + } + + does_object_var_have_prop_name(ctx, symbol, ident.name.as_str(), prop_name) + } + }) +} + +/// Given the name of an identifier, find the variable declaration in the current scope. +fn find_var_in_scope<'c>( + ctx: &'c LintContext, + node: &AstNode, + name: &str, +) -> Option<&'c AstNode<'c>> { + ctx.scopes() + .find_binding(node.scope_id(), name) + .map(|symbol_id| ctx.semantic().symbol_declaration(symbol_id)) +} + +/// Returns whether a given object has a property with the given name. +fn is_object_with_prop_name( + obj_props: &oxc_allocator::Vec<'_, ObjectPropertyKind<'_>>, + prop_name: &str, +) -> bool { + obj_props.iter().any(|prop| { + let ObjectPropertyKind::ObjectProperty(obj_prop) = prop else { + return false; + }; + obj_prop.key.static_name().is_some_and(|key| key == prop_name) + }) +} diff --git a/crates/oxc_linter/src/snapshots/no_danger_with_children.snap b/crates/oxc_linter/src/snapshots/no_danger_with_children.snap new file mode 100644 index 0000000000000..a06ebd1198fb6 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_danger_with_children.snap @@ -0,0 +1,140 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:2:9] + 1 │ + 2 │ ╭─▶
+ 3 │ │ Children + 4 │ ╰─▶
+ 5 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:1:1] + 1 │
+ · ──────────────────────────────────────────────────────────────────────── + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:3:9] + 2 │ const props = { dangerouslySetInnerHTML: { __html: "HTML" } }; + 3 │
Children
+ · ────────────────────────────── + 4 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:3:9] + 2 │ const props = { children: "Children", dangerouslySetInnerHTML: { __html: "HTML" } }; + 3 │
+ · ────────────────── + 4 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:2:9] + 1 │ + 2 │ ╭─▶ + 3 │ │ Children + 4 │ ╰─▶ + 5 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:1:1] + 1 │ + · ────────────────────────────────────────────────────────────────────────── + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:1:1] + 1 │ + · ───────────────────────────────────────────────────────────── + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:2:9] + 1 │ + 2 │ ╭─▶ React.createElement( + 3 │ │ "div", + 4 │ │ { dangerouslySetInnerHTML: { __html: "HTML" } }, + 5 │ │ "Children" + 6 │ ╰─▶ ); + 7 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:2:9] + 1 │ + 2 │ ╭─▶ React.createElement( + 3 │ │ "div", + 4 │ │ { + 5 │ │ dangerouslySetInnerHTML: { __html: "HTML" }, + 6 │ │ children: "Children", + 7 │ │ } + 8 │ ╰─▶ ); + 9 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:2:9] + 1 │ + 2 │ ╭─▶ React.createElement( + 3 │ │ "Hello", + 4 │ │ { dangerouslySetInnerHTML: { __html: "HTML" } }, + 5 │ │ "Children" + 6 │ ╰─▶ ); + 7 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:2:9] + 1 │ + 2 │ ╭─▶ React.createElement( + 3 │ │ "Hello", + 4 │ │ { + 5 │ │ dangerouslySetInnerHTML: { __html: "HTML" }, + 6 │ │ children: "Children", + 7 │ │ } + 8 │ ╰─▶ ); + 9 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:3:9] + 2 │ const props = { dangerouslySetInnerHTML: { __html: "HTML" } }; + 3 │ React.createElement("div", props, "Children"); + · ───────────────────────────────────────────── + 4 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:3:9] + 2 │ const props = { children: "Children", dangerouslySetInnerHTML: { __html: "HTML" } }; + 3 │ React.createElement("div", props); + · ───────────────────────────────── + 4 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime. + + ⚠ eslint-plugin-react(no-danger-with-children): Only set one of `children` or `props.dangerouslySetInnerHTML` + ╭─[no_danger_with_children.tsx:5:9] + 4 │ const props = { ...otherProps, dangerouslySetInnerHTML: { __html: "HTML" } }; + 5 │ React.createElement("div", props); + · ───────────────────────────────── + 6 │ + ╰──── + help: `dangerouslySetInnerHTML` is not compatible with also passing children and React will throw a warning at runtime.