-
-
Notifications
You must be signed in to change notification settings - Fork 269
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 autocorrect for RSpec/ExpectActual #855
Conversation
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.
Looks good to me.
Thanks for the contribution!
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.
Few minor notes.
expectation = node.parent.parent | ||
rhs = expectation.children.last | ||
return unless rhs.is_a?(RuboCop::AST::MethodDispatchNode) | ||
return if rhs.method_name != :eq |
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.
Maybe also :==
, :eql?
, :equal?
, and :be
?
A culprit with :be
is that it might not have an argument:
expect(3).to be
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.
Hi @pirj, thanks for the suggestions. Could I trouble you to provide examples using the first three methods? I interpreted your suggestion as applying like so:
expect(123).to ==(bar)
expect(123).to eql?(bar)
expect(123).to equal?(bar)
…but that doesn't appear to be valid:
Failure/Error: expect(123).to equal?(123)
ArgumentError:
The expect syntax does not support operator matchers, so you must pass a matcher to `#to`.
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.
My bad. Should of course be:
expect(123).to be
expect(123).to be == 123
expect(123).to eql(123)
expect(123).to equal(123)
Thanks for the extra care.
Do you need a hand with this @dduugg ? Please let me know if you get stuck with something. |
Hi @pirj I'll close this until I have more bandwidth to cover the various examples. It seemed much simpler when I was able to autocorrect the cop's spec and my code with ease 😸 |
Adds an autocorrect to
RSpec/ExpectActual
, swapping the arguments toexpect
andeq
when possible.Before submitting the PR make sure the following are checked:
master
(if not - rebase it).bundle exec rake
) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).