Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1fb5d8a

Browse filesBrowse files
rafaelss95mgechev
rafaelss95
authored and
mgechev
committed
fix(rule): template-accessibility-label-for not recognizing options and interpolated values (#812)
* refactor: simplify and rename mayContainChildComponent function * fix(rule): template-accessibility-label-for not recognizing options and interpolated values
1 parent 8d816c8 commit 1fb5d8a
Copy full SHA for 1fb5d8a

4 files changed

+224-120Lines changed: 224 additions & 120 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/templateAccessibilityLabelForRule.ts‎

Copy file name to clipboardExpand all lines: src/templateAccessibilityLabelForRule.ts
+131-55Lines changed: 131 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,91 +1,167 @@
11
import { ElementAst } from '@angular/compiler';
2-
import { IRuleMetadata, RuleFailure, Rules, Utils } from 'tslint/lib';
2+
import { IOptions, IRuleMetadata, RuleFailure } from 'tslint/lib';
3+
import { AbstractRule } from 'tslint/lib/rules';
4+
import { dedent } from 'tslint/lib/utils';
35
import { SourceFile } from 'typescript/lib/typescript';
6+
import { ComponentMetadata } from './angular/metadata';
47
import { NgWalker, NgWalkerConfig } from './angular/ngWalker';
58
import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor';
6-
import { mayContainChildComponent } from './util/mayContainChildComponent';
9+
import { isChildNodeOf } from './util/isChildNodeOf';
10+
import { objectKeys } from './util/objectKeys';
711

8-
interface ILabelForOptions {
9-
labelComponents: string[];
10-
labelAttributes: string[];
11-
controlComponents: string[];
12-
}
13-
export class Rule extends Rules.AbstractRule {
12+
const OPTION_CONTROL_COMPONENTS = 'controlComponents';
13+
const OPTION_LABEL_ATTRIBUTES = 'labelAttributes';
14+
const OPTION_LABEL_COMPONENTS = 'labelComponents';
15+
16+
const OPTION_SCHEMA_VALUE = {
17+
properties: {
18+
items: {
19+
type: 'string'
20+
},
21+
type: 'array',
22+
uniqueItems: true
23+
},
24+
type: 'object'
25+
};
26+
27+
const DEFAULT_CONTROL_COMPONENTS = ['button', 'input', 'meter', 'output', 'progress', 'select', 'textarea'];
28+
const DEFAULT_LABEL_ATTRIBUTES = ['for', 'htmlFor'];
29+
const DEFAULT_LABEL_COMPONENTS = ['label'];
30+
31+
type OptionKeys = typeof OPTION_CONTROL_COMPONENTS | typeof OPTION_LABEL_ATTRIBUTES | typeof OPTION_LABEL_COMPONENTS;
32+
33+
type OptionDictionary = Readonly<Record<OptionKeys, ReadonlyArray<string>>>;
34+
35+
const getReadableItems = (items: ReadonlyArray<string>): string => {
36+
const { length: itemsLength } = items;
37+
38+
if (itemsLength === 1) return `"${items[0]}"`;
39+
40+
return `${items
41+
.map(x => `"${x}"`)
42+
.slice(0, itemsLength - 1)
43+
.join(', ')} and "${[...items].pop()}"`;
44+
};
45+
46+
export class Rule extends AbstractRule {
1447
static readonly metadata: IRuleMetadata = {
15-
description: 'Checks if the label has associated for attribute or a form element',
16-
optionExamples: [[true, { labelComponents: ['app-label'], labelAttributes: ['id'], controlComponents: ['app-input', 'app-select'] }]],
17-
options: {
18-
items: {
19-
type: 'object',
20-
properties: {
21-
labelComponents: {
22-
type: 'array',
23-
items: {
24-
type: 'string'
25-
}
26-
},
27-
labelAttributes: {
28-
type: 'array',
29-
items: {
30-
type: 'string'
31-
}
32-
},
33-
controlComponents: {
34-
type: 'array',
35-
items: {
36-
type: 'string'
37-
}
38-
}
48+
description: 'Checks if a label component is associated with a form element',
49+
optionExamples: [
50+
true,
51+
[
52+
true,
53+
{
54+
[OPTION_CONTROL_COMPONENTS]: ['app-input']
3955
}
56+
],
57+
[
58+
true,
59+
{
60+
[OPTION_CONTROL_COMPONENTS]: ['app-input', 'app-select'],
61+
[OPTION_LABEL_ATTRIBUTES]: ['id'],
62+
[OPTION_LABEL_COMPONENTS]: ['app-label']
63+
}
64+
]
65+
],
66+
options: {
67+
additionalProperties: false,
68+
properties: {
69+
[OPTION_CONTROL_COMPONENTS]: OPTION_SCHEMA_VALUE,
70+
[OPTION_LABEL_ATTRIBUTES]: OPTION_SCHEMA_VALUE,
71+
[OPTION_LABEL_COMPONENTS]: OPTION_SCHEMA_VALUE
4072
},
41-
type: 'array'
73+
type: 'object'
4274
},
43-
optionsDescription: 'Add custom label, label attribute and controls',
44-
rationale: Utils.dedent`
45-
The label tag should either have a for attribute or should have associated control.
46-
This rule supports two ways, either the label component should explicitly have a for attribute or a control nested inside the label component
47-
It also supports adding custom control component and custom label component support.`,
75+
optionsDescription: dedent`
76+
An optional object with optional \`${OPTION_CONTROL_COMPONENTS}\`, \`${OPTION_LABEL_ATTRIBUTES}\` and \`${OPTION_LABEL_COMPONENTS}\` properties.
77+
78+
* \`${OPTION_CONTROL_COMPONENTS}\` - components that must be inside a label component. Default and non overridable values are
79+
${getReadableItems(DEFAULT_CONTROL_COMPONENTS)}.
80+
* \`${OPTION_LABEL_ATTRIBUTES}\` - attributes that must be set on label components. Default and non overridable values are
81+
${getReadableItems(DEFAULT_LABEL_ATTRIBUTES)}.
82+
* \`${OPTION_LABEL_COMPONENTS}\` - components that act like a label. Default and non overridable values are
83+
${getReadableItems(DEFAULT_LABEL_COMPONENTS)}.
84+
`,
4885
ruleName: 'template-accessibility-label-for',
4986
type: 'functionality',
5087
typescriptOnly: true
5188
};
5289

53-
static readonly FAILURE_STRING = 'A form label must be associated with a control';
54-
static readonly FORM_ELEMENTS = ['input', 'select', 'textarea'];
90+
static readonly FAILURE_STRING = 'A label component must be associated with a form element';
5591

5692
apply(sourceFile: SourceFile): RuleFailure[] {
5793
const walkerConfig: NgWalkerConfig = { templateVisitorCtrl: TemplateVisitorCtrl };
5894
const walker = new NgWalker(sourceFile, this.getOptions(), walkerConfig);
5995

6096
return this.applyWithWalker(walker);
6197
}
98+
99+
isEnabled(): boolean {
100+
return super.isEnabled() && this.areOptionsValid();
101+
}
102+
103+
private areOptionsValid(): boolean {
104+
const { length: ruleArgumentsLength } = this.ruleArguments;
105+
106+
if (ruleArgumentsLength === 0) return true;
107+
108+
if (ruleArgumentsLength > 1) return false;
109+
110+
const {
111+
metadata: { options: ruleOptions }
112+
} = Rule;
113+
const [ruleArgument] = this.ruleArguments as ReadonlyArray<OptionDictionary>;
114+
const ruleArgumentsKeys = objectKeys(ruleArgument);
115+
const propertiesKeys = objectKeys(ruleOptions.properties as OptionDictionary);
116+
117+
return (
118+
ruleArgumentsKeys.every(argumentKey => propertiesKeys.includes(argumentKey)) &&
119+
ruleArgumentsKeys
120+
.map(argumentKey => ruleArgument[argumentKey])
121+
.every(argumentValue => Array.isArray(argumentValue) && argumentValue.length > 0)
122+
);
123+
}
62124
}
63125

64126
class TemplateVisitorCtrl extends BasicTemplateAstVisitor {
65-
visitElement(element: ElementAst, context: any) {
127+
private readonly controlComponents: ReadonlySet<string>;
128+
private readonly labelAttributes: ReadonlySet<string>;
129+
private readonly labelComponents: ReadonlySet<string>;
130+
131+
constructor(sourceFile: SourceFile, options: IOptions, context: ComponentMetadata, templateStart: number) {
132+
super(sourceFile, options, context, templateStart);
133+
134+
const { controlComponents, labelAttributes, labelComponents } = (options.ruleArguments[0] || {}) as OptionDictionary;
135+
136+
this.controlComponents = new Set([...DEFAULT_CONTROL_COMPONENTS, ...controlComponents]);
137+
this.labelAttributes = new Set([...DEFAULT_LABEL_ATTRIBUTES, ...labelAttributes]);
138+
this.labelComponents = new Set([...DEFAULT_LABEL_COMPONENTS, ...labelComponents]);
139+
}
140+
141+
visitElement(element: ElementAst, context: any): any {
66142
this.validateElement(element);
67143
super.visitElement(element, context);
68144
}
69145

70-
private validateElement(element: ElementAst) {
71-
let { labelAttributes, labelComponents, controlComponents }: ILabelForOptions = this.getOptions() || {};
72-
controlComponents = Rule.FORM_ELEMENTS.concat(controlComponents || []);
73-
labelComponents = ['label'].concat(labelComponents || []);
74-
labelAttributes = ['for'].concat(labelAttributes || []);
146+
private hasControlComponentInsideElement(element: ElementAst): boolean {
147+
return Array.from(this.controlComponents).some(controlComponentName => isChildNodeOf(element, controlComponentName));
148+
}
75149

76-
if (labelComponents.indexOf(element.name) === -1) {
77-
return;
78-
}
79-
const hasForAttr = element.attrs.some(attr => labelAttributes.indexOf(attr.name) !== -1);
80-
const hasForInput = element.inputs.some(input => {
81-
return labelAttributes.indexOf(input.name) !== -1;
82-
});
150+
private hasValidAttrOrInput(element: ElementAst): boolean {
151+
return [...element.attrs, ...element.inputs]
152+
.map(attrOrInput => attrOrInput.name)
153+
.some(attrOrInputName => this.labelAttributes.has(attrOrInputName));
154+
}
83155

84-
const hasImplicitFormElement = controlComponents.some(component => mayContainChildComponent(element, component));
156+
private isLabelComponent(element: ElementAst): boolean {
157+
return this.labelComponents.has(element.name);
158+
}
85159

86-
if (hasForAttr || hasForInput || hasImplicitFormElement) {
160+
private validateElement(element: ElementAst): void {
161+
if (!this.isLabelComponent(element) || this.hasValidAttrOrInput(element) || this.hasControlComponentInsideElement(element)) {
87162
return;
88163
}
164+
89165
const {
90166
sourceSpan: {
91167
end: { offset: endOffset },
Collapse file

‎src/util/isChildNodeOf.ts‎

Copy file name to clipboard
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { ElementAst } from '@angular/compiler';
2+
3+
export const isChildNodeOf = (root: ElementAst, childNodeName: string): boolean => {
4+
const traverseChildNodes = (node: ElementAst): boolean => {
5+
return node.children.some(childNode => {
6+
return childNode instanceof ElementAst && (childNode.name === childNodeName || traverseChildNodes(childNode));
7+
});
8+
};
9+
10+
return traverseChildNodes(root);
11+
};
Collapse file

‎src/util/mayContainChildComponent.ts‎

Copy file name to clipboardExpand all lines: src/util/mayContainChildComponent.ts
-22Lines changed: 0 additions & 22 deletions
This file was deleted.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.