-
Notifications
You must be signed in to change notification settings - Fork 54
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
Add Chef/RespondToResourceName and Chef/RespondToProvides cops #180
Conversation
Catch some old school gating that's floating around. Signed-off-by: Tim Smith <tsmith@chef.io>
e69662e
to
145875c
Compare
class Provider | ||
class Icinga2Environment < Chef::Provider::LWRPBase | ||
provides :icinga2_environment if respond_to?(:provides) | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ respond_to?(:provides) in resources is no longer necessary in Chef Infra Client 12+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't you target only the if respond_to?(:provides)
bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure how to do that to be honest.
Here's the ast of the above test:
(if
(send nil :respond_to?
(sym :provides))
(send nil :provides
(sym :icinga2_environment)) nil)
We need to match the if respond_to?(:provides), but you can't do a selector that doesn't select on the child node of (sym :icinga2_environment) as far as I know. That's why I matches the whole node and manipulated it in the autocorrect.
end | ||
|
||
def_node_matcher :if_respond_to_resource_name?, <<~PATTERN | ||
(if (send nil? :respond_to? ( :sym :resource_name ) ) ... ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm guessing (if (send nil? ${:respond_to? ( :sym :resource_name )} ) ... )
(maybe without the {}'s again tho?) and then use the matcher instead of the expression for the location and the autocorrection (and then the autocorrect is simpler and deletes the entire match).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That particular match doesn't seem to work. The only thing you can do is match the whole bit since there's no negative match functionality.
Merging this as-is since there isn't an easy way to target this via the node pattern as the bit we want to keep it actually a child of the bit we want to delete. There doesn't seem to be a way to do that in the node syntax from what I can tell. If you can figure out a way do PR it. This is less than ideal. |
Catch some old school gating that's floating around.
Signed-off-by: Tim Smith tsmith@chef.io