From 4f1ae0c91b0e8650382984a81a93aac4b9dcfba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20D=C4=85dela?= Date: Fri, 9 Nov 2018 16:55:01 +0100 Subject: [PATCH 1/2] Implement & test WhileRule traversal. --- lib/src/rule.dart | 7 ++++++- test/rule_test.dart | 26 +++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/src/rule.dart b/lib/src/rule.dart index b8369da..769411d 100644 --- a/lib/src/rule.dart +++ b/lib/src/rule.dart @@ -263,7 +263,12 @@ abstract class Rule @override List visitWhileRule(WhileRule node) { - throw new UnimplementedError(); + var lint = []; + lint.addAll(node.condition.accept(this)); + for (var child in node.children) { + lint.addAll(child.accept(this)); + } + return lint; } List _visitInterpolation(Interpolation node) { diff --git a/test/rule_test.dart b/test/rule_test.dart index 3e472cd..ac0520e 100644 --- a/test/rule_test.dart +++ b/test/rule_test.dart @@ -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', () { var lints = getLints(r''' @for $i from 1 through 3 { $a: red; @@ -204,6 +204,30 @@ void main() { expect(lint.column, 14); }); + test('reports lint when boolean is found in a @while condition', () { + var lints = getLints(r''' + @while false { + $a: red; + } + '''); + + expect(lints, hasLength(1)); + expect(lints[0].line, 0); + expect(lints[0].column, 15); + }); + + test('reports lint when boolean is found in a @while body', () { + var lints = getLints(r''' + @while 'hello' != 'world' { + $a: true; + } + '''); + + expect(lints, hasLength(1)); + expect(lints[0].line, 1); + expect(lints[0].column, 14); + }); + test('reports lint when boolean is found in a @function body', () { var lints = getLints(r''' @function grid-width($n) { From f9a1f1c4f215740fa3e7a37a74053d877aa1decd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20D=C4=85dela?= Date: Fri, 9 Nov 2018 16:55:27 +0100 Subject: [PATCH 2/2] New rule: no_condition_parens_rule (#15) --- lib/src/engine.dart | 10 ++-- lib/src/rules/no_condition_parens.dart | 49 +++++++++++++++++ test/rules/no_condition_parens_test.dart | 67 ++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 lib/src/rules/no_condition_parens.dart create mode 100644 test/rules/no_condition_parens_test.dart diff --git a/lib/src/engine.dart b/lib/src/engine.dart index c6bbb5b..fbc162e 100644 --- a/lib/src/engine.dart +++ b/lib/src/engine.dart @@ -9,6 +9,7 @@ import 'linter.dart'; import 'rule.dart'; import 'rules/no_debug.dart'; import 'rules/no_empty_style.dart'; +import 'rules/no_condition_parens.dart'; import 'rules/no_loud_comment.dart'; import 'rules/non_numeric_dimension.dart'; import 'rules/quote_map_keys.dart'; @@ -21,6 +22,7 @@ import 'rules/use_falsey_null.dart'; final allRules = [ new NoDebugRule(), new NoEmptyStyleRule(), + new NoConditionParensRule(), new NoLoudCommentRule(), new NonNumericDimensionRule(), new QuoteMapKeysRule(), @@ -86,7 +88,7 @@ class Engine { } Iterable _scssFilesInDir(String path) => new Directory(path) - .listSync(recursive: true) - .where((entity) => entity is File) - .map((entity) => entity.path) - .where((path) => path.endsWith('.scss')); + .listSync(recursive: true) + .where((entity) => entity is File) + .map((entity) => entity.path) + .where((path) => path.endsWith('.scss')); diff --git a/lib/src/rules/no_condition_parens.dart b/lib/src/rules/no_condition_parens.dart new file mode 100644 index 0000000..a97bc14 --- /dev/null +++ b/lib/src/rules/no_condition_parens.dart @@ -0,0 +1,49 @@ +// Copyright 2018 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +// The sass package's API is not necessarily stable. It is being imported with +// the Sass team's explicit knowledge and approval. See +// https://github.com/sass/dart-sass/issues/236. +import 'package:sass/src/ast/sass.dart'; + +import '../lint.dart'; +import '../rule.dart'; + +/// A lint rule that reports unnecessary parentheses in conditionals. +/// +/// Sometimes users who are familiar with other languages forget that Sass +/// doesn't need parentheses around its conditionals (as in, `@if (...) {`, or +/// `@else if (...) {`, or `@while (...) {`). This rule suggests removing these +/// parentheses. +class NoConditionParensRule extends Rule { + NoConditionParensRule() : super('no_condition_parens_rule'); + + @override + List visitIfRule(IfRule node) { + var lint = []; + for (var clause in node.clauses) { + if (clause.expression is ParenthesizedExpression) { + lint.add(Lint( + rule: this, + span: clause.expression.span, + message: + 'Parentheses around ${clause.expression.span.text} are unnecessary')); + } + } + return lint..addAll(super.visitIfRule(node)); + } + + @override + List visitWhileRule(WhileRule node) { + var lint = []; + if (node.condition is ParenthesizedExpression) { + lint.add(Lint( + rule: this, + span: node.condition.span, + message: + 'Parentheses around ${node.condition.span.text} are unnecessary')); + } + return lint..addAll(super.visitWhileRule(node)); + } +} diff --git a/test/rules/no_condition_parens_test.dart b/test/rules/no_condition_parens_test.dart new file mode 100644 index 0000000..bbbdf97 --- /dev/null +++ b/test/rules/no_condition_parens_test.dart @@ -0,0 +1,67 @@ +// Copyright 2018 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'package:sass_linter/src/rules/no_condition_parens.dart'; +import 'package:sass_linter/src/lint.dart'; +import 'package:sass_linter/src/linter.dart'; +import 'package:test/test.dart'; + +final url = 'a.scss'; +final rule = new NoConditionParensRule(); + +void main() { + test('does not report lint when there are no parens', () { + var lints = getLints(r'@if 1 != 7 { @debug("quack"); }'); + + expect(lints, isEmpty); + }); + + test('reports lint when there is a paren in @if', () { + var lints = getLints(r'@if (1 != 7) { @debug("quack"); }'); + + expect(lints, hasLength(1)); + + var lint = lints.single; + expect(lint.rule, rule); + expect(lint.message, contains('Parentheses')); + expect(lint.message, contains('(1 != 7)')); + expect(lint.message, contains('unnecessary')); + expect(lint.url, new Uri.file(url)); + expect(lint.line, 0); + expect(lint.column, 4); + }); + + test('reports lint when there is a paren in @else if', () { + var lints = getLints(r'@if 1 {} @else if (2 != 3) { @debug("quack"); }'); + + expect(lints, hasLength(1)); + + var lint = lints.single; + expect(lint.rule, rule); + expect(lint.message, contains('Parentheses')); + expect(lint.message, contains('(2 != 3)')); + expect(lint.message, contains('unnecessary')); + expect(lint.url, new Uri.file(url)); + expect(lint.line, 0); + expect(lint.column, 18); + }); + + test('reports lint when there is a paren in @while', () { + var lints = getLints(r'@while (2 == 3) { @debug("quack"); }'); + + expect(lints, hasLength(1)); + + var lint = lints.single; + expect(lint.rule, rule); + expect(lint.message, contains('Parentheses')); + expect(lint.message, contains('(2 == 3)')); + expect(lint.message, contains('unnecessary')); + expect(lint.url, new Uri.file(url)); + expect(lint.line, 0); + expect(lint.column, 18); + }); +} + +List getLints(String source) => + new Linter(source, [rule], url: url).run();