rubocop-rspec icon indicating copy to clipboard operation
rubocop-rspec copied to clipboard

Feature request: RSpec/ContextWording supports Japanese

Open pocke opened this issue 6 years ago • 2 comments

Problem

RSpec/ContextWording does not work well to check Japanese. It has two problems.

First, usually we use a suffix to specify "when". For example, when a user exists is translated to ユーザーが存在する時. means when. But the cop only supports prefix, so we cannot configure it for Japanese.

Second, Japanese does not separate words with space. For example, ユーザー が 存在する 時 is not appropriate as Japanese. But the cop split the sentence with space. https://github.com/rubocop-hq/rubocop-rspec/blob/0442757fb4af570994b37b3e0ca35baeec635eb0/lib/rubocop/cop/rspec/context_wording.rb#L45 So If the cop supports suffix, it does not enough.

Solution Ideas

I have two ides.

First, adding Suffixes and Split options to the cop. For example:

RSpec/ContextWording:
  Suffixes:
    - '時'
  Split: false # default: true

The cop splits context sentence only if Split option is enabled. If it is disabled, the cop just checks with start_with or end_wtih method.

Second, adding Patterns option to the cop. For example

RSpec/ContextWording:
  Patterns:
    - "^when"
    - "^if"
    # Suffix pattern for japanese
    - "時$"

I like the first idea. The second idea is more powerful, but I think we do not need regexp in most cases.


The original idea is issued in rubocop-hq/rubocop-jp by @kyohah. https://github.com/rubocop-hq/rubocop-jp/issues/49 Thank you!

pocke avatar Feb 27 '19 10:02 pocke

I remember we decided to split as opposed to using start_with? due to "whenever" that would also match the /^when/ pattern.

The logic of composition of Prefixes with Suffixes is not obvious. While reading the configuration, it would be hard to understand if it is "one of the Prefixes and one of the Suffixes", or "one of the Prefixes or one of the Suffixes".

WDYT of failing the cop when both Suffixes and Prefixes are defined in the configuration?

pirj avatar May 09 '19 21:05 pirj

I think the point about composing Suffixes and Prefixes is a good one--it very well might be confusing to have both. I don't know if we need regexes or not, but at least that is relatively clear with a list of patterns. The pattern still be easily used with a , \b, or \s appended to each item, but if a user wants to customize the list they are might forget to do that.

The other issue in composition is you need to be able to make any/all prefixes valid options. If, in japanese or another language, you only care about the suffix, then you can't whitelist all prefixes using the existing configuration since it could be any character / word.

Re: Splitting, another option that might be worth considering is specifying a delimiter (' ' for us, '' would split on each character, but i guess that wouldn't work if you wanted to read multiple characters).

Regex might be overkill, but it seems like it might be simpler than the Suffixes/Prefixes/Split combinations.

dgollahon avatar Jun 18 '19 07:06 dgollahon

I overlooked this Issue, but I think this Issue is resolved in https://github.com/rubocop/rubocop-rspec/pull/1358 because it has the same as https://github.com/rubocop/rubocop-rspec/issues/745 .

ydah avatar Jan 23 '23 07:01 ydah

Thank you, @ydah for triaging this!

Fixed in #1358

pirj avatar Jan 23 '23 07:01 pirj