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

feat(rule): add pipe-prefix rule #693

Merged
merged 4 commits into from
Aug 2, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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']);
});
});
});