Merge pull request #45 from primer/release-8.1.0

Release 8.1.0

__tests__/no-override.js

@@ -1,11 +1,25 @@
 const {lint, extendDefaultConfig} = require('./utils')
 
 describe('primer/no-override', () => {
+  it(`doesn't run when disabled`, () => {
+    const config = extendDefaultConfig({
+      rules: {
+        'primer/no-override': false
+      }
+    })
+    return lint(`.text-gray { color: #111; }`, config).then(data => {
+      expect(data).not.toHaveErrored()
+      expect(data).toHaveWarningsLength(0)
+    })
+  })
+
   it('reports instances of utility classes', () => {
     return lint('.text-gray { color: #111; }').then(data => {
       expect(data).toHaveErrored()
       expect(data).toHaveWarningsLength(1)
-      expect(data).toHaveWarnings([`".text-gray" should not be overridden (found in utilities). (primer/no-override)`])
+      expect(data).toHaveWarnings([
+        `".text-gray" should not be overridden (defined in @primer/css/utilities). (primer/no-override)`
+      ])
     })
   })
 
@@ -14,7 +28,9 @@ describe('primer/no-override', () => {
     return lint(`${selector} { color: #f00; }`).then(data => {
       expect(data).toHaveErrored()
       expect(data).toHaveWarningsLength(1)
-      expect(data).toHaveWarnings([`"${selector}" should not be overridden (found in utilities). (primer/no-override)`])
+      expect(data).toHaveWarnings([
+        `"${selector}" should not be overridden (defined in @primer/css/utilities). (primer/no-override)`
+      ])
     })
   })
 
@@ -24,22 +40,110 @@ describe('primer/no-override', () => {
       expect(data).toHaveErrored()
       expect(data).toHaveWarningsLength(1)
       expect(data).toHaveWarnings([
-        `"${selector}" should not be overridden in ".foo ${selector}:focus" (found in utilities). (primer/no-override)`
+        `"${selector}" should not be overridden in ".foo ${selector}:focus" (defined in @primer/css/utilities). (primer/no-override)`
       ])
     })
   })
 
-  it('warns when you pass an invalid bundle name', () => {
-    const config = extendDefaultConfig({
+  it('only reports class selectors', () => {
+    const config = {
+      plugins: [require.resolve('../plugins/no-override')],
       rules: {
-        'primer/no-override': [true, {bundles: ['asdf']}]
+        'primer/no-override': [true, {bundles: ['base']}]
       }
-    })
-    return lint('.foo { color: #f00; }', config).then(data => {
+    }
+    return lint(`body { color: #f00; }`, config).then(data => {
       expect(data).not.toHaveErrored()
-      expect(data.results[0].invalidOptionWarnings).toHaveLength(1)
-      expect(data.results[0].invalidOptionWarnings[0]).toEqual({
-        text: `The "bundles" option must be an array of valid bundles; got: "asdf"`
+      expect(data).toHaveWarningsLength(0)
+    })
+  })
+
+  describe('ignoreSelectors option', () => {
+    it('ignores selectors listed as strings', () => {
+      const config = extendDefaultConfig({
+        rules: {
+          'primer/no-override': [
+            true,
+            {
+              bundles: ['utilities'],
+              ignoreSelectors: ['.px-4']
+            }
+          ]
+        }
+      })
+      return lint(`.px-4 { margin: 0 4px !important; }`, config).then(data => {
+        expect(data).not.toHaveErrored()
+        expect(data).toHaveWarningsLength(0)
+      })
+    })
+
+    it('ignores selectors listed as regular expressions', () => {
+      const config = extendDefaultConfig({
+        rules: {
+          'primer/no-override': [
+            true,
+            {
+              bundles: ['utilities'],
+              ignoreSelectors: [/\.px-[0-9]/]
+            }
+          ]
+        }
+      })
+      return lint(`.px-4 { margin: 0 4px !important; }`, config).then(data => {
+        expect(data).not.toHaveErrored()
+        expect(data).toHaveWarningsLength(0)
+      })
+    })
+
+    it('ignores selectors when ignoreSelectors is a function', () => {
+      const config = extendDefaultConfig({
+        rules: {
+          'primer/no-override': [
+            true,
+            {
+              bundles: ['utilities'],
+              ignoreSelectors: selector => selector === '.px-4'
+            }
+          ]
+        }
+      })
+      return lint(`.px-4 { margin: 0 4px !important; }`, config).then(data => {
+        expect(data).not.toHaveErrored()
+        expect(data).toHaveWarningsLength(0)
+      })
+    })
+  })
+
+  describe('invalid options', () => {
+    it('warns when you bundles is not an array', () => {
+      const config = extendDefaultConfig({
+        rules: {
+          'primer/no-override': [true, {bundles: 'derp'}]
+        }
+      })
+      return lint('.foo { color: #f00; }', config).then(data => {
+        expect(data).not.toHaveErrored()
+        expect(data.results[0].invalidOptionWarnings).toEqual([
+          {
+            text: `The "bundles" option must be an array of valid bundles; got: (not an array)`
+          }
+        ])
+      })
+    })
+
+    it('warns when you pass an invalid bundle name', () => {
+      const config = extendDefaultConfig({
+        rules: {
+          'primer/no-override': [true, {bundles: ['asdf']}]
+        }
+      })
+      return lint('.foo { color: #f00; }', config).then(data => {
+        expect(data).not.toHaveErrored()
+        expect(data.results[0].invalidOptionWarnings).toEqual([
+          {
+            text: `The "bundles" option must be an array of valid bundles; got: "asdf"`
+          }
+        ])
       })
     })
   })

package.json

@@ -1,6 +1,6 @@
 {
   "name": "stylelint-config-primer",
-  "version": "8.0.0",
+  "version": "8.1.0",
   "description": "Sharable stylelint config used by GitHub's CSS",
   "homepage": "http://primer.style/css/tools/linting",
   "author": "GitHub, Inc.",

plugins/no-override.js

@@ -4,25 +4,40 @@ const {requirePrimerFile} = require('../src/primer')
 const ruleName = 'primer/no-override'
 const CLASS_PATTERN = /(\.[-\w]+)/
 const CLASS_PATTERN_ALL = new RegExp(CLASS_PATTERN, 'g')
+const CLASS_PATTERN_ONLY = /^\.[-\w]+(:{1,2}[-\w]+)?$/
 
 module.exports = stylelint.createPlugin(ruleName, (enabled, options = {}) => {
   if (!enabled) {
     return noop
   }
 
-  const {bundles = ['utilities']} = options
+  const {bundles = ['utilities'], ignoreSelectors = []} = options
+
+  const isSelectorIgnored =
+    typeof ignoreSelectors === 'function'
+      ? ignoreSelectors
+      : selector => {
+          return ignoreSelectors.some(pattern => {
+            return pattern instanceof RegExp ? pattern.test(selector) : selector.includes(pattern)
+          })
+        }
 
   const primerMeta = requirePrimerFile('dist/meta.json')
   const availableBundles = Object.keys(primerMeta.bundles)
 
+  // These map selectors to the bundle in which they're defined.
+  // If there's no entry for a given selector, it means that it's not defined
+  // in one of the *specified* bundles, since we're iterating over the list of
+  // bundle names in the options.
   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`)
-    const selectors = stats.selectors.values.filter(selector => CLASS_PATTERN.test(selector))
+    const selectors = stats.selectors.values
     for (const selector of selectors) {
       immutableSelectors.set(selector, bundle)
       for (const classSelector of getClassSelectors(selector)) {
@@ -32,11 +47,11 @@ module.exports = stylelint.createPlugin(ruleName, (enabled, options = {}) => {
   }
 
   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}.`
+    rejected: ({rule, selector, bundle}) => {
+      const definedIn = ` (defined in @primer/css/${bundle})`
+      const ruleSelector = collapseWhitespace(rule.selector)
+      const context = selector === rule.selector ? '' : ` in "${ruleSelector}"`
+      return `"${collapseWhitespace(selector)}" should not be overridden${context}${definedIn}.`
     }
   })
 
@@ -51,29 +66,37 @@ module.exports = stylelint.createPlugin(ruleName, (enabled, options = {}) => {
       })
     }
 
-    if (!enabled) {
-      return
-    }
-
-    const report = (rule, subject) =>
+    const report = subject =>
       stylelint.utils.report({
-        message: messages.rejected(rule, subject),
-        node: rule,
+        message: messages.rejected(subject),
+        node: subject.rule,
         result,
         ruleName
       })
 
     root.walkRules(rule => {
-      const subject = {rule}
-      if (immutableSelectors.has(rule.selector)) {
-        subject.bundle = immutableSelectors.get(rule.selector)
-        report(rule, subject)
-      } else {
-        for (const classSelector of getClassSelectors(rule.selector)) {
-          if (immutableClassSelectors.has(classSelector)) {
-            subject.bundle = immutableClassSelectors.get(classSelector)
-            subject.selector = classSelector
-            report(rule, subject)
+      const {selector} = rule
+      if (immutableSelectors.has(selector)) {
+        if (isClassSelector(selector)) {
+          if (!isSelectorIgnored(selector)) {
+            return report({
+              rule,
+              bundle: immutableSelectors.get(selector),
+              selector
+            })
+          }
+        } else {
+          // console.log(`not a class selector: "${selector}"`)
+        }
+      }
+      for (const classSelector of getClassSelectors(selector)) {
+        if (immutableClassSelectors.has(classSelector)) {
+          if (!isSelectorIgnored(classSelector)) {
+            return report({
+              rule,
+              bundle: immutableClassSelectors.get(classSelector),
+              selector: classSelector
+            })
           }
         }
       }
@@ -86,4 +109,12 @@ function getClassSelectors(selector) {
   return match ? [...match] : []
 }
 
+function isClassSelector(selector) {
+  return CLASS_PATTERN_ONLY.test(selector)
+}
+
+function collapseWhitespace(str) {
+  return str.trim().replace(/\s+/g, ' ')
+}
+
 function noop() {}