[refactor] - Refactor hasAttribute to be more semantically correct to getAttribute (#38)

beefancohen · beefancohen · commit 0b66c904c59c · 2016-04-19T16:18:30.000-07:00
Returns undefined if it is not found… still a falsy value.
diff --git a/src/rules/aria-unsupported-elements.js b/src/rules/aria-unsupported-elements.js
@@ -10,7 +10,7 @@
 
 import DOM from '../util/attributes/DOM';
 import ARIA from '../util/attributes/ARIA';
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 import getNodeType from '../util/getNodeType';
 
 const errorMessage = 'This element does not support ARIA roles, states and properties.';
@@ -27,9 +27,9 @@ module.exports = context => ({
     }
 
     const invalidAttributes = Object.keys(ARIA).concat('ROLE');
-    const hasAria = hasAttribute(node.attributes, ...invalidAttributes);
+    const hasInvalidAttribute = getAttribute(node.attributes, ...invalidAttributes) !== undefined;
 
-    if (hasAria) {
+    if (hasInvalidAttribute) {
       context.report({
         node,
         message: errorMessage
diff --git a/src/rules/href-no-hash.js b/src/rules/href-no-hash.js
@@ -8,7 +8,7 @@
 // Rule Definition
 // ----------------------------------------------------------------------------
 
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 import getAttributeValue from '../util/getAttributeValue';
 import getNodeType from '../util/getNodeType';
 
@@ -24,7 +24,7 @@ module.exports = context => ({
       return;
     }
 
-    const href = hasAttribute(node.attributes, 'href');
+    const href = getAttribute(node.attributes, 'href');
     const value = getAttributeValue(href);
 
     if (href && value === '#') {
diff --git a/src/rules/img-has-alt.js b/src/rules/img-has-alt.js
@@ -8,7 +8,7 @@
 // Rule Definition
 // ----------------------------------------------------------------------------
 
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 import getAttributeValue from '../util/getAttributeValue';
 import getNodeType from '../util/getNodeType';
 
@@ -22,18 +22,18 @@ module.exports = context => ({
       return;
     }
 
-    const hasRoleProp = hasAttribute(node.attributes, 'role');
-    const roleValue = getAttributeValue(hasRoleProp);
-    const isPresentation = hasRoleProp && roleValue.toLowerCase() === 'presentation';
+    const roleProp = getAttribute(node.attributes, 'role');
+    const roleValue = getAttributeValue(roleProp);
+    const isPresentation = roleProp && roleValue.toLowerCase() === 'presentation';
 
     if (isPresentation) {
       return;
     }
 
-    const hasAltProp = hasAttribute(node.attributes, 'alt');
+    const altProp = getAttribute(node.attributes, 'alt');
 
     // Missing alt prop error.
-    if (!hasAltProp) {
+    if (altProp === undefined) {
       context.report({
         node,
         message: `${nodeType} elements must have an alt prop or use role="presentation".`
@@ -42,8 +42,8 @@ module.exports = context => ({
     }
 
     // Check if alt prop is undefined.
-    const altValue = getAttributeValue(hasAltProp);
-    const isNullValued = hasAltProp.value === null; // <img alt />
+    const altValue = getAttributeValue(altProp);
+    const isNullValued = altProp.value === null; // <img alt />
 
     if ((altValue && !isNullValued) || altValue === '') {
       return;
diff --git a/src/rules/img-redundant-alt.js b/src/rules/img-redundant-alt.js
@@ -8,7 +8,7 @@
 // Rule Definition
 // ----------------------------------------------------------------------------
 
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 import getAttributeValue from '../util/getAttributeValue';
 import isHiddenFromScreenReader from '../util/isHiddenFromScreenReader';
 import getNodeType from '../util/getNodeType';
@@ -34,9 +34,9 @@ module.exports = context => ({
       return;
     }
 
-    const altProp = hasAttribute(node.attributes, 'alt');
+    const altProp = getAttribute(node.attributes, 'alt');
     // Return if alt prop is not present.
-    if (altProp === false) {
+    if (altProp === undefined) {
       return;
     }
 
diff --git a/src/rules/label-has-for.js b/src/rules/label-has-for.js
@@ -8,7 +8,7 @@
 // Rule Definition
 // ----------------------------------------------------------------------------
 
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 import getAttributeValue from '../util/getAttributeValue';
 import getNodeType from '../util/getNodeType';
 
@@ -25,7 +25,7 @@ module.exports = context => ({
       return;
     }
 
-    const htmlForAttr = hasAttribute(node.attributes, 'htmlFor');
+    const htmlForAttr = getAttribute(node.attributes, 'htmlFor');
     const htmlForValue = getAttributeValue(htmlForAttr);
     const isInvalid = htmlForAttr === false || !htmlForValue;
 
diff --git a/src/rules/mouse-events-have-key-events.js b/src/rules/mouse-events-have-key-events.js
@@ -9,7 +9,7 @@
 // Rule Definition
 // ----------------------------------------------------------------------------
 
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 import getAttributeValue from '../util/getAttributeValue';
 
 const mouseOverErrorMessage = 'onMouseOver must be accompanied by onFocus for accessibility.';
@@ -20,11 +20,11 @@ module.exports = context => ({
     const attributes = node.attributes;
 
     // Check onmouseover / onfocus pairing.
-    const hasOnMouseOver = hasAttribute(attributes, 'onMouseOver');
-    const onMouseOverValue = getAttributeValue(hasOnMouseOver);
+    const onMouseOver = getAttribute(attributes, 'onMouseOver');
+    const onMouseOverValue = getAttributeValue(onMouseOver);
 
-    if (Boolean(hasOnMouseOver) === true && (onMouseOverValue !== null || onMouseOverValue !== undefined)) {
-      const hasOnFocus = hasAttribute(attributes, 'onFocus');
+    if (onMouseOver && (onMouseOverValue !== null || onMouseOverValue !== undefined)) {
+      const hasOnFocus = getAttribute(attributes, 'onFocus');
       const onFocusValue = getAttributeValue(hasOnFocus);
 
       if (hasOnFocus === false || onFocusValue === null || onFocusValue === undefined) {
@@ -36,10 +36,10 @@ module.exports = context => ({
     }
 
     // Checkout onmouseout / onblur pairing
-    const hasOnMouseOut = hasAttribute(attributes, 'onMouseOut');
-    const onMouseOutValue = getAttributeValue(hasOnMouseOut);
-    if (Boolean(hasOnMouseOut) === true && (onMouseOutValue !== null || onMouseOutValue !== undefined)) {
-      const hasOnBlur = hasAttribute(attributes, 'onBlur');
+    const onMouseOut = getAttribute(attributes, 'onMouseOut');
+    const onMouseOutValue = getAttributeValue(onMouseOut);
+    if (onMouseOut && (onMouseOutValue !== null || onMouseOutValue !== undefined)) {
+      const hasOnBlur = getAttribute(attributes, 'onBlur');
       const onBlurValue = getAttributeValue(hasOnBlur);
 
       if (hasOnBlur === false || onBlurValue === null || onBlurValue === undefined) {
diff --git a/src/rules/no-access-key.js b/src/rules/no-access-key.js
@@ -8,7 +8,7 @@
 // Rule Definition
 // ----------------------------------------------------------------------------
 
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 import getAttributeValue from '../util/getAttributeValue';
 
 const errorMessage = 'No access key attribute allowed. Inconsistencies ' +
@@ -17,10 +17,10 @@ const errorMessage = 'No access key attribute allowed. Inconsistencies ' +
 
 module.exports = context => ({
   JSXOpeningElement: node => {
-    const hasAccessKey = hasAttribute(node.attributes, 'accesskey');
-    const accessKeyValue = getAttributeValue(hasAccessKey);
+    const accessKey = getAttribute(node.attributes, 'accesskey');
+    const accessKeyValue = getAttributeValue(accessKey);
 
-    if (Boolean(hasAccessKey) === true && Boolean(accessKeyValue) === true) {
+    if (accessKey && accessKeyValue) {
       context.report({
         node,
         message: errorMessage
diff --git a/src/rules/no-onchange.js b/src/rules/no-onchange.js
@@ -8,18 +8,18 @@
 // Rule Definition
 // ----------------------------------------------------------------------------
 
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 
 const errorMessage = 'onBlur must be used instead of onchange, ' +
   'unless absolutely necessary and it causes no negative consequences ' +
   'for keyboard only or screen reader users.';
 
 module.exports = context => ({
   JSXOpeningElement: node => {
-    const hasOnChange = hasAttribute(node.attributes, 'onChange');
-    const hasOnBlur = hasAttribute(node.attributes, 'onBlur');
+    const onChange = getAttribute(node.attributes, 'onChange');
+    const hasOnBlur = getAttribute(node.attributes, 'onBlur') !== undefined;
 
-    if (Boolean(hasOnChange) === true && hasOnBlur === false) {
+    if (onChange && !hasOnBlur) {
       context.report({
         node,
         message: errorMessage
diff --git a/src/rules/onclick-has-focus.js b/src/rules/onclick-has-focus.js
@@ -6,7 +6,7 @@
 
 import isHiddenFromScreenReader from '../util/isHiddenFromScreenReader';
 import isInteractiveElement from '../util/isInteractiveElement';
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 import getNodeType from '../util/getNodeType';
 import getAttributeValue from '../util/getAttributeValue';
 
@@ -21,7 +21,7 @@ const errorMessage = 'Elements with onClick handlers must be focusable. ' +
 module.exports = context => ({
   JSXOpeningElement: node => {
     const { attributes } = node;
-    if (hasAttribute(attributes, 'onClick') === false) {
+    if (getAttribute(attributes, 'onClick') === undefined) {
       return;
     }
 
@@ -31,7 +31,7 @@ module.exports = context => ({
       return;
     } else if (isInteractiveElement(type, attributes)) {
       return;
-    } else if (getAttributeValue(hasAttribute(attributes, 'tabIndex'))) {
+    } else if (getAttributeValue(getAttribute(attributes, 'tabIndex'))) {
       return;
     }
 
diff --git a/src/rules/onclick-has-role.js b/src/rules/onclick-has-role.js
@@ -7,7 +7,7 @@
 
 import isHiddenFromScreenReader from '../util/isHiddenFromScreenReader';
 import isInteractiveElement from '../util/isInteractiveElement';
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 import getAttributeValue from '../util/getAttributeValue';
 import getNodeType from '../util/getNodeType';
 
@@ -21,7 +21,7 @@ const errorMessage = 'Visible, non-interactive elements with click handlers must
 module.exports = context => ({
   JSXOpeningElement: node => {
     const attributes = node.attributes;
-    if (hasAttribute(attributes, 'onclick') === false) {
+    if (getAttribute(attributes, 'onclick') === undefined) {
       return;
     }
 
@@ -31,7 +31,7 @@ module.exports = context => ({
       return;
     } else if (isInteractiveElement(type, attributes)) {
       return;
-    } else if (getAttributeValue(hasAttribute(attributes, 'role'))) {
+    } else if (getAttributeValue(getAttribute(attributes, 'role'))) {
       return;
     }
 
diff --git a/src/rules/role-has-required-aria-props.js b/src/rules/role-has-required-aria-props.js
@@ -10,7 +10,7 @@
 
 import validRoleTypes from '../util/attributes/role';
 import { getLiteralAttributeValue } from '../util/getAttributeValue';
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 
 const errorMessage = (role, requiredProps) =>
   `Elements with the ARIA role "${role}" must have the following ` +
@@ -38,7 +38,7 @@ module.exports = context => ({
       const { requiredProps } = validRoleTypes[role];
 
       if (requiredProps.length > 0) {
-        const hasRequiredProps = requiredProps.every(prop => hasAttribute(attribute.parent.attributes, prop));
+        const hasRequiredProps = requiredProps.every(prop => getAttribute(attribute.parent.attributes, prop));
 
         if (hasRequiredProps === false) {
           context.report({
diff --git a/src/rules/role-supports-aria-props.js b/src/rules/role-supports-aria-props.js
@@ -9,7 +9,7 @@
 // Rule Definition
 // ----------------------------------------------------------------------------
 
-import hasAttribute from '../util/hasAttribute';
+import getAttribute from '../util/getAttribute';
 import { getLiteralAttributeValue } from '../util/getAttributeValue';
 import getNodeType from '../util/getNodeType';
 import ROLES from '../util/attributes/role';
@@ -28,29 +28,29 @@ module.exports = context => ({
   JSXOpeningElement: node => {
     // If role is not explicitly defined, then try and get its implicit role.
     const type = getNodeType(node);
-    const hasRole = hasAttribute(node.attributes, 'role');
-    const role = hasRole ? getLiteralAttributeValue(hasRole) : getImplicitRole(type, node.attributes);
-    const isImplicit = role && !hasRole;
+    const role = getAttribute(node.attributes, 'role');
+    const roleValue = role ? getLiteralAttributeValue(role) : getImplicitRole(type, node.attributes);
+    const isImplicit = roleValue && role === undefined;
 
     // If there is no explicit or implicit role, then assume that the element
     // can handle the global set of aria-* properties.
     // This actually isn't true - should fix in future release.
-    if (!role || ROLES[role.toUpperCase()] === undefined) {
+    if (!roleValue || ROLES[roleValue.toUpperCase()] === undefined) {
       return;
     }
 
     // Make sure it has no aria-* properties defined outside of its property set.
-    const propertySet = ROLES[role.toUpperCase()].props;
+    const propertySet = ROLES[roleValue.toUpperCase()].props;
     const invalidAriaPropsForRole = Object.keys(ARIA).filter(attribute => propertySet.indexOf(attribute) === -1);
-    const invalidAttr = hasAttribute(node.attributes, ...invalidAriaPropsForRole);
+    const invalidAttr = getAttribute(node.attributes, ...invalidAriaPropsForRole);
 
-    if (invalidAttr === false) {
+    if (invalidAttr === undefined) {
       return;
     }
 
     context.report({
       node,
-      message: errorMessage(invalidAttr.name.name, role, type, isImplicit)
+      message: errorMessage(invalidAttr.name.name, roleValue, type, isImplicit)
     });
 
   }
diff --git a/src/util/getAttribute.js b/src/util/getAttribute.js
@@ -1,12 +1,12 @@
 'use strict';
 
 /**
- * Returns the JSXAttribute itself or false, indicating the attribute
+ * Returns the JSXAttribute itself or undefined, 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.
  *
  */
-const hasAttribute = (attributesOnNode, ...attributesToCheck) => {
+const getAttribute = (attributesOnNode, ...attributesToCheck) => {
   let nodeAttribute = undefined;
 
   // Normalize.
@@ -27,7 +27,7 @@ const hasAttribute = (attributesOnNode, ...attributesToCheck) => {
     return false;
   });
 
-  return hasAttr ? nodeAttribute : false;
+  return hasAttr ? nodeAttribute : undefined;
 };
 
-export default hasAttribute;
+export default getAttribute;
diff --git a/src/util/getAttributeValue.js b/src/util/getAttributeValue.js
@@ -5,7 +5,7 @@ import getValue, { getLiteralValue } from './values';
 
 
 const extractValue = (attribute, extractor) => {
-  if (attribute.type === 'JSXAttribute') {
+  if (attribute && attribute.type === 'JSXAttribute') {
     if (attribute.value === null) {
       // Null valued attributes imply truthiness.
       // For example: <div aria-hidden />
diff --git a/src/util/implicitRoles/a.js b/src/util/implicitRoles/a.js
@@ -1,10 +1,10 @@
-import hasAttribute from '../hasAttribute';
+import getAttribute from '../getAttribute';
 
 /**
  * Returns the implicit role for an anchor tag.
  */
 export default function getImplicitRoleForAnchor(attributes) {
-  if (hasAttribute(attributes, 'href')) {
+  if (getAttribute(attributes, 'href')) {
     return 'link';
   }
 
diff --git a/src/util/implicitRoles/area.js b/src/util/implicitRoles/area.js
@@ -1,10 +1,10 @@
-import hasAttribute from '../hasAttribute';
+import getAttribute from '../getAttribute';
 
 /**
  * Returns the implicit role for an area tag.
  */
 export default function getImplicitRoleForArea(attributes) {
-  if (hasAttribute(attributes, 'href')) {
+  if (getAttribute(attributes, 'href')) {
     return 'link';
   }
 
diff --git a/src/util/implicitRoles/img.js b/src/util/implicitRoles/img.js
diff --git a/src/util/implicitRoles/input.js b/src/util/implicitRoles/input.js
diff --git a/src/util/implicitRoles/link.js b/src/util/implicitRoles/link.js
diff --git a/src/util/implicitRoles/menu.js b/src/util/implicitRoles/menu.js
diff --git a/src/util/implicitRoles/menuitem.js b/src/util/implicitRoles/menuitem.js
diff --git a/src/util/isHiddenFromScreenReader.js b/src/util/isHiddenFromScreenReader.js
diff --git a/src/util/isInteractiveElement.js b/src/util/isInteractiveElement.js
