Fix - check value of props to determine their existence because undefined is not really there.

Ethan Cohen · Ethan Cohen · commit 8086b92693b1 · 2016-03-01T18:07:23.000-08:00
Remove case normalizing as well for html elements. lower case
convention applies. If user has custom Label component, it should not
look for htmlFor, for example.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "eslint-plugin-jsx-a11y",
-  "version": "0.2.0",
+  "version": "0.2.1",
   "description": "A static analysis linter of jsx and their accessibility with screen readers.",
   "keywords": [
     "eslint",
diff --git a/src/rules/img-uses-alt.js b/src/rules/img-uses-alt.js
@@ -15,7 +15,7 @@ const errorMessage = 'img elements must have an alt tag.';
 module.exports = context => ({
   JSXOpeningElement: node => {
     const type = node.name.name;
-    if (type.toUpperCase() !== 'IMG') {
+    if (type !== 'img') {
       return;
     }
 
diff --git a/src/rules/redundant-alt.js b/src/rules/redundant-alt.js
@@ -23,15 +23,15 @@ const errorMessage = 'Redundant alt attribute. Screen-readers already announce `
 module.exports = context => ({
   JSXOpeningElement: node => {
     const type = node.name.name;
-    if (type.toUpperCase() !== 'IMG') {
+    if (type !== 'img') {
       return;
     }
 
-    const hasAltProp = hasAttribute(node.attributes, 'alt');
+    const altProp = hasAttribute(node.attributes, 'alt');
     const isVisible = isHiddenFromScreenReader(node.attributes) === false;
 
-    if (Boolean(hasAltProp) && isVisible) {
-      const hasRedundancy = REDUNDANT_WORDS.some(word => Boolean(hasAltProp.value.value.match(new RegExp(word, 'gi'))));
+    if (Boolean(altProp) && typeof altProp === 'string' && isVisible) {
+      const hasRedundancy = REDUNDANT_WORDS.some(word => Boolean(altProp.match(new RegExp(word, 'gi'))));
 
       if (hasRedundancy === true) {
         context.report({
diff --git a/src/rules/use-label-for.js b/src/rules/use-label-for.js
@@ -16,7 +16,7 @@ const errorMessage = 'Form controls using a label to identify them must be ' +
 module.exports = context => ({
   JSXOpeningElement: node => {
     const type = node.name.name;
-    if (type.toUpperCase() !== 'LABEL') {
+    if (type !== 'label') {
       return;
     }
 
diff --git a/src/util/getAttributeValue.js b/src/util/getAttributeValue.js
@@ -0,0 +1,30 @@
+'use strict';
+
+const getAttributeValue = attribute => {
+  if (attribute.value === null) {
+    return null;
+  } else if (attribute.type === 'JSXAttribute') {
+
+    if (attribute.value.type === 'Literal') {
+      return attribute.value.value;
+    } else if (attribute.value.type === 'JSXExpressionContainer') {
+      const expression = attribute.value.expression;
+
+      switch (expression.type) {
+        case 'Literal':
+          return expression.value;
+        case 'Identifier':
+          return expression.name == 'undefined' ? undefined : expression.name;
+        case 'ArrowFunctionExpression':
+        case 'FunctionExpression':
+          return () => void 0;
+        default:
+          return undefined;
+      }
+    }
+  }
+
+  return undefined;
+};
+
+export default getAttributeValue;
diff --git a/src/util/hasAttribute.js b/src/util/hasAttribute.js
@@ -1,24 +1,28 @@
 'use strict';
 
+import getAttributeValue from './getAttributeValue';
+
 const hasAttribute = (attributes, attribute) => {
-  let idx = 0;
+  let value = false;
 
-  const hasAttr = attributes.some((attr, index) => {
+  const hasAttr = attributes.some(attr => {
     // If the attributes contain a spread attribute, then skip.
     if (attr.type === 'JSXSpreadAttribute') {
       return false;
     }
 
     // Normalize.
     if (attr.name.name.toUpperCase() === attribute.toUpperCase()) {
-      idx = index; // Keep track of the index.
-      return true;
+      value = getAttributeValue(attr);
+
+      // If the value is undefined, it doesn't really have the attribute.
+      return value !== undefined;
     }
 
     return false;
   });
 
-  return hasAttr ? attributes[idx] : false;
+  return hasAttr ? value : false;
 };
 
 export default hasAttribute;
diff --git a/src/util/isHiddenFromScreenReader.js b/src/util/isHiddenFromScreenReader.js
@@ -1,16 +1,10 @@
 'use strict';
 
-const isHiddenFromScreenReader = attributes => (
-  attributes.some(attribute => {
-    if (attribute.type === 'JSXSpreadAttribute') {
-      return false;
-    }
+import hasAttribute from './hasAttribute';
 
-    const name = attribute.name.name.toUpperCase();
-    const value = attribute.value && attribute.value.value;
-
-    return name === 'ARIA-HIDDEN' && (value === true || value === null);
-  })
-);
+const isHiddenFromScreenReader = attributes => {
+  const hasAriaHidden = hasAttribute(attributes, 'aria-hidden');
+  return hasAriaHidden && (hasAriaHidden === true || hasAriaHidden === null);
+};
 
 export default isHiddenFromScreenReader;
diff --git a/src/util/isInteractiveElement.js b/src/util/isInteractiveElement.js
@@ -11,7 +11,7 @@ const interactiveMap = {
   button: () => true,
   input: attributes => {
     const hasTypeAttr = hasAttribute(attributes, 'type');
-    return hasTypeAttr ? hasTypeAttr.value.value.toUpperCase() !== 'HIDDEN' : true;
+    return hasTypeAttr ? hasTypeAttr.toUpperCase() !== 'HIDDEN' : true;
   },
   option: () => true,
   select: () => true,
diff --git a/tests/src/rules/img-uses-alt.js b/tests/src/rules/img-uses-alt.js
@@ -33,13 +33,21 @@ const expectedError = {
 ruleTester.run('img-uses-alt', rule, {
   valid: [
     { code: '<img alt="foo" />;', parserOptions },
+    { code: '<img alt={"foo"} />;', parserOptions },
+    { code: '<img alt={alt} />;', parserOptions },
     { code: '<img ALT="foo" />;', parserOptions },
     { code: '<img ALt="foo" />;', parserOptions },
+    { code: '<img alt="foo" salt={undefined} />;', parserOptions },
     { code: '<img {...this.props} alt="foo" />', parserOptions },
-    { code: '<a />', parserOptions }
+    { code: '<a />', parserOptions },
+    { code: '<img alt={function(e) {} } />', parserOptions },
+    { code: '<div alt={function(e) {} } />', parserOptions },
+    { code: '<img alt={() => void 0} />', parserOptions },
+    { code: '<IMG />', parserOptions }
   ],
   invalid: [
     { code: '<img />;', errors: [ expectedError ], parserOptions },
+    { code: '<img alt={undefined} />;', errors: [ expectedError ], parserOptions },
     { code: '<img src="xyz" />', errors: [ expectedError ], parserOptions },
     { code: '<img {...this.props} />', errors: [ expectedError ], parserOptions }
   ]
diff --git a/tests/src/rules/mouse-events-map-to-key-events.js b/tests/src/rules/mouse-events-map-to-key-events.js
@@ -35,17 +35,23 @@ const mouseOutError = {
   type: 'JSXOpeningElement'
 };
 
-ruleTester.run('mouseEvents-require-keyEvents', rule, {
+ruleTester.run('mouse-events-map-to-key-events', rule, {
   valid: [
     { code: '<div onMouseOver={() => void 0} onFocus={() => void 0} />;', parserOptions },
     { code: '<div onMouseOver={() => void 0} onFocus={() => void 0} {...props} />;', parserOptions },
+    { code: '<div onMouseOver={handleMouseOver} onFocus={handleFocus} />;', parserOptions },
+    { code: '<div onMouseOver={handleMouseOver} onFocus={handleFocus} {...props} />;', parserOptions },
     { code: '<div />;', parserOptions },
     { code: '<div onMouseOut={() => void 0} onBlur={() => void 0} />', parserOptions },
-    { code: '<div onMouseOut={() => void 0} onBlur={() => void 0} {...props} />', parserOptions }
+    { code: '<div onMouseOut={() => void 0} onBlur={() => void 0} {...props} />', parserOptions },
+    { code: '<div onMouseOut={handleMouseOut} onBlur={handleOnBlur} />', parserOptions },
+    { code: '<div onMouseOut={handleMouseOut} onBlur={handleOnBlur} {...props} />', parserOptions }
   ],
   invalid: [
     { code: '<div onMouseOver={() => void 0} />;', errors: [ mouseOverError ], parserOptions },
     { code: '<div onMouseOut={() => void 0} />', errors: [ mouseOutError ], parserOptions },
+    { code: '<div onMouseOver={() => void 0} onFocus={undefined} />;', errors: [ mouseOverError ], parserOptions },
+    { code: '<div onMouseOut={() => void 0} onBlur={undefined} />', errors: [ mouseOutError ], parserOptions },
     { code: '<div onMouseOver={() => void 0} {...props} />', errors: [ mouseOverError ], parserOptions },
     { code: '<div onMouseOut={() => void 0} {...props} />', errors: [ mouseOutError ], parserOptions }
   ]
diff --git a/tests/src/rules/no-access-key.js b/tests/src/rules/no-access-key.js
@@ -35,12 +35,15 @@ const expectedError = {
 ruleTester.run('no-access-key', rule, {
   valid: [
     { code: '<div />;', parserOptions },
-    { code: '<div {...props} />', parserOptions }
+    { code: '<div {...props} />', parserOptions },
+    { code: '<div accessKey={undefined} />', parserOptions }
   ],
   invalid: [
     { code: '<div accesskey="h" />', errors: [ expectedError ], parserOptions },
     { code: '<div accessKey="h" />', errors: [ expectedError ], parserOptions },
     { code: '<div accessKey="h" {...props} />', errors: [ expectedError ], parserOptions },
-    { code: '<div acCesSKeY="y" />', errors: [ expectedError ], parserOptions }
+    { code: '<div acCesSKeY="y" />', errors: [ expectedError ], parserOptions },
+    { code: '<div accessKey={"y"} />', errors: [ expectedError ], parserOptions },
+    { code: '<div accessKey={accessKey} />', errors: [ expectedError ], parserOptions }
   ]
 });
diff --git a/tests/src/rules/onclick-uses-role.js b/tests/src/rules/onclick-uses-role.js
@@ -31,14 +31,17 @@ const  expectedError  = {
   type: 'JSXOpeningElement'
 };
 
-ruleTester.run('onClick-uses-role', rule, {
+ruleTester.run('onclick-uses-role', rule, {
   valid: [
     { code: '<div onClick={() => void 0} role="button" />;', parserOptions },
+    { code: '<div onClick={() => void 0} role={role} />;', parserOptions },
+    { code: '<div onClick={() => void 0} role={"button"} />;', parserOptions },
     { code: '<div onClick={() => void 0} role="button" {...props} />;', parserOptions },
     { code: '<div className="foo" />;', parserOptions },
     { code: '<div onClick={() => void 0} role="button" aria-hidden />;', parserOptions },
     { code: '<div onClick={() => void 0} role="button" aria-hidden={true} />;', parserOptions },
     { code: '<div onClick={() => void 0} role="button" aria-hidden={false} />;', parserOptions },
+    { code: '<div onClick={() => void 0} role="button" aria-hidden={undefined} />;', parserOptions },
     { code: '<input type="text" onClick={() => void 0} />', parserOptions },
     { code: '<input onClick={() => void 0} />', parserOptions },
     { code: '<button onClick={() => void 0} className="foo" />', parserOptions },
@@ -52,6 +55,7 @@ ruleTester.run('onClick-uses-role', rule, {
   ],
   invalid: [
     { code: '<div onClick={() => void 0} />;', errors: [ expectedError ], parserOptions },
+    { code: '<div onClick={() => void 0} role={undefined} />;', 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 },
diff --git a/tests/src/rules/redundant-alt.js b/tests/src/rules/redundant-alt.js
@@ -31,22 +31,30 @@ const expectedError = {
   type: 'JSXOpeningElement'
 };
 
-ruleTester.run('img-uses-alt', rule, {
+ruleTester.run('redundant-alt', rule, {
   valid: [
     { code: '<img alt="foo" />;', parserOptions },
     { code: '<img alt="picture of me taking a photo of an image" aria-hidden />', parserOptions },
     { code: '<img aria-hidden alt="photo of image" />', parserOptions },
     { code: '<img ALt="foo" />;', parserOptions },
     { code: '<img {...this.props} alt="foo" />', parserOptions },
+    { code: '<img {...this.props} alt={"foo"} />', parserOptions },
+    { code: '<img {...this.props} alt={alt} />', parserOptions },
     { code: '<a />', parserOptions },
     { code: '<img />', parserOptions },
+    { code: '<IMG />', parserOptions },
+    { code: '<img alt={undefined} />', parserOptions },
+    { code: '<img alt={"undefined"} />', parserOptions },
+    { code: '<img alt={() => {}} />', parserOptions },
+    { code: '<img alt={function(e){}} />', parserOptions },
     { code: '<img aria-hidden={false} alt="Doing cool things." />', parserOptions }
   ],
   invalid: [
     { code: '<img alt="Photo of friend." />;', errors: [ expectedError ], parserOptions },
     { code: '<img alt="Picture of friend." />;', errors: [ expectedError ], parserOptions },
     { code: '<img alt="Image of friend." />;', errors: [ expectedError ], parserOptions },
     { code: '<img alt="PhOtO of friend." />;', errors: [ expectedError ], parserOptions },
+    { code: '<img alt={"photo"} />;', errors: [ expectedError ], parserOptions },
     { code: '<img alt="piCTUre of friend." />;', errors: [ expectedError ], parserOptions },
     { code: '<img alt="imAGE of friend." />;', errors: [ expectedError ], parserOptions },
     { code: '<img alt="photo of cool person" aria-hidden={false} />', errors: [ expectedError ], parserOptions },
diff --git a/tests/src/rules/use-label-for.js b/tests/src/rules/use-label-for.js
@@ -34,11 +34,16 @@ const expectedError = {
 ruleTester.run('use-label-for', rule, {
   valid: [
     { code: '<label htmlFor="foo" />', parserOptions },
+    { code: '<label htmlFor={"foo"} />', parserOptions },
+    { code: '<label htmlFor={foo} />', parserOptions },
     { code: '<div />', parserOptions },
-    { code: '<label htmlFor="foo">Test!</label>', parserOptions }
+    { code: '<label htmlFor="foo">Test!</label>', parserOptions },
+    { code: '<Label />', parserOptions }, // lower-case convention refers to real HTML elements.
+    { code: '<Label htmlFor="foo" />', parserOptions }
   ],
   invalid: [
     { code: '<label id="foo" />', errors: [ expectedError ], parserOptions },
+    { code: '<label htmlFor={undefined} />', errors: [ expectedError ], parserOptions },
     { code: '<label>First Name</label>', errors: [ expectedError ], parserOptions },
     { code: '<label {...props}>Foo</label>', errors: [ expectedError ], parserOptions }
   ]
diff --git a/tests/src/rules/use-onblur-not-onchange.js b/tests/src/rules/use-onblur-not-onchange.js
@@ -34,12 +34,14 @@ const expectedError = {
 ruleTester.run('use-onblur-not-onchange', rule, {
   valid: [
     { code: '<div onBlur={() => {}} />;', parserOptions },
+    { code: '<div onBlur={handleOnBlur} />;', parserOptions },
     { code: '<div />;', parserOptions },
     { code: '<div onBlur={() => {}} onChange={() => {}} />;', parserOptions },
     { code: '<div {...props} />', parserOptions }
   ],
   invalid: [
     { code: '<div onChange={() => {}} />;', errors: [ expectedError ], parserOptions },
+    { code: '<div onChange={handleOnChange} />;', errors: [ expectedError ], parserOptions },
     { code: '<input onChange={() => {}} />', errors: [ expectedError ], parserOptions },
     { code: '<input onChange={() => {}} {...props} />', errors: [ expectedError ], parserOptions }
   ]

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "eslint-plugin-jsx-a11y",`
`3`		`- "version": "0.2.0",`
	`3`	`+ "version": "0.2.1",`
`4`	`4`	`"description": "A static analysis linter of jsx and their accessibility with screen readers.",`
`5`	`5`	`"keywords": [`
`6`	`6`	`"eslint",`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ const errorMessage = 'img elements must have an alt tag.';`
`15`	`15`	`module.exports = context => ({`
`16`	`16`	`JSXOpeningElement: node => {`
`17`	`17`	`const type = node.name.name;`
`18`		`- if (type.toUpperCase() !== 'IMG') {`
	`18`	`+ if (type !== 'img') {`
`19`	`19`	`return;`
`20`	`20`	`}`
`21`	`21`
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ const errorMessage = 'Form controls using a label to identify them must be ' +`
`16`	`16`	`module.exports = context => ({`
`17`	`17`	`JSXOpeningElement: node => {`
`18`	`18`	`const type = node.name.name;`
`19`		`- if (type.toUpperCase() !== 'LABEL') {`
	`19`	`+ if (type !== 'label') {`
`20`	`20`	`return;`
`21`	`21`	`}`
`22`	`22`