Skip to content

Commit 43d415a

Browse files
rafaelss95wKoza
rafaelss95
authored and
wKoza
committed
fix(rule): no-input-prefix not being able to check for multiple concurrent prefixes (#590)
LGTM
1 parent 75f9de6 commit 43d415a

File tree

2 files changed

+128
-100
lines changed

2 files changed

+128
-100
lines changed

‎src/noInputPrefixRule.ts‎

Lines changed: 57 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,75 @@
1-
import * as Lint from 'tslint';
2-
import * as ts from 'typescript';
31
import { sprintf } from 'sprintf-js';
2+
import { IOptions, IRuleMetadata, RuleFailure, Rules } from 'tslint/lib';
3+
import { arrayify } from 'tslint/lib/utils';
4+
import { Decorator, Node, PropertyAccessExpression, PropertyDeclaration, SourceFile } from 'typescript';
5+
46
import { NgWalker } from './angular/ngWalker';
57

6-
export class Rule extends Lint.Rules.AbstractRule {
7-
public static metadata: Lint.IRuleMetadata = {
8-
ruleName: 'no-input-prefix',
9-
type: 'maintainability',
10-
description: 'Input names should not be prefixed with the configured disallowed prefixes.',
11-
rationale: `HTML attributes are not prefixed. It's considered best not to prefix Inputs.
12-
* Example: 'enabled' is prefered over 'isEnabled'.
13-
`,
8+
export class Rule extends Rules.AbstractRule {
9+
static readonly metadata: IRuleMetadata = {
10+
description: 'Input names should not be prefixed by the configured disallowed prefixes.',
11+
optionExamples: ['[true, "can", "is", "should"]'],
1412
options: {
15-
type: 'array',
16-
items: [{ type: 'string' }]
13+
items: [{ type: 'string' }],
14+
type: 'array'
1715
},
18-
optionExamples: ['["is", "can", "should"]'],
1916
optionsDescription: 'Options accept a string array of disallowed input prefixes.',
17+
rationale: `HTML attributes are not prefixed. It's considered best not to prefix Inpu
18+
* Example: 'enabled' is prefered over 'isEnabled'.
19+
`,
20+
ruleName: 'no-input-prefix',
21+
type: 'maintainability',
2022
typescriptOnly: true
2123
};
2224

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

25-
public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
26-
return this.applyWithWalker(new InputWalker(sourceFile, this.getOptions()));
27+
apply(sourceFile: SourceFile): RuleFailure[] {
28+
return this.applyWithWalker(new NoInputPrefixWalker(sourceFile, this.getOptions()));
2729
}
2830
}
2931

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

37-
if (memberName) {
38-
const foundInvalid = options.find(x => memberName.startsWith(x));
39-
prefixLength = foundInvalid ? foundInvalid.length : 0;
40-
}
35+
if (prefixesLength === 1) {
36+
return `"${prefixes[0]}"`;
37+
}
4138

42-
if (
43-
prefixLength > 0 &&
44-
!(memberName.length >= prefixLength + 1 && memberName[prefixLength] !== memberName[prefixLength].toUpperCase())
45-
) {
46-
const failureConfig: string[] = [Rule.FAILURE_STRING, className, memberName, options.join(', ')];
47-
const errorMessage = sprintf.apply(null, failureConfig);
48-
this.addFailure(this.createFailure(property.getStart(), property.getWidth(), errorMessage));
39+
return `${prefixes
40+
.map(x => `"${x}"`)
41+
.slice(0, prefixesLength - 1)
42+
.join(', ')} or "${[...prefixes].pop()}"`;
43+
};
44+
45+
export const getFailureMessage = (className: string, propertyName: string, prefixes: string[]): string => {
46+
return sprintf(Rule.FAILURE_STRING, className, propertyName, getReadablePrefixes(prefixes));
47+
};
48+
49+
class NoInputPrefixWalker extends NgWalker {
50+
private readonly blacklistedPrefixes: string[];
51+
52+
constructor(source: SourceFile, options: IOptions) {
53+
super(source, options);
54+
this.blacklistedPrefixes = arrayify<string>(options.ruleArguments).slice(1);
55+
}
56+
57+
protected visitNgInput(property: PropertyDeclaration, input: Decorator, args: string[]) {
58+
this.validatePrefix(property, input, args);
59+
super.visitNgInput(property, input, args);
60+
}
61+
62+
private validatePrefix(property: PropertyDeclaration, input: Decorator, args: string[]) {
63+
const memberName = property.name.getText();
64+
const isBlackListedPrefix = this.blacklistedPrefixes.some(x => new RegExp(`^${x}[^a-z]`).test(memberName));
65+
66+
if (!isBlackListedPrefix) {
67+
return;
4968
}
69+
70+
const className = (property.parent as PropertyAccessExpression).name.getText();
71+
const failure = getFailureMessage(className, memberName, this.blacklistedPrefixes);
72+
73+
this.addFailureAtNode(property, failure);
5074
}
5175
}

‎test/noInputPrefixRule.spec.ts‎

Lines changed: 71 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,117 +1,121 @@
1-
import { assertSuccess, assertAnnotated } from './testHelper';
1+
import { getFailureMessage, Rule } from '../src/noInputPrefixRule';
2+
import { assertAnnotated, assertSuccess } from './testHelper';
23

3-
describe('no-input-prefix', () => {
4-
describe('invalid directive input property', () => {
5-
it('should fail, when a component input property is named with is prefix', () => {
6-
const source = `
7-
@Component()
8-
class ButtonComponent {
9-
@Input() isDisabled: boolean;
10-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
11-
}
12-
`;
13-
assertAnnotated({
14-
ruleName: 'no-input-prefix',
15-
options: ['is'],
16-
message: 'In the class "ButtonComponent", the input property "isDisabled" should not be prefixed with is',
17-
source
18-
});
19-
});
4+
const {
5+
FAILURE_STRING,
6+
metadata: { ruleName }
7+
} = Rule;
8+
const className = 'Test';
9+
10+
const getFailureAnnotations = (num: number): string => {
11+
return '~'.repeat(num);
12+
};
13+
14+
const getComposedOptions = (prefixes: string[]): (boolean | string)[] => {
15+
return [true, ...prefixes];
16+
};
2017

21-
it('should fail, when a directive input property is named with is prefix', () => {
18+
describe(ruleName, () => {
19+
describe('failure', () => {
20+
it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by one prefix', () => {
21+
const prefixes = ['is'];
22+
const propertyName = `${prefixes[0]}Disabled`;
23+
const inputExpression = `@Input() ${propertyName}: boolean;`;
2224
const source = `
2325
@Directive()
24-
class ButtonDirective {
25-
@Input() isDisabled: boolean;
26-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
26+
class ${className} {
27+
${inputExpression}
28+
${getFailureAnnotations(inputExpression.length)}
2729
}
2830
`;
2931
assertAnnotated({
30-
ruleName: 'no-input-prefix',
31-
options: ['is'],
32-
message: 'In the class "ButtonDirective", the input property "isDisabled" should not be prefixed with is',
32+
message: getFailureMessage(className, propertyName, prefixes),
33+
options: getComposedOptions(prefixes),
34+
ruleName,
3335
source
3436
});
3537
});
3638

37-
it('should fail, when a directive input property is named with is prefix', () => {
39+
it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by two prefixes', () => {
40+
const prefixes = ['can', 'is'];
41+
const propertyName = `${prefixes[0]}Enable`;
42+
const inputExpression = `@Input() ${propertyName}: boolean;`;
3843
const source = `
39-
@Directive()
40-
class ButtonDirective {
41-
@Input() mustDisable: string;
42-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
44+
@Component()
45+
class ${className} {
46+
${inputExpression}
47+
${getFailureAnnotations(inputExpression.length)}
4348
}
4449
`;
4550
assertAnnotated({
46-
ruleName: 'no-input-prefix',
47-
options: ['must'],
48-
message: 'In the class "ButtonDirective", the input property "mustDisable" should not be prefixed with must',
51+
message: getFailureMessage(className, propertyName, prefixes),
52+
options: getComposedOptions(prefixes),
53+
ruleName,
4954
source
5055
});
5156
});
5257

53-
it('should fail, when a directive input property is named with is prefix', () => {
58+
it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by two concurrent prefixes', () => {
59+
const prefixes = ['is', 'isc'];
60+
const propertyName = `${prefixes[1]}Hange`;
61+
const inputExpression = `@Input() ${propertyName}: boolean;`;
5462
const source = `
55-
@Directive()
56-
class ButtonDirective {
57-
@Input() is = true;
58-
~~~~~~~~~~~~~~~~~~~
63+
@Component()
64+
class ${className} {
65+
${inputExpression}
66+
${getFailureAnnotations(inputExpression.length)}
5967
}
6068
`;
6169
assertAnnotated({
62-
ruleName: 'no-input-prefix',
63-
options: ['is'],
64-
message: 'In the class "ButtonDirective", the input property "is" should not be prefixed with is',
70+
message: getFailureMessage(className, propertyName, prefixes),
71+
options: getComposedOptions(prefixes),
72+
ruleName,
6573
source
6674
});
6775
});
6876

69-
it('should fail, when a directive input property is named with can prefix', () => {
77+
it('should fail when an input property is snakecased and contains a blacklisted prefix', () => {
78+
const prefixes = ['do'];
79+
const propertyName = `${prefixes[0]}_it`;
80+
const inputExpression = `@Input() ${propertyName}: number;`;
7081
const source = `
7182
@Directive()
72-
class ButtonDirective {
73-
@Input() canEnable = true;
74-
~~~~~~~~~~~~~~~~~~~~~~~~~~
83+
class ${className} {
84+
${inputExpression}
85+
${getFailureAnnotations(inputExpression.length)}
7586
}
7687
`;
7788
assertAnnotated({
78-
ruleName: 'no-input-prefix',
79-
options: ['can', 'is'],
80-
message: 'In the class "ButtonDirective", the input property "canEnable" should not be prefixed with can, is',
89+
message: getFailureMessage(className, propertyName, prefixes),
90+
options: getComposedOptions(prefixes),
91+
ruleName,
8192
source
8293
});
8394
});
8495
});
8596

86-
describe('valid directive input property', () => {
87-
it('should succeed, when a directive input property is properly named', () => {
88-
const source = `
89-
@Directive()
90-
class ButtonComponent {
91-
@Input() disabled = true;
92-
}
93-
`;
94-
assertSuccess('no-input-prefix', source);
95-
});
96-
97-
it('should succeed, when a directive input property is properly named', () => {
97+
describe('success', () => {
98+
it('should succeed when an input property is not prefixed', () => {
9899
const source = `
99100
@Directive()
100-
class ButtonComponent {
101-
@Input() disabled = "yes";
101+
class ${className} {
102+
@Input() mustmust = true;
102103
}
103104
`;
104-
assertSuccess('no-input-prefix', source);
105+
assertSuccess(ruleName, source, getComposedOptions(['must']));
105106
});
106107

107-
it('should succeed, when a component input property is properly named with is', () => {
108+
it('should succeed when multiple input properties are prefixed by something not present in the blacklist', () => {
108109
const source = `
109110
@Component()
110-
class ButtonComponent {
111-
@Input() isometric: boolean;
111+
class ${className} {
112+
@Input() cana: string;
113+
@Input() disabledThing: boolean;
114+
@Input() isFoo = 'yes';
115+
@Input() shoulddoit: boolean;
112116
}
113117
`;
114-
assertSuccess('no-input-prefix', source);
118+
assertSuccess(ruleName, source, getComposedOptions(['can', 'should', 'dis', 'disable']));
115119
});
116120
});
117121
});

0 commit comments

Comments
 (0)