diff --git a/.rubocop.yml b/.rubocop.yml index 9b2c450d8..954319c6a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -4,6 +4,9 @@ inherit_mode: merge: - Exclude +require: + - ./lib/rubocop/cop/style/prefer_data_define + # Find out more about the rubocop cops https://github.com/bbatsov/rubocop/blob/master/config/enabled.yml AllCops: diff --git a/config/application.rb b/config/application.rb index 83b7e22bc..0fa173876 100644 --- a/config/application.rb +++ b/config/application.rb @@ -19,7 +19,7 @@ class Application < Rails::Application # Please, add to the `ignore` list any other `lib` subdirectories that do # not contain `.rb` files, or that should not be reloaded or eager loaded. # Common ones are `templates`, `generators`, or `middleware`, for example. - config.autoload_lib(ignore: %w[assets tasks omniauth]) + config.autoload_lib(ignore: %w[assets tasks omniauth rubocop]) # Configuration for the application, engines, and railties goes here. # diff --git a/lib/rubocop/cop/style/prefer_data_define.rb b/lib/rubocop/cop/style/prefer_data_define.rb new file mode 100644 index 000000000..e282ba33a --- /dev/null +++ b/lib/rubocop/cop/style/prefer_data_define.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # Prefer Data.define over Struct.new for immutable value objects. + # + # Data defines immutable value objects with keyword-aware equality; + # Struct instances are mutable, so the rewrite is not safely mechanical. + class PreferDataDefine < Base + MSG = 'Prefer `Data.define` over `Struct.new` for immutable value objects.' + + def on_send(node) + return unless node.receiver&.const_type? && node.receiver.const_name == 'Struct' + return unless node.method?(:new) + + add_offense(node) + end + end + end + end +end diff --git a/spec/helpers/email_header_helper_spec.rb b/spec/helpers/email_header_helper_spec.rb index 829ec8b76..1359a4499 100644 --- a/spec/helpers/email_header_helper_spec.rb +++ b/spec/helpers/email_header_helper_spec.rb @@ -6,7 +6,7 @@ end describe '#mail_to_member' do - let(:member) { Struct.new(:id, :email).new(1, 'test@example.com') } + let(:member) { instance_double(Member, id: 1, email: 'test@example.com') } it 'calls mail with correct arguments for valid email' do allow(helper).to receive(:mail).with( @@ -38,37 +38,37 @@ end it 'returns SkippedEmail for nil email' do - member = Struct.new(:id, :email).new(1, nil) + member = instance_double(Member, id: 1, email: nil) result = helper.mail_to_member(member, 'Test Subject') expect(result).to be_a(EmailHeaderHelper::SkippedEmail) end it 'returns SkippedEmail for blank email' do - member = Struct.new(:id, :email).new(1, '') + member = instance_double(Member, id: 1, email: '') result = helper.mail_to_member(member, 'Test Subject') expect(result).to be_a(EmailHeaderHelper::SkippedEmail) end it 'returns SkippedEmail for invalid email format' do - member = Struct.new(:id, :email).new(1, 'invalid-email') + member = instance_double(Member, id: 1, email: 'invalid-email') result = helper.mail_to_member(member, 'Test Subject') expect(result).to be_a(EmailHeaderHelper::SkippedEmail) end it 'returns SkippedEmail for email missing @ symbol' do - member = Struct.new(:id, :email).new(1, 'invalidexample.com') + member = instance_double(Member, id: 1, email: 'invalidexample.com') result = helper.mail_to_member(member, 'Test Subject') expect(result).to be_a(EmailHeaderHelper::SkippedEmail) end it 'returns SkippedEmail for email missing TLD' do - member = Struct.new(:id, :email).new(1, 'invalid@example') + member = instance_double(Member, id: 1, email: 'invalid@example') result = helper.mail_to_member(member, 'Test Subject') expect(result).to be_a(EmailHeaderHelper::SkippedEmail) end it 'logs the skip' do - member = Struct.new(:id, :email).new(1, 'bad-email') + member = instance_double(Member, id: 1, email: 'bad-email') allow(Rails.logger).to receive(:info).with(/Skipped email to member 1/) helper.mail_to_member(member, 'Test Subject') diff --git a/spec/rubocop/cop/style/prefer_data_define_spec.rb b/spec/rubocop/cop/style/prefer_data_define_spec.rb new file mode 100644 index 000000000..849ee7383 --- /dev/null +++ b/spec/rubocop/cop/style/prefer_data_define_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'rubocop' +require 'rubocop/rspec/expect_offense' +require 'rubocop/rspec/support' +require_relative '../../../../lib/rubocop/cop/style/prefer_data_define' + +RSpec.describe RuboCop::Cop::Style::PreferDataDefine, :config, :ruby40 do + include RuboCop::RSpec::ExpectOffense + + it 'registers an offense for Struct.new' do + expect_offense(<<~RUBY) + Struct.new(:id, :email) + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Data.define` over `Struct.new` for immutable value objects. + RUBY + end + + it 'registers an offense for ::Struct.new' do + expect_offense(<<~RUBY) + ::Struct.new(:id, :email) + ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `Data.define` over `Struct.new` for immutable value objects. + RUBY + end + + it 'does not register an offense for Data.define' do + expect_no_offenses(<<~RUBY) + Data.define(:id, :email) + RUBY + end + + it 'does not register an offense for namespaced receivers' do + expect_no_offenses(<<~RUBY) + Other::Struct.new(:id, :email) + RUBY + end + + it 'does not register an offense for other Struct methods' do + expect_no_offenses(<<~RUBY) + Struct.members + RUBY + end + + it 'does not register an offense for OtherClass.new' do + expect_no_offenses(<<~RUBY) + Member.new(:id, :email) + RUBY + end +end