diff --git a/CHANGELOG.md b/CHANGELOG.md index 777e08817..5379bc811 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Add new `RSpec/FactoryBot/FactoryNameStyle` cop. ([@ydah]) - Fix wrong autocorrection in `n_times` style on `RSpec/FactoryBot/CreateList`. ([@r7kamura]) - Fix a false positive for `RSpec/FactoryBot/ConsistentParenthesesStyle` when using `generate` with multiple arguments. ([@ydah]) +- Add `RSpec/FactoryBot/AssociationStyle` cop. ([@r7kamura]) ## 2.15.0 (2022-11-03) diff --git a/config/default.yml b/config/default.yml index 63696a7ac..2f26f03fb 100644 --- a/config/default.yml +++ b/config/default.yml @@ -911,6 +911,22 @@ RSpec/FactoryBot: Include: *1 Language: *2 +RSpec/FactoryBot/AssociationStyle: + Description: Use 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-rspec/RuboCop/Cop/RSpec/FactoryBot/AssociationStyle + RSpec/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 541b45f04..296062fd9 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -104,6 +104,7 @@ === Department xref:cops_rspec_factorybot.adoc[RSpec/FactoryBot] +* xref:cops_rspec_factorybot.adoc#rspecfactorybot/associationstyle[RSpec/FactoryBot/AssociationStyle] * xref:cops_rspec_factorybot.adoc#rspecfactorybot/attributedefinedstatically[RSpec/FactoryBot/AttributeDefinedStatically] * xref:cops_rspec_factorybot.adoc#rspecfactorybot/consistentparenthesesstyle[RSpec/FactoryBot/ConsistentParenthesesStyle] * xref:cops_rspec_factorybot.adoc#rspecfactorybot/createlist[RSpec/FactoryBot/CreateList] diff --git a/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc b/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc index 366c36b1a..847beb2e9 100644 --- a/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc +++ b/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc @@ -1,5 +1,74 @@ = RSpec/FactoryBot +== RSpec/FactoryBot/AssociationStyle + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| No +| Yes (Unsafe) +| <> +| - +|=== + +Use 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 +association :user + +# good +user +---- + +==== EnforcedStyle: explicit + +[source,ruby] +---- +# bad +user + +# good +association :user + +# good (defined in NonImplicitAssociationMethodNames) +skip_create +---- + +=== 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-rspec/RuboCop/Cop/RSpec/FactoryBot/AssociationStyle + == RSpec/FactoryBot/AttributeDefinedStatically |=== diff --git a/lib/rubocop/cop/rspec/factory_bot/association_style.rb b/lib/rubocop/cop/rspec/factory_bot/association_style.rb new file mode 100644 index 000000000..f39af489d --- /dev/null +++ b/lib/rubocop/cop/rspec/factory_bot/association_style.rb @@ -0,0 +1,254 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + module FactoryBot + # Use 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 + # association :user + # + # # good + # user + # + # @example EnforcedStyle: explicit + # # bad + # user + # + # # good + # association :user + # + # # good (defined in NonImplicitAssociationMethodNames) + # skip_create + class AssociationStyle < 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 + + # @param node [RuboCop::AST::SendNode] + # @return [void] + 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) + # @param node [RuboCop::AST::SendNode] + # @return [Boolean] + def_node_matcher :explicit_association?, <<~PATTERN + (send nil? :association sym ...) + PATTERN + + # @!method implicit_association?(node) + # @param node [RuboCop::AST::SendNode] + # @return [Boolean] + def_node_matcher :implicit_association?, <<~PATTERN + (send nil? !#non_implicit_association_method_name? ...) + PATTERN + + # @!method factory_option_matcher(node) + # @param node [RuboCop::AST::SendNode] + # @return [Array, Symbol, nil] + def_node_matcher :factory_option_matcher, <<~PATTERN + (send + nil? + :association + ... + (hash + < + (pair + (sym :factory) + { + (sym $_) | + (array (sym $_)*) + } + ) + ... + > + ) + ) + PATTERN + + # @!method trait_arguments_matcher(node) + # @param node [RuboCop::AST::SendNode] + # @return [Array, nil] + def_node_matcher :trait_arguments_matcher, <<~PATTERN + (send nil? :association _ (sym $_)* ...) + PATTERN + + # @!method trait_option_matcher(node) + # @param node [RuboCop::AST::SendNode] + # @return [Array, nil] + def_node_matcher :trait_option_matcher, <<~PATTERN + (send + nil? + :association + ... + (hash + < + (pair + (sym :traits) + (array (sym $_)*) + ) + ... + > + ) + ) + PATTERN + + # @param corrector [RuboCop::Cop::Corrector] + # @param node [RuboCop::AST::SendNode] + def autocorrect(corrector, node) + case style + when :explicit + autocorrect_to_explicit_style(corrector, node) + when :implicit + autocorrect_to_implicit_style(corrector, node) + end + end + + # @param corrector [RuboCop::Cop::Corrector] + # @param node [RuboCop::AST::SendNode] + # @return [void] + def autocorrect_to_explicit_style(corrector, node) + arguments = [ + ":#{node.method_name}", + *node.arguments.map(&:source) + ] + corrector.replace(node, "association #{arguments.join(', ')}") + end + + # @param corrector [RuboCop::Cop::Corrector] + # @param node [RuboCop::AST::SendNode] + # @return [void] + 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 + + # @param node [RuboCop::AST::SendNode] + # @return [Boolean] + def autocorrectable_to_implicit_style?(node) + node.arguments.one? + end + + # @param node [RuboCop::AST::SendNode] + # @return [Boolean] + def bad?(node) + case style + when :explicit + implicit_association?(node) + when :implicit + explicit_association?(node) + end + end + + # @param node [RuboCop::AST::SendNode] + # @return [Array] + def bad_associations_in(node) + children_of_factory_block(node).select do |child| + bad?(child) + end + end + + # @param node [RuboCop::AST::SendNode] + # @return [Array] + def children_of_factory_block(node) + block = node.parent + return [] unless block + return [] unless block.block_type? + return [] unless block.body + + if block.body.begin_type? + block.body.children + else + [block.body] + end + end + + # @param node [RuboCop::AST::SendNode] + # @return [Array] + 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 + + # @param method_name [Symbol] + # @return [Boolean] + def non_implicit_association_method_name?(method_name) + non_implicit_association_method_names.include?(method_name.to_s) + end + + # @return [Array] + def non_implicit_association_method_names + DEFAULT_NON_IMPLICIT_ASSOCIATION_METHOD_NAMES + + (cop_config['NonImplicitAssociationMethodNames'] || []) + end + + # @param node [RuboCop::AST::SendNode] + # @return [Hash{Symbol => String}] + 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 + + # @param node [RuboCop::AST::SendNode] + # @return [Hash{Symbol => String}] + def options_for_autocorrect_to_implicit_style(node) + options = options_from_explicit(node) + options.delete(:traits) + factory_names = factory_names_from_explicit(node) + unless factory_names.empty? + options[:factory] = "%i[#{factory_names.join(' ')}]" + end + options + end + + # @param node [RuboCop::AST::SendNode] + # @return [Array] + def trait_names_from_explicit(node) + (trait_arguments_matcher(node) || []) + + (trait_option_matcher(node) || []) + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index d68fa240e..9953f5cf9 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -8,6 +8,7 @@ require_relative 'rspec/capybara/specific_matcher' require_relative 'rspec/capybara/visibility_matcher' +require_relative 'rspec/factory_bot/association_style' require_relative 'rspec/factory_bot/attribute_defined_statically' require_relative 'rspec/factory_bot/consistent_parentheses_style' require_relative 'rspec/factory_bot/create_list' diff --git a/spec/rubocop/cop/rspec/factory_bot/association_style_spec.rb b/spec/rubocop/cop/rspec/factory_bot/association_style_spec.rb new file mode 100644 index 000000000..0dba183c1 --- /dev/null +++ b/spec/rubocop/cop/rspec/factory_bot/association_style_spec.rb @@ -0,0 +1,310 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::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 + + context 'when `association` is called with traits option' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :author, traits: %i[admin] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + author factory: %i[author admin] + end + RUBY + end + end + + context 'when `association` is called with factory and traits options' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :author, factory: :user, traits: [:admin] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + author factory: %i[user admin] + end + RUBY + end + end + + context 'when `association` is called with trait arguments and factory' \ + 'and traits options' do + it 'registers and corrects an offense' do + expect_offense(<<~RUBY) + factory :article do + association :author, :active, factory: :user, traits: [:admin] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use implicit style to define associations. + end + RUBY + + expect_correction(<<~RUBY) + factory :article do + author factory: %i[user active 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 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