robotframework-robocop
robotframework-robocop copied to clipboard
[Bug] Rule #1005 ignores comment lines
What happened?
An example code
Keyword1
Keyword2
# Comment
Keyword3
Keyword4
won't pass the rule 1005 (empty-lines-between-keywords). The received output will be as follows:
Invalid number of empty lines between keywords (2/1)
It's down to preferences, but as a RF user I see use cases for having e.g. keyword library section separators (and multiple other use cases) that could be done with single-line comments between keywords.
Operating System
Windows
Robocop version
2.0.2
It could be parametrized - ie "ignore_comments" parameter. Or we could ignore comments with specific patterns (such as starting with triple ###
).
Parameterizing the option to ignore comments could make sense to me, as this syntax could be a yellow flag for some other projects. Introducing a specific pattern (e.g. ###
) is perhaps a bit too much pushing towards a non-existing style guide.
I've started working on this (I started from adding the parameter etc) but I quickly got stuck. I need we need to rethink how the rule is checking the empty lines by default. In your example:
The comments are ignored, so it yields 2 empty lines - I think it's totally unexpected behaviour. Also it does not differentiate between comments that stay have the same indentation as keywords and "standalone" comments:
(3/1 empty lines detected).
As bare minimum it should:
- differentiate between comments that still belongs to the keyword, and those that don't (so 3/1 situation would become 2/1)
- then it should only count empty lines between keyword and first 'standalone' comment (comment starting from the beginning of the line) - 2/1 case would become 1/1
Then we can add extra parameter that would check if empty lines between/after standalone comments are okay too (check_standalone_comments or smth). Example:
Yellow ones should be checked by default, blue ones when using extra parameter/flag.
Thoughs @mnojek ?
We could by default check after/before keyword empty spaces with standalone comments, like here:
(every yellow space should contain only 1/1 empty lines)
but there is case where people are using comments right before keywords, and the default rule would then trip:
Personally I think that this approach is near a somewhat expected behavior.
We could by default check after/before keyword empty spaces with standalone comments, like here:
(every yellow space should contain only 1/1 empty lines)
but there is case where people are using comments right before keywords, and the default rule would then trip:
If the rule allows max 1 empty line before & after for a comment - or in other words is capable of ignoring a surrounding empty line if such exists - then I think it would be both flexible but would still catch unintended double empty lines, whether with comments or not. The complexity kicks in in cases like
Keyword
Step
# comment
Keyword2
...
as for a parser ignoring empty lines around comments it shouldn't either look like this:
Keyword
Step
Keyword2
...
Do you think such a functionality is achievable?
I would say that the rule should not be too complicated and it should not allow more than 1 line between comments. If we ignore comments, users shouldn't be forced to understand what special conditions they need to follow. It should be intuitive. Ignoring comments should ignore all comments and lines around them, but the rule should be violated when there are 2 or more consecutive empty lines.
Example:
This should fail (note 2 consecutive empty lines between comment 3 and comment 4):
Keyword 1
Step
# comment 1
# comment 2
# comment 3
# comment 4
Keyword 2
Step
but this should be fine:
Keyword 1
Step
# comment 1
# comment 2
# comment 3
# comment 4
Keyword 2
Step
WDYT? @bhirsz @antonpaa
Yes, fully agreed that this would be an ideal definition!
@mnojek I agree, this will be intuitive for the users
@antonpaa Issue was fixed and released in Robocop 2.5.0 yesterday - please check and provide the feedback if necessary :)