Add rule `max-attributes-per-line` (resolves #47) #60

filipalacerda · 2017-06-29T10:26:27Z

WIP for #47

Adds rule to verify the number of props per line.

Adds documentation.

filipalacerda · 2017-06-29T10:28:05Z

@mysticatea @michalsnik
I opened this PR as a WIP to check the rule definition with you before starting implementing, hope that's ok.

Can you please check https://github.com/vuejs/eslint-plugin-vue/pull/60/files#diff-c73e4e318ae2ee54c86d3c86d4f0434f and see if the rule definition matches what you were expecting or if I need to change it?

Thanks!

michalsnik · 2017-06-29T10:32:15Z

docs/rules/max-attributes-per-line.md

@@ -0,0 +1,88 @@
+# Define the number of attributes allows per line (max-attributes-per-line)
+
+Limits the maximum number of props per line to improve readability


I'd say maximum number of attributes/properties and add . at the end of the sentence.

michalsnik · 2017-06-29T10:33:10Z

docs/rules/max-attributes-per-line.md

+
+## Rule Details
+
+This rule aims to enforce a number of props per line in templates. It checks all the elements in a template and verifies that the number of props per line does not exceed the defined maximum.


Again attributes, unless we only care about components' props. But I think we should care about every attribute. Prop is just another type of property/attribute.

michalsnik · 2017-06-29T10:35:54Z

docs/rules/max-attributes-per-line.md

+## Rule Details
+
+This rule aims to enforce a number of props per line in templates. It checks all the elements in a template and verifies that the number of props per line does not exceed the defined maximum.
+A prop is considered to be in a new line when there is a line break between two props.


Maybe something like: There is configurable number of props that are acceptable in one-line case, as well as how many attributes should be acceptable per line in multi-line case.?

I am a little lost here, should I add it or replace something with it?

Instead of text on line 9.

michalsnik · 2017-06-29T10:41:39Z

docs/rules/max-attributes-per-line.md

+}
+```
+### `singleline`
+Number of maximum number of props when a tag is in a single line.


When the opening tag is in a single line to be precise.

michalsnik · 2017-06-29T10:41:42Z

docs/rules/max-attributes-per-line.md

+
+```js
+
+<component foo="1" bar="3"></component>


I think default should be:

singleline: 3

multiline: 1

Then examples could look like this:
Incorrect:

<component lorem="1" ipsum="2" dolor="3" sit="4"> </component> <component lorem="1" ipsum="2" dolor="3" sit="4" > </component>

Correct:

<component lorem="1" ipsum="2" dolor="3"> </component> <component lorem="1" ipsum="2" dolor="3" sit="4" > </component>

We should probably add additional option for specifying if we accept attribute in first line or not too, maybe something like firstline: 1, that would make it possible to have one property in the first line, or more if specified. WDYT?

Makes sense to me!

filipalacerda · 2017-06-29T10:53:06Z

@michalsnik thank you so much for your feedback 😊 going to update it asap! :)

michalsnik

We're almost there :) Few additional tweaks. Great work so far!

michalsnik · 2017-06-29T11:37:59Z

docs/rules/max-attributes-per-line.md


-The default is one prop per line
+There is predefined number of attributes that are acceptable in one-line case, as well as how many attributes are acceptable per line in multi-line case.


Let's update a little bit more:

There is a configurable number of attributes that are acceptable in one-line case (default 3), as well as how many attributes are acceptable per line in multi-line case (default 1).

And then we can remove the next sentence.

michalsnik · 2017-06-29T11:41:16Z

docs/rules/max-attributes-per-line.md

+### `firstline`
+For muliline declarations, defines if it accepts an attribute in the first line.
+
+The following patterns is considered a warning:


Let's be consistent :)

Example of **incorrect** code for this setting:

michalsnik · 2017-06-29T11:42:25Z

docs/rules/max-attributes-per-line.md

+</component>;
+```
+
+The following pattern is not considered a warning:


Example of **correct** code for this setting:

Fixed them all ✅

michalsnik · 2017-06-29T11:43:29Z

docs/rules/max-attributes-per-line.md

 ### `singleline`
-Number of maximum number of props when a tag is in a single line.
+Number of maximum attributes per line when the opening tag is in a single line.

 The following patterns is considered a warning:


As above:

Example of **incorrect** code for this setting:

michalsnik · 2017-06-29T11:44:05Z

docs/rules/max-attributes-per-line.md

    "singleline": 2,
    "multiline": 1
  }]
 }
 ```
+
+### `firstline`
+For muliline declarations, defines if it accepts an attribute in the first line.


For multi-line declarations, defines how many attributes are acceptable to be put in the first line. (Default 0)

We can discuss default settings later :)

filipalacerda · 2017-07-09T11:35:37Z

@michalsnik can you please review this? Thank you!

mysticatea

Thank you for the contributing!

Looks good to me but I think we can improve this rule in some points. Please see below:

mysticatea · 2017-07-09T15:21:29Z

lib/rules/max-attributes-per-line.js

+      category: 'Stylistic Issues',
+      recommended: false
+    },
+    fixable: 'code',


It should be fixable: null.
fixable: 'code' means that this rule modifies source code with eslint --fix.

🤦‍♀️ forgot about that! Fixed!

mysticatea · 2017-07-09T15:23:41Z

lib/rules/max-attributes-per-line.js

+    const configuration = context.options[0]
+    const multilineMaximum = configuration.multiline
+    const singlelinemMaximum = configuration.singleline
+    const canHaveFirstLine = configuration.firstline !== 0


I think boolean or string option is more proper in this case.
How about the below?

A: Simple boolean option.

{ "singleline": 3, "multiline": 1, "allowFirstLine": false // or true }

B: I think that the first attribute position is a taste for each people. It might be good if people can specify the position. But I'm not sure it's a responsibility of this rule.

{ "singleline": 3, "multiline": 1, "firstAttributeOfMultiline": "newline" // or "beside" }

C: I'm wondering that it's a setting of multiline. Should it locate in the multiline?

{ "singleline": 3, "multiline": {"max": 1, "allowFirstLine": false} }

It is only for multiline, I will go with C, what do you think @mysticatea ?

I'd go with C but still keep it as number, e.g.:

{ "singleline": 3, "multiline": {"max": 1, "maxInFirstLine": 2} }

So it's possible to have two properties in the first line but the rest in new lines in capacity of 1 per line, like here:

<input name="someField" class="input" v-model="message" placeholder="Lorem ipsum" >

What do you think @filipalacerda @mysticatea ?

Ah, I think I get it know @mysticatea. You meant that the value for a singleline option should be considered the max for the first line, when allowFirstLine is true? If that's the case I'm all for it.

@michalsnik This is suggestion which is based on this line of the implementation: const canHaveFirstLine = configuration.firstline !== 0. It uses multiline.max for the first line if allowFirstLine: true.

mysticatea · 2017-07-09T15:25:27Z

lib/rules/max-attributes-per-line.js

+  create: function (context) {
+    const configuration = context.options[0]
+    const multilineMaximum = configuration.multiline
+    const singlelinemMaximum = configuration.singleline


Documents say the options have each default value. But this looks to not have default values.

@mysticatea
I am looking at the docs and I am a little lost here. I thought they were assumed by default, meaning, if no options were provided they would fallback to the ones in the schema.

Should I do something like:
const configuration = context.options[0] || { firstline: 0, singleline: 1, multiline: 1}
?

I think it should be something like:

const configuration = context.options[0] || {} const multilineMaximum = configuration.multiline || 1 const singlelinemMaximum = configuration.singleline || 1

so you can pass only multiline option and still get fallback for singleline

mysticatea · 2017-07-09T15:47:25Z

lib/rules/max-attributes-per-line.js

+          context.report({
+            node,
+            loc: node.loc,
+            message: 'It has more than {{singlelinemMaximum}} attributes per line.',


I think better if this rule reports the violated attributes instead of elements which have the violated attributes.
Because editors can indicate more proper locations with red lines.

Question:

with the default of 3, if it has more than 3, we would trigger an error saying:"attribute(s) x and z should be in a new line."
But if the multiline max is 1 putting those last ones in a new line will trigger another error.

🤔

What do you think we should do?

I have imaged like that logic. (sorry, I have not run that code.)

Or we can simply put:
There are 3 attributes in this line, but the maximum is 2

I would rather go with @michalsnik suggestion, since as a user I think it will help me more to know the exact numbers :)

mysticatea · 2017-07-09T15:49:29Z

tests/lib/rules/max-attributes-per-line.js

+        </component>
+      </template>`,
+      options: [{ singleline: 3, multiline: 2, firstline: 0 }]
+    }


I want to see tests with no options (use default values) as well.

filipalacerda · 2017-07-10T08:52:28Z

Thanks @mysticatea! Will update asap :)

- Adds defaults values - Adds tests for the default values

filipalacerda · 2017-07-13T09:18:06Z

@mysticatea updated some of the things you mentioned but I need your help regarding the single line error: #60 (comment)

And I need you to confirm the defaults part, not sure if it is the right way of doing it 😅

Thanks for your help!

filipalacerda · 2017-07-18T14:05:38Z

@mysticatea @michalsnik all points were addressed :)

Can you please check again?

michalsnik · 2017-07-19T11:16:58Z

tests/lib/rules/max-attributes-per-line.js

@@ -71,7 +71,7 @@ ruleTester.run('max-attributes-per-line', rule, {
  invalid: [
    {
      code: `<template><component name="John Doe" age="30" job="Vet" petname="Snoopy"></component></template>`,
-      errors: ['It has more than 3 attributes per line.']
+      errors: ['There are 4 in this line, but the maximum is 3.']


Please add attributes keyword to the message :) So it says:
There are 4 attributes in this line, but the maximum is 3.

🤦‍♀️

michalsnik · 2017-07-19T11:22:32Z

lib/rules/max-attributes-per-line.js

+  },
+
+  create: function (context) {
+    const configuration = context.options[0] || { singleline: 3, multiline: { max: 1, allowFirstLine: false }}


Please update settings so it's possible to override only part of it without the need to update everything. You can use Object.assign to merge passed settings with default ones:

const configuration = Object.assign({}, context.options[0] || {}, { singleline: 3, multiline: { max: 1, allowFirstLine: false } })

And I'd also consider accepting setting: multiline: 4 as just a number, then we would assume allowFirstLine is false. What do you think?

I think having only one way of setting a rule would make user's lives easier, but I can change it if you don't agree :)

What do you think @michalsnik ?

const configuration = context.options[0] || { singleline: 3, multiline: { max: 1, allowFirstLine: false }}

In that case, if somebody write ["error", {"singleline": 1}] then it's going to throw a TypeError at const multilineMaximum = configuration.multiline.max. I'd like to define parseOptions function:

function parseOptions(options) { const result = { singlelineMax: 3, multilineMax: 1, allowFirstLine: false } if (options) { if (typeof options.singleline === "number") { result.singlelineMax = options.singleline } else if (options.singleline && options.singleline.max) { result.singlelineMax = options.singleline.max } if (typeof options.multiline === "number") { result.multilineMax = options.multiline } else if (options.multiline) { if (options.multiline.max) { result.multilineMax = options.multiline.max } if (options.multiline.allowFirstLine) { result.allowFirstLine = options.multiline.allowFirstLine } } } return result }

mysticatea

I'm sorry for late response.
I think that we can improve this rule in some points.

More consistent for the option schema.
About the default values of options.
The location of warnings.

mysticatea · 2017-07-19T21:05:17Z

lib/rules/max-attributes-per-line.js

+          type: 'number',
+          minimum: 1
+        },
+        'multiline': {


I'd like to change this schema to https://jsonschemalint.com/#/version/draft-06/markup/json?gist=a6a3592eaf8365d069031288f29e5445

That schema allows {singleline: 1, multiline: 3} if somebody don't mind {allowFirstLine: false}. It's simple.

On the other hand, that schema allows {singleline: {max: 1}, multiline: {max: 3}} if somebody want to write as details. It's more consistent than {singleline: 1, multiline: {max: 3}}.

mysticatea · 2017-07-19T21:14:24Z

lib/rules/max-attributes-per-line.js

+  },
+
+  create: function (context) {
+    const configuration = context.options[0] || { singleline: 3, multiline: { max: 1, allowFirstLine: false }}


const configuration = context.options[0] || { singleline: 3, multiline: { max: 1, allowFirstLine: false }}

In that case, if somebody write ["error", {"singleline": 1}] then it's going to throw a TypeError at const multilineMaximum = configuration.multiline.max. I'd like to define parseOptions function:

function parseOptions(options) { const result = { singlelineMax: 3, multilineMax: 1, allowFirstLine: false } if (options) { if (typeof options.singleline === "number") { result.singlelineMax = options.singleline } else if (options.singleline && options.singleline.max) { result.singlelineMax = options.singleline.max } if (typeof options.multiline === "number") { result.multilineMax = options.multiline } else if (options.multiline) { if (options.multiline.max) { result.multilineMax = options.multiline.max } if (options.multiline.allowFirstLine) { result.allowFirstLine = options.multiline.allowFirstLine } } } return result }

mysticatea · 2017-07-19T21:28:04Z

lib/rules/max-attributes-per-line.js

+        if (isSingleLine(node) && numberOfAttributes > singlelinemMaximum) {
+          context.report({
+            node,
+            loc: node.loc,


I still think better if this rule reports the violated attributes instead of elements which have the violated attributes.
This is consistent with the core rules which have similar functionally.

/*eslint max-statements-per-line: error, newline-per-chained-call: error, object-property-newline: error, array-element-newline: error */ aaa(); bbb(); ccc(); test.aaa().bbb().ccc().ddd() var obj = {aaa:1, bbb:2, ccc:3} var ary = [1, 2, 3]

filipalacerda · 2017-07-20T17:45:30Z

@michalsnik @mysticatea I am little lost 😅 , can you confirm this is what you are suggesting:

Update config to accept:

{
  singleline: 1,
  multiline: { max: 3, allowFirstLine: false}
}

and

{
  singleline: 1,
  multiline: 3
}

In the latter, assumes the default false for allowFirstLine

Show which attributes are wrong in the single line case
Update defaults to allow for only one of them being provided

Is this it?
Thanks!

mysticatea · 2017-07-20T19:20:46Z

@filipalacerda

Yes. I hope that the codes in my comments help you.
I think that this rule should report attributes instead of elements at all. Like this.
Yes.

filipalacerda · 2017-07-24T08:25:42Z

@mysticatea @michalsnik

I need help :) how do I set the schema to accept 2 different configs for multiline? 🤔

schema: [
  'multiline': {
    type: // can be an object or a number
   }
]

Should I not set a schema? I do not see it in most of the other rules 🤔

filipalacerda · 2017-07-24T08:29:25Z

found it! ignore me! 🙈

Changes options parser to allow to redifine only one prop Changes error thrown for singleline errors Updates tests and docs accordingly

filipalacerda · 2017-07-24T13:11:38Z

@mysticatea @michalsnik all fixed :) Can you please check again? Thanks!

armano2 · 2017-07-24T14:15:37Z

@filipalacerda i like this but i see one issue here you are allways reporting VStartTag instead of VAttribute

    message: 'Attribute age should be on a new line.',
    line: 1,
    column: 11,
    nodeType: 'VStartTag',

i will love to have accurate reports for editors to match this:

diff --git a/tests/lib/rules/max-attributes-per-line.js b/tests/lib/rules/max-attributes-per-line.js
index ab98db5..f3a65b9 100644
--- a/tests/lib/rules/max-attributes-per-line.js
+++ b/tests/lib/rules/max-attributes-per-line.js
@@ -83,7 +83,11 @@ ruleTester.run('max-attributes-per-line', rule, {
   invalid: [
     {
       code: `<template><component name="John Doe" age="30" job="Vet" petname="Snoopy"></component></template>`,
-      errors: ['Attribute petname should be on a new line.']
+      errors: [{
+        message: 'Attribute petname should be on a new line.',
+        type: 'VAttribute',
+        line: 1
+      }]
     },
     {
       code: `<template><component job="Vet"
@@ -91,12 +95,24 @@ ruleTester.run('max-attributes-per-line', rule, {
         age="30">
         </component>
       </template>`,
-      errors: ['Attribute job should be on a new line.']
+      errors: [{
+        message: 'Attribute job should be on a new line.',
+        type: 'VAttribute',
+        line: 1
+      }]
     },
     {
       code: `<template><component name="John Doe" age="30" job="Vet"></component></template>`,
       options: [{ singleline: 1, multiline: { max: 1, allowFirstLine: false }}],
-      errors: ['Attribute age should be on a new line.', 'Attribute job should be on a new line.']
+      errors: [{
+        message: 'Attribute age should be on a new line.',
+        type: 'VAttribute',
+        line: 1
+      }, {
+        message: 'Attribute job should be on a new line.',
+        type: 'VAttribute',
+        line: 1
+      }]
     },
     {
       code: `<template><component name="John Doe"
@@ -104,7 +120,11 @@ ruleTester.run('max-attributes-per-line', rule, {
         </component>
       </template>`,
       options: [{ singleline: 3, multiline: { max: 1, allowFirstLine: false }}],
-      errors: ['Attribute name should be on a new line.']
+      errors: [{
+        message: 'Attribute name should be on a new line.',
+        type: 'VAttribute',
+        line: 1
+      }]
     },
     {
       code: `<template><component
@@ -113,7 +133,11 @@ ruleTester.run('max-attributes-per-line', rule, {
         </component>
       </template>`,
       options: [{ singleline: 3, multiline: { max: 1, allowFirstLine: false }}],
-      errors: ['Attribute age should be on a new line.']
+      errors: [{
+        message: 'Attribute age should be on a new line.',
+        type: 'VAttribute',
+        line: 2
+      }]
     },
     {
       code: `<template><component
@@ -122,7 +146,11 @@ ruleTester.run('max-attributes-per-line', rule, {
         </component>
       </template>`,
       options: [{ singleline: 3, multiline: { max: 2, allowFirstLine: false }}],
-      errors: ['Attribute petname should be on a new line.']
+      errors: [{
+        message: 'Attribute petname should be on a new line.',
+        type: 'VAttribute',
+        line: 3
+      }]
     },
     {
       code: `<template><component
@@ -131,10 +159,15 @@ ruleTester.run('max-attributes-per-line', rule, {
         </component>
       </template>`,
       options: [{ singleline: 3, multiline: { max: 2, allowFirstLine: false }}],
-      errors: [
-        'Attribute petname should be on a new line.',
-        'Attribute extra should be on a new line.'
-      ]
+      errors: [{
+        message: 'Attribute petname should be on a new line.',
+        type: 'VAttribute',
+        line: 3
+      }, {
+        message: 'Attribute extra should be on a new line.',
+        type: 'VAttribute',
+        line: 3
+      }]
     }
   ]
 })

michalsnik

Great work @filipalacerda ! 💪

I added few last tiny suggestions and it's all from my side :)

michalsnik · 2017-07-24T22:20:24Z

lib/rules/max-attributes-per-line.js

+
+          const propsPerLine = [[node.attributes[0]]]
+
+          node.attributes.reduce(function (previous, current) {


Use arrow function

I'd create groupAttrsByLine or collectAttributes function that would take node.attributes as parameter and return 2d array, using such function in this place would make this code more readable:

const attrsGroupedByLine = groupAttrsByLine(node.attributes); attrsGroupedByLine .filter(attrs => attrs.length > multilineMaximum) .forEach(attrs => showErrors(attrs.splice(multilineMaximum), node)

michalsnik · 2017-07-24T22:26:10Z

lib/rules/max-attributes-per-line.js

+    // ----------------------------------------------------------------------
+    // Helpers
+    // ----------------------------------------------------------------------
+    function isSingleLine (node) {


Consider moving this function to utils

michalsnik · 2017-07-24T22:26:49Z

lib/rules/max-attributes-per-line.js

+        context.report({
+          node,
+          loc: node.loc,
+          message: 'Attribute {{propName}} should be on a new line.',


Add "" around propName, it will make there errors even more readable :)

michalsnik · 2017-07-24T22:29:20Z

lib/rules/max-attributes-per-line.js

+            return current
+          })
+
+          propsPerLine.forEach(function (attributes) {


You can use arrow function, you don't need the context here

Uses arrow function Improves readability

armano2 · 2017-07-30T15:40:30Z

lib/utils/index.js

+      /**
+     * Check whether the component is declared in a single line or not.
+     * @param {ASTNode} node
+     * @returns {boolean


you are missing here }

filipalacerda · 2017-07-30T15:50:48Z

@michalsnik all done :) Can you please check again?

@armano2 I opted by this report since it matches the error style thrown in the other rules. (At least at the time I opened this MR) I would rather keep it, if you do not mind :)

armano2 · 2017-07-30T15:53:02Z

docs/rules/max-attributes-per-line.md

+
+Examples of **correct** code for this rule:
+
+```js


armano2 · 2017-07-30T15:53:27Z

docs/rules/max-attributes-per-line.md

+
+Examples of **incorrect** code for this rule:
+
+```js


armano2 · 2017-07-30T15:57:03Z

docs/rules/max-attributes-per-line.md

+Limits the maximum number of attributes/properties per line to improve readability.
+
+
+## Rule Details


## :book: Rule Details

armano2 · 2017-07-30T15:57:19Z

docs/rules/max-attributes-per-line.md

+
+There is a configurable number of attributes that are acceptable in one-line case (default 3), as well as how many attributes are acceptable per line in multi-line case (default 1).
+
+Examples of **incorrect** code for this rule:


:-1: Examples of **incorrect** code for this rule:

armano2 · 2017-07-30T15:57:36Z

docs/rules/max-attributes-per-line.md

+</component>
+```
+
+Examples of **correct** code for this rule:


:+1: Examples of **correct** code for this rule:

armano2 · 2017-07-30T15:59:08Z

docs/rules/max-attributes-per-line.md

+### `allowFirstLine`
+For multi-line declarations, defines if allows attributes to be put in the first line. (Default false)
+
+Example of **incorrect** code for this setting:


:-1: Examples of **incorrect** code for this rule:

armano2 · 2017-07-30T15:59:14Z

docs/rules/max-attributes-per-line.md

+### `multiline`
+Number of maximum attributes per line when a tag is in multiple lines. (Default is 1)
+
+Example of **incorrect** code for this setting:


:-1: Examples of **incorrect** code for this rule:

armano2 · 2017-07-30T15:59:29Z

docs/rules/max-attributes-per-line.md

+</component>;
+```
+
+Example of **correct** code for this setting:


:+1: Examples of **correct** code for this rule:

armano2 · 2017-07-30T16:01:03Z

docs/rules/max-attributes-per-line.md

+}
+```
+
+### `allowFirstLine`


it should be 4th lvl header because its part of Options

armano2 · 2017-07-30T16:02:25Z

docs/rules/max-attributes-per-line.md

+
+```
+
+### Options


## :wrench: Options

armano2 · 2017-07-30T18:31:52Z

@filipalacerda i added few comments about documentation

* master: Add rule `vue/require-valid-default-prop`. (vuejs#119) 3.10.0 Update readme to 3.10.0 Chore: remove package-lock.json (vuejs#128) Fix: parserService must exist always (fixes vuejs#125) (vuejs#127) Add rule `require-render-return`. (vuejs#114) 3.9.0 Update package-lock Update: options for `no-duplicate-attributes` (fixes vuejs#112)(vuejs#113) New: add rule `attribute-hyphenation`. (fixes vuejs#92)(vuejs#95) Add namespace check of svg & mathML instead of tag names (vuejs#120) ⚠️ Add support for deprecated state in update-rules ⚠️ (vuejs#121) Add rules: `no-dupe-keys` and `no-reserved-keys`. (vuejs#88) Chore: Improve tests for name-property-casing & improve documentation (vuejs#115) New: add `require-prop-types` rule (fixes vuejs#19)(vuejs#85) Update: upgrade vue-eslint-parser (fixes vuejs#36, fixes vuejs#56, fixes vuejs#96) (vuejs#116)

filipalacerda · 2017-08-05T13:30:57Z

@michalsnik @mysticatea @armano2

I have made all the changes you requested, do you think this is ready to go?

mysticatea · 2017-08-05T20:51:01Z

lib/rules/max-attributes-per-line.js

+      attributes.forEach((prop) => {
+        context.report({
+          node,
+          loc: node.loc,


I still think better if this rule reports the violated attributes instead of elements which have the violated attributes.

@filipalacerda can you look on it?

I will try, but probably only next weekend! If any of you wants/is able to take over please feel free to do so, I can give you access to the fork 😊

47 props per line

mysticatea

Thank you for the amazing work!!

LGTM.

mysticatea · 2017-08-14T05:21:42Z

@michalsnik Could you re-review and merge?

michalsnik

LGTM 🚀 Thanks @filipalacerda for great work! :)

Inits new rule.

a7d9313

Adds documentation.

michalsnik reviewed Jun 29, 2017

View reviewed changes

michalsnik added the work in progress label Jun 29, 2017

michalsnik changed the title ~~WIP: Add rule to verify number of props per line~~ Add rule max-attributes-per-line (resolves #47) Jun 29, 2017

Improves rule definition

7ef1ca5

michalsnik reviewed Jun 29, 2017

View reviewed changes

Filipa Lacerda added 2 commits June 30, 2017 19:08

Update docs to match review

6a62bdd

Adds rule to check for number of attributes per line

1d34713

mysticatea suggested changes Jul 9, 2017

View reviewed changes

Updates per code review:

c78b20d

- Adds defaults values - Adds tests for the default values

Updates single line max attributes error as per code review

56f5f70

michalsnik suggested changes Jul 19, 2017

View reviewed changes

michalsnik reviewed Jul 19, 2017

View reviewed changes

mysticatea suggested changes Jul 19, 2017

View reviewed changes

michalsnik added the semver-minor label Jul 21, 2017

Changes schema to allow a different config

ca31f09

Changes options parser to allow to redifine only one prop Changes error thrown for singleline errors Updates tests and docs accordingly

michalsnik reviewed Jul 24, 2017

View reviewed changes

Moves function into utils

ed73ad8

Uses arrow function Improves readability

filipalacerda and others added 2 commits July 30, 2017 16:32

Merge branch 'master' into 47-props-per-line

df1207b

Fix eslint error

39d0743

armano2 reviewed Jul 30, 2017

View reviewed changes

Improves code coverage

796611e

armano2 suggested changes Jul 30, 2017

View reviewed changes

Filipa Lacerda added 2 commits August 5, 2017 14:22

Update docs as per code review

982d9f2

mysticatea reviewed Aug 5, 2017

View reviewed changes

armano2 and others added 3 commits August 9, 2017 21:33

Merge branch 'master' into 47-props-per-line

5539686

reports the violated attributes instead of elements

dcb3ee3

Merge pull request #1 from armano2/47-props-per-line

dc64c62

47 props per line

armano2 approved these changes Aug 9, 2017

View reviewed changes

mysticatea approved these changes Aug 10, 2017

View reviewed changes

michalsnik approved these changes Aug 15, 2017

View reviewed changes

michalsnik merged commit 85558dc into vuejs:master Aug 15, 2017

michalsnik removed the work in progress label Aug 15, 2017

		@@ -0,0 +1,88 @@
		# Define the number of attributes allows per line (max-attributes-per-line)

		Limits the maximum number of props per line to improve readability


		## Rule Details

		This rule aims to enforce a number of props per line in templates. It checks all the elements in a template and verifies that the number of props per line does not exceed the defined maximum.


		The default is one prop per line
		There is predefined number of attributes that are acceptable in one-line case, as well as how many attributes are acceptable per line in multi-line case.


		const propsPerLine = [[node.attributes[0]]]

		node.attributes.reduce(function (previous, current) {

		Limits the maximum number of attributes/properties per line to improve readability.


		## Rule Details


		There is a configurable number of attributes that are acceptable in one-line case (default 3), as well as how many attributes are acceptable per line in multi-line case (default 1).

		Examples of incorrect code for this rule:

Add rule max-attributes-per-line (resolves #47) #60

Add rule max-attributes-per-line (resolves #47) #60

Conversation

filipalacerda commented Jun 29, 2017

filipalacerda commented Jun 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalsnik Jun 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filipalacerda commented Jun 29, 2017

michalsnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filipalacerda commented Jul 9, 2017

mysticatea left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filipalacerda Jul 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalsnik Jul 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filipalacerda Jul 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filipalacerda commented Jul 10, 2017

filipalacerda commented Jul 13, 2017

filipalacerda commented Jul 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filipalacerda Jul 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mysticatea left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filipalacerda commented Jul 20, 2017

mysticatea commented Jul 20, 2017

filipalacerda commented Jul 24, 2017

filipalacerda commented Jul 24, 2017

filipalacerda commented Jul 24, 2017

armano2 commented Jul 24, 2017

michalsnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

filipalacerda commented Jul 30, 2017

Add rule `max-attributes-per-line` (resolves #47) #60

Add rule `max-attributes-per-line` (resolves #47) #60

michalsnik Jun 29, 2017 •

edited

Loading

filipalacerda Jul 13, 2017 •

edited

Loading

michalsnik Jul 16, 2017 •

edited

Loading

filipalacerda Jul 13, 2017 •

edited

Loading

filipalacerda Jul 19, 2017 •

edited

Loading

armano2 Jul 30, 2017 •

edited

Loading

armano2 Jul 30, 2017 •

edited

Loading