Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

exclude specific input types from react-a11y-input-elements rule #754

Merged
Merged
Show file tree
Hide file tree
Changes from 3 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
20 changes: 19 additions & 1 deletion src/reactA11yInputElementsRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ExtendedMetadata } from './utils/ExtendedMetadata';
export const MISSING_PLACEHOLDER_INPUT_FAILURE_STRING: string = 'Input elements must include default, place-holding characters if empty';
export const MISSING_PLACEHOLDER_TEXTAREA_FAILURE_STRING: string =
'Textarea elements must include default, place-holding characters if empty';
const EXCLUDED_INPUT_TYPES = ['checkbox', 'radio', 'file'];

/**
* Implementation of the react-a11y-input-elements rule.
Expand Down Expand Up @@ -45,14 +46,31 @@ export class Rule extends Lint.Rules.AbstractRule {
}
}

function isExcludedInputType(node: ts.JsxSelfClosingElement): boolean {
for (const attribute of node.attributes.properties) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got this type from below. Let me know if this looks right to you.

if (tsutils.isJsxAttribute(attribute)) {
const isInputAttributeType = getJsxAttributesFromJsxElement(node).type;
if (attribute.initializer !== undefined && tsutils.isStringLiteral(attribute.initializer)) {
const attributeText = attribute.initializer.text;
if (isInputAttributeType !== undefined && EXCLUDED_INPUT_TYPES.indexOf(attributeText) !== -1) {
return true;
}
}
}
}
return false;
}

function walk(ctx: Lint.WalkContext<void>) {
function cb(node: ts.Node): void {
if (tsutils.isJsxSelfClosingElement(node)) {
const tagName = node.tagName.getText();

if (tagName === 'input') {
const attributes = getJsxAttributesFromJsxElement(node);
if (isEmpty(attributes.value) && isEmpty(attributes.placeholder)) {
const isExcludedInputTypeValueEmpty = isEmpty(attributes.value) && isExcludedInputType(node);
const isPlaceholderEmpty = isEmpty(attributes.placeholder) && !isExcludedInputType(node);
if ((isEmpty(attributes.value) && isPlaceholderEmpty) || isExcludedInputTypeValueEmpty) {
ctx.addFailureAt(node.getStart(), node.getWidth(), MISSING_PLACEHOLDER_INPUT_FAILURE_STRING);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬 might this be simplified?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple things to try:

  • Pass attributes into isExcludedInputType.type so you don't have to call getJsxAttributesFromJsxElement each time
  • Only make one call to isExcludedInputType(node) and store that in a variable, so you don't need to call it twice.

...but the test cases look good, build passes, and this is still a pretty simple rule; I'm fine with merging this in as is & filing a followup issue to look into cleaning it up.

I'll wait for direction from you if you want to tackle it now - totally up to you! 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! Dried it up ⬇️

}
} else if (tagName === 'textarea') {
Expand Down
73 changes: 72 additions & 1 deletion src/tests/ReactA11yInputElementsRuleTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,81 @@ describe('reactA11yInputElementsRule', (): void => {
TestHelper.assertViolations(ruleName, script, []);
});

it('should pass on input elements without placeholder of type radio, checkbox, file', (): void => {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
const script: string = `
import React = require('react');
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
const a = <input type="radio" value="foo" />;
const b = <input type="checkbox" value="foo" />;
const c = <input type="file" value="foo" />;
`;

TestHelper.assertViolations(ruleName, script, []);
});

it('should fail on input elements without value of type radio, checkbox, file', (): void => {
const script: string = `
import React = require('react');
const a = <input type="radio" />;
const b = <input type="checkbox" />;
const c = <input type="file" />;
`;

TestHelper.assertViolations(ruleName, script, [
{
failure: MISSING_PLACEHOLDER_INPUT_FAILURE_STRING,
name: Utils.absolutePath('file.tsx'),
ruleName: 'react-a11y-input-elements',
startPosition: { character: 23, line: 3 }
},
{
failure: MISSING_PLACEHOLDER_INPUT_FAILURE_STRING,
name: Utils.absolutePath('file.tsx'),
ruleName: 'react-a11y-input-elements',
startPosition: { character: 23, line: 4 }
},
{
failure: MISSING_PLACEHOLDER_INPUT_FAILURE_STRING,
name: Utils.absolutePath('file.tsx'),
ruleName: 'react-a11y-input-elements',
startPosition: { character: 23, line: 5 }
}
]);
});

it('should fail on input elements without placeholder, when attribute is not type', (): void => {
const script: string = `
import React = require('react');
const a = <input foo="radio" />;
const b = <input bar="checkbox" />;
const c = <input baz="file" />;
`;

TestHelper.assertViolations(ruleName, script, [
{
failure: MISSING_PLACEHOLDER_INPUT_FAILURE_STRING,
name: Utils.absolutePath('file.tsx'),
ruleName: 'react-a11y-input-elements',
startPosition: { character: 23, line: 3 }
},
{
failure: MISSING_PLACEHOLDER_INPUT_FAILURE_STRING,
name: Utils.absolutePath('file.tsx'),
ruleName: 'react-a11y-input-elements',
startPosition: { character: 23, line: 4 }
},
{
failure: MISSING_PLACEHOLDER_INPUT_FAILURE_STRING,
name: Utils.absolutePath('file.tsx'),
ruleName: 'react-a11y-input-elements',
startPosition: { character: 23, line: 5 }
}
]);
});

it('should fail on empty input elements without placeholder', (): void => {
const script: string = `
import React = require('react');
const a = <input />;
const a = <input type="button" />;
const b = <textarea />;
const c = <textarea></textarea>;
`;
Expand Down