Skip to content

Commit

Permalink
feat(rule): add pipe-prefix rule (#693)
Browse files Browse the repository at this point in the history
* feat(rule): add pipe-prefix rule

* docs(pipe-naming): mention pipe-prefix in pipe-naming deprecation

* test(pipe-prefix): fix assertion error
  • Loading branch information
sneas authored and mgechev committed Aug 2, 2018
1 parent a9d74be commit 71660ae
Show file tree
Hide file tree
Showing 4 changed files with 204 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export { Rule as NoTemplateCallExpressionRule } from './noTemplateCallExpression
export { Rule as NoUnusedCssRule } from './noUnusedCssRule';
export { Rule as PipeImpureRule } from './pipeImpureRule';
export { Rule as PipeNamingRule } from './pipeNamingRule';
export { Rule as PipePrefixRule } from './pipePrefixRule';
export { Rule as PreferInlineDecorator } from './preferInlineDecoratorRule';
export { Rule as PreferOutputReadonlyRule } from './preferOutputReadonlyRule';
export { Rule as TemplateConditionalComplexityRule } from './templateConditionalComplexityRule';
Expand Down
2 changes: 1 addition & 1 deletion src/pipeNamingRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const OPTION_KEBAB_CASE = 'kebab-case';

export class Rule extends Lint.Rules.AbstractRule {
static readonly metadata: Lint.IRuleMetadata = {
deprecationMessage: `You can name your pipes only ${OPTION_CAMEL_CASE}. If you try to use snake-case then your application will not compile.`,
deprecationMessage: `You can name your pipes only ${OPTION_CAMEL_CASE}. If you try to use snake-case then your application will not compile. For prefix validation use pipe-prefix rule.`,
description: 'Enforce consistent case and prefix for pipes.',
optionExamples: [
[true, OPTION_CAMEL_CASE, 'myPrefix'],
Expand Down
98 changes: 98 additions & 0 deletions src/pipePrefixRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { sprintf } from 'sprintf-js';
import * as Lint from 'tslint';
import * as ts from 'typescript';
import { NgWalker } from './angular/ngWalker';
import { SelectorValidator } from './util/selectorValidator';
import { getDecoratorArgument } from './util/utils';

export class Rule extends Lint.Rules.AbstractRule {
static readonly metadata: Lint.IRuleMetadata = {
description: 'Enforce consistent prefix for pipes.',
optionExamples: [[true, 'myPrefix'], [true, 'myPrefix', 'myOtherPrefix']],
options: {
items: [
{
type: 'string'
}
],
minLength: 1,
type: 'array'
},
optionsDescription: Lint.Utils.dedent`
* The list of arguments are supported prefixes (given as strings).
`,
rationale: 'Consistent conventions make it easy to quickly identify and reference assets of different types.',
ruleName: 'pipe-prefix',
type: 'style',
typescriptOnly: true
};

static FAILURE_STRING = `The name of the Pipe decorator of class %s should start with prefix %s, however its value is "%s"`;

prefix: string;
private prefixChecker: Function;

constructor(options: Lint.IOptions) {
super(options);

let args = options.ruleArguments;
if (!(args instanceof Array)) {
args = [args];
}
this.prefix = args.join(',');
let prefixExpression = args.join('|');
this.prefixChecker = SelectorValidator.prefix(prefixExpression, 'camelCase');
}

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

isEnabled(): boolean {
const {
metadata: {
options: { minLength }
}
} = Rule;
const { length } = this.ruleArguments;

return super.isEnabled() && length >= minLength;
}

validatePrefix(prefix: string): boolean {
return this.prefixChecker(prefix);
}
}

export class ClassMetadataWalker extends NgWalker {
constructor(sourceFile: ts.SourceFile, private rule: Rule) {
super(sourceFile, rule.getOptions());
}

protected visitNgPipe(controller: ts.ClassDeclaration, decorator: ts.Decorator) {
let className = controller.name!.text;
this.validateProperties(className, decorator);
super.visitNgPipe(controller, decorator);
}

private validateProperties(className: string, pipe: ts.Decorator) {
const argument = getDecoratorArgument(pipe)!;

argument.properties
.filter(p => p.name && ts.isIdentifier(p.name) && p.name.text === 'name')
.forEach(this.validateProperty.bind(this, className));
}

private validateProperty(className: string, property: ts.Node) {
const initializer = ts.isPropertyAssignment(property) ? property.initializer : undefined;

if (initializer && ts.isStringLiteral(initializer)) {
const propName = initializer.text;
const isValid = this.rule.validatePrefix(propName);

if (!isValid) {
this.addFailureAtNode(property, sprintf(Rule.FAILURE_STRING, className, this.rule.prefix, propName));
}
}
}
}
104 changes: 104 additions & 0 deletions test/pipePrefixRule.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import { assertSuccess, assertAnnotated } from './testHelper';

describe('pipe-prefix', () => {
describe('invalid pipe name', () => {
it('should fail when Pipe has no prefix ng', () => {
let source = `
@Pipe({
name: 'foo-bar'
~~~~~~~~~~~~~~~
})
class Test {}
`;
assertAnnotated({
ruleName: 'pipe-prefix',
message: 'The name of the Pipe decorator of class Test should start with prefix ng, however its value is "foo-bar"',
source,
options: ['ng']
});
});

it('should fail when Pipe has no prefix applying multiple prefixes', () => {
let source = `
@Pipe({
name: 'foo-bar'
~~~~~~~~~~~~~~~
})
class Test {}
`;
assertAnnotated({
ruleName: 'pipe-prefix',
message: 'The name of the Pipe decorator of class Test should start' + ' with prefix ng,mg,sg, however its value is "foo-bar"',
source,
options: ['ng', 'mg', 'sg']
});
});
});

describe('empty pipe', () => {
it('should not fail when @Pipe not invoked', () => {
let source = `
@Pipe
class Test {}
`;
assertSuccess('pipe-prefix', source, ['ng']);
});
});

describe('pipe with name as variable', () => {
it('should ignore the rule when the name is a variable', () => {
const source = `
export function mockPipe(name: string): any {
@Pipe({ name })
class MockPipe implements PipeTransform {
transform(input: any): any {
return input;
}
}
return MockPipe;
}
`;
assertSuccess('pipe-prefix', source, ['ng']);
});
});

describe('valid pipe name', () => {
it('should succeed with prefix ng in @Pipe', () => {
let source = `
@Pipe({
name: 'ngBarFoo'
})
class Test {}
`;
assertSuccess('pipe-prefix', source, ['ng']);
});

it('should succeed with multiple prefixes in @Pipe', () => {
let source = `
@Pipe({
name: 'ngBarFoo'
})
class Test {}
`;
assertSuccess('pipe-prefix', source, ['ng', 'sg', 'mg']);
});

it('should succeed when the class is not a Pipe', () => {
let source = `
class Test {}
`;
assertSuccess('pipe-prefix', source, ['ng']);
});

it('should do nothing if the name of the pipe is not a literal', () => {
let source = `
const pipeName = 'fooBar';
@Pipe({
name: pipeName
})
class Test {}
`;
assertSuccess('pipe-prefix', source, ['ng']);
});
});
});

0 comments on commit 71660ae

Please sign in to comment.