-
Notifications
You must be signed in to change notification settings - Fork 198
Add react-a11y-proptypes rule. #241
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,197 @@ | ||
/** | ||
* Enforce ARIA state and property values are valid. | ||
*/ | ||
|
||
import * as ts from 'typescript'; | ||
import * as Lint from 'tslint/lib/lint'; | ||
|
||
import { ExtendedMetadata } from './utils/ExtendedMetadata'; | ||
import { getPropName, getStringLiteral } from './utils/JsxAttribute'; | ||
import { IAria } from './utils/attributes/IAria'; | ||
import { | ||
isStringLiteral, | ||
isNumericLiteral, | ||
isJsxExpression, | ||
isFalseKeyword, | ||
isTrueKeyword, | ||
isNullKeyword | ||
} from './utils/TypeGuard'; | ||
|
||
// tslint:disable-next-line:no-require-imports no-var-requires | ||
const aria: { [attributeName: string]: IAria } = require('./utils/attributes/ariaSchema.json'); | ||
|
||
export function getFailureString(propName: string, expectedType: string, permittedValues: string[]): string { | ||
switch (expectedType) { | ||
case 'tristate': | ||
return `The value for ${propName} must be a boolean or the string 'mixed'.`; | ||
case 'token': | ||
return `The value for ${propName} must be a single token from the following: ${permittedValues}.`; | ||
case 'tokenlist': | ||
return `The value for ${propName} must be a list of one or more tokens from the following: ${permittedValues}.`; | ||
case 'boolean': | ||
case 'string': | ||
case 'integer': | ||
case 'number': | ||
default: // tslint:disable-line:no-switch-case-fall-through | ||
return `The value for ${propName} must be a ${expectedType}.`; | ||
} | ||
} | ||
|
||
export class Rule extends Lint.Rules.AbstractRule { | ||
public static metadata: ExtendedMetadata = { | ||
ruleName: 'react-a11y-proptypes', | ||
type: 'maintainability', | ||
description: 'Enforce ARIA state and property values are valid.', | ||
options: null, | ||
issueClass: 'Non-SDL', | ||
issueType: 'Warning', | ||
severity: 'Important', | ||
level: 'Opportunity for Excellence', | ||
group: 'Accessibility' | ||
}; | ||
|
||
public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { | ||
return sourceFile.languageVariant === ts.LanguageVariant.JSX | ||
? this.applyWithWalker(new ReactA11yProptypesWalker(sourceFile, this.getOptions())) | ||
: []; | ||
} | ||
} | ||
|
||
class ReactA11yProptypesWalker extends Lint.RuleWalker { | ||
public visitJsxAttribute(node: ts.JsxAttribute): void { | ||
const propName: string = getPropName(node).toLowerCase(); | ||
|
||
// if there is not a aria-* attribute, don't check it. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there is no aria-* attribute, skip it. |
||
if (!aria[propName]) { | ||
return; | ||
} | ||
|
||
const allowUndefined: boolean = aria[propName].allowUndefined || false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although the logic is the same, I'd prefer using |
||
const expectedType: string = aria[propName].type; | ||
const permittedValues: string[] = aria[propName].values; | ||
const propValue: string = getStringLiteral(node); | ||
|
||
if (this.isUndefined(node.initializer)) { | ||
if (!allowUndefined) { | ||
this.addFailure(this.createFailure( | ||
node.getStart(), | ||
node.getWidth(), | ||
getFailureString(propName, expectedType, permittedValues) | ||
)); | ||
} | ||
|
||
return; | ||
} else if (this.isComplexType(node.initializer)) { | ||
return; | ||
} | ||
|
||
if (!this.validityCheck(node.initializer, propValue, expectedType, permittedValues)) { | ||
this.addFailure(this.createFailure( | ||
node.getStart(), | ||
node.getWidth(), | ||
getFailureString(propName, expectedType, permittedValues) | ||
)); | ||
} | ||
} | ||
|
||
private validityCheck( | ||
propValueExpression: ts.Expression, | ||
propValue: string, | ||
expectedType: string, | ||
permittedValues: string[] | ||
): boolean { | ||
switch (expectedType) { | ||
case 'boolean': return this.isBoolean(propValueExpression); | ||
case 'tristate': return this.isBoolean(propValueExpression) || this.isMixed(propValueExpression); | ||
case 'integer': | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can explicitly check integer type. E.g. should fail on |
||
case 'number': return this.isNumber(propValueExpression); | ||
case 'string': return this.isString(propValueExpression); | ||
case 'token': | ||
return this.isString(propValueExpression) && permittedValues.indexOf(propValue.toLowerCase()) > -1; | ||
case 'tokenlist': | ||
return this.isString(propValueExpression) && | ||
propValue.split(' ').every(token => permittedValues.indexOf(token.toLowerCase()) > -1); | ||
default: | ||
return false; | ||
} | ||
} | ||
|
||
private isUndefined(node: ts.Expression): boolean { | ||
if (!node) { | ||
return true; | ||
} else if (isJsxExpression(node)) { | ||
const expression: ts.Expression = node.expression; | ||
if (!expression) { | ||
return true; | ||
} else if (expression.kind === ts.SyntaxKind.Identifier) { | ||
return expression.getText() === 'undefined'; | ||
} else if (isNullKeyword(expression)) { | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/** | ||
* For this case <div prop={ x + 1 } /> | ||
* we can't check the type of atrribute's expression until running time. | ||
*/ | ||
private isComplexType(node: ts.Expression): boolean { | ||
return !this.isUndefined(node) && isJsxExpression(node) && | ||
!isStringLiteral(node.expression) && !isNumericLiteral(node.expression) && | ||
!isTrueKeyword(node.expression) && !isFalseKeyword(node.expression); | ||
} | ||
|
||
private isBoolean(node: ts.Expression): boolean { | ||
if (isStringLiteral(node)) { | ||
const propValue: string = node.text.toLowerCase(); | ||
|
||
return propValue === 'true' || propValue === 'false'; | ||
} else if (isJsxExpression(node)) { | ||
const expression: ts.Expression = node.expression; | ||
|
||
if (isStringLiteral(expression)) { | ||
const propValue: string = expression.text.toLowerCase(); | ||
|
||
return propValue === 'true' || propValue === 'false'; | ||
} else { | ||
return isFalseKeyword(expression) || isTrueKeyword(expression); | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private isMixed(node: ts.Expression): boolean { | ||
if (isStringLiteral(node)) { | ||
return node.text.toLowerCase() === 'mixed'; | ||
} else if (isJsxExpression(node)) { | ||
const expression: ts.Expression = node.expression; | ||
|
||
return isStringLiteral(expression) && expression.text.toLowerCase() === 'mixed'; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private isNumber(node: ts.Expression): boolean { | ||
if (isStringLiteral(node)) { | ||
return !isNaN(Number(node.text)); | ||
} else if (isJsxExpression(node)) { | ||
const expression: ts.Expression = node.expression; | ||
|
||
if (isStringLiteral(expression)) { | ||
return !isNaN(Number(expression.text)); | ||
} else { | ||
return isNumericLiteral(expression); | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private isString(node: ts.Expression): boolean { | ||
return isStringLiteral(node) || (isJsxExpression(node) && isStringLiteral(node.expression)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,3 +40,13 @@ export function isJsxSelfClosingElement(node: ts.Node): node is ts.JsxSelfClosin | |
export function isJsxOpeningElement(node: ts.Node): node is ts.JsxOpeningElement { | ||
return node && node.kind === ts.SyntaxKind.JsxOpeningElement; | ||
} | ||
|
||
export function isTrueKeyword(node: ts.Node): node is ts.LiteralExpression { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do they have type like ts.TrueKeyworkd? If not, ignore it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They haven't ts.TrueKeyWord. Thanks. |
||
return node && node.kind === ts.SyntaxKind.TrueKeyword; | ||
} | ||
export function isFalseKeyword(node: ts.Node): node is ts.LiteralExpression { | ||
return node && node.kind === ts.SyntaxKind.FalseKeyword; | ||
} | ||
export function isNullKeyword(node: ts.Node): node is ts.LiteralExpression { | ||
return node && node.kind === ts.SyntaxKind.NullKeyword; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-hidden='yes' /> | ||
const b = <div aria-hidden='no' /> | ||
const c = <div aria-hidden={ 0 } /> | ||
const d = <div aria-hidden={ 1 } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-level='yes' /> | ||
const b = <div aria-level='no' /> | ||
const c = <div aria-level={ true } /> | ||
const d = <div aria-level={ 'false' } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import React = require('react'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The expression { } is undefined for our function isUndefined(), but there have an error "error TS17000: JSX attributes must only be assigned a non-empty 'expression'". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems Typescript error, then skip this test. From: bounces+848413-bc81-supersingerman=[email protected] [mailto:bounces+848413-bc81-supersingerman=[email protected]] On Behalf Of t-ligu In test-data/ReactA11yProptypes/FailingTestInputs/notAllowUndefined.tsx #241 (comment) :
The expression { } is undefined for our function isUndefined(), but there have an error "error TS17000: JSX attributes must only be assigned a non-empty 'expression'". — |
||
|
||
const a = <div aria-label /> | ||
const b = <div aria-label={ undefined } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-valuemax='yes' /> | ||
const b = <div aria-valuemax='no' /> | ||
const c = <div aria-valuemax={ true } /> | ||
const d = <div aria-valuemax={ 'false' } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-label={ true } /> | ||
const b = <div aria-label={ false } /> | ||
const c = <div aria-label={ 1234 } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-sort='' /> | ||
const b = <div aria-sort={ true } /> | ||
const c = <div aria-sort={ 123 } /> | ||
const d = <div aria-sort={ 'false' } /> | ||
const e = <div aria-sort='ascending descending' /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-relevant='' /> | ||
const b = <div aria-relevant='foobar' /> | ||
const c = <div aria-relevant={ true } /> | ||
const d = <div aria-relevant={ 'false' } /> | ||
const e = <div aria-relevant='additions removalss' /> | ||
const f = <div aria-relevant='additions removalss ' /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-checked={ undefined } /> | ||
const b = <div aria-checked='yes' /> | ||
const c = <div aria-checked='no' /> | ||
const d = <div aria-checked={ 1234 } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-expanded /> | ||
const b = <div aria-grabbed /> | ||
const c = <div aria-selected /> | ||
const d = <div aria-expanded={ undefined } /> | ||
const e = <div aria-grabbed={ null } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-hidden='true' /> | ||
const b = <div aria-hidden='false' /> | ||
const c = <div aria-hidden={ 'true' } /> | ||
const d = <div aria-hidden={ 'false' } /> | ||
const e = <div aria-hidden={ true } /> | ||
const f = <div aria-hidden={ false } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import React = require('react'); | ||
|
||
// We can't get the type of those tests until run time, | ||
// so those tests will pass. | ||
const num: number = 1; | ||
const error: boolean = false; | ||
const a = <div aria-hidden={ !true } /> | ||
const b = <div aria-hidden={ `${true}` } /> | ||
const c = <div aria-hidden={ 'tr' + 'ue' } /> | ||
const d = <div aria-hidden={ +123 } /> | ||
const e = <div aria-hidden={ -123 } /> | ||
const f = <div aria-hidden={ num } /> | ||
const g = <div aria-hidden={ this.props.hidden } /> | ||
const h = <div aria-hidden={ <div /> } /> | ||
const i = <input aria-hidden={ error ? 'true' : 'false' } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-level='123' /> | ||
const b = <div aria-level='+123' /> | ||
const c = <div aria-level='-123' /> | ||
const d = <div aria-level={ '123' } /> | ||
const e = <div aria-level={ '+123' } /> | ||
const f = <div aria-level={ '-123' } /> | ||
const g = <div aria-valuemax={ 123 } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-level='123' /> | ||
const b = <div aria-level='+123' /> | ||
const c = <div aria-level='-123' /> | ||
const d = <div aria-level={ '123' } /> | ||
const e = <div aria-level={ '+123' } /> | ||
const f = <div aria-level={ '-123' } /> | ||
const g = <div aria-valuemax={ 123 } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-label='Close' /> | ||
const b = <div aria-label={ 'Close' } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-sort='ascending' /> | ||
const b = <div aria-sort='ASCENDING' /> | ||
const c = <div aria-sort={ 'ascending' } /> | ||
const d = <div aria-sort='descending' /> | ||
const e = <div aria-sort={ 'descending' } /> | ||
const f = <div aria-sort='none' /> | ||
const g = <div aria-sort={ 'none' } /> | ||
const h = <div aria-sort='other' /> | ||
const i = <div aria-sort={ 'other' } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-relevant='additions' /> | ||
const b = <div aria-relevant={ 'additions' } /> | ||
const c = <div aria-relevant='additions removals' /> | ||
const d = <div aria-relevant='additions additions' /> | ||
const e = <div aria-relevant='additions removals text' /> | ||
const f = <div aria-relevant={ 'additions removals text' } /> | ||
const g = <div aria-relevant='additions removals text all' /> | ||
const h = <div aria-relevant={ 'additions removals text all' } /> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import React = require('react'); | ||
|
||
const a = <div aria-checked='true' /> | ||
const b = <div aria-checked={ 'false' } /> | ||
const c = <div aria-checked='mixed' /> | ||
const d = <div aria-checked={ 'mixed' } /> | ||
const e = <div aria-checked={ true } /> | ||
const f = <div aria-checked={ false } /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enforce the type of aria state and property values are correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ipip2005 I fixed comments. Thanks.