Skip to content

Commit 3645a88

Browse files
aubreyquinnaubreyquinn
committed
fixed false negative on the field component
1 parent 362a590 commit 3645a88

10 files changed

+228
-89
lines changed

‎docs/rules/field-needs-labelling.md

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
1-
# Accessibility: Field must have either label, validationMessage and hint attributes (`@microsoft/fluentui-jsx-a11y/field-needs-labelling`)
1+
# Accessibility: Field must have label attribute (`@microsoft/fluentui-jsx-a11y/field-needs-labelling`)
22

33
💼 This rule is enabled in the ✅ `recommended` config.
44

55
<!-- end auto-generated rule header -->
66

7-
Field must have `label` prop and either `validationMessage` or `hint` prop.
7+
Field must have `label` prop.
88

99
<https://www.w3.org/TR/html-aria/>
1010

1111
## Ways to fix
1212

1313
- Make sure that Field component has following props:
1414
- `label`
15-
- `validationMessage` or `hint`
1615

1716
## Rule Details
1817

@@ -21,41 +20,33 @@ This rule aims to make Field component accessible.
2120
Examples of **incorrect** code for this rule:
2221

2322
```jsx
24-
<Field
25-
label="Example field"
26-
validationState="success"
27-
>
23+
<Field label="Example field" validationState="success">
2824
<ProgressBar value={0.5} max={1} />
2925
</Field>
3026
```
3127

3228
```jsx
33-
<Field
34-
validationState="success"
35-
hint="This is a hint."
36-
>
29+
<Field validationState="success" hint="This is a hint.">
3730
<ProgressBar value={0.5} max={1} />
3831
</Field>
3932
```
4033

4134
Examples of **correct** code for this rule:
4235

4336
```jsx
44-
<Field
45-
label="Example field"
46-
validationState="success"
47-
validationMessage="This is a success message."
48-
>
37+
<Field label="Example field">
38+
<Input />
39+
</Field>
40+
```
41+
42+
```jsx
43+
<Field label="Example field" validationState="success" validationMessage="This is a success message.">
4944
<ProgressBar value={0.5} max={1} />
5045
</Field>
5146
```
5247

5348
```jsx
54-
<Field
55-
label="Example field"
56-
validationState="success"
57-
hint="This is a hint."
58-
>
49+
<Field label="Example field" validationState="success" hint="This is a hint.">
5950
<ProgressBar value={0.5} max={1} />
6051
</Field>
6152
```

‎lib/rules/field-needs-labelling.js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ module.exports = {
1414
meta: {
1515
// possible error messages for the rule
1616
messages: {
17-
noUnlabelledField: "Accessibility: Field must have either label, validationMessage and hint attributes"
17+
noUnlabelledField: "Accessibility: Field must have label"
1818
},
1919
// "problem" means the rule is identifying code that either will cause an error or may cause a confusing behavior: https://eslint.org/docs/latest/developer-guide/working-with-rules
2020
type: "problem",
2121
// docs for the rule
2222
docs: {
23-
description: "Accessibility: Field must have either label, validationMessage and hint attributes",
23+
description: "Accessibility: Field must have label",
2424
recommended: true,
2525
url: "https://www.w3.org/TR/html-aria/" // URL to the documentation page for this rule
2626
},
@@ -36,10 +36,7 @@ module.exports = {
3636
return;
3737
}
3838

39-
if (
40-
hasNonEmptyProp(node.attributes, "label", true) &&
41-
(hasNonEmptyProp(node.attributes, "validationMessage", true) || hasNonEmptyProp(node.attributes, "hint", true))
42-
) {
39+
if (hasNonEmptyProp(node.attributes, "label")) {
4340
return;
4441
}
4542

@@ -52,4 +49,3 @@ module.exports = {
5249
};
5350
}
5451
};
55-

‎lib/util/flattenChildren.js

Lines changed: 0 additions & 21 deletions
This file was deleted.

‎lib/util/flattenChildren.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
import { TSESTree } from "@typescript-eslint/types";
5+
6+
// Flatten the JSX tree structure by recursively collecting all child elements
7+
export default function flattenChildren(node: TSESTree.JSXElement): TSESTree.JSXElement[] {
8+
const flatChildren: TSESTree.JSXElement[] = [];
9+
10+
if (node.children && node.children.length > 0) {
11+
node.children.forEach(child => {
12+
if (child.type === "JSXElement") {
13+
const jsxChild = child as TSESTree.JSXElement;
14+
flatChildren.push(jsxChild, ...flattenChildren(jsxChild));
15+
}
16+
});
17+
}
18+
19+
return flatChildren;
20+
}

‎lib/util/hasFieldParent.js

Lines changed: 0 additions & 31 deletions
This file was deleted.

‎lib/util/hasFieldParent.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
import { TSESTree } from "@typescript-eslint/types";
5+
import { elementType } from "jsx-ast-utils";
6+
import { TSESLint } from "@typescript-eslint/utils";
7+
import { JSXOpeningElement } from "estree-jsx";
8+
9+
// Function to check if the current node has a "Field" parent JSXElement
10+
export function hasFieldParent(context: TSESLint.RuleContext<string, unknown[]>): boolean {
11+
const ancestors: TSESTree.Node[] = context.getAncestors();
12+
13+
if (ancestors == null || ancestors.length === 0) {
14+
return false;
15+
}
16+
17+
let field = false;
18+
19+
ancestors.forEach(item => {
20+
if (
21+
item.type === "JSXElement" &&
22+
item.openingElement != null &&
23+
item.openingElement.type === "JSXOpeningElement" &&
24+
elementType(item.openingElement as unknown as JSXOpeningElement) === "Field"
25+
) {
26+
field = true;
27+
}
28+
});
29+
30+
return field;
31+
}

‎lib/util/hasLabelledChildImage.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
const { flattenChildren } = require("./flattenChildren");
4+
const { flattenChildren } = require("./flattenChildren.ts");
55
const { hasProp, getPropValue } = require("jsx-ast-utils");
66
const { hasNonEmptyProp } = require("./hasNonEmptyProp");
77
const { fluentImageComponents, imageDomNodes } = require("../applicableComponents/imageBasedComponents");

‎tests/lib/rules/utils/flattenChildren.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { flattenChildren } from "../../../../lib/util/flattenChildren";
2+
import { TSESTree } from "@typescript-eslint/types";
3+
4+
describe("flattenChildren", () => {
5+
test("should return an empty array when node has no children", () => {
6+
const node: TSESTree.JSXElement = {
7+
type: "JSXElement",
8+
openingElement: {} as TSESTree.JSXOpeningElement,
9+
closingElement: null,
10+
children: []
11+
};
12+
const result = flattenChildren(node);
13+
expect(result).toEqual([]);
14+
});
15+
16+
test("should return the same array when node has no nested JSXElement children", () => {
17+
const node: TSESTree.JSXElement = {
18+
type: "JSXElement",
19+
openingElement: {} as TSESTree.JSXOpeningElement,
20+
closingElement: null,
21+
children: [
22+
{ type: "JSXText", value: "Hello" } as TSESTree.JSXText,
23+
{ type: "JSXExpressionContainer" } as TSESTree.JSXExpressionContainer
24+
]
25+
};
26+
const result = flattenChildren(node);
27+
expect(result).toEqual([]);
28+
});
29+
30+
test("should flatten nested JSXElement children", () => {
31+
const node: TSESTree.JSXElement = {
32+
type: "JSXElement",
33+
openingElement: {} as TSESTree.JSXOpeningElement,
34+
closingElement: null,
35+
children: [
36+
{
37+
type: "JSXElement",
38+
openingElement: {} as TSESTree.JSXOpeningElement,
39+
closingElement: null,
40+
children: [{ type: "JSXText", value: "Nested" } as TSESTree.JSXText]
41+
} as TSESTree.JSXElement
42+
]
43+
};
44+
const result = flattenChildren(node);
45+
expect(result).toEqual([node.children[0], { type: "JSXText", value: "Nested" }]);
46+
});
47+
48+
test("should handle mixed nested and non-nested JSXElement children", () => {
49+
const node: TSESTree.JSXElement = {
50+
type: "JSXElement",
51+
openingElement: {} as TSESTree.JSXOpeningElement,
52+
closingElement: null,
53+
children: [
54+
{
55+
type: "JSXElement",
56+
openingElement: {} as TSESTree.JSXOpeningElement,
57+
closingElement: null,
58+
children: [
59+
{ type: "JSXText", value: "Text" } as TSESTree.JSXText,
60+
{
61+
type: "JSXElement",
62+
openingElement: {} as TSESTree.JSXOpeningElement,
63+
closingElement: null,
64+
children: []
65+
} as TSESTree.JSXElement
66+
]
67+
} as TSESTree.JSXElement
68+
]
69+
};
70+
const result = flattenChildren(node);
71+
expect(result).toEqual([node.children[0], { type: "JSXText", value: "Text" }, node.children[0].children[1]]);
72+
});
73+
});

‎tests/lib/rules/utils/hasFieldParent.test.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import { hasFieldParent } from "../../../../lib/util/hasFieldParent";
2+
import { TSESTree } from "@typescript-eslint/types";
3+
import { TSESLint } from "@typescript-eslint/utils";
4+
import { elementType } from "jsx-ast-utils";
5+
6+
// Mock the elementType function to return "Field" when needed
7+
jest.mock("jsx-ast-utils", () => ({
8+
elementType: jest.fn()
9+
}));
10+
11+
// Mock context to simulate ESLint's RuleContext
12+
const createMockContext = (ancestors: TSESTree.Node[]): TSESLint.RuleContext<string, []> => ({
13+
// Mock the required properties of RuleContext
14+
id: "mockRule",
15+
options: [],
16+
settings: {},
17+
parserPath: "",
18+
parserOptions: {},
19+
getCwd: jest.fn(),
20+
getFilename: jest.fn(() => "mockFile.js"),
21+
getScope: jest.fn(),
22+
report: jest.fn(),
23+
getAncestors: () => ancestors,
24+
getSourceCode: jest.fn(),
25+
getDeclaredVariables: function (node: TSESTree.Node): readonly TSESLint.Scope.Variable[] {
26+
throw new Error("Function not implemented.");
27+
},
28+
markVariableAsUsed: function (name: string): boolean {
29+
throw new Error("Function not implemented.");
30+
}
31+
});
32+
33+
describe("hasFieldParent", () => {
34+
beforeEach(() => {
35+
jest.clearAllMocks();
36+
});
37+
38+
test("should return false when there are no ancestors", () => {
39+
const mockContext = createMockContext([]);
40+
const result = hasFieldParent(mockContext);
41+
expect(result).toBe(false);
42+
});
43+
44+
test("should return false when there are ancestors but none are JSXElements", () => {
45+
const mockContext = createMockContext([{ type: "Literal" } as TSESTree.Literal]);
46+
const result = hasFieldParent(mockContext);
47+
expect(result).toBe(false);
48+
});
49+
50+
test('should return false when none of the ancestors are "Field" elements', () => {
51+
const mockContext = createMockContext([
52+
{
53+
type: "JSXElement",
54+
openingElement: { type: "JSXOpeningElement" }
55+
} as TSESTree.JSXElement
56+
]);
57+
(elementType as jest.Mock).mockReturnValue("NotAField");
58+
const result = hasFieldParent(mockContext);
59+
expect(result).toBe(false);
60+
});
61+
62+
test('should return true when one of the ancestors is a "Field" element', () => {
63+
const mockContext = createMockContext([
64+
{
65+
type: "JSXElement",
66+
openingElement: { type: "JSXOpeningElement" }
67+
} as TSESTree.JSXElement
68+
]);
69+
(elementType as jest.Mock).mockReturnValue("Field");
70+
const result = hasFieldParent(mockContext);
71+
expect(result).toBe(true);
72+
});
73+
74+
test('should handle multiple ancestors with one "Field" element', () => {
75+
const mockContext = createMockContext([
76+
{ type: "JSXElement", openingElement: { type: "JSXOpeningElement" } } as TSESTree.JSXElement,
77+
{ type: "Literal" } as TSESTree.Literal,
78+
{ type: "JSXElement", openingElement: { type: "JSXOpeningElement" } } as TSESTree.JSXElement
79+
]);
80+
(elementType as jest.Mock).mockReturnValueOnce("NotAField").mockReturnValueOnce("Field");
81+
const result = hasFieldParent(mockContext);
82+
expect(result).toBe(true);
83+
});
84+
});

0 commit comments

Comments
 (0)