From 47a4f7934a3ea20997ab8f512f2abfb59f5a78d4 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 26 Jul 2019 14:21:51 -0700 Subject: [PATCH 1/8] added new rule --- .../README.md | 41 +++++++++++++++ .../__tests__/index.js | 52 +++++++++++++++++++ .../index.js | 13 +++++ src/rules/index.js | 2 + 4 files changed, 108 insertions(+) create mode 100644 src/rules/at-conditional-rule-no-parentheses/README.md create mode 100644 src/rules/at-conditional-rule-no-parentheses/__tests__/index.js create mode 100644 src/rules/at-conditional-rule-no-parentheses/index.js diff --git a/src/rules/at-conditional-rule-no-parentheses/README.md b/src/rules/at-conditional-rule-no-parentheses/README.md new file mode 100644 index 00000000..9daa297a --- /dev/null +++ b/src/rules/at-conditional-rule-no-parentheses/README.md @@ -0,0 +1,41 @@ +# at-conditional-rule-no-parentheses + +Disallow parentheses in conditional @ rules (if, elsif, while) + +```css + @if (true) {} +/** ↑ ↑ + * Get rid of parentheses like this. */ +``` + +## Options + +### `true` + +The following patterns are considered warnings: + +```scss +@if(true) +``` + +```scss +@else if(true) +``` + +```scss +@while(true) +``` + +The following patterns are *not* considered warnings: + +```scss +@if true +``` + +```scss +@else if true +``` + +```scss +@while true +``` diff --git a/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js b/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js new file mode 100644 index 00000000..f8adae88 --- /dev/null +++ b/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js @@ -0,0 +1,52 @@ +import rule, { ruleName, messages } from ".."; + +testRule(rule, { + ruleName, + config: [true], + syntax: "scss", + + accept: [ + { + code: "@if true {}", + description: "accepts @if with no parentheses" + }, + { + code: "@while true {}", + description: "accepts @while with no parentheses" + }, + { + code: `@if true {} + @else if true {}`, + description: "accepts @else if with no parentheses" + } + ], + + reject: [ + { + code: "@if(true) {}", + fixed: "@if true {}", + message: messages.rejected, + description: "does not accept @if with parentheses" + }, + { + code: "@while true {}", + fixed: "@while true {}", + message: messages.rejected, + description: "does not accept @while with parentheses" + }, + { + code: `@if true {} + @else if(true){}`, + fixed: `@if true {} + @else if true {}`, + message: messages.rejected, + description: "does not accept @else if with parentheses" + }, + { + code: "@if (true) {}", + fixed: "@if true {}", + message: messages.rejected, + description: "doesn't allow parentheses even if there's spaces" + } + ] +}); diff --git a/src/rules/at-conditional-rule-no-parentheses/index.js b/src/rules/at-conditional-rule-no-parentheses/index.js new file mode 100644 index 00000000..c1c0d076 --- /dev/null +++ b/src/rules/at-conditional-rule-no-parentheses/index.js @@ -0,0 +1,13 @@ +import { isRegExp, isString } from "lodash"; +import { rules, utils } from "stylelint"; +import { namespace } from "../../utils"; + +export const ruleName = namespace("at-conditional-rule-no-parentheses"); + +export const messages = utils.ruleMessages(ruleName, { + rejected: "Do not use () to surround statements for @-rules" +}); + +export default function(primaryOption, secondaryOptions) { + return (root, result) => {}; +} diff --git a/src/rules/index.js b/src/rules/index.js index 5d264e1c..afd07def 100644 --- a/src/rules/index.js +++ b/src/rules/index.js @@ -1,3 +1,4 @@ +import atConditionalRuleNoParen from "./at-conditional-rule-no-parentheses"; import atExtendNoMissingPlaceholder from "./at-extend-no-missing-placeholder"; import atElseClosingBraceNewlineAfter from "./at-else-closing-brace-newline-after"; import atElseClosingBraceSpaceAfter from "./at-else-closing-brace-space-after"; @@ -47,6 +48,7 @@ import selectorNoRedundantNestingSelector from "./selector-no-redundant-nesting- import selectorNoUnionClassName from "./selector-no-union-class-name"; export default { + "at-conditional-rule-no-parentheses": atConditionalRuleNoParen, "at-extend-no-missing-placeholder": atExtendNoMissingPlaceholder, "at-else-closing-brace-newline-after": atElseClosingBraceNewlineAfter, "at-else-closing-brace-space-after": atElseClosingBraceSpaceAfter, From c24d4a33ede3d37a30c1573b54d4661bcf47ab04 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 26 Jul 2019 14:31:20 -0700 Subject: [PATCH 2/8] time to test --- .../index.js | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/src/rules/at-conditional-rule-no-parentheses/index.js b/src/rules/at-conditional-rule-no-parentheses/index.js index c1c0d076..5a13ff4f 100644 --- a/src/rules/at-conditional-rule-no-parentheses/index.js +++ b/src/rules/at-conditional-rule-no-parentheses/index.js @@ -1,5 +1,4 @@ -import { isRegExp, isString } from "lodash"; -import { rules, utils } from "stylelint"; +import { utils } from "stylelint"; import { namespace } from "../../utils"; export const ruleName = namespace("at-conditional-rule-no-parentheses"); @@ -8,6 +7,32 @@ export const messages = utils.ruleMessages(ruleName, { rejected: "Do not use () to surround statements for @-rules" }); -export default function(primaryOption, secondaryOptions) { - return (root, result) => {}; +const conditional_rules = ["if", "while", "else if"]; + +export default function(primary) { + return (root, result) => { + const validOptions = utils.validateOptions(result, ruleName, { + actual: primary + }); + + if (!validOptions) { + return; + } + + root.walkAtRules(atrule => { + // Check if this is a conditional rule. + if (!conditional_rules.includes(atrule.name)) { + return; + } + + if (atrule.params.match(/ ?\(.*\) ?/)) { + utils.report({ + message: messages.rejected, + node: atrule, + result, + ruleName + }); + } + }); + }; } From 9081d72078ece7ffd3bed8d43ccb168ac6bd7297 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 26 Jul 2019 14:46:39 -0700 Subject: [PATCH 3/8] rule and tests work --- .../README.md | 2 ++ .../__tests__/index.js | 2 +- .../index.js | 30 ++++++++++++++----- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/rules/at-conditional-rule-no-parentheses/README.md b/src/rules/at-conditional-rule-no-parentheses/README.md index 9daa297a..b9b4af6c 100644 --- a/src/rules/at-conditional-rule-no-parentheses/README.md +++ b/src/rules/at-conditional-rule-no-parentheses/README.md @@ -8,6 +8,8 @@ Disallow parentheses in conditional @ rules (if, elsif, while) * Get rid of parentheses like this. */ ``` + + ## Options ### `true` diff --git a/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js b/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js index f8adae88..308ea1f0 100644 --- a/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js +++ b/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js @@ -29,7 +29,7 @@ testRule(rule, { description: "does not accept @if with parentheses" }, { - code: "@while true {}", + code: "@while(true){}", fixed: "@while true {}", message: messages.rejected, description: "does not accept @while with parentheses" diff --git a/src/rules/at-conditional-rule-no-parentheses/index.js b/src/rules/at-conditional-rule-no-parentheses/index.js index 5a13ff4f..c1e52b3a 100644 --- a/src/rules/at-conditional-rule-no-parentheses/index.js +++ b/src/rules/at-conditional-rule-no-parentheses/index.js @@ -7,7 +7,17 @@ export const messages = utils.ruleMessages(ruleName, { rejected: "Do not use () to surround statements for @-rules" }); -const conditional_rules = ["if", "while", "else if"]; +// postcss picks up else-if as else. +const conditional_rules = ["if", "while", "else"]; + +function report(atrule, result) { + utils.report({ + message: messages.rejected, + node: atrule, + result, + ruleName + }); +} export default function(primary) { return (root, result) => { @@ -25,13 +35,17 @@ export default function(primary) { return; } - if (atrule.params.match(/ ?\(.*\) ?/)) { - utils.report({ - message: messages.rejected, - node: atrule, - result, - ruleName - }); + // Else uses a different regex + // params are of format "`if (cond)` or `if cond` + // instead of `(cond)` or `cond`" + if (atrule.name === "else") { + if (atrule.params.match(/ ?if ?\(.*\) ?/)) { + report(atrule, result); + } + } else { + if (atrule.params.match(/ ?\(.*\) ?/)) { + report(atrule, result); + } } }); }; From fd044dee7a2dcb7b678eb5fddb4f6b402a20ce69 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Mon, 5 Aug 2019 13:06:30 -0700 Subject: [PATCH 4/8] tests pass --- .../__tests__/index.js | 17 +++++++------- .../index.js | 23 ++++++++++++++++--- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js b/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js index 308ea1f0..d7af9c8b 100644 --- a/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js +++ b/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js @@ -4,6 +4,7 @@ testRule(rule, { ruleName, config: [true], syntax: "scss", + fix: true, accept: [ { @@ -18,6 +19,12 @@ testRule(rule, { code: `@if true {} @else if true {}`, description: "accepts @else if with no parentheses" + }, + { + code: `@if true {} + @else if true {} + @else {}`, + description: "accepts @else unconditionally" } ], @@ -29,24 +36,18 @@ testRule(rule, { description: "does not accept @if with parentheses" }, { - code: "@while(true){}", + code: "@while(true) {}", fixed: "@while true {}", message: messages.rejected, description: "does not accept @while with parentheses" }, { code: `@if true {} - @else if(true){}`, + @else if(true) {}`, fixed: `@if true {} @else if true {}`, message: messages.rejected, description: "does not accept @else if with parentheses" - }, - { - code: "@if (true) {}", - fixed: "@if true {}", - message: messages.rejected, - description: "doesn't allow parentheses even if there's spaces" } ] }); diff --git a/src/rules/at-conditional-rule-no-parentheses/index.js b/src/rules/at-conditional-rule-no-parentheses/index.js index c1e52b3a..acba340e 100644 --- a/src/rules/at-conditional-rule-no-parentheses/index.js +++ b/src/rules/at-conditional-rule-no-parentheses/index.js @@ -19,7 +19,16 @@ function report(atrule, result) { }); } -export default function(primary) { +function fix(atrule) { + const regex = /(if)? ?\((.*)\)/; + + // 2 regex groups: 'if ' and cond. + const groups = atrule.params.match(regex).slice(1); + + atrule.params = Array.from(new Set(groups)).join(" "); +} + +export default function(primary, _, context) { return (root, result) => { const validOptions = utils.validateOptions(result, ruleName, { actual: primary @@ -40,11 +49,19 @@ export default function(primary) { // instead of `(cond)` or `cond`" if (atrule.name === "else") { if (atrule.params.match(/ ?if ?\(.*\) ?/)) { - report(atrule, result); + if (context.fix) { + fix(atrule); + } else { + report(atrule, result); + } } } else { if (atrule.params.match(/ ?\(.*\) ?/)) { - report(atrule, result); + if (context.fix) { + fix(atrule); + } else { + report(atrule, result); + } } } }); From 1713d534d21e6f0fbd7bd3038796216ab051e426 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Mon, 12 Aug 2019 10:44:10 -0700 Subject: [PATCH 5/8] PR comments --- src/rules/at-conditional-rule-no-parentheses/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rules/at-conditional-rule-no-parentheses/index.js b/src/rules/at-conditional-rule-no-parentheses/index.js index acba340e..1960f947 100644 --- a/src/rules/at-conditional-rule-no-parentheses/index.js +++ b/src/rules/at-conditional-rule-no-parentheses/index.js @@ -1,10 +1,11 @@ import { utils } from "stylelint"; import { namespace } from "../../utils"; +import _ from "lodash"; export const ruleName = namespace("at-conditional-rule-no-parentheses"); export const messages = utils.ruleMessages(ruleName, { - rejected: "Do not use () to surround statements for @-rules" + rejected: "Unexpected () used to surround statements for @-rules" }); // postcss picks up else-if as else. @@ -25,7 +26,7 @@ function fix(atrule) { // 2 regex groups: 'if ' and cond. const groups = atrule.params.match(regex).slice(1); - atrule.params = Array.from(new Set(groups)).join(" "); + atrule.params = _.uniq(groups).join(" "); } export default function(primary, _, context) { From 6a24e63d6792ed448820e50a0fb0a73a78ad62f0 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Mon, 12 Aug 2019 11:01:38 -0700 Subject: [PATCH 6/8] added one more test --- .../at-conditional-rule-no-parentheses/__tests__/index.js | 4 ++++ src/rules/at-conditional-rule-no-parentheses/index.js | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js b/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js index d7af9c8b..2a5d7724 100644 --- a/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js +++ b/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js @@ -11,6 +11,10 @@ testRule(rule, { code: "@if true {}", description: "accepts @if with no parentheses" }, + { + code: "@if (1 + 1) == 2 {}", + description: "accepts parentheses statement where () used for math" + }, { code: "@while true {}", description: "accepts @while with no parentheses" diff --git a/src/rules/at-conditional-rule-no-parentheses/index.js b/src/rules/at-conditional-rule-no-parentheses/index.js index 1960f947..8284ceb3 100644 --- a/src/rules/at-conditional-rule-no-parentheses/index.js +++ b/src/rules/at-conditional-rule-no-parentheses/index.js @@ -49,7 +49,7 @@ export default function(primary, _, context) { // params are of format "`if (cond)` or `if cond` // instead of `(cond)` or `cond`" if (atrule.name === "else") { - if (atrule.params.match(/ ?if ?\(.*\) ?/)) { + if (atrule.params.match(/ ?if ?\(.*\) ?$/)) { if (context.fix) { fix(atrule); } else { @@ -57,7 +57,7 @@ export default function(primary, _, context) { } } } else { - if (atrule.params.match(/ ?\(.*\) ?/)) { + if (atrule.params.match(/ ?\(.*\) ?$/)) { if (context.fix) { fix(atrule); } else { From 0e63041287084d68c4ec2658ef1e7715f5c4a973 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Mon, 26 Aug 2019 13:31:12 -0700 Subject: [PATCH 7/8] name change --- .../README.md | 2 +- .../__tests__/index.js | 0 .../index.js | 2 +- src/rules/index.js | 4 ++-- 4 files changed, 4 insertions(+), 4 deletions(-) rename src/rules/{at-conditional-rule-no-parentheses => at-rule-conditional-no-parentheses}/README.md (92%) rename src/rules/{at-conditional-rule-no-parentheses => at-rule-conditional-no-parentheses}/__tests__/index.js (100%) rename src/rules/{at-conditional-rule-no-parentheses => at-rule-conditional-no-parentheses}/index.js (96%) diff --git a/src/rules/at-conditional-rule-no-parentheses/README.md b/src/rules/at-rule-conditional-no-parentheses/README.md similarity index 92% rename from src/rules/at-conditional-rule-no-parentheses/README.md rename to src/rules/at-rule-conditional-no-parentheses/README.md index b9b4af6c..cc981f39 100644 --- a/src/rules/at-conditional-rule-no-parentheses/README.md +++ b/src/rules/at-rule-conditional-no-parentheses/README.md @@ -1,4 +1,4 @@ -# at-conditional-rule-no-parentheses +# at-rule-conditional-no-parentheses Disallow parentheses in conditional @ rules (if, elsif, while) diff --git a/src/rules/at-conditional-rule-no-parentheses/__tests__/index.js b/src/rules/at-rule-conditional-no-parentheses/__tests__/index.js similarity index 100% rename from src/rules/at-conditional-rule-no-parentheses/__tests__/index.js rename to src/rules/at-rule-conditional-no-parentheses/__tests__/index.js diff --git a/src/rules/at-conditional-rule-no-parentheses/index.js b/src/rules/at-rule-conditional-no-parentheses/index.js similarity index 96% rename from src/rules/at-conditional-rule-no-parentheses/index.js rename to src/rules/at-rule-conditional-no-parentheses/index.js index 8284ceb3..93982d4f 100644 --- a/src/rules/at-conditional-rule-no-parentheses/index.js +++ b/src/rules/at-rule-conditional-no-parentheses/index.js @@ -2,7 +2,7 @@ import { utils } from "stylelint"; import { namespace } from "../../utils"; import _ from "lodash"; -export const ruleName = namespace("at-conditional-rule-no-parentheses"); +export const ruleName = namespace("at-rule-conditional-no-parentheses"); export const messages = utils.ruleMessages(ruleName, { rejected: "Unexpected () used to surround statements for @-rules" diff --git a/src/rules/index.js b/src/rules/index.js index afd07def..26eeac30 100644 --- a/src/rules/index.js +++ b/src/rules/index.js @@ -1,4 +1,3 @@ -import atConditionalRuleNoParen from "./at-conditional-rule-no-parentheses"; import atExtendNoMissingPlaceholder from "./at-extend-no-missing-placeholder"; import atElseClosingBraceNewlineAfter from "./at-else-closing-brace-newline-after"; import atElseClosingBraceSpaceAfter from "./at-else-closing-brace-space-after"; @@ -17,6 +16,7 @@ import atMixinNamedArguments from "./at-mixin-named-arguments"; import atMixinParenthesesSpaceBefore from "./at-mixin-parentheses-space-before"; import atMixinPattern from "./at-mixin-pattern"; import atEachKeyValue from "./at-each-key-value-single-line"; +import atRuleConditionalNoParen from "./at-rule-conditional-no-parentheses"; import atRuleNoUnknown from "./at-rule-no-unknown"; import commentNoLoud from "./comment-no-loud"; import declarationNestedProperties from "./declaration-nested-properties"; @@ -48,7 +48,6 @@ import selectorNoRedundantNestingSelector from "./selector-no-redundant-nesting- import selectorNoUnionClassName from "./selector-no-union-class-name"; export default { - "at-conditional-rule-no-parentheses": atConditionalRuleNoParen, "at-extend-no-missing-placeholder": atExtendNoMissingPlaceholder, "at-else-closing-brace-newline-after": atElseClosingBraceNewlineAfter, "at-else-closing-brace-space-after": atElseClosingBraceSpaceAfter, @@ -67,6 +66,7 @@ export default { "at-mixin-parentheses-space-before": atMixinParenthesesSpaceBefore, "at-mixin-pattern": atMixinPattern, "at-each-key-value-single-line": atEachKeyValue, + "at-rule-conditional-no-parentheses": atRuleConditionalNoParen, "at-rule-no-unknown": atRuleNoUnknown, "comment-no-loud": commentNoLoud, "declaration-nested-properties": declarationNestedProperties, From a6d304e6d666e259879e7c21c3e88f34e8b97128 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Mon, 26 Aug 2019 13:35:27 -0700 Subject: [PATCH 8/8] use lodash for includes --- src/rules/at-rule-conditional-no-parentheses/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/at-rule-conditional-no-parentheses/index.js b/src/rules/at-rule-conditional-no-parentheses/index.js index 93982d4f..e4aa3831 100644 --- a/src/rules/at-rule-conditional-no-parentheses/index.js +++ b/src/rules/at-rule-conditional-no-parentheses/index.js @@ -29,7 +29,7 @@ function fix(atrule) { atrule.params = _.uniq(groups).join(" "); } -export default function(primary, _, context) { +export default function(primary, _unused, context) { return (root, result) => { const validOptions = utils.validateOptions(result, ruleName, { actual: primary @@ -41,7 +41,7 @@ export default function(primary, _, context) { root.walkAtRules(atrule => { // Check if this is a conditional rule. - if (!conditional_rules.includes(atrule.name)) { + if (!_.includes(conditional_rules, atrule.name)) { return; }