Skip to content
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

Retire RSpec/InvalidPredicateMatcher #938

Closed
pirj opened this issue Jun 17, 2020 · 4 comments · Fixed by #940
Closed

Retire RSpec/InvalidPredicateMatcher #938

pirj opened this issue Jun 17, 2020 · 4 comments · Fixed by #940
Milestone

Comments

@pirj
Copy link
Member

pirj commented Jun 17, 2020

In that case, won’t RSpec tell you about the mistake? Something like

expected something to respond to `available??`

As far as I remember, according to our non-goals (a rather incomplete version of what we have in mind) it's not our business to notify the user about things that RSpec will notify with a spec failure, warning, or an error.

I clearly understand that with the editor integration it may highlight this problem earlier.
On the other hand, we don't provide auto-correction anyway.

It's an interesting topic of what we should keep, and what to retire, as this cop might be helpful for some people in some circumstances, but in general, it's not the goal of RuboCop RSpec.

WDYT @bquorning @Darhazer ?

@bquorning
Copy link
Collaborator

I stand by my comment in #441 (comment):

👎 IMHO, Rubocop is not the right tool for solving this problem. The extra ? at the end of the predicate matcher is a typo, and as all (well, most) other typos it will be caught during runtime.

As for the “editor integration” argument, that opens the door to catch a lot of “typo” class offenses that I really think we shouldn’t bother with.

pirj added a commit that referenced this issue Jun 18, 2020
@Darhazer
Copy link
Member

I guess in the early days when there were not so many cops, all kinds of checks were accepted. Now we can choose wisely.

On the other hand, do we need to retire it? I would suggest we do so in major versions, like when we bump the requirement of rubocop to 1.0, and probably - do a review of all cops and decide if some are to be retired, so have all the deprecations in one release ;)

Last, but not least, Rubocop itself has a ConfigObsoletion class that informs users for removed cops, as users can have them actually in their configurations. Should we adopt the same process? Maybe we should look into how extension could extend those lists?

@bquorning
Copy link
Collaborator

bquorning commented Jun 18, 2020

I agree, looking into how Rubocop extensions can use ConfigObsoletion would be nice.

Which makes me think – do we need a forum for discussing RuboCop extensions together with maintainers of the other official extensions?

@pirj pirj added this to the 2.0 milestone Aug 2, 2020
bquorning pushed a commit that referenced this issue Oct 22, 2020
@bquorning
Copy link
Collaborator

While not fixed in master branch yet, this issue is fixed in the release-2.0 branch which will be merged into master soon.

Closing.

pirj added a commit that referenced this issue Oct 22, 2020
pirj added a commit that referenced this issue Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants