feat(rome_js_analyzer): rule `noChildrenProp`
MODIFY
crates/rome_diagnostics_categories/src/categories.rs
crates/rome_diagnostics_categories/src/categories.rs
@@ -46,6 +46,7 @@ define_dategories! {
"lint/nursery/useButtonType": "https://rome.tools/docs/lint/rules/useButtonType",
"lint/nursery/useCamelCase": "https://rome.tools/docs/lint/rules/useCamelCase",
"lint/nursery/useOptionalChain": "https://rome.tools/docs/lint/rules/useOptionalChain",
+ "lint/nursery/noChildrenProp": "https://rome.tools/docs/lint/rules/noChildrenProp",
"lint/nursery/useFragmentSyntax": "https://rome.tools/docs/lint/rules/useFragmentSyntax",
"lint/style/noNegationElse": "https://rome.tools/docs/lint/rules/noNegationElse",
"lint/style/noShoutyConstants": "https://rome.tools/docs/lint/rules/noShoutyConstants",

MODIFY
crates/rome_js_analyze/src/react.rs
crates/rome_js_analyze/src/react.rs
-//! A series of utilities to work with the React library
+//! A series of AST utilities to work with the React library
use rome_js_semantic::SemanticModel;
use rome_js_syntax::{
JsAnyCallArgument, JsAnyExpression, JsArrayExpression, JsCallExpression, JsIdentifierBinding,
- JsImport, JsObjectExpression, JsxMemberName, JsxReferenceIdentifier,
+ JsImport, JsObjectExpression, JsPropertyObjectMember, JsxMemberName, JsxReferenceIdentifier,
};
use rome_rowan::{AstNode, AstSeparatedList};
@@ -20,6 +20,27 @@ pub(crate) struct ReactCreateElementCall {
pub(crate) children: Option<JsArrayExpression>,
}
+impl ReactCreateElementCall {
+ /// It scans the current props and returns the property that matches the passed name
+ pub(crate) fn find_prop_by_name(&self, prop_name: &str) -> Option<JsPropertyObjectMember> {
+ self.props.as_ref().and_then(|props| {
+ let members = props.members();
+ members.into_iter().find_map(|member| {
+ let member = member.ok()?;
+ let property = member.as_js_property_object_member()?;
+ let property_name = property.name().ok()?;
+
+ let property_name = property_name.as_js_literal_member_name()?;
+ if property_name.name().ok()? == prop_name {
+ Some(property.clone())
+ } else {
+ None
+ }
+ })
+ })
+ }
+}
+
/// Checks if the current node is a possible `createElement` call.
///
/// There are two cases:
@@ -148,7 +169,8 @@ pub(crate) fn jsx_member_name_is_react_fragment(
.find_map(|ancestor| JsImport::cast_ref(&ancestor))
{
let source_is_react = js_import.source_is("react").ok()?;
- maybe_react_fragment = source_is_react;
+ maybe_react_fragment =
+ source_is_react && member.value_token().ok()?.text_trimmed() == "Fragment";
} else {
// `React.Fragment` is a binding but it doesn't come from the "react" package
maybe_react_fragment = false;

MODIFY
crates/rome_js_analyze/src/semantic_analyzers/nursery.rs
crates/rome_js_analyze/src/semantic_analyzers/nursery.rs
//! Generated file, do not edit by hand, see `xtask/codegen`
use rome_analyze::declare_group;
+mod no_children_prop;
mod no_dangerously_set_inner_html;
mod no_render_return_value;
mod no_unused_variables;
@@ -8,4 +9,4 @@ mod no_useless_fragments;
mod use_button_type;
mod use_camel_case;
mod use_fragment_syntax;
-declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_dangerously_set_inner_html :: NoDangerouslySetInnerHtml , self :: no_render_return_value :: NoRenderReturnValue , self :: no_unused_variables :: NoUnusedVariables , self :: no_useless_fragments :: NoUselessFragments , self :: use_button_type :: UseButtonType , self :: use_camel_case :: UseCamelCase , self :: use_fragment_syntax :: UseFragmentSyntax ,] } }
+declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_children_prop :: NoChildrenProp , self :: no_dangerously_set_inner_html :: NoDangerouslySetInnerHtml , self :: no_render_return_value :: NoRenderReturnValue , self :: no_unused_variables :: NoUnusedVariables , self :: no_useless_fragments :: NoUselessFragments , self :: use_button_type :: UseButtonType , self :: use_camel_case :: UseCamelCase , self :: use_fragment_syntax :: UseFragmentSyntax ,] } }

ADD
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_children_prop.rs
+use crate::react::is_react_create_element;
+use crate::semantic_services::Semantic;
+use rome_analyze::context::RuleContext;
+use rome_analyze::{declare_rule, Rule, RuleCategory, RuleDiagnostic};
+use rome_console::markup;
+use rome_js_syntax::{JsCallExpression, JsPropertyObjectMember, JsxAttribute, JsxName};
+use rome_rowan::{declare_node_union, AstNode};
+declare_rule! {
+ /// Prevent passing of **children** as props.
+ ///
+ /// When using JSX, the children should be nested between the opening and closing tags.
+ /// When not using JSX, the children should be passed as additional arguments to `React.createElement`.
+ ///
+ /// ## Examples
+ ///
+ /// ### Invalid
+ ///
+ /// ```js,expect_diagnostic
+ /// <FirstComponent children={'foo'} />
+ /// ```
+ ///
+ /// ```js,expect_diagnostic
+ /// React.createElement('div', { children: 'foo' });
+ /// ```
+ pub(crate) NoChildrenProp {
+ version: "0.10.0",
+ name: "noChildrenProp",
+ recommended: false,
+ }
+}
+
+declare_node_union! {
+ pub(crate) NoChildrenPropQuery = JsxAttribute | JsCallExpression
+}
+
+pub(crate) enum NoChildrenPropState {
+ JsxProp(JsxName),
+ MemberProp(JsPropertyObjectMember),
+}
+
+impl Rule for NoChildrenProp {
+ const CATEGORY: RuleCategory = RuleCategory::Syntax;
+ type Query = Semantic<NoChildrenPropQuery>;
+ type State = NoChildrenPropState;
+ type Signals = Option<Self::State>;
+
+ fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+ let node = ctx.query();
+
+ match node {
+ NoChildrenPropQuery::JsxAttribute(attribute) => {
+ let name = attribute.name().ok()?;
+ let name = name.as_jsx_name()?;
+ if name.value_token().ok()?.text() == "children" {
+ return Some(NoChildrenPropState::JsxProp(name.clone()));
+ }
+
+ None
+ }
+ NoChildrenPropQuery::JsCallExpression(call_expression) => {
+ let model = ctx.model();
+ if let Some(react_create_element) = is_react_create_element(call_expression, model)
+ {
+ let children_prop = react_create_element.find_prop_by_name("children");
+
+ if let Some(children_prop) = children_prop {
+ return Some(NoChildrenPropState::MemberProp(children_prop));
+ }
+ }
+ None
+ }
+ }
+ }
+
+ fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
+ let (range, footer_help) = match state {
+ NoChildrenPropState::JsxProp(name) => {
+ (
+ name.syntax().text_trimmed_range(),
+ (markup! {
+ "The canonical way to pass children in React is to use JSX elements"
+ }).to_owned()
+ )
+ }
+ NoChildrenPropState::MemberProp(children_prop) => (
+ children_prop.name().ok()?.syntax().text_trimmed_range(),
+ (markup! {
+ "The canonical way to pass children in React is to use additional arguments to React.createElement"
+ }).to_owned()
+ ),
+ };
+
+ Some(
+ RuleDiagnostic::new(
+ range,
+ markup! {
+ "Avoid passing "<Emphasis>"children"</Emphasis>" using a prop"
+ },
+ )
+ .footer_note(footer_help),
+ )
+ }
+}

MODIFY
crates/rome_js_analyze/src/semantic_analyzers/nursery/use_button_type.rs
crates/rome_js_analyze/src/semantic_analyzers/nursery/use_button_type.rs
-use crate::react::{is_react_create_element, ReactCreateElementCall};
+use crate::react::is_react_create_element;
use crate::semantic_services::Semantic;
use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleCategory, RuleDiagnostic};
use rome_console::codespan::Severity;
@@ -114,39 +114,20 @@ impl Rule for UseButtonType {
let model = ctx.model();
if let Some(react_create_element) = is_react_create_element(call_expression, model)
{
- let ReactCreateElementCall {
- element_type,
- props,
- ..
- } = react_create_element;
-
// first argument needs to be a string
- let first_argument = element_type
+ let first_argument = react_create_element
+ .element_type
.as_js_any_expression()?
.as_js_any_literal_expression()?
.as_js_string_literal_expression()?;
// case sensitive is important, <button> is different from <Button>
if first_argument.inner_string_text().ok()? == "button" {
- return if let Some(props) = props {
- let members = props.members();
-
- let type_member = members.into_iter().find_map(|member| {
- let member = member.ok()?;
- let property = member.as_js_property_object_member()?;
- let property_name = property.name().ok()?;
- let property_value = property.value().ok()?;
-
- let property_name = property_name.as_js_literal_member_name()?;
- if property_name.name().ok()? == "type" {
- Some(property_value)
- } else {
- None
- }
- });
-
- if let Some(type_member) = type_member {
- let value = type_member
+ return if let Some(props) = react_create_element.props.as_ref() {
+ let type_member = react_create_element.find_prop_by_name("type");
+ if let Some(member) = type_member {
+ let property_value = member.value().ok()?;
+ let value = property_value
.as_js_any_literal_expression()?
.as_js_string_literal_expression()?;
@@ -163,7 +144,7 @@ impl Rule for UseButtonType {
// if we are here, it means that we haven't found the property "type" and
// we have to return a diagnostic
Some(UseButtonTypeState {
- node: UseButtonTypeNode::from(props),
+ node: UseButtonTypeNode::from(props.clone()),
missing_prop: false,
})
} else {

ADD
crates/rome_js_analyze/tests/specs/nursery/noChildrenProp/noChildrenPropInvalid.jsx
+<>
+ <Component children={'foo'}></Component>
+</>
+
+createElement('div', {
+ children: 'foo'
+})
+
+React.createElement('div', {
+ children: 'foo'
+})

ADD
crates/rome_js_analyze/tests/specs/nursery/noChildrenProp/noChildrenPropValid.jsx
+<>
+ <Component><AnotherComponent /></Component>
+ <React.StrictMode>
+ <Component />
+ </React.StrictMode>
+</>
+
+createElement('div', {}, 'foo')
+

ADD
crates/rome_js_analyze/tests/specs/nursery/noChildrenProp/noChildrenPropValid.jsx.snap
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: noChildrenPropValid.jsx
+---
+# Input
+```js
+<>
+ <Component><AnotherComponent /></Component>
+ <React.StrictMode>
+ <Component />
+ </React.StrictMode>
+</>
+
+createElement('div', {}, 'foo')
+
+
+```
+
+

MODIFY
crates/rome_js_analyze/tests/specs/nursery/noUselessFragments/withChildren.jsx
crates/rome_js_analyze/tests/specs/nursery/noUselessFragments/withChildren.jsx
<>foo</>
<React.Fragment>foo</React.Fragment>
<Fragment>foo</Fragment>
+ {/* valid */}
+ <React.StrictMode><App /></React.StrictMode>
</>

MODIFY
crates/rome_js_analyze/tests/specs/nursery/noUselessFragments/withChildren.jsx.snap
crates/rome_js_analyze/tests/specs/nursery/noUselessFragments/withChildren.jsx.snap
@@ -8,6 +8,8 @@ expression: withChildren.jsx
<>foo</>
<React.Fragment>foo</React.Fragment>
<Fragment>foo</Fragment>
+ {/* valid */}
+ <React.StrictMode><App /></React.StrictMode>
</>
```

MODIFY
rome.json
rome.json
"noUnreachable": "error",
"noUnusedVariables": "error",
"useCamelCase": "off",
- "noDangerouslySetInnerHtml": "error"
+ "noDangerouslySetInnerHtml": "error",
+ "noRenderReturnValue": "error",
+ "useButtonType": "error",
+ "useOptionalChain": "error",
+ "noUselessFragments": "error",
+ "noNewSymbol": "error"
}
}
}

MODIFY
website/playground/src/romeWorker.ts
website/playground/src/romeWorker.ts
@@ -119,6 +119,7 @@ self.addEventListener("message", async (e) => {
noRenderReturnValue: "error",
useButtonType: "error",
useOptionalChain: "error",
+ noUselessFragments: "error",
},
},
};

feat(rome_js_analyzer): rule `noChildrenProp` (#3269)