From 049be667a6c91940b5d203d7e7e09af4ef50b550 Mon Sep 17 00:00:00 2001 From: Rafael Santana Date: Wed, 6 Mar 2019 19:59:14 -0300 Subject: [PATCH] refactor(rule): rename/clean up 'no-attribute-parameter-decorator' (#781) --- src/index.ts | 2 +- src/noAttributeDecoratorRule.ts | 61 ++++++++++ src/noAttributeParameterDecoratorRule.ts | 58 --------- test/noAttributeDecoratorRule.spec.ts | 112 ++++++++++++++++++ .../noAttributeParameterDecoratorRule.spec.ts | 78 ------------ 5 files changed, 174 insertions(+), 137 deletions(-) create mode 100644 src/noAttributeDecoratorRule.ts delete mode 100644 src/noAttributeParameterDecoratorRule.ts create mode 100644 test/noAttributeDecoratorRule.spec.ts delete mode 100644 test/noAttributeParameterDecoratorRule.spec.ts diff --git a/src/index.ts b/src/index.ts index c9f3e6593..10372dce9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,7 +7,7 @@ export { Rule as ContextualLifecycleRule } from './contextualLifecycleRule'; export { Rule as DirectiveClassSuffixRule } from './directiveClassSuffixRule'; export { Rule as DirectiveSelectorRule } from './directiveSelectorRule'; export { Rule as ImportDestructuringSpacingRule } from './importDestructuringSpacingRule'; -export { Rule as NoAttributeParameterDecoratorRule } from './noAttributeParameterDecoratorRule'; +export { Rule as NoAttributeDecoratorRule } from './noAttributeDecoratorRule'; export { Rule as NoConflictingLifecycleRule } from './noConflictingLifecycleRule'; export { Rule as NoForwardRefRule } from './noForwardRefRule'; export { Rule as NoHostMetadataPropertyRule } from './noHostMetadataPropertyRule'; diff --git a/src/noAttributeDecoratorRule.ts b/src/noAttributeDecoratorRule.ts new file mode 100644 index 000000000..b56ed6d4b --- /dev/null +++ b/src/noAttributeDecoratorRule.ts @@ -0,0 +1,61 @@ +import { IRuleMetadata, RuleFailure, WalkContext } from 'tslint'; +import { AbstractRule } from 'tslint/lib/rules'; +import { + ConstructorDeclaration, + Decorator, + forEachChild, + isConstructorDeclaration, + Node, + ParameterDeclaration, + SourceFile +} from 'typescript'; +import { getDecoratorName } from './util/utils'; + +export class Rule extends AbstractRule { + static readonly metadata: IRuleMetadata = { + description: 'Disallows usage of @Attribute decorator.', + options: null, + optionsDescription: 'Not configurable.', + rationale: '@Attribute is considered bad practice. Use @Input instead.', + ruleName: 'no-attribute-decorator', + type: 'functionality', + typescriptOnly: true + }; + + static readonly FAILURE_STRING = '@Attribute is considered bad practice. Use @Input instead.'; + + apply(sourceFile: SourceFile): RuleFailure[] { + return this.applyWithFunction(sourceFile, walk); + } +} + +const isAttributeDecorator = (decorator: Decorator): boolean => getDecoratorName(decorator) === 'Attribute'; + +const validateConstructor = (context: WalkContext, node: ConstructorDeclaration): void => + node.parameters.forEach(parameter => validateParameter(context, parameter)); + +const validateDecorator = (context: WalkContext, decorator: Decorator): void => { + if (!isAttributeDecorator(decorator)) return; + + context.addFailureAtNode(decorator, Rule.FAILURE_STRING); +}; + +const validateParameter = (context: WalkContext, parameter: ParameterDeclaration): void => { + const { decorators } = parameter; + + if (!decorators) return; + + decorators.forEach(decorator => validateDecorator(context, decorator)); +}; + +const walk = (context: WalkContext): void => { + const { sourceFile } = context; + + const callback = (node: Node): void => { + if (isConstructorDeclaration(node)) validateConstructor(context, node); + + forEachChild(node, callback); + }; + + forEachChild(sourceFile, callback); +}; diff --git a/src/noAttributeParameterDecoratorRule.ts b/src/noAttributeParameterDecoratorRule.ts deleted file mode 100644 index 577680229..000000000 --- a/src/noAttributeParameterDecoratorRule.ts +++ /dev/null @@ -1,58 +0,0 @@ -import * as Lint from 'tslint'; -import * as ts from 'typescript'; -import { sprintf } from 'sprintf-js'; -import { validate, all } from './walkerFactory/walkerFn'; -import { Maybe, listToMaybe } from './util/function'; -import { withIdentifier, callExpression } from './util/astQuery'; -import { Failure } from './walkerFactory/walkerFactory'; - -export class Rule extends Lint.Rules.AbstractRule { - static readonly metadata: Lint.IRuleMetadata = { - description: 'Disallow usage of @Attribute decorator.', - options: null, - optionsDescription: 'Not configurable.', - rationale: '@Attribute is considered bad practice. Use @Input instead.', - ruleName: 'no-attribute-parameter-decorator', - type: 'maintainability', - typescriptOnly: true - }; - - static readonly FAILURE_STRING = - 'In the constructor of class "%s", the parameter "%s" uses the @Attribute decorator, which is considered as a bad practice. Please, consider construction of type "@Input() %s: string"'; - - private static readonly walkerBuilder = all( - validate(ts.SyntaxKind.Constructor)(node => { - return Maybe.lift(node.parent) - .fmap(parent => { - if (parent && ts.isClassExpression(parent) && (parent.parent as any).name) { - return (parent.parent as any).name.text; - } else if (parent && ts.isClassDeclaration(parent)) { - return parent.name!.text; - } - }) - .bind(parentName => { - const failures: Maybe[] = (node as any).parameters.map(p => { - const text = p.name.getText(); - - return Maybe.lift(p.decorators).bind(decorators => { - // Check if any @Attribute - const decoratorsFailed = listToMaybe(decorators.map(d => Rule.decoratorIsAttribute(d))); - - // We only care about 1 since we highlight the whole 'parameter' - return (decoratorsFailed as any).fmap(() => new Failure(p, sprintf(Rule.FAILURE_STRING, parentName, text, text))); - }); - }); - - return listToMaybe(failures) as any; - }); - }) - ); - - private static decoratorIsAttribute(dec: ts.Decorator): Maybe { - return callExpression(dec).bind(withIdentifier('Attribute') as any); - } - - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(Rule.walkerBuilder(sourceFile, this.getOptions())); - } -} diff --git a/test/noAttributeDecoratorRule.spec.ts b/test/noAttributeDecoratorRule.spec.ts new file mode 100644 index 000000000..14ef84743 --- /dev/null +++ b/test/noAttributeDecoratorRule.spec.ts @@ -0,0 +1,112 @@ +import { Rule } from '../src/noAttributeDecoratorRule'; +import { assertAnnotated, assertMultipleAnnotated, assertSuccess } from './testHelper'; + +const { + FAILURE_STRING, + metadata: { ruleName } +} = Rule; + +describe(ruleName, () => { + describe('failure', () => { + it('should fail if @Attribute decorator is used', () => { + const source = ` + class Test { + label: string; + + constructor(@Attribute('label') label) { + ~~~~~~~~~~~~~~~~~~~ + this.label = label; + } + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail if @Attribute decorator is used in a class expression', () => { + const source = ` + class Test { + private count = 0; + + InnerTestClass = new class { + constructor(@Attribute('label') label: string) {} + ~~~~~~~~~~~~~~~~~~~ + }(this); + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail if @Attribute decorator is used in a property class declaration', () => { + const source = ` + class Test { + static InnerTestClass = class extends TestCase { + constructor(@Attribute('label') label: string) {} + ~~~~~~~~~~~~~~~~~~~ + }; + } + `; + assertAnnotated({ + message: FAILURE_STRING, + ruleName, + source + }); + }); + + it('should fail if multiple @Attribute decorators are used', () => { + const source = ` + class Test { + constructor(@Attribute('label') label: string) { + ~~~~~~~~~~~~~~~~~~~ + } + + InnerTest1 = new class { + constructor(@Attribute('label') label: string) {} + ^^^^^^^^^^^^^^^^^^^ + }(this); + + static InnerTestClass2 = class extends TestCase { + constructor(@Attribute('label') label: string) {} + ################### + }; + } + `; + assertMultipleAnnotated({ + failures: [ + { + char: '~', + msg: FAILURE_STRING + }, + { + char: '^', + msg: FAILURE_STRING + }, + { + char: '#', + msg: FAILURE_STRING + } + ], + ruleName, + source + }); + }); + }); + + describe('success', () => { + it('should succeed if no @Attribute decorator is used', () => { + const source = ` + class Test { + constructor(@Property('label') label: string) {} + } + `; + assertSuccess(ruleName, source); + }); + }); +}); diff --git a/test/noAttributeParameterDecoratorRule.spec.ts b/test/noAttributeParameterDecoratorRule.spec.ts deleted file mode 100644 index e1eee7261..000000000 --- a/test/noAttributeParameterDecoratorRule.spec.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { assertAnnotated, assertSuccess } from './testHelper'; - -describe('no-attribute-parameter-decorator', () => { - describe('invalid class constructor', () => { - it('should work with anonymous classes', () => { - const source = ` - type Constructor = new (...args: any[]) => T; - - export function MyUtils>(Base: T) { - return class extends Base { - constructor() {} - public myMethod(a, b): boolean {} - }; - } - `; - assertSuccess('no-attribute-parameter-decorator', source); - }); - - it("should fail, when it's used attribute decorator", () => { - let source = ` - class ButtonComponent { - label: string; - constructor(@Attribute('label') label) { - ~~~~~~~~~~~~~~~~~~~~~~~~~ - this.label = label; - } - } - `; - assertAnnotated({ - ruleName: 'no-attribute-parameter-decorator', - message: - 'In the constructor of class "ButtonComponent", the parameter "label" uses the @Attribute decorator, ' + - 'which is considered as a bad practice. Please, consider construction of type "@Input() label: string"', - source - }); - }); - - it('should fail, when property class declaration uses @Attribute decorator', () => { - let source = ` - class TestCaseSample { - static SampleTestCase = class extends TestCase { - constructor(@Attribute('label') label) {} - ~~~~~~~~~~~~~~~~~~~~~~~~~ - }; - } - `; - assertAnnotated({ - ruleName: 'no-attribute-parameter-decorator', - message: - 'In the constructor of class "SampleTestCase", the parameter "label" uses the @Attribute decorator, ' + - 'which is considered as a bad practice. Please, consider construction of type "@Input() label: string"', - source - }); - }); - }); - - describe('valid class constructor', () => { - it('should succeed, when is not used attribute decorator', () => { - let source = ` - class ButtonComponent { - constructor() {} - } - `; - assertSuccess('no-attribute-parameter-decorator', source); - }); - - it('should succeed, when is not used attribute decorator in property class declaration', () => { - let source = ` - class TestCaseSample { - static SampleTestCase = class extends TestCase { - constructor() {} - }; - } - `; - assertSuccess('no-attribute-parameter-decorator', source); - }); - }); -});