-
-
Notifications
You must be signed in to change notification settings - Fork 277
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 RSpec/StringAsInstanceDoubleConstant #1957
base: master
Are you sure you want to change the base?
Conversation
63c936b
to
9f1a248
Compare
Getting
However, have run |
9f1a248
to
d3d3328
Compare
lib/rubocop/cop/rspec/no_stringified_instance_double_constant.rb
Outdated
Show resolved
Hide resolved
spec/rubocop/cop/rspec/no_stringified_instance_double_constant_spec.rb
Outdated
Show resolved
Hide resolved
Did you also “add docs/ to the commit”? When I run the rake task I see a diff in the docs folder. |
There are some wip rubocop cop naming guidelines, and they insist on naming cops so that they highlight the problem. This should be “StringifiedInstabceDoubleConstant” |
d3d3328
to
53de88b
Compare
Looks like my rake problems were mostly self-inflicted, I tried to add my own documentation while I was following the contribution guidelines. For ease of new development, I'd recommend updating all of our rake task feedback to say "bundle exec rake etc" so it's easier to copy and execute |
53de88b
to
7c794c1
Compare
Addresses rubocop#1136 Adds a cop which can autocorrect from String declarations for instance_double to Class declarations. Symbol declarations are not affected.
7c794c1
to
91101d8
Compare
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 am biased since it was extracted from our app, but in favor of this 👍🏻
Interestingly, instance_double
does support a string as the name, at least according to its rdoc. I am looking at some of the implementation (instance_double
and ObjectReference.for(doubled_class)
but I couldn't tell you what happens for classes vs strings.
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.
Two questions:
- Could / should we make the same rule for
class_double
too? - Shouldn’t we also detect and replace symbols? As I understand the source, calling
instance_double
with a symbol won’t even work.
Addresses #1136
Adds a cop which can autocorrect from String declarations for
instance_double
to Class declarations. Symbol declarations are not affected.Autocorrect is not marked as safe for the class instantiation reasons mentioned in the original issue. However, we have run this as safe autocorrect internally for years, and have not met with any issues.
Before submitting the PR make sure the following are checked:
master
(if not - rebase it).CHANGELOG.md
if the new code introduces user-observable changes.bundle exec rake
) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).If you have created a new cop:
config/default.yml
.Enabled: pending
inconfig/default.yml
.Enabled: true
in.rubocop.yml
.VersionAdded: "<<next>>"
indefault/config.yml
.If you have modified an existing cop's configuration options:
VersionChanged: "<<next>>"
inconfig/default.yml
.