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

[Fix #11436] Add new consistent_either SupportedShorthandSyntax to Style/HashSyntax #12904

Merged
merged 4 commits into from
May 21, 2024

Conversation

pawelma
Copy link
Contributor

@pawelma pawelma commented May 15, 2024

Related to feature request: #11436
Based on: #10776

WHAT

This commit adds a new consistent_either option to EnforcedShorthandSyntax in Style/HashSyntax.

This allows to use both explicit and shorthand syntax, similarly to existing either option, but with the difference that it enforces consistency within a single hash.

This PR extends existing consistent option, and skips the no_mixed_shorthand_syntax_check offences when all values in the hash are ommitable.

@example EnforcedShorthandSyntax: consistent_either

+  # good - `foo` and `bar` values can be omitted, but they are consistent, so it's accepted
+ {foo: foo, bar: bar}

-  # bad - `bar` value can be omitted
-  {foo:, bar: bar}

-  # bad - mixed syntaxes
-  {foo:, bar: baz}

+  # good
+  {foo:, bar:}
  
+  # good - can't omit `baz`
+  {foo: foo, bar: baz}
bundle exec rake default - output
bundle exec rake default
Files:         681
Modules:        92 (   13 undocumented)
Classes:       643 (    4 undocumented)
Constants:    1171 ( 1155 undocumented)
Attributes:     37 (    0 undocumented)
Methods:      1480 ( 1291 undocumented)
 28.05% documented
/Users/pawel.madejski/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/parser-3.3.1.0/lib/parser/builders/default.rb:2266: warning: regular expression has redundant nested repeat operator '+': /(?:x+)+/
/Users/pawel.madejski/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/parser-3.3.1.0/lib/parser/builders/default.rb:2266: warning: nested repeat operator '+' and '?' was replaced with '*' in regular expression: /(?:x+)?/
Starting test-queue master (/tmp/test_queue_62161_14840.sock)

==> Summary (10 workers in 110.4666s)

    [ 1]                            136 examples, 0 failures         1 suites in 110.4388s      (pid 62344 exit 0 )
    [ 2]                              9 examples, 0 failures         1 suites in 110.4381s      (pid 62345 exit 0 )
    [ 3]                            122 examples, 0 failures         1 suites in 110.4376s      (pid 62346 exit 0 )
    [ 4]                           2189 examples, 0 failures       103 suites in 110.4364s      (pid 62347 exit 0 )
    [ 5]                            119 examples, 0 failures         1 suites in 110.4375s      (pid 62348 exit 0 )
    [ 6]                           3745 examples, 0 failures       111 suites in 110.4375s      (pid 62349 exit 0 )
    [ 7]                           3377 examples, 0 failures       109 suites in 110.4383s      (pid 62350 exit 0 )
    [ 8]                           4569 examples, 0 failures       116 suites in 110.4392s      (pid 62351 exit 0 )
    [ 9]                           3997 examples, 0 failures       110 suites in 110.4377s      (pid 62352 exit 0 )
    [10]                4014 examples, 1 pending, 0 failures       117 suites in 110.4368s      (pid 62353 exit 0 )

PARSER_ENGINE=parser_prism bundle exec rake spec
Starting test-queue master (/tmp/test_queue_62664_5220.sock)

==> Summary (10 workers in 106.6212s)

    [ 1]                           3244 examples, 0 failures       109 suites in 35.3706s      (pid 62671 exit 0 )
    [ 2]                            122 examples, 0 failures         1 suites in 39.7622s      (pid 62672 exit 0 )
    [ 3]                            136 examples, 0 failures         1 suites in 106.5994s      (pid 62673 exit 0 )
    [ 4]                              9 examples, 0 failures         1 suites in 106.5998s      (pid 62674 exit 0 )
    [ 5]                            119 examples, 0 failures         1 suites in 106.6000s      (pid 62675 exit 0 )
    [ 6]                           2274 examples, 0 failures       102 suites in 106.5998s      (pid 62676 exit 0 )
    [ 7]                3827 examples, 1 pending, 0 failures       115 suites in 106.6022s      (pid 62677 exit 0 )
    [ 8]                           3530 examples, 0 failures       112 suites in 106.6020s      (pid 62678 exit 0 )
    [ 9]                           4008 examples, 0 failures       111 suites in 106.6021s      (pid 62680 exit 0 )
    [10]                           4393 examples, 0 failures       117 suites in 106.6007s      (pid 62681 exit 0 )

Running RuboCop...
Inspecting 1536 files
................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

1536 files inspected, no offenses detected

Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

This commit adds a new `consistent_either` to `EnforcedShorthandSyntax`.

Related to feature request: rubocop#11436

This allows to use both explicit and shorthand syntax, similarly to
existing `either` option, but with the difference that it enforces
consistency within a single hash.

This PR extends existing `consistent` option, and skips the
`no_mixed_shorthand_syntax_check` offences when all values in the hash
are ommitable.
@pawelma pawelma changed the title Add new consistent_either SupportedShorthandSyntax to Style/HashSyntax Add new consistent_either SupportedShorthandSyntax to Style/HashSyntax May 15, 2024
@pawelma pawelma changed the title Add new consistent_either SupportedShorthandSyntax to Style/HashSyntax [Fix #11436] Add new consistent_either SupportedShorthandSyntax to Style/HashSyntax May 19, 2024
@@ -172,6 +173,11 @@ def hash_with_values_that_cant_be_omitted?(hash_value_type_breakdown)
hash_value_type_breakdown[:value_needed]&.any?
end

def ignore_explicit_ommitable_hash_shorthand_syntax?(hash_value_type_breakdown)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no word "ommittable" - I think you meant to use "omissible". :-) See https://www.merriam-webster.com/dictionary/omissible

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, TIL, updated f91da7d

@bbatsov
Copy link
Collaborator

bbatsov commented May 20, 2024

The suggested change seems reasonable to me. Apart from my remark about the use of the non-existing word "ommitable" here and there I'd also suggest changing the name to either_consistent, as to me at least it's easier to understand what it's supposed to do if the words are in this order. Perhaps this should have been the behavior for the consistent option or the either option but that ship has sailed.

@rubocop/rubocop-core Feel free to chime on the name here.

…consistent

This will indicate that it works like `either` option which contains a
consistency within a single hash definition.

Suggested in
rubocop#12904 (comment) PR
comment
@pawelma
Copy link
Contributor Author

pawelma commented May 20, 2024

I'd also suggest changing the name to either_consistent, as to me at least it's easier to understand what it's supposed to do if the words are in this order.

I like the suggested name updated in scope of 9ce04cf

@pawelma pawelma requested a review from bbatsov May 21, 2024 19:56
@bbatsov bbatsov merged commit aee64e2 into rubocop:master May 21, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants