From 1196d931e2639a241b8ceb4511eb25d7109eec3d Mon Sep 17 00:00:00 2001 From: lizzzp1 Date: Mon, 31 Dec 2018 20:59:14 -0500 Subject: [PATCH 1/4] exclude specific input types from react-a11y-input-elements rule --- src/reactA11yInputElementsRule.ts | 22 ++++++++++++++++++-- src/tests/ReactA11yInputElementsRuleTests.ts | 13 +++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/reactA11yInputElementsRule.ts b/src/reactA11yInputElementsRule.ts index 1c621ee7d..03376481a 100644 --- a/src/reactA11yInputElementsRule.ts +++ b/src/reactA11yInputElementsRule.ts @@ -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. @@ -45,17 +46,34 @@ export class Rule extends Lint.Rules.AbstractRule { } } +function isExcludedInputType(node: ts.JsxSelfClosingElement): boolean { + let isExcludedType = false; + node.attributes.properties.forEach( + (attribute: ts.JsxAttributeLike): void => { + if (attribute.kind === ts.SyntaxKind.JsxAttribute) { + if (attribute.initializer !== undefined && attribute.initializer.kind === ts.SyntaxKind.JsxExpression) { + const attributeText: string = ((attribute).initializer).text; + if (EXCLUDED_INPUT_TYPES.indexOf(attributeText) !== -1) { + isExcludedType = true; + } + } + } + } + ); + return isExcludedType; +} + function walk(ctx: Lint.WalkContext) { function cb(node: ts.Node): void { if (tsutils.isJsxSelfClosingElement(node)) { const tagName = node.tagName.getText(); - if (tagName === 'input') { + if (tagName === 'input' && !isExcludedInputType(node)) { const attributes = getJsxAttributesFromJsxElement(node); if (isEmpty(attributes.value) && isEmpty(attributes.placeholder)) { ctx.addFailureAt(node.getStart(), node.getWidth(), MISSING_PLACEHOLDER_INPUT_FAILURE_STRING); } - } else if (tagName === 'textarea') { + } else if (tagName === 'textarea' && !isExcludedInputType(node)) { const attributes = getJsxAttributesFromJsxElement(node); if (isEmpty(attributes.placeholder)) { ctx.addFailureAt(node.getStart(), node.getWidth(), MISSING_PLACEHOLDER_TEXTAREA_FAILURE_STRING); diff --git a/src/tests/ReactA11yInputElementsRuleTests.ts b/src/tests/ReactA11yInputElementsRuleTests.ts index 70fc38f36..2365cd632 100644 --- a/src/tests/ReactA11yInputElementsRuleTests.ts +++ b/src/tests/ReactA11yInputElementsRuleTests.ts @@ -20,10 +20,21 @@ describe('reactA11yInputElementsRule', (): void => { TestHelper.assertViolations(ruleName, script, []); }); + it('should pass on input elements without placeholder of type radio, checkbox, file', (): void => { + const script: string = ` + import React = require('react'); + const a = ; + const b = ; + const c = ; + `; + + TestHelper.assertViolations(ruleName, script, []); + }); + it('should fail on empty input elements without placeholder', (): void => { const script: string = ` import React = require('react'); - const a = ; + const a = ; const b = ; `; From 72a6c416feb60a4e1aebf3e8095bc4372a19b622 Mon Sep 17 00:00:00 2001 From: lizzzp1 Date: Tue, 1 Jan 2019 08:55:10 -0500 Subject: [PATCH 2/4] feedback - use for of, tsutils for type checks --- src/reactA11yInputElementsRule.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/reactA11yInputElementsRule.ts b/src/reactA11yInputElementsRule.ts index 03376481a..81d428937 100644 --- a/src/reactA11yInputElementsRule.ts +++ b/src/reactA11yInputElementsRule.ts @@ -48,18 +48,18 @@ export class Rule extends Lint.Rules.AbstractRule { function isExcludedInputType(node: ts.JsxSelfClosingElement): boolean { let isExcludedType = false; - node.attributes.properties.forEach( - (attribute: ts.JsxAttributeLike): void => { - if (attribute.kind === ts.SyntaxKind.JsxAttribute) { - if (attribute.initializer !== undefined && attribute.initializer.kind === ts.SyntaxKind.JsxExpression) { - const attributeText: string = ((attribute).initializer).text; - if (EXCLUDED_INPUT_TYPES.indexOf(attributeText) !== -1) { - isExcludedType = true; - } + + for (const attribute of node.attributes.properties) { + if (tsutils.isJsxAttribute(attribute)) { + if (attribute.initializer !== undefined && tsutils.isStringLiteral(attribute.initializer)) { + const attributeText = attribute.initializer.text; + if (EXCLUDED_INPUT_TYPES.indexOf(attributeText) !== -1) { + isExcludedType = true; + return isExcludedType; } } } - ); + } return isExcludedType; } @@ -73,7 +73,7 @@ function walk(ctx: Lint.WalkContext) { if (isEmpty(attributes.value) && isEmpty(attributes.placeholder)) { ctx.addFailureAt(node.getStart(), node.getWidth(), MISSING_PLACEHOLDER_INPUT_FAILURE_STRING); } - } else if (tagName === 'textarea' && !isExcludedInputType(node)) { + } else if (tagName === 'textarea') { const attributes = getJsxAttributesFromJsxElement(node); if (isEmpty(attributes.placeholder)) { ctx.addFailureAt(node.getStart(), node.getWidth(), MISSING_PLACEHOLDER_TEXTAREA_FAILURE_STRING); From aceec023909bd09f863ccfb2956cc09445e15533 Mon Sep 17 00:00:00 2001 From: lizzzp1 Date: Tue, 1 Jan 2019 11:41:04 -0500 Subject: [PATCH 3/4] feedback - add tests for non type attributes with excluded values, clean up boolean --- src/reactA11yInputElementsRule.ts | 16 ++--- src/tests/ReactA11yInputElementsRuleTests.ts | 62 +++++++++++++++++++- 2 files changed, 69 insertions(+), 9 deletions(-) diff --git a/src/reactA11yInputElementsRule.ts b/src/reactA11yInputElementsRule.ts index 81d428937..f6aef7a21 100644 --- a/src/reactA11yInputElementsRule.ts +++ b/src/reactA11yInputElementsRule.ts @@ -47,20 +47,18 @@ export class Rule extends Lint.Rules.AbstractRule { } function isExcludedInputType(node: ts.JsxSelfClosingElement): boolean { - let isExcludedType = false; - for (const attribute of node.attributes.properties) { if (tsutils.isJsxAttribute(attribute)) { + const isInputAttributeType = getJsxAttributesFromJsxElement(node).type; if (attribute.initializer !== undefined && tsutils.isStringLiteral(attribute.initializer)) { const attributeText = attribute.initializer.text; - if (EXCLUDED_INPUT_TYPES.indexOf(attributeText) !== -1) { - isExcludedType = true; - return isExcludedType; + if (isInputAttributeType !== undefined && EXCLUDED_INPUT_TYPES.indexOf(attributeText) !== -1) { + return true; } } } } - return isExcludedType; + return false; } function walk(ctx: Lint.WalkContext) { @@ -68,9 +66,11 @@ function walk(ctx: Lint.WalkContext) { if (tsutils.isJsxSelfClosingElement(node)) { const tagName = node.tagName.getText(); - if (tagName === 'input' && !isExcludedInputType(node)) { + 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); } } else if (tagName === 'textarea') { diff --git a/src/tests/ReactA11yInputElementsRuleTests.ts b/src/tests/ReactA11yInputElementsRuleTests.ts index 2365cd632..ded9b22b8 100644 --- a/src/tests/ReactA11yInputElementsRuleTests.ts +++ b/src/tests/ReactA11yInputElementsRuleTests.ts @@ -21,6 +21,17 @@ describe('reactA11yInputElementsRule', (): void => { }); it('should pass on input elements without placeholder of type radio, checkbox, file', (): void => { + const script: string = ` + import React = require('react'); + const a = ; + const b = ; + const c = ; + `; + + 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 = ; @@ -28,7 +39,56 @@ describe('reactA11yInputElementsRule', (): void => { const c = ; `; - TestHelper.assertViolations(ruleName, script, []); + 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 = ; + const b = ; + const c = ; + `; + + 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 => { From e94dc9836b8f788dacf11fd4ab82b31a814194b3 Mon Sep 17 00:00:00 2001 From: lizzzp1 Date: Tue, 1 Jan 2019 19:35:18 -0500 Subject: [PATCH 4/4] dry things up --- src/reactA11yInputElementsRule.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/reactA11yInputElementsRule.ts b/src/reactA11yInputElementsRule.ts index f6aef7a21..6be16c878 100644 --- a/src/reactA11yInputElementsRule.ts +++ b/src/reactA11yInputElementsRule.ts @@ -46,10 +46,10 @@ export class Rule extends Lint.Rules.AbstractRule { } } -function isExcludedInputType(node: ts.JsxSelfClosingElement): boolean { +function isExcludedInputType(node: ts.JsxSelfClosingElement, attributes: { [propName: string]: ts.JsxAttribute }): boolean { for (const attribute of node.attributes.properties) { if (tsutils.isJsxAttribute(attribute)) { - const isInputAttributeType = getJsxAttributesFromJsxElement(node).type; + const isInputAttributeType = attributes.type; if (attribute.initializer !== undefined && tsutils.isStringLiteral(attribute.initializer)) { const attributeText = attribute.initializer.text; if (isInputAttributeType !== undefined && EXCLUDED_INPUT_TYPES.indexOf(attributeText) !== -1) { @@ -68,8 +68,9 @@ function walk(ctx: Lint.WalkContext) { if (tagName === 'input') { const attributes = getJsxAttributesFromJsxElement(node); - const isExcludedInputTypeValueEmpty = isEmpty(attributes.value) && isExcludedInputType(node); - const isPlaceholderEmpty = isEmpty(attributes.placeholder) && !isExcludedInputType(node); + const isExcludedInput = isExcludedInputType(node, attributes); + const isExcludedInputTypeValueEmpty = isEmpty(attributes.value) && isExcludedInput; + const isPlaceholderEmpty = isEmpty(attributes.placeholder) && !isExcludedInput; if ((isEmpty(attributes.value) && isPlaceholderEmpty) || isExcludedInputTypeValueEmpty) { ctx.addFailureAt(node.getStart(), node.getWidth(), MISSING_PLACEHOLDER_INPUT_FAILURE_STRING); }