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(rule): template-accessibility-label-for not recognizing options and interpolated values #812

Merged
merged 2 commits into from
Apr 17, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 131 additions & 55 deletions src/templateAccessibilityLabelForRule.ts
Original file line number Diff line number Diff line change
@@ -1,91 +1,167 @@
import { ElementAst } from '@angular/compiler';
import { IRuleMetadata, RuleFailure, Rules, Utils } from 'tslint/lib';
import { IOptions, IRuleMetadata, RuleFailure } from 'tslint/lib';
import { AbstractRule } from 'tslint/lib/rules';
import { dedent } from 'tslint/lib/utils';
import { SourceFile } from 'typescript/lib/typescript';
import { ComponentMetadata } from './angular/metadata';
import { NgWalker, NgWalkerConfig } from './angular/ngWalker';
import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor';
import { mayContainChildComponent } from './util/mayContainChildComponent';
import { isChildNodeOf } from './util/isChildNodeOf';
import { objectKeys } from './util/objectKeys';

interface ILabelForOptions {
labelComponents: string[];
labelAttributes: string[];
controlComponents: string[];
}
export class Rule extends Rules.AbstractRule {
const OPTION_CONTROL_COMPONENTS = 'controlComponents';
const OPTION_LABEL_ATTRIBUTES = 'labelAttributes';
const OPTION_LABEL_COMPONENTS = 'labelComponents';

const OPTION_SCHEMA_VALUE = {
properties: {
items: {
type: 'string'
},
type: 'array',
uniqueItems: true
},
type: 'object'
};

const DEFAULT_CONTROL_COMPONENTS = ['button', 'input', 'meter', 'output', 'progress', 'select', 'textarea'];
const DEFAULT_LABEL_ATTRIBUTES = ['for', 'htmlFor'];
const DEFAULT_LABEL_COMPONENTS = ['label'];

type OptionKeys = typeof OPTION_CONTROL_COMPONENTS | typeof OPTION_LABEL_ATTRIBUTES | typeof OPTION_LABEL_COMPONENTS;

type OptionDictionary = Readonly<Record<OptionKeys, ReadonlyArray<string>>>;

const getReadableItems = (items: ReadonlyArray<string>): string => {
const { length: itemsLength } = items;

if (itemsLength === 1) return `"${items[0]}"`;

return `${items
.map(x => `"${x}"`)
.slice(0, itemsLength - 1)
.join(', ')} and "${[...items].pop()}"`;
};

export class Rule extends AbstractRule {
static readonly metadata: IRuleMetadata = {
description: 'Checks if the label has associated for attribute or a form element',
optionExamples: [[true, { labelComponents: ['app-label'], labelAttributes: ['id'], controlComponents: ['app-input', 'app-select'] }]],
options: {
items: {
type: 'object',
properties: {
labelComponents: {
type: 'array',
items: {
type: 'string'
}
},
labelAttributes: {
type: 'array',
items: {
type: 'string'
}
},
controlComponents: {
type: 'array',
items: {
type: 'string'
}
}
description: 'Checks if a label component is associated with a form element',
optionExamples: [
true,
[
true,
{
[OPTION_CONTROL_COMPONENTS]: ['app-input']
}
],
[
true,
{
[OPTION_CONTROL_COMPONENTS]: ['app-input', 'app-select'],
[OPTION_LABEL_ATTRIBUTES]: ['id'],
[OPTION_LABEL_COMPONENTS]: ['app-label']
}
]
],
options: {
additionalProperties: false,
properties: {
[OPTION_CONTROL_COMPONENTS]: OPTION_SCHEMA_VALUE,
[OPTION_LABEL_ATTRIBUTES]: OPTION_SCHEMA_VALUE,
[OPTION_LABEL_COMPONENTS]: OPTION_SCHEMA_VALUE
},
type: 'array'
type: 'object'
},
optionsDescription: 'Add custom label, label attribute and controls',
rationale: Utils.dedent`
The label tag should either have a for attribute or should have associated control.
This rule supports two ways, either the label component should explicitly have a for attribute or a control nested inside the label component
It also supports adding custom control component and custom label component support.`,
optionsDescription: dedent`
An optional object with optional \`${OPTION_CONTROL_COMPONENTS}\`, \`${OPTION_LABEL_ATTRIBUTES}\` and \`${OPTION_LABEL_COMPONENTS}\` properties.

* \`${OPTION_CONTROL_COMPONENTS}\` - components that must be inside a label component. Default and non overridable values are
${getReadableItems(DEFAULT_CONTROL_COMPONENTS)}.
* \`${OPTION_LABEL_ATTRIBUTES}\` - attributes that must be set on label components. Default and non overridable values are
${getReadableItems(DEFAULT_LABEL_ATTRIBUTES)}.
* \`${OPTION_LABEL_COMPONENTS}\` - components that act like a label. Default and non overridable values are
${getReadableItems(DEFAULT_LABEL_COMPONENTS)}.
`,
ruleName: 'template-accessibility-label-for',
type: 'functionality',
typescriptOnly: true
};

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

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

return this.applyWithWalker(walker);
}

isEnabled(): boolean {
return super.isEnabled() && this.areOptionsValid();
}

private areOptionsValid(): boolean {
const { length: ruleArgumentsLength } = this.ruleArguments;

if (ruleArgumentsLength === 0) return true;

if (ruleArgumentsLength > 1) return false;

const {
metadata: { options: ruleOptions }
} = Rule;
const [ruleArgument] = this.ruleArguments as ReadonlyArray<OptionDictionary>;
const ruleArgumentsKeys = objectKeys(ruleArgument);
const propertiesKeys = objectKeys(ruleOptions.properties as OptionDictionary);

return (
ruleArgumentsKeys.every(argumentKey => propertiesKeys.includes(argumentKey)) &&
ruleArgumentsKeys
.map(argumentKey => ruleArgument[argumentKey])
.every(argumentValue => Array.isArray(argumentValue) && argumentValue.length > 0)
);
}
}

class TemplateVisitorCtrl extends BasicTemplateAstVisitor {
visitElement(element: ElementAst, context: any) {
private readonly controlComponents: ReadonlySet<string>;
private readonly labelAttributes: ReadonlySet<string>;
private readonly labelComponents: ReadonlySet<string>;

constructor(sourceFile: SourceFile, options: IOptions, context: ComponentMetadata, templateStart: number) {
super(sourceFile, options, context, templateStart);

const { controlComponents, labelAttributes, labelComponents } = (options.ruleArguments[0] || {}) as OptionDictionary;

this.controlComponents = new Set([...DEFAULT_CONTROL_COMPONENTS, ...controlComponents]);
this.labelAttributes = new Set([...DEFAULT_LABEL_ATTRIBUTES, ...labelAttributes]);
this.labelComponents = new Set([...DEFAULT_LABEL_COMPONENTS, ...labelComponents]);
}

visitElement(element: ElementAst, context: any): any {
this.validateElement(element);
super.visitElement(element, context);
}

private validateElement(element: ElementAst) {
let { labelAttributes, labelComponents, controlComponents }: ILabelForOptions = this.getOptions() || {};
controlComponents = Rule.FORM_ELEMENTS.concat(controlComponents || []);
labelComponents = ['label'].concat(labelComponents || []);
labelAttributes = ['for'].concat(labelAttributes || []);
private hasControlComponentInsideElement(element: ElementAst): boolean {
return Array.from(this.controlComponents).some(controlComponentName => isChildNodeOf(element, controlComponentName));
}

if (labelComponents.indexOf(element.name) === -1) {
return;
}
const hasForAttr = element.attrs.some(attr => labelAttributes.indexOf(attr.name) !== -1);
const hasForInput = element.inputs.some(input => {
return labelAttributes.indexOf(input.name) !== -1;
});
private hasValidAttrOrInput(element: ElementAst): boolean {
return [...element.attrs, ...element.inputs]
.map(attrOrInput => attrOrInput.name)
.some(attrOrInputName => this.labelAttributes.has(attrOrInputName));
}

const hasImplicitFormElement = controlComponents.some(component => mayContainChildComponent(element, component));
private isLabelComponent(element: ElementAst): boolean {
return this.labelComponents.has(element.name);
}

if (hasForAttr || hasForInput || hasImplicitFormElement) {
private validateElement(element: ElementAst): void {
if (!this.isLabelComponent(element) || this.hasValidAttrOrInput(element) || this.hasControlComponentInsideElement(element)) {
return;
}

const {
sourceSpan: {
end: { offset: endOffset },
Expand Down
11 changes: 11 additions & 0 deletions src/util/isChildNodeOf.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { ElementAst } from '@angular/compiler';

export const isChildNodeOf = (root: ElementAst, childNodeName: string): boolean => {
const traverseChildNodes = (node: ElementAst): boolean => {
return node.children.some(childNode => {
return childNode instanceof ElementAst && (childNode.name === childNodeName || traverseChildNodes(childNode));
});
};

return traverseChildNodes(root);
};
22 changes: 0 additions & 22 deletions src/util/mayContainChildComponent.ts

This file was deleted.

Loading