Address comments

Author: jfuchs

docs/rules/no-system-props.md

@@ -1,12 +1,12 @@
-# Disallow use of style system props (no-system-colors)
+# Disallow use of styled-system props (no-system-colors)
 
 🔧 The `--fix` option on the [ESLint CLI](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.
 
-System props are deprecated in Primer components (excluding utility components).
+[Styled-system](https://styled-system.com/table) props are deprecated in Primer components (excluding utility components).
 
 ## Rule details
 
-This rule disallows use of any styled system prop on a Primer component.
+This rule disallows the use of any styled-system prop on Primer components.
 
 \*The two non-deprecated utility components (`Box` and `Text`) are allowed to use system props.
 
@@ -26,13 +26,15 @@ import {Button} from '@primer/components'
 /* eslint primer-react/no-system-props: "error" */
 import {Box, Button, ProgressBar} from '@primer/components'
 import {Avatar} from 'some-other-library'
-
-<Button sx={{width: 200}} />,
-<Button someOtherProp="foo" />,
-
-<ProgressBar bg="howdy" />
-
-<Box width={200} />,
-
-<Avatar width={200} />,
+// Non-system props are allowed
+<Button someOtherProp="foo" />
+// If you need to override styles, use the `sx` prop instead of system props
+<Button sx={{mr: 2}} />
+// Some component prop names overlap with styled-system prop names.
+// These props are still allowed
+<ProgressBar bg="success.emphasis" />
+// Utility components like Box and Text still accept system props
+<Box mr={2} />
+// System props passed to non-Primer components are allowed
+<Avatar mr={2} />
 ```

src/rules/no-system-props.js

@@ -14,6 +14,7 @@ const excludedComponentProps = new Map([
   ['AnchoredOverlay', new Set(['width', 'height'])],
   ['Avatar', new Set(['size'])],
   ['Dialog', new Set(['width', 'height'])],
+  ['Flash', new Set(['variant'])],
   ['Label', new Set(['variant'])],
   ['ProgressBar', new Set(['bg'])],
   ['Spinner', new Set(['size'])],
@@ -26,7 +27,7 @@ module.exports = {
     fixable: 'code',
     schema: [],
     messages: {
-      noSystemProps: 'Styled-system props are deprecated ({{componentName}} called with props: {{ propNames }})'
+      noSystemProps: 'Styled-system props are deprecated ({{ componentName }} called with props: {{ propNames }})'
     }
   },
   create(context) {
@@ -36,61 +37,61 @@ module.exports = {
         if (excludedComponents.has(jsxNode.name.name)) return
 
         // Create an object mapping from prop name to the AST node for that attribute
-        const propsByNameObject = jsxNode.attributes.reduce((object, cur) => {
-          // We don't do anything about spreads for now — only named attributes:
-          if (cur.type === 'JSXAttribute') {
-            object[cur.name.name] = cur
+        const propsByNameObject = jsxNode.attributes.reduce((object, attribute) => {
+          // We don't do anything about spreads for now — only named attributes
+          if (attribute.type === 'JSXAttribute') {
+            object[attribute.name.name] = attribute
           }
 
           return object
         }, {})
 
-        // Create an array of bad prop attribute nodes
-        let badProps = Object.values(pick(propsByNameObject))
+        // Create an array of system prop attribute nodes
+        let systemProps = Object.values(pick(propsByNameObject))
 
         // Filter out our exceptional props
-        badProps = badProps.filter(prop => {
+        systemProps = systemProps.filter(prop => {
           const excludedProps = excludedComponentProps.get(jsxNode.name.name)
           if (!excludedProps) {
             return true
           }
           return !excludedProps.has(prop.name.name)
         })
 
-        if (badProps.length !== 0) {
+        if (systemProps.length !== 0) {
           context.report({
             node: jsxNode,
             messageId: 'noSystemProps',
             data: {
               componentName: jsxNode.name.name,
-              propNames: badProps.map(a => a.name.name).join(', ')
+              propNames: systemProps.map(a => a.name.name).join(', ')
             },
             fix(fixer) {
               const existingSxProp = jsxNode.attributes.find(
                 attribute => attribute.type === 'JSXAttribute' && attribute.name.name === 'sx'
               )
-              const badPropStylesMap = stylesMapFromPropNodes(badProps, context)
+              const systemPropstylesMap = stylesMapFromPropNodes(systemProps, context)
               if (existingSxProp && existingSxProp.value.expression.type !== 'ObjectExpression') {
                 return
               }
 
               const stylesToAdd = existingSxProp
-                ? excludeSxEntriesFromStyleMap(badPropStylesMap, existingSxProp)
-                : badPropStylesMap
+                ? excludeSxEntriesFromStyleMap(systemPropstylesMap, existingSxProp)
+                : systemPropstylesMap
 
               return [
-                // Remove the bad props:
-                ...badProps.map(node => fixer.remove(node)),
+                // Remove the bad props
+                ...systemProps.map(node => fixer.remove(node)),
                 ...(stylesToAdd.size > 0
                   ? [
                       existingSxProp
-                        ? // Update an existing sx prop:
+                        ? // Update an existing sx prop
                           fixer.insertTextAfter(
                             last(existingSxProp.value.expression.properties),
                             `, ${objectEntriesStringFromStylesMap(stylesToAdd)}`
                           )
                         : // Insert new sx prop
-                          fixer.insertTextAfter(last(jsxNode.attributes), sxPropTextFromStylesMap(badPropStylesMap))
+                          fixer.insertTextAfter(last(jsxNode.attributes), sxPropTextFromStylesMap(systemPropstylesMap))
                     ]
                   : [])
               ]
@@ -111,9 +112,9 @@ const objectEntriesStringFromStylesMap = styles => {
 }
 
 // Given an array of styled prop attributes, return a mapping from attribute to expression
-const stylesMapFromPropNodes = (badProps, context) => {
+const stylesMapFromPropNodes = (systemProps, context) => {
   return new Map(
-    badProps.map(a => [
+    systemProps.map(a => [
       a.name.name,
       a.value === null ? 'true' : a.value.raw || context.getSourceCode().getText(a.value.expression)
     ])