Various bugfixes.

Ethan Cohen · Ethan Cohen · commit 346d70b2829d · 2016-04-05T14:17:37.000-07:00
Fixes #8 Fixes #9 Fixes first example in #6
diff --git a/src/rules/img-uses-alt.js b/src/rules/img-uses-alt.js
@@ -9,6 +9,7 @@
 // ----------------------------------------------------------------------------
 
 import hasAttribute from '../util/hasAttribute';
+import getAttributeValue from '../util/getAttributeValue';
 import getNodeType from '../util/getNodeType';
 
 const errorMessage = type => `${type} elements must have an alt tag.`;
@@ -24,9 +25,11 @@ module.exports = context => ({
     }
 
     const hasAltProp = hasAttribute(node.attributes, 'alt');
+    const altProp = hasAltProp ? getAttributeValue(hasAltProp) : undefined;
+    const isInvalid = hasAltProp === false || Boolean(altProp) === false;
 
     // alt must have a value.
-    if (hasAltProp === false || hasAltProp === null) {
+    if (isInvalid) {
       context.report({
         node,
         message: errorMessage(nodeType)
diff --git a/src/rules/label-uses-for.js b/src/rules/label-uses-for.js
@@ -9,6 +9,7 @@
 // ----------------------------------------------------------------------------
 
 import hasAttribute from '../util/hasAttribute';
+import getAttributeValue from '../util/getAttributeValue';
 import getNodeType from '../util/getNodeType';
 
 const errorMessage = 'Form controls using a label to identify them must be ' +
@@ -24,9 +25,11 @@ module.exports = context => ({
       return;
     }
 
-    const hasHtmlForAttr = hasAttribute(node.attributes, 'htmlFor');
+    const htmlForAttr = hasAttribute(node.attributes, 'htmlFor');
+    const htmlForValue = getAttributeValue(htmlForAttr);
+    const isInvalid = htmlForAttr === false || htmlForValue === null || htmlForValue === undefined;
 
-    if (hasHtmlForAttr === false) {
+    if (isInvalid) {
       context.report({
         node,
         message: errorMessage
diff --git a/src/rules/mouse-events-map-to-key-events.js b/src/rules/mouse-events-map-to-key-events.js
@@ -10,6 +10,7 @@
 // ----------------------------------------------------------------------------
 
 import hasAttribute from '../util/hasAttribute';
+import getAttributeValue from '../util/getAttributeValue';
 
 const mouseOverErrorMessage = 'onMouseOver must be accompanied by onFocus for accessibility.';
 const mouseOutErrorMessage = 'onMouseOut must be accompanied by onBlur for accessibility.';
@@ -20,9 +21,13 @@ module.exports = context => ({
 
     // Check onmouseover / onfocus pairing.
     const hasOnMouseOver = hasAttribute(attributes, 'onMouseOver');
-    if (Boolean(hasOnMouseOver) === true) {
+    const onMouseOverValue = getAttributeValue(hasOnMouseOver);
+
+    if (Boolean(hasOnMouseOver) === true && (onMouseOverValue !== null || onMouseOverValue !== undefined)) {
       const hasOnFocus = hasAttribute(attributes, 'onFocus');
-      if (hasOnFocus === false) {
+      const onFocusValue = getAttributeValue(hasOnFocus);
+
+      if (hasOnFocus === false || onFocusValue === null || onFocusValue === undefined) {
         context.report({
           node,
           message: mouseOverErrorMessage
@@ -32,9 +37,12 @@ module.exports = context => ({
 
     // Checkout onmouseout / onblur pairing
     const hasOnMouseOut = hasAttribute(attributes, 'onMouseOut');
-    if (Boolean(hasOnMouseOut) === true) {
+    const onMouseOutValue = getAttributeValue(hasOnMouseOut);
+    if (Boolean(hasOnMouseOut) === true && (onMouseOutValue !== null || onMouseOutValue !== undefined)) {
       const hasOnBlur = hasAttribute(attributes, 'onBlur');
-      if (hasOnBlur === false) {
+      const onBlurValue = getAttributeValue(hasOnBlur);
+
+      if (hasOnBlur === false || onBlurValue === null || onBlurValue === undefined) {
         context.report({
           node,
           message: mouseOutErrorMessage
diff --git a/src/rules/no-access-key.js b/src/rules/no-access-key.js
@@ -9,16 +9,18 @@
 // ----------------------------------------------------------------------------
 
 import hasAttribute from '../util/hasAttribute';
+import getAttributeValue from '../util/getAttributeValue';
 
-const errorMessage = 'No access key attribute allowed. Incosistencies ' +
+const errorMessage = 'No access key attribute allowed. Inconsistencies ' +
   'between keyboard shortcuts and keyboard comments used by screenreader ' +
   'and keyboard only users create a11y complications.';
 
 module.exports = context => ({
   JSXOpeningElement: node => {
     const hasAccessKey = hasAttribute(node.attributes, 'accesskey');
+    const accessKeyValue = getAttributeValue(hasAccessKey);
 
-    if (Boolean(hasAccessKey) === true) {
+    if (Boolean(hasAccessKey) === true && Boolean(accessKeyValue) === true) {
       context.report({
         node,
         message: errorMessage
diff --git a/src/rules/no-hash-href.js b/src/rules/no-hash-href.js
@@ -9,6 +9,7 @@
 // ----------------------------------------------------------------------------
 
 import hasAttribute from '../util/hasAttribute';
+import getAttributeValue from '../util/getAttributeValue';
 import getNodeType from '../util/getNodeType';
 
 const errorMessage = 'Links must not point to "#". Use a more descriptive href or use a button instead.';
@@ -24,8 +25,9 @@ module.exports = context => ({
     }
 
     const href = hasAttribute(node.attributes, 'href');
+    const value = getAttributeValue(href);
 
-    if (href === '#') {
+    if (href && value === '#') {
       context.report({
         node,
         message: errorMessage
diff --git a/src/rules/onclick-uses-role.js b/src/rules/onclick-uses-role.js
@@ -8,6 +8,7 @@
 import isHiddenFromScreenReader from '../util/isHiddenFromScreenReader';
 import isInteractiveElement from '../util/isInteractiveElement';
 import hasAttribute from '../util/hasAttribute';
+import getAttributeValue from '../util/getAttributeValue';
 import getNodeType from '../util/getNodeType';
 
 // ----------------------------------------------------------------------------
@@ -26,7 +27,8 @@ module.exports = context => ({
 
     const isVisible = isHiddenFromScreenReader(attributes) === false;
     const isNonInteractive = isInteractiveElement(getNodeType(node), attributes) === false;
-    const noRoleAttribute = hasAttribute(attributes, 'role') === false;
+    const roleAttribute = hasAttribute(attributes, 'role');
+    const noRoleAttribute = roleAttribute === false || Boolean(getAttributeValue(roleAttribute)) === false;
 
     // Visible, non-interactive elements require role attribute.
     if (isVisible && isNonInteractive && noRoleAttribute) {
diff --git a/src/rules/redundant-alt.js b/src/rules/redundant-alt.js
@@ -9,6 +9,7 @@
 // ----------------------------------------------------------------------------
 
 import hasAttribute from '../util/hasAttribute';
+import getAttributeValue from '../util/getAttributeValue';
 import isHiddenFromScreenReader from '../util/isHiddenFromScreenReader';
 import getNodeType from '../util/getNodeType';
 
@@ -21,6 +22,11 @@ const REDUNDANT_WORDS = [
 const errorMessage = 'Redundant alt attribute. Screen-readers already announce `img` tags as an image. ' +
   'You don\'t need to use the words `image`, `photo,` or `picture` in the alt prop.';
 
+const validTypes = [
+  'LITERAL',
+  'TEMPLATELITERAL'
+];
+
 module.exports = context => ({
   JSXOpeningElement: node => {
     const type = getNodeType(node);
@@ -29,11 +35,26 @@ module.exports = context => ({
     }
 
     const altProp = hasAttribute(node.attributes, 'alt');
+    // Return if alt prop is not present.
+    if (altProp === false) {
+      return;
+    }
+
+    // Only check literals, as we should not enforce variable names :P
+    const normalizedType = altProp.value && altProp.value.type.toUpperCase() === 'JSXEXPRESSIONCONTAINER' ?
+      altProp.value.expression.type.toUpperCase() :
+      altProp.value.type.toUpperCase();
+
+    if (validTypes.indexOf(normalizedType) === -1) {
+      return;
+    }
+
+    const value = getAttributeValue(altProp);
     const isVisible = isHiddenFromScreenReader(node.attributes) === false;
 
-    if (Boolean(altProp) && typeof altProp === 'string' && isVisible) {
+    if (Boolean(value) && typeof value === 'string' && isVisible) {
       const hasRedundancy = REDUNDANT_WORDS
-        .some(word => Boolean(altProp.match(new RegExp(`(?!{)${word}(?!})`, 'gi'))));
+        .some(word => Boolean(value.match(new RegExp(`(?!{)${word}(?!})`, 'gi'))));
 
       if (hasRedundancy === true) {
         context.report({
diff --git a/src/util/hasAttribute.js b/src/util/hasAttribute.js
@@ -1,17 +1,14 @@
 'use strict';
 
-import getAttributeValue from './getAttributeValue';
 
 /**
- * Returns the value of the attribute or false, indicating the attribute
- * is not present on the JSX opening element. This skips over spread attributes
+ * Returns the JSXAttribute itself or false, indicating the attribute
+ * is not present on the JSXOpeningElement. This skips over spread attributes
  * as the purpose of this linter is to do hard checks of explicit JSX props.
  *
- * This treats undefined values as missing props, as they will not be used for
- * rendering on elements that live closest to the DOM (pure html JSX elements).
  */
 const hasAttribute = (attributes, attribute) => {
-  let value = false;
+  let nodeAttribute = undefined;
 
   const hasAttr = attributes.some(attr => {
     // If the attributes contain a spread attribute, then skip.
@@ -21,16 +18,14 @@ const hasAttribute = (attributes, attribute) => {
 
     // Normalize.
     if (attr.name.name.toUpperCase() === attribute.toUpperCase()) {
-      value = getAttributeValue(attr);
-
-      // If the value is undefined, it doesn't really have the attribute.
-      return value !== undefined;
+      nodeAttribute = attr;
+      return true;
     }
 
     return false;
   });
 
-  return hasAttr ? value : false;
+  return hasAttr ? nodeAttribute : false;
 };
 
 export default hasAttribute;
diff --git a/src/util/isHiddenFromScreenReader.js b/src/util/isHiddenFromScreenReader.js
@@ -1,6 +1,7 @@
 'use strict';
 
 import hasAttribute from './hasAttribute';
+import getAttributeValue from './getAttributeValue';
 
 /**
  * Returns boolean indicating that the aria-hidden prop
@@ -9,8 +10,8 @@ import hasAttribute from './hasAttribute';
  * <div aria-hidden /> is equivalent to the DOM as <div aria-hidden=true />.
  */
 const isHiddenFromScreenReader = attributes => {
-  const hasAriaHidden = hasAttribute(attributes, 'aria-hidden');
-  return hasAriaHidden && (hasAriaHidden === true || hasAriaHidden === null);
+  const ariaHidden = getAttributeValue(hasAttribute(attributes, 'aria-hidden'));
+  return ariaHidden === true || ariaHidden === null;
 };
 
 export default isHiddenFromScreenReader;
diff --git a/src/util/isInteractiveElement.js b/src/util/isInteractiveElement.js
@@ -1,6 +1,7 @@
 'use strict';
 
 import hasAttribute from './hasAttribute';
+import getAttributeValue from './getAttributeValue';
 
 const interactiveMap = {
   a: attributes => {
@@ -10,8 +11,8 @@ const interactiveMap = {
   },
   button: () => true,
   input: attributes => {
-    const hasTypeAttr = hasAttribute(attributes, 'type');
-    return hasTypeAttr ? hasTypeAttr.toUpperCase() !== 'HIDDEN' : true;
+    const typeAttr = getAttributeValue(hasAttribute(attributes, 'type'));
+    return typeAttr ? typeAttr.toUpperCase() !== 'HIDDEN' : true;
   },
   option: () => true,
   select: () => true,
diff --git a/tests/src/rules/no-access-key.js b/tests/src/rules/no-access-key.js
@@ -26,7 +26,7 @@ const parserOptions = {
 const ruleTester = new RuleTester();
 
 const expectedError = {
-  message: 'No access key attribute allowed. Incosistencies ' +
+  message: 'No access key attribute allowed. Inconsistencies ' +
   'between keyboard shortcuts and keyboard comments used by screenreader ' +
   'and keyboard only users create a11y complications.',
   type: 'JSXOpeningElement'
diff --git a/tests/src/rules/redundant-alt.js b/tests/src/rules/redundant-alt.js
@@ -55,7 +55,8 @@ ruleTester.run('redundant-alt', rule, {
     { code: '<img alt={() => {}} />', parserOptions },
     { code: '<img alt={function(e){}} />', parserOptions },
     { code: '<img aria-hidden={false} alt="Doing cool things." />', parserOptions },
-    { code: '<UX.Layout>test</UX.Layout>', parserOptions }
+    { code: '<UX.Layout>test</UX.Layout>', parserOptions },
+    { code: '<img alt={imageAlt} />', parserOptions }
   ],
   invalid: [
     { code: '<img alt="Photo of friend." />;', errors: [ expectedError ], parserOptions },
@@ -71,11 +72,11 @@ ruleTester.run('redundant-alt', rule, {
     { code: '<img alt="photo" {...this.props} />', errors: [ expectedError ], parserOptions },
     { code: '<img alt="image" {...this.props} />', errors: [ expectedError ], parserOptions },
     { code: '<img alt="picture" {...this.props} />', errors: [ expectedError ], parserOptions },
-    { code: '<img alt="{`picture doing ${things}`}" {...this.props} />', errors: [ expectedError ], parserOptions },
-    { code: '<img alt="{`photo doing ${things}`}" {...this.props} />', errors: [ expectedError ], parserOptions },
-    { code: '<img alt="{`image doing ${things}`}" {...this.props} />', errors: [ expectedError ], parserOptions },
-    { code: '<img alt="{`picture doing ${picture}`}" {...this.props} />', errors: [ expectedError ], parserOptions },
-    { code: '<img alt="{`photo doing ${photo}`}" {...this.props} />', errors: [ expectedError ], parserOptions },
-    { code: '<img alt="{`image doing ${image}`}" {...this.props} />', errors: [ expectedError ], parserOptions }
+    { code: '<img alt={`picture doing ${things}`} {...this.props} />', errors: [ expectedError ], parserOptions },
+    { code: '<img alt={`photo doing ${things}`} {...this.props} />', errors: [ expectedError ], parserOptions },
+    { code: '<img alt={`image doing ${things}`} {...this.props} />', errors: [ expectedError ], parserOptions },
+    { code: '<img alt={`picture doing ${picture}`} {...this.props} />', errors: [ expectedError ], parserOptions },
+    { code: '<img alt={`photo doing ${photo}`} {...this.props} />', errors: [ expectedError ], parserOptions },
+    { code: '<img alt={`image doing ${image}`} {...this.props} />', errors: [ expectedError ], parserOptions }
   ]
 });
