From 0d01dce396a9f1c3231d7e5fb854e59418deec4e Mon Sep 17 00:00:00 2001 From: Joachim Seminck Date: Wed, 5 Jul 2017 10:35:09 +0200 Subject: [PATCH 1/7] Validate parenthesis in JSXExpressionContainers --- lib/rules/jsx-wrap-multilines.js | 14 ++++++++++- tests/lib/rules/jsx-wrap-multilines.js | 32 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/lib/rules/jsx-wrap-multilines.js b/lib/rules/jsx-wrap-multilines.js index ca12f033e6..f734af5a71 100644 --- a/lib/rules/jsx-wrap-multilines.js +++ b/lib/rules/jsx-wrap-multilines.js @@ -14,7 +14,8 @@ const DEFAULTS = { declaration: true, assignment: true, return: true, - arrow: true + arrow: true, + conditional: false }; // ------------------------------------------------------------------------------ @@ -44,6 +45,9 @@ module.exports = { }, arrow: { type: 'boolean' + }, + conditional: { + type: 'boolean' } }, additionalProperties: false @@ -126,6 +130,14 @@ module.exports = { } }, + JSXExpressionContainer: function(node) { + if (!isEnabled('conditional')) { + return; + } + + check(node.expression.right); + }, + 'ArrowFunctionExpression:exit': function (node) { const arrowBody = node.body; diff --git a/tests/lib/rules/jsx-wrap-multilines.js b/tests/lib/rules/jsx-wrap-multilines.js index 3d3b7bdc1d..690b6fb6d1 100644 --- a/tests/lib/rules/jsx-wrap-multilines.js +++ b/tests/lib/rules/jsx-wrap-multilines.js @@ -134,6 +134,26 @@ const ARROW_NO_PAREN = ` ; `; +const CONDITIONAL_PAREN = ` +
+ {unreadMessages.length > 0 && + (

+ You have {unreadMessages.length} unread messages. +

) + } +
+`; + +const CONDITIONAL_NO_PAREN = ` +
+ {unreadMessages.length > 0 && +

+ You have {unreadMessages.length} unread messages. +

+ } +
+`; + // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ @@ -187,6 +207,13 @@ ruleTester.run('jsx-wrap-multilines', rule, { }, { code: ARROW_NO_PAREN, options: [{arrow: false}] + }, { + code: CONDITIONAL_PAREN + }, { + code: CONDITIONAL_PAREN, + options: [{conditional: true}] + }, { + code: CONDITIONAL_NO_PAREN } ], @@ -237,6 +264,11 @@ ruleTester.run('jsx-wrap-multilines', rule, { output: ARROW_PAREN, options: [{arrow: true}], errors: [{message: 'Missing parentheses around multilines JSX'}] + }, { + code: CONDITIONAL_NO_PAREN, + output: CONDITIONAL_PAREN, + options: [{conditional: true}], + errors: [{message: 'Missing parentheses around multilines JSX'}] } ] }); From 4c943775d4457b1c135179febec9b54aa595af6b Mon Sep 17 00:00:00 2001 From: Joachim Seminck Date: Wed, 5 Jul 2017 10:36:59 +0200 Subject: [PATCH 2/7] Add a test when using conditional with a single line JSX: that should still be valid. --- tests/lib/rules/jsx-wrap-multilines.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/lib/rules/jsx-wrap-multilines.js b/tests/lib/rules/jsx-wrap-multilines.js index 690b6fb6d1..991029db20 100644 --- a/tests/lib/rules/jsx-wrap-multilines.js +++ b/tests/lib/rules/jsx-wrap-multilines.js @@ -134,6 +134,12 @@ const ARROW_NO_PAREN = ` ; `; +const CONDITIONAL_SINGLE_LINE = ` +
+ {unreadMessages.length > 0 &&
Hello World
} +
+`; + const CONDITIONAL_PAREN = `
{unreadMessages.length > 0 && @@ -212,6 +218,9 @@ ruleTester.run('jsx-wrap-multilines', rule, { }, { code: CONDITIONAL_PAREN, options: [{conditional: true}] + }, { + code: CONDITIONAL_SINGLE_LINE, + options: [{conditional: true}] }, { code: CONDITIONAL_NO_PAREN } From 4a5b9fbf627f615982c3dd54eba08aa2346b8360 Mon Sep 17 00:00:00 2001 From: Joachim Seminck Date: Wed, 5 Jul 2017 10:44:03 +0200 Subject: [PATCH 3/7] Update docs --- docs/rules/jsx-wrap-multilines.md | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/docs/rules/jsx-wrap-multilines.md b/docs/rules/jsx-wrap-multilines.md index d80bf70d04..a55219b93a 100644 --- a/docs/rules/jsx-wrap-multilines.md +++ b/docs/rules/jsx-wrap-multilines.md @@ -6,7 +6,7 @@ Wrapping multiline JSX in parentheses can improve readability and/or convenience ## Rule Details -This rule optionally takes a second parameter in the form of an object, containing places to apply the rule. By default, all the syntax listed below will be checked, but these can be explicitly disabled. Any syntax type missing in the object will follow the default behavior (become enabled). +This rule optionally takes a second parameter in the form of an object, containing places to apply the rule. By default, all the syntax listed below will be checked except the conditionals, but these can be explicitly disabled. Any syntax type missing in the object will follow the default behavior (become enabled). There are the possible syntax available: @@ -14,6 +14,7 @@ There are the possible syntax available: * `assignment` * `return` * `arrow` +* `conditional` (not enabled by default) The following patterns are considered warnings: @@ -101,6 +102,7 @@ function hello() { ); } ``` + The following patterns are considered warnings when configured `{arrow: true}`. ```jsx @@ -118,3 +120,27 @@ var hello = () => (
); ``` + +The following patterns are considered warnings when configured `{conditional: true}`. + +```jsx +
+ {myExpression && +
+

Hello World

+
+ } +
+``` + +The following patterns are not considered warnings when configured `{arrow: true}`. + +```jsx +
+ {myExpression && ( +
+

Hello World

+
+ )} +
+``` From eb25e26c821fce102ca88c9e867c004cc2cee9cb Mon Sep 17 00:00:00 2001 From: Joachim Seminck Date: Wed, 5 Jul 2017 10:44:55 +0200 Subject: [PATCH 4/7] Add another valid test case when the parenthesis are not on the same line as the opening/closing JSX elements --- tests/lib/rules/jsx-wrap-multilines.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/lib/rules/jsx-wrap-multilines.js b/tests/lib/rules/jsx-wrap-multilines.js index 991029db20..45dfcd03bf 100644 --- a/tests/lib/rules/jsx-wrap-multilines.js +++ b/tests/lib/rules/jsx-wrap-multilines.js @@ -150,6 +150,16 @@ const CONDITIONAL_PAREN = ` `; +const CONDITIONAL_PAREN_DIFFERENT_LINE = ` +
+ {unreadMessages.length > 0 && ( +

+ You have {unreadMessages.length} unread messages. +

+ )} +
+`; + const CONDITIONAL_NO_PAREN = `
{unreadMessages.length > 0 && @@ -218,6 +228,9 @@ ruleTester.run('jsx-wrap-multilines', rule, { }, { code: CONDITIONAL_PAREN, options: [{conditional: true}] + }, { + code: CONDITIONAL_PAREN_DIFFERENT_LINE, + options: [{conditional: true}] }, { code: CONDITIONAL_SINGLE_LINE, options: [{conditional: true}] From 09d6ad463f0c858a29f000d87f3c54a530650ef0 Mon Sep 17 00:00:00 2001 From: Joachim Seminck Date: Wed, 5 Jul 2017 19:18:01 +0200 Subject: [PATCH 5/7] Better fixer for conditional --- lib/rules/jsx-wrap-multilines.js | 13 ++++++++++--- tests/lib/rules/jsx-wrap-multilines.js | 13 ------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/lib/rules/jsx-wrap-multilines.js b/lib/rules/jsx-wrap-multilines.js index f734af5a71..6baed96371 100644 --- a/lib/rules/jsx-wrap-multilines.js +++ b/lib/rules/jsx-wrap-multilines.js @@ -70,7 +70,7 @@ module.exports = { return node.loc.start.line !== node.loc.end.line; } - function check(node) { + function check(node, fixerFn) { if (!node || node.type !== 'JSXElement') { return; } @@ -79,7 +79,7 @@ module.exports = { context.report({ node: node, message: 'Missing parentheses around multilines JSX', - fix: function(fixer) { + fix: fixerFn || function(fixer) { return fixer.replaceText(node, `(${sourceCode.getText(node)})`); } }); @@ -135,7 +135,14 @@ module.exports = { return; } - check(node.expression.right); + const rightNode = node.expression.right; + + check(rightNode, (fixer) => { + return [ + fixer.insertTextAfterRange(sourceCode.getTokenBefore(rightNode).range, ' ('), + fixer.insertTextBeforeRange(sourceCode.getLastToken(node).range, ')') + ]; + }); }, 'ArrowFunctionExpression:exit': function (node) { diff --git a/tests/lib/rules/jsx-wrap-multilines.js b/tests/lib/rules/jsx-wrap-multilines.js index 45dfcd03bf..1dac8fa1b1 100644 --- a/tests/lib/rules/jsx-wrap-multilines.js +++ b/tests/lib/rules/jsx-wrap-multilines.js @@ -141,16 +141,6 @@ const CONDITIONAL_SINGLE_LINE = ` `; const CONDITIONAL_PAREN = ` -
- {unreadMessages.length > 0 && - (

- You have {unreadMessages.length} unread messages. -

) - } -
-`; - -const CONDITIONAL_PAREN_DIFFERENT_LINE = `
{unreadMessages.length > 0 && (

@@ -228,9 +218,6 @@ ruleTester.run('jsx-wrap-multilines', rule, { }, { code: CONDITIONAL_PAREN, options: [{conditional: true}] - }, { - code: CONDITIONAL_PAREN_DIFFERENT_LINE, - options: [{conditional: true}] }, { code: CONDITIONAL_SINGLE_LINE, options: [{conditional: true}] From bbe67f97e234135fe853175a26f9025234340caa Mon Sep 17 00:00:00 2001 From: Joachim Seminck Date: Thu, 6 Jul 2017 08:33:05 +0200 Subject: [PATCH 6/7] Update issue in the doc --- docs/rules/jsx-wrap-multilines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/jsx-wrap-multilines.md b/docs/rules/jsx-wrap-multilines.md index a55219b93a..26a8511997 100644 --- a/docs/rules/jsx-wrap-multilines.md +++ b/docs/rules/jsx-wrap-multilines.md @@ -133,7 +133,7 @@ The following patterns are considered warnings when configured `{conditional: tr

``` -The following patterns are not considered warnings when configured `{arrow: true}`. +The following patterns are not considered warnings when configured `{conditional: true}`. ```jsx
From bb533a356127e59872df92f9ee6f9d591732408e Mon Sep 17 00:00:00 2001 From: Joachim Seminck Date: Sun, 23 Jul 2017 08:38:16 +0300 Subject: [PATCH 7/7] Fix linting errors --- lib/rules/jsx-wrap-multilines.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/rules/jsx-wrap-multilines.js b/lib/rules/jsx-wrap-multilines.js index 6baed96371..db8ed6b651 100644 --- a/lib/rules/jsx-wrap-multilines.js +++ b/lib/rules/jsx-wrap-multilines.js @@ -137,12 +137,10 @@ module.exports = { const rightNode = node.expression.right; - check(rightNode, (fixer) => { - return [ - fixer.insertTextAfterRange(sourceCode.getTokenBefore(rightNode).range, ' ('), - fixer.insertTextBeforeRange(sourceCode.getLastToken(node).range, ')') - ]; - }); + check(rightNode, fixer => ([ + fixer.insertTextAfterRange(sourceCode.getTokenBefore(rightNode).range, ' ('), + fixer.insertTextBeforeRange(sourceCode.getLastToken(node).range, ')') + ])); }, 'ArrowFunctionExpression:exit': function (node) {