From 5bef08ae08e5c3917f28a037cca36d08446ce1ab Mon Sep 17 00:00:00 2001 From: Armano Date: Tue, 6 Nov 2018 22:29:13 +0100 Subject: [PATCH 1/7] Fix issues with props * prop-name-casing: is working now with array props [literals] * prop-name-casing: reports all errors if there are non Literal keys in it * require-prop-types: reports names for types diffrent than literals * add new getPropsProperties helper to easly deal with props --- lib/rules/prop-name-casing.js | 37 ++++--------- lib/rules/require-default-prop.js | 4 +- lib/rules/require-prop-type-constructor.js | 36 +++++-------- lib/rules/require-prop-types.js | 62 ++++++++-------------- lib/rules/require-valid-default-prop.js | 17 ++---- lib/utils/index.js | 35 ++++++++++++ tests/lib/rules/prop-name-casing.js | 22 +++++++- tests/lib/rules/require-prop-types.js | 4 +- 8 files changed, 110 insertions(+), 107 deletions(-) diff --git a/lib/rules/prop-name-casing.js b/lib/rules/prop-name-casing.js index cf2fa3d67..5ab64fb4b 100644 --- a/lib/rules/prop-name-casing.js +++ b/lib/rules/prop-name-casing.js @@ -8,12 +8,12 @@ const utils = require('../utils') const casing = require('../utils/casing') const allowedCaseOptions = ['camelCase', 'snake_case'] -function canFixPropertyName (node, originalName) { +function canFixPropertyName (node, key, originalName) { // Can not fix of computed property names & shorthand if (node.computed || node.shorthand) { return false } - const key = node.key + // Can not fix of unknown types if (key.type !== 'Literal' && key.type !== 'Identifier') { return false @@ -36,42 +36,25 @@ function create (context) { // ---------------------------------------------------------------------- return utils.executeOnVue(context, (obj) => { - const node = obj.properties.find(p => - p.type === 'Property' && - p.key.type === 'Identifier' && - p.key.name === 'props' && - (p.value.type === 'ObjectExpression' || p.value.type === 'ArrayExpression') - ) - - if (!node) return + const items = utils.getPropsProperties(obj) + .filter(cp => cp.key.type === 'Literal' || (cp.key.type === 'Identifier' && !cp.node.computed)) - const items = node.value.type === 'ObjectExpression' ? node.value.properties : node.value.elements for (const item of items) { - if (item.type !== 'Property') { - return - } - if (item.computed) { - if (item.key.type !== 'Literal') { - // TemplateLiteral | Identifier(variable) | Expression(s) - return - } - if (typeof item.key.value !== 'string') { - // (boolean | null | number | RegExp) Literal - return - } - } - const propName = item.key.type === 'Literal' ? item.key.value : item.key.name + if (typeof propName !== 'string') { + // (boolean | null | number | RegExp) Literal + continue + } const convertedName = converter(propName) if (convertedName !== propName) { context.report({ - node: item, + node: item.node, message: 'Prop "{{name}}" is not in {{caseType}}.', data: { name: propName, caseType: caseType }, - fix: canFixPropertyName(item, propName) ? fixer => { + fix: canFixPropertyName(item.node, item.key, propName) ? fixer => { return item.key.type === 'Literal' ? fixer.replaceText(item.key, item.key.raw.replace(item.key.value, convertedName)) : fixer.replaceText(item.key, convertedName) diff --git a/lib/rules/require-default-prop.js b/lib/rules/require-default-prop.js index 3d75387b3..1f0bc321b 100644 --- a/lib/rules/require-default-prop.js +++ b/lib/rules/require-default-prop.js @@ -137,7 +137,7 @@ module.exports = { const propsWithoutDefault = findPropsWithoutDefaultValue(propsNode) const propsToReport = excludeBooleanProps(propsWithoutDefault) - propsToReport.forEach(prop => { + for (const prop of propsToReport) { context.report({ node: prop, message: `Prop '{{propName}}' requires default value to be set.`, @@ -145,7 +145,7 @@ module.exports = { propName: prop.key.name } }) - }) + } }) } } diff --git a/lib/rules/require-prop-type-constructor.js b/lib/rules/require-prop-type-constructor.js index abc768485..259bfc851 100644 --- a/lib/rules/require-prop-type-constructor.js +++ b/lib/rules/require-prop-type-constructor.js @@ -70,31 +70,23 @@ module.exports = { } return utils.executeOnVueComponent(context, (obj) => { - const node = obj.properties.find(p => - p.type === 'Property' && - p.key.type === 'Identifier' && - p.key.name === 'props' && - p.value.type === 'ObjectExpression' - ) + const properties = utils.getPropsProperties(obj) + .filter(cp => cp.value) - if (!node) return + for (const p of properties) { + if (isForbiddenType(p.value) || p.value.type === 'ArrayExpression') { + checkPropertyNode(p.key, p.value) + } else if (p.value.type === 'ObjectExpression') { + const typeProperty = p.value.properties.find(prop => + prop.type === 'Property' && + prop.key.name === 'type' + ) - node.value.properties - .forEach(p => { - const pValue = utils.unwrapTypes(p.value) - if (isForbiddenType(pValue) || pValue.type === 'ArrayExpression') { - checkPropertyNode(p.key, pValue) - } else if (pValue.type === 'ObjectExpression') { - const typeProperty = pValue.properties.find(prop => - prop.type === 'Property' && - prop.key.name === 'type' - ) + if (!typeProperty) continue - if (!typeProperty) return - - checkPropertyNode(p.key, utils.unwrapTypes(typeProperty.value)) - } - }) + checkPropertyNode(p.key, utils.unwrapTypes(typeProperty.value)) + } + } }) } } diff --git a/lib/rules/require-prop-types.js b/lib/rules/require-prop-types.js index 0899426bf..4bbefdc9d 100644 --- a/lib/rules/require-prop-types.js +++ b/lib/rules/require-prop-types.js @@ -42,30 +42,26 @@ module.exports = { return Boolean(typeProperty || validatorProperty) } - function checkProperties (items) { - for (const cp of items) { - if (cp.type !== 'Property') { - return - } - let hasType = true - const cpValue = utils.unwrapTypes(cp.value) + function checkProperty (key, value, node) { + let hasType = true - if (cpValue.type === 'ObjectExpression') { // foo: { - hasType = objectHasType(cpValue) - } else if (cpValue.type === 'ArrayExpression') { // foo: [ - hasType = cpValue.elements.length > 0 - } else if (cpValue.type === 'FunctionExpression' || cpValue.type === 'ArrowFunctionExpression') { - hasType = false - } - if (!hasType) { - context.report({ - node: cp, - message: 'Prop "{{name}}" should define at least its type.', - data: { - name: cp.key.name - } - }) - } + if (!value) { + hasType = false + } else if (value.type === 'ObjectExpression') { // foo: { + hasType = objectHasType(value) + } else if (value.type === 'ArrayExpression') { // foo: [ + hasType = value.elements.length > 0 + } else if (value.type === 'FunctionExpression' || value.type === 'ArrowFunctionExpression') { + hasType = false + } + if (!hasType) { + context.report({ + node, + message: 'Prop "{{name}}" should define at least its type.', + data: { + name: utils.getStaticPropertyName(key) + } + }) } } @@ -74,24 +70,10 @@ module.exports = { // ---------------------------------------------------------------------- return utils.executeOnVue(context, (obj) => { - const node = obj.properties - .find(p => - p.type === 'Property' && - p.key.type === 'Identifier' && - p.key.name === 'props' - ) + const properties = utils.getPropsProperties(obj) - if (!node) return - - if (node.value.type === 'ObjectExpression') { - checkProperties(node.value.properties) - } - - if (node.value.type === 'ArrayExpression') { - context.report({ - node, - message: 'Props should at least define their types.' - }) + for (const cp of properties) { + checkProperty(cp.key, cp.value, cp.node) } }) } diff --git a/lib/rules/require-valid-default-prop.js b/lib/rules/require-valid-default-prop.js index 21e1f2537..c83eac691 100644 --- a/lib/rules/require-valid-default-prop.js +++ b/lib/rules/require-valid-default-prop.js @@ -88,20 +88,11 @@ module.exports = { // ---------------------------------------------------------------------- return utils.executeOnVue(context, obj => { - const props = obj.properties.find(p => - isPropertyIdentifier(p) && - p.key.name === 'props' && - p.value.type === 'ObjectExpression' - ) - if (!props) return - - const properties = props.value.properties.filter(p => - isPropertyIdentifier(p) && - utils.unwrapTypes(p.value).type === 'ObjectExpression' - ) + const properties = utils.getPropsProperties(obj) + .filter(cp => cp.value && cp.value.type === 'ObjectExpression') for (const prop of properties) { - const type = getPropertyNode(utils.unwrapTypes(prop.value), 'type') + const type = getPropertyNode(prop.value, 'type') if (!type) continue const typeNames = new Set(getTypes(type.value) @@ -111,7 +102,7 @@ module.exports = { // There is no native types detected if (typeNames.size === 0) continue - const def = getPropertyNode(utils.unwrapTypes(prop.value), 'default') + const def = getPropertyNode(prop.value, 'default') if (!def) continue const defType = getValueType(def.value) diff --git a/lib/utils/index.js b/lib/utils/index.js index 4a8a8cb11..618a2e6e7 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -365,6 +365,41 @@ module.exports = { return null }, + /** + * Get all props by looking at all component's properties + * @param {ObjectExpression} Object with component definition + * @return {Array<{key: ASTNode, value: ASTNode | null, node: ASTNode}>} Array of props in format: + */ + getPropsProperties (componentObject) { + const propsNode = componentObject.properties + .find(p => + p.type === 'Property' && + p.key.type === 'Identifier' && + p.key.name === 'props' && + (p.value.type === 'ObjectExpression' || p.value.type === 'ArrayExpression') + ) + + if (!propsNode) { return [] } + + let props + + if (propsNode.value.type === 'ObjectExpression') { + props = propsNode.value.properties + .filter(cp => cp.type === 'Property') + .map(cp => { + return { key: cp.key, value: this.unwrapTypes(cp.value), node: cp } + }) + } else { + props = propsNode.value.elements + .filter(cp => cp.type === 'Literal' && typeof cp.value === 'string') + .map(cp => { + return { key: cp, value: null, node: cp } + }) + } + + return props + }, + /** * Get all computed properties by looking at all component's properties * @param {ObjectExpression} Object with component definition diff --git a/tests/lib/rules/prop-name-casing.js b/tests/lib/rules/prop-name-casing.js index d9a23a466..842e07ab8 100644 --- a/tests/lib/rules/prop-name-casing.js +++ b/tests/lib/rules/prop-name-casing.js @@ -67,7 +67,7 @@ ruleTester.run('prop-name-casing', rule, { filename: 'test.vue', code: ` export default { - props: ['greetingText'] + props: ['greeting_text'] } `, options: ['snake_case'], @@ -338,6 +338,26 @@ ruleTester.run('prop-name-casing', rule, { line: 4 }] }, + { + filename: 'test.vue', + code: ` + export default { + props: ['greeting_text'] + } + `, + options: ['camelCase'], + output: ` + export default { + props: ['greetingText'] + } + `, + parserOptions, + errors: [{ + message: 'Prop "greeting_text" is not in camelCase.', + type: 'Literal', + line: 3 + }] + }, { filename: 'test.vue', code: ` diff --git a/tests/lib/rules/require-prop-types.js b/tests/lib/rules/require-prop-types.js index 3cd2adcd1..130a0a0bf 100644 --- a/tests/lib/rules/require-prop-types.js +++ b/tests/lib/rules/require-prop-types.js @@ -154,7 +154,7 @@ ruleTester.run('require-prop-types', rule, { `, parserOptions: { ecmaVersion: 6, sourceType: 'module' }, errors: [{ - message: 'Props should at least define their types.', + message: 'Prop "foo" should define at least its type.', line: 3 }] }, @@ -167,7 +167,7 @@ ruleTester.run('require-prop-types', rule, { `, parserOptions: { ecmaVersion: 6, sourceType: 'module' }, errors: [{ - message: 'Props should at least define their types.', + message: 'Prop "foo" should define at least its type.', line: 3 }] }, From d841a94127e9bb5f8a5775ab1437b9bb8b86fd5a Mon Sep 17 00:00:00 2001 From: Armano Date: Tue, 6 Nov 2018 23:45:34 +0100 Subject: [PATCH 2/7] Add unit test for getPropsProperties --- .gitignore | 1 + lib/rules/prop-name-casing.js | 1 + lib/rules/require-default-prop.js | 2 +- lib/rules/require-prop-type-constructor.js | 1 + lib/rules/require-prop-types.js | 11 ++- lib/rules/require-valid-default-prop.js | 1 + lib/utils/index.js | 20 ++++-- tests/lib/rules/require-prop-types.js | 4 +- tests/lib/utils/index.js | 78 ++++++++++++++++++++++ 9 files changed, 109 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 1828a214c..c07de3ba2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ .idea +*.iml /.nyc_output /coverage /tests/integrations/*/node_modules diff --git a/lib/rules/prop-name-casing.js b/lib/rules/prop-name-casing.js index 5ab64fb4b..d01365a93 100644 --- a/lib/rules/prop-name-casing.js +++ b/lib/rules/prop-name-casing.js @@ -37,6 +37,7 @@ function create (context) { return utils.executeOnVue(context, (obj) => { const items = utils.getPropsProperties(obj) + .props .filter(cp => cp.key.type === 'Literal' || (cp.key.type === 'Identifier' && !cp.node.computed)) for (const item of items) { diff --git a/lib/rules/require-default-prop.js b/lib/rules/require-default-prop.js index 1f0bc321b..94b184036 100644 --- a/lib/rules/require-default-prop.js +++ b/lib/rules/require-default-prop.js @@ -21,7 +21,7 @@ module.exports = { schema: [] }, - create: function (context) { + create (context) { // ---------------------------------------------------------------------- // Helpers // ---------------------------------------------------------------------- diff --git a/lib/rules/require-prop-type-constructor.js b/lib/rules/require-prop-type-constructor.js index 259bfc851..d03f9877c 100644 --- a/lib/rules/require-prop-type-constructor.js +++ b/lib/rules/require-prop-type-constructor.js @@ -71,6 +71,7 @@ module.exports = { return utils.executeOnVueComponent(context, (obj) => { const properties = utils.getPropsProperties(obj) + .props .filter(cp => cp.value) for (const p of properties) { diff --git a/lib/rules/require-prop-types.js b/lib/rules/require-prop-types.js index 4bbefdc9d..f194aa597 100644 --- a/lib/rules/require-prop-types.js +++ b/lib/rules/require-prop-types.js @@ -72,8 +72,15 @@ module.exports = { return utils.executeOnVue(context, (obj) => { const properties = utils.getPropsProperties(obj) - for (const cp of properties) { - checkProperty(cp.key, cp.value, cp.node) + if (properties.type === 'ArrayExpression') { + context.report({ + node: properties.node, + message: 'Props should at least define their types.' + }) + } else { + for (const cp of properties.props) { + checkProperty(cp.key, cp.value, cp.node) + } } }) } diff --git a/lib/rules/require-valid-default-prop.js b/lib/rules/require-valid-default-prop.js index c83eac691..02a5004ad 100644 --- a/lib/rules/require-valid-default-prop.js +++ b/lib/rules/require-valid-default-prop.js @@ -89,6 +89,7 @@ module.exports = { return utils.executeOnVue(context, obj => { const properties = utils.getPropsProperties(obj) + .props .filter(cp => cp.value && cp.value.type === 'ObjectExpression') for (const prop of properties) { diff --git a/lib/utils/index.js b/lib/utils/index.js index 618a2e6e7..82bfc7504 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -367,8 +367,8 @@ module.exports = { /** * Get all props by looking at all component's properties - * @param {ObjectExpression} Object with component definition - * @return {Array<{key: ASTNode, value: ASTNode | null, node: ASTNode}>} Array of props in format: + * @param {ObjectExpression} componentObject Object with component definition + * @return {Object} Array of props in format: { node: ASTNode, type: string, props: Array<{key: ASTNode, value: ASTNode | null, node: ASTNode}> } */ getPropsProperties (componentObject) { const propsNode = componentObject.properties @@ -379,7 +379,13 @@ module.exports = { (p.value.type === 'ObjectExpression' || p.value.type === 'ArrayExpression') ) - if (!propsNode) { return [] } + if (!propsNode) { + return { + node: null, + type: null, + props: [] + } + } let props @@ -397,12 +403,16 @@ module.exports = { }) } - return props + return { + node: propsNode, + type: propsNode.value.type, + props: props + } }, /** * Get all computed properties by looking at all component's properties - * @param {ObjectExpression} Object with component definition + * @param {ObjectExpression} componentObject Object with component definition * @return {Array} Array of computed properties in format: [{key: String, value: ASTNode}] */ getComputedProperties (componentObject) { diff --git a/tests/lib/rules/require-prop-types.js b/tests/lib/rules/require-prop-types.js index 130a0a0bf..3cd2adcd1 100644 --- a/tests/lib/rules/require-prop-types.js +++ b/tests/lib/rules/require-prop-types.js @@ -154,7 +154,7 @@ ruleTester.run('require-prop-types', rule, { `, parserOptions: { ecmaVersion: 6, sourceType: 'module' }, errors: [{ - message: 'Prop "foo" should define at least its type.', + message: 'Props should at least define their types.', line: 3 }] }, @@ -167,7 +167,7 @@ ruleTester.run('require-prop-types', rule, { `, parserOptions: { ecmaVersion: 6, sourceType: 'module' }, errors: [{ - message: 'Prop "foo" should define at least its type.', + message: 'Props should at least define their types.', line: 3 }] }, diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js index 067ddc593..c370f10bd 100644 --- a/tests/lib/utils/index.js +++ b/tests/lib/utils/index.js @@ -247,3 +247,81 @@ describe('getRegisteredComponents', () => { ) }) }) + +describe('getComputedProperties', () => { + let node + + const parse = function (code) { + const data = babelEslint.parse(code).body[0].declarations[0].init + return utils.getPropsProperties(data) + } + + it('should return empty array when there is no props property', () => { + node = parse(`const test = { + name: 'test', + data() { + return {} + } + }`) + + assert.notOk(node.type) + assert.notOk(node.node) + assert.equal(node.props.length,0) + }) + + it('should return computed props', () => { + node = parse(`const test = { + name: 'test', + data() { + return {} + }, + props: { + ...foo, + a: String, + b: {}, + c: [String], + d + } + }`) + + assert.ok(node.node) + assert.equal(node.type, 'ObjectExpression', 'it detects correct type') + assert.equal(node.props.length, 4, 'it detects all props' ) + + assert.ok(node.props[0].value) + assert.ok(node.props[0].key.type === 'Identifier') + assert.ok(node.props[0].node.type === 'Property') + assert.ok(node.props[0].value.type === 'Identifier') + + assert.ok(node.props[1].value) + assert.ok(node.props[1].key.type === 'Identifier') + assert.ok(node.props[1].node.type === 'Property') + assert.ok(node.props[1].value.type === 'ObjectExpression') + + assert.ok(node.props[2].value) + assert.ok(node.props[2].key.type === 'Identifier') + assert.ok(node.props[2].node.type === 'Property') + assert.ok(node.props[2].value.type === 'ArrayExpression') + + assert.ok(node.props[3].value) + assert.ok(node.props[3].key.type === node.props[3].value.type) + assert.ok(node.props[3].node.type === 'Property') + }) + + it('should return computed from array props', () => { + node = parse(`const test = { + name: 'test', + data() { + return {} + }, + props: ['a', b, \`c\`, null] + }`) + + assert.ok(node.node) + assert.equal(node.type, 'ArrayExpression', 'it detects correct type') + assert.equal(node.props.length, 1, 'it detects all props' ) + + assert.notOk(node.props[0].value) + assert.ok(node.props[0].key.type === 'Literal') + }) +}) From 67e6ceddea2600d45587d6e43d8bcccf30a92760 Mon Sep 17 00:00:00 2001 From: Armano Date: Tue, 6 Nov 2018 23:49:01 +0100 Subject: [PATCH 3/7] Fix linting issues --- tests/lib/utils/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js index c370f10bd..fc2259bd9 100644 --- a/tests/lib/utils/index.js +++ b/tests/lib/utils/index.js @@ -266,7 +266,7 @@ describe('getComputedProperties', () => { assert.notOk(node.type) assert.notOk(node.node) - assert.equal(node.props.length,0) + assert.equal(node.props.length, 0) }) it('should return computed props', () => { @@ -286,7 +286,7 @@ describe('getComputedProperties', () => { assert.ok(node.node) assert.equal(node.type, 'ObjectExpression', 'it detects correct type') - assert.equal(node.props.length, 4, 'it detects all props' ) + assert.equal(node.props.length, 4, 'it detects all props') assert.ok(node.props[0].value) assert.ok(node.props[0].key.type === 'Identifier') @@ -319,7 +319,7 @@ describe('getComputedProperties', () => { assert.ok(node.node) assert.equal(node.type, 'ArrayExpression', 'it detects correct type') - assert.equal(node.props.length, 1, 'it detects all props' ) + assert.equal(node.props.length, 1, 'it detects all props') assert.notOk(node.props[0].value) assert.ok(node.props[0].key.type === 'Literal') From 1c597a8c2fcf50878d6fbc4a65071d352ba088a1 Mon Sep 17 00:00:00 2001 From: Armano Date: Wed, 7 Nov 2018 00:15:14 +0100 Subject: [PATCH 4/7] require-default-prop: * fix issue #596 * allow to use shorthand --- lib/rules/require-default-prop.js | 41 +++++++++++++------------ tests/lib/rules/require-default-prop.js | 12 ++++++++ 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/lib/rules/require-default-prop.js b/lib/rules/require-default-prop.js index 94b184036..36559c981 100644 --- a/lib/rules/require-default-prop.js +++ b/lib/rules/require-default-prop.js @@ -6,6 +6,16 @@ const utils = require('../utils') +const NATIVE_TYPES = new Set([ + 'String', + 'Number', + 'Boolean', + 'Function', + 'Object', + 'Array', + 'Symbol' +]) + // ------------------------------------------------------------------------------ // Rule Definition // ------------------------------------------------------------------------------ @@ -32,7 +42,7 @@ module.exports = { * @return {boolean} */ function propIsRequired (prop) { - const propRequiredNode = utils.unwrapTypes(prop.value).properties + const propRequiredNode = prop.value.properties .find(p => p.type === 'Property' && p.key.name === 'required' && @@ -49,7 +59,7 @@ module.exports = { * @return {boolean} */ function propHasDefault (prop) { - const propDefaultNode = utils.unwrapTypes(prop.value).properties + const propDefaultNode = prop.value.properties .find(p => p.key && (p.key.name === 'default' || p.key.value === 'default') @@ -60,15 +70,14 @@ module.exports = { /** * Finds all props that don't have a default value set - * @param {Property} propsNode - Vue component's "props" node + * @param {Array} props - Vue component's "props" node * @return {Array} Array of props without "default" value */ - function findPropsWithoutDefaultValue (propsNode) { - return propsNode.value.properties - .filter(prop => prop.type === 'Property') + function findPropsWithoutDefaultValue (props) { + return props .filter(prop => { - if (utils.unwrapTypes(prop.value).type !== 'ObjectExpression') { - return true + if (prop.value.type !== 'ObjectExpression') { + return (prop.value.type !== 'CallExpression' && prop.value.type !== 'Identifier') || NATIVE_TYPES.has(prop.value.name) } return !propIsRequired(prop) && !propHasDefault(prop) @@ -124,22 +133,16 @@ module.exports = { // ---------------------------------------------------------------------- return utils.executeOnVue(context, (obj) => { - const propsNode = obj.properties - .find(p => - p.type === 'Property' && - p.key.type === 'Identifier' && - p.key.name === 'props' && - p.value.type === 'ObjectExpression' - ) - - if (!propsNode) return + const props = utils.getPropsProperties(obj) + .props + .filter(prop => prop.value && !prop.node.shorthand) - const propsWithoutDefault = findPropsWithoutDefaultValue(propsNode) + const propsWithoutDefault = findPropsWithoutDefaultValue(props) const propsToReport = excludeBooleanProps(propsWithoutDefault) for (const prop of propsToReport) { context.report({ - node: prop, + node: prop.node, message: `Prop '{{propName}}' requires default value to be set.`, data: { propName: prop.key.name diff --git a/tests/lib/rules/require-default-prop.js b/tests/lib/rules/require-default-prop.js index cdbecaca6..7eb689a3d 100644 --- a/tests/lib/rules/require-default-prop.js +++ b/tests/lib/rules/require-default-prop.js @@ -141,6 +141,18 @@ ruleTester.run('require-default-prop', rule, { }); `, parser: 'typescript-eslint-parser' + }, + { + filename: 'test.vue', + code: ` + export default { + props: { + bar, + baz: prop, + bar1: foo() + } + } + ` } ], From d19f958200757d833cce3a3fc36cc19e4684c81b Mon Sep 17 00:00:00 2001 From: Armano Date: Wed, 7 Nov 2018 00:29:19 +0100 Subject: [PATCH 5/7] Add additioanl test fix issue #595 --- tests/lib/rules/require-prop-type-constructor.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/require-prop-type-constructor.js b/tests/lib/rules/require-prop-type-constructor.js index 81a189ef8..fd86930d7 100644 --- a/tests/lib/rules/require-prop-type-constructor.js +++ b/tests/lib/rules/require-prop-type-constructor.js @@ -15,9 +15,9 @@ const RuleTester = require('eslint').RuleTester // Tests // ------------------------------------------------------------------------------ -var ruleTester = new RuleTester({ +const ruleTester = new RuleTester({ parserOptions: { - ecmaVersion: 7, + ecmaVersion: 2018, sourceType: 'module' } }) @@ -29,6 +29,7 @@ ruleTester.run('require-prop-type-constructor', rule, { code: ` export default { props: { + ...props, myProp: Number, anotherType: [Number, String], extraProp: { From d794a895934ac8f6d798757014db965faad8dfb2 Mon Sep 17 00:00:00 2001 From: Armano Date: Wed, 7 Nov 2018 23:39:42 +0100 Subject: [PATCH 6/7] Improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * rename `getPropsProperties` to `getComponentProps` … * fix false error in `require-prop-types` when is set to empty array * `require-prop-types` will return now errors about each prop from ArrayExpression * add more tests --- lib/rules/prop-name-casing.js | 7 +- lib/rules/require-default-prop.js | 5 +- lib/rules/require-prop-type-constructor.js | 7 +- lib/rules/require-prop-types.js | 15 +--- lib/rules/require-valid-default-prop.js | 7 +- lib/utils/index.js | 20 ++--- tests/lib/rules/require-prop-types.js | 44 ++++++++++- tests/lib/utils/index.js | 89 ++++++++++++++-------- 8 files changed, 117 insertions(+), 77 deletions(-) diff --git a/lib/rules/prop-name-casing.js b/lib/rules/prop-name-casing.js index d01365a93..b424664aa 100644 --- a/lib/rules/prop-name-casing.js +++ b/lib/rules/prop-name-casing.js @@ -36,11 +36,10 @@ function create (context) { // ---------------------------------------------------------------------- return utils.executeOnVue(context, (obj) => { - const items = utils.getPropsProperties(obj) - .props - .filter(cp => cp.key.type === 'Literal' || (cp.key.type === 'Identifier' && !cp.node.computed)) + const props = utils.getComponentProps(obj) + .filter(cp => cp.key && cp.key.type === 'Literal' || (cp.key.type === 'Identifier' && !cp.node.computed)) - for (const item of items) { + for (const item of props) { const propName = item.key.type === 'Literal' ? item.key.value : item.key.name if (typeof propName !== 'string') { // (boolean | null | number | RegExp) Literal diff --git a/lib/rules/require-default-prop.js b/lib/rules/require-default-prop.js index 36559c981..1e1995de6 100644 --- a/lib/rules/require-default-prop.js +++ b/lib/rules/require-default-prop.js @@ -133,9 +133,8 @@ module.exports = { // ---------------------------------------------------------------------- return utils.executeOnVue(context, (obj) => { - const props = utils.getPropsProperties(obj) - .props - .filter(prop => prop.value && !prop.node.shorthand) + const props = utils.getComponentProps(obj) + .filter(prop => prop.key && prop.value && !prop.node.shorthand) const propsWithoutDefault = findPropsWithoutDefaultValue(props) const propsToReport = excludeBooleanProps(propsWithoutDefault) diff --git a/lib/rules/require-prop-type-constructor.js b/lib/rules/require-prop-type-constructor.js index d03f9877c..5e507abc8 100644 --- a/lib/rules/require-prop-type-constructor.js +++ b/lib/rules/require-prop-type-constructor.js @@ -70,11 +70,10 @@ module.exports = { } return utils.executeOnVueComponent(context, (obj) => { - const properties = utils.getPropsProperties(obj) - .props - .filter(cp => cp.value) + const props = utils.getComponentProps(obj) + .filter(cp => cp.key && cp.value) - for (const p of properties) { + for (const p of props) { if (isForbiddenType(p.value) || p.value.type === 'ArrayExpression') { checkPropertyNode(p.key, p.value) } else if (p.value.type === 'ObjectExpression') { diff --git a/lib/rules/require-prop-types.js b/lib/rules/require-prop-types.js index f194aa597..a70107ad5 100644 --- a/lib/rules/require-prop-types.js +++ b/lib/rules/require-prop-types.js @@ -59,7 +59,7 @@ module.exports = { node, message: 'Prop "{{name}}" should define at least its type.', data: { - name: utils.getStaticPropertyName(key) + name: utils.getStaticPropertyName(key || node) || '***' } }) } @@ -70,17 +70,10 @@ module.exports = { // ---------------------------------------------------------------------- return utils.executeOnVue(context, (obj) => { - const properties = utils.getPropsProperties(obj) + const props = utils.getComponentProps(obj) - if (properties.type === 'ArrayExpression') { - context.report({ - node: properties.node, - message: 'Props should at least define their types.' - }) - } else { - for (const cp of properties.props) { - checkProperty(cp.key, cp.value, cp.node) - } + for (const prop of props) { + checkProperty(prop.key, prop.value, prop.node) } }) } diff --git a/lib/rules/require-valid-default-prop.js b/lib/rules/require-valid-default-prop.js index 02a5004ad..7baf34d6e 100644 --- a/lib/rules/require-valid-default-prop.js +++ b/lib/rules/require-valid-default-prop.js @@ -88,11 +88,10 @@ module.exports = { // ---------------------------------------------------------------------- return utils.executeOnVue(context, obj => { - const properties = utils.getPropsProperties(obj) - .props - .filter(cp => cp.value && cp.value.type === 'ObjectExpression') + const props = utils.getComponentProps(obj) + .filter(cp => cp.key && cp.value && cp.value.type === 'ObjectExpression') - for (const prop of properties) { + for (const prop of props) { const type = getPropertyNode(prop.value, 'type') if (!type) continue diff --git a/lib/utils/index.js b/lib/utils/index.js index 82bfc7504..40ec71e92 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -368,9 +368,9 @@ module.exports = { /** * Get all props by looking at all component's properties * @param {ObjectExpression} componentObject Object with component definition - * @return {Object} Array of props in format: { node: ASTNode, type: string, props: Array<{key: ASTNode, value: ASTNode | null, node: ASTNode}> } + * @return {Array} Array of component props in format: [{key?: String, value?: ASTNode, node: ASTNod}] */ - getPropsProperties (componentObject) { + getComponentProps (componentObject) { const propsNode = componentObject.properties .find(p => p.type === 'Property' && @@ -380,11 +380,7 @@ module.exports = { ) if (!propsNode) { - return { - node: null, - type: null, - props: [] - } + return [] } let props @@ -397,17 +393,13 @@ module.exports = { }) } else { props = propsNode.value.elements - .filter(cp => cp.type === 'Literal' && typeof cp.value === 'string') .map(cp => { - return { key: cp, value: null, node: cp } + const key = cp.type === 'Literal' && typeof cp.value === 'string' ? cp : null + return { key, value: null, node: cp } }) } - return { - node: propsNode, - type: propsNode.value.type, - props: props - } + return props }, /** diff --git a/tests/lib/rules/require-prop-types.js b/tests/lib/rules/require-prop-types.js index 3cd2adcd1..f46a36e3f 100644 --- a/tests/lib/rules/require-prop-types.js +++ b/tests/lib/rules/require-prop-types.js @@ -114,6 +114,24 @@ ruleTester.run('require-prop-types', rule, { `, parserOptions: { ecmaVersion: 6, sourceType: 'module' } }, + { + filename: 'test.vue', + code: ` + export default { + props: [] + } + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module' } + }, + { + filename: 'test.vue', + code: ` + export default { + props: {} + } + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module' } + }, { filename: 'test.vue', code: ` @@ -149,12 +167,21 @@ ruleTester.run('require-prop-types', rule, { filename: 'test.vue', code: ` export default { - props: ['foo'] + props: ['foo', bar, \`baz\`, foo()] } `, parserOptions: { ecmaVersion: 6, sourceType: 'module' }, errors: [{ - message: 'Props should at least define their types.', + message: 'Prop "foo" should define at least its type.', + line: 3 + }, { + message: 'Prop "bar" should define at least its type.', + line: 3 + }, { + message: 'Prop "baz" should define at least its type.', + line: 3 + }, { + message: 'Prop "***" should define at least its type.', line: 3 }] }, @@ -162,12 +189,21 @@ ruleTester.run('require-prop-types', rule, { filename: 'test.js', code: ` new Vue({ - props: ['foo'] + props: ['foo', bar, \`baz\`, foo()] }) `, parserOptions: { ecmaVersion: 6, sourceType: 'module' }, errors: [{ - message: 'Props should at least define their types.', + message: 'Prop "foo" should define at least its type.', + line: 3 + }, { + message: 'Prop "bar" should define at least its type.', + line: 3 + }, { + message: 'Prop "baz" should define at least its type.', + line: 3 + }, { + message: 'Prop "***" should define at least its type.', line: 3 }] }, diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js index fc2259bd9..0b8c07e2d 100644 --- a/tests/lib/utils/index.js +++ b/tests/lib/utils/index.js @@ -248,30 +248,47 @@ describe('getRegisteredComponents', () => { }) }) -describe('getComputedProperties', () => { - let node +describe('getComponentProps', () => { + let props const parse = function (code) { const data = babelEslint.parse(code).body[0].declarations[0].init - return utils.getPropsProperties(data) + return utils.getComponentProps(data) } - it('should return empty array when there is no props property', () => { - node = parse(`const test = { + it('should return empty array when there is no component props', () => { + props = parse(`const test = { name: 'test', data() { return {} } }`) - assert.notOk(node.type) - assert.notOk(node.node) - assert.equal(node.props.length, 0) + assert.equal(props.length, 0) + }) + + it('should return empty array when component props is empty array', () => { + props = parse(`const test = { + name: 'test', + props: [] + }`) + + assert.equal(props.length, 0) + }) + + it('should return empty array when component props is empty object', () => { + props = parse(`const test = { + name: 'test', + props: {} + }`) + + assert.equal(props.length, 0) }) it('should return computed props', () => { - node = parse(`const test = { + props = parse(`const test = { name: 'test', + ...test, data() { return {} }, @@ -284,32 +301,27 @@ describe('getComputedProperties', () => { } }`) - assert.ok(node.node) - assert.equal(node.type, 'ObjectExpression', 'it detects correct type') - assert.equal(node.props.length, 4, 'it detects all props') + assert.equal(props.length, 4, 'it detects all props') - assert.ok(node.props[0].value) - assert.ok(node.props[0].key.type === 'Identifier') - assert.ok(node.props[0].node.type === 'Property') - assert.ok(node.props[0].value.type === 'Identifier') + assert.ok(props[0].key.type === 'Identifier') + assert.ok(props[0].node.type === 'Property') + assert.ok(props[0].value.type === 'Identifier') - assert.ok(node.props[1].value) - assert.ok(node.props[1].key.type === 'Identifier') - assert.ok(node.props[1].node.type === 'Property') - assert.ok(node.props[1].value.type === 'ObjectExpression') + assert.ok(props[1].key.type === 'Identifier') + assert.ok(props[1].node.type === 'Property') + assert.ok(props[1].value.type === 'ObjectExpression') - assert.ok(node.props[2].value) - assert.ok(node.props[2].key.type === 'Identifier') - assert.ok(node.props[2].node.type === 'Property') - assert.ok(node.props[2].value.type === 'ArrayExpression') + assert.ok(props[2].key.type === 'Identifier') + assert.ok(props[2].node.type === 'Property') + assert.ok(props[2].value.type === 'ArrayExpression') - assert.ok(node.props[3].value) - assert.ok(node.props[3].key.type === node.props[3].value.type) - assert.ok(node.props[3].node.type === 'Property') + assert.deepEqual(props[3].key, props[3].value) + assert.ok(props[3].node.type === 'Property') + assert.ok(props[3].value.type === 'Identifier') }) it('should return computed from array props', () => { - node = parse(`const test = { + props = parse(`const test = { name: 'test', data() { return {} @@ -317,11 +329,22 @@ describe('getComputedProperties', () => { props: ['a', b, \`c\`, null] }`) - assert.ok(node.node) - assert.equal(node.type, 'ArrayExpression', 'it detects correct type') - assert.equal(node.props.length, 1, 'it detects all props') + assert.equal(props.length, 4, 'it detects all props') + + assert.ok(props[0].node.type === 'Literal') + assert.deepEqual(props[0].key, props[0].node) + assert.notOk(props[0].value) + + assert.ok(props[1].node.type === 'Identifier') + assert.notOk(props[1].key) + assert.notOk(props[1].value) + + assert.ok(props[2].node.type === 'TemplateLiteral') + assert.notOk(props[2].key) + assert.notOk(props[2].value) - assert.notOk(node.props[0].value) - assert.ok(node.props[0].key.type === 'Literal') + assert.ok(props[3].node.type === 'Literal') + assert.notOk(props[3].key) + assert.notOk(props[3].value) }) }) From aecef064e8ffe48b7253c68c4dcf896b265a8c1a Mon Sep 17 00:00:00 2001 From: Armano Date: Sat, 10 Nov 2018 21:13:08 +0100 Subject: [PATCH 7/7] Change *** to Unknown prop and normalize variable names --- lib/rules/prop-name-casing.js | 2 +- lib/rules/require-prop-type-constructor.js | 18 +++++++++--------- lib/rules/require-prop-types.js | 2 +- lib/rules/require-valid-default-prop.js | 2 +- lib/utils/index.js | 12 ++++++------ tests/lib/rules/require-prop-types.js | 4 ++-- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/rules/prop-name-casing.js b/lib/rules/prop-name-casing.js index b424664aa..fffbdefff 100644 --- a/lib/rules/prop-name-casing.js +++ b/lib/rules/prop-name-casing.js @@ -37,7 +37,7 @@ function create (context) { return utils.executeOnVue(context, (obj) => { const props = utils.getComponentProps(obj) - .filter(cp => cp.key && cp.key.type === 'Literal' || (cp.key.type === 'Identifier' && !cp.node.computed)) + .filter(prop => prop.key && prop.key.type === 'Literal' || (prop.key.type === 'Identifier' && !prop.node.computed)) for (const item of props) { const propName = item.key.type === 'Literal' ? item.key.value : item.key.name diff --git a/lib/rules/require-prop-type-constructor.js b/lib/rules/require-prop-type-constructor.js index 5e507abc8..b79ecf29f 100644 --- a/lib/rules/require-prop-type-constructor.js +++ b/lib/rules/require-prop-type-constructor.js @@ -71,20 +71,20 @@ module.exports = { return utils.executeOnVueComponent(context, (obj) => { const props = utils.getComponentProps(obj) - .filter(cp => cp.key && cp.value) + .filter(prop => prop.key && prop.value) - for (const p of props) { - if (isForbiddenType(p.value) || p.value.type === 'ArrayExpression') { - checkPropertyNode(p.key, p.value) - } else if (p.value.type === 'ObjectExpression') { - const typeProperty = p.value.properties.find(prop => - prop.type === 'Property' && - prop.key.name === 'type' + for (const prop of props) { + if (isForbiddenType(prop.value) || prop.value.type === 'ArrayExpression') { + checkPropertyNode(prop.key, prop.value) + } else if (prop.value.type === 'ObjectExpression') { + const typeProperty = prop.value.properties.find(property => + property.type === 'Property' && + property.key.name === 'type' ) if (!typeProperty) continue - checkPropertyNode(p.key, utils.unwrapTypes(typeProperty.value)) + checkPropertyNode(prop.key, typeProperty.value) } } }) diff --git a/lib/rules/require-prop-types.js b/lib/rules/require-prop-types.js index a70107ad5..b3b840351 100644 --- a/lib/rules/require-prop-types.js +++ b/lib/rules/require-prop-types.js @@ -59,7 +59,7 @@ module.exports = { node, message: 'Prop "{{name}}" should define at least its type.', data: { - name: utils.getStaticPropertyName(key || node) || '***' + name: utils.getStaticPropertyName(key || node) || 'Unknown prop' } }) } diff --git a/lib/rules/require-valid-default-prop.js b/lib/rules/require-valid-default-prop.js index 7baf34d6e..04db93939 100644 --- a/lib/rules/require-valid-default-prop.js +++ b/lib/rules/require-valid-default-prop.js @@ -89,7 +89,7 @@ module.exports = { return utils.executeOnVue(context, obj => { const props = utils.getComponentProps(obj) - .filter(cp => cp.key && cp.value && cp.value.type === 'ObjectExpression') + .filter(prop => prop.key && prop.value && prop.value.type === 'ObjectExpression') for (const prop of props) { const type = getPropertyNode(prop.value, 'type') diff --git a/lib/utils/index.js b/lib/utils/index.js index 40ec71e92..28caa444d 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -387,15 +387,15 @@ module.exports = { if (propsNode.value.type === 'ObjectExpression') { props = propsNode.value.properties - .filter(cp => cp.type === 'Property') - .map(cp => { - return { key: cp.key, value: this.unwrapTypes(cp.value), node: cp } + .filter(prop => prop.type === 'Property') + .map(prop => { + return { key: prop.key, value: this.unwrapTypes(prop.value), node: prop } }) } else { props = propsNode.value.elements - .map(cp => { - const key = cp.type === 'Literal' && typeof cp.value === 'string' ? cp : null - return { key, value: null, node: cp } + .map(prop => { + const key = prop.type === 'Literal' && typeof prop.value === 'string' ? prop : null + return { key, value: null, node: prop } }) } diff --git a/tests/lib/rules/require-prop-types.js b/tests/lib/rules/require-prop-types.js index f46a36e3f..aac856342 100644 --- a/tests/lib/rules/require-prop-types.js +++ b/tests/lib/rules/require-prop-types.js @@ -181,7 +181,7 @@ ruleTester.run('require-prop-types', rule, { message: 'Prop "baz" should define at least its type.', line: 3 }, { - message: 'Prop "***" should define at least its type.', + message: 'Prop "Unknown prop" should define at least its type.', line: 3 }] }, @@ -203,7 +203,7 @@ ruleTester.run('require-prop-types', rule, { message: 'Prop "baz" should define at least its type.', line: 3 }, { - message: 'Prop "***" should define at least its type.', + message: 'Prop "Unknown prop" should define at least its type.', line: 3 }] },