Fix Lint/DuplicateBranch cop (mastodon#24766)

hachyderm · May 2, 2023 · 88d33f3 · 88d33f3
1 parent f501057
commit 88d33f3
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 31 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -119,15 +119,6 @@ Lint/ConstantDefinitionInBlock:
     - 'spec/lib/connection_pool/shared_timed_stack_spec.rb'
     - 'spec/models/concerns/remotable_spec.rb'
 
-# Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches.
-Lint/DuplicateBranch:
-  Exclude:
-    - 'app/lib/permalink_redirector.rb'
-    - 'app/models/account_statuses_filter.rb'
-    - 'app/validators/email_mx_validator.rb'
-    - 'app/validators/vote_validator.rb'
-    - 'lib/mastodon/maintenance_cli.rb'
-
 # Configuration parameters: AllowComments, AllowEmptyLambdas.
 Lint/EmptyBlock:
   Exclude:

diff --git a/app/lib/permalink_redirector.rb b/app/lib/permalink_redirector.rb
@@ -8,19 +8,49 @@ def initialize(path)
   end
 
   def redirect_path
-    if path_segments[0].present? && path_segments[0].start_with?('@') && path_segments[1] =~ /\d/
-      find_status_url_by_id(path_segments[1])
-    elsif path_segments[0].present? && path_segments[0].start_with?('@')
-      find_account_url_by_name(path_segments[0])
-    elsif path_segments[0] == 'statuses' && path_segments[1] =~ /\d/
-      find_status_url_by_id(path_segments[1])
-    elsif path_segments[0] == 'accounts' && path_segments[1] =~ /\d/
-      find_account_url_by_id(path_segments[1])
+    if at_username_status_request? || statuses_status_request?
+      find_status_url_by_id(second_segment)
+    elsif at_username_request?
+      find_account_url_by_name(first_segment)
+    elsif accounts_request? && record_integer_id_request?
+      find_account_url_by_id(second_segment)
     end
   end
 
   private
 
+  def at_username_status_request?
+    at_username_request? && record_integer_id_request?
+  end
+
+  def statuses_status_request?
+    statuses_request? && record_integer_id_request?
+  end
+
+  def at_username_request?
+    first_segment.present? && first_segment.start_with?('@')
+  end
+
+  def statuses_request?
+    first_segment == 'statuses'
+  end
+
+  def accounts_request?
+    first_segment == 'accounts'
+  end
+
+  def record_integer_id_request?
+    second_segment =~ /\d/
+  end
+
+  def first_segment
+    path_segments.first
+  end
+
+  def second_segment
+    path_segments.second
+  end
+
   def path_segments
     @path_segments ||= @path.gsub(/\A\//, '').split('/')
   end

diff --git a/app/models/account_statuses_filter.rb b/app/models/account_statuses_filter.rb
@@ -32,9 +32,9 @@ def results
   private
 
   def initial_scope
-    if suspended?
-      Status.none
-    elsif anonymous?
+    return Status.none if suspended?
+
+    if anonymous?
       account.statuses.where(visibility: %i(public unlisted))
     elsif author?
       account.statuses.all # NOTE: #merge! does not work without the #all

diff --git a/app/validators/email_mx_validator.rb b/app/validators/email_mx_validator.rb
@@ -8,9 +8,7 @@ def validate(user)
 
     domain = get_domain(user.email)
 
-    if domain.blank?
-      user.errors.add(:email, :invalid)
-    elsif domain.include?('..')
+    if domain.blank? || domain.include?('..')
       user.errors.add(:email, :invalid)
     elsif !on_allowlist?(domain)
       resolved_ips, resolved_domains = resolve_mx(domain)

diff --git a/app/validators/vote_validator.rb b/app/validators/vote_validator.rb
@@ -6,15 +6,23 @@ def validate(vote)
 
     vote.errors.add(:base, I18n.t('polls.errors.invalid_choice')) if invalid_choice?(vote)
 
-    if vote.poll_multiple? && already_voted_for_same_choice_on_multiple_poll?(vote)
-      vote.errors.add(:base, I18n.t('polls.errors.already_voted'))
-    elsif !vote.poll_multiple? && already_voted_on_non_multiple_poll?(vote)
-      vote.errors.add(:base, I18n.t('polls.errors.already_voted'))
-    end
+    vote.errors.add(:base, I18n.t('polls.errors.already_voted')) if additional_voting_not_allowed?(vote)
   end
 
   private
 
+  def additional_voting_not_allowed?(vote)
+    poll_multiple_and_already_voted?(vote) || poll_non_multiple_and_already_voted?(vote)
+  end
+
+  def poll_multiple_and_already_voted?(vote)
+    vote.poll_multiple? && already_voted_for_same_choice_on_multiple_poll?(vote)
+  end
+
+  def poll_non_multiple_and_already_voted?(vote)
+    !vote.poll_multiple? && already_voted_on_non_multiple_poll?(vote)
+  end
+
   def invalid_choice?(vote)
     vote.choice.negative? || vote.choice >= vote.poll.options.size
   end

diff --git a/lib/mastodon/maintenance_cli.rb b/lib/mastodon/maintenance_cli.rb
@@ -664,9 +664,7 @@ def find_duplicate_accounts
 
     def remove_index_if_exists!(table, name)
       ActiveRecord::Base.connection.remove_index(table, name: name)
-    rescue ArgumentError
-      nil
-    rescue ActiveRecord::StatementInvalid
+    rescue ArgumentError, ActiveRecord::StatementInvalid
       nil
     end
   end