Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
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
2 changes: 1 addition & 1 deletion 2 src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
61 changes: 61 additions & 0 deletions 61 src/noAttributeDecoratorRule.ts
Original file line number Diff line number Diff line change
@@ -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.';

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this message...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's any place in the Angular Style Guide that says it's a bad practice to use @Attribute we should link it here, otherwise better rephrase this (and also the rationale).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


apply(sourceFile: SourceFile): RuleFailure[] {
return this.applyWithFunction(sourceFile, walk);
}
}

const isAttributeDecorator = (decorator: Decorator): boolean => getDecoratorName(decorator) === 'Attribute';

const validateConstructor = (context: WalkContext<void>, node: ConstructorDeclaration): void =>
node.parameters.forEach(parameter => validateParameter(context, parameter));

const validateDecorator = (context: WalkContext<void>, decorator: Decorator): void => {
if (!isAttributeDecorator(decorator)) return;

context.addFailureAtNode(decorator, Rule.FAILURE_STRING);
};

const validateParameter = (context: WalkContext<void>, parameter: ParameterDeclaration): void => {
const { decorators } = parameter;

if (!decorators) return;

decorators.forEach(decorator => validateDecorator(context, decorator));
};

const walk = (context: WalkContext<void>): void => {
const { sourceFile } = context;

const callback = (node: Node): void => {
if (isConstructorDeclaration(node)) validateConstructor(context, node);

forEachChild(node, callback);
};

forEachChild(sourceFile, callback);
};
58 changes: 0 additions & 58 deletions 58 src/noAttributeParameterDecoratorRule.ts

This file was deleted.

112 changes: 112 additions & 0 deletions 112 test/noAttributeDecoratorRule.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
78 changes: 0 additions & 78 deletions 78 test/noAttributeParameterDecoratorRule.spec.ts

This file was deleted.

Morty Proxy This is a proxified and sanitized view of the page, visit original site.