Fix false negatives for CSS-In-JS in color-no-invalid-hex (#3957)

stylelint · Feb 21, 2019 · ee266b6 · ee266b6
1 parent ed3d097
commit ee266b6
Show file tree

Hide file tree

Showing 21 changed files with 116 additions and 111 deletions.
diff --git a/lib/rules/at-rule-no-unknown/__tests__/index.js b/lib/rules/at-rule-no-unknown/__tests__/index.js
@@ -160,7 +160,6 @@ testRule(rule, {
   ]
 });
 
-
 testRule(rule, {
   ruleName,
   config: [true, { ignoreAtRules: [/^my-/] }],

diff --git a/lib/rules/color-no-invalid-hex/__tests__/index.js b/lib/rules/color-no-invalid-hex/__tests__/index.js
@@ -79,3 +79,24 @@ testRule(rule, {
     }
   ]
 });
+
+testRule(rule, {
+  ruleName,
+  config: [true],
+  syntax: "css-in-js",
+
+  accept: [
+    {
+      code: 'export default <h1 style={{ color: "#ffff" }}>Test</h1>;'
+    }
+  ],
+
+  reject: [
+    {
+      code: 'export default <h1 style={{ color: "#fffff" }}>Test</h1>;',
+      message: messages.rejected("#fffff"),
+      line: 1,
+      column: 36
+    }
+  ]
+});
diff --git a/lib/rules/color-no-invalid-hex/index.js b/lib/rules/color-no-invalid-hex/index.js
@@ -1,10 +1,11 @@
 "use strict";
 
+const declarationValueIndex = require("../../utils/declarationValueIndex");
 const isValidHex = require("../../utils/isValidHex");
 const report = require("../../utils/report");
 const ruleMessages = require("../../utils/ruleMessages");
-const styleSearch = require("style-search");
 const validateOptions = require("../../utils/validateOptions");
+const valueParser = require("postcss-value-parser");
 
 const ruleName = "color-no-invalid-hex";
 
@@ -21,34 +22,21 @@ const rule = function(actual) {
     }
 
     root.walkDecls(decl => {
-      const declString = decl.toString();
+      valueParser(decl.value).walk(({ value, type, sourceIndex }) => {
+        if (type !== "word") return;
 
-      styleSearch({ source: declString, target: "#" }, match => {
-        // If there's not a colon, comma, or whitespace character before, we'll assume this is
-        // not intended to be a hex color, but is instead something like the
-        // hash in a url() argument
-        if (!/[:,\s]/.test(declString[match.startIndex - 1])) {
-          return;
-        }
+        const hexMatch = /^#[0-9A-Za-z]+/.exec(value);
 
-        const hexMatch = /^#[0-9A-Za-z]+/.exec(
-          declString.substr(match.startIndex)
-        );
-
-        if (!hexMatch) {
-          return;
-        }
+        if (!hexMatch) return;
 
         const hexValue = hexMatch[0];
 
-        if (isValidHex(hexValue)) {
-          return;
-        }
+        if (isValidHex(hexValue)) return;
 
         report({
           message: messages.rejected(hexValue),
           node: decl,
-          index: match.startIndex,
+          index: declarationValueIndex(decl) + sourceIndex,
           result,
           ruleName
         });

diff --git a/lib/rules/comment-word-blacklist/__tests__/index.js b/lib/rules/comment-word-blacklist/__tests__/index.js
@@ -304,7 +304,6 @@ testRule(rule, {
   ]
 });
 
-
 testRule(rule, {
   ruleName,
   config: [/^TODO:/, "bad-word"],
@@ -323,4 +322,4 @@ testRule(rule, {
       column: 1
     }
   ]
-})
+});
diff --git a/lib/rules/declaration-property-value-blacklist/__tests__/index.js b/lib/rules/declaration-property-value-blacklist/__tests__/index.js
@@ -143,7 +143,6 @@ testRule(rule, {
   ]
 });
 
-
 testRule(rule, {
   ruleName,
 

diff --git a/lib/rules/font-family-no-duplicate-names/__tests__/index.js b/lib/rules/font-family-no-duplicate-names/__tests__/index.js
@@ -113,10 +113,7 @@ testRule(rule, {
 
 testRule(rule, {
   ruleName,
-  config: [
-    true,
-    { ignoreFontFamilyNames: [/my-/] }
-  ],
+  config: [true, { ignoreFontFamilyNames: [/my-/] }],
 
   accept: [
     {

diff --git a/lib/rules/media-feature-name-no-unknown/__tests__/index.js b/lib/rules/media-feature-name-no-unknown/__tests__/index.js
@@ -212,10 +212,7 @@ testRule(rule, {
 
 testRule(rule, {
   ruleName,
-  config: [
-    true,
-    { ignoreMediaFeatureNames: [/^my-/] }
-  ],
+  config: [true, { ignoreMediaFeatureNames: [/^my-/] }],
 
   accept: [
     {

diff --git a/lib/rules/media-feature-name-value-whitelist/__tests__/index.js b/lib/rules/media-feature-name-value-whitelist/__tests__/index.js
@@ -105,12 +105,11 @@ testRule(rule, {
   ]
 });
 
-
 testRule(rule, {
   ruleName,
   config: [
     {
-      "/resolution/": [/dpcm$/], // Only dpcm unit
+      "/resolution/": [/dpcm$/] // Only dpcm unit
     }
   ],
 
@@ -135,7 +134,6 @@ testRule(rule, {
       message: messages.rejected("min-resolution", "2dpi"),
       line: 1,
       column: 37
-    },
-
+    }
   ]
 });
diff --git a/lib/rules/media-feature-name-whitelist/__tests__/index.js b/lib/rules/media-feature-name-whitelist/__tests__/index.js
@@ -76,7 +76,6 @@ testRule(rule, {
   ]
 });
 
-
 testRule(rule, {
   ruleName,
   config: [/^my-/],

diff --git a/lib/rules/number-max-precision/__tests__/index.js b/lib/rules/number-max-precision/__tests__/index.js
@@ -204,7 +204,6 @@ testRule(rule, {
   ]
 });
 
-
 testRule(rule, {
   ruleName,
   config: [0, { ignoreUnits: [/^my-/] }],

diff --git a/lib/rules/property-blacklist/__tests__/index.js b/lib/rules/property-blacklist/__tests__/index.js
@@ -120,7 +120,6 @@ testRule(rule, {
   ]
 });
 
-
 testRule(rule, {
   ruleName,
 

diff --git a/lib/rules/property-no-vendor-prefix/__tests__/index.js b/lib/rules/property-no-vendor-prefix/__tests__/index.js
@@ -146,10 +146,7 @@ testRule(rule, {
 
 testRule(rule, {
   ruleName,
-  config: [
-    true,
-    { ignoreProperties: [/^animation-/i] }
-  ],
+  config: [true, { ignoreProperties: [/^animation-/i] }],
 
   accept: [
     {

diff --git a/lib/rules/unit-blacklist/__tests__/index.js b/lib/rules/unit-blacklist/__tests__/index.js
@@ -514,7 +514,6 @@ testRule(rule, {
   ]
 });
 
-
 testRule(rule, {
   ruleName,
 

diff --git a/lib/rules/unit-blacklist/index.js b/lib/rules/unit-blacklist/index.js
@@ -41,8 +41,14 @@ const rule = function(blacklistInput, options) {
         optional: true,
         actual: options,
         possible: {
-          ignoreProperties: validateObjectWithArrayProps([_.isString, _.isRegExp]),
-          ignoreMediaFeatureNames: validateObjectWithArrayProps([_.isString, _.isRegExp]),
+          ignoreProperties: validateObjectWithArrayProps([
+            _.isString,
+            _.isRegExp
+          ]),
+          ignoreMediaFeatureNames: validateObjectWithArrayProps([
+            _.isString,
+            _.isRegExp
+          ])
         }
       }
     );

diff --git a/lib/rules/unit-whitelist/index.js b/lib/rules/unit-whitelist/index.js
@@ -32,7 +32,10 @@ const rule = function(whitelistInput, options) {
         optional: true,
         actual: options,
         possible: {
-          ignoreProperties: validateObjectWithArrayProps([_.isString, _.isRegExp])
+          ignoreProperties: validateObjectWithArrayProps([
+            _.isString,
+            _.isRegExp
+          ])
         }
       }
     );

diff --git a/lib/rules/value-keyword-case/__tests__/index.js b/lib/rules/value-keyword-case/__tests__/index.js
@@ -2101,13 +2101,9 @@ testRule(rule, {
   ]
 });
 
-
 testRule(rule, {
   ruleName,
-  config: [
-    "upper",
-    { ignoreKeywords: [/^(f|F)lex$/] }
-  ],
+  config: ["upper", { ignoreKeywords: [/^(f|F)lex$/] }],
   fix: true,
 
   accept: [
@@ -2177,7 +2173,6 @@ testRule(rule, {
   ]
 });
 
-
 testRule(rule, {
   ruleName,
   config: ["lower", { ignoreProperties: [/^(b|B)ackground$/] }],

diff --git a/lib/utils/__tests__/validateObjectWithArrayProps.test.js b/lib/utils/__tests__/validateObjectWithArrayProps.test.js
@@ -2,51 +2,61 @@
 
 const validateObjectWithArrayProps = require("../validateObjectWithArrayProps");
 
-describe('validateObjectWithArrayProps', () => {
-  it('should return a function', () => {
-    expect(validateObjectWithArrayProps(x => x)).toBeInstanceOf(Function)
-  })
-
-  describe('returned validator', () => {
-    const validator = validateObjectWithArrayProps(x => x)
-
-    it('should return false if any of the object properties are not an array', () => {
-      expect(validator({
-        arrayProp: [1, 2],
-        nonArrayProp: 3
-      })).toBeFalsy()
-    })
-
-    it('should return false if any of the object properties array values do not pass the test', () => {
-      expect(validator({
-        arrayProp: [1, 2],
-        nonArrayProp: [0, 3]
-      })).toBeFalsy()
-    })
-
-    it('should return true otherwise', () => {
-      expect(validator({
-        arrayProp: [1, 2],
-        nonArrayProp: [3, 4]
-      })).toBeTruthy()
-    })
-  })
-
-  describe('returned validator with array', () => {
-    const validator = validateObjectWithArrayProps([x => x > 0, x => x < 0])
-
-    it('should accept an array of validators, any of which can return true', () => {
-      expect(validator({
-        arrayProp: [1, 2],
-        nonArrayProp: [-1, 3]
-      })).toBeTruthy()
-    })
-
-    it('should be false if none of the validators are true for any value', () => {
-      expect(validator({
-        arrayProp: [1, 2],
-        nonArrayProp: [-1, 0]
-      })).toBeFalsy()
-    })
-  })
-})
+describe("validateObjectWithArrayProps", () => {
+  it("should return a function", () => {
+    expect(validateObjectWithArrayProps(x => x)).toBeInstanceOf(Function);
+  });
+
+  describe("returned validator", () => {
+    const validator = validateObjectWithArrayProps(x => x);
+
+    it("should return false if any of the object properties are not an array", () => {
+      expect(
+        validator({
+          arrayProp: [1, 2],
+          nonArrayProp: 3
+        })
+      ).toBeFalsy();
+    });
+
+    it("should return false if any of the object properties array values do not pass the test", () => {
+      expect(
+        validator({
+          arrayProp: [1, 2],
+          nonArrayProp: [0, 3]
+        })
+      ).toBeFalsy();
+    });
+
+    it("should return true otherwise", () => {
+      expect(
+        validator({
+          arrayProp: [1, 2],
+          nonArrayProp: [3, 4]
+        })
+      ).toBeTruthy();
+    });
+  });
+
+  describe("returned validator with array", () => {
+    const validator = validateObjectWithArrayProps([x => x > 0, x => x < 0]);
+
+    it("should accept an array of validators, any of which can return true", () => {
+      expect(
+        validator({
+          arrayProp: [1, 2],
+          nonArrayProp: [-1, 3]
+        })
+      ).toBeTruthy();
+    });
+
+    it("should be false if none of the validators are true for any value", () => {
+      expect(
+        validator({
+          arrayProp: [1, 2],
+          nonArrayProp: [-1, 0]
+        })
+      ).toBeFalsy();
+    });
+  });
+});
diff --git a/lib/utils/validateObjectWithArrayProps.js b/lib/utils/validateObjectWithArrayProps.js
@@ -13,9 +13,7 @@ const _ = require("lodash");
  * }
  */
 
-module.exports = (
-  validator /*: Function */
-) => (
+module.exports = (validator /*: Function */) => (
   value /*: Object*/
 ) /*: boolean*/ => {
   if (!_.isPlainObject(value)) {
@@ -30,10 +28,10 @@ module.exports = (
     // Make sure the array items are strings
     return value[key].every(item => {
       if (Array.isArray(validator)) {
-        return validator.some(v => v(item))
+        return validator.some(v => v(item));
       }
 
-      return validator(item)
+      return validator(item);
     });
   });
 };