From 9487af20e5380a6cb03a86841d7d19c5c5e8d7ea Mon Sep 17 00:00:00 2001 From: Dima Snisarenko Date: Thu, 26 Jul 2018 17:36:40 +0200 Subject: [PATCH 1/3] feat(rule): add pipe-prefix rule --- src/index.ts | 1 + src/pipePrefixRule.ts | 98 +++++++++++++++++++++++++++++++++ test/pipePrefixRule.spec.ts | 104 ++++++++++++++++++++++++++++++++++++ 3 files changed, 203 insertions(+) create mode 100644 src/pipePrefixRule.ts create mode 100644 test/pipePrefixRule.spec.ts diff --git a/src/index.ts b/src/index.ts index d050a2a28..88b2cd4a2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -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'; diff --git a/src/pipePrefixRule.ts b/src/pipePrefixRule.ts new file mode 100644 index 000000000..e175da409 --- /dev/null +++ b/src/pipePrefixRule.ts @@ -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)); + } + } + } +} diff --git a/test/pipePrefixRule.spec.ts b/test/pipePrefixRule.spec.ts new file mode 100644 index 000000000..f13ec05b1 --- /dev/null +++ b/test/pipePrefixRule.spec.ts @@ -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: 'fooBar' + ~~~~~~~~~~~~~~~ + }) + 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 "fooBar"', + source, + options: ['ng'] + }); + }); + + it('should fail when Pipe has no prefix applying multiple prefixes', () => { + let source = ` + @Pipe({ + name: 'fooBar' + ~~~~~~~~~~~~~~~ + }) + 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 "fooBar"', + 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']); + }); + }); +}); From 7eefb31de6821eecd26a657e1e4acf07c807a187 Mon Sep 17 00:00:00 2001 From: Dima Snisarenko Date: Thu, 26 Jul 2018 17:49:56 +0200 Subject: [PATCH 2/3] docs(pipe-naming): mention pipe-prefix in pipe-naming deprecation --- src/pipeNamingRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pipeNamingRule.ts b/src/pipeNamingRule.ts index 2b3791415..a52c58322 100644 --- a/src/pipeNamingRule.ts +++ b/src/pipeNamingRule.ts @@ -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'], From f388737c1047c28f955e59996a9e3b4ec4323f76 Mon Sep 17 00:00:00 2001 From: Dima Snisarenko Date: Thu, 26 Jul 2018 19:29:42 +0200 Subject: [PATCH 3/3] test(pipe-prefix): fix assertion error --- test/pipePrefixRule.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/pipePrefixRule.spec.ts b/test/pipePrefixRule.spec.ts index f13ec05b1..05f907a6c 100644 --- a/test/pipePrefixRule.spec.ts +++ b/test/pipePrefixRule.spec.ts @@ -5,14 +5,14 @@ describe('pipe-prefix', () => { it('should fail when Pipe has no prefix ng', () => { let source = ` @Pipe({ - name: 'fooBar' + 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 "fooBar"', + message: 'The name of the Pipe decorator of class Test should start with prefix ng, however its value is "foo-bar"', source, options: ['ng'] }); @@ -21,14 +21,14 @@ describe('pipe-prefix', () => { it('should fail when Pipe has no prefix applying multiple prefixes', () => { let source = ` @Pipe({ - name: 'fooBar' + 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 "fooBar"', + 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'] });