Merge pull request #386 from calinoracation/no-redundant-roles-exception

jessebeach · web-flow · commit 9b9745cec000 · 2018-01-28T13:25:53.000-08:00
Allow exception for implicit navigation role in no-redundant-roles rule
diff --git a/__tests__/src/rules/no-redundant-roles-test.js b/__tests__/src/rules/no-redundant-roles-test.js
@@ -12,6 +12,7 @@
 import { RuleTester } from 'eslint';
 import parserOptionsMapper from '../../__util__/parserOptionsMapper';
 import rule from '../../../src/rules/no-redundant-roles';
+import ruleOptionsMapperFactory from '../../__util__/ruleOptionsMapperFactory';
 
 // -----------------------------------------------------------------------------
 // Tests
@@ -24,16 +25,41 @@ const expectedError = (element, implicitRole) => ({
   type: 'JSXOpeningElement',
 });
 
-ruleTester.run('no-redundant-roles', rule, {
+const ruleName = 'jsx-a11y/no-redundant-roles';
+
+const alwaysValid = [
+  { code: '<div />;' },
+  { code: '<button role="main" />' },
+  { code: '<MyComponent role="button" />' },
+  { code: '<button role={`${foo}button`} />' },
+];
+
+const neverValid = [
+  { code: '<button role="button" />', errors: [expectedError('button', 'button')] },
+  { code: '<body role="DOCUMENT" />', errors: [expectedError('body', 'document')] },
+  { code: '<button role={`${undefined}button`} />', errors: [expectedError('button', 'button')] },
+];
+
+ruleTester.run(`${ruleName}:recommended`, rule, {
   valid: [
-    { code: '<div />;' },
-    { code: '<button role="main" />' },
-    { code: '<MyComponent role="button" />' },
-    { code: '<button role={`${foo}button`} />' },
-  ].map(parserOptionsMapper),
+    ...alwaysValid,
+    { code: '<nav role="navigation" />' },
+  ]
+    .map(parserOptionsMapper),
+  invalid: neverValid
+    .map(parserOptionsMapper),
+});
+
+const noNavExceptionsOptions = { nav: [] };
+
+ruleTester.run(`${ruleName}:recommended`, rule, {
+  valid: alwaysValid
+    .map(ruleOptionsMapperFactory(noNavExceptionsOptions))
+    .map(parserOptionsMapper),
   invalid: [
-    { code: '<button role="button" />', errors: [expectedError('button', 'button')] },
-    { code: '<body role="DOCUMENT" />', errors: [expectedError('body', 'document')] },
-    { code: '<button role={`${undefined}button`} />', errors: [expectedError('button', 'button')] },
-  ].map(parserOptionsMapper),
+    ...neverValid,
+    { code: '<nav role="navigation" />', errors: [expectedError('nav', 'navigation')] },
+  ]
+    .map(ruleOptionsMapperFactory(noNavExceptionsOptions))
+    .map(parserOptionsMapper),
 });
diff --git a/docs/rules/no-redundant-roles.md b/docs/rules/no-redundant-roles.md
@@ -7,7 +7,17 @@ Some HTML elements have native semantics that are implemented by the browser. Th
 
 ## Rule details
 
-This rule takes no arguments.
+The default options for this rule allow an implicit role of `navigation` to be applied to a `nav` element as is [advised by w3](https://www.w3.org/WAI/GL/wiki/Using_HTML5_nav_element#Example:The_.3Cnav.3E_element). The options are provided as an object keyed by HTML element name; the value is an array of implicit ARIA roles that are allowed on the specified element.
+
+```
+{
+  'jsx-a11y/no-redundant-roles': [
+  'error',
+  {
+    nav: ['navigation'],
+  },
+}
+```
 
 ### Succeed
 ```jsx
diff --git a/package.json b/package.json
@@ -66,6 +66,7 @@
     "axobject-query": "^1.0.1",
     "damerau-levenshtein": "^1.0.0",
     "emoji-regex": "^6.1.0",
+    "has": "^1.0.1",
     "jsx-ast-utils": "^2.0.0"
   },
   "peerDependencies": {
diff --git a/src/rules/no-interactive-element-to-noninteractive-role.js b/src/rules/no-interactive-element-to-noninteractive-role.js
@@ -20,6 +20,7 @@ import {
 } from 'jsx-ast-utils';
 import type { JSXIdentifier } from 'ast-types-flow';
 import includes from 'array-includes';
+import has from 'has';
 import type { ESLintContext } from '../../flow/eslint';
 import type { ESLintJSXAttribute } from '../../flow/eslint-jsx';
 import isInteractiveElement from '../util/isInteractiveElement';
@@ -69,10 +70,7 @@ module.exports = {
         // Allow overrides from rule configuration for specific elements and
         // roles.
         const allowedRoles = (options[0] || {});
-        if (
-          Object.prototype.hasOwnProperty.call(allowedRoles, type)
-          && includes(allowedRoles[type], role)
-        ) {
+        if (has(allowedRoles, type) && includes(allowedRoles[type], role)) {
           return;
         }
         if (
diff --git a/src/rules/no-noninteractive-element-interactions.js b/src/rules/no-noninteractive-element-interactions.js
@@ -20,6 +20,7 @@ import {
 } from 'jsx-ast-utils';
 import type { JSXOpeningElement } from 'ast-types-flow';
 import includes from 'array-includes';
+import has from 'has';
 import type { ESLintContext } from '../../flow/eslint';
 import { arraySchema, generateObjSchema } from '../util/schemas';
 import isAbstractRole from '../util/isAbstractRole';
@@ -61,7 +62,7 @@ module.exports = {
         const config = (options[0] || {});
         const interactiveProps = config.handlers || defaultInteractiveProps;
         // Allow overrides from rule configuration for specific elements and roles.
-        if (Object.prototype.hasOwnProperty.call(config, type)) {
+        if (has(config, type)) {
           attributes = attributes.filter(attr => !includes(config[type], propName(attr)));
         }
 
diff --git a/src/rules/no-noninteractive-element-to-interactive-role.js b/src/rules/no-noninteractive-element-to-interactive-role.js
@@ -20,6 +20,7 @@ import type {
   JSXIdentifier,
 } from 'ast-types-flow';
 import includes from 'array-includes';
+import has from 'has';
 import type { ESLintContext } from '../../flow/eslint';
 import type { ESLintJSXAttribute } from '../../flow/eslint-jsx';
 import getExplicitRole from '../util/getExplicitRole';
@@ -69,10 +70,7 @@ module.exports = {
         // Allow overrides from rule configuration for specific elements and
         // roles.
         const allowedRoles = (options[0] || {});
-        if (
-          Object.prototype.hasOwnProperty.call(allowedRoles, type)
-          && includes(allowedRoles[type], role)
-        ) {
+        if (has(allowedRoles, type) && includes(allowedRoles[type], role)) {
           return;
         }
         if (
diff --git a/src/rules/no-redundant-roles.js b/src/rules/no-redundant-roles.js
@@ -2,46 +2,75 @@
  * @fileoverview Enforce explicit role property is not the
  * same as implicit/default role property on element.
  * @author Ethan Cohen <@evcohen>
+ * @flow
  */
 
 // ----------------------------------------------------------------------------
 // Rule Definition
 // ----------------------------------------------------------------------------
 
 import { elementType } from 'jsx-ast-utils';
-import { generateObjSchema } from '../util/schemas';
+import includes from 'array-includes';
+import has from 'has';
+import type { JSXOpeningElement } from 'ast-types-flow';
+import type { ESLintContext } from '../../flow/eslint';
 import getExplicitRole from '../util/getExplicitRole';
 import getImplicitRole from '../util/getImplicitRole';
 
 const errorMessage = (element, implicitRole) =>
   `The element ${element} has an implicit role of ${implicitRole}. Defining this explicitly is redundant and should be avoided.`;
 
-const schema = generateObjSchema();
+const DEFAULT_ROLE_EXCEPTIONS = { nav: ['navigation'] };
 
 module.exports = {
   meta: {
     docs: {
       url: 'https://github.com/evcohen/eslint-plugin-jsx-a11y/tree/master/docs/rules/no-redundant-roles.md',
     },
-    schema: [schema],
+    schema: [{
+      type: 'object',
+      additionalProperties: {
+        type: 'array',
+        items: {
+          type: 'string',
+        },
+        uniqueItems: true,
+      },
+    }],
   },
 
-  create: context => ({
-    JSXOpeningElement: (node) => {
-      const type = elementType(node);
-      const implicitRole = getImplicitRole(type, node.attributes);
-      const explicitRole = getExplicitRole(type, node.attributes);
-
-      if (!implicitRole || !explicitRole) {
-        return;
-      }
-
-      if (implicitRole === explicitRole) {
-        context.report({
-          node,
-          message: errorMessage(type, implicitRole.toLowerCase()),
-        });
-      }
-    },
-  }),
+  create: (context: ESLintContext) => {
+    const { options } = context;
+    return {
+      JSXOpeningElement: (node: JSXOpeningElement) => {
+        const type = elementType(node);
+        const implicitRole = getImplicitRole(type, node.attributes);
+        const explicitRole = getExplicitRole(type, node.attributes);
+
+        if (!implicitRole || !explicitRole) {
+          return;
+        }
+
+        if (implicitRole === explicitRole) {
+          const allowedRedundantRoles = (options[0] || {});
+          let redundantRolesForElement;
+
+          if (has(allowedRedundantRoles, type)) {
+            redundantRolesForElement = allowedRedundantRoles[type];
+          } else {
+            redundantRolesForElement = DEFAULT_ROLE_EXCEPTIONS[type] || [];
+          }
+
+          if (includes(redundantRolesForElement, implicitRole)) {
+            return;
+          }
+
+          context.report({
+            node,
+            message: errorMessage(type, implicitRole.toLowerCase()),
+          });
+        }
+      },
+    };
+  },
 };
