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

no-restricted-invocations doesn't support modifiers #2656

Closed
achambers opened this issue Oct 24, 2022 · 4 comments · Fixed by #2657
Closed

no-restricted-invocations doesn't support modifiers #2656

achambers opened this issue Oct 24, 2022 · 4 comments · Fixed by #2657
Labels

Comments

@achambers
Copy link
Contributor

achambers commented Oct 24, 2022

I noticed today that the no-restricted-invocations rule doesn't support modifiers and it feels like it should.

Seems like it'd just take adding ElementModifierStatement to

return {
BlockStatement: checkDenylist,
ElementNode: checkDenylist,
MustacheStatement: checkDenylist,
SubExpression: checkDenylist,
};

Let me know if this is an improvement to the rule that you support. I've knocked together a short PR to implement it - #2657

@bmish
Copy link
Member

bmish commented Oct 24, 2022

Seems reasonable. As is, it's a breaking change that would need to go in with #2319.

@achambers
Copy link
Contributor Author

Seems reasonable. As is, it's a breaking change that would need to go in with #2319.

Coolio. Anything you need from me to make this so?

@bmish
Copy link
Member

bmish commented Oct 24, 2022

If you wanted to get it in sooner and also reduce risk, you could put this functionality behind an option (e.g. includeModifiers or something) and then we could just enable the option during the major release. But since it's not that big, it could also just go in during the major release as-is (whenever that happens).

@rwjblue
Copy link
Member

rwjblue commented Oct 27, 2022

I totally agree that its breaking, but I also think that we could consider it a bug fix. In other words, it seems pretty clear that this is a bug (we probably should have always considered modifiers here, but when the rule was written modifiers didn't exist), and semver clearly allows for the conceptual space of "breaking bugfixes".

@bmish bmish closed this as completed Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants