Reimplement CallbackConditionalsBinding cop & specs
#204
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Specs are consolidated by using `expect_correction` to test offenses and correction in the same example. Doing this reveals that we actually have corrections which do not handle whitespace correctly. For now, the incorrect whitespace has been included in the specs, so they reflect the existing behavior. In addition, some examples are added covering edge cases, to capture the existing behavior ahead of changing it.
This rewrites the `CallbackConditionalsBinding` cop, making the following changes: - The cop no longer inherits from the deprecated `RuboCop::Cop::Cop` class. - The `CALLBACKS` constant is renamed to `RESTRICT_ON_SEND`, which allows RuboCop to use it to limit which methods `on_send` is called for. - Offense detection is simplified by leveraging a declarative node pattern describing offending conditional callbacks. - Correction is simplified by better leveraging the AST API.
This changes which node `CallbackConditionalsBinding` offenses are attached to from the entire "macro" node to just the keyword argument pair node containing the `if:/unless:` keyword, and the offending block.
paracycle
approved these changes
Mar 19, 2024
vinistock
approved these changes
Mar 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This (in three separate commits, for ease of review)
Improves the
CallbackConditionalsBindingspecsSpecs are consolidated by using
expect_correctionto test offenses and correction in the same example.Doing this reveals that we actually have corrections which do not handle whitespace correctly. For now, the incorrect whitespace has been included in the specs, so they reflect the existing behavior.
In addition, some examples are added covering edge cases, to capture the existing behavior ahead of changing it.
Refactors the
CallbackConditionalsBindingcopThis rewrites the
CallbackConditionalsBindingcop, making the following changes:RuboCop::Cop::Copclass.CALLBACKSconstant is renamed toRESTRICT_ON_SEND, which allows RuboCop to use it to limit which methodson_sendis called for.Changes the node to which
CallbackConditionalsBindingoffenses are addedThis changes which node
CallbackConditionalsBindingoffenses are attached to from the entire "macro" node to just the keyword argument pair node containing theif:/unless:keyword, and the offending block.For changelog purposes, these changes could be summarized as