Merge pull request #596 from tdeo/sequence_symbol_proc_argument

Fix false positive in when a sequence has a symbol proc argument.
rubocop · Apr 10, 2018 · 7f0fcb2 · 7f0fcb2
2 parents 24c18ac + c26e8e2
commit 7f0fcb2
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Master (Unreleased)
 
 * Fix false positive in `RSpec/Pending` cop when pending is used as a method name.  ([@Darhazer][])
+* Fix `FactoryBot/DynamicAttributeDefinedStatically` false positive when using symbol proc argument for a sequence. ([@tdeo][])
 
 ## 1.25.0 (2018-04-07)
 
@@ -330,3 +331,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
 [@jojos003]: https://github.com/jojos003
 [@abrom]: https://github.com/abrom
 [@patrickomatic]: https://github.com/patrickomatic
+[@tdeo]: https://github.com/tdeo
diff --git a/lib/rubocop/cop/rspec/factory_bot/dynamic_attribute_defined_statically.rb b/lib/rubocop/cop/rspec/factory_bot/dynamic_attribute_defined_statically.rb
@@ -31,14 +31,9 @@ class DynamicAttributeDefinedStatically < Cop
             (block (send nil? {:factory :trait} ...) _ { (begin $...) $(send ...) } )
           PATTERN
 
-          def_node_matcher :callback_with_symbol_proc?, <<-PATTERN
-            (send nil? {:before :after} sym (block_pass sym))
-          PATTERN
-
           def on_block(node)
             factory_attributes(node).to_a.flatten.each do |attribute|
-              next if callback_with_symbol_proc?(attribute) ||
-                  static?(attribute)
+              next if static_or_proc?(attribute)
               add_offense(attribute, location: :expression)
             end
           end
@@ -55,8 +50,10 @@ def autocorrect(node)
 
           private
 
-          def static?(attribute)
-            value_matcher(attribute).to_a.all?(&:recursive_literal_or_const?)
+          def static_or_proc?(attribute)
+            value_matcher(attribute).to_a.all? do |value|
+              value.block_pass_type? || value.recursive_literal_or_const?
+            end
           end
 
           def value_hash_without_braces?(node)

diff --git a/spec/rubocop/cop/rspec/factory_bot/dynamic_attribute_defined_statically_spec.rb b/spec/rubocop/cop/rspec/factory_bot/dynamic_attribute_defined_statically_spec.rb
@@ -87,6 +87,16 @@
         RUBY
       end
 
+      it 'accepts valid sequence definition' do
+        expect_no_offenses(<<-RUBY)
+          #{factory_bot}.define do
+            factory :post do
+              sequence :negative_numbers, &:-@
+            end
+          end
+        RUBY
+      end
+
       bad = <<-RUBY
         #{factory_bot}.define do
           factory :post do