From dbe6686f6745db9e9ca5ec7e37e70f73d152d5d8 Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Sat, 10 Jun 2017 22:28:09 -0700 Subject: [PATCH 1/3] [New] `jsx-boolean-value`: add inverse option for always/never Fixes #1249 --- docs/rules/jsx-boolean-value.md | 12 ++-- lib/rules/jsx-boolean-value.js | 103 +++++++++++++++++++-------- tests/lib/rules/jsx-boolean-value.js | 30 ++++++-- 3 files changed, 106 insertions(+), 39 deletions(-) diff --git a/docs/rules/jsx-boolean-value.md b/docs/rules/jsx-boolean-value.md index 5dcfabe9e3..c888e7af70 100644 --- a/docs/rules/jsx-boolean-value.md +++ b/docs/rules/jsx-boolean-value.md @@ -6,27 +6,29 @@ ## Rule Details -This rule takes one argument. If it is `"always"` then it warns whenever an attribute is missing its value. If `"never"` then it warns if an attribute has a `true` value. The default value of this option is `"never"`. +This rule takes two arguments. If the first argument is `"always"` then it warns whenever an attribute is missing its value. If `"never"` then it warns if an attribute has a `true` value. The default value of this option is `"never"`. -The following patterns are considered warnings when configured `"never"`: +The second argument is optional: if provided, it must be an object with a `"never"` property (if the first argument is `"always"`), or an `"always"` property (if the first argument is `"never"`). This property’s value must be an array of strings representing prop names. + +The following patterns are considered warnings when configured `"never"`, or with `"always", { "never": ["personal"] }`: ```jsx var Hello = ; ``` -The following patterns are not considered warnings when configured `"never"`: +The following patterns are not considered warnings when configured `"never"`, or with `"always", { "never": ["personal"] }`: ```jsx var Hello = ; ``` -The following patterns are considered warnings when configured `"always"`: +The following patterns are considered warnings when configured `"always"`, or with `"never", { "always": ["personal"] }`: ```jsx var Hello = ; ``` -The following patterns are not considered warnings when configured `"always"`: +The following patterns are not considered warnings when configured `"always"`, or with `"never", { "always": ["personal"] }`: ```jsx var Hello = ; diff --git a/lib/rules/jsx-boolean-value.js b/lib/rules/jsx-boolean-value.js index 3a824e5ab9..7581be7eb3 100644 --- a/lib/rules/jsx-boolean-value.js +++ b/lib/rules/jsx-boolean-value.js @@ -8,6 +8,15 @@ // Rule Definition // ------------------------------------------------------------------------------ +const exceptionsSchema = { + type: 'array', + items: {type: 'string', minLength: 1}, + uniqueItems: true +}; + +const ALWAYS = 'always'; +const NEVER = 'never'; + module.exports = { meta: { docs: { @@ -17,44 +26,78 @@ module.exports = { }, fixable: 'code', - schema: [{ - enum: ['always', 'never'] - }] + schema: { + anyOf: [{ + type: 'array', + items: [{enum: [ALWAYS, NEVER]}], + additionalItems: false + }, { + type: 'array', + items: [{ + enum: [ALWAYS] + }, { + type: 'object', + additionalProperties: false, + properties: { + [NEVER]: exceptionsSchema + } + }], + additionalItems: false + }, { + type: 'array', + items: [{ + enum: [NEVER] + }, { + type: 'object', + additionalProperties: false, + properties: { + [ALWAYS]: exceptionsSchema + } + }], + additionalItems: false + }] + } }, create: function(context) { - var configuration = context.options[0] || 'never'; + const configuration = context.options[0] || NEVER; + const configObject = context.options[1] || {}; + const exceptions = new Set((configuration === ALWAYS ? configObject[NEVER] : configObject[ALWAYS]) || []); + const exceptionProps = Array.from(exceptions, (name) => `\`${name}\``).join(', '); + const exceptionsMessage = exceptions.size > 0 ? ` for the following props: ${exceptionProps}` : ''; + const data = {exceptionsMessage: exceptionsMessage}; - var NEVER_MESSAGE = 'Value must be omitted for boolean attributes'; - var ALWAYS_MESSAGE = 'Value must be set for boolean attributes'; + const NEVER_MESSAGE = 'Value must be omitted for boolean attributes{{exceptionsMessage}}'; + const ALWAYS_MESSAGE = 'Value must be set for boolean attributes{{exceptionsMessage}}'; return { - JSXAttribute: function(node) { - switch (configuration) { - case 'always': - if (node.value === null) { - context.report({ - node: node, - message: ALWAYS_MESSAGE, - fix: function(fixer) { - return fixer.insertTextAfter(node, '={true}'); - } - }); + JSXAttribute(node) { + const propName = node.name && node.name.name; + const value = node.value; + const isException = exceptions.has(propName); + + const isAlways = configuration === ALWAYS ? !isException : isException; + const isNever = configuration === NEVER ? !isException : isException; + + if (isAlways && value === null) { + context.report({ + node: node, + message: ALWAYS_MESSAGE, + data: data, + fix(fixer) { + return fixer.insertTextAfter(node, '={true}'); } - break; - case 'never': - if (node.value && node.value.type === 'JSXExpressionContainer' && node.value.expression.value === true) { - context.report({ - node: node, - message: NEVER_MESSAGE, - fix: function(fixer) { - return fixer.removeRange([node.name.range[1], node.value.range[1]]); - } - }); + }); + } + if (isNever && value && value.type === 'JSXExpressionContainer' && value.expression.value === true) { + context.report({ + node: node, + message: NEVER_MESSAGE, + data: data, + fix(fixer) { + return fixer.removeRange([node.name.range[1], value.range[1]]); } - break; - default: - break; + }); } } }; diff --git a/tests/lib/rules/jsx-boolean-value.js b/tests/lib/rules/jsx-boolean-value.js index 67ab4ddaed..d737ef8d5f 100644 --- a/tests/lib/rules/jsx-boolean-value.js +++ b/tests/lib/rules/jsx-boolean-value.js @@ -28,20 +28,42 @@ var ruleTester = new RuleTester({parserOptions}); ruleTester.run('jsx-boolean-value', rule, { valid: [ {code: ';', options: ['never']}, + {code: ';', options: ['always', {never: ['foo']}]}, {code: ';'}, - {code: ';', options: ['always']} + {code: ';', options: ['always']}, + {code: ';', options: ['never', {always: ['foo']}]} ], invalid: [{ - code: ';', output: ';', options: ['never'], + code: ';', + output: ';', + options: ['never'], errors: [{message: 'Value must be omitted for boolean attributes'}] }, { - code: ';', output: ';', + code: ';', + output: ';', + options: ['always', {never: ['foo', 'bar']}], + errors: [ + {message: 'Value must be omitted for boolean attributes for the following props: `foo`, `bar`'}, + {message: 'Value must be omitted for boolean attributes for the following props: `foo`, `bar`'} + ] + }, { + code: ';', + output: ';', errors: [{message: 'Value must be omitted for boolean attributes'}] }, { - code: ';', output: ';', + code: ';', + output: ';', errors: [{message: 'Value must be omitted for boolean attributes'}] }, { code: ';', output: ';', options: ['always'], errors: [{message: 'Value must be set for boolean attributes'}] + }, { + code: ';', + output: ';', + options: ['never', {always: ['foo', 'bar']}], + errors: [ + {message: 'Value must be set for boolean attributes for the following props: `foo`, `bar`'}, + {message: 'Value must be set for boolean attributes for the following props: `foo`, `bar`'} + ] }] }); From ed4978c3651f82766902180bd07b94b58fe77962 Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Wed, 21 Jun 2017 19:48:11 -0700 Subject: [PATCH 2/3] Only build up data when needed. Per https://github.com/yannickcr/eslint-plugin-react/pull/1250#discussion_r123388966 --- lib/rules/jsx-boolean-value.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/rules/jsx-boolean-value.js b/lib/rules/jsx-boolean-value.js index 7581be7eb3..2ad941c1a7 100644 --- a/lib/rules/jsx-boolean-value.js +++ b/lib/rules/jsx-boolean-value.js @@ -17,6 +17,16 @@ const exceptionsSchema = { const ALWAYS = 'always'; const NEVER = 'never'; +const errorData = new WeakMap(); +function getErrorData(exceptions) { + if (!errorData.has(exceptions)) { + const exceptionProps = Array.from(exceptions, (name) => `\`${name}\``).join(', '); + const exceptionsMessage = exceptions.size > 0 ? ` for the following props: ${exceptionProps}` : ''; + errorData.set(exceptions, {exceptionsMessage: exceptionsMessage}); + } + return errorData.get(exceptions); +} + module.exports = { meta: { docs: { @@ -63,9 +73,6 @@ module.exports = { const configuration = context.options[0] || NEVER; const configObject = context.options[1] || {}; const exceptions = new Set((configuration === ALWAYS ? configObject[NEVER] : configObject[ALWAYS]) || []); - const exceptionProps = Array.from(exceptions, (name) => `\`${name}\``).join(', '); - const exceptionsMessage = exceptions.size > 0 ? ` for the following props: ${exceptionProps}` : ''; - const data = {exceptionsMessage: exceptionsMessage}; const NEVER_MESSAGE = 'Value must be omitted for boolean attributes{{exceptionsMessage}}'; const ALWAYS_MESSAGE = 'Value must be set for boolean attributes{{exceptionsMessage}}'; @@ -80,6 +87,7 @@ module.exports = { const isNever = configuration === NEVER ? !isException : isException; if (isAlways && value === null) { + const data = getErrorData(exceptions); context.report({ node: node, message: ALWAYS_MESSAGE, @@ -90,6 +98,7 @@ module.exports = { }); } if (isNever && value && value.type === 'JSXExpressionContainer' && value.expression.value === true) { + const data = getErrorData(exceptions); context.report({ node: node, message: NEVER_MESSAGE, From a7941015f461abeeb704395cf6518d19616b00ee Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Wed, 21 Jun 2017 21:09:44 -0700 Subject: [PATCH 3/3] Refactor isAlways/isNever logic Per https://github.com/yannickcr/eslint-plugin-react/pull/1250#discussion_r123389197 --- lib/rules/jsx-boolean-value.js | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/rules/jsx-boolean-value.js b/lib/rules/jsx-boolean-value.js index 2ad941c1a7..21bfa6cdbe 100644 --- a/lib/rules/jsx-boolean-value.js +++ b/lib/rules/jsx-boolean-value.js @@ -27,6 +27,22 @@ function getErrorData(exceptions) { return errorData.get(exceptions); } +function isAlways(configuration, exceptions, propName) { + const isException = exceptions.has(propName); + if (configuration === ALWAYS) { + return !isException; + } + return isException; +} + +function isNever(configuration, exceptions, propName) { + const isException = exceptions.has(propName); + if (configuration === NEVER) { + return !isException; + } + return isException; +} + module.exports = { meta: { docs: { @@ -69,7 +85,7 @@ module.exports = { } }, - create: function(context) { + create(context) { const configuration = context.options[0] || NEVER; const configObject = context.options[1] || {}; const exceptions = new Set((configuration === ALWAYS ? configObject[NEVER] : configObject[ALWAYS]) || []); @@ -81,12 +97,8 @@ module.exports = { JSXAttribute(node) { const propName = node.name && node.name.name; const value = node.value; - const isException = exceptions.has(propName); - - const isAlways = configuration === ALWAYS ? !isException : isException; - const isNever = configuration === NEVER ? !isException : isException; - if (isAlways && value === null) { + if (isAlways(configuration, exceptions, propName) && value === null) { const data = getErrorData(exceptions); context.report({ node: node, @@ -97,7 +109,7 @@ module.exports = { } }); } - if (isNever && value && value.type === 'JSXExpressionContainer' && value.expression.value === true) { + if (isNever(configuration, exceptions, propName) && value && value.type === 'JSXExpressionContainer' && value.expression.value === true) { const data = getErrorData(exceptions); context.report({ node: node,