Add 'for' and 'if' default prefixes for ContextWording

[josephan]

Add for as a default prefix in ContextWording check.

An example:

context "when a team has the display_name, 'TGIF'" do
  # ...
end

# or...

context "for a team with display_name, 'TGIF'" do
  # ...
end

---

Before submitting the PR make sure the following are checked:

• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the changelog if the new code introduces user-observable changes.
• The build (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:

• Added the new cop to config/default.yml.
• The cop documents examples of good and bad code.
• The tests assert both that bad code is reported and that good code is not reported.

[andyw8]

Looks like there's a problem with the existing documentation here, I've opened #758

[pirj]

But why change the default? It's configurable.
RSpec Style Guide's Context Descriptions recommend to limit it to "'when', 'with', or 'without'".

Can you please provide more context for your example?

[andyw8]

I originally suggested to @josephan to propose this change, so I'll add my support here.

As some background, initially only when was recommended:
rubocop/rspec-style-guide@ec71884

It was later updated to add with and without to match rubocop-rspec: rubocop/rspec-style-guide#28

I think using for as part of the description is another common and natural way to express the context.

Here's a gist showing code from real-world-rails which uses this:

https://gist.github.com/andyw8/217a5d62ac060a7031065fe5d9dca648

[pirj]

Just for the context, below are prefixes for context from real-world-rails (not sure if it makes sense to include real-world-ruby into the calculation), top ten most used:

when 23760
with 8150
for 2090
and 1993
as 1204
without 1035
in 719
user 688
on 509
if 482

total: 67688 contexts.

[andyw8]

Other than "user", I think all of those could be valid candidates for this rule.

[pirj]

I can add more to this list:

• that
• twhose
• which
• unless

which can form readable sentences in conjunction with context and both with surrounding docstrings.

EDIT:
At the same time, it's still possible to write specs in way that the resulting docstring is a complete nonsense by using just the allowed prefixes to context.

I don't have a good idea of how to improve the cop, but it seems too restrictive and is addressing symptoms. Wondering if it would be possible to check the correctness of the complete sentence, I only know of misspell, but it only checks misspelled words.

[andyw8]

Can you give an example of using "those" at the beginning of a context?

[pirj]

Sorry, my bad. Edited.

[andyw8]

I don't have a good idea of how to improve the cop, but it seems too restrictive and is addressing symptoms. Wondering if it would be possible to check the correctness of the complete sentence, I only know of misspell, but it only checks misspelled words.

I think automatically checking the grammar is probably a step too far. It's probably not feasible to predict all the words that might be acceptable, so perhaps the guidance wording should be relaxed? Instead of saying

block descriptions should start with 'when', or 'with'

it could say

block descriptions should start with a word which forms a readable sentence in conjunction with the parent context or describe block. Common words are 'when', or 'with', but you can configure Prefixes to match your needs.

[pirj]

Agree. That's about how rspec-style-guide's Context wording describes it:

context block descriptions should always start with 'when', 'with', or 'without' and be in the form of a sentence with proper grammar when composed with it block descriptions.

I believe this rule may be relaxed somehow so as a result both the full sentence is readable, and it's clear what distinguishes the context in question from the other ones just by looking at its docstring.

Is it even possible to enforce this? Does it make sense to make rubocop-rspec more strict than the style guide?

[andyw8]

I think it would be ok for rubocop-rspec to be more strict than the styleguide if it can clearly indicate that the default entries are only suggestions rather than a 'definitive' list.

[dgollahon]

I think relaxing the style guide wording and adding a note in the cop documentation to half-encourage people to think of adding to the list is fine, but selecting a small list of preferred words is something I think is helpful.

Personally, I hate having multiple ways of doing things if they're all equally good. For instance, I can't think of a case where i'd use if because all the examples I can think of right now would be equivalent with when. 'if the value is above 100' v.s. 'when the value is above 100'. If someone would prefer using if, I don't think that's bad but I also think having a terse set of standard clauses is nice. I'm not saying we should definitely not add if, I'm just suggesting that a short list has some advantages. I see more of a case for for since it has different applications.

I don't think we should expect to be able to validate the full grammar or catch 100% of reasonable prefixes, but limiting ourselves to a handful usually makes for more uniform / easy to read test descriptions, imo.

[pirj]

Completely agree that the guide should better say something like:

When composed with it block descriptions, context block docstrings should be in the form of a sentence with proper grammar.
It is very common to start context block docstrings with 'when', 'with', or 'without'.

WDYT of relaxing the guide this way and keeping the cop default as is?
The list of the prefixes is configurable, and RuboCop encourages users to configure it to match the project style.

[pirj]

@andyw8 Agree that it makes sense to emphasize that the default list is not set in stone, and if "sentence with proper grammar" condition is met, together with something like "reflects the distinction from other contexts", it's ok to use any set of prefixes.

[pirj]

@andyw8 has updated the guide to be less strict regarding prefixes:

When an example block description is composed with context block descriptions, it should result in a sentence with proper grammar.
This typically means the context should start with a term such as 'when', 'with', or 'without'.

WDYT of closing this, and adding a section to README to emphasize that RuboCop-RSpec welcomes changes to its default configuration?

From RuboCop's own README:

Out of the box it will enforce many of the guidelines outlined in the community Ruby Style Guide.

RuboCop is extremely flexible and most aspects of its behavior can be tweaked via various configuration options.

[pirj]

Ping @josephan @andyw8

[josephan]

@pirj sounds good!