Skip to content

Commit

Permalink
Fix issues with props (#632)
Browse files Browse the repository at this point in the history 
* 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
* Add unit test for getPropsProperties
* require-default-prop: allow to use shorthand
* 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
  • Loading branch information
@armano2 @michalsnik
armano2 authored and michalsnik committed Nov 10, 2018
1 parent b363379 commit 6032f21
Show file tree
Hide file tree
Showing 12 changed files with 292 additions and 134 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
@@ -1,4 +1,5 @@
.idea
*.iml
/.nyc_output
/coverage
/tests/integrations/*/node_modules
Expand Down
39 changes: 11 additions & 28 deletions lib/rules/prop-name-casing.js
View file
Expand Up @@ -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
Expand All @@ -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 = 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 props = utils.getComponentProps(obj)
.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
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)
Expand Down
46 changes: 24 additions & 22 deletions lib/rules/require-default-prop.js
View file
Expand Up @@ -6,6 +6,16 @@

const utils = require('../utils')

const NATIVE_TYPES = new Set([
'String',
'Number',
'Boolean',
'Function',
'Object',
'Array',
'Symbol'
])

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
Expand All @@ -21,7 +31,7 @@ module.exports = {
schema: []
},

create: function (context) {
create (context) {
// ----------------------------------------------------------------------
// Helpers
// ----------------------------------------------------------------------
Expand All @@ -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' &&
Expand All @@ -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')
Expand All @@ -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)
Expand Down Expand Up @@ -124,28 +133,21 @@ 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.getComponentProps(obj)
.filter(prop => prop.key && prop.value && !prop.node.shorthand)

const propsWithoutDefault = findPropsWithoutDefaultValue(propsNode)
const propsWithoutDefault = findPropsWithoutDefaultValue(props)
const propsToReport = excludeBooleanProps(propsWithoutDefault)

propsToReport.forEach(prop => {
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
}
})
})
}
})
}
}
36 changes: 14 additions & 22 deletions lib/rules/require-prop-type-constructor.js
View file
Expand Up @@ -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 props = utils.getComponentProps(obj)
.filter(prop => prop.key && prop.value)

if (!node) return
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'
)

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(prop.key, typeProperty.value)
}
}
})
}
}
62 changes: 22 additions & 40 deletions lib/rules/require-prop-types.js
View file
Expand Up @@ -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 || node) || 'Unknown prop'
}
})
}
}

Expand All @@ -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 props = utils.getComponentProps(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 prop of props) {
checkProperty(prop.key, prop.value, prop.node)
}
})
}
Expand Down
19 changes: 5 additions & 14 deletions lib/rules/require-valid-default-prop.js
View file
Expand Up @@ -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 props = utils.getComponentProps(obj)
.filter(prop => prop.key && prop.value && prop.value.type === 'ObjectExpression')

for (const prop of properties) {
const type = getPropertyNode(utils.unwrapTypes(prop.value), 'type')
for (const prop of props) {
const type = getPropertyNode(prop.value, 'type')
if (!type) continue

const typeNames = new Set(getTypes(type.value)
Expand All @@ -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)
Expand Down
39 changes: 38 additions & 1 deletion lib/utils/index.js
View file
Expand Up @@ -365,9 +365,46 @@ module.exports = {
return null
},

/**
* Get all props by looking at all component's properties
* @param {ObjectExpression} componentObject Object with component definition
* @return {Array} Array of component props in format: [{key?: String, value?: ASTNode, node: ASTNod}]
*/
getComponentProps (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(prop => prop.type === 'Property')
.map(prop => {
return { key: prop.key, value: this.unwrapTypes(prop.value), node: prop }
})
} else {
props = propsNode.value.elements
.map(prop => {
const key = prop.type === 'Literal' && typeof prop.value === 'string' ? prop : null
return { key, value: null, node: prop }
})
}

return 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) {
Expand Down

0 comments on commit 6032f21

Please sign in to comment.