From 27e9741c95eee8f1de186a75e1ab7afc0b73fbb2 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 12 Jul 2019 10:29:05 -0700 Subject: [PATCH 1/9] Added new rule --- src/rules/at-if-no-null/README.md | 1 + src/rules/at-if-no-null/__tests__/index.js | 38 ++++++++++++++++++++++ src/rules/at-if-no-null/index.js | 10 ++++++ 3 files changed, 49 insertions(+) create mode 100644 src/rules/at-if-no-null/README.md create mode 100644 src/rules/at-if-no-null/__tests__/index.js create mode 100644 src/rules/at-if-no-null/index.js diff --git a/src/rules/at-if-no-null/README.md b/src/rules/at-if-no-null/README.md new file mode 100644 index 00000000..a5f2106a --- /dev/null +++ b/src/rules/at-if-no-null/README.md @@ -0,0 +1 @@ +# at-if-no-null diff --git a/src/rules/at-if-no-null/__tests__/index.js b/src/rules/at-if-no-null/__tests__/index.js new file mode 100644 index 00000000..c49618d7 --- /dev/null +++ b/src/rules/at-if-no-null/__tests__/index.js @@ -0,0 +1,38 @@ +import rule, { ruleName, messages } from ".."; + +testRule(rule, { + ruleName, + config: ["always-last-in-chain"], + syntax: "scss", + fix: true, + + accept: [ + { + code: `a { + @if ($x == 1) {} + width: 10px; + }`, + description: "always-last-in-chain (no @else, has newline after)." + } + ], + + reject: [ + { + code: `a { + @if ($x == 1) { + + } width: 10px; + }`, + fixed: `a { + @if ($x == 1) { + + } +width: 10px; + }`, + description: + "always-last-in-chain (has decl on the same line as its closing brace).", + message: messages.expected, + line: 4 + } + ] +}); diff --git a/src/rules/at-if-no-null/index.js b/src/rules/at-if-no-null/index.js new file mode 100644 index 00000000..c1b367db --- /dev/null +++ b/src/rules/at-if-no-null/index.js @@ -0,0 +1,10 @@ +import { isSingleLineString, namespace } from "../../utils"; +import { utils } from "stylelint"; +import { isBoolean } from "lodash"; + +export const ruleName = namespace("at-if-no-null"); + +export const messages = utils.ruleMessages(ruleName, { + equals_null: "Expected @if(statement) rather than @if(statement == null)", + not_equals_null: "Expected @if(!statement) rather than @if(statement != null)" +}); From ed1f3fbddf19fa1c7d49dcb32bf629007e7bc430 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 12 Jul 2019 10:33:42 -0700 Subject: [PATCH 2/9] option parsing + tests --- src/rules/at-if-no-null/__tests__/index.js | 30 ++++++++++++---------- src/rules/at-if-no-null/index.js | 12 +++++++++ 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/rules/at-if-no-null/__tests__/index.js b/src/rules/at-if-no-null/__tests__/index.js index c49618d7..2bd9f972 100644 --- a/src/rules/at-if-no-null/__tests__/index.js +++ b/src/rules/at-if-no-null/__tests__/index.js @@ -9,28 +9,32 @@ testRule(rule, { accept: [ { code: `a { - @if ($x == 1) {} - width: 10px; + @if ($x) {} }`, - description: "always-last-in-chain (no @else, has newline after)." + description: "does not use the != null format" + }, + { + code: `a { + @if not ($x) {} + }`, + description: "does not use the == null format" } ], reject: [ { code: `a { - @if ($x == 1) { - - } width: 10px; + @if ($x == null) {} }`, - fixed: `a { - @if ($x == 1) { - - } -width: 10px; + description: "uses the == null format", + message: messages.expected, + line: 4 + }, + { + code: `a { + @if ($x != null) {} }`, - description: - "always-last-in-chain (has decl on the same line as its closing brace).", + description: "uses the != null format", message: messages.expected, line: 4 } diff --git a/src/rules/at-if-no-null/index.js b/src/rules/at-if-no-null/index.js index c1b367db..b6923bb8 100644 --- a/src/rules/at-if-no-null/index.js +++ b/src/rules/at-if-no-null/index.js @@ -8,3 +8,15 @@ export const messages = utils.ruleMessages(ruleName, { equals_null: "Expected @if(statement) rather than @if(statement == null)", not_equals_null: "Expected @if(!statement) rather than @if(statement != null)" }); + +export default function(expectation, _, context) { + return (root, result) => { + const validOptions = utils.validateOptions(result, ruleName, { + actual: expectation + }); + + if (!validOptions) { + return; + } + }; +} From 0647d5238e04d0fc91c85506886a63d6423a60d1 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 12 Jul 2019 10:52:32 -0700 Subject: [PATCH 3/9] getting things working --- src/rules/at-if-no-null/__tests__/index.js | 6 +++--- src/rules/at-if-no-null/index.js | 23 ++++++++++++++++++++++ src/rules/index.js | 2 ++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/rules/at-if-no-null/__tests__/index.js b/src/rules/at-if-no-null/__tests__/index.js index 2bd9f972..7ed09e5a 100644 --- a/src/rules/at-if-no-null/__tests__/index.js +++ b/src/rules/at-if-no-null/__tests__/index.js @@ -2,7 +2,7 @@ import rule, { ruleName, messages } from ".."; testRule(rule, { ruleName, - config: ["always-last-in-chain"], + config: [true], syntax: "scss", fix: true, @@ -27,7 +27,7 @@ testRule(rule, { @if ($x == null) {} }`, description: "uses the == null format", - message: messages.expected, + message: messages.equals_null, line: 4 }, { @@ -35,7 +35,7 @@ testRule(rule, { @if ($x != null) {} }`, description: "uses the != null format", - message: messages.expected, + message: messages.not_equals_null, line: 4 } ] diff --git a/src/rules/at-if-no-null/index.js b/src/rules/at-if-no-null/index.js index b6923bb8..5bac9cf9 100644 --- a/src/rules/at-if-no-null/index.js +++ b/src/rules/at-if-no-null/index.js @@ -18,5 +18,28 @@ export default function(expectation, _, context) { if (!validOptions) { return; } + + root.walkAtRules(atrule => { + // Do nothing if it's not an @if + if (atrule.name !== "if") { + return; + } + + if (atrule.params.match(/\(.* == null\)/)) { + utils.report({ + message: messages.equals_null, + node: atrule, + result, + ruleName + }); + } else if (atrule.params.match(/\(.* != null \)/)) { + utils.report({ + message: messages.not_equals_null, + node: atrule, + result, + ruleName + }); + } + }); }; } diff --git a/src/rules/index.js b/src/rules/index.js index 5d264e1c..71817c9f 100644 --- a/src/rules/index.js +++ b/src/rules/index.js @@ -8,6 +8,7 @@ import atFunctionParenthesesSpaceBefore from "./at-function-parentheses-space-be import atFunctionPattern from "./at-function-pattern"; import atIfClosingBraceNewlineAfter from "./at-if-closing-brace-newline-after"; import atIfClosingBraceSpaceAfter from "./at-if-closing-brace-space-after"; +import atIfNoNull from "./at-if-no-null"; import atImportNoPartialLeadingUnderscore from "./at-import-no-partial-leading-underscore"; import atImportPartialExtensionBlacklist from "./at-import-partial-extension-blacklist"; import atImportPartialExtensionWhitelist from "./at-import-partial-extension-whitelist"; @@ -57,6 +58,7 @@ export default { "at-function-pattern": atFunctionPattern, "at-if-closing-brace-newline-after": atIfClosingBraceNewlineAfter, "at-if-closing-brace-space-after": atIfClosingBraceSpaceAfter, + "at-if-no-null": atIfNoNull, "at-import-no-partial-leading-underscore": atImportNoPartialLeadingUnderscore, "at-import-partial-extension-blacklist": atImportPartialExtensionBlacklist, "at-import-partial-extension-whitelist": atImportPartialExtensionWhitelist, From e8692edbcfe297d69233135eb1a01dc90d9ab9ba Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 12 Jul 2019 10:58:50 -0700 Subject: [PATCH 4/9] it works! --- src/rules/at-if-no-null/__tests__/index.js | 5 ++--- src/rules/at-if-no-null/index.js | 9 ++++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/rules/at-if-no-null/__tests__/index.js b/src/rules/at-if-no-null/__tests__/index.js index 7ed09e5a..90491960 100644 --- a/src/rules/at-if-no-null/__tests__/index.js +++ b/src/rules/at-if-no-null/__tests__/index.js @@ -4,7 +4,6 @@ testRule(rule, { ruleName, config: [true], syntax: "scss", - fix: true, accept: [ { @@ -28,7 +27,7 @@ testRule(rule, { }`, description: "uses the == null format", message: messages.equals_null, - line: 4 + line: 2 }, { code: `a { @@ -36,7 +35,7 @@ testRule(rule, { }`, description: "uses the != null format", message: messages.not_equals_null, - line: 4 + line: 2 } ] }); diff --git a/src/rules/at-if-no-null/index.js b/src/rules/at-if-no-null/index.js index 5bac9cf9..b3145a95 100644 --- a/src/rules/at-if-no-null/index.js +++ b/src/rules/at-if-no-null/index.js @@ -1,6 +1,5 @@ -import { isSingleLineString, namespace } from "../../utils"; +import { namespace } from "../../utils"; import { utils } from "stylelint"; -import { isBoolean } from "lodash"; export const ruleName = namespace("at-if-no-null"); @@ -9,7 +8,7 @@ export const messages = utils.ruleMessages(ruleName, { not_equals_null: "Expected @if(!statement) rather than @if(statement != null)" }); -export default function(expectation, _, context) { +export default function(expectation) { return (root, result) => { const validOptions = utils.validateOptions(result, ruleName, { actual: expectation @@ -25,14 +24,14 @@ export default function(expectation, _, context) { return; } - if (atrule.params.match(/\(.* == null\)/)) { + if (atrule.params.match(/\([ \t]*.* == null[ \t]*\)/)) { utils.report({ message: messages.equals_null, node: atrule, result, ruleName }); - } else if (atrule.params.match(/\(.* != null \)/)) { + } else if (atrule.params.match(/\([ \t]*.* != null[ \t]*\)/)) { utils.report({ message: messages.not_equals_null, node: atrule, From 8eeda73ffa5692dbfa85b8eefd937a200ce87594 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 12 Jul 2019 11:23:13 -0700 Subject: [PATCH 5/9] docs --- src/rules/at-if-no-null/README.md | 43 +++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/rules/at-if-no-null/README.md b/src/rules/at-if-no-null/README.md index a5f2106a..5deb45e0 100644 --- a/src/rules/at-if-no-null/README.md +++ b/src/rules/at-if-no-null/README.md @@ -1 +1,44 @@ # at-if-no-null + +Check for equality to null is unnecessarily explicit since `null` is falsey in Sass. + +```scss +a { + @if ($x == null) {} +/** ↑ ↑ + * == or != null is unncessary */ +} +``` + +## Options + +always + +### `always` + +The following patterns are considered warnings: +```scss +a { + @if ($x == null) {} +} +``` + +```scss +a { + @if ($x != null) {} +} +``` + +The following patterns are *not* considered warnings: + +```scss +a { + @if ($x) {} +} +``` + +```scss +a { + @if not ($x) {} +} +``` \ No newline at end of file From 77b9046bd86fe1088fa4ef2c656e790dca577c81 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 26 Jul 2019 13:54:29 -0700 Subject: [PATCH 6/9] some PR comments --- src/rules/at-if-no-null/README.md | 16 ++++++++-------- src/rules/at-if-no-null/__tests__/index.js | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rules/at-if-no-null/README.md b/src/rules/at-if-no-null/README.md index 5deb45e0..66a1edba 100644 --- a/src/rules/at-if-no-null/README.md +++ b/src/rules/at-if-no-null/README.md @@ -4,7 +4,7 @@ Check for equality to null is unnecessarily explicit since `null` is falsey in S ```scss a { - @if ($x == null) {} + @if $x == null {} /** ↑ ↑ * == or != null is unncessary */ } @@ -12,20 +12,20 @@ a { ## Options -always +true -### `always` +### `true` The following patterns are considered warnings: ```scss a { - @if ($x == null) {} + @if $x == null {} } ``` ```scss a { - @if ($x != null) {} + @if $x != null {} } ``` @@ -33,12 +33,12 @@ The following patterns are *not* considered warnings: ```scss a { - @if ($x) {} + @if $x {} } ``` ```scss a { - @if not ($x) {} + @if not $x {} } -``` \ No newline at end of file +``` diff --git a/src/rules/at-if-no-null/__tests__/index.js b/src/rules/at-if-no-null/__tests__/index.js index 90491960..ede427e7 100644 --- a/src/rules/at-if-no-null/__tests__/index.js +++ b/src/rules/at-if-no-null/__tests__/index.js @@ -8,13 +8,13 @@ testRule(rule, { accept: [ { code: `a { - @if ($x) {} + @if $x {} }`, description: "does not use the != null format" }, { code: `a { - @if not ($x) {} + @if not $x {} }`, description: "does not use the == null format" } @@ -23,7 +23,7 @@ testRule(rule, { reject: [ { code: `a { - @if ($x == null) {} + @if $x == null {} }`, description: "uses the == null format", message: messages.equals_null, @@ -31,7 +31,7 @@ testRule(rule, { }, { code: `a { - @if ($x != null) {} + @if $x != null {} }`, description: "uses the != null format", message: messages.not_equals_null, From ea7f2ac0e59d9478492a9d0675e81cbac0320cb1 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 26 Jul 2019 13:58:57 -0700 Subject: [PATCH 7/9] paren are optional on if rules --- src/rules/at-if-no-null/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/at-if-no-null/index.js b/src/rules/at-if-no-null/index.js index b3145a95..e57fc8bb 100644 --- a/src/rules/at-if-no-null/index.js +++ b/src/rules/at-if-no-null/index.js @@ -24,14 +24,14 @@ export default function(expectation) { return; } - if (atrule.params.match(/\([ \t]*.* == null[ \t]*\)/)) { + if (atrule.params.match(/\(?[ \t]*.* == null[ \t]*\)?/)) { utils.report({ message: messages.equals_null, node: atrule, result, ruleName }); - } else if (atrule.params.match(/\([ \t]*.* != null[ \t]*\)/)) { + } else if (atrule.params.match(/\(?[ \t]*.* != null[ \t]*\)?/)) { utils.report({ message: messages.not_equals_null, node: atrule, From 3f3919aa4f6ca19e27d161d74c7d779a6eaec775 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 26 Jul 2019 13:59:51 -0700 Subject: [PATCH 8/9] better error messages --- src/rules/at-if-no-null/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/at-if-no-null/index.js b/src/rules/at-if-no-null/index.js index e57fc8bb..800e8842 100644 --- a/src/rules/at-if-no-null/index.js +++ b/src/rules/at-if-no-null/index.js @@ -4,8 +4,8 @@ import { utils } from "stylelint"; export const ruleName = namespace("at-if-no-null"); export const messages = utils.ruleMessages(ruleName, { - equals_null: "Expected @if(statement) rather than @if(statement == null)", - not_equals_null: "Expected @if(!statement) rather than @if(statement != null)" + equals_null: "Expected @if not statement rather than @if statement == null", + not_equals_null: "Expected @if statement rather than @if statement != null" }); export default function(expectation) { From 0b89e0eff832a1979675ef3812b8c980ce996258 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Mon, 12 Aug 2019 16:33:14 -0700 Subject: [PATCH 9/9] if != null and --- src/rules/at-if-no-null/__tests__/index.js | 6 ++++++ src/rules/at-if-no-null/index.js | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/src/rules/at-if-no-null/__tests__/index.js b/src/rules/at-if-no-null/__tests__/index.js index ede427e7..6609f4b6 100644 --- a/src/rules/at-if-no-null/__tests__/index.js +++ b/src/rules/at-if-no-null/__tests__/index.js @@ -17,6 +17,12 @@ testRule(rule, { @if not $x {} }`, description: "does not use the == null format" + }, + { + code: `a { + @if $x != null and $x > 1 {} + }`, + description: "does not use the == null format" } ], diff --git a/src/rules/at-if-no-null/index.js b/src/rules/at-if-no-null/index.js index 800e8842..62d9c6b6 100644 --- a/src/rules/at-if-no-null/index.js +++ b/src/rules/at-if-no-null/index.js @@ -24,6 +24,11 @@ export default function(expectation) { return; } + // If rule != null and (expr), skip + if (atrule.params.match(/\(?[ \t]*.* != null and .*\)?/)) { + return; + } + if (atrule.params.match(/\(?[ \t]*.* == null[ \t]*\)?/)) { utils.report({ message: messages.equals_null,