Merge pull request #240 from ignatiusreza/label/nesting

Allow label-has-for rule to pass if containing nesting content.

Author: ljharb

__tests__/src/rules/label-has-for-test.js

@@ -19,14 +19,22 @@ import rule from '../../../src/rules/label-has-for';
 const ruleTester = new RuleTester();
 
 const expectedError = {
-  message: 'Form controls using a label to identify them must be ' +
-  'programmatically associated with the control using htmlFor',
+  message: 'Form label must have associated control',
   type: 'JSXOpeningElement',
 };
 
 const array = [{
   components: ['Label', 'Descriptor'],
 }];
+const optionsRequiredNesting = [{
+  required: 'nesting',
+}];
+const optionsRequiredSome = [{
+  required: { some: ['nesting', 'id'] },
+}];
+const optionsRequiredEvery = [{
+  required: { every: ['nesting', 'id'] },
+}];
 
 ruleTester.run('label-has-for', rule, {
   valid: [
@@ -54,6 +62,10 @@ ruleTester.run('label-has-for', rule, {
     { code: '<Descriptor htmlFor={`${id}`} />', options: array },
     { code: '<div />', options: array },
     { code: '<Descriptor htmlFor="foo">Test!</Descriptor>', options: array },
+    { code: '<label><input /></label>', options: optionsRequiredNesting },
+    { code: '<label><input /></label>', options: optionsRequiredSome },
+    { code: '<label htmlFor="foo" />', options: optionsRequiredSome },
+    { code: '<label htmlFor="foo"><input /></label>', options: optionsRequiredEvery },
   ].map(parserOptionsMapper),
   invalid: [
     // DEFAULT ELEMENT 'label' TESTS
@@ -62,6 +74,7 @@ ruleTester.run('label-has-for', rule, {
     { code: '<label htmlFor={`${undefined}`} />', errors: [expectedError] },
     { code: '<label>First Name</label>', errors: [expectedError] },
     { code: '<label {...props}>Foo</label>', errors: [expectedError] },
+    { code: '<label><input /></label>', errors: [expectedError] },
 
     // CUSTOM ELEMENT ARRAY OPTION TESTS
     { code: '<Label id="foo" />', errors: [expectedError], options: array },
@@ -102,5 +115,11 @@ ruleTester.run('label-has-for', rule, {
       errors: [expectedError],
       options: array,
     },
+    { code: '<label htmlFor="foo" />', errors: [expectedError], options: optionsRequiredNesting },
+    { code: '<label>First Name</label>', errors: [expectedError], options: optionsRequiredNesting },
+    { code: '<label>First Name</label>', errors: [expectedError], options: optionsRequiredSome },
+    { code: '<label htmlFor="foo" />', errors: [expectedError], options: optionsRequiredEvery },
+    { code: '<label><input /></label>', errors: [expectedError], options: optionsRequiredEvery },
+    { code: '<label>First Name</label>', errors: [expectedError], options: optionsRequiredEvery },
   ].map(parserOptionsMapper),
 });

docs/rules/label-has-for.md

@@ -1,6 +1,13 @@
 # label-has-for
 
-Enforce label tags have htmlFor attribute. Form controls using a label to identify them must have only one label that is programmatically associated with the control using: label htmlFor=[ID of control].
+Enforce label tags have associated control.
+
+There are two supported ways to associate a label with a control:
+
+- nesting: by wrapping a control in a label tag
+- id: by using the prop `htmlFor` as in `htmlFor=[ID of control]`
+
+To fully cover 100% of assistive devices, you're encouraged to validate for both nesting and id.
 
 ## Rule details
 
@@ -11,7 +18,10 @@ This rule takes one optional object argument of type object:
     "rules": {
         "jsx-a11y/label-has-for": [ 2, {
             "components": [ "Label" ],
-          }],
+            "required": {
+                "every": [ "nesting", "id" ]
+            }
+        }],
     }
 }
 ```
@@ -43,6 +53,14 @@ return (
 );
 ```
 
+The `required` option (defaults to `"required": "id"`) determines which checks are activated. You're allowed to pass in one of the following types:
+
+- string: must be one of the acceptable strings (`"nesting"` or `"id"`)
+- object, must have one of the following properties:
+
+  - some: an array of acceptable strings, will pass if ANY of the requested checks passed
+  - every: an array of acceptable strings, will pass if ALL of the requested checks passed
+
 Note that passing props as spread attribute without `htmlFor` explicitly defined will cause this rule to fail. Explicitly pass down `htmlFor` prop for rule to pass. The prop must have an actual value to pass. Use `Label` component above as a reference. **It is a good thing to explicitly pass props that you expect to be passed for self-documentation.** For example:
 
 #### Bad

src/rules/label-has-for.js

@@ -8,12 +8,45 @@
 // ----------------------------------------------------------------------------
 
 import { getProp, getPropValue, elementType } from 'jsx-ast-utils';
-import { generateObjSchema, arraySchema } from '../util/schemas';
+import { generateObjSchema, arraySchema, enumArraySchema } from '../util/schemas';
 
-const errorMessage = 'Form controls using a label to identify them must be ' +
-  'programmatically associated with the control using htmlFor';
+const errorMessage = 'Form label must have associated control';
 
-const schema = generateObjSchema({ components: arraySchema });
+const enumValues = ['nesting', 'id'];
+const schema = {
+  type: 'object',
+  properties: {
+    components: arraySchema,
+    required: {
+      oneOf: [
+        { type: 'string', enum: enumValues },
+        generateObjSchema({ some: enumArraySchema(enumValues) }, ['some']),
+        generateObjSchema({ every: enumArraySchema(enumValues) }, ['every']),
+      ],
+    },
+  },
+};
+
+const validateNesting = node => !!node.parent.children.find(child => child.type === 'JSXElement');
+const validateId = (node) => {
+  const htmlForAttr = getProp(node.attributes, 'htmlFor');
+  const htmlForValue = getPropValue(htmlForAttr);
+
+  return htmlForAttr !== false && !!htmlForValue;
+};
+const validate = (node, required) => (
+  required === 'nesting' ? validateNesting(node) : validateId(node)
+);
+
+const isValid = (node, required) => {
+  if (Array.isArray(required.some)) {
+    return required.some.some(rule => validate(node, rule));
+  } else if (Array.isArray(required.every)) {
+    return required.every.every(rule => validate(node, rule));
+  }
+
+  return validate(node, required);
+};
 
 module.exports = {
   meta: {
@@ -33,11 +66,9 @@ module.exports = {
         return;
       }
 
-      const htmlForAttr = getProp(node.attributes, 'htmlFor');
-      const htmlForValue = getPropValue(htmlForAttr);
-      const isInvalid = htmlForAttr === false || !htmlForValue;
+      const required = options.required || 'id';
 
-      if (isInvalid) {
+      if (!isValid(node, required)) {
         context.report({
           node,
           message: errorMessage,

src/util/schemas.js

@@ -24,7 +24,8 @@ export const enumArraySchema = (enumeratedList = []) => Object.assign({}, arrayS
  * Factory function to generate an object schema
  * with specified properties object
  */
-export const generateObjSchema = (properties = {}) => ({
+export const generateObjSchema = (properties = {}, required) => ({
   type: 'object',
   properties,
+  required,
 });