Merge pull request #37 from primer/no-override-classes

Improve primer/no-override to match classes, better reporting
primer · Sep 9, 2019 · e59866d · e59866d
2 parents 806730e + 29bcf19
commit e59866d
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 19 deletions.
diff --git a/__tests__/no-override.js b/__tests__/no-override.js
@@ -5,7 +5,7 @@ describe('primer/no-override', () => {
     return lint('.text-gray { color: #111; }').then(data => {
       expect(data).toHaveErrored()
       expect(data).toHaveWarningsLength(1)
-      expect(data).toHaveWarnings([`The selector ".text-gray" should not be overridden. (primer/no-override)`])
+      expect(data).toHaveWarnings([`".text-gray" should not be overridden (found in utilities). (primer/no-override)`])
     })
   })
 
@@ -14,7 +14,7 @@ describe('primer/no-override', () => {
     return lint(`${selector} { color: #f00; }`).then(data => {
       expect(data).toHaveErrored()
       expect(data).toHaveWarningsLength(1)
-      expect(data).toHaveWarnings([`The selector "${selector}" should not be overridden. (primer/no-override)`])
+      expect(data).toHaveWarnings([`"${selector}" should not be overridden (found in utilities). (primer/no-override)`])
     })
   })
 
@@ -23,7 +23,9 @@ describe('primer/no-override', () => {
     return lint(`.foo ${selector}:focus { color: #f00; }`).then(data => {
       expect(data).toHaveErrored()
       expect(data).toHaveWarningsLength(1)
-      expect(data).toHaveWarnings([`The selector "${selector}" should not be overridden. (primer/no-override)`])
+      expect(data).toHaveWarnings([
+        `"${selector}" should not be overridden in ".foo ${selector}:focus" (found in utilities). (primer/no-override)`
+      ])
     })
   })
 

diff --git a/plugins/no-override.js b/plugins/no-override.js
@@ -2,11 +2,8 @@ const stylelint = require('stylelint')
 const {requirePrimerFile} = require('../src/primer')
 
 const ruleName = 'primer/no-override'
-const CLASS_PATTERN = /(\.[-\w]+)/g
-
-const messages = stylelint.utils.ruleMessages(ruleName, {
-  rejected: selector => `The selector "${selector}" should not be overridden.`
-})
+const CLASS_PATTERN = /(\.[-\w]+)/
+const CLASS_PATTERN_ALL = new RegExp(CLASS_PATTERN, 'g')
 
 module.exports = stylelint.createPlugin(ruleName, (enabled, options = {}) => {
   if (!enabled) {
@@ -18,22 +15,30 @@ module.exports = stylelint.createPlugin(ruleName, (enabled, options = {}) => {
   const primerMeta = requirePrimerFile('dist/meta.json')
   const availableBundles = Object.keys(primerMeta.bundles)
 
-  const immutableSelectors = new Set()
-  const immutableClassSelectors = new Set()
+  const immutableSelectors = new Map()
+  const immutableClassSelectors = new Map()
   for (const bundle of bundles) {
     if (!availableBundles.includes(bundle)) {
       continue
     }
     const stats = requirePrimerFile(`dist/stats/${bundle}.json`)
-    for (const selector of stats.selectors.values) {
-      immutableSelectors.add(selector)
+    const selectors = stats.selectors.values.filter(selector => CLASS_PATTERN.test(selector))
+    for (const selector of selectors) {
+      immutableSelectors.set(selector, bundle)
       for (const classSelector of getClassSelectors(selector)) {
-        immutableClassSelectors.add(classSelector)
+        immutableClassSelectors.set(classSelector, bundle)
       }
     }
   }
 
-  // console.warn(`Got ${immutableSelectors.size} immutable selectors from: "${bundles.join('", "')}"`)
+  const messages = stylelint.utils.ruleMessages(ruleName, {
+    rejected: (rule, {selector, bundle}) => {
+      const suffix = bundle ? ` (found in ${bundle})` : ''
+      return selector
+        ? `"${selector}" should not be overridden in "${rule.selector}"${suffix}.`
+        : `"${rule.selector}" should not be overridden${suffix}.`
+    }
+  })
 
   return (root, result) => {
     if (!Array.isArray(bundles) || bundles.some(bundle => !availableBundles.includes(bundle))) {
@@ -50,21 +55,25 @@ module.exports = stylelint.createPlugin(ruleName, (enabled, options = {}) => {
       return
     }
 
-    const report = (rule, selector) =>
+    const report = (rule, subject) =>
       stylelint.utils.report({
-        message: messages.rejected(selector),
+        message: messages.rejected(rule, subject),
         node: rule,
         result,
         ruleName
       })
 
     root.walkRules(rule => {
+      const subject = {rule}
       if (immutableSelectors.has(rule.selector)) {
-        report(rule, rule.selector)
+        subject.bundle = immutableSelectors.get(rule.selector)
+        report(rule, subject)
       } else {
         for (const classSelector of getClassSelectors(rule.selector)) {
           if (immutableClassSelectors.has(classSelector)) {
-            report(rule, classSelector)
+            subject.bundle = immutableClassSelectors.get(classSelector)
+            subject.selector = classSelector
+            report(rule, subject)
           }
         }
       }
@@ -73,7 +82,7 @@ module.exports = stylelint.createPlugin(ruleName, (enabled, options = {}) => {
 })
 
 function getClassSelectors(selector) {
-  const match = selector.match(CLASS_PATTERN)
+  const match = selector.match(CLASS_PATTERN_ALL)
   return match ? [...match] : []
 }