Upgrade stylelint config and adding stylelint scripts (#1934)

* Upgrading to @primer/stylelint-config@12.3.1

* Adding scripts for automating stylelint tasks

* Disabling violations after upgrade

package.json

@@ -25,7 +25,10 @@
   "scripts": {
     "dist": "script/dist.js",
     "dist:watch": "chokidar \"src/**/*.scss\" -c \"script/dist.js\"",
-    "stylelint": "stylelint --quiet 'src/**/*.scss'",
+    "stylelint": "stylelint --quiet --rd 'src/**/*.scss'",
+    "stylelint:fix": "yarn stylelint -- --fix",
+    "stylelint:remove-disables": "script/stylelint-remove-disables.js 'src/**/*.scss'",
+    "stylelint:add-disables": "script/stylelint-add-disables.js 'src/**/*.scss'",
     "eslint": "eslint deprecations.js __tests__ script",
     "prepublishOnly": "script/prepublish",
     "start": "yarn dev",
@@ -43,7 +46,7 @@
     "@changesets/cli": "2.20.0",
     "@github/prettier-config": "0.0.4",
     "@koddsson/postcss-sass": "5.0.1",
-    "@primer/stylelint-config": "^12.3.0",
+    "@primer/stylelint-config": "^12.3.1",
     "autoprefixer": "10.4.2",
     "chokidar-cli": "3.0.0",
     "cssstats": "4.0.2",

script/remove-needless-disables

@@ -1,11 +0,0 @@
-#!/bin/bash
-set -e
-
-disables=$(stylelint src --quiet --report-needless-disables -f unix | grep '^/')
-
-while IFS= read -r line; do
-  case $line in
-    (*:*:*:*) file=${line%:*}
-  esac
-  echo $file
-done < <(echo "$disables")

script/stylelint-add-disables.js

@@ -0,0 +1,56 @@
+#!/usr/bin/env node
+// Disables stylelint rules in SASS/CSS files with next-line comments. This is
+// useful when introducing a new rule that causes many failures. The comments
+// can be fixed and removed at while updating the file later.
+//
+// Usage:
+//
+//  script/stylelint-add-disables.js src/**/*.scss
+
+import fs from 'fs'
+import {execFile} from 'child_process'
+
+execFile('stylelint', ['--quiet', '--formatter', 'json', process.argv[2]], (error, stdout) => {
+  for (const result of JSON.parse(stdout)) {
+    const filename = result.source
+    const jsLines = fs.readFileSync(filename, 'utf8').split('\n')
+    const offensesByLine = {}
+    let addedLines = 0
+
+    // Produces {47: ['github/no-d-none', 'github/no-blur'], 83: ['github/no-blur']}
+    for (const message of result.warnings) {
+      if (offensesByLine[message.lin] && offensesByLine[message.line].includes(message.rule)) {
+        continue
+      } else if (offensesByLine[message.line]) {
+        offensesByLine[message.line].push(message.rule)
+      } else {
+        offensesByLine[message.line] = [message.rule]
+      }
+    }
+
+    for (const line of Object.keys(offensesByLine)) {
+      const lineIndex = Number(line) - 1 + addedLines
+      const previousLine = jsLines[lineIndex - 1]
+      const ruleIds = Array.from(new Set(offensesByLine[line])).join(', ')
+      if (isDisableComment(previousLine)) {
+        if (previousLine.match(/\s?\*\/$/)) {
+          jsLines[lineIndex - 1] = previousLine.replace(/\s?\*\/$/, `, ${ruleIds} */`)
+        } else {
+          jsLines[lineIndex - 1] = `${jsLines[lineIndex - 1]}, ${ruleIds}`
+        }
+      } else {
+        const leftPad = ' '.repeat(jsLines[lineIndex].match(/^\s*/g)[0].length)
+        jsLines.splice(lineIndex, 0, `${leftPad}// stylelint-disable-next-line ${ruleIds}`)
+        addedLines += 1
+      }
+    }
+
+    if (result.warnings.length !== 0) {
+      fs.writeFileSync(filename, jsLines.join('\n'), 'utf8')
+    }
+  }
+})
+
+function isDisableComment(line) {
+  return line && line.match(/\/(\*|\/) stylelint-disable(-next-line)? .+(\*\/)?/)
+}

script/stylelint-remove-disables.js

@@ -0,0 +1,82 @@
+#!/usr/bin/env node
+import {execFile} from 'child_process'
+import {readFileSync, writeFileSync} from 'fs'
+
+const files = process.argv.slice(2)
+if (files.length === 0) {
+  files.push('app/assets/stylesheets')
+}
+
+// we use an empty "marker" to delineate removed lines
+const REMOVED = `===REMOVED@${Date.now()}===`
+
+execFile('stylelint', ['--rd', '--quiet', '--formatter', 'json', process.argv[2]], (error, stdout) => {
+
+  // Filter to only needless diables comments
+  const results = JSON.parse(stdout)
+    .filter(result => result.warnings.length > 0)
+    .map(({source, warnings}) => {
+      return {
+        source,
+        warnings: warnings.filter(warning => warning.rule === '--report-needless-disables')
+      }
+    })
+
+  for (const {source, warnings} of results) {
+    console.log(`--- ${source}\n+++ ${source}`)
+    const lines = readFileSync(source, 'utf8').split(/\n/)
+    for (const {text, line: lineNum} of warnings) {
+      const ruleName = text.match(/^Needless disable for "(.+)"$/)[1]
+      const line = lines[lineNum - 1]
+      let disableComment = parseDisableComment(line)
+
+      if (disableComment) {
+        const rules = new Set(disableComment.rules)
+        rules.delete(ruleName)
+        let replacementLine = line
+        if (rules.size === 0) {
+          console.log(`@@ ${lineNum},${lineNum - 1} @@`)
+          replacementLine = line.replace(`// ${disableComment.content}`, REMOVED)
+        } else {
+          console.log(`@@ ${lineNum},${lineNum} @@`)
+          replacementLine = line.replace(disableComment.content, `${disableComment.type} ${Array.from(rules).join(', ')}`)
+        }
+        lines[lineNum - 1] = replacementLine
+
+        if (lines[lineNum - 2]) {
+          console.log(`${lines[lineNum - 2]}`)
+        }
+        console.log(`- ${line}`)
+        if (!replacementLine.includes(REMOVED)) {
+          console.log(`+ ${replacementLine}`)
+        }
+        if (lines[lineNum]) {
+          console.log(`${lines[lineNum]}`)
+        }
+      }
+    }
+    console.log('')
+    const output = lines.map(line => {
+      if (line.trim() === REMOVED) {
+        return null
+      } else if (line.includes(REMOVED)) {
+        line = line.replace(REMOVED, '').trimEnd()
+      }
+
+      return line
+    }).filter(line => line !== null).join('\n')
+
+    writeFileSync(source, output, 'utf8')
+  }
+})
+
+function parseDisableComment(str) {
+  const match = str.match(/(stylelint-disable((-next)?-line)?)\s+(.+)$/)
+  return match
+    ? {
+        content: match[0],
+        type: match[1],
+        rules: match[4].split(/,\s+/)
+      }
+    : false
+}

src/actionlist/action-list-item.scss

@@ -389,6 +389,7 @@
 
   // if nested list exists, remove default padding
   .ActionList {
+    // stylelint-disable-next-line primer/spacing
     padding: unset;
   }
 }
@@ -397,6 +398,7 @@
 .ActionList-content {
   position: relative;
   display: grid;
+  // stylelint-disable-next-line primer/spacing
   padding: $actionList-item-padding-vertical $actionList-item-padding-horizontal;
   font-size: $body-font-size;
   font-weight: $font-weight-normal;
@@ -423,11 +425,13 @@
 
   &.ActionList-content--sizeMedium {
     // 44px total height
+    // stylelint-disable-next-line primer/spacing
     padding: $actionList-item-padding-vertical-md $actionList-item-padding-horizontal;
   }
 
   &.ActionList-content--sizeLarge {
     // 48px total height
+    // stylelint-disable-next-line primer/spacing
     padding: $actionList-item-padding-vertical-lg $actionList-item-padding-horizontal;
   }
 
@@ -437,6 +441,7 @@
 
   // On pointer:coarse (mobile), all list items are large
   @media (pointer: coarse) {
+    // stylelint-disable-next-line primer/spacing
     padding: $actionList-item-padding-vertical-lg $actionList-item-padding-horizontal;
   }
 
@@ -493,6 +498,7 @@
   align-items: baseline;
 
   .ActionList-item-description {
+    // stylelint-disable-next-line primer/spacing
     margin-left: $actionList-item-padding-horizontal;
   }
 }

src/forms/radio-group.scss

@@ -7,6 +7,7 @@
 .radio-label {
   float: left;
   padding: 6px $spacer-3 6px ($spacer-3 + 12px + $spacer-2); // 12px is the size of the radio-input
+  // stylelint-disable-next-line primer/spacing
   margin-left: -1px;
   font-size: $body-font-size;
   // stylelint-disable-next-line primer/typography
@@ -41,6 +42,7 @@
 .radio-input {
   z-index: 3;
   float: left;
+  // stylelint-disable-next-line primer/spacing
   margin: 10px (-$spacer-5) 0 $spacer-3;
 
   &:disabled {

src/layout/page-layout.scss

@@ -361,11 +361,14 @@ $Layout-responsive-variant-max-breakpoint: 'md' !default;
 .PageLayout-content,
 .PageLayout-pane,
 .PageLayout-footer {
+  // stylelint-disable-next-line primer/spacing
   padding: var(--Layout-inner-spacing-min);
 }
 
 .PageLayout-content {
+  // stylelint-disable-next-line primer/spacing
   padding-right: var(--Layout-inner-spacing-max);
+  // stylelint-disable-next-line primer/spacing
   padding-left: var(--Layout-inner-spacing-max);
   grid-area: content;
 }

yarn.lock

@@ -1153,10 +1153,10 @@
   resolved "https://registry.yarnpkg.com/@primer/primitives/-/primitives-7.4.0.tgz#75df54a80233a432b687c0e3010e4be6bd60a82d"
   integrity sha512-gD6yHXN7YKox/bdUNgxhoSS/WXZVaORK1r4dOAyTrdoPrLV/ucIfRInPyVcTF+Mqr0zcTFJtiMtuA5Y8CSyOEg==
 
-"@primer/stylelint-config@^12.3.0":
-  version "12.3.0"
-  resolved "https://registry.yarnpkg.com/@primer/stylelint-config/-/stylelint-config-12.3.0.tgz#de45e16f21f245f31a634ab76a5fcdef796495ac"
-  integrity sha512-BZG1ARFaWLQP8avLObAtpSj88z/CLAvhSND+Ts2H1KEYnKNmwkgqwc6CW5OVpLD5BUKqDwun+8OVa37+ueeSQA==
+"@primer/stylelint-config@^12.3.1":
+  version "12.3.1"
+  resolved "https://registry.yarnpkg.com/@primer/stylelint-config/-/stylelint-config-12.3.1.tgz#96905721dd15ff52ed26a97e81c165a189d44eaf"
+  integrity sha512-Bii+wGxPmXf23sfStuYPXD/os5wKPgnnUZLv0U+SiYGJTlVFTtdcxz1lk2xEI56cCRMjvTOgUWDVeGy3TeW+/w==
   dependencies:
     anymatch "^3.1.1"
     globby "^11.0.1"