diff --git a/CHANGELOG.md b/CHANGELOG.md index a5a62cf..56c4602 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Fix an error for `FactoryBot/AssociationStyle` cop when `trait` is not inside `factory` block. ([@viralpraxis]) - Fix an error for `FactoryBot/ConsistentParenthesesStyle` cop when using keyword splat argument. ([@viralpraxis]) - Fix a false negative for `FactoryBot/SyntaxMethods` when method is used inside a module. ([@lovro-bikic]) +- Fix an autocorrection for `FactoryBot/AssociationStyle` cop when the trait is not a symbol literal. ([@ydah]) ## 2.27.1 (2025-03-12) diff --git a/lib/rubocop/cop/factory_bot/association_style.rb b/lib/rubocop/cop/factory_bot/association_style.rb index 7f823cb..5263ef2 100644 --- a/lib/rubocop/cop/factory_bot/association_style.rb +++ b/lib/rubocop/cop/factory_bot/association_style.rb @@ -131,9 +131,9 @@ def on_send(node) ) PATTERN - # @!method trait_names_from_explicit(node) - def_node_matcher :trait_names_from_explicit, <<~PATTERN - (send nil? :association _ (sym $_)* ...) + # @!method trait_arguments(node) + def_node_matcher :trait_arguments, <<~PATTERN + (send nil? :association sym $_ ...) PATTERN # @!method association_names(node) @@ -214,11 +214,34 @@ def children_of_factory_block(node) end def factory_names_from_explicit(node) - trait_names = trait_names_from_explicit(node) - factory_names = Array(factory_option_matcher(node)) - result = factory_names + trait_names - if factory_names.empty? && !trait_names.empty? - result.prepend(node.first_argument.value) + factory_option_names = Array(factory_option_matcher(node)) + trait_args = collect_trait_arguments(node) + if factory_option_names.any? + build_factory_list_with_option(factory_option_names, trait_args) + elsif trait_args.any? + build_factory_list_without_option(node.first_argument.value, + trait_args) + else + [] + end + end + + def collect_trait_arguments(node) + node.arguments[1..].reject(&:hash_type?) + end + + def build_factory_list_with_option(factory_option_names, trait_args) + result = factory_option_names.dup + trait_args.each do |arg| + result << (arg.sym_type? ? arg.value : arg.source) + end + result + end + + def build_factory_list_without_option(association_name, trait_args) + result = [association_name] + trait_args.each do |arg| + result << (arg.sym_type? ? arg.value : arg.source) end result end @@ -244,11 +267,23 @@ def options_for_autocorrect_to_implicit_style(node) options = options_from_explicit(node) factory_names = factory_names_from_explicit(node) unless factory_names.empty? - options[:factory] = "%i[#{factory_names.join(' ')}]" + options[:factory] = implicit_style(factory_names) end options end + def implicit_style(factory_names) + if factory_names.any?(String) + "[#{formatted_elements(factory_names).join(', ')}]" + else + "%i[#{factory_names.join(' ')}]" + end + end + + def formatted_elements(factory_names) + factory_names.map { |name| name.is_a?(Symbol) ? ":#{name}" : name } + end + def trait_within_trait?(node, factory_node) trait_name(factory_node).include?(node.method_name) end diff --git a/spec/rubocop/cop/factory_bot/association_style_spec.rb b/spec/rubocop/cop/factory_bot/association_style_spec.rb index b0dc804..f070aa7 100644 --- a/spec/rubocop/cop/factory_bot/association_style_spec.rb +++ b/spec/rubocop/cop/factory_bot/association_style_spec.rb @@ -113,6 +113,111 @@ def inspected_source_filename end end + context 'when `association` is called with non-symbol trait' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :user, foo + ^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + user factory: [:user, foo] + end + RUBY + end + end + + context 'when `association` is called with mixed symbol and ' \ + 'non-symbol traits' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :user, :admin, foo + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + user factory: [:user, :admin, foo] + end + RUBY + end + end + + context 'when `association` is called with multiple non-symbol traits' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :user, foo, bar + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + user factory: [:user, foo, bar] + end + RUBY + end + end + + context 'when `association` is called with non-symbol trait and ' \ + 'factory option' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :author, foo, factory: :user + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + author factory: [:user, foo] + end + RUBY + end + end + + context 'when `association` is called with method call as trait' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :user, get_trait + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + user factory: [:user, get_trait] + end + RUBY + end + end + + context 'when `association` is called with conditional expression ' \ + 'as trait' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :user, admin? ? :admin : :regular + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + user factory: [:user, admin? ? :admin : :regular] + end + RUBY + end + end + context 'when `association` is called with factory option' do it 'registers and corrects an offense' do expect_offense(<<~RUBY)