[New] add jsx-no-leaked-render

CHANGELOG.md

@@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 * [`no-unknown-property`]: Allow crossOrigin on image tag (SVG) ([#3251][] @zpao)
 * [`jsx-tag-spacing`]: Add `multiline-always` option ([#3260][] @Nokel81)
 * [`function-component-definition`]: replace `var` by `const` in certain situations ([#3248][] @JohnBerd @SimeonC)
+* add [`jsx-no-leaked-render`] ([#3203][] @Belco90)
 
 ### Fixed
 * [`hook-use-state`]: Allow UPPERCASE setState setter prefixes ([#3244][] @duncanbeevers)
@@ -26,6 +27,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 * [Refactor] [`no-deprecated`]: improve performance ([#3271][] @golopot)
 * [Refactor] [`no-did-mount-set-state`], [`no-did-update-set-state`], [`no-will-update-set-state`]: improve performance ([#3272][] @golopot)
 * [Refactor] improve performance by avoiding unnecessary `Components.detect` ([#3273][] @golopot)
+* [Refactor] add `isParenthesized` AST util ([#3203][] @Belco90)
 
 [#3273]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3273
 [#3272]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3272
@@ -39,6 +41,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 [#3258]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3258
 [#3254]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3254
 [#3251]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3251
+[#3203]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3203
 [#3248]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3248
 [#3244]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3244
 [#3235]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3235
@@ -3683,6 +3686,7 @@ If you're still not using React 15 you can keep the old behavior by setting the
 [`jsx-no-comment-textnodes`]: docs/rules/jsx-no-comment-textnodes.md
 [`jsx-no-constructed-context-values`]: docs/rules/jsx-no-constructed-context-values.md
 [`jsx-no-duplicate-props`]: docs/rules/jsx-no-duplicate-props.md
+[`jsx-no-leaked-render`]: docs/rules/jsx-no-leaked-render.md
 [`jsx-no-literals`]: docs/rules/jsx-no-literals.md
 [`jsx-no-script-url`]: docs/rules/jsx-no-script-url.md
 [`jsx-no-target-blank`]: docs/rules/jsx-no-target-blank.md

README.md

@@ -202,6 +202,7 @@ Enable the rules that you would like to use.
 | ✔ |  | [react/jsx-no-comment-textnodes](docs/rules/jsx-no-comment-textnodes.md) | Comments inside children section of tag should be placed inside braces |
 |  |  | [react/jsx-no-constructed-context-values](docs/rules/jsx-no-constructed-context-values.md) | Prevents JSX context provider values from taking values that will cause needless rerenders. |
 | ✔ |  | [react/jsx-no-duplicate-props](docs/rules/jsx-no-duplicate-props.md) | Enforce no duplicate props |
+|  | 🔧 | [react/jsx-no-leaked-render](docs/rules/jsx-no-leaked-render.md) | Prevent problematic leaked values from being rendered |
 |  |  | [react/jsx-no-literals](docs/rules/jsx-no-literals.md) | Prevent using string literals in React component definition |
 |  |  | [react/jsx-no-script-url](docs/rules/jsx-no-script-url.md) | Forbid `javascript:` URLs |
 | ✔ | 🔧 | [react/jsx-no-target-blank](docs/rules/jsx-no-target-blank.md) | Forbid `target="_blank"` attribute without `rel="noreferrer"` |

docs/rules/jsx-no-leaked-render.md

@@ -0,0 +1,208 @@
+# Prevent problematic leaked values from being rendered (react/jsx-no-leaked-render)
+
+Using the `&&` operator to render some element conditionally in JSX can cause unexpected values being rendered, or even crashing the rendering.
+
+
+## Rule Details
+
+This rule aims to prevent dangerous leaked values from being rendered since they can cause unexpected values reaching the final DOM or even crashing your render method.
+
+In React, you might end up rendering unexpected values like `0` or `NaN`. In React Native, your render method will crash if you render `0`, `''`, or `NaN`:
+
+```jsx
+const Example = () => {
+  return (
+    <>
+      {0 && <Something/>}
+      {/* React: renders undesired 0 */}
+      {/* React Native: crashes 💥 */}
+
+      {'' && <Something/>}
+      {/* React: renders nothing */}
+      {/* React Native: crashes 💥 */}
+
+      {NaN && <Something/>}
+      {/* React: renders undesired NaN */}
+      {/* React Native: crashes 💥 */}
+    </>
+  )
+}
+```
+
+This can be avoided by:
+- coercing the conditional to a boolean: `{!!someValue && <Something />}`
+- transforming the binary expression into a ternary expression which returns `null` for falsy values: `{someValue ? <Something /> : null}`
+
+This rule is autofixable; check the Options section to read more about the different strategies available.
+
+Examples of **incorrect** code for this rule:
+
+```jsx
+const Component = ({ count, title }) => {
+  return <div>{count && title}</div>
+}
+```
+
+```jsx
+const Component = ({ count }) => {
+  return <div>{count && <span>There are {count} results</span>}</div>
+}
+```
+
+```jsx
+const Component = ({ elements }) => {
+  return <div>{elements.length && <List elements={elements}/>}</div>
+}
+```
+
+```jsx
+const Component = ({ nestedCollection }) => {
+  return (
+    <div>
+      {nestedCollection.elements.length && <List elements={nestedCollection.elements} />}
+    </div>
+  )
+}
+```
+
+```jsx
+const Component = ({ elements }) => {
+  return <div>{elements[0] && <List elements={elements}/>}</div>
+}
+```
+
+```jsx
+const Component = ({ numberA, numberB }) => {
+  return <div>{(numberA || numberB) && <Results>{numberA+numberB}</Results>}</div>
+}
+```
+
+```jsx
+// If the condition is a boolean value, this rule will report the logical expression
+// since it can't infer the type of the condition.
+const Component = ({ someBool }) => {
+  return <div>{someBool && <Results>{numberA+numberB}</Results>}</div>
+}
+```
+
+Examples of **correct** code for this rule:
+
+```jsx
+const Component = ({ elements }) => {
+  return <div>{elements}</div>
+}
+```
+
+```jsx
+// An OR condition it's considered valid since it's assumed as a way
+// to render some fallback if the first value is falsy, not to render something conditionally.
+const Component = ({ customTitle }) => {
+  return <div>{customTitle || defaultTitle}</div>
+}
+```
+
+```jsx
+const Component = ({ elements }) => {
+  return <div>There are {elements.length} elements</div>
+}
+```
+
+```jsx
+const Component = ({ elements, count }) => {
+  return <div>{!count && 'No results found'}</div>
+}
+```
+
+```jsx
+const Component = ({ elements }) => {
+  return <div>{!!elements.length && <List elements={elements}/>}</div>
+}
+```
+
+```jsx
+const Component = ({ elements }) => {
+  return <div>{Boolean(elements.length) && <List elements={elements}/>}</div>
+}
+```
+
+```jsx
+const Component = ({ elements }) => {
+  return <div>{elements.length > 0 && <List elements={elements}/>}</div>
+}
+```
+
+```jsx
+const Component = ({ elements }) => {
+  return <div>{elements.length ? <List elements={elements}/> : null}</div>
+}
+```
+
+### Options
+
+The supported options are:
+
+### `validStrategies`
+An array containing `"coerce"`, `"ternary"`, or both (default: `["ternary", "coerce"]`) - Decide which strategies are considered valid to prevent leaked renders (at least 1 is required). The "coerce" option will transform the conditional of the JSX expression to a boolean. The "ternary" option transforms the binary expression into a ternary expression returning `null` for falsy values. The first option from the array will be the strategy used when autofixing, so the order of the values matters.
+
+It can be set like:
+```json5
+{
+  // ...
+  "react/jsx-no-leaked-render": [<enabled>, { "validStrategies": ["ternary", "coerce"] }]
+  // ...
+}
+```
+
+Assuming the following options: `{ "validStrategies": ["ternary"] }`
+
+Examples of **incorrect** code for this rule, with the above configuration:
+```jsx
+const Component = ({ count, title }) => {
+  return <div>{count && title}</div>
+}
+```
+
+```jsx
+const Component = ({ count, title }) => {
+  return <div>{!!count && title}</div>
+}
+```
+
+Examples of **correct** code for this rule, with the above configuration:
+```jsx
+const Component = ({ count, title }) => {
+  return <div>{count ? title : null}</div>
+}
+```
+
+Assuming the following options: `{ "validStrategies": ["coerce"] }`
+
+Examples of **incorrect** code for this rule, with the above configuration:
+```jsx
+const Component = ({ count, title }) => {
+  return <div>{count && title}</div>
+}
+```
+
+```jsx
+const Component = ({ count, title }) => {
+  return <div>{count ? title : null}</div>
+}
+```
+
+Examples of **correct** code for this rule, with the above configuration:
+```jsx
+const Component = ({ count, title }) => {
+  return <div>{!!count && title}</div>
+}
+```
+
+## When Not To Use It
+
+If you are working in a typed-codebase which encourages you to always use boolean conditions, this rule can be disabled.
+
+## Further Reading
+
+- [React docs: Inline If with Logical && Operator](https://reactjs.org/docs/conditional-rendering.html#inline-if-with-logical--operator)
+- [Good advice on JSX conditionals - Beware of zero](https://thoughtspile.github.io/2022/01/17/jsx-conditionals/)
+- [Twitter: rendering falsy values in React and React Native](https://twitter.com/kadikraman/status/1507654900376875011?s=21&t=elEXXbHhzWthrgKaPRMjNg)

index.js

@@ -38,6 +38,7 @@ const allRules = {
   'jsx-no-comment-textnodes': require('./lib/rules/jsx-no-comment-textnodes'),
   'jsx-no-constructed-context-values': require('./lib/rules/jsx-no-constructed-context-values'),
   'jsx-no-duplicate-props': require('./lib/rules/jsx-no-duplicate-props'),
+  'jsx-no-leaked-render': require('./lib/rules/jsx-no-leaked-render'),
   'jsx-no-literals': require('./lib/rules/jsx-no-literals'),
   'jsx-no-script-url': require('./lib/rules/jsx-no-script-url'),
   'jsx-no-target-blank': require('./lib/rules/jsx-no-target-blank'),

lib/rules/jsx-no-leaked-render.js

@@ -0,0 +1,134 @@
+/**
+ * @fileoverview Prevent problematic leaked values from being rendered
+ * @author Mario Beltrán
+ */
+
+'use strict';
+
+const docsUrl = require('../util/docsUrl');
+const report = require('../util/report');
+const isParenthesized = require('../util/ast').isParenthesized;
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+const messages = {
+  noPotentialLeakedRender: 'Potential leaked value that might cause unintentionally rendered values or rendering crashes',
+};
+
+const COERCE_STRATEGY = 'coerce';
+const TERNARY_STRATEGY = 'ternary';
+const DEFAULT_VALID_STRATEGIES = [TERNARY_STRATEGY, COERCE_STRATEGY];
+const COERCE_VALID_LEFT_SIDE_EXPRESSIONS = ['UnaryExpression', 'BinaryExpression', 'CallExpression'];
+
+function trimLeftNode(node) {
+  // Remove double unary expression (boolean coercion), so we avoid trimming valid negations
+  if (node.type === 'UnaryExpression' && node.argument.type === 'UnaryExpression') {
+    return trimLeftNode(node.argument.argument);
+  }
+
+  return node;
+}
+
+function ruleFixer(context, fixStrategy, fixer, reportedNode, leftNode, rightNode) {
+  const sourceCode = context.getSourceCode();
+  const rightSideText = sourceCode.getText(rightNode);
+
+  if (fixStrategy === COERCE_STRATEGY) {
+    let leftSideText = sourceCode.getText(leftNode);
+    if (isParenthesized(context, leftNode)) {
+      leftSideText = `(${leftSideText})`;
+    }
+
+    const shouldPrefixDoubleNegation = leftNode.type !== 'UnaryExpression';
+
+    return fixer.replaceText(reportedNode, `${shouldPrefixDoubleNegation ? '!!' : ''}${leftSideText} && ${rightSideText}`);
+  }
+
+  if (fixStrategy === TERNARY_STRATEGY) {
+    let leftSideText = sourceCode.getText(trimLeftNode(leftNode));
+    if (isParenthesized(context, leftNode)) {
+      leftSideText = `(${leftSideText})`;
+    }
+    return fixer.replaceText(reportedNode, `${leftSideText} ? ${rightSideText} : null`);
+  }
+
+  throw new TypeError('Invalid value for "validStrategies" option');
+}
+
+/**
+ * @type {import('eslint').Rule.RuleModule}
+ */
+module.exports = {
+  meta: {
+    docs: {
+      description: 'Prevent problematic leaked values from being rendered',
+      category: 'Possible Errors',
+      recommended: false,
+      url: docsUrl('jsx-no-leaked-render'),
+    },
+
+    messages,
+
+    fixable: 'code',
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          validStrategies: {
+            type: 'array',
+            items: {
+              enum: [
+                TERNARY_STRATEGY,
+                COERCE_STRATEGY,
+              ],
+            },
+            uniqueItems: true,
+            default: DEFAULT_VALID_STRATEGIES,
+          },
+        },
+        additionalProperties: false,
+      },
+    ],
+  },
+
+  create(context) {
+    const config = context.options[0] || {};
+    const validStrategies = new Set(config.validStrategies || DEFAULT_VALID_STRATEGIES);
+    const fixStrategy = Array.from(validStrategies)[0];
+
+    return {
+      'JSXExpressionContainer > LogicalExpression[operator="&&"]'(node) {
+        const leftSide = node.left;
+
+        if (
+          validStrategies.has(COERCE_STRATEGY)
+          && COERCE_VALID_LEFT_SIDE_EXPRESSIONS.some((validExpression) => validExpression === leftSide.type)
+        ) {
+          return;
+        }
+
+        report(context, messages.noPotentialLeakedRender, 'noPotentialLeakedRender', {
+          node,
+          fix(fixer) {
+            return ruleFixer(context, fixStrategy, fixer, node, leftSide, node.right);
+          },
+        });
+      },
+
+      'JSXExpressionContainer > ConditionalExpression'(node) {
+        if (validStrategies.has(TERNARY_STRATEGY)) {
+          return;
+        }
+
+        report(context, messages.noPotentialLeakedRender, 'noPotentialLeakedRender', {
+          node,
+          fix(fixer) {
+            return ruleFixer(context, fixStrategy, fixer, node, node.test, node.consequent);
+          },
+        });
+      },
+    };
+  },
+};