Skip to content

fix noInputRenameRule#585

Merged
mgechev merged 2 commits intomgechev:masterfrom
wKoza:gh580
Apr 29, 2018
Merged

fix noInputRenameRule#585
mgechev merged 2 commits intomgechev:masterfrom
wKoza:gh580

Conversation

@wKoza
Copy link
Collaborator

@wKoza wKoza commented Apr 28, 2018

In the hope that it clarifies things:

  • A component shouldn't rename its @input
  • A directive shouldn't rename its @input expect when the directive name is also an input property and the directive name doesn't describe the property.

Fixes #580

@wKoza wKoza changed the title WIP: fix noInputRenameRule fix noInputRenameRule Apr 28, 2018
@wKoza wKoza requested a review from mgechev April 28, 2018 10:59
source
});
});
it('should fail when a input property has the same name that the alias', () => {
Copy link
Collaborator

@rafaelss95 rafaelss95 Apr 28, 2018

Choose a reason for hiding this comment

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

nit: break line before it. Also, shouldn't it be ...an input...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

});

it("should fail when input property is renamed and it's different from directive's selector", () => {
it('should fail when a input property has the same name that the alias', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

an input?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

if (
args.length === 0 ||
(this.directiveSelector &&
(input.expression as any).arguments.some(arg => this.directiveSelector.indexOf(arg.text) !== -1 && memberName !== arg.text))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of casting to any, what about (input.expression as ts.CallExpression)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done


const getMessage = (className: string, propertyName: string): string => {
return `In the class "${className}", the directive input property "${propertyName}" should not be renamed.`;
return (
Copy link
Collaborator

@rafaelss95 rafaelss95 Apr 28, 2018

Choose a reason for hiding this comment

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

To be always sync with the rule's name and also metadata, I'd refactor it to:

const { FAILURE_STRING, metadata: { ruleName } } = Rule;

const getFailureMessage = (className: string, propertyName: string): string => {
  sprintf(FAILURE_STRING, className, propertyName);
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants