diff --git a/docs/rules/require-prop-types.md b/docs/rules/require-prop-types.md new file mode 100644 index 000000000..4c0c0c465 --- /dev/null +++ b/docs/rules/require-prop-types.md @@ -0,0 +1,42 @@ +# Prop definitions should be detailed (require-prop-types) + +In committed code, prop definitions should always be as detailed as possible, specifying at least type(s). + +## :book: Rule Details + +This rule enforces that a `props` statement contains type definition. + +:-1: Examples of **incorrect** code for this rule: + +```js +export default { + props: ['status'] +} +``` + +:+1: Examples of **correct** code for this rule: + +```js +export default { + props: { + status: String + } +} +``` + +```js +export default { + props: { + status: { + type: String, + required: true, + validate: function (value) { + return ['syncing', 'synced', 'version-conflict', 'error'].indexOf(value) !== -1 + } + } + } +} +``` +## :wrench: Options + +Nothing. diff --git a/lib/rules/name-property-casing.js b/lib/rules/name-property-casing.js index c4136d85d..89744d4bc 100644 --- a/lib/rules/name-property-casing.js +++ b/lib/rules/name-property-casing.js @@ -21,11 +21,11 @@ function create (context) { return utils.executeOnVue(context, (obj) => { const node = obj.properties - .filter(item => ( + .find(item => ( item.type === 'Property' && item.key.name === 'name' && item.value.type === 'Literal' - ))[0] + )) if (!node) return diff --git a/lib/rules/require-prop-types.js b/lib/rules/require-prop-types.js new file mode 100644 index 000000000..d8291cf82 --- /dev/null +++ b/lib/rules/require-prop-types.js @@ -0,0 +1,94 @@ +/** + * @fileoverview Prop definitions should be detailed + * @author Armano + */ +'use strict' + +const utils = require('../utils') + +function create (context) { + // ---------------------------------------------------------------------- + // Helpers + // ---------------------------------------------------------------------- + + function objectHasType (node) { + const typeProperty = node.properties + .find(p => + utils.getStaticPropertyName(p.key) === 'type' && + ( + p.value.type !== 'ArrayExpression' || + p.value.elements.length > 0 + ) + ) + return Boolean(typeProperty) + } + + function checkProperties (items) { + for (const cp of items) { + if (cp.type !== 'Property') { + return + } + let hasType = true + if (cp.value.type === 'ObjectExpression') { // foo: { + hasType = objectHasType(cp.value) + } else if (cp.value.type === 'ArrayExpression') { // foo: [ + hasType = cp.value.elements.length > 0 + } else if (cp.value.type === 'FunctionExpression' || cp.value.type === 'ArrowFunctionExpression') { + hasType = false + } + if (!hasType) { + context.report({ + node: cp, + message: 'Prop "{{name}}" should define at least it\'s type.', + data: { + name: cp.key.name + } + }) + } + } + } + + // ---------------------------------------------------------------------- + // Public + // ---------------------------------------------------------------------- + + return utils.executeOnVue(context, (obj) => { + const node = obj.properties + .find(p => + p.type === 'Property' && + p.key.type === 'Identifier' && + p.key.name === 'props' + ) + + if (!node) return + + if (node.value.type === 'ObjectExpression') { + checkProperties(node.value.properties) + } else { + context.report({ + node, + message: 'Props should at least define their types.' + }) + } + }) +} + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'Prop definitions should be detailed', + category: 'Best Practices', + recommended: false + }, + fixable: null, // or "code" or "whitespace" + schema: [ + // fill in your schema + ] + }, + + create +} diff --git a/lib/utils/index.js b/lib/utils/index.js index 018b9493c..71a024180 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -272,6 +272,48 @@ module.exports = { return members.reverse() }, + /** + * Gets the property name of a given node. + * @param {ASTNode} node - The node to get. + * @return {string|null} The property name if static. Otherwise, null. + */ + getStaticPropertyName (node) { + let prop + switch (node && node.type) { + case 'Property': + case 'MethodDefinition': + prop = node.key + break + case 'MemberExpression': + prop = node.property + break + case 'Literal': + case 'TemplateLiteral': + case 'Identifier': + prop = node + break + // no default + } + + switch (prop && prop.type) { + case 'Literal': + return String(prop.value) + case 'TemplateLiteral': + if (prop.expressions.length === 0 && prop.quasis.length === 1) { + return prop.quasis[0].value.cooked + } + break + case 'Identifier': + if (!node.computed) { + return prop.name + } + break + // no default + } + + return null + }, + /** * Get all computed properties by looking at all component's properties * @param {ObjectExpression} Object with component definition @@ -279,11 +321,11 @@ module.exports = { */ getComputedProperties (componentObject) { const computedPropertiesNode = componentObject.properties - .filter(p => + .find(p => p.key.type === 'Identifier' && p.key.name === 'computed' && p.value.type === 'ObjectExpression' - )[0] + ) if (!computedPropertiesNode) { return [] } diff --git a/tests/lib/rules/require-prop-types.js b/tests/lib/rules/require-prop-types.js new file mode 100644 index 000000000..835d2552e --- /dev/null +++ b/tests/lib/rules/require-prop-types.js @@ -0,0 +1,150 @@ +/** + * @fileoverview Prop definitions should be detailed + * @author Armano + */ +'use strict' + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const rule = require('../../../lib/rules/require-prop-types') + +const RuleTester = require('eslint').RuleTester + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +var ruleTester = new RuleTester() +ruleTester.run('require-prop-types', rule, { + + valid: [ + { + filename: 'test.vue', + code: ` + export default { + props: { + ...test(), + foo: String + } + } + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module', ecmaFeatures: { experimentalObjectRestSpread: true }} + }, + { + filename: 'test.vue', + code: ` + export default { + props: { + foo: [String, Number] + } + } + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module' } + }, + { + filename: 'test.vue', + code: ` + export default { + props: { + foo: { + type: String + } + } + } + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module' } + }, + { + filename: 'test.vue', + code: ` + export default { + props: { + foo: { + ['type']: String + } + } + } + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module' } + } + ], + + invalid: [ + { + filename: 'test.vue', + code: ` + export default { + props: ['foo'] + } + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module' }, + errors: [{ + message: 'Props should at least define their types.', + line: 3 + }] + }, + { + filename: 'test.js', + code: ` + new Vue({ + props: ['foo'] + }) + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module' }, + errors: [{ + message: 'Props should at least define their types.', + line: 3 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + props: { + foo: { + } + } + } + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module' }, + errors: [{ + message: 'Prop "foo" should define at least it\'s type.', + line: 4 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + props: { + foo: { + type: [] + } + } + } + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module' }, + errors: [{ + message: 'Prop "foo" should define at least it\'s type.', + line: 4 + }] + }, + { + filename: 'test.vue', + code: ` + export default { + props: { + foo() {} + } + } + `, + parserOptions: { ecmaVersion: 6, sourceType: 'module' }, + errors: [{ + message: 'Prop "foo" should define at least it\'s type.', + line: 4 + }] + } + ] +}) diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js index 13293fb3a..2189f8960 100644 --- a/tests/lib/utils/index.js +++ b/tests/lib/utils/index.js @@ -121,3 +121,48 @@ describe('getComputedProperties', () => { assert.ok(computedProperties[0].value) }) }) + +describe('getStaticPropertyName', () => { + let node + + const parse = function (code) { + return babelEslint.parse(code).body[0].declarations[0].init + } + + it('should parse property expression with identifier', () => { + node = parse(`const test = { computed: { } }`) + + const parsed = utils.getStaticPropertyName(node.properties[0]) + assert.ok(parsed === 'computed') + }) + it('should parse property expression with literal', () => { + node = parse(`const test = { ['computed'] () {} }`) + + const parsed = utils.getStaticPropertyName(node.properties[0]) + assert.ok(parsed === 'computed') + }) + it('should parse property expression with template literal', () => { + node = parse(`const test = { [\`computed\`] () {} }`) + + const parsed = utils.getStaticPropertyName(node.properties[0]) + assert.ok(parsed === 'computed') + }) + it('should parse identifier', () => { + node = parse(`const test = { computed: { } }`) + + const parsed = utils.getStaticPropertyName(node.properties[0].key) + assert.ok(parsed === 'computed') + }) + it('should parse literal', () => { + node = parse(`const test = { ['computed'] () {} }`) + + const parsed = utils.getStaticPropertyName(node.properties[0].key) + assert.ok(parsed === 'computed') + }) + it('should parse template literal', () => { + node = parse(`const test = { [\`computed\`] () {} }`) + + const parsed = utils.getStaticPropertyName(node.properties[0].key) + assert.ok(parsed === 'computed') + }) +})