Add a string methods cop #2204
Add a string methods cop #2204
Conversation
| @@ -636,6 +636,16 @@ Style/StringLiteralsInInterpolation: | |||
| - single_quotes | |||
| - double_quotes | |||
|
|
|||
| Style/StringMethods: | |||
There was a problem hiding this comment.
You need to add the cop in enabled.yml too.
There was a problem hiding this comment.
Will do. Just noticed that CollectionMethods is not in there - is that intentional?
There was a problem hiding this comment.
I found it in disabled.yml and that's because we expect it to generate a fair amount of false positives. StringMethods might be OK to enable by default since the method intern is less likely to appear in other classes, as far as I can see.
Something like this will likely yield many false positives. |
| inspect_source(cop, "'something'.#{method}") | ||
| expect(cop.offenses.size).to eq(1) | ||
| expect(cop.messages) | ||
| .to eq(["Prefer `#{preferred_method}` over `#{method}`."]) |
There was a problem hiding this comment.
Should there be a check for cop.highlights?
|
@bbatsov I'll scrub this from my commit message. TBH I was struggling to find another example for |
This change adds a cop that can be configured to add an offense when a string method is used when another equivalent method is preferred. It adds a default to prefer `to_sym` over `intern`. It is modeled after the `CollectionMethods` cop which checks for preferred methods on `Enumerable`.
imtayadeway
force-pushed
the
add-string-methods-cop
branch
from
662c246 to
a61915f
Compare
|
@jonas054 @rrosenblum just updated to incorporate your comments/suggestions. Can you rereview? |
|
👍 Looks good to me. |
|
👍 |
|
Damn. I should have actually read the PR - 4 commits, instead of 1 and no changelog entry. Guess I'll do the unthinkable and rewrite the history. |
|
@bbatsov apologies on missing the changelog. anything I can do to help? I'll make sure to squash next time prior to merging. Thanks for your assistance in getting this merged ❤️ |
Guess you can add it in another PR. I've combined the 4 commits and force-pushed to |
|
@bbatsov thanks! I'll take care of that part |
This change adds a cop that can be configured to add an offense when a
string method is used when another equivalent method is preferred. It
adds a default to prefer
to_symoverintern. It is modeled after theCollectionMethodscop which checks for preferred methods onEnumerable./cc @jrafanie @Fryguy