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): no-input-prefix not being able to check for multiple concu… #590

Merged
merged 1 commit into from Apr 30, 2018
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
90 changes: 57 additions & 33 deletions src/noInputPrefixRule.ts
@@ -1,51 +1,75 @@
import * as Lint from 'tslint';
import * as ts from 'typescript';
import { sprintf } from 'sprintf-js';
import { IOptions, IRuleMetadata, RuleFailure, Rules } from 'tslint/lib';
import { arrayify } from 'tslint/lib/utils';
import { Decorator, Node, PropertyAccessExpression, PropertyDeclaration, SourceFile } from 'typescript';

import { NgWalker } from './angular/ngWalker';

export class Rule extends Lint.Rules.AbstractRule {
public static metadata: Lint.IRuleMetadata = {
ruleName: 'no-input-prefix',
type: 'maintainability',
description: 'Input names should not be prefixed with the configured disallowed prefixes.',
rationale: `HTML attributes are not prefixed. It's considered best not to prefix Inputs.
* Example: 'enabled' is prefered over 'isEnabled'.
`,
export class Rule extends Rules.AbstractRule {
static readonly metadata: IRuleMetadata = {
description: 'Input names should not be prefixed by the configured disallowed prefixes.',
optionExamples: ['[true, "can", "is", "should"]'],
options: {
type: 'array',
items: [{ type: 'string' }]
items: [{ type: 'string' }],
type: 'array'
},
optionExamples: ['["is", "can", "should"]'],
optionsDescription: 'Options accept a string array of disallowed input prefixes.',
rationale: `HTML attributes are not prefixed. It's considered best not to prefix Inpu
* Example: 'enabled' is prefered over 'isEnabled'.
`,
ruleName: 'no-input-prefix',
type: 'maintainability',
typescriptOnly: true
};

static FAILURE_STRING: string = 'In the class "%s", the input property "%s" should not be prefixed with %s';
static readonly FAILURE_STRING = 'In the class "%s", the input property "%s" should not be prefixed by %s';

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new InputWalker(sourceFile, this.getOptions()));
apply(sourceFile: SourceFile): RuleFailure[] {
return this.applyWithWalker(new NoInputPrefixWalker(sourceFile, this.getOptions()));
}
}

class InputWalker extends NgWalker {
visitNgInput(property: ts.PropertyDeclaration, input: ts.Decorator, args: string[]) {
const className = (<any>property).parent.name.text;
const memberName = (<any>property.name).text as string;
const options = this.getOptions() as string[];
let prefixLength: number;
const getReadablePrefixes = (prefixes: string[]): string => {
const prefixesLength = prefixes.length;

if (memberName) {
const foundInvalid = options.find(x => memberName.startsWith(x));
prefixLength = foundInvalid ? foundInvalid.length : 0;
}
if (prefixesLength === 1) {
return `"${prefixes[0]}"`;
}

if (
prefixLength > 0 &&
!(memberName.length >= prefixLength + 1 && memberName[prefixLength] !== memberName[prefixLength].toUpperCase())
) {
const failureConfig: string[] = [Rule.FAILURE_STRING, className, memberName, options.join(', ')];
const errorMessage = sprintf.apply(null, failureConfig);
this.addFailure(this.createFailure(property.getStart(), property.getWidth(), errorMessage));
return `${prefixes
.map(x => `"${x}"`)
.slice(0, prefixesLength - 1)
.join(', ')} or "${[...prefixes].pop()}"`;
};

export const getFailureMessage = (className: string, propertyName: string, prefixes: string[]): string => {
return sprintf(Rule.FAILURE_STRING, className, propertyName, getReadablePrefixes(prefixes));
};

class NoInputPrefixWalker extends NgWalker {
private readonly blacklistedPrefixes: string[];

constructor(source: SourceFile, options: IOptions) {
super(source, options);
this.blacklistedPrefixes = arrayify<string>(options.ruleArguments).slice(1);
}

protected visitNgInput(property: PropertyDeclaration, input: Decorator, args: string[]) {
this.validatePrefix(property, input, args);
super.visitNgInput(property, input, args);
}

private validatePrefix(property: PropertyDeclaration, input: Decorator, args: string[]) {
const memberName = property.name.getText();
const isBlackListedPrefix = this.blacklistedPrefixes.some(x => new RegExp(`^${x}[^a-z]`).test(memberName));

if (!isBlackListedPrefix) {
return;
}

const className = (property.parent as PropertyAccessExpression).name.getText();
const failure = getFailureMessage(className, memberName, this.blacklistedPrefixes);

this.addFailureAtNode(property, failure);
}
}
138 changes: 71 additions & 67 deletions test/noInputPrefixRule.spec.ts
@@ -1,117 +1,121 @@
import { assertSuccess, assertAnnotated } from './testHelper';
import { getFailureMessage, Rule } from '../src/noInputPrefixRule';
import { assertAnnotated, assertSuccess } from './testHelper';

describe('no-input-prefix', () => {
describe('invalid directive input property', () => {
it('should fail, when a component input property is named with is prefix', () => {
const source = `
@Component()
class ButtonComponent {
@Input() isDisabled: boolean;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
}
`;
assertAnnotated({
ruleName: 'no-input-prefix',
options: ['is'],
message: 'In the class "ButtonComponent", the input property "isDisabled" should not be prefixed with is',
source
});
});
const {
FAILURE_STRING,
metadata: { ruleName }
} = Rule;
const className = 'Test';

const getFailureAnnotations = (num: number): string => {
return '~'.repeat(num);
};

const getComposedOptions = (prefixes: string[]): (boolean | string)[] => {
return [true, ...prefixes];
};

it('should fail, when a directive input property is named with is prefix', () => {
describe(ruleName, () => {
describe('failure', () => {
it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by one prefix', () => {
const prefixes = ['is'];
const propertyName = `${prefixes[0]}Disabled`;
const inputExpression = `@Input() ${propertyName}: boolean;`;
const source = `
@Directive()
class ButtonDirective {
@Input() isDisabled: boolean;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
class ${className} {
${inputExpression}
${getFailureAnnotations(inputExpression.length)}
}
`;
assertAnnotated({
ruleName: 'no-input-prefix',
options: ['is'],
message: 'In the class "ButtonDirective", the input property "isDisabled" should not be prefixed with is',
message: getFailureMessage(className, propertyName, prefixes),
options: getComposedOptions(prefixes),
ruleName,
source
});
});

it('should fail, when a directive input property is named with is prefix', () => {
it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by two prefixes', () => {
const prefixes = ['can', 'is'];
const propertyName = `${prefixes[0]}Enable`;
const inputExpression = `@Input() ${propertyName}: boolean;`;
const source = `
@Directive()
class ButtonDirective {
@Input() mustDisable: string;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@Component()
class ${className} {
${inputExpression}
${getFailureAnnotations(inputExpression.length)}
}
`;
assertAnnotated({
ruleName: 'no-input-prefix',
options: ['must'],
message: 'In the class "ButtonDirective", the input property "mustDisable" should not be prefixed with must',
message: getFailureMessage(className, propertyName, prefixes),
options: getComposedOptions(prefixes),
ruleName,
source
});
});

it('should fail, when a directive input property is named with is prefix', () => {
it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by two concurrent prefixes', () => {
const prefixes = ['is', 'isc'];
const propertyName = `${prefixes[1]}Hange`;
const inputExpression = `@Input() ${propertyName}: boolean;`;
const source = `
@Directive()
class ButtonDirective {
@Input() is = true;
~~~~~~~~~~~~~~~~~~~
@Component()
class ${className} {
${inputExpression}
${getFailureAnnotations(inputExpression.length)}
}
`;
assertAnnotated({
ruleName: 'no-input-prefix',
options: ['is'],
message: 'In the class "ButtonDirective", the input property "is" should not be prefixed with is',
message: getFailureMessage(className, propertyName, prefixes),
options: getComposedOptions(prefixes),
ruleName,
source
});
});

it('should fail, when a directive input property is named with can prefix', () => {
it('should fail when an input property is snakecased and contains a blacklisted prefix', () => {
const prefixes = ['do'];
const propertyName = `${prefixes[0]}_it`;
const inputExpression = `@Input() ${propertyName}: number;`;
const source = `
@Directive()
class ButtonDirective {
@Input() canEnable = true;
~~~~~~~~~~~~~~~~~~~~~~~~~~
class ${className} {
${inputExpression}
${getFailureAnnotations(inputExpression.length)}
}
`;
assertAnnotated({
ruleName: 'no-input-prefix',
options: ['can', 'is'],
message: 'In the class "ButtonDirective", the input property "canEnable" should not be prefixed with can, is',
message: getFailureMessage(className, propertyName, prefixes),
options: getComposedOptions(prefixes),
ruleName,
source
});
});
});

describe('valid directive input property', () => {
it('should succeed, when a directive input property is properly named', () => {
const source = `
@Directive()
class ButtonComponent {
@Input() disabled = true;
}
`;
assertSuccess('no-input-prefix', source);
});

it('should succeed, when a directive input property is properly named', () => {
describe('success', () => {
it('should succeed when an input property is not prefixed', () => {
const source = `
@Directive()
class ButtonComponent {
@Input() disabled = "yes";
class ${className} {
@Input() mustmust = true;
}
`;
assertSuccess('no-input-prefix', source);
assertSuccess(ruleName, source, getComposedOptions(['must']));
});

it('should succeed, when a component input property is properly named with is', () => {
it('should succeed when multiple input properties are prefixed by something not present in the blacklist', () => {
const source = `
@Component()
class ButtonComponent {
@Input() isometric: boolean;
class ${className} {
@Input() cana: string;
@Input() disabledThing: boolean;
@Input() isFoo = 'yes';
@Input() shoulddoit: boolean;
}
`;
assertSuccess('no-input-prefix', source);
assertSuccess(ruleName, source, getComposedOptions(['can', 'should', 'dis', 'disable']));
});
});
});