[Fix #10359] Fix a false positive and negative for Style/HashSyntax

Fixes #10359. This PR fixes a false positive and negative for `Style/HashSyntax` when using hash value omission. It resolves the lack of consideration for https://bugs.ruby-lang.org/issues/18396 and false positives for modifier forms.
rubocop · Jan 19, 2022 · e6b5607 · e6b5607
1 parent bb7e5b0
commit e6b5607
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 29 deletions.
diff --git a/changelog/fix_false_positive_for_style_hash_syntax.md b/changelog/fix_false_positive_for_style_hash_syntax.md
@@ -0,0 +1 @@
+* [#10359](https://github.com/rubocop/rubocop/issues/10359): Fix a false positive and negative for `Style/HashSyntax` when using hash value omission. ([@koic][])
diff --git a/lib/rubocop/cop/mixin/hash_shorthand_syntax.rb b/lib/rubocop/cop/mixin/hash_shorthand_syntax.rb
@@ -36,32 +36,38 @@ def enforced_shorthand_syntax
       end
 
       def require_hash_value?(hash_key_source, node)
-        return true if without_parentheses_call_expr_follows?(node)
+        return true if require_hash_value_for_around_hash_literal(node)
 
         hash_value = node.value
         return true unless hash_value.send_type? || hash_value.lvar_type?
 
         hash_key_source != hash_value.source || hash_key_source.end_with?('!', '?')
       end
 
-      def without_parentheses_call_expr_follows?(node)
+      def require_hash_value_for_around_hash_literal(node)
         return false unless (ancestor = node.parent.parent)
 
-        right_sibling = ancestor.right_sibling
-
-        return true if right_sibling.nil? && without_parentheses?(ancestor)
-        return false unless right_sibling
-        return true if node_with_block_and_arguments?(right_sibling)
+        !use_element_of_hash_literal_as_receiver?(ancestor, node.parent) &&
+          (use_modifier_form_without_parenthesized_method_call?(ancestor) ||
+           without_parentheses_call_expr_follows?(ancestor))
+      end
 
-        without_parentheses?(ancestor) && without_parentheses?(right_sibling)
+      def use_element_of_hash_literal_as_receiver?(ancestor, parent)
+        # `{value:}.do_something` is a valid syntax.
+        ancestor.send_type? && ancestor.receiver == parent
       end
 
-      def without_parentheses?(node)
-        node.respond_to?(:parenthesized?) && !node.parenthesized?
+      def use_modifier_form_without_parenthesized_method_call?(ancestor)
+        return false if ancestor.respond_to?(:parenthesized?) && ancestor.parenthesized?
+        return false unless (parent = ancestor.parent)
+
+        parent.respond_to?(:modifier_form?) && parent.modifier_form?
       end
 
-      def node_with_block_and_arguments?(node)
-        node.respond_to?(:block_type?) && node.block_type? && node.children&.first&.arguments?
+      def without_parentheses_call_expr_follows?(ancestor)
+        return false unless (right_sibling = ancestor.right_sibling)
+
+        ancestor.respond_to?(:parenthesized?) && !ancestor.parenthesized? && right_sibling
       end
     end
   end

diff --git a/spec/rubocop/cop/style/hash_syntax_spec.rb b/spec/rubocop/cop/style/hash_syntax_spec.rb
@@ -819,6 +819,39 @@
         RUBY
       end
 
+      it 'registers and corrects an offense when `Hash[foo: foo]`' do
+        expect_offense(<<~RUBY)
+          Hash[foo: foo]
+                    ^^^ Omit the hash value.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          Hash[foo:]
+        RUBY
+      end
+
+      it 'registers and corrects an offense when hash key and hash value are the same and it in the method body' do
+        expect_offense(<<~RUBY)
+          def do_something
+            {
+              foo: foo,
+                   ^^^ Omit the hash value.
+              bar: bar
+                   ^^^ Omit the hash value.
+            }
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          def do_something
+            {
+              foo:,
+              bar:
+            }
+          end
+        RUBY
+      end
+
       it 'does not register an offense when hash values are omitted' do
         expect_no_offenses(<<~RUBY)
           {foo:, bar:}
@@ -891,12 +924,40 @@
         RUBY
       end
 
-      it 'does not register an offense when one line condition follows' do
+      it 'registers an offense when one line `if` condition follows (with parentheses)' do
+        expect_offense(<<~RUBY)
+          foo(value: value) if bar
+                     ^^^^^ Omit the hash value.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo(value:) if bar
+        RUBY
+      end
+
+      it 'does not register an offense when one line `if` condition follows (without parentheses)' do
         expect_no_offenses(<<~RUBY)
           foo value: value if bar
         RUBY
       end
 
+      it 'registers an offense when one line `until` condition follows (with parentheses)' do
+        expect_offense(<<~RUBY)
+          foo(value: value) until bar
+                     ^^^^^ Omit the hash value.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo(value:) until bar
+        RUBY
+      end
+
+      it 'does not register an offense when one line `until` condition follows (without parentheses)' do
+        expect_no_offenses(<<~RUBY)
+          foo value: value until bar
+        RUBY
+      end
+
       it 'does not register an offense when call expr with argument and a block follows' do
         expect_no_offenses(<<~RUBY)
           foo value: value
@@ -906,33 +967,30 @@
         RUBY
       end
 
-      it 'registers and corrects an offense when call expr without arguments and with a block follows' do
-        expect_offense(<<~RUBY)
+      it 'does not register an offense when call expr without arguments and with a block follows' do
+        # Prevent syntax semantic changes shown in the URL: https://bugs.ruby-lang.org/issues/18396
+        expect_no_offenses(<<~RUBY)
           foo value: value
-                     ^^^^^ Omit the hash value.
-          bar do
-            value
-          end
-        RUBY
-
-        expect_correction(<<~RUBY)
-          foo value:
           bar do
             value
           end
         RUBY
       end
 
-      it 'registers and corrects an offense when with parentheses call expr follows' do
-        expect_offense(<<~RUBY)
+      it 'does not register an offense when with parentheses call expr follows' do
+        # Prevent syntax semantic changes shown in the URL: https://bugs.ruby-lang.org/issues/18396
+        expect_no_offenses(<<~RUBY)
           foo value: value
-                     ^^^^^ Omit the hash value.
           foo(arg)
         RUBY
+      end
 
-        expect_correction(<<~RUBY)
-          foo value:
-          foo(arg)
+      it 'does not register an offense when hash key and hash value are partially the same' do
+        expect_no_offenses(<<~RUBY)
+          def do_something
+            do_something foo: foo
+            do_something(arg)
+          end
         RUBY
       end
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		* [#10359](https://github.com/rubocop/rubocop/issues/10359): Fix a false positive and negative for `Style/HashSyntax` when using hash value omission. ([@koic][])