Skip to content

Commit 1ed8d8c

Browse files
rafaelss95mgechev
rafaelss95
authored and
mgechev
committed
fix(no-input-prefix): exact strings not being reported (#597)
1 parent 09c1309 commit 1ed8d8c

File tree

2 files changed

+58
-54
lines changed

2 files changed

+58
-54
lines changed

‎src/noInputPrefixRule.ts‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { sprintf } from 'sprintf-js';
22
import { IOptions, IRuleMetadata, RuleFailure, Rules } from 'tslint/lib';
3-
import { arrayify } from 'tslint/lib/utils';
43
import { Decorator, Node, PropertyAccessExpression, PropertyDeclaration, SourceFile } from 'typescript';
54

65
import { NgWalker } from './angular/ngWalker';
@@ -14,15 +13,15 @@ export class Rule extends Rules.AbstractRule {
1413
type: 'array'
1514
},
1615
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
16+
rationale: `HTML attributes are not prefixed. It's considered best not to prefix Inputs.
1817
* Example: 'enabled' is prefered over 'isEnabled'.
1918
`,
2019
ruleName: 'no-input-prefix',
2120
type: 'maintainability',
2221
typescriptOnly: true
2322
};
2423

25-
static readonly FAILURE_STRING = 'In the class "%s", the input property "%s" should not be prefixed by %s';
24+
static readonly FAILURE_STRING = '@Inputs should not be prefixed by %s';
2625

2726
apply(sourceFile: SourceFile): RuleFailure[] {
2827
return this.applyWithWalker(new NoInputPrefixWalker(sourceFile, this.getOptions()));
@@ -42,16 +41,16 @@ const getReadablePrefixes = (prefixes: string[]): string => {
4241
.join(', ')} or "${[...prefixes].pop()}"`;
4342
};
4443

45-
export const getFailureMessage = (className: string, propertyName: string, prefixes: string[]): string => {
46-
return sprintf(Rule.FAILURE_STRING, className, propertyName, getReadablePrefixes(prefixes));
44+
export const getFailureMessage = (prefixes: string[]): string => {
45+
return sprintf(Rule.FAILURE_STRING, getReadablePrefixes(prefixes));
4746
};
4847

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

5251
constructor(source: SourceFile, options: IOptions) {
5352
super(source, options);
54-
this.blacklistedPrefixes = arrayify<string>(options.ruleArguments).slice(1);
53+
this.blacklistedPrefixes = options.ruleArguments.slice(1);
5554
}
5655

5756
protected visitNgInput(property: PropertyDeclaration, input: Decorator, args: string[]) {
@@ -61,14 +60,13 @@ class NoInputPrefixWalker extends NgWalker {
6160

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

6665
if (!isBlackListedPrefix) {
6766
return;
6867
}
6968

70-
const className = (property.parent as PropertyAccessExpression).name.getText();
71-
const failure = getFailureMessage(className, memberName, this.blacklistedPrefixes);
69+
const failure = getFailureMessage(this.blacklistedPrefixes);
7270

7371
this.addFailureAtNode(property, failure);
7472
}

‎test/noInputPrefixRule.spec.ts‎

Lines changed: 51 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,89 +5,93 @@ const {
55
FAILURE_STRING,
66
metadata: { ruleName }
77
} = Rule;
8-
const className = 'Test';
98

10-
const getFailureAnnotations = (num: number): string => {
11-
return '~'.repeat(num);
12-
};
13-
14-
const getComposedOptions = (prefixes: string[]): (boolean | string)[] => {
15-
return [true, ...prefixes];
9+
const getComposedOptions = (blacklistedPrefixes: string[]): (boolean | string)[] => {
10+
return [true, ...blacklistedPrefixes];
1611
};
1712

1813
describe(ruleName, () => {
1914
describe('failure', () => {
2015
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;`;
16+
const blacklistedPrefixes = ['is'];
17+
const source = `
18+
@Directive()
19+
class Test {
20+
@Input() isDisabled: boolean;
21+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
22+
}
23+
`;
24+
assertAnnotated({
25+
message: getFailureMessage(blacklistedPrefixes),
26+
options: getComposedOptions(blacklistedPrefixes),
27+
ruleName,
28+
source
29+
});
30+
});
31+
32+
it('should fail when an input property is strictly equal to a blacklisted prefix', () => {
33+
const blacklistedPrefixes = ['should'];
2434
const source = `
2535
@Directive()
26-
class ${className} {
27-
${inputExpression}
28-
${getFailureAnnotations(inputExpression.length)}
36+
class Test {
37+
@Input() should: boolean;
38+
~~~~~~~~~~~~~~~~~~~~~~~~~
2939
}
3040
`;
3141
assertAnnotated({
32-
message: getFailureMessage(className, propertyName, prefixes),
33-
options: getComposedOptions(prefixes),
42+
message: getFailureMessage(blacklistedPrefixes),
43+
options: getComposedOptions(blacklistedPrefixes),
3444
ruleName,
3545
source
3646
});
3747
});
3848

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;`;
49+
it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by two blacklistedPrefixes', () => {
50+
const blacklistedPrefixes = ['can', 'is'];
4351
const source = `
4452
@Component()
45-
class ${className} {
46-
${inputExpression}
47-
${getFailureAnnotations(inputExpression.length)}
53+
class Test {
54+
@Input() canEnable: boolean;
55+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4856
}
4957
`;
5058
assertAnnotated({
51-
message: getFailureMessage(className, propertyName, prefixes),
52-
options: getComposedOptions(prefixes),
59+
message: getFailureMessage(blacklistedPrefixes),
60+
options: getComposedOptions(blacklistedPrefixes),
5361
ruleName,
5462
source
5563
});
5664
});
5765

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;`;
66+
it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by two concurrent blacklistedPrefixes', () => {
67+
const blacklistedPrefixes = ['is', 'isc'];
6268
const source = `
6369
@Component()
64-
class ${className} {
65-
${inputExpression}
66-
${getFailureAnnotations(inputExpression.length)}
70+
class Test {
71+
@Input() iscHange: boolean;
72+
~~~~~~~~~~~~~~~~~~~~~~~~~~~
6773
}
6874
`;
6975
assertAnnotated({
70-
message: getFailureMessage(className, propertyName, prefixes),
71-
options: getComposedOptions(prefixes),
76+
message: getFailureMessage(blacklistedPrefixes),
77+
options: getComposedOptions(blacklistedPrefixes),
7278
ruleName,
7379
source
7480
});
7581
});
7682

7783
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;`;
84+
const blacklistedPrefixes = ['do'];
8185
const source = `
8286
@Directive()
83-
class ${className} {
84-
${inputExpression}
85-
${getFailureAnnotations(inputExpression.length)}
87+
class Test {
88+
@Input() do_it: number;
89+
~~~~~~~~~~~~~~~~~~~~~~~
8690
}
8791
`;
8892
assertAnnotated({
89-
message: getFailureMessage(className, propertyName, prefixes),
90-
options: getComposedOptions(prefixes),
93+
message: getFailureMessage(blacklistedPrefixes),
94+
options: getComposedOptions(blacklistedPrefixes),
9195
ruleName,
9296
source
9397
});
@@ -96,26 +100,28 @@ describe(ruleName, () => {
96100

97101
describe('success', () => {
98102
it('should succeed when an input property is not prefixed', () => {
103+
const blacklistedPrefixes = ['must'];
99104
const source = `
100105
@Directive()
101-
class ${className} {
106+
class Test {
102107
@Input() mustmust = true;
103108
}
104109
`;
105-
assertSuccess(ruleName, source, getComposedOptions(['must']));
110+
assertSuccess(ruleName, source, getComposedOptions(blacklistedPrefixes));
106111
});
107112

108113
it('should succeed when multiple input properties are prefixed by something not present in the blacklist', () => {
114+
const blacklistedPrefixes = ['can', 'dis', 'disable', 'should'];
109115
const source = `
110116
@Component()
111-
class ${className} {
117+
class Test {
112118
@Input() cana: string;
113119
@Input() disabledThing: boolean;
114120
@Input() isFoo = 'yes';
115121
@Input() shoulddoit: boolean;
116122
}
117123
`;
118-
assertSuccess(ruleName, source, getComposedOptions(['can', 'should', 'dis', 'disable']));
124+
assertSuccess(ruleName, source, getComposedOptions(blacklistedPrefixes));
119125
});
120126
});
121127
});

0 commit comments

Comments
 (0)