-
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
Update Deprecations rules to latest RuboCop format #720
Conversation
- Deprecations section. Signed-off-by: Scott Vidmar <svidmar@chef.io>
Signed-off-by: Scott Vidmar <svidmar@chef.io>
@@ -37,15 +37,12 @@ class ChefHandlerRecipe < Cop | |||
(send nil? :include_recipe (str {"chef_handler" "chef_handler::default"})) | |||
PATTERN | |||
|
|||
extend AutoCorrector |
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'd prefer to have the extends right up with the class so it's clear that this class extends AutoCorrector
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.
Agreed. Fixed.
return | ||
end | ||
def fixer(node, corrector) | ||
rewind_gem_install?(node) do |
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.
We should be able to extract this mess into each corrector not. It's one of the reasons they did the new autocorrect inline format.
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.
Fixed
def investigate(processed_source) | ||
return if processed_source.blank? | ||
def on_new_investigation | ||
f_name = processed_source.file_path |
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.
We limit the filename in the config so there's no need to do the file name check again.
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.
Fixed.
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.
@scottvidmar This doesn't look fixed yet
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.
Now it should be fixed. Sorry about that!
add_offense(action, location: :expression, message: MSG, severity: :warning) if action.source == ':uninstall' | ||
add_offense(action, message: MSG, severity: :warning) do |corrector| | ||
corrector.replace(action, ':remove') | ||
end if action.source == ':uninstall' |
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.
It's a lot easier to follow these by removing the if
on the block at line 48 and instead add next unless action.source == ':uninstall'
before the add_offense
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.
These should be addressed.
def on_send(node) | ||
deprecated_mixin?(node) do | ||
add_offense(node, location: :expression, message: MSG, severity: :warning) | ||
add_offense(node, message: MSG, severity: :warning) do |corrector| | ||
fixer(node, corrector) |
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.
We should be able to call the individual autocorrects here as well
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.
Should be all fixed up.
Signed-off-by: Scott Vidmar <svidmar@chef.io>
Signed-off-by: Scott Vidmar <svidmar@chef.io>
Addressed PR feedback. Tests still passing: bundle exec rspec spec -f p All examples were filtered out; ignoring {:focus=>true} Randomized with seed 1869 Finished in 1.3 seconds (files took 1.12 seconds to load) Randomized with seed 1869 |
Signed-off-by: Scott Vidmar <svidmar@chef.io>
def autocorrect(node) | ||
lambda do |corrector| | ||
corrector.replace(node.loc.expression, node.loc.expression.source.gsub(/^hash/, 'plist_hash')) | ||
return unless match_property_in_resource?(:launchd, 'hash', node) do |offense| |
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.
There shouldn't be a need for the return unless bit here. If it matches it'll run the block
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.
Good call. Removed that bit.
- Remove file name checks from Cheffile cop. File set in config. - Remove erroneous return if statement around matcher in launchd_deprecated_hash_property. Signed-off-by: Scott Vidmar <svidmar@chef.io>
Thanks for taking the time to fix these all up. It'll allow us to upgrade RuboCop whenever they rip out the old classes. |
Description
This refactors deprecations rules to use the latest format in use by RuboCop:
https://docs.rubocop.org/rubocop/development.html
No new functionality added.
Types of changes
Checklist:
Testing
Ran existing RSpec tests, as this is not adding or removing functionality.
$ bundle exec rspec spec -f p
Run options: include {:focus=>true}
All examples were filtered out; ignoring {:focus=>true}
Randomized with seed 304
................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
Finished in 0.88479 seconds (files took 2.08 seconds to load)
752 examples, 0 failures
Randomized with seed 304