eslint-plugin-ember
eslint-plugin-ember copied to clipboard
ember-cli
New rule: newlines between sections
I am curious if it is possible to enforce that there is a newline between each section enforced by order-in-components etc. So you would have to have like:
Bad
store: service(),
fooAlias: alias('foo'),
barAlias: alias('bar'),
init(){},
actions: {}
Good
store: service(),
fooAlias: alias('foo'),
barAlias: alias('bar'),
init(){},
actions: {}
Is this possible to enforce?
This is tricky. As @michalsnik suggests, it is not quite straightforward. This rule would have to rely on the order-in-* rules being valid, but I don't think rules should depend on each other like that. We also don't want to put this logic in the order-in-* rules themselves, since that would violate the single responsibility principle.
Would be great to hear some suggestions on how to approach this.
@kevinkucharczyk could it not be an option for the order-in-* rules? I know a lot of ESLint rules do a lot of things if you configure them correctly, but I am unsure where to draw the line on that.
Yeah, adding it as an option should work. I would be fine with it, I think it would be a great "nice-to-have" linter, thought I would keep the option off by default. @rwwagner90 would you like to implement the rule?
So @michalsnik and I had a chat about this rule and we came up with an alternative approach, which we believe is a nice compromise.
First of all, this would be its own rule, so we wouldn't be implementing this as an option do order-in-* after all.
The way it would work would be to report missing newlines between all properties that are in separate sections. So if your properties are ordered into the right sections, you'd only get warnings in the right places - between sections. If your properties are unordered, however, you would get a bunch of newline reports along with the ordering reports. This might be a little annoying, since the newline reports are useless while you are getting order reports, but they will go away once the order is fixed.
To give an example, for properly ordered properties:
store: service(), <-- report: missing empty line after property
fooAlias: alias('foo'),
barAlias: alias('bar'), <-- report: missing empty line after property
init(){},
and for unordered ones:
store: service(), <-- report: missing empty line after property
fooAlias: alias('foo'), <-- report: missing empty line after property
init(){}, <-- report: missing empty line after property
barAlias: alias('bar'), <-- report: barAlias should be above init (and possible newline report, depending on what's below)
Also, since we're only reporting simple newlines, it should be easy enough to also add a fix for the newline linter, since it would only be responsible for adding newlines without caring about the actual section order.
Yeah, plus if we add the fix method we'd have to implement reversed functionality aswell. Because if you try to fix (add) empty lines while your properties are unordered you'll get bunch of unnecessary new lines, that this rule should also remove when are in wrong places.
@kevinkucharczyk I think that sounds fine. My only concern is that it kind of only makes sense to turn this rule on, if you have the order-in-* rules on, and that people wouldn't realize that and would try turning it on anyway, hence the reasoning behind wanting it to be an option to order-in-*. If you guys think that's a non-issue, then I'm totally fine with it 😃
@rwwagner90 you just made me realise that there would still be another problem if we introduce this as a separate rule: what if a developer defines a custom order section array for his order-in-* rule? We would need to have the same array inside our newline rule, since we need to know which sections are considered equal, so that we don't enforce newlines between those properties.
Going back to introducing this rule as an option to order-in-*: it would require a bit of a refactor and I am not entirely convinced the effort would be worth it.. And, of course, if it's just an option to this rule, we wouldn't be able to introduce a nice fix for it.
@kevinkucharczyk ah I did not consider people overriding their orders. I would be okay with there being no fix option for it. I'd rather be able to enforce it, and have people fix it manually, than not be able to enforce it.
As a side note, I would love to be able to enforce alphabetical order per section as well 😄
@rwwagner90 alphabetical order as well? We'll gladly accept a PR, if you're willing to implement those rules ;)
@kevinkucharczyk so should these rules be implemented as options for order-in-*? I'm not sure that they make sense otherwise.
@rwwagner90 I agree, options do seem to be the best approach here, since the newline rule is so closely tied to the validity of the sectioning.