Add a validation for supported styles other than EnforcedStyle #3765
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I added a validation for supported styles other than EnforcedStyle.
Problem
======
Currently, EnforcedStyle value is validated by SupportedStyles.
e.g.
```sh
$ cat .rubocop.yml
Style/AlignParameters:
EnforcedStyle: aaaaaaaaaaaaaaaaaaaaaa
$ rubocop
Error: invalid EnforcedStyle 'aaaaaaaaaaaaaaaaaaaaaa' for Style/AlignParameters found in /tmp/tmp.SyHaGtdT0U/.rubocop.yml
Valid choices are: with_first_parameter, with_fixed_indentation
```
However, other styles aren't validated.
e.g.
```sh
$ cat .rubocop.yml
Style/SpaceAroundBlockParameters:
EnforcedStyleInsidePipes: aaaaaaaaaaaaaaaaaaaaaa
$ rubocop
Inspecting 0 files
0 files inspected, no offenses detected
```
Goal
======
Validate the invalid value.
e.g.
```sh
$ cat .rubocop.yml
Style/SpaceAroundBlockParameters:
EnforcedStyleInsidePipes: aaaaaaaaaaaaaaaaaaaaaa
$ rubocop
Error: invalid EnforcedStyleInsidePipes 'aaaaaaaaaaaaaaaaaaaaaa' for Style/SpaceAroundBlockParameters found in /tmp/tmp.SyHaGtdT0U/.rubocop.yml
Valid choices are: space, no_space
```
Changes
=====
- Add validation for styles other than `EnforcedStyle`
- And rename some `SupportedStyles` names to match EnforcedStyle name
- e.g. `SupportedStyles` to `SupportedStylesInsidePipes` (for `EnforcedStyleInsidePipes`)
- Add `SupportedStyles` for missing styles in config/default.yml
- Add a spec to check existence of `SupportedStyles` into `project_spec.rb`
- Add test cases to check validation
pocke
force-pushed
the
check-supported-style
branch
from
110b1be
to
fcffd8a
Compare
bbatsov
reviewed
Dec 4, 2016
| @@ -271,6 +271,15 @@ def compatible_external_encoding_for?(src) | |||
| src = src.dup if RUBY_VERSION < '2.3' | |||
| src.force_encoding(Encoding.default_external).valid_encoding? | |||
| end | |||
|
|
|||
| def to_supported_styles(enforced_style) | |||
There was a problem hiding this comment.
I don't like this method that much - too many things seem hardcoded in it, which means people are likely going to forget updating it and me might end up with more unvalidated config down the road.
|
I'll have to think about this a bit more. While I think your idea is good it might be worth reconsidering the configuration of cops with multiple configurable styles in general. Frankly, even I find some of our configuration options to be extremely confusing. |
I think so too. I hope the hardcoded names will be removed.
There are several rules of EnforcedStyle's naming convention.
For example
By this rename, the hardcorded problem will be solved. I think confusion will be resolved by this rename. However, it's braking change... |
Let's do it. I'd rather do a breaking change now than after 1.0. |
See. rubocop#3765 (comment) - Rename EnforcedMode to EnforcedStyle - Rename AlignWith to EnforcedStyleAlignWith - Rename IndentWhenRelativeTo to EnforcedStyle
|
@bbatsov I renamed them. |
11 tasks
pocke
added a commit
to pocke/rubocop
that referenced
this pull request
Jan 14, 2017
This change improves obsolete warnings in three ways. 1. Show all obsoletes -------- Currently, RuboCop displays first obsolete warning only. ```yaml # two obsolete cops Style/SingleSpaceBeforeFirstArg: Enabled: false Style/SpaceBeforeModifierKeyword: Enabled: false ``` ``` Error: The `Style/SingleSpaceBeforeFirstArg` cop has been renamed to `Style/SpaceBeforeFirstArg. (obsolete configuration found in /tmp/tmp.9T2ers7SW8/.rubocop.yml, please update it) ``` This change makes to display all obsolete warnings. ``` Error: The `Style/SingleSpaceBeforeFirstArg` cop has been renamed to `Style/SpaceBeforeFirstArg.` (obsolete configuration found in /tmp/tmp.9T2ers7SW8/.rubocop.yml, please update it) The `Style/SpaceBeforeModifierKeyword` cop has been removed. Please use `Style/SpaceAroundKeyword` instead. (obsolete configuration found in /tmp/tmp.9T2ers7SW8/.rubocop.yml, please update it) ``` 2. Add obsolete cops --------- `Style/MethodCallParentheses` and `Lint/Eval` are renamed. - rubocop#3797 - rubocop#3820 So, I added the cops to `OBSOLATE_COPS`. 3. Add obsolete parameters In rubocop#3765, some cop's parameters are renamed. So, I added the parameters to `OBSOLATE_PARAMETERS`
11 tasks
bbatsov
pushed a commit
that referenced
this pull request
Jan 14, 2017
This change improves obsolete warnings in three ways. 1. Show all obsoletes -------- Currently, RuboCop displays first obsolete warning only. ```yaml # two obsolete cops Style/SingleSpaceBeforeFirstArg: Enabled: false Style/SpaceBeforeModifierKeyword: Enabled: false ``` ``` Error: The `Style/SingleSpaceBeforeFirstArg` cop has been renamed to `Style/SpaceBeforeFirstArg. (obsolete configuration found in /tmp/tmp.9T2ers7SW8/.rubocop.yml, please update it) ``` This change makes to display all obsolete warnings. ``` Error: The `Style/SingleSpaceBeforeFirstArg` cop has been renamed to `Style/SpaceBeforeFirstArg.` (obsolete configuration found in /tmp/tmp.9T2ers7SW8/.rubocop.yml, please update it) The `Style/SpaceBeforeModifierKeyword` cop has been removed. Please use `Style/SpaceAroundKeyword` instead. (obsolete configuration found in /tmp/tmp.9T2ers7SW8/.rubocop.yml, please update it) ``` 2. Add obsolete cops --------- `Style/MethodCallParentheses` and `Lint/Eval` are renamed. - #3797 - #3820 So, I added the cops to `OBSOLATE_COPS`. 3. Add obsolete parameters In #3765, some cop's parameters are renamed. So, I added the parameters to `OBSOLATE_PARAMETERS`
toshimaru
added a commit
to toshimaru/rubocop-rails
that referenced
this pull request
Jan 16, 2017
volmer
added a commit
to Shopify/ruby-style-guide
that referenced
this pull request
Jan 20, 2017
Our current config does not work with RuboCop 0.47 and up because unfortunately RuboCop introduced a breaking change in rubocop/rubocop#3765 Sadly this commit will break all configs that inherit from our `rubocop.yml` that are still run using RuboCop 0.46 or older.
koic
pushed a commit
to rubocop/rubocop-rails
that referenced
this pull request
Oct 26, 2018
See. rubocop/rubocop#3765 (comment) - Rename EnforcedMode to EnforcedStyle - Rename AlignWith to EnforcedStyleAlignWith - Rename IndentWhenRelativeTo to EnforcedStyle
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added a validation for supported styles other than EnforcedStyle.
Problem
Currently, EnforcedStyle value is validated by SupportedStyles.
e.g.
However, other styles aren't validated.
e.g.
Goal
Validate the invalid value.
e.g.
Changes
EnforcedStyleSupportedStylesnames to match EnforcedStyle nameSupportedStylestoSupportedStylesInsidePipes(forEnforcedStyleInsidePipes)SupportedStylesfor missing styles in config/default.ymlSupportedStylesintoproject_spec.rbBefore submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).and description in grammatically correct, complete sentences.
rake generate_cops_documentation(required only when you've added a new cop or changed the configuration/documentation of an existing cop).