Merge pull request #152 from microsoft/users/aubreyquinn/factory

aubreyquinn · web-flow · commit 3977d4404a19 · 2025-09-16T22:36:14.000+01:00
updated rule factory with tooltip, describedby, and labeled child
diff --git a/lib/rules/swatchpicker-needs-labelling.ts b/lib/rules/swatchpicker-needs-labelling.ts
@@ -11,12 +11,15 @@ import { makeLabeledControlRule } from "../util/ruleFactory";
 export default ESLintUtils.RuleCreator.withoutDocs(
     makeLabeledControlRule({
         component: "SwatchPicker",
+        messageId: "noUnlabeledSwatchPicker",
+        description: "Accessibility: SwatchPicker must have an accessible name via aria-label, aria-labelledby, Field component, etc..",
         labelProps: ["aria-label"],
         allowFieldParent: true,
-        allowFor: false,
+        allowHtmlFor: false,
         allowLabelledBy: true,
         allowWrappingLabel: false,
-        messageId: "noUnlabeledSwatchPicker",
-        description: "Accessibility: SwatchPicker must have an accessible name via aria-label, aria-labelledby, Field component, etc.."
+        allowTooltipParent: false,
+        allowDescribedBy: false,
+        allowLabeledChild: false
     })
 );
diff --git a/lib/util/hasLabeledChild.ts b/lib/util/hasLabeledChild.ts
@@ -0,0 +1,10 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+import { TSESLint, TSESTree } from "@typescript-eslint/utils";
+
+// eslint-disable-next-line no-unused-vars
+export const hasLabeledChild = (openingElement: TSESTree.JSXOpeningElement, context: TSESLint.RuleContext<string, unknown[]>): boolean => {
+    // TODO: function not yet implemented
+    return false;
+};
diff --git a/lib/util/ruleFactory.ts b/lib/util/ruleFactory.ts
@@ -3,86 +3,79 @@
 
 import { TSESLint, TSESTree } from "@typescript-eslint/utils";
 import { hasNonEmptyProp } from "./hasNonEmptyProp";
-import { hasAssociatedLabelViaAriaLabelledBy, isInsideLabelTag, hasAssociatedLabelViaHtmlFor } from "./labelUtils";
+import {
+    hasAssociatedLabelViaAriaLabelledBy,
+    isInsideLabelTag,
+    hasAssociatedLabelViaHtmlFor,
+    hasAssociatedLabelViaAriaDescribedby
+} from "./labelUtils";
 import { hasFieldParent } from "./hasFieldParent";
 import { elementType } from "jsx-ast-utils";
 import { JSXOpeningElement } from "estree-jsx";
+import { hasToolTipParent } from "./hasTooltipParent";
+import { hasLabeledChild } from "./hasLabeledChild";
 
 export type LabeledControlConfig = {
     component: string | RegExp;
-    labelProps: string[]; // e.g. ["label", "aria-label"]
-    allowFieldParent: boolean; // e.g. <Field label=...><RadioGroup/></Field>
-    allowFor: boolean; // htmlFor
-    allowLabelledBy: boolean; // aria-labelledby
-    allowWrappingLabel: boolean; // <label>...</label>
     messageId: string;
     description: string;
+    labelProps: string[]; // e.g. ["aria-label", "title", "label"]
+    /** Accept a parent <Field label="..."> wrapper as providing the label. */
+    allowFieldParent: boolean; // default false
+    allowHtmlFor: boolean /** Accept <label htmlFor="..."> association. */;
+    allowLabelledBy: boolean /** Accept aria-labelledby association. */;
+    allowWrappingLabel: boolean /** Accept being wrapped in a <label> element. */;
+    allowTooltipParent: boolean /** Accept a parent <Tooltip content="..."> wrapper as providing the label. */;
+    /**
+     * Accept aria-describedby as a labeling strategy.
+     * NOTE: This is discouraged for *primary* labeling; prefer text/aria-label/labelledby.
+     * Keep this off unless a specific component (e.g., Icon-only buttons) intentionally uses it.
+     */
+    allowDescribedBy: boolean;
+    // NEW: treat labeled child content (img alt, svg title, aria-label on role="img") as the name
+    allowLabeledChild: boolean;
 };
 
 /**
  * Returns `true` if the JSX opening element is considered **accessibly labelled**
- * per the rule configuration. This function centralizes all supported labelling
- * strategies so the rule stays small and testable.
+ * per the rule configuration. This centralizes all supported labeling strategies.
  *
- * The supported strategies (gated by `config` flags) are:
- *  1) A parent `<Field>`-like wrapper that provides the label context (`allowFieldParent`).
- *  2) A non-empty inline prop such as `aria-label` or `title` (`labelProps`).
- *  3) Being wrapped by a `<label>` element (`allowWrappingLabel`).
- *  4) Associated `<label for="...">` / `htmlFor` relation (`allowFor`).
- *  5) `aria-labelledby` association to an element with textual content (`allowLabelledBy`).
+ * Supported strategies (gated by config flags):
+ *  1) Parent <Field label="..."> context .............................. (allowFieldParent)
+ *  2) Non-empty inline prop(s) like aria-label/title .................. (labelProps)
+ *  3) Wrapped by a <label> ............................................ (allowWrappingLabel)
+ *  4) <label htmlFor="..."> / htmlFor association ..................... (allowFor)
+ *  5) aria-labelledby association ..................................... (allowLabelledBy)
+ *  6) Parent <Tooltip content="..."> context .......................... (allowTooltipParent)
+ *  7) aria-describedby association (opt-in; discouraged as primary) .... (allowDescribedBy)
+ *  8) treat labeled child content (img alt, svg title, aria-label on role="img") as the name
  *
- * Note: This does not validate contrast or UX; it only checks the existence of
- * an accessible **name** via common HTML/ARIA labelling patterns.
- *
- * @param node - The JSX opening element we’re inspecting (e.g., `<Input ...>` opening node).
- * @param context - ESLint rule context or tree-walker context used by helper functions to
- *              resolve scope/ancestors and collect referenced nodes.
- * @param config - Rule configuration describing which components/props/associations count as labelled.
- *              Expected shape:
- *              - `component: string | RegExp` — component tag name or regex to match.
- *              - `labelProps: string[]` — prop names that, when non-empty, count as labels (e.g., `["aria-label","title"]`).
- *              - `allowFieldParent?: boolean` — if true, a recognized parent “Field” wrapper satisfies labelling.
- *              - `allowWrappingLabel?: boolean` — if true, being inside a `<label>` satisfies labelling.
- *              - `allowFor?: boolean` — if true, `<label htmlFor>` association is considered.
- *              - `allowLabelledBy?: boolean` — if true, `aria-labelledby` association is considered.
- * @returns `true` if any configured labelling strategy succeeds; otherwise `false`.
+ * This checks for presence of an accessible *name* only; not contrast or UX.
  */
 export function hasAccessibleLabel(node: TSESTree.JSXOpeningElement, context: any, config: LabeledControlConfig): boolean {
-    if (config.allowFieldParent && hasFieldParent(context)) return true;
-    if (config.labelProps.some(p => hasNonEmptyProp(node.attributes, p))) return true;
-    if (config.allowWrappingLabel && isInsideLabelTag(context)) return true;
-    if (config.allowFor && hasAssociatedLabelViaHtmlFor(node, context)) return true;
-    if (config.allowLabelledBy && hasAssociatedLabelViaAriaLabelledBy(node, context)) return true;
+    const allowFieldParent = !!config.allowFieldParent;
+    const allowWrappingLabel = !!config.allowWrappingLabel;
+    const allowHtmlFor = !!config.allowHtmlFor;
+    const allowLabelledBy = !!config.allowLabelledBy;
+    const allowTooltipParent = !!config.allowTooltipParent;
+    const allowDescribedBy = !!config.allowDescribedBy;
+    const allowLabeledChild = !!config.allowLabeledChild;
+
+    if (allowFieldParent && hasFieldParent(context)) return true;
+    if (config.labelProps?.some(p => hasNonEmptyProp(node.attributes, p))) return true;
+    if (allowWrappingLabel && isInsideLabelTag(context)) return true;
+    if (allowHtmlFor && hasAssociatedLabelViaHtmlFor(node, context)) return true;
+    if (allowLabelledBy && hasAssociatedLabelViaAriaLabelledBy(node, context)) return true;
+    if (allowTooltipParent && hasToolTipParent(context)) return true;
+    if (allowDescribedBy && hasAssociatedLabelViaAriaDescribedby(node, context)) return true;
+    if (allowLabeledChild && hasLabeledChild(node, context)) return true;
+
     return false;
 }
 
 /**
  * Factory for a minimal, strongly-configurable ESLint rule that enforces
- * accessible labelling on a specific JSX element/component.
- *
- * The rule:
- *  • Matches opening elements by `config.component` (exact name or RegExp).
- *  • Uses `hasAccessibleLabel` to decide whether the element is labelled.
- *  • Reports with `messageId` if no labelling strategy succeeds.
- *
- * Example:
- * ```ts
- * export default makeLabeledControlRule(
- *   {
- *     component: /^(?:input|textarea|Select|ComboBox)$/i,
- *     labelProps: ["aria-label", "aria-labelledby", "title"],
- *     allowFieldParent: true,
- *     allowWrappingLabel: true,
- *     allowFor: true,
- *     allowLabelledBy: true,
- *   },
- *   "missingLabel",
- *   "Provide an accessible label (e.g., via <label>, htmlFor, aria-label, or aria-labelledby)."
- * );
- * ```
- *
- * @param config - See `hasAccessibleLabel` for the configuration fields and semantics.
- * @returns An ESLint `RuleModule` that reports when the configured component lacks an accessible label.
+ * accessible labeling on a specific JSX element/component.
  */
 export function makeLabeledControlRule(config: LabeledControlConfig): TSESLint.RuleModule<string, []> {
     return {
diff --git a/tests/lib/rules/utils/hasLabeledChild.ts b/tests/lib/rules/utils/hasLabeledChild.ts
@@ -0,0 +1,40 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+import { AST_NODE_TYPES, TSESTree } from "@typescript-eslint/types";
+import { TSESLint } from "@typescript-eslint/utils";
+import { hasLabeledChild } from "../../../../lib/util/hasLabeledChild";
+
+// helper for loc/range
+const mockLocRange = () => ({
+    loc: {
+        start: { line: 0, column: 0 },
+        end: { line: 0, column: 0 }
+    },
+    range: [0, 0] as [number, number]
+});
+
+// minimal JSXOpeningElement: <img />
+const openingEl: TSESTree.JSXOpeningElement = {
+    type: AST_NODE_TYPES.JSXOpeningElement,
+    name: {
+        type: AST_NODE_TYPES.JSXIdentifier,
+        name: "img",
+        ...mockLocRange()
+    },
+    attributes: [], // no props
+    selfClosing: true, // <img />
+    ...mockLocRange()
+};
+
+// minimal RuleContext mock
+const mockContext = {
+    report: jest.fn(),
+    getSourceCode: jest.fn()
+} as unknown as TSESLint.RuleContext<string, unknown[]>;
+
+describe("hasLabeledChild", () => {
+    it("returns false for a self-closing <img /> with no labeled children", () => {
+        expect(hasLabeledChild(openingEl, mockContext)).toBe(false);
+    });
+});
diff --git a/tests/lib/rules/utils/ruleFactory.test.ts b/tests/lib/rules/utils/ruleFactory.test.ts
@@ -7,30 +7,47 @@ jest.mock("../../../../lib/util/hasNonEmptyProp", () => ({
     hasNonEmptyProp: jest.fn()
 }));
 jest.mock("../../../../lib/util/labelUtils", () => ({
+    hasAssociatedLabelViaAriaDescribedby: jest.fn(),
     hasAssociatedLabelViaAriaLabelledBy: jest.fn(),
     isInsideLabelTag: jest.fn(),
     hasAssociatedLabelViaHtmlFor: jest.fn()
 }));
 jest.mock("../../../../lib/util/hasFieldParent", () => ({
     hasFieldParent: jest.fn()
 }));
+jest.mock("../../../../lib/util/hasLabeledChild", () => ({
+    hasLabeledChild: jest.fn()
+}));
+jest.mock("../../../../lib/util/hasTooltipParent", () => ({
+    hasToolTipParent: jest.fn()
+}));
 
 import { hasNonEmptyProp } from "../../../../lib/util/hasNonEmptyProp";
-import { hasAssociatedLabelViaAriaLabelledBy, isInsideLabelTag, hasAssociatedLabelViaHtmlFor } from "../../../../lib/util/labelUtils";
+import {
+    hasAssociatedLabelViaAriaLabelledBy,
+    isInsideLabelTag,
+    hasAssociatedLabelViaHtmlFor,
+    hasAssociatedLabelViaAriaDescribedby
+} from "../../../../lib/util/labelUtils";
 import { hasFieldParent } from "../../../../lib/util/hasFieldParent";
 
 // Import the module under test AFTER mocks
 import { hasAccessibleLabel, LabeledControlConfig, makeLabeledControlRule } from "../../../../lib/util/ruleFactory";
 import type { TSESTree } from "@typescript-eslint/utils";
 import { Rule, RuleTester } from "eslint";
+import { hasLabeledChild } from "../../../../lib/util/hasLabeledChild";
+import { hasToolTipParent } from "../../../../lib/util/hasTooltipParent";
 
 // Helper: reset all mocks to a default "false" stance
 const resetAllMocksToFalse = () => {
     (hasNonEmptyProp as jest.Mock).mockReset().mockReturnValue(false);
     (hasAssociatedLabelViaAriaLabelledBy as jest.Mock).mockReset().mockReturnValue(false);
+    (hasAssociatedLabelViaAriaDescribedby as jest.Mock).mockReset().mockReturnValue(false);
     (isInsideLabelTag as jest.Mock).mockReset().mockReturnValue(false);
     (hasAssociatedLabelViaHtmlFor as jest.Mock).mockReset().mockReturnValue(false);
     (hasFieldParent as jest.Mock).mockReset().mockReturnValue(false);
+    (hasLabeledChild as jest.Mock).mockReset().mockReturnValue(false);
+    (hasToolTipParent as jest.Mock).mockReset().mockReturnValue(false);
 };
 
 beforeEach(() => {
@@ -59,11 +76,14 @@ describe("hasAccessibleLabel (unit)", () => {
         component: "RadioGroup",
         labelProps: ["label", "aria-label"],
         allowFieldParent: true,
-        allowFor: true,
+        allowHtmlFor: true,
         allowLabelledBy: true,
         allowWrappingLabel: true,
+        allowTooltipParent: true,
+        allowDescribedBy: true,
         messageId: "errorMsg",
-        description: "anything"
+        description: "anything",
+        allowLabeledChild: false
     };
 
     test("returns false when no heuristics pass", () => {
@@ -78,6 +98,12 @@ describe("hasAccessibleLabel (unit)", () => {
         expect(hasAccessibleLabel(node, {}, cfg)).toBe(true);
     });
 
+    test("true when allowTooltipParent and hasTooltipParent(ctx) === true", () => {
+        (hasToolTipParent as jest.Mock).mockReturnValue(true);
+        const node = makeOpeningElement("RadioGroup");
+        expect(hasAccessibleLabel(node, {}, cfg)).toBe(true);
+    });
+
     test("true when a label prop is non-empty via hasNonEmptyProp", () => {
         (hasNonEmptyProp as jest.Mock).mockImplementation((attrs, name) => (name === "label" ? true : false));
         const node = makeOpeningElement("RadioGroup", [
@@ -106,6 +132,12 @@ describe("hasAccessibleLabel (unit)", () => {
         const node = makeOpeningElement("RadioGroup");
         expect(hasAccessibleLabel(node, {}, cfg)).toBe(true);
     });
+
+    test("true when allowDescribedByBy and hasAssociatedLabelViaAriaDescribedBy(...) === true", () => {
+        (hasAssociatedLabelViaAriaDescribedby as jest.Mock).mockReturnValue(true);
+        const node = makeOpeningElement("RadioGroup");
+        expect(hasAccessibleLabel(node, {}, cfg)).toBe(true);
+    });
 });
 
 /* -------------------------------------------------------------------------- */
@@ -119,11 +151,14 @@ describe("makeLabeledControlRule (RuleTester integration)", () => {
         component: "RadioGroup",
         labelProps: ["label", "aria-label"],
         allowFieldParent: true,
-        allowFor: true,
+        allowHtmlFor: true,
         allowLabelledBy: true,
         allowWrappingLabel: true,
+        allowTooltipParent: true,
+        allowDescribedBy: true,
         messageId: "noUnlabeledRadioGroup",
-        description: "Accessibility: RadioGroup must have a programmatic and visual label."
+        description: "Accessibility: RadioGroup must have a programmatic and visual label.",
+        allowLabeledChild: false
     };
 
     // 1) No heuristics -> report
@@ -253,4 +288,48 @@ describe("makeLabeledControlRule (RuleTester integration)", () => {
             invalid: []
         });
     });
+
+    // 8) in rare cases
+    describe("accepts when aria-describedby is present", () => {
+        beforeEach(() => {
+            resetAllMocksToFalse();
+            (hasAssociatedLabelViaAriaDescribedby as jest.Mock).mockImplementation(
+                (node: any) =>
+                    Array.isArray(node?.attributes) &&
+                    node.attributes.some((a: any) => a?.type === "JSXAttribute" && a?.name?.name === "aria-describedby")
+            );
+        });
+
+        const rule = makeLabeledControlRule(baseCfg);
+
+        ruleTester.run("no-unlabeled-radio-group (aria-describedby)", rule as unknown as Rule.RuleModule, {
+            valid: [{ code: `<RadioGroup aria-describedby="groupLabelId" />` }],
+            invalid: [{ code: `<RadioGroup />`, errors: [{ messageId: baseCfg.messageId }] }]
+        });
+    });
+
+    // 9) tool tip parent
+    describe("accepts when parent Tooltip provides label", () => {
+        beforeEach(() => {
+            resetAllMocksToFalse();
+            (hasToolTipParent as jest.Mock).mockReturnValue(true);
+        });
+
+        const rule = makeLabeledControlRule(baseCfg);
+
+        ruleTester.run("no-unlabeled-radio-group (Tooltip parent)", rule as unknown as Rule.RuleModule, {
+            valid: [
+                {
+                    code: `
+                        <>
+                        <Tooltip label="Account type">
+                            <RadioGroup />
+                        </Tooltip>
+                        </>
+                    `
+                }
+            ],
+            invalid: []
+        });
+    });
 });
