[fix] Fix checks involving the tabIndex attribute that do not account for integer literals (#48)

* Add failing tests to onclick-has-focus when tabIndexes are 0

* Reorganize onclick-has-focus failing tests

* Coerce tabIndex attributes to a number to check for their validity

Author: sohkai

src/rules/onclick-has-focus.js

@@ -15,8 +15,8 @@ import getAttributeValue from '../util/getAttributeValue';
 // ----------------------------------------------------------------------------
 
 const errorMessage = 'Elements with onClick handlers must be focusable. ' +
-  'Either set the tabIndex property (usually 0), or use an element type which ' +
-  'is inherently focusable such as `button`.';
+  'Either set the tabIndex property to a valid value (usually 0), or use ' +
+  'an element type which is inherently focusable such as `button`.';
 
 module.exports = context => ({
   JSXOpeningElement: node => {
@@ -31,7 +31,7 @@ module.exports = context => ({
       return;
     } else if (isInteractiveElement(type, attributes)) {
       return;
-    } else if (getAttributeValue(getAttribute(attributes, 'tabIndex'))) {
+    } else if (!isNaN(Number(getAttributeValue(getAttribute(attributes, 'tabIndex'))))) {
       return;
     }
 

src/util/isInteractiveElement.js

@@ -1,17 +1,17 @@
 'use strict';
 
 import getAttribute from './getAttribute';
-import { getLiteralAttributeValue } from './getAttributeValue';
+import getAttributeValue, { getLiteralAttributeValue } from './getAttributeValue';
 import DOMElements from './attributes/DOM';
 
 
 
 // Map of tagNames to functions that return whether that element is interactive or not.
 const interactiveMap = {
   a: attributes => {
-    const href = getAttribute(attributes, 'href');
-    const tabIndex = getAttribute(attributes, 'tabIndex');
-    return (Boolean(href) || (!href && Boolean(tabIndex)));
+    const href = getAttributeValue(getAttribute(attributes, 'href'));
+    const tabIndex = getAttributeValue(getAttribute(attributes, 'tabIndex'));
+    return Boolean(href) || !isNaN(Number(tabIndex));
   },
   // This is same as `a` interactivity function
   area: attributes => interactiveMap.a(attributes),

tests/src/rules/onclick-has-focus.js

@@ -27,8 +27,8 @@ const ruleTester = new RuleTester();
 
 const  expectedError  = {
   message: 'Elements with onClick handlers must be focusable. ' +
-    'Either set the tabIndex property (usually 0), or use an element type which ' +
-    'is inherently focusable such as `button`.',
+    'Either set the tabIndex property to a valid value (usually 0), ' +
+    'or use an element type which is inherently focusable such as `button`.',
   type: 'JSXOpeningElement'
 };
 
@@ -46,20 +46,23 @@ ruleTester.run('onclick-has-focus', rule, {
     { code: '<div aria-hidden={2 >= 1} onClick={() => void 0} />', parserOptions },
     { code: '<input type="text" onClick={() => void 0} />', parserOptions },
     { code: '<input type="hidden" onClick={() => void 0} tabIndex="-1" />', parserOptions },
+    { code: '<input type="hidden" onClick={() => void 0} tabIndex={-1} />', parserOptions },
     { code: '<input onClick={() => void 0} />', parserOptions },
     { code: '<button onClick={() => void 0} className="foo" />', parserOptions },
     { code: '<option onClick={() => void 0} className="foo" />', parserOptions },
     { code: '<select onClick={() => void 0} className="foo" />', parserOptions },
     { code: '<area href="#" onClick={() => void 0} className="foo" />', parserOptions },
     { code: '<textarea onClick={() => void 0} className="foo" />', parserOptions },
     { code: '<a tabIndex="0" onClick={() => void 0} />', parserOptions },
+    { code: '<a tabIndex={0} onClick={() => void 0} />', parserOptions },
     { code: '<a role="button" href="#" onClick={() => void 0} />', parserOptions },
     { code: '<a onClick={() => void 0} href="http://x.y.z" />', parserOptions },
     { code: '<a onClick={() => void 0} href="http://x.y.z" tabIndex="0" />', parserOptions },
+    { code: '<a onClick={() => void 0} href="http://x.y.z" tabIndex={0} />', parserOptions },
     { code: '<TestComponent onClick={doFoo} />', parserOptions },
     { code: '<input onClick={() => void 0} type="hidden" />;', parserOptions },
     { code: '<span onClick="doSomething();" tabIndex="0">Click me!</span>', parserOptions },
-    { code: '<span onClick="doSomething();" tabIndex="0">Click me!</span>', parserOptions },
+    { code: '<span onClick="doSomething();" tabIndex={0}>Click me!</span>', parserOptions },
     { code: '<span onClick="doSomething();" tabIndex="-1">Click me too!</span>', parserOptions },
     { code: '<a href="javascript:void(0);" onClick="doSomething();">Click ALL the things!</a>', parserOptions },
     { code: '<Foo.Bar onClick={() => void 0} aria-hidden={false} />;', parserOptions },
@@ -69,17 +72,22 @@ ruleTester.run('onclick-has-focus', rule, {
   invalid: [
     { code: '<span onClick="submitForm();">Submit</span>', errors: [ expectedError ], parserOptions },
     { code: '<span onClick="submitForm();" tabIndex={undefined}>Submit</span>', errors: [ expectedError ], parserOptions },
+    { code: '<span onClick="submitForm();" tabIndex="bad">Submit</span>', errors: [ expectedError ], parserOptions },
     { code: '<a onClick="showNextPage();">Next page</a>', errors: [ expectedError ], parserOptions },
+    { code: '<a onClick="showNextPage();" tabIndex={undefined}>Next page</a>', errors: [ expectedError ], parserOptions },
+    { code: '<a onClick="showNextPage();" tabIndex="bad">Next page</a>', errors: [ expectedError ], parserOptions },
+    { code: '<a onClick={() => void 0} />', errors: [ expectedError ], parserOptions },
     { code: '<area onClick={() => void 0} className="foo" />', errors: [ expectedError ], parserOptions },
     { code: '<div onClick={() => void 0} />;', errors: [ expectedError ], parserOptions },
+    { code: '<div onClick={() => void 0} tabIndex={undefined} />;', errors: [ expectedError ], parserOptions },
+    { code: '<div onClick={() => void 0} tabIndex="bad" />;', errors: [ expectedError ], parserOptions },
     { code: '<div onClick={() => void 0} role={undefined} />;', errors: [ expectedError ], parserOptions },
+    { code: '<div onClick={() => void 0} aria-hidden={false} />;', errors: [ expectedError ], parserOptions },
     { code: '<div onClick={() => void 0} {...props} />;', errors: [ expectedError ], parserOptions },
     { code: '<section onClick={() => void 0} />;', errors: [ expectedError ], parserOptions },
     { code: '<main onClick={() => void 0} />;', errors: [ expectedError ], parserOptions },
     { code: '<article onClick={() => void 0} />;', errors: [ expectedError ], parserOptions },
     { code: '<header onClick={() => void 0} />;', errors: [ expectedError ], parserOptions },
-    { code: '<footer onClick={() => void 0} />;', errors: [ expectedError ], parserOptions },
-    { code: '<div onClick={() => void 0} aria-hidden={false} />;', errors: [ expectedError ], parserOptions },
-    { code: '<a onClick={() => void 0} />', errors: [ expectedError ], parserOptions }
+    { code: '<footer onClick={() => void 0} />;', errors: [ expectedError ], parserOptions }
   ]
 });