feat: role-supports-aria-props rule (no aria-label-misuse)

smockle · web-flow · commit cc51b62b54e6 · 2023-01-10T15:36:50.000-05:00
diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
@@ -1,32 +1,30 @@
 // For format details, see https://aka.ms/devcontainer.json. For config options, see the README at:
 // https://github.com/microsoft/vscode-dev-containers/tree/v0.222.0/containers/javascript-node
 {
-	"name": "Node.js",
-	"build": {
-		"dockerfile": "Dockerfile",
-		// Update 'VARIANT' to pick a Node version: 16, 14, 12.
-		// Append -bullseye or -buster to pin to an OS version.
-		// Use -bullseye variants on local arm64/Apple Silicon.
-		"args": { "VARIANT": "16" }
-	},
+  "name": "Node.js",
+  "build": {
+    "dockerfile": "Dockerfile",
+    // Update 'VARIANT' to pick a Node version: 16, 14, 12.
+    // Append -bullseye or -buster to pin to an OS version.
+    // Use -bullseye variants on local arm64/Apple Silicon.
+    "args": {"VARIANT": "16"}
+  },
 
-	// Set *default* container specific settings.json values on container create.
-	"settings": {},
+  // Set *default* container specific settings.json values on container create.
+  "settings": {},
 
-	// Add the IDs of extensions you want installed when the container is created.
-	"extensions": [
-		"dbaeumer.vscode-eslint"
-	],
+  // Add the IDs of extensions you want installed when the container is created.
+  "extensions": ["dbaeumer.vscode-eslint"],
 
-	// Use 'forwardPorts' to make a list of ports inside the container available locally.
-	// "forwardPorts": [],
+  // Use 'forwardPorts' to make a list of ports inside the container available locally.
+  // "forwardPorts": [],
 
-	// Use 'postCreateCommand' to run commands after the container is created.
-	// "postCreateCommand": "yarn install",
+  // Use 'postCreateCommand' to run commands after the container is created.
+  // "postCreateCommand": "yarn install",
 
-	// Comment out to connect as root instead. More info: https://aka.ms/vscode-remote/containers/non-root.
-	"remoteUser": "node",
-	"features": {
-		"git": "latest"
-	}
+  // Comment out to connect as root instead. More info: https://aka.ms/vscode-remote/containers/non-root.
+  "remoteUser": "node",
+  "features": {
+    "git": "latest"
+  }
 }
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -1,7 +1,7 @@
 module.exports = {
   root: true,
   parserOptions: {
-    ecmaVersion: 2020,
+    ecmaVersion: 13,
   },
   env: {
     es6: true,
diff --git a/README.md b/README.md
@@ -54,8 +54,8 @@ For each component, you may specify a `default` and/or `props`. `default` may ma
   "settings": {
     "github": {
       "components": {
-        "Box": { "default": "p" },
-        "Link": { "props": {"as": { "undefined": "a", "a": "a", "button": "button"}}},
+        "Box": {"default": "p"},
+        "Link": {"props": {"as": {"undefined": "a", "a": "a", "button": "button"}}}
       }
     }
   }
@@ -94,3 +94,4 @@ This config will be interpreted in the following way:
 #### Accessibility-focused rules (prefixed with a11y)
 
 - [No Generic Link Text](./docs/rules/a11y-no-generic-link-text.md)
+- [Role Supports ARIA Props](./docs/rules/role-supports-aria-props.md)
diff --git a/docs/rules/a11y-no-generic-link-text.md b/docs/rules/a11y-no-generic-link-text.md
@@ -63,14 +63,18 @@ Please perform browser tests and spot checks:
 ```
 
 ```jsx
-<a href="github.com/about" aria-label="Why dogs are awesome">Read more</a>
+<a href="github.com/about" aria-label="Why dogs are awesome">
+  Read more
+</a>
 ```
 
 ```jsx
-<a href="github.com/about" aria-describedby="element123">Read more</a>
+<a href="github.com/about" aria-describedby="element123">
+  Read more
+</a>
 ```
 
-### **Correct** code for this rule  👍
+### **Correct** code for this rule 👍
 
 ```jsx
 <a href="github.com/about">Learn more about GitHub</a>
diff --git a/docs/rules/array-foreach.md b/docs/rules/array-foreach.md
@@ -93,6 +93,7 @@ for (const el of els) {
 ```
 
 Use [`entries()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/entries) to get access to the index:
+
 ```js
 for (const [i, el] of els.entries()) {
   el.name = `Element ${i}`
diff --git a/docs/rules/authenticity-token.md b/docs/rules/authenticity-token.md
@@ -33,7 +33,7 @@ on('click', '.js-my-button', function (e) {
 
   fetch(form.action, {
     method: form.method,
-    body: new FormData(form)
+    body: new FormData(form),
   }).then(function () {
     alert('Success!')
   })
@@ -52,7 +52,7 @@ An alternate, but less preferred approach is to include the a signed CSRF url in
 on('click', '.js-my-button', function (e) {
   csrfRequest(this.getAttribute('data-url'), {
     method: 'PUT',
-    body: data
+    body: data,
   }).then(function () {
     alert('Success!')
   })
diff --git a/docs/rules/no-useless-passive.md b/docs/rules/no-useless-passive.md
@@ -21,7 +21,7 @@ window.addEventListener(
   () => {
     console.log('Scroll event fired!')
   },
-  {passive: true}
+  {passive: true},
 )
 ```
 
diff --git a/docs/rules/prefer-observers.md b/docs/rules/prefer-observers.md
@@ -34,7 +34,7 @@ window.addEventListener('scroll', () => {
   const isIntersecting = checkIfElementIntersects(
     element.getBoundingClientRect(),
     window.innerHeight,
-    document.clientHeight
+    document.clientHeight,
   )
   element.classList.toggle('intersecting', isIntersecting)
 })
diff --git a/docs/rules/require-passive-events.md b/docs/rules/require-passive-events.md
@@ -28,7 +28,7 @@ window.addEventListener(
   () => {
     /* ... */
   },
-  {passive: true}
+  {passive: true},
 )
 ```
 
diff --git a/docs/rules/role-supports-aria-props.md b/docs/rules/role-supports-aria-props.md
@@ -0,0 +1,74 @@
+# Role Supports ARIA Props
+
+## Rule Details
+
+This rule enforces that elements with explicit or implicit roles defined contain only `aria-*` properties supported by that `role`.
+
+For example, this rule aims to discourage common misuse of the `aria-label` and `aria-labelledby` attribute. `aria-label` and `aria-labelledby` support is only guaranteed on interactive elements like `button` or `a`, or on static elements like `div` and `span` with a permitted `role`. This rule will allow `aria-label` and `aria-labelledby` usage on `div` and `span` elements if it set to a role other than the ones listed in [WSC: a list of ARIA roles which cannot be named](https://w3c.github.io/aria/#namefromprohibited). This rule will never permit usage of `aria-label` and `aria-labelledby` on `h1`, `h2`, `h3`, `h4`, `h5`, `h6`, `strong`, `i`, `p`, `b`, or `code`.
+
+### "Help! I'm trying to set a tooltip on a static element and this rule flagged it!"
+
+Please do not use tooltips on static elements. It is a highly discouraged, inaccessible pattern.
+See [Primer: Tooltip alternatives](https://primer.style/design/accessibility/tooltip-alternatives) for what to do instead.
+
+### Resources
+
+- [w3c/aria Consider prohibiting author naming certain roles #833](https://github.com/w3c/aria/issues/833)
+- [Not so short note on aria-label usage - Big Table Edition](https://html5accessibility.com/stuff/2020/11/07/not-so-short-note-on-aria-label-usage-big-table-edition/)
+- [Your tooltips are bogus](https://heydonworks.com/article/your-tooltips-are-bogus/)
+- [Primer: Tooltip alternatives](https://primer.style/design/accessibility/tooltip-alternatives)
+
+### Disclaimer
+
+There are conflicting resources and opinions on what elements should support these naming attributes. For now, this rule will operate under a relatively simple heuristic aimed to minimize false positives. This may have room for future improvements. Learn more at [W3C Name Calcluation](https://w3c.github.io/aria/#namecalculation).
+
+### **Incorrect** code for this rule 👎
+
+```erb
+<span class="tooltipped" aria-label="This is a tooltip">I am some text.</span>
+```
+
+```erb
+<span aria-label="This is some content that will completely override the button content">Please be careful of the following.</span>
+```
+
+```erb
+<div aria-labelledby="heading1">Goodbye</div>
+```
+
+```erb
+<h1 aria-label="This will override the page title completely">Page title</h1>
+```
+
+### **Correct** code for this rule 👍
+
+```erb
+<button aria-label="Close">
+  <svg src="closeIcon"></svg>
+</button>
+```
+
+```erb
+<button aria-label="Bold" aria-describedby="tooltip1">
+  <svg src="boldIcon"></svg>
+</button>
+<p id="tooltip1" class="tooltip">Add bold text or turn selection into bold text</p>
+```
+
+```erb
+<span>Hello</span>
+```
+
+```erb
+<div>Goodbye</div>
+```
+
+```erb
+<h1>Page title</h1>
+```
+
+```erb
+<div role="dialog" aria-labelledby="dialogHeading">
+  <h1 id="dialogHeading">Heading</h1>
+</div>
+```
diff --git a/lib/configs/react.js b/lib/configs/react.js
@@ -9,5 +9,7 @@ module.exports = {
   extends: ['plugin:jsx-a11y/recommended'],
   rules: {
     'github/a11y-no-generic-link-text': 'error',
+    'github/role-supports-aria-props': 'error',
+    'jsx-a11y/role-supports-aria-props': 'off',
   },
 }
diff --git a/lib/index.js b/lib/index.js
@@ -17,6 +17,7 @@ module.exports = {
     'no-then': require('./rules/no-then'),
     'no-useless-passive': require('./rules/no-useless-passive'),
     'prefer-observers': require('./rules/prefer-observers'),
+    'role-supports-aria-props': require('./rules/role-supports-aria-props'),
     'require-passive-events': require('./rules/require-passive-events'),
     'unescaped-html-literal': require('./rules/unescaped-html-literal'),
   },
diff --git a/lib/rules/role-supports-aria-props.js b/lib/rules/role-supports-aria-props.js
@@ -0,0 +1,101 @@
+// @ts-check
+const {aria, elementRoles, roles} = require('aria-query')
+const {getProp, getPropValue, propName} = require('jsx-ast-utils')
+const {getElementType} = require('../utils/get-element-type')
+const ObjectMap = require('../utils/object-map')
+
+// Clean-up `elementRoles` from `aria-query`
+const elementRolesMap = new ObjectMap()
+for (const [key, value] of elementRoles.entries()) {
+  // - Remove unused `constraints` key
+  delete key.constraints
+  key.attributes = key.attributes?.filter(attribute => !('constraints' in attribute))
+  // - Remove empty `attributes` key
+  if (!key.attributes || key.attributes?.length === 0) {
+    delete key.attributes
+  }
+  elementRolesMap.set(key, value)
+}
+// - Remove insufficiently-disambiguated `menuitem` entry
+elementRolesMap.delete({name: 'menuitem'})
+// - Disambiguate `menuitem` and `menu` roles by `type`
+elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'command'}]}, ['menuitem'])
+elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'radio'}]}, ['menuitemradio'])
+elementRolesMap.set({name: 'menuitem', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])
+elementRolesMap.set({name: 'menu', attributes: [{name: 'type', value: 'toolbar'}]}, ['toolbar'])
+
+module.exports = {
+  meta: {
+    docs: {
+      description:
+        'Enforce that elements with explicit or implicit roles defined contain only `aria-*` properties supported by that `role`.',
+      url: require('../url')(module),
+    },
+    schema: [],
+  },
+
+  create(context) {
+    return {
+      JSXOpeningElement(node) {
+        // Assemble a key for looking-up the element’s role in the `elementRolesMap`
+        // - Get the element’s name
+        const key = {name: getElementType(context, node)}
+        // - Get the element’s disambiguating attributes
+        for (const prop of ['aria-expanded', 'type', 'size', 'role', 'href', 'multiple', 'scope']) {
+          // - Only provide `aria-expanded` when it’s required for disambiguation
+          if (prop === 'aria-expanded' && key.name !== 'summary') continue
+          const value = getPropValue(getProp(node.attributes, prop))
+          if (value) {
+            if (!('attributes' in key)) {
+              key.attributes = []
+            }
+            if (prop === 'href') {
+              key.attributes.push({name: prop})
+            } else {
+              key.attributes.push({name: prop, value})
+            }
+          }
+        }
+        // Get the element’s explicit or implicit role
+        const role = getPropValue(getProp(node.attributes, 'role')) ?? elementRolesMap.get(key)?.[0]
+
+        // Return early if role could not be determined
+        if (!role) return
+
+        // Get allowed ARIA attributes:
+        // - From the role itself
+        let allowedProps = Object.keys(roles.get(role)?.props || {})
+        // - From parent roles
+        for (const parentRole of roles.get(role)?.superClass.flat() ?? []) {
+          allowedProps = allowedProps.concat(Object.keys(roles.get(parentRole)?.props || {}))
+        }
+        // Dedupe, for performance
+        allowedProps = Array.from(new Set(allowedProps))
+
+        // Get prohibited ARIA attributes:
+        // - From the role itself
+        let prohibitedProps = roles.get(role)?.prohibitedProps || []
+        // - From parent roles
+        for (const parentRole of roles.get(role)?.superClass.flat() ?? []) {
+          prohibitedProps = prohibitedProps.concat(roles.get(parentRole)?.prohibitedProps || [])
+        }
+        // - From comparing allowed vs all ARIA attributes
+        prohibitedProps = prohibitedProps.concat(aria.keys().filter(x => !allowedProps.includes(x)))
+        // Dedupe, for performance
+        prohibitedProps = Array.from(new Set(prohibitedProps))
+
+        for (const prop of node.attributes) {
+          // Return early if prohibited ARIA attribute is set to an ignorable value
+          if (getPropValue(prop) == null || prop.type === 'JSXSpreadAttribute') return
+
+          if (prohibitedProps?.includes(propName(prop))) {
+            context.report({
+              node,
+              message: `The attribute ${propName(prop)} is not supported by the role ${role}.`,
+            })
+          }
+        }
+      },
+    }
+  },
+}
diff --git a/lib/utils/object-map.js b/lib/utils/object-map.js
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
diff --git a/tests/role-supports-aria-props.js b/tests/role-supports-aria-props.js
diff --git a/tests/utils/object-map.js b/tests/utils/object-map.js

Original file line number	Diff line number	Diff line change
@@ -54,8 +54,8 @@ For each component, you may specify a `default` and/or `props`. `default` may ma
`54`	`54`	`"settings": {`
`55`	`55`	`"github": {`
`56`	`56`	`"components": {`
`57`		`- "Box": { "default": "p" },`
`58`		`- "Link": { "props": {"as": { "undefined": "a", "a": "a", "button": "button"}}},`
	`57`	`+ "Box": {"default": "p"},`
	`58`	`+ "Link": {"props": {"as": {"undefined": "a", "a": "a", "button": "button"}}}`
`59`	`59`	`}`
`60`	`60`	`}`
`61`	`61`	`}`
`@@ -94,3 +94,4 @@ This config will be interpreted in the following way:`
`94`	`94`	`#### Accessibility-focused rules (prefixed with a11y)`
`95`	`95`
`96`	`96`	`- [No Generic Link Text](./docs/rules/a11y-no-generic-link-text.md)`
	`97`	`+- [Role Supports ARIA Props](./docs/rules/role-supports-aria-props.md)`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ window.addEventListener(`
`21`	`21`	`() => {`
`22`	`22`	`console.log('Scroll event fired!')`
`23`	`23`	`},`
`24`		`- {passive: true}`
	`24`	`+ {passive: true},`
`25`	`25`	`)`
`26`	`26`	```
`27`	`27`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ window.addEventListener('scroll', () => {`
`34`	`34`	`const isIntersecting = checkIfElementIntersects(`
`35`	`35`	`element.getBoundingClientRect(),`
`36`	`36`	`window.innerHeight,`
`37`		`- document.clientHeight`
	`37`	`+ document.clientHeight,`
`38`	`38`	`)`
`39`	`39`	`element.classList.toggle('intersecting', isIntersecting)`
`40`	`40`	`})`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ window.addEventListener(`
`28`	`28`	`() => {`
`29`	`29`	`/* ... */`
`30`	`30`	`},`
`31`		`- {passive: true}`
	`31`	`+ {passive: true},`
`32`	`32`	`)`
`33`	`33`	```
`34`	`34`