From c5eebe70f113cfaa5e2e02f067f81373897d7480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Sat, 21 Jul 2018 22:42:06 +0200 Subject: [PATCH 1/2] Ignore Boolean props in require-default-prop rule --- docs/rules/require-default-prop.md | 10 +++++-- lib/rules/require-default-prop.js | 38 +++++++++++++++++++++++-- tests/lib/rules/require-default-prop.js | 33 ++++++++++++++------- 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/docs/rules/require-default-prop.md b/docs/rules/require-default-prop.md index 93fc6b5a9..3fb467b7a 100644 --- a/docs/rules/require-default-prop.md +++ b/docs/rules/require-default-prop.md @@ -2,7 +2,7 @@ - :gear: This rule is included in `"plugin:vue/strongly-recommended"` and `"plugin:vue/recommended"`. -This rule requires default value to be set for each props that are not marked as `required`. +This rule requires default value to be set for each props that are not marked as `required` (except `Boolean` props). ## Rule Details @@ -12,10 +12,11 @@ Examples of **incorrect** code for this rule: props: { a: Number, b: [Number, String], - c: { + c: [Boolean, Number], + d: { type: Number }, - d: { + e: { type: Number, required: false } @@ -38,6 +39,9 @@ props: { type: Number, default: 0, required: false + }, + d: { + type: Boolean, // Boolean is the only type that doesn't require default } } ``` diff --git a/lib/rules/require-default-prop.js b/lib/rules/require-default-prop.js index 19cf4fe68..6889c1de8 100644 --- a/lib/rules/require-default-prop.js +++ b/lib/rules/require-default-prop.js @@ -61,7 +61,7 @@ module.exports = { /** * Finds all props that don't have a default value set * @param {Property} propsNode - Vue component's "props" node - * @return {boolean} + * @return {Array} Array of props without "default" value */ function findPropsWithoutDefaultValue (propsNode) { return propsNode.value.properties @@ -75,6 +75,39 @@ module.exports = { }) } + /** + * Detects whether given prop is a Boolean + * @param {Node} prop + * @return {Boolean} + */ + function isBooleanProp (prop) { + const value = prop.value + return ( + value.type === 'Identifier' && + value.name === 'Boolean' + ) || ( + value.type === 'ArrayExpression' && + value.elements.length === 1 && + value.elements[0].type === 'Identifier' && + value.elements[0].name === 'Boolean' + ) || ( + value.type === 'ObjectExpression' && + value.properties.find(p => p.key.type === 'Identifier' && + p.key.name === 'type' && + p.value.type === 'Identifier' && + p.value.name === 'Boolean') + ) + } + + /** + * Excludes purely Boolean props from the Array + * @param {Array} props - Array with props + * @return {Array} + */ + function excludeBooleanProps (props) { + return props.filter(prop => !isBooleanProp(prop)) + } + // ---------------------------------------------------------------------- // Public // ---------------------------------------------------------------------- @@ -91,8 +124,9 @@ module.exports = { if (!propsNode) return const propsWithoutDefault = findPropsWithoutDefaultValue(propsNode) + const propsToReport = excludeBooleanProps(propsWithoutDefault) - propsWithoutDefault.forEach(prop => { + propsToReport.forEach(prop => { context.report({ node: prop, message: `Prop '{{propName}}' requires default value to be set.`, diff --git a/tests/lib/rules/require-default-prop.js b/tests/lib/rules/require-default-prop.js index df5fc0afc..2c9709a60 100644 --- a/tests/lib/rules/require-default-prop.js +++ b/tests/lib/rules/require-default-prop.js @@ -20,7 +20,7 @@ const parserOptions = { // Tests // ------------------------------------------------------------------------------ -const ruleTester = new RuleTester() +const ruleTester = new RuleTester({ parserOptions }) ruleTester.run('require-default-prop', rule, { valid: [ @@ -47,12 +47,24 @@ ruleTester.run('require-default-prop', rule, { required: false, 'default': 'lorem' }, + e: { + type: Boolean + }, + f: { + type: Boolean, + required: false + }, + g: { + type: Boolean, + default: true + }, + h: Boolean, + i: [Boolean], // eslint-disable-next-line require-default-prop - e: Number + j: Number } } - `, - parserOptions + ` }, { filename: 'test.vue', @@ -71,8 +83,7 @@ ruleTester.run('require-default-prop', rule, { } } } - `, - parserOptions + ` }, { filename: 'test.vue', @@ -99,8 +110,7 @@ ruleTester.run('require-default-prop', rule, { } } } - `, - parserOptions + ` } ], @@ -118,11 +128,11 @@ ruleTester.run('require-default-prop', rule, { d: { type: Number, required: false - } + }, + e: [Boolean, String], } } `, - parserOptions, errors: [{ message: `Prop 'a' requires default value to be set.`, line: 4 @@ -135,6 +145,9 @@ ruleTester.run('require-default-prop', rule, { }, { message: `Prop 'd' requires default value to be set.`, line: 9 + }, { + message: `Prop 'e' requires default value to be set.`, + line: 13 }] } ] From d678a0862f49114cede97b73d12bbb89dd2ae70a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Mon, 30 Jul 2018 11:29:51 +0200 Subject: [PATCH 2/2] Support edgecase with array of types --- lib/rules/require-default-prop.js | 29 +++++++++++++++++-------- tests/lib/rules/require-default-prop.js | 15 ++++++++++--- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/lib/rules/require-default-prop.js b/lib/rules/require-default-prop.js index 9280b7a9c..ac4cddb34 100644 --- a/lib/rules/require-default-prop.js +++ b/lib/rules/require-default-prop.js @@ -76,12 +76,11 @@ module.exports = { } /** - * Detects whether given prop is a Boolean - * @param {Node} prop + * Detects whether given value node is a Boolean type + * @param {Node} value * @return {Boolean} */ - function isBooleanProp (prop) { - const value = prop.value + function isValueNodeOfBooleanType (value) { return ( value.type === 'Identifier' && value.name === 'Boolean' @@ -90,12 +89,24 @@ module.exports = { value.elements.length === 1 && value.elements[0].type === 'Identifier' && value.elements[0].name === 'Boolean' - ) || ( + ) + } + + /** + * Detects whether given prop node is a Boolean + * @param {Node} prop + * @return {Boolean} + */ + function isBooleanProp (prop) { + const value = prop.value + + return isValueNodeOfBooleanType(value) || ( value.type === 'ObjectExpression' && - value.properties.find(p => p.key.type === 'Identifier' && - p.key.name === 'type' && - p.value.type === 'Identifier' && - p.value.name === 'Boolean') + value.properties.find(p => + p.key.type === 'Identifier' && + p.key.name === 'type' && + isValueNodeOfBooleanType(p.value) + ) ) } diff --git a/tests/lib/rules/require-default-prop.js b/tests/lib/rules/require-default-prop.js index 3edd87e32..e5f2f3462 100644 --- a/tests/lib/rules/require-default-prop.js +++ b/tests/lib/rules/require-default-prop.js @@ -57,10 +57,13 @@ ruleTester.run('require-default-prop', rule, { type: Boolean, default: true }, - h: Boolean, - i: [Boolean], + h: { + type: [Boolean] + }, + i: Boolean, + j: [Boolean], // eslint-disable-next-line require-default-prop - j: Number + k: Number } } ` @@ -129,6 +132,9 @@ ruleTester.run('require-default-prop', rule, { required: false }, e: [Boolean, String], + f: { + type: [Boolean, String], + } } } `, @@ -147,6 +153,9 @@ ruleTester.run('require-default-prop', rule, { }, { message: `Prop 'e' requires default value to be set.`, line: 13 + }, { + message: `Prop 'f' requires default value to be set.`, + line: 14 }] } ]