Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eslint-plugin-template): [label-has-associated-control] check id's in the for attribute of a label for existence #1761

Merged
merged 6 commits into from May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -27,6 +27,7 @@ The rule accepts an options object with the following properties:

```ts
interface Options {
checkIds?: boolean;
/**
* Default: `["input","meter","output","progress","select","textarea"]`
*/
Expand Down Expand Up @@ -84,6 +85,37 @@ interface Options {

#### Custom Config

```json
{
"rules": {
"@angular-eslint/template/label-has-associated-control": [
"error",
{
"checkIds": true
}
]
}
}
```

<br>

#### ❌ Invalid Code

```html
<label for="id">Label</label>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<input id="otherId" />
```

<br>

---

<br>

#### Custom Config

```json
{
"rules": {
Expand Down Expand Up @@ -253,6 +285,36 @@ interface Options {
</app-label>
```

<br>

---

<br>

#### Custom Config

```json
{
"rules": {
"@angular-eslint/template/label-has-associated-control": [
"error",
{
"checkIds": true
}
]
}
}
```

<br>

#### ✅ Valid Code

```html
<input type="radio" id="id"/>
<label for="id"></label>
```

</details>

<br>
@@ -1,4 +1,7 @@
import type { TmplAstElement } from '@angular-eslint/bundled-angular-compiler';
import type {
AST,
TmplAstElement,
} from '@angular-eslint/bundled-angular-compiler';
import { getTemplateParserServices } from '@angular-eslint/utils';
import { createESLintRule } from '../utils/create-eslint-rule';
import { isChildNodeOf } from '../utils/is-child-node-of';
Expand All @@ -11,6 +14,7 @@ type Options = [
{
readonly controlComponents?: readonly string[];
readonly labelComponents?: readonly LabelComponent[];
readonly checkIds?: boolean;
},
];
export type MessageIds = 'labelHasAssociatedControl';
Expand All @@ -29,6 +33,7 @@ const DEFAULT_LABEL_COMPONENTS: readonly LabelComponent[] = [
const DEFAULT_OPTIONS: Options[0] = {
controlComponents: DEFAULT_CONTROL_COMPONENTS,
labelComponents: DEFAULT_LABEL_COMPONENTS,
checkIds: false,
};

export default createESLintRule<Options, MessageIds>({
Expand All @@ -43,6 +48,7 @@ export default createESLintRule<Options, MessageIds>({
{
additionalProperties: false,
properties: {
checkIds: { type: 'boolean' },
controlComponents: {
items: { type: 'string' },
type: 'array',
Expand Down Expand Up @@ -75,7 +81,7 @@ export default createESLintRule<Options, MessageIds>({
},
},
defaultOptions: [DEFAULT_OPTIONS],
create(context, [{ controlComponents, labelComponents }]) {
create(context, [{ checkIds, controlComponents, labelComponents }]) {
const parserServices = getTemplateParserServices(context);
const allControlComponents: ReadonlySet<string> = new Set([
...DEFAULT_CONTROL_COMPONENTS,
Expand All @@ -85,34 +91,58 @@ export default createESLintRule<Options, MessageIds>({
...DEFAULT_LABEL_COMPONENTS,
...(labelComponents ?? []),
] as const;
const labelSelectors = allLabelComponents.map(({ selector }) => selector);
const labelComponentsPattern = toPattern(labelSelectors);
let inputItems: TmplAstElement[] = [];
let labelItems: TmplAstElement[] = [];

return {
[`Element$1[name=${labelComponentsPattern}]`](node: TmplAstElement) {
[`Element$1`](node: TmplAstElement) {
if (allControlComponents.has(node.name)) {
inputItems.push(node);
}
const element = allLabelComponents.find(
({ selector }) => selector === node.name,
);
if (element) {
labelItems.push(node);
}
},

if (!element) return;
onCodePathEnd() {
for (const node of labelItems) {
const element = allLabelComponents.find(
({ selector }) => selector === node.name,
);

const attributesInputs: ReadonlySet<string> = new Set(
[...node.attributes, ...node.inputs].map(({ name }) => name),
);
const hasInput = element.inputs?.some((input) =>
attributesInputs.has(input),
);
if (!element) continue;
const attributesInputs: ReadonlyMap<string, string | AST> = new Map(
[...node.attributes, ...node.inputs].map(({ name, value }) => [
name,
value,
]),
);
const inputValues = (
element.inputs?.map((input) => attributesInputs.get(input)) ?? []
).filter(filterUndefined);
let hasFor = inputValues.length > 0;
if (hasFor && checkIds) {
const value = inputValues[0];
hasFor = hasControlComponentWithId(inputItems, value);
}
if (hasFor || hasControlComponentIn(allControlComponents, node)) {
continue;
}
const loc = parserServices.convertNodeSourceSpanToLoc(
node.sourceSpan,
);

if (hasInput || hasControlComponentIn(allControlComponents, node)) {
return;
context.report({
loc,
messageId: 'labelHasAssociatedControl',
});
}

const loc = parserServices.convertNodeSourceSpanToLoc(node.sourceSpan);

context.report({
loc,
messageId: 'labelHasAssociatedControl',
});
inputItems = [];
labelItems = [];
},
};
},
Expand All @@ -129,6 +159,19 @@ function hasControlComponentIn(
);
}

function toPattern(value: readonly unknown[]): RegExp {
return RegExp(`^(${value.join('|')})$`);
function hasControlComponentWithId(
controlComponents: TmplAstElement[],
id: string | AST,
) {
return Boolean(
[...controlComponents].some((node) => {
return !![...node.attributes, ...node.inputs].find(
(input) => input.name === 'id' && input.value === id,
);
}),
);
}

function filterUndefined<T>(value: T | undefined | null): value is T {
return value !== undefined && value !== null;
}
Expand Up @@ -58,6 +58,13 @@ export const valid = [
},
],
},
{
code: `
<input type="radio" id="id"/>
<label for="id"></label>
`,
options: [{ checkIds: true }],
},
];

export const invalid = [
Expand All @@ -69,6 +76,17 @@ export const invalid = [
~~~~~~~~~~~~~~~~~~~~
`,
}),
convertAnnotatedSourceToFailureCase({
messageId,
description:
'should fail if a label does a "for" attribute with a non matching id and ids are checked',
annotatedSource: `
<label for="id">Label</label>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<input id="otherId" />
`,
options: [{ checkIds: true }],
}),
convertAnnotatedSourceToFailureCase({
messageId,
description:
Expand Down