Skip to content

Commit e970efd

Browse files
author
Iryna Vasylenko
committed
Refactor Tag rules to use ruleFactory
1 parent d4ca313 commit e970efd

File tree

4 files changed

+117
-67
lines changed

4 files changed

+117
-67
lines changed

lib/rules/tag-needs-name.ts

Lines changed: 18 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,26 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
import { ESLintUtils, TSESTree } from "@typescript-eslint/utils";
5-
import { elementType } from "jsx-ast-utils";
6-
import { hasNonEmptyProp } from "../util/hasNonEmptyProp";
7-
import { hasTextContentChild } from "../util/hasTextContentChild";
8-
import { hasAssociatedLabelViaAriaLabelledBy } from "../util/labelUtils";
9-
import { JSXOpeningElement } from "estree-jsx";
4+
import { ESLintUtils } from "@typescript-eslint/utils";
5+
import { makeLabeledControlRule } from "../util/ruleFactory";
106

117
//------------------------------------------------------------------------------
128
// Rule Definition
139
//------------------------------------------------------------------------------
1410

15-
const rule = ESLintUtils.RuleCreator.withoutDocs({
16-
defaultOptions: [],
17-
meta: {
18-
type: "problem",
19-
docs: {
20-
description:
21-
"This rule aims to ensure that Tag component have an accessible name via text content, aria-label, or aria-labelledby.",
22-
recommended: "strict",
23-
url: "https://react.fluentui.dev/?path=/docs/components-tag-tag--docs"
24-
},
25-
fixable: undefined,
26-
schema: [],
27-
messages: {
28-
missingAriaLabel: "Accessibility: Tag must have an accessible name"
29-
}
30-
},
31-
create(context) {
32-
return {
33-
// visitor functions for different types of nodes
34-
JSXElement(node: TSESTree.JSXElement) {
35-
const openingElement = node.openingElement;
36-
37-
// if it is not a Tag, return
38-
if (elementType(openingElement as JSXOpeningElement) !== "Tag") {
39-
return;
40-
}
41-
42-
// Check if tag has any accessible name
43-
const hasTextContent = hasTextContentChild(node);
44-
const hasAriaLabel = hasNonEmptyProp(openingElement.attributes, "aria-label");
45-
const hasAriaLabelledBy = hasAssociatedLabelViaAriaLabelledBy(openingElement, context);
46-
const hasAccessibleName = hasTextContent || hasAriaLabel || hasAriaLabelledBy;
47-
48-
if (!hasAccessibleName) {
49-
context.report({
50-
node,
51-
messageId: `missingAriaLabel`
52-
});
53-
}
54-
}
55-
};
56-
}
57-
});
58-
59-
export default rule;
11+
export default ESLintUtils.RuleCreator.withoutDocs(
12+
makeLabeledControlRule({
13+
component: "Tag",
14+
messageId: "missingAriaLabel",
15+
description: "Accessibility: Tag must have an accessible name",
16+
labelProps: ["aria-label"],
17+
allowFieldParent: false,
18+
allowHtmlFor: false,
19+
allowLabelledBy: true,
20+
allowWrappingLabel: false,
21+
allowTooltipParent: false,
22+
allowDescribedBy: false,
23+
allowLabeledChild: false,
24+
allowTextContentChild: true
25+
})
26+
);

lib/util/hasTriggerProp.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
import { TSESTree } from "@typescript-eslint/utils";
5+
import { hasProp } from "jsx-ast-utils";
6+
import { JSXAttribute } from "estree-jsx";
7+
8+
/**
9+
* Checks if a component has a specific trigger prop.
10+
* This is useful for rules that only apply when certain props are present
11+
* (e.g., dismissible, expandable, collapsible, etc.)
12+
*/
13+
export const hasTriggerProp = (openingElement: TSESTree.JSXOpeningElement, triggerProp: string): boolean => {
14+
return hasProp(openingElement.attributes as JSXAttribute[], triggerProp);
15+
};

lib/util/hasValidNestedProp.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
import { TSESTree } from "@typescript-eslint/utils";
5+
import { getProp, getPropValue } from "jsx-ast-utils";
6+
import { JSXAttribute } from "estree-jsx";
7+
8+
/**
9+
* Checks if a value is a non-empty string
10+
*/
11+
const isNonEmptyString = (value: any): boolean => {
12+
return typeof value === "string" && value.trim().length > 0;
13+
};
14+
15+
/**
16+
* Validates nested properties within a complex prop (object prop).
17+
* This is useful for props like dismissIcon={{ "aria-label": "close", role: "button" }}
18+
* where you need to check specific properties within the object.
19+
*/
20+
export const hasValidNestedProp = (openingElement: TSESTree.JSXOpeningElement, propName: string, nestedKey: string): boolean => {
21+
const prop = getProp(openingElement.attributes as JSXAttribute[], propName);
22+
if (!prop) {
23+
return false;
24+
}
25+
26+
const propValue = getPropValue(prop);
27+
return Boolean(propValue && typeof propValue === "object" && isNonEmptyString((propValue as any)[nestedKey]));
28+
};

lib/util/ruleFactory.ts

Lines changed: 56 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,31 @@ import { elementType } from "jsx-ast-utils";
1414
import { JSXOpeningElement } from "estree-jsx";
1515
import { hasToolTipParent } from "./hasTooltipParent";
1616
import { hasLabeledChild } from "./hasLabeledChild";
17+
import { hasTextContentChild } from "./hasTextContentChild";
18+
import { hasTriggerProp } from "./hasTriggerProp";
1719

1820
export type LabeledControlConfig = {
1921
component: string | RegExp;
2022
messageId: string;
2123
description: string;
2224
labelProps: string[]; // e.g. ["aria-label", "title", "label"]
23-
/** Accept a parent <Field label="..."> wrapper as providing the label. */
24-
allowFieldParent: boolean; // default false
25-
allowHtmlFor: boolean /** Accept <label htmlFor="..."> association. */;
26-
allowLabelledBy: boolean /** Accept aria-labelledby association. */;
27-
allowWrappingLabel: boolean /** Accept being wrapped in a <label> element. */;
28-
allowTooltipParent: boolean /** Accept a parent <Tooltip content="..."> wrapper as providing the label. */;
25+
allowFieldParent: boolean; // Accept a parent <Field label="..."> wrapper as providing the label.
26+
allowHtmlFor: boolean; // Accept <label htmlFor="..."> association.
27+
allowLabelledBy: boolean; // Accept aria-labelledby association.
28+
allowWrappingLabel: boolean; // Accept being wrapped in a <label> element.
29+
allowTooltipParent: boolean; // Accept a parent <Tooltip content="..."> wrapper as providing the label.
2930
/**
3031
* Accept aria-describedby as a labeling strategy.
3132
* NOTE: This is discouraged for *primary* labeling; prefer text/aria-label/labelledby.
3233
* Keep this off unless a specific component (e.g., Icon-only buttons) intentionally uses it.
3334
*/
3435
allowDescribedBy: boolean;
35-
// NEW: treat labeled child content (img alt, svg title, aria-label on role="img") as the name
36-
allowLabeledChild: boolean;
36+
allowLabeledChild: boolean; // Accept labeled child elements to provide the label e.g. <Button><img alt="..." /></Button>
37+
allowTextContentChild?: boolean; // Accept text children to provide the label e.g. <Button>Click me</Button>
38+
/** Only apply rule when this trigger prop is present (e.g., "dismissible", "disabled") */
39+
triggerProp?: string;
40+
/** Custom validation function for complex scenarios */
41+
customValidator?: Function;
3742
};
3843

3944
/**
@@ -49,6 +54,8 @@ export type LabeledControlConfig = {
4954
* 6) Parent <Tooltip content="..."> context .......................... (allowTooltipParent)
5055
* 7) aria-describedby association (opt-in; discouraged as primary) .... (allowDescribedBy)
5156
* 8) treat labeled child content (img alt, svg title, aria-label on role="img") as the name
57+
* 9) Conditional application based on trigger prop ................... (triggerProp)
58+
* 10) Custom validation for complex scenarios ......................... (customValidator)
5259
*
5360
* This checks for presence of an accessible *name* only; not contrast or UX.
5461
*/
@@ -92,19 +99,52 @@ export function makeLabeledControlRule(config: LabeledControlConfig): TSESLint.R
9299
defaultOptions: [],
93100

94101
create(context: TSESLint.RuleContext<string, []>) {
95-
return {
96-
JSXOpeningElement(node: TSESTree.JSXOpeningElement) {
97-
// elementType expects an ESTree JSX node — cast is fine
98-
const name = elementType(node as unknown as JSXOpeningElement);
99-
const matches = typeof config.component === "string" ? name === config.component : config.component.test(name);
102+
const validateElement = (node: TSESTree.JSXOpeningElement, parentElement?: TSESTree.JSXElement) => {
103+
// elementType expects an ESTree JSX node — cast is fine
104+
const name = elementType(node as unknown as JSXOpeningElement);
105+
const matches = typeof config.component === "string" ? name === config.component : config.component.test(name);
100106

101-
if (!matches) return;
107+
if (!matches) return;
102108

103-
if (!hasAccessibleLabel(node, context, config)) {
104-
context.report({ node, messageId: config.messageId });
109+
// If trigger prop is specified, only apply rule when it's present
110+
if (config.triggerProp && !hasTriggerProp(node, config.triggerProp)) {
111+
return;
112+
}
113+
114+
// Use custom validator if provided, otherwise use standard accessibility check
115+
let isValid: boolean;
116+
if (config.customValidator) {
117+
isValid = config.customValidator(node);
118+
} else {
119+
// For text content checking, we need the parent element
120+
if (config.allowTextContentChild && parentElement) {
121+
// Create a modified config for hasAccessibleLabel that includes text content checking
122+
const modifiedConfig = { ...config };
123+
isValid = hasAccessibleLabel(node, context, modifiedConfig) || hasTextContentChild(parentElement);
124+
} else {
125+
isValid = hasAccessibleLabel(node, context, config);
105126
}
106127
}
128+
129+
if (!isValid) {
130+
context.report({ node, messageId: config.messageId });
131+
}
107132
};
133+
134+
// If we need text content checking, we must visit JSXElement to get access to children
135+
if (config.allowTextContentChild) {
136+
return {
137+
JSXElement(node: TSESTree.JSXElement) {
138+
validateElement(node.openingElement, node);
139+
}
140+
};
141+
} else {
142+
return {
143+
JSXOpeningElement(node: TSESTree.JSXOpeningElement) {
144+
validateElement(node);
145+
}
146+
};
147+
}
108148
}
109149
};
110150
}

0 commit comments

Comments
 (0)