From 7ccff100909ec510c3eebaee7c5ed658773ebcf0 Mon Sep 17 00:00:00 2001 From: golopot Date: Sun, 5 May 2019 20:52:21 +0800 Subject: [PATCH] [New] add `jsx-no-useless-fragment` rule Co-Authored-By: Jordan Harband --- README.md | 1 + docs/rules/jsx-no-useless-fragment.md | 54 ++++++ index.js | 1 + lib/rules/jsx-no-useless-fragment.js | 210 +++++++++++++++++++++ lib/types.d.ts | 2 + lib/util/jsx.js | 45 ++++- tests/lib/rules/jsx-no-useless-fragment.js | 206 ++++++++++++++++++++ 7 files changed, 518 insertions(+), 1 deletion(-) create mode 100644 docs/rules/jsx-no-useless-fragment.md create mode 100644 lib/rules/jsx-no-useless-fragment.js create mode 100644 tests/lib/rules/jsx-no-useless-fragment.js diff --git a/README.md b/README.md index 3f2ae8e809..f6cd505a54 100644 --- a/README.md +++ b/README.md @@ -170,6 +170,7 @@ Enable the rules that you would like to use. * [react/jsx-no-literals](docs/rules/jsx-no-literals.md): Prevent usage of unwrapped JSX strings * [react/jsx-no-target-blank](docs/rules/jsx-no-target-blank.md): Prevent usage of unsafe `target='_blank'` * [react/jsx-no-undef](docs/rules/jsx-no-undef.md): Disallow undeclared variables in JSX +* [react/jsx-no-useless-fragment](docs/rules/jsx-no-useless-fragment.md): Disallow unnescessary fragments (fixable) * [react/jsx-one-expression-per-line](docs/rules/jsx-one-expression-per-line.md): Limit to one expression per line in JSX * [react/jsx-curly-brace-presence](docs/rules/jsx-curly-brace-presence.md): Enforce curly braces or disallow unnecessary curly braces in JSX * [react/jsx-fragments](docs/rules/jsx-fragments.md): Enforce shorthand or standard form for React fragments diff --git a/docs/rules/jsx-no-useless-fragment.md b/docs/rules/jsx-no-useless-fragment.md new file mode 100644 index 0000000000..fce91f901f --- /dev/null +++ b/docs/rules/jsx-no-useless-fragment.md @@ -0,0 +1,54 @@ +# Disallow unnecessary fragments (react/jsx-no-useless-fragment) + +A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a [keyed fragment](https://reactjs.org/docs/fragments.html#keyed-fragments). + +**Fixable:** This rule is sometimes automatically fixable using the `--fix` flag on the command line. + +## Rule Details + +The following patterns are considered warnings: + +```jsx +<>{foo} + +<> + +

<>foo

+ +<> + +foo + +foo + +
+ <> +
+
+ +
+``` + +The following patterns are **not** considered warnings: + +```jsx +<> + + + + +<>foo {bar} + +<> {foo} + +const cat = <>meow + + + <> +
+
+ + + +{item.value} +``` diff --git a/index.js b/index.js index e094c4bc31..c9511bb1e3 100644 --- a/index.js +++ b/index.js @@ -35,6 +35,7 @@ const allRules = { 'jsx-no-duplicate-props': require('./lib/rules/jsx-no-duplicate-props'), 'jsx-no-literals': require('./lib/rules/jsx-no-literals'), 'jsx-no-target-blank': require('./lib/rules/jsx-no-target-blank'), + 'jsx-no-useless-fragment': require('./lib/rules/jsx-no-useless-fragment'), 'jsx-one-expression-per-line': require('./lib/rules/jsx-one-expression-per-line'), 'jsx-no-undef': require('./lib/rules/jsx-no-undef'), 'jsx-curly-brace-presence': require('./lib/rules/jsx-curly-brace-presence'), diff --git a/lib/rules/jsx-no-useless-fragment.js b/lib/rules/jsx-no-useless-fragment.js new file mode 100644 index 0000000000..778975068a --- /dev/null +++ b/lib/rules/jsx-no-useless-fragment.js @@ -0,0 +1,210 @@ +/** + * @fileoverview Disallow useless fragments + */ + +'use strict'; + +const pragmaUtil = require('../util/pragma'); +const jsxUtil = require('../util/jsx'); +const docsUrl = require('../util/docsUrl'); + +function isJSXText(node) { + return !!node && (node.type === 'JSXText' || node.type === 'Literal'); +} + +/** + * @param {string} text + * @returns {boolean} + */ +function isOnlyWhitespace(text) { + return text.trim().length === 0; +} + +/** + * @param {ASTNode} node + * @returns {boolean} + */ +function isNonspaceJSXTextOrJSXCurly(node) { + return (isJSXText(node) && !isOnlyWhitespace(node.raw)) || node.type === 'JSXExpressionContainer'; +} + +/** + * Somehow fragment like this is useful: ee eeee eeee ...} /> + * @param {ASTNode} node + * @returns {boolean} + */ +function isFragmentWithOnlyTextAndIsNotChild(node) { + return node.children.length === 1 && + isJSXText(node.children[0]) && + !(node.parent.type === 'JSXElement' || node.parent.type === 'JSXFragment'); +} + +/** + * @param {string} text + * @returns {string} + */ +function trimLikeReact(text) { + const leadingSpaces = /^\s*/.exec(text)[0]; + const trailingSpaces = /\s*$/.exec(text)[0]; + + const start = leadingSpaces.includes('\n') ? leadingSpaces.length : 0; + const end = trailingSpaces.includes('\n') ? text.length - trailingSpaces.length : text.length; + + return text.slice(start, end); +} + +/** + * Test if node is like `_` + * @param {JSXElement} node + * @returns {boolean} + */ +function isKeyedElement(node) { + return node.type === 'JSXElement' && + node.openingElement.attributes && + node.openingElement.attributes.some(jsxUtil.isJSXAttributeKey); +} + +module.exports = { + meta: { + type: 'suggestion', + fixable: 'code', + docs: { + description: 'Disallow unnecessary fragments', + category: 'Possible Errors', + recommended: false, + url: docsUrl('jsx-no-useless-fragment') + }, + messages: { + NeedsMoreChidren: 'Fragments should contain more than one child - otherwise, there‘s no need for a Fragment at all.', + ChildOfHtmlElement: 'Passing a fragment to an HTML element is useless.' + } + }, + + create(context) { + const reactPragma = pragmaUtil.getFromContext(context); + const fragmentPragma = pragmaUtil.getFragmentFromContext(context); + + /** + * Test whether a node is an padding spaces trimmed by react runtime. + * @param {ASTNode} node + * @returns {boolean} + */ + function isPaddingSpaces(node) { + return isJSXText(node) && + isOnlyWhitespace(node.raw) && + node.raw.includes('\n'); + } + + /** + * Test whether a JSXElement has less than two children, excluding paddings spaces. + * @param {JSXElement|JSXFragment} node + * @returns {boolean} + */ + function hasLessThanTwoChildren(node) { + if (!node || !node.children || node.children.length < 2) { + return true; + } + + return ( + node.children.length - + (+isPaddingSpaces(node.children[0])) - + (+isPaddingSpaces(node.children[node.children.length - 1])) + ) < 2; + } + + /** + * @param {JSXElement|JSXFragment} node + * @returns {boolean} + */ + function isChildOfHtmlElement(node) { + return node.parent.type === 'JSXElement' && + node.parent.openingElement.name.type === 'JSXIdentifier' && + /^[a-z]+$/.test(node.parent.openingElement.name.name); + } + + /** + * @param {JSXElement|JSXFragment} node + * @return {boolean} + */ + function isChildOfComponentElement(node) { + return node.parent.type === 'JSXElement' && + !isChildOfHtmlElement(node) && + !jsxUtil.isFragment(node.parent, reactPragma, fragmentPragma); + } + + /** + * @param {ASTNode} node + * @returns {boolean} + */ + function canFix(node) { + // Not safe to fix fragments without a jsx parent. + if (!(node.parent.type === 'JSXElement' || node.parent.type === 'JSXFragment')) { + // const a = <> + if (node.children.length === 0) { + return false; + } + + // const a = <>cat {meow} + if (node.children.some(isNonspaceJSXTextOrJSXCurly)) { + return false; + } + } + + // Not safe to fix `<>foo` because `Eeee` might require its children be a ReactElement. + if (isChildOfComponentElement(node)) { + return false; + } + + return true; + } + + /** + * @param {ASTNode} node + * @returns {Function | undefined} + */ + function getFix(node) { + if (!canFix(node)) { + return undefined; + } + + return function fix(fixer) { + const opener = node.type === 'JSXFragment' ? node.openingFragment : node.openingElement; + const closer = node.type === 'JSXFragment' ? node.closingFragment : node.closingElement; + const childrenText = context.getSourceCode().getText().slice(opener.range[1], closer.range[0]); + + return fixer.replaceText(node, trimLikeReact(childrenText)); + }; + } + + function checkNode(node) { + if (isKeyedElement(node)) { + return; + } + + if (hasLessThanTwoChildren(node) && !isFragmentWithOnlyTextAndIsNotChild(node)) { + context.report({ + node, + messageId: 'NeedsMoreChidren', + fix: getFix(node) + }); + } + + if (isChildOfHtmlElement(node)) { + context.report({ + node, + messageId: 'ChildOfHtmlElement', + fix: getFix(node) + }); + } + } + + return { + JSXElement(node) { + if (jsxUtil.isFragment(node, reactPragma, fragmentPragma)) { + checkNode(node); + } + }, + JSXFragment: checkNode + }; + } +}; diff --git a/lib/types.d.ts b/lib/types.d.ts index d1ae21f948..91e5b4bd48 100644 --- a/lib/types.d.ts +++ b/lib/types.d.ts @@ -9,6 +9,8 @@ declare global { type Token = eslint.AST.Token; type Fixer = eslint.Rule.RuleFixer; type JSXAttribute = ASTNode; + type JSXElement = ASTNode; + type JSXFragment = ASTNode; type JSXSpreadAttribute = ASTNode; interface Context extends eslint.SourceCode { diff --git a/lib/util/jsx.js b/lib/util/jsx.js index 3b7aac324b..6f8cbfde29 100644 --- a/lib/util/jsx.js +++ b/lib/util/jsx.js @@ -26,6 +26,35 @@ function isDOMComponent(node) { return COMPAT_TAG_REGEX.test(name); } +/** + * Test whether a JSXElement is a fragment + * @param {JSXElement} node + * @param {string} reactPragma + * @param {string} fragmentPragma + * @returns {boolean} + */ +function isFragment(node, reactPragma, fragmentPragma) { + const name = node.openingElement.name; + + // + if (name.type === 'JSXIdentifier' && name.name === fragmentPragma) { + return true; + } + + // + if ( + name.type === 'JSXMemberExpression' && + name.object.type === 'JSXIdentifier' && + name.object.name === reactPragma && + name.property.type === 'JSXIdentifier' && + name.property.name === fragmentPragma + ) { + return true; + } + + return false; +} + /** * Checks if a node represents a JSX element or fragment. * @param {object} node - node to check. @@ -35,7 +64,21 @@ function isJSX(node) { return node && ['JSXElement', 'JSXFragment'].indexOf(node.type) >= 0; } +/** + * Check if node is like `key={...}` as in `` + * @param {ASTNode} node + * @returns {boolean} + */ +function isJSXAttributeKey(node) { + return node.type === 'JSXAttribute' && + node.name && + node.name.type === 'JSXIdentifier' && + node.name.name === 'key'; +} + module.exports = { isDOMComponent, - isJSX + isFragment, + isJSX, + isJSXAttributeKey }; diff --git a/tests/lib/rules/jsx-no-useless-fragment.js b/tests/lib/rules/jsx-no-useless-fragment.js new file mode 100644 index 0000000000..9349fd2a6d --- /dev/null +++ b/tests/lib/rules/jsx-no-useless-fragment.js @@ -0,0 +1,206 @@ +/** + * @fileoverview Test file for jsx-no-useless-fragment + */ + +'use strict'; + +// ----------------------------------------------------------------------------- +// Requirements +// ----------------------------------------------------------------------------- + +const eslint = require('eslint'); +const rule = require('../../../lib/rules/jsx-no-useless-fragment'); +const parsers = require('../../helpers/parsers'); + +const RuleTester = eslint.RuleTester; + +const parserOptions = { + ecmaVersion: 2018, + ecmaFeatures: { + jsx: true + } +}; + +// ----------------------------------------------------------------------------- +// Tests +// ----------------------------------------------------------------------------- + +const ruleTester = new RuleTester({parserOptions}); + +ruleTester.run('jsx-no-uselses-fragment', rule, { + valid: [ + { + code: '<>', + parser: parsers.BABEL_ESLINT + }, + { + code: '<>foo
', + parser: parsers.BABEL_ESLINT + }, + { + code: '<>
', + parser: parsers.BABEL_ESLINT + }, + { + code: '<>{"moo"} ', + parser: parsers.BABEL_ESLINT + }, + '', + '', + '', + { + code: '<>
', + parser: parsers.BABEL_ESLINT + }, + { + code: '
{"a"}{"b"}} />', + parser: parsers.BABEL_ESLINT + }, + { + code: '{item.value}', + parser: parsers.BABEL_ESLINT + }, + { + code: 'eeee ee eeeeeee eeeeeeee} />', + parser: parsers.BABEL_ESLINT + } + ], + invalid: [ + { + code: '<>', + output: null, + errors: [{messageId: 'NeedsMoreChidren', type: 'JSXFragment'}], + parser: parsers.BABEL_ESLINT + }, + { + code: '

moo<>foo

', + output: '

moofoo

', + errors: [{messageId: 'NeedsMoreChidren'}, {messageId: 'ChildOfHtmlElement'}], + parser: parsers.BABEL_ESLINT + }, + { + code: '<>{meow}', + output: null, + errors: [{messageId: 'NeedsMoreChidren'}], + parser: parsers.BABEL_ESLINT + }, + { + code: '

<>{meow}

', + output: '

{meow}

', + errors: [{messageId: 'NeedsMoreChidren'}, {messageId: 'ChildOfHtmlElement'}], + parser: parsers.BABEL_ESLINT + }, + { + code: '<>
', + output: '
', + errors: [{messageId: 'NeedsMoreChidren'}], + parser: parsers.BABEL_ESLINT + }, + { + code: ` + <> +
+ + `, + output: ` +
+ `, + errors: [{messageId: 'NeedsMoreChidren'}], + parser: parsers.BABEL_ESLINT + }, + { + code: '', + errors: [{messageId: 'NeedsMoreChidren'}] + }, + { + code: ` + + + + `, + output: ` + + `, + errors: [{messageId: 'NeedsMoreChidren'}] + }, + { + code: ` + + {foo} + + `, + settings: { + react: { + pragma: 'SomeReact', + fragment: 'SomeFragment' + } + }, + errors: [{messageId: 'NeedsMoreChidren'}] + }, + { + // Not safe to fix this case because `Eeee` might require child be ReactElement + code: '<>foo', + output: null, + errors: [{messageId: 'NeedsMoreChidren'}], + parser: parsers.BABEL_ESLINT + }, + { + code: '
<>foo
', + output: '
foo
', + errors: [{messageId: 'NeedsMoreChidren'}, {messageId: 'ChildOfHtmlElement'}], + parser: parsers.BABEL_ESLINT + }, + { + code: '
<>{"a"}{"b"}
', + output: '
{"a"}{"b"}
', + errors: [{messageId: 'ChildOfHtmlElement'}], + parser: parsers.BABEL_ESLINT + }, + { + code: ` +
+ + + <>{"a"}{"b"} +
`, + output: ` +
+ + + {"a"}{"b"} +
`, + errors: [{messageId: 'ChildOfHtmlElement'}], + parser: parsers.BABEL_ESLINT + }, + { + code: '
{"a"}{"b"}
', + output: '
{"a"}{"b"}
', + errors: [{messageId: 'ChildOfHtmlElement'}] + }, + { + // whitepace tricky case + code: ` +
+ git<> + hub. + + + git<> hub +
`, + output: ` +
+ github. + + git hub +
`, + errors: [{messageId: 'ChildOfHtmlElement'}, {messageId: 'ChildOfHtmlElement'}], + parser: parsers.BABEL_ESLINT + }, + { + code: '
a <>{""}{""} a
', + output: '
a {""}{""} a
', + errors: [{messageId: 'ChildOfHtmlElement'}], + parser: parsers.BABEL_ESLINT + } + ] +});