Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #616 - update max attrs per line error message, remove white spaces #643

Merged
merged 5 commits into from
Nov 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = {
'eslint-plugin'
],
rules: {
'eslint-plugin/report-message-format': ['error', '^[A-Z`\'].*\\.$'],
'eslint-plugin/report-message-format': ['error', '^[A-Z`\'{].*\\.$'],
'eslint-plugin/prefer-placeholders': 'error',
'eslint-plugin/consistent-output': 'error'
},
Expand Down
48 changes: 39 additions & 9 deletions lib/rules/max-attributes-per-line.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ module.exports = {
const multilineMaximum = configuration.multiline
const singlelinemMaximum = configuration.singleline
const canHaveFirstLine = configuration.allowFirstLine
const template = context.parserServices.getTemplateBodyTokenStore && context.parserServices.getTemplateBodyTokenStore()

return utils.defineTemplateBodyVisitor(context, {
'VStartTag' (node) {
Expand All @@ -78,17 +79,17 @@ module.exports = {
if (!numberOfAttributes) return

if (utils.isSingleLine(node) && numberOfAttributes > singlelinemMaximum) {
showErrors(node.attributes.slice(singlelinemMaximum), node)
showErrors(node.attributes.slice(singlelinemMaximum))
}

if (!utils.isSingleLine(node)) {
if (!canHaveFirstLine && node.attributes[0].loc.start.line === node.loc.start.line) {
showErrors([node.attributes[0]], node)
showErrors([node.attributes[0]])
}

groupAttrsByLine(node.attributes)
.filter(attrs => attrs.length > multilineMaximum)
.forEach(attrs => showErrors(attrs.splice(multilineMaximum), node))
.forEach(attrs => showErrors(attrs.splice(multilineMaximum)))
}
}
})
Expand Down Expand Up @@ -128,16 +129,45 @@ module.exports = {
return defaults
}

function showErrors (attributes, node) {
function getPropData (prop) {
let propType = 'Attribute'
let propName = prop.key.name

if (utils.isBindingAttribute(prop)) {
propType = 'Binding'
propName = prop.key.raw.argument
Copy link
Contributor

@armano2 armano2 Nov 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argument can be null when v-bind="$props" and error message is:

Binding "null" should be on a new line.
Suggested change
propName = prop.key.raw.argument
propName = prop.key.raw.argument || 'object as props'

see. https://vuejs.org/v2/guide/components-props.html#Passing-the-Properties-of-an-Object

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I forgot about this case. I think it should actually report Directive "bind" should be on a new line

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you 👍

} else if (utils.isEventAttribute(prop)) {
propType = 'Event'
propName = prop.key.raw.argument
} else if (prop.directive) {
propType = 'Directive'
}

return { propType, propName }
}

function showErrors (attributes) {
attributes.forEach((prop, i) => {
const fix = (fixer) => {
if (i !== 0) return null

// Find the closest token before the current prop
// that is not a white space
const prevToken = template.getTokenBefore(prop, {
filter: (token) => token.type !== 'HTMLWhitespace'
})

const range = [prevToken.range[1], prop.range[0]]

return fixer.replaceTextRange(range, '\n')
}

context.report({
node: prop,
loc: prop.loc,
message: 'Attribute "{{propName}}" should be on a new line.',
data: {
propName: prop.key.name
},
fix: i === 0 ? (fixer) => fixer.insertTextBefore(prop, '\n') : undefined
message: '{{propType}} "{{propName}}" should be on a new line.',
data: getPropData(prop),
fix
})
})
}
Expand Down
20 changes: 20 additions & 0 deletions lib/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,26 @@ module.exports = {
return VOID_ELEMENT_NAMES.has(name)
},

/**
* Check whether the given attribute node is a binding
* @param {ASTNode} name The attribute to check.
* @returns {boolean}
*/
isBindingAttribute (attribute) {
return attribute.directive &&
attribute.key.name === 'bind' &&
attribute.key.argument
},

/**
* Check whether the given attribute node is an event
* @param {ASTNode} name The attribute to check.
* @returns {boolean}
*/
isEventAttribute (attribute) {
return attribute.directive && attribute.key.name === 'on'
},

/**
* Parse member expression node to get array with all of its parts
* @param {ASTNode} MemberExpression
Expand Down
54 changes: 45 additions & 9 deletions tests/lib/rules/max-attributes-per-line.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,53 @@ ruleTester.run('max-attributes-per-line', rule, {
invalid: [
{
code: `<template><component name="John Doe" age="30"></component></template>`,
output: `<template><component name="John Doe"
output: `<template><component name="John Doe"
age="30"></component></template>`,
errors: ['Attribute "age" should be on a new line.']
},
{
code: `<template><component :name="user.name" :age="user.age"></component></template>`,
output: `<template><component :name="user.name"
:age="user.age"></component></template>`,
errors: ['Binding "age" should be on a new line.']
},
{
code: `<template><component :is="test" v-bind="user"></component></template>`,
output: `<template><component :is="test"
v-bind="user"></component></template>`,
errors: ['Directive "bind" should be on a new line.']
},
{
code: `<template><component :name="user.name" @buy="buyProduct"></component></template>`,
output: `<template><component :name="user.name"
@buy="buyProduct"></component></template>`,
errors: ['Event "buy" should be on a new line.']
},
{
code: `<template><component :name="user.name" @click.stop></component></template>`,
output: `<template><component :name="user.name"
@click.stop></component></template>`,
errors: ['Event "click" should be on a new line.']
},
{
code: `<template><component :name="user.name" v-if="something"></component></template>`,
output: `<template><component :name="user.name"
v-if="something"></component></template>`,
errors: ['Directive "if" should be on a new line.']
},
{
code: `<template><component name="John Doe" v-bind:age="user.age"></component></template>`,
output: `<template><component name="John Doe"
v-bind:age="user.age"></component></template>`,
errors: ['Binding "age" should be on a new line.']
},
{
code: `<template><component job="Vet"
name="John Doe"
age="30">
</component>
</template>`,
output: `<template><component
output: `<template><component
job="Vet"
name="John Doe"
age="30">
Expand All @@ -116,7 +152,7 @@ job="Vet"
{
code: `<template><component name="John Doe" age="30" job="Vet"></component></template>`,
options: [{ singleline: { max: 2 }}],
output: `<template><component name="John Doe" age="30"
output: `<template><component name="John Doe" age="30"
job="Vet"></component></template>`,
errors: [{
message: 'Attribute "job" should be on a new line.',
Expand All @@ -127,7 +163,7 @@ job="Vet"></component></template>`,
{
code: `<template><component name="John Doe" age="30" job="Vet"></component></template>`,
options: [{ singleline: 1, multiline: { max: 1, allowFirstLine: false }}],
output: `<template><component name="John Doe"
output: `<template><component name="John Doe"
age="30" job="Vet"></component></template>`,
errors: [{
message: 'Attribute "age" should be on a new line.',
Expand All @@ -145,7 +181,7 @@ age="30" job="Vet"></component></template>`,
</component>
</template>`,
options: [{ singleline: 3, multiline: { max: 1, allowFirstLine: false }}],
output: `<template><component
output: `<template><component
name="John Doe"
age="30">
</component>
Expand All @@ -164,7 +200,7 @@ name="John Doe"
</template>`,
options: [{ singleline: 3, multiline: { max: 1, allowFirstLine: false }}],
output: `<template><component
name="John Doe"
name="John Doe"
age="30"
job="Vet">
</component>
Expand All @@ -183,7 +219,7 @@ age="30"
</template>`,
options: [{ singleline: 3, multiline: 1 }],
output: `<template><component
name="John Doe"
name="John Doe"
age="30"
job="Vet">
</component>
Expand All @@ -203,7 +239,7 @@ age="30"
options: [{ singleline: 3, multiline: { max: 2, allowFirstLine: false }}],
output: `<template><component
name="John Doe" age="30"
job="Vet" pet="dog"
job="Vet" pet="dog"
petname="Snoopy">
</component>
</template>`,
Expand All @@ -222,7 +258,7 @@ petname="Snoopy">
options: [{ singleline: 3, multiline: { max: 2, allowFirstLine: false }}],
output: `<template><component
name="John Doe" age="30"
job="Vet" pet="dog"
job="Vet" pet="dog"
petname="Snoopy" extra="foo">
</component>
</template>`,
Expand Down