feat(label-has-associated-control): add option for enforcing label's …

…htmlFor matches control's id This change adds an option to the `label-has-associated-control` rule, enforcing that the label's htmlFor attribute matches the associated control's id attribute. Previously, the only validation done on htmlFor was that it was on the label component and had text. There was no attempt to cross-check that value against any attribute on the associated control. Not, when the option is enabled, cases where they don't match will report. I also took the opportunity to update the error messages so that each assert type gets an error message with verbiage specific to the assertion. (not sure if this should be called out as a separate feature in the changelog?). Note: the current implementation only checks the first instance it finds of child component that matches each control component type. It assumes that there won't be any acceptable cases where a label would have multiple inputs nested beneath it. Let me know if that assumption doesn't hold. Closes:
jsx-eslint · Dec 25, 2024 · d38df62 · d38df62
1 parent bfb0e9e
commit d38df62
Show file tree

Hide file tree

Showing 6 changed files with 424 additions and 63 deletions.
diff --git a/__tests__/src/rules/label-has-associated-control-test.js b/__tests__/src/rules/label-has-associated-control-test.js
@@ -27,6 +27,7 @@ const errorMessages = {
   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.',
+  htmlForShouldMatchId: 'A form label must have a htmlFor attribute that matches the id of the associated control.',
 };
 const expectedErrors = {};
 Object.keys(errorMessages).forEach((key) => {
@@ -58,6 +59,7 @@ const htmlForValid = [
   { code: '<label htmlFor="js_id" aria-label="A label" />' },
   { code: '<label htmlFor="js_id" aria-labelledby="A label" />' },
   { code: '<div><label htmlFor="js_id">A label</label><input id="js_id" /></div>' },
+  { code: '<div><label htmlFor={inputId}>A label</label><input id={inputId} /></div>' },
   { code: '<label for="js_id"><span><span><span>A label</span></span></span></label>', options: [{ depth: 4 }], settings: attributesSettings },
   { code: '<label for="js_id" aria-label="A label" />', settings: attributesSettings },
   { code: '<label for="js_id" aria-labelledby="A label" />', settings: attributesSettings },
@@ -128,6 +130,12 @@ const alwaysValid = [
   { code: '<input type="hidden" />' },
 ];
 
+const shouldHtmlForMatchIdValid = [
+  { code: '<label htmlFor="js_id" aria-label="A label"><span><span><input id="js_id" /></span></span></label>', options: [{ depth: 4, shouldHtmlForMatchId: true }] },
+  { code: '<div><label htmlFor="js_id">A label</label><input id="js_id" /></div>', options: [{ shouldHtmlForMatchId: true }] },
+  { code: '<div><label htmlFor={inputId} aria-label="A label" /><input id={inputId} /></div>', options: [{ shouldHtmlForMatchId: true }] },
+];
+
 const htmlForInvalid = (assertType) => {
   const expectedError = expectedErrors[assertType];
   return [
@@ -164,6 +172,13 @@ const nestingInvalid = (assertType) => {
     { code: '<CustomLabel><span>A label<CustomInput /></span></CustomLabel>', settings: componentsSettings, errors: [expectedError] },
   ];
 };
+const shouldHtmlForMatchIdInvalid = [
+  { code: '<label htmlFor="js_id" aria-label="A label"><span><span><input /></span></span></label>', options: [{ depth: 4, shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
+  { code: '<label htmlFor="js_id" aria-label="A label"><span><span><input name="js_id" /></span></span></label>', options: [{ depth: 4, shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
+  { code: '<div><label htmlFor="js_id">A label</label><input /></div>', options: [{ shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
+  { code: '<div><label htmlFor="js_id">A label</label><input name="js_id" /></div>', options: [{ shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
+  { code: '<div><label htmlFor={inputId} aria-label="A label" /><input name={inputId} /></div>', options: [{ shouldHtmlForMatchId: true }], errors: [expectedErrors.htmlForShouldMatchId] },
+];
 
 const neverValid = (assertType) => {
   const expectedError = expectedErrors[assertType];
@@ -266,3 +281,21 @@ ruleTester.run(ruleName, rule, {
     assert: 'both',
   })).map(parserOptionsMapper),
 });
+
+// shouldHtmlForMatchId
+ruleTester.run(ruleName, rule, {
+  valid: parsers.all([].concat(
+    ...shouldHtmlForMatchIdValid,
+  ))
+    .map(ruleOptionsMapperFactory({
+      assert: 'htmlFor',
+    }))
+    .map(parserOptionsMapper),
+  invalid: parsers.all([].concat(
+    ...shouldHtmlForMatchIdInvalid,
+  ))
+    .map(ruleOptionsMapperFactory({
+      assert: 'htmlFor',
+    }))
+    .map(parserOptionsMapper),
+});
diff --git a/__tests__/src/util/getChildComponent-test.js b/__tests__/src/util/getChildComponent-test.js
@@ -0,0 +1,224 @@
+import test from 'tape';
+
+import getChildComponent from '../../../src/util/getChildComponent';
+import JSXAttributeMock from '../../../__mocks__/JSXAttributeMock';
+import JSXElementMock from '../../../__mocks__/JSXElementMock';
+import JSXExpressionContainerMock from '../../../__mocks__/JSXExpressionContainerMock';
+
+test('mayContainChildComponent', (t) => {
+  t.equal(
+    getChildComponent(
+      JSXElementMock('div', [], [
+        JSXElementMock('div', [], [
+          JSXElementMock('span', [], []),
+          JSXElementMock('span', [], [
+            JSXElementMock('span', [], []),
+            JSXElementMock('span', [], [
+              JSXElementMock('span', [], []),
+            ]),
+          ]),
+        ]),
+        JSXElementMock('span', [], []),
+        JSXElementMock('img', [
+          JSXAttributeMock('src', 'some/path'),
+        ]),
+      ]),
+      'FancyComponent',
+      5,
+    ),
+    undefined,
+    'no FancyComponent returns undefined',
+  );
+
+  t.test('contains an indicated component', (st) => {
+    const inputMock = JSXElementMock('input');
+    st.equal(
+      getChildComponent(
+        JSXElementMock('div', [], [
+          inputMock,
+        ]),
+        'input',
+      ),
+      inputMock,
+      'returns input',
+    );
+
+    const FancyComponentMock = JSXElementMock('FancyComponent');
+    st.equal(
+      getChildComponent(
+        JSXElementMock('div', [], [
+          FancyComponentMock,
+        ]),
+        'FancyComponent',
+      ),
+      FancyComponentMock,
+      'returns FancyComponent',
+    );
+
+    st.equal(
+      getChildComponent(
+        JSXElementMock('div', [], [
+          JSXElementMock('div', [], [
+            JSXElementMock('FancyComponent'),
+          ]),
+        ]),
+        'FancyComponent',
+      ),
+      undefined,
+      'FancyComponent is outside of default depth, should return undefined',
+    );
+
+    st.equal(
+      getChildComponent(
+        JSXElementMock('div', [], [
+          JSXElementMock('div', [], [
+            FancyComponentMock,
+          ]),
+        ]),
+        'FancyComponent',
+        2,
+      ),
+      FancyComponentMock,
+      'FancyComponent is inside of custom depth, should return FancyComponent',
+    );
+
+    st.equal(
+      getChildComponent(
+        JSXElementMock('div', [], [
+          JSXElementMock('div', [], [
+            JSXElementMock('span', [], []),
+            JSXElementMock('span', [], [
+              JSXElementMock('span', [], []),
+              JSXElementMock('span', [], [
+                JSXElementMock('span', [], [
+                  JSXElementMock('span', [], [
+                    FancyComponentMock,
+                  ]),
+                ]),
+              ]),
+            ]),
+          ]),
+          JSXElementMock('span', [], []),
+          JSXElementMock('img', [
+            JSXAttributeMock('src', 'some/path'),
+          ]),
+        ]),
+        'FancyComponent',
+        6,
+      ),
+      FancyComponentMock,
+      'deep nesting, returns FancyComponent',
+    );
+
+    st.end();
+  });
+
+  const MysteryBox = JSXExpressionContainerMock('mysteryBox');
+  t.equal(
+    getChildComponent(
+      JSXElementMock('div', [], [
+        MysteryBox,
+      ]),
+      'FancyComponent',
+    ),
+    MysteryBox,
+    'Indeterminate situations + expression container children - returns component',
+  );
+
+  t.test('Glob name matching - component name contains question mark ? - match any single character', (st) => {
+    const FancyComponentMock = JSXElementMock('FancyComponent');
+    st.equal(
+      getChildComponent(
+        JSXElementMock('div', [], [
+          FancyComponentMock,
+        ]),
+        'Fanc?Co??onent',
+      ),
+      FancyComponentMock,
+      'returns component',
+    );
+
+    st.equal(
+      getChildComponent(
+        JSXElementMock('div', [], [
+          JSXElementMock('FancyComponent'),
+        ]),
+        'FancyComponent?',
+      ),
+      undefined,
+      'returns undefined',
+    );
+
+    st.test('component name contains asterisk * - match zero or more characters', (s2t) => {
+      s2t.equal(
+        getChildComponent(
+          JSXElementMock('div', [], [
+            FancyComponentMock,
+          ]),
+          'Fancy*',
+        ),
+        FancyComponentMock,
+        'returns component',
+      );
+
+      s2t.equal(
+        getChildComponent(
+          JSXElementMock('div', [], [
+            FancyComponentMock,
+          ]),
+          '*Component',
+        ),
+        FancyComponentMock,
+        'returns component',
+      );
+
+      s2t.equal(
+        getChildComponent(
+          JSXElementMock('div', [], [
+            FancyComponentMock,
+          ]),
+          'Fancy*C*t',
+        ),
+        FancyComponentMock,
+        'returns component',
+      );
+
+      s2t.end();
+    });
+
+    st.end();
+  });
+
+  t.test('using a custom elementType function', (st) => {
+    const CustomInputMock = JSXElementMock('CustomInput');
+    st.equal(
+      getChildComponent(
+        JSXElementMock('div', [], [
+          CustomInputMock,
+        ]),
+        'input',
+        2,
+        () => 'input',
+      ),
+      CustomInputMock,
+      'returns the component when the custom elementType returns the proper name',
+    );
+
+    st.equal(
+      getChildComponent(
+        JSXElementMock('div', [], [
+          JSXElementMock('CustomInput'),
+        ]),
+        'input',
+        2,
+        () => 'button',
+      ),
+      undefined,
+      'returns undefined when the custom elementType returns a wrong name',
+    );
+
+    st.end();
+  });
+
+  t.end();
+});
diff --git a/docs/rules/label-has-associated-control.md b/docs/rules/label-has-associated-control.md
@@ -28,7 +28,7 @@ The simplest way to achieve an association between a label and an input is to wr
 
 All modern browsers and assistive technology will associate the label with the control.
 
-### Case: The label is a sibling of the control.
+### Case: The label is a sibling of the control
 
 In this case, use `htmlFor` and an ID to associate the controls.
 
@@ -37,7 +37,7 @@ In this case, use `htmlFor` and an ID to associate the controls.
 <input type="text" id={domId} />
 ```
 
-### Case: My label and input components are custom components.
+### Case: My label and input components are custom components
 
 You can configure the rule to be aware of your custom components.
 
@@ -49,14 +49,14 @@ You can configure the rule to be aware of your custom components.
 
 And the configuration:
 
-```json
+```js
 {
-  "rules": {
-    "jsx-a11y/label-has-associated-control": [ 2, {
-      "labelComponents": ["CustomInputLabel"],
-      "labelAttributes": ["label"],
-      "controlComponents": ["CustomInput"],
-      "depth": 3,
+  rules: {
+    'jsx-a11y/label-has-associated-control': [ 2, {
+      labelComponents: ['CustomInputLabel'],
+      labelAttributes: ['label'],
+      controlComponents: ['CustomInput'],
+      depth: 3,
     }],
   }
 }
@@ -89,15 +89,16 @@ To restate, **every ID needs to be deterministic**, on the server and the client
 
 This rule takes one optional object argument of type object:
 
-```json
+```js
 {
-  "rules": {
-    "jsx-a11y/label-has-associated-control": [ 2, {
-      "labelComponents": ["CustomLabel"],
-      "labelAttributes": ["inputLabel"],
-      "controlComponents": ["CustomInput"],
-      "assert": "both",
-      "depth": 3,
+  rules: {
+    'jsx-a11y/label-has-associated-control': [ 2, {
+      labelComponents: ['CustomLabel'],
+      labelAttributes: ['inputLabel'],
+      controlComponents: ['CustomInput'],
+      assert: 'both',
+      depth: 3,
+      shouldHtmlForMatchId: true,
     }],
   }
 }
@@ -113,14 +114,18 @@ This rule takes one optional object argument of type object:
 
 `depth` (default 2, max 25) is an integer that determines how deep within a `JSXElement` label the rule should look for text content or an element with a label to determine if the `label` element will have an accessible label.
 
+`shouldHtmlForMatchId` (default `false`) is a boolean dictating whether the `htmlFor` attribute on a label element should be validated as matching the `id` attributed on an associated control (nested or sibling as described in the cases above).
+
 ### Fail
+
 ```jsx
 function Foo(props) {
   return <label {...props} />
 }
 ```
 
 ### Succeed
+
 ```jsx
 function Foo(props) {
     const {
@@ -133,12 +138,14 @@ function Foo(props) {
 ```
 
 ### Fail
+
 ```jsx
 <input type="text" />
 <label>Surname</label>
 ```
 
 ### Succeed
+
 ```jsx
 <label>
   <input type="text" />
@@ -147,6 +154,7 @@ function Foo(props) {
 ```
 
 ## Accessibility guidelines
+
 - [WCAG 1.3.1](https://www.w3.org/WAI/WCAG21/Understanding/info-and-relationships)
 - [WCAG 3.3.2](https://www.w3.org/WAI/WCAG21/Understanding/labels-or-instructions)
 - [WCAG 4.1.2](https://www.w3.org/WAI/WCAG21/Understanding/name-role-value)