From bfb0e9e89ae1605dcd1e79eacb745e819ae45f27 Mon Sep 17 00:00:00 2001
From: michael faith <michaelfaith@users.noreply.github.com>
Date: Fri, 20 Dec 2024 16:38:23 -0600
Subject: [PATCH] [patch] `label-has-associated-control`: improve error
 messages

This change updates the error messages of the label-has-associated-control rule so that each assert type gets an error message with verbiage specific to the assertion.

I wanted to land this before adding support for matching a label's htmlFor attribute with the associated control's id
---
 .../label-has-associated-control-test.js      | 156 ++++++++++--------
 src/rules/label-has-associated-control.js     |  58 ++++---
 2 files changed, 123 insertions(+), 91 deletions(-)

diff --git a/__tests__/src/rules/label-has-associated-control-test.js b/__tests__/src/rules/label-has-associated-control-test.js
index 633275c3..01d02e06 100644
--- a/__tests__/src/rules/label-has-associated-control-test.js
+++ b/__tests__/src/rules/label-has-associated-control-test.js
@@ -21,15 +21,20 @@ const ruleTester = new RuleTester();
 
 const ruleName = 'label-has-associated-control';
 
-const expectedError = {
-  message: 'A form label must be associated with a control.',
-  type: 'JSXOpeningElement',
-};
-
-const expectedErrorNoLabel = {
-  message: 'A form label must have accessible text.',
-  type: 'JSXOpeningElement',
+const errorMessages = {
+  accessibleLabel: 'A form label must have accessible text.',
+  htmlFor: 'A form label must have a valid htmlFor attribute.',
+  nesting: 'A form label must have an associated control as a descendant.',
+  either: 'A form label must either have a valid htmlFor attribute or a control as a descendant.',
+  both: 'A form label must have a valid htmlFor attribute and a control as a descendant.',
 };
+const expectedErrors = {};
+Object.keys(errorMessages).forEach((key) => {
+  expectedErrors[key] = {
+    message: errorMessages[key],
+    type: 'JSXOpeningElement',
+  };
+});
 
 const componentsSettings = {
   'jsx-a11y': {
@@ -123,59 +128,68 @@ const alwaysValid = [
   { code: '<input type="hidden" />' },
 ];
 
-const htmlForInvalid = [
-  { code: '<label htmlFor="js_id"><span><span><span>A label</span></span></span></label>', options: [{ depth: 4 }], errors: [expectedError] },
-  { code: '<label htmlFor="js_id" aria-label="A label" />', errors: [expectedError] },
-  { code: '<label htmlFor="js_id" aria-labelledby="A label" />', errors: [expectedError] },
-  // Custom label component.
-  { code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedError] },
-  { code: '<CustomLabel htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
-  { code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', settings: componentsSettings, errors: [expectedError] },
-  // Custom label attributes.
-  { code: '<label htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
-];
-const nestingInvalid = [
-  { code: '<label>A label<input /></label>', errors: [expectedError] },
-  { code: '<label>A label<textarea /></label>', errors: [expectedError] },
-  { code: '<label><img alt="A label" /><input /></label>', errors: [expectedError] },
-  { code: '<label><img aria-label="A label" /><input /></label>', errors: [expectedError] },
-  { code: '<label><span>A label<input /></span></label>', errors: [expectedError] },
-  { code: '<label><span><span>A label<input /></span></span></label>', options: [{ depth: 3 }], errors: [expectedError] },
-  { code: '<label><span><span><span>A label<input /></span></span></span></label>', options: [{ depth: 4 }], errors: [expectedError] },
-  { code: '<label><span><span><span><span>A label</span><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
-  { code: '<label><span><span><span><span aria-label="A label" /><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
-  { code: '<label><span><span><span><input aria-label="A label" /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
-  // Custom controlComponents.
-  { code: '<label>A label<OtherCustomInput /></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedError] },
-  { code: '<label><span>A label<CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedError] },
-  { code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
-  { code: '<CustomLabel><span label="A label"><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedError] },
-  { code: '<label><span>A label<CustomInput /></span></label>', settings: componentsSettings, errors: [expectedError] },
-  { code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedError] },
-];
+const htmlForInvalid = (assertType) => {
+  const expectedError = expectedErrors[assertType];
+  return [
+    { code: '<label htmlFor="js_id"><span><span><span>A label</span></span></span></label>', options: [{ depth: 4 }], errors: [expectedError] },
+    { code: '<label htmlFor="js_id" aria-label="A label" />', errors: [expectedError] },
+    { code: '<label htmlFor="js_id" aria-labelledby="A label" />', errors: [expectedError] },
+    // Custom label component.
+    { code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedError] },
+    { code: '<CustomLabel htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
+    { code: '<CustomLabel htmlFor="js_id" aria-label="A label" />', settings: componentsSettings, errors: [expectedError] },
+    // Custom label attributes.
+    { code: '<label htmlFor="js_id" label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
+  ];
+};
+const nestingInvalid = (assertType) => {
+  const expectedError = expectedErrors[assertType];
+  return [
+    { code: '<label>A label<input /></label>', errors: [expectedError] },
+    { code: '<label>A label<textarea /></label>', errors: [expectedError] },
+    { code: '<label><img alt="A label" /><input /></label>', errors: [expectedError] },
+    { code: '<label><img aria-label="A label" /><input /></label>', errors: [expectedError] },
+    { code: '<label><span>A label<input /></span></label>', errors: [expectedError] },
+    { code: '<label><span><span>A label<input /></span></span></label>', options: [{ depth: 3 }], errors: [expectedError] },
+    { code: '<label><span><span><span>A label<input /></span></span></span></label>', options: [{ depth: 4 }], errors: [expectedError] },
+    { code: '<label><span><span><span><span>A label</span><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
+    { code: '<label><span><span><span><span aria-label="A label" /><input /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
+    { code: '<label><span><span><span><input aria-label="A label" /></span></span></span></label>', options: [{ depth: 5 }], errors: [expectedError] },
+    // Custom controlComponents.
+    { code: '<label>A label<OtherCustomInput /></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedError] },
+    { code: '<label><span>A label<CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedError] },
+    { code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
+    { code: '<CustomLabel><span label="A label"><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedError] },
+    { code: '<label><span>A label<CustomInput /></span></label>', settings: componentsSettings, errors: [expectedError] },
+    { code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedError] },
+  ];
+};
 
-const neverValid = [
-  { code: '<label htmlFor="js_id" />', errors: [expectedErrorNoLabel] },
-  { code: '<label htmlFor="js_id"><input /></label>', errors: [expectedErrorNoLabel] },
-  { code: '<label htmlFor="js_id"><textarea /></label>', errors: [expectedErrorNoLabel] },
-  { code: '<label></label>', errors: [expectedErrorNoLabel] },
-  { code: '<label>A label</label>', errors: [expectedError] },
-  { code: '<div><label /><input /></div>', errors: [expectedErrorNoLabel] },
-  { code: '<div><label>A label</label><input /></div>', errors: [expectedError] },
-  // Custom label component.
-  { code: '<CustomLabel aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedError] },
-  { code: '<MUILabel aria-label="A label" />', options: [{ labelComponents: ['???Label'] }], errors: [expectedError] },
-  { code: '<CustomLabel label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
-  { code: '<CustomLabel aria-label="A label" />', settings: componentsSettings, errors: [expectedError] },
-  // Custom label attributes.
-  { code: '<label label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
-  // Custom controlComponents.
-  { code: '<label><span><CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedErrorNoLabel] },
-  { code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedErrorNoLabel] },
-  { code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedErrorNoLabel] },
-  { code: '<label><span><CustomInput /></span></label>', settings: componentsSettings, errors: [expectedErrorNoLabel] },
-  { code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedErrorNoLabel] },
-];
+const neverValid = (assertType) => {
+  const expectedError = expectedErrors[assertType];
+  return [
+    { code: '<label htmlFor="js_id" />', errors: [expectedErrors.accessibleLabel] },
+    { code: '<label htmlFor="js_id"><input /></label>', errors: [expectedErrors.accessibleLabel] },
+    { code: '<label htmlFor="js_id"><textarea /></label>', errors: [expectedErrors.accessibleLabel] },
+    { code: '<label></label>', errors: [expectedErrors.accessibleLabel] },
+    { code: '<label>A label</label>', errors: [expectedError] },
+    { code: '<div><label /><input /></div>', errors: [expectedErrors.accessibleLabel] },
+    { code: '<div><label>A label</label><input /></div>', errors: [expectedError] },
+    // Custom label component.
+    { code: '<CustomLabel aria-label="A label" />', options: [{ labelComponents: ['CustomLabel'] }], errors: [expectedError] },
+    { code: '<MUILabel aria-label="A label" />', options: [{ labelComponents: ['???Label'] }], errors: [expectedError] },
+    { code: '<CustomLabel label="A label" />', options: [{ labelAttributes: ['label'], labelComponents: ['CustomLabel'] }], errors: [expectedError] },
+    { code: '<CustomLabel aria-label="A label" />', settings: componentsSettings, errors: [expectedError] },
+    // Custom label attributes.
+    { code: '<label label="A label" />', options: [{ labelAttributes: ['label'] }], errors: [expectedError] },
+    // Custom controlComponents.
+    { code: '<label><span><CustomInput /></span></label>', options: [{ controlComponents: ['CustomInput'] }], errors: [expectedErrors.accessibleLabel] },
+    { code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'] }], errors: [expectedErrors.accessibleLabel] },
+    { code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', options: [{ controlComponents: ['CustomInput'], labelComponents: ['CustomLabel'], labelAttributes: ['label'] }], errors: [expectedErrors.accessibleLabel] },
+    { code: '<label><span><CustomInput /></span></label>', settings: componentsSettings, errors: [expectedErrors.accessibleLabel] },
+    { code: '<CustomLabel><span><CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedErrors.accessibleLabel] },
+  ];
+};
 // htmlFor valid
 ruleTester.run(ruleName, rule, {
   valid: parsers.all([].concat(
@@ -187,8 +201,8 @@ ruleTester.run(ruleName, rule, {
     }))
     .map(parserOptionsMapper),
   invalid: parsers.all([].concat(
-    ...neverValid,
-    ...nestingInvalid,
+    ...neverValid('htmlFor'),
+    ...nestingInvalid('htmlFor'),
   ))
     .map(ruleOptionsMapperFactory({
       assert: 'htmlFor',
@@ -207,8 +221,8 @@ ruleTester.run(ruleName, rule, {
     }))
     .map(parserOptionsMapper),
   invalid: parsers.all([].concat(
-    ...neverValid,
-    ...htmlForInvalid,
+    ...neverValid('nesting'),
+    ...htmlForInvalid('nesting'),
   ))
     .map(ruleOptionsMapperFactory({
       assert: 'nesting',
@@ -228,8 +242,10 @@ ruleTester.run(ruleName, rule, {
     }))
     .map(parserOptionsMapper),
   invalid: parsers.all([].concat(
-    ...neverValid,
-  )).map(parserOptionsMapper),
+    ...neverValid('either'),
+  )).map(ruleOptionsMapperFactory({
+    assert: 'either',
+  })).map(parserOptionsMapper),
 });
 
 // both valid
@@ -243,6 +259,10 @@ ruleTester.run(ruleName, rule, {
     }))
     .map(parserOptionsMapper),
   invalid: parsers.all([].concat(
-    ...neverValid,
-  )).map(parserOptionsMapper),
+    ...neverValid('both'),
+    ...htmlForInvalid('both'),
+    ...nestingInvalid('both'),
+  )).map(ruleOptionsMapperFactory({
+    assert: 'both',
+  })).map(parserOptionsMapper),
 });
diff --git a/src/rules/label-has-associated-control.js b/src/rules/label-has-associated-control.js
index dd6b199b..d65abe98 100644
--- a/src/rules/label-has-associated-control.js
+++ b/src/rules/label-has-associated-control.js
@@ -18,8 +18,13 @@ import getElementType from '../util/getElementType';
 import mayContainChildComponent from '../util/mayContainChildComponent';
 import mayHaveAccessibleLabel from '../util/mayHaveAccessibleLabel';
 
-const errorMessage = 'A form label must be associated with a control.';
-const errorMessageNoLabel = 'A form label must have accessible text.';
+const errorMessages = {
+  accessibleLabel: 'A form label must have accessible text.',
+  htmlFor: 'A form label must have a valid htmlFor attribute.',
+  nesting: 'A form label must have an associated control as a descendant.',
+  either: 'A form label must either have a valid htmlFor attribute or a control as a descendant.',
+  both: 'A form label must have a valid htmlFor attribute and a control as a descendant.',
+};
 
 const schema = generateObjSchema({
   labelComponents: arraySchema,
@@ -37,7 +42,7 @@ const schema = generateObjSchema({
   },
 });
 
-function validateID(node, context) {
+const validateHtmlFor = (node, context) => {
   const { settings } = context;
   const htmlForAttributes = settings['jsx-a11y']?.attributes?.for ?? ['htmlFor'];
 
@@ -52,7 +57,7 @@ function validateID(node, context) {
   }
 
   return false;
-}
+};
 
 export default ({
   meta: {
@@ -76,20 +81,21 @@ export default ({
         return;
       }
 
-      const controlComponents = [
+      const controlComponents = [].concat(
         'input',
         'meter',
         'output',
         'progress',
         'select',
         'textarea',
-      ].concat((options.controlComponents || []));
+        options.controlComponents || [],
+      );
       // Prevent crazy recursion.
       const recursionDepth = Math.min(
         options.depth === undefined ? 2 : options.depth,
         25,
       );
-      const hasLabelId = validateID(node.openingElement, context);
+      const hasHtmlFor = validateHtmlFor(node.openingElement, context);
       // Check for multiple control components.
       const hasNestedControl = controlComponents.some((name) => mayContainChildComponent(
         node,
@@ -105,44 +111,50 @@ export default ({
         controlComponents,
       );
 
+      // Bail out immediately if we don't have an accessible label.
       if (!hasAccessibleLabel) {
         context.report({
           node: node.openingElement,
-          message: errorMessageNoLabel,
+          message: errorMessages.accessibleLabel,
         });
         return;
       }
-
       switch (assertType) {
         case 'htmlFor':
-          if (hasLabelId) {
-            return;
+          if (!hasHtmlFor) {
+            context.report({
+              node: node.openingElement,
+              message: errorMessages.htmlFor,
+            });
           }
           break;
         case 'nesting':
-          if (hasNestedControl) {
-            return;
+          if (!hasNestedControl) {
+            context.report({
+              node: node.openingElement,
+              message: errorMessages.nesting,
+            });
           }
           break;
         case 'both':
-          if (hasLabelId && hasNestedControl) {
-            return;
+          if (!hasHtmlFor || !hasNestedControl) {
+            context.report({
+              node: node.openingElement,
+              message: errorMessages.both,
+            });
           }
           break;
         case 'either':
-          if (hasLabelId || hasNestedControl) {
-            return;
+          if (!hasHtmlFor && !hasNestedControl) {
+            context.report({
+              node: node.openingElement,
+              message: errorMessages.either,
+            });
           }
           break;
         default:
           break;
       }
-
-      // htmlFor case
-      context.report({
-        node: node.openingElement,
-        message: errorMessage,
-      });
     };
 
     // Create visitor selectors.