Add option to Metrics/BlockNesting to actually check blocks #3670
Conversation
georgyangelov
force-pushed
the
block-nesting-block-check-option
branch
from
8ab78d7
to
4b7d379
Compare
|
I took a cursory glance at the code and I wonder why are single-line and multi-line blocks treated separately. After all blocks are blocks, despite of their length... |
| def message(max) | ||
| "Avoid more than #{max} levels of block nesting." | ||
| end | ||
|
|
||
| def style_parameter_name |
There was a problem hiding this comment.
Seems odd to be using the style-related code for something that's not the cop's enforced style, but an additional config option. @jonas054 Thoughts?
|
@bbatsov, I agree. However, there may be multiple points of view regarding what is considered a nesting level. One might consider only the indented code as a nesting level (which does not include inline blocks). This is not my view on the point, but I think this will allow for more customizability and may ease the transition. If you think the distinction is not needed, I will leave it out. |
|
Let's not overdo this. I'd suggest treating all blocks in the same manner for starters and see what feedback this change is going to get when merged. |
|
ping @georgyangelov :-) |
These mixins both defined the `parameter_name` method which was being overriden by the other mixin. These two were renamed to `style_parameter_name` and `max_parameter_name`.
georgyangelov
force-pushed
the
block-nesting-block-check-option
branch
from
4b7d379
to
9ca21c0
Compare
|
@bbatsov, sorry for the delay. I've updated the option and rebased the branch on the latest master. I've left the |
|
Thanks! 👍 |
Currently, the
Metrics/BlockNestingcop does not consider blocks to be a nesting level. There are some cases where this is desirable.This PR makes the block nesting cop configurable. The option is called
ConsideredBlocksand can have a value ofnone,multilineandall.noneis the default which does not change the cop's current behavior.multilineconsiders multiline blocks as a nesting level andallcounts both single-line and multiline blocks.While implementing this, I saw that the two mixins
ConfigurableMaxandConfigurableEnforcedStylecannot be used both in a cop. They were both defining aparameter_namemethod and one was overriding the other. I have changed the name of these methods tostyle_parameter_nameandmax_parameter_name.Please verify that my approach in the block detection is correct since this is my first time editing a cop and I'm not entirely familiar with Node's interface.
P.S. I changed my GitHub username and edited that in the contributor list.