diff --git a/CHANGELOG.md b/CHANGELOG.md index 40f323aa..80eed7e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Add `FactoryBot/RedundantFactoryOption` cop. ([@r7kamura]) - Add `ExplicitOnly` configuration option to `FactoryBot/ConsistentParenthesesStyle`, `FactoryBot/CreateList` and `FactoryBot/FactoryNameStyle`. ([@ydah]) - Change `FactoryBot/CreateList` so that it checks same factory calls in an Array. ([@r7kamura]) +- Add `FactoryBot/AssociationStyle` cop. ([@r7kamura]) ## 2.22.0 (2023-05-04) diff --git a/config/default.yml b/config/default.yml index 67b9f278..8640bad7 100644 --- a/config/default.yml +++ b/config/default.yml @@ -8,6 +8,22 @@ FactoryBot: - "**/features/support/factories/**/*.rb" DocumentationBaseURL: https://docs.rubocop.org/rubocop-factory_bot +FactoryBot/AssociationStyle: + Description: Use a consistent style to define associations. + Enabled: pending + Safe: false + Include: + - spec/factories.rb + - spec/factories/**/*.rb + - features/support/factories/**/*.rb + VersionAdded: "<>" + EnforcedStyle: implicit + SupportedStyles: + - explicit + - implicit + NonImplicitAssociationMethodNames: ~ + Reference: https://www.rubydoc.info/gems/rubocop-factory_bot/RuboCop/Cop/FactoryBot/AssociationStyle + FactoryBot/AttributeDefinedStatically: Description: Always declare attribute values as blocks. Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 62f77339..d3c52e64 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -2,6 +2,7 @@ === Department xref:cops_factorybot.adoc[FactoryBot] +* xref:cops_factorybot.adoc#factorybotassociationstyle[FactoryBot/AssociationStyle] * xref:cops_factorybot.adoc#factorybotattributedefinedstatically[FactoryBot/AttributeDefinedStatically] * xref:cops_factorybot.adoc#factorybotconsistentparenthesesstyle[FactoryBot/ConsistentParenthesesStyle] * xref:cops_factorybot.adoc#factorybotcreatelist[FactoryBot/CreateList] diff --git a/docs/modules/ROOT/pages/cops_factorybot.adoc b/docs/modules/ROOT/pages/cops_factorybot.adoc index d3a8f8e4..d7cc2a42 100644 --- a/docs/modules/ROOT/pages/cops_factorybot.adoc +++ b/docs/modules/ROOT/pages/cops_factorybot.adoc @@ -1,5 +1,88 @@ = FactoryBot +== FactoryBot/AssociationStyle + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| No +| Yes (Unsafe) +| <> +| - +|=== + +Use a consistent style to define associations. + +=== Safety + +This cop may cause false-positives in `EnforcedStyle: explicit` +case. It recognizes any method call that has no arguments as an +implicit association but it might be a user-defined trait call. + +=== Examples + +==== EnforcedStyle: implicit (default) + +[source,ruby] +---- +# bad +factory :post do + association :user +end + +# good +factory :post do + user +end +---- + +==== EnforcedStyle: explicit + +[source,ruby] +---- +# bad +factory :post do + user +end + +# good +factory :post do + association :user +end + +# good (NonImplicitAssociationMethodNames: ['email']) +sequence :email do |n| + "person#{n}@example.com" +end + +factory :user do + email +end +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| Include +| `spec/factories.rb`, `+spec/factories/**/*.rb+`, `+features/support/factories/**/*.rb+` +| Array + +| EnforcedStyle +| `implicit` +| `explicit`, `implicit` + +| NonImplicitAssociationMethodNames +| `` +| +|=== + +=== References + +* https://www.rubydoc.info/gems/rubocop-factory_bot/RuboCop/Cop/FactoryBot/AssociationStyle + == FactoryBot/AttributeDefinedStatically |=== diff --git a/lib/rubocop/cop/factory_bot/association_style.rb b/lib/rubocop/cop/factory_bot/association_style.rb new file mode 100644 index 00000000..726a78ab --- /dev/null +++ b/lib/rubocop/cop/factory_bot/association_style.rb @@ -0,0 +1,196 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module FactoryBot + # Use a consistent style to define associations. + # + # @safety + # This cop may cause false-positives in `EnforcedStyle: explicit` + # case. It recognizes any method call that has no arguments as an + # implicit association but it might be a user-defined trait call. + # + # @example EnforcedStyle: implicit (default) + # # bad + # factory :post do + # association :user + # end + # + # # good + # factory :post do + # user + # end + # + # @example EnforcedStyle: explicit + # # bad + # factory :post do + # user + # end + # + # # good + # factory :post do + # association :user + # end + # + # # good (NonImplicitAssociationMethodNames: ['email']) + # sequence :email do |n| + # "person#{n}@example.com" + # end + # + # factory :user do + # email + # end + class AssociationStyle < ::RuboCop::Cop::Base # rubocop:disable Metrics/ClassLength + extend AutoCorrector + + include ConfigurableEnforcedStyle + + DEFAULT_NON_IMPLICIT_ASSOCIATION_METHOD_NAMES = %w[ + association + sequence + skip_create + traits_for_enum + ].freeze + + RESTRICT_ON_SEND = %i[factory trait].freeze + + def on_send(node) + bad_associations_in(node).each do |association| + add_offense( + association, + message: "Use #{style} style to define associations." + ) do |corrector| + autocorrect(corrector, association) + end + end + end + + private + + # @!method explicit_association?(node) + def_node_matcher :explicit_association?, <<~PATTERN + (send nil? :association sym ...) + PATTERN + + # @!method implicit_association?(node) + def_node_matcher :implicit_association?, <<~PATTERN + (send nil? !#non_implicit_association_method_name? ...) + PATTERN + + # @!method factory_option_matcher(node) + def_node_matcher :factory_option_matcher, <<~PATTERN + (send + nil? + :association + ... + (hash + < + (pair + (sym :factory) + { + (sym $_) | + (array (sym $_)*) + } + ) + ... + > + ) + ) + PATTERN + + # @!method trait_names_from_explicit(node) + def_node_matcher :trait_names_from_explicit, <<~PATTERN + (send nil? :association _ (sym $_)* ...) + PATTERN + + def autocorrect(corrector, node) + if style == :explicit + autocorrect_to_explicit_style(corrector, node) + else + autocorrect_to_implicit_style(corrector, node) + end + end + + def autocorrect_to_explicit_style(corrector, node) + arguments = [ + ":#{node.method_name}", + *node.arguments.map(&:source) + ] + corrector.replace(node, "association #{arguments.join(', ')}") + end + + def autocorrect_to_implicit_style(corrector, node) + source = node.first_argument.value.to_s + options = options_for_autocorrect_to_implicit_style(node) + unless options.empty? + rest = options.map { |option| option.join(': ') }.join(', ') + source += " #{rest}" + end + corrector.replace(node, source) + end + + def bad?(node) + if style == :explicit + implicit_association?(node) + else + explicit_association?(node) + end + end + + def bad_associations_in(node) + children_of_factory_block(node).select do |child| + bad?(child) + end + end + + def children_of_factory_block(node) + block = node.parent + return [] unless block + return [] unless block.body + + if block.body.begin_type? + block.body.children + else + [block.body] + end + 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) + end + result + end + + def non_implicit_association_method_name?(method_name) + non_implicit_association_method_names.include?(method_name.to_s) + end + + def non_implicit_association_method_names + DEFAULT_NON_IMPLICIT_ASSOCIATION_METHOD_NAMES + + (cop_config['NonImplicitAssociationMethodNames'] || []) + end + + def options_from_explicit(node) + return {} unless node.last_argument.hash_type? + + node.last_argument.pairs.inject({}) do |options, pair| + options.merge(pair.key.value => pair.value.source) + end + end + + 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(' ')}]" + end + options + end + end + end + end +end diff --git a/lib/rubocop/cop/factory_bot_cops.rb b/lib/rubocop/cop/factory_bot_cops.rb index 80b9ea9f..1c680da3 100644 --- a/lib/rubocop/cop/factory_bot_cops.rb +++ b/lib/rubocop/cop/factory_bot_cops.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require_relative 'factory_bot/association_style' require_relative 'factory_bot/attribute_defined_statically' require_relative 'factory_bot/consistent_parentheses_style' require_relative 'factory_bot/create_list' diff --git a/spec/rubocop/cop/factory_bot/association_style_spec.rb b/spec/rubocop/cop/factory_bot/association_style_spec.rb new file mode 100644 index 00000000..71cf6b27 --- /dev/null +++ b/spec/rubocop/cop/factory_bot/association_style_spec.rb @@ -0,0 +1,275 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::FactoryBot::AssociationStyle do + def inspected_source_filename + 'spec/factories.rb' + end + + let(:cop_config) do + { 'EnforcedStyle' => enforced_style } + end + + context 'when EnforcedStyle is :implicit' do + let(:enforced_style) { :implicit } + + context 'when factory block is empty' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + factory :user do + end + RUBY + end + end + + context 'with when factory has no block' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + factory :user + RUBY + end + end + + context 'when implicit style is used' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + factory :article do + user + end + RUBY + end + end + + context 'when `association` is called in attribute block' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + factory :article do + author do + association :user + end + end + RUBY + end + end + + context 'when `association` has only 1 argument' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :user + ^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + user + end + RUBY + end + end + + context 'when `association` is called in trait block' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + trait :with_user do + association :user + ^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + trait :with_user do + user + end + end + RUBY + end + end + + context 'when `association` is called with trait' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :user, :admin + ^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + user factory: %i[user admin] + end + RUBY + end + end + + context 'when `association` is called with factory option' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :author, factory: :user + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + author factory: %i[user] + end + RUBY + end + end + + context 'when `association` is called with array factory option' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :author, factory: %i[user] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + author factory: %i[user] + end + RUBY + end + end + + context 'when `association` is called with trait arguments and factory' \ + 'option' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :author, :admin, factory: :user + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + author factory: %i[user admin] + end + RUBY + end + end + end + + context 'when EnforcedStyle is :explicit' do + let(:enforced_style) { :explicit } + + context 'when explicit style is used' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + factory :article do + association :user + end + RUBY + end + end + + context 'when implicit association is used without any arguments' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + user + ^^^^ Use explicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + association :user + end + RUBY + end + end + + context 'when implicit association is used with arguments' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + author factory: :user + ^^^^^^^^^^^^^^^^^^^^^ Use explicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + association :author, factory: :user + end + RUBY + end + end + + context 'when implicit association has factory and traits' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + author factory: %i[user admin] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use explicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + association :author, factory: %i[user admin] + end + RUBY + end + end + + context 'when default non implicit association method name is used' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + factory :article do + skip_create + end + RUBY + end + end + + context 'when custom non implicit association method name is used' do + let(:cop_config) do + { 'NonImplicitAssociationMethods' => %w[email] } + end + + it 'does not register an offense' do + expect_no_offenses(<<~'RUBY') + sequence(:email) { |n| "person#{n}@example.com" } + + factory :user do + email + + skip_create + end + RUBY + end + end + + context 'when implicit association is called in trait block' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + trait :with_user do + user + ^^^^ Use explicit style to define associations. + end + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + trait :with_user do + association :user + end + end + RUBY + end + end + end +end