Skip to content

Commit

Permalink
[refactor] - Refactor hasAttribute to be more semantically correct to…
Browse files Browse the repository at this point in the history
… getAttribute (#38)

Returns undefined if it is not found… still a falsy value.
  • Loading branch information
beefancohen committed Apr 19, 2016
1 parent da3be73 commit 0b66c90
Show file tree
Hide file tree
Showing 23 changed files with 90 additions and 89 deletions.
6 changes: 3 additions & 3 deletions src/rules/aria-unsupported-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.';
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/rules/href-no-hash.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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 === '#') {
Expand Down
16 changes: 8 additions & 8 deletions src/rules/img-has-alt.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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".`
Expand All @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/rules/img-redundant-alt.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/rules/label-has-for.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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;

Expand Down
18 changes: 9 additions & 9 deletions src/rules/mouse-events-have-key-events.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.';
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions src/rules/no-access-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ' +
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/rules/no-onchange.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/rules/onclick-has-focus.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down
6 changes: 3 additions & 3 deletions src/rules/onclick-has-role.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions src/rules/role-has-required-aria-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ` +
Expand Down Expand Up @@ -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({
Expand Down
18 changes: 9 additions & 9 deletions src/rules/role-supports-aria-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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)
});

}
Expand Down
8 changes: 4 additions & 4 deletions src/util/hasAttribute.js → src/util/getAttribute.js
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -27,7 +27,7 @@ const hasAttribute = (attributesOnNode, ...attributesToCheck) => {
return false;
});

return hasAttr ? nodeAttribute : false;
return hasAttr ? nodeAttribute : undefined;
};

export default hasAttribute;
export default getAttribute;
2 changes: 1 addition & 1 deletion src/util/getAttributeValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 />
Expand Down
4 changes: 2 additions & 2 deletions src/util/implicitRoles/a.js
Original file line number Diff line number Diff line change
@@ -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';
}

Expand Down
4 changes: 2 additions & 2 deletions src/util/implicitRoles/area.js
Original file line number Diff line number Diff line change
@@ -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';
}

Expand Down
Loading

0 comments on commit 0b66c90

Please sign in to comment.