Skip to content

Commit

Permalink
Fix mail_later Rubocop cop when method called on variable reference (#…
Browse files Browse the repository at this point in the history
…11642)

* Add example code demonstrating lint failure

* Dump stderr on Rubocop error result

* Add failing test case

changelog: Internal, Static Analysis, Fix custom linter for configurable mail delivery

* Verify send type before checking method name

Fail if can't check type

* Disable linter for intentional spec example
  • Loading branch information
aduth authored Dec 18, 2024
1 parent 6b5f274 commit 3881e95
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 4 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ lint: ## Runs all lint tests
@echo "--- rubocop ---"
mkdir -p tmp
ifdef JUNIT_OUTPUT
bundle exec rubocop --parallel --format progress --format junit --out rubocop.xml --display-only-failed --color 2> tmp/rubocop.txt
bundle exec rubocop --parallel --format progress --format junit --out rubocop.xml --display-only-failed --color 2> tmp/rubocop.txt || (cat tmp/rubocop.txt; exit 1)
else
bundle exec rubocop --parallel --color 2> tmp/rubocop.txt
bundle exec rubocop --parallel --color 2> tmp/rubocop.txt || (cat tmp/rubocop.txt; exit 1)
endif
awk 'NF {exit 1}' tmp/rubocop.txt || (printf "Error: Unexpected stderr output from Rubocop\n"; cat tmp/rubocop.txt; exit 1)
@echo "--- analytics_events ---"
Expand Down
4 changes: 2 additions & 2 deletions lib/linters/mail_later_linter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ def on_send(node)
mailer_name = if receiver.const_type?
# MailerClass.email.send_later
receiver.const_name
elsif receiver.method_name == :with
elsif receiver.send_type? && receiver.method_name == :with
# MailerClass.with(...).email.send_later
receiver.receiver.const_name
end

add_offense(node) if mailer_name == 'UserMailer'
add_offense(node) if mailer_name.nil? || mailer_name == 'UserMailer'
end
end
end
Expand Down
8 changes: 8 additions & 0 deletions spec/lib/linters/mail_later_linter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@
RUBY
end

it 'registers offense when calling .with(...).deliver_now method with variable mailer' do
expect_offense(<<~RUBY)
mailer = UserMailer.with(user)
mailer.send_email(params).deliver_later
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ IdentityIdp/MailLaterLinter: Please send mail using deliver_now_or_later instead
RUBY
end

it 'does not register offenses for the ReportMailer' do
expect_no_offenses(<<~RUBY)
ReportMailer.send_email(foobar).deliver_now
Expand Down
7 changes: 7 additions & 0 deletions spec/mailers/user_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1465,6 +1465,13 @@ def expect_email_body_to_have_help_and_contact_links
end

describe '#deliver_later' do
it 'queues email without raising' do
# rubocop:disable IdentityIdp/MailLaterLinter
mailer = UserMailer.with(user:, email_address: user.email_addresses.first)
mailer.suspended_create_account.deliver_later
# rubocop:enable IdentityIdp/MailLaterLinter
end

it 'does not queue email if it potentially contains sensitive value' do
user = create(:user)
mailer = UserMailer.with(
Expand Down

0 comments on commit 3881e95

Please sign in to comment.