New rule: no_condition_parens_rule #30
Conversation
| @@ -263,7 +263,12 @@ abstract class Rule | |||
|
|
|||
| @override | |||
| List<Lint> visitWhileRule(WhileRule node) { | |||
| throw new UnimplementedError(); | |||
| var lint = <Lint>[]; | |||
There was a problem hiding this comment.
You could shorten this to look like, e.g. visitStylesheet.
| rule: this, | ||
| span: clause.expression.span, | ||
| message: | ||
| 'Parentheses around ${clause.expression.span.text} are unnecessary')); |
There was a problem hiding this comment.
To avoid some wrapping here and lots of law-of-demeter, I'd recommend instantiating a variable either for clause.expression (above the if) or clause.expression.span (above the lint.add).
| rule: this, | ||
| span: clause.expression.span, | ||
| message: | ||
| 'Parentheses around ${clause.expression.span.text} are unnecessary')); |
There was a problem hiding this comment.
Please wrap ${clause.expression.span.text} in either single or double quotes. I forget what the convention is in other lints...
| @@ -175,7 +175,7 @@ void main() { | |||
| expect(lint.column, 27); | |||
| }); | |||
|
|
|||
| test('reports lint when boolean is found in a @for condition', () { | |||
| test('reports lint when numbers are found in a @for condition', () { | |||
There was a problem hiding this comment.
Haha yes, thanks for fixing this. You will get some merge conflicts with #29 where I overhaul this file.
|
|
||
| void main() { | ||
| test('does not report lint when there are no parens', () { | ||
| var lints = getLints(r'@if 1 != 7 { @debug("quack"); }'); |
There was a problem hiding this comment.
Great tests! I'd love one more test with an inner expression that contains parens, like:
@if (a + b) * c < 3 { @debug("quack"); }
This resolves issue #15.
Also implemented & tested
WhileRuletraversal.