Simplify scope declarations (#2434)

david-a-wheeler · web-flow · commit 5383d41c8492 · 2025-08-31T09:50:04.000-04:00
* Simplify scope declarations

More recent versions of Rails, combined with more recent versions
of Ruby, enable a simpler syntax for defining scopes, replacing
5 lines of code with 1 in each case. We have several scopes to define,
so let's switch to the simpler syntax.

We also add a number of unit tests to make *sure* nothing changed.

Signed-off-by: David A. Wheeler &lt;dwheeler@dwheeler.com&gt;

* Update CLAUDE.md

Rubocop's -A option (unsafe autocorrect) is much trouble than it's
worth. It's better to let the AI fix it than to run the unsafe
autocorrect.

* Remove bad cop in .rubocop.yml, fix projects controller

Signed-off-by: David A. Wheeler &lt;dwheeler@dwheeler.com&gt;
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -33,7 +33,7 @@ plugins:
 
 AllCops:
   EnabledByDefault: true
-  TargetRubyVersion: 2.5
+  TargetRubyVersion: 3.3
   DisplayCopNames: true
   DisplayStyleGuide: true
   Include:
@@ -265,3 +265,14 @@ Style/YodaExpression:
   Enabled: false
 Metrics/BlockLength:
   Max: 36
+Lint/RedundantRequireStatement:
+  Enabled: true  # Let RuboCop remove truly redundant requires
+Rails/WhereRange:
+  Enabled: true  # Modern Rails range syntax is cleaner and equivalent
+
+# The cop Performance/ArraySemiInfiniteRangeSlice
+# was created due to a mistake in a microbenchmark. Worse, it can raise
+# invalid complaints on strings. We've expressly disabled it, with this
+# comment, to avoid the risk of later re-enabling it.
+Performance/ArraySemiInfiniteRangeSlice:
+  Enabled: false # Do NOT enable this
diff --git a/CLAUDE.md b/CLAUDE.md
@@ -39,8 +39,23 @@ For specific files:
 - `mdl FILENAME.md` - Markdownlint only the file `FILENAME.md`
 - `rubocop FILENAME.rb` - Run rubocop on just `FILENAME.rb`
 
-Don't run `rails_best_practices` to analyze individual files, as
-it needs the full context to do its best.
+Don't run `rails_best_practices` to analyze individual files.
+The `rails_best_practices` program needs the full context of all files
+to do its best.
+
+**IMPORTANT**: You may ONLY use `rubocop -a` (safe autocorrect)
+for RuboCop corrections.
+
+**NEVER use `rubocop -A` (unsafe autocorrect)** -
+it frequently introduces subtle bugs by making assumptions
+about code context that are incorrect. Examples include:
+
+- Changing array methods to string methods (`.drop()` vs `[1..]`)
+- Changing hash syntax that breaks in complex ActiveRecord queries
+
+Always use `-a` (safe) and manually review any remaining suggestions from
+`rubocop` without flags to determine if they're actually appropriate
+for the specific context.
 
 ### Workflow
 
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
@@ -101,7 +101,7 @@ def append_info_to_payload(payload)
 
   # Combined cache control header value for CDN surrogate control
   BADGE_CACHE_SURROGATE_CONTROL =
-    "max-age=#{BADGE_CACHE_MAX_AGE}, stale-if-error=#{BADGE_CACHE_STALE_AGE}"
+    "max-age=#{BADGE_CACHE_MAX_AGE}, stale-if-error=#{BADGE_CACHE_STALE_AGE}".freeze
 
   # Set default cache control - don't externally cache.
   # This is the safe behavior, so we make it the default.
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
@@ -53,7 +53,7 @@ class ProjectsController < ApplicationController
   ).freeze
 
   # Used to validate deletion rationale.
-  AT_LEAST_15_NON_WHITESPACE = /\A\s*(\S\s*){15}.*/.freeze
+  AT_LEAST_15_NON_WHITESPACE = /\A\s*(\S\s*){15}.*/
 
   # as= values, which redirect to alternative views
   ALLOWED_AS = %w[badge entry].freeze
@@ -428,7 +428,7 @@ def reminders_summary
 
   # Regex pattern for validating repository rights changes.
   # Rights changes, if provided, must match this pattern.
-  VALID_ADD_RIGHTS_CHANGES = /\A *[+-] *\d+ *(, *\d+)*\z/.freeze
+  VALID_ADD_RIGHTS_CHANGES = /\A *[+-] *\d+ *(, *\d+)*\z/
 
   # Number of days before a user may change repo_url.
   # NOTE: If you change this value, you may also need to change
@@ -589,8 +589,8 @@ def require_adequate_deletion_rationale
   # @return [void]
   # rubocop:disable Metrics/MethodLength
   def update_additional_rights_forced(id, new_additional_rights)
-    command = new_additional_rights.first
-    new_list = new_additional_rights[1..-1].split(',').map(&:to_i).uniq.sort
+    command = new_additional_rights[0] # rubocop:disable Style/ArrayFirstLast
+    new_list = new_additional_rights[1..].split(',').map(&:to_i).sort.uniq
     if command == '-'
       AdditionalRight.where(project_id: id, user_id: new_list).destroy_all
     else # '+'
@@ -769,7 +769,7 @@ def repo_data(github = nil)
 
     repos.map do |repo|
       [repo.full_name, repo.fork, repo.homepage, repo.html_url]
-    end.compact
+    end
   end
   # rubocop:enable Metrics/AbcSize
   # rubocop:enable Style/MethodCalledOnDoEndBlock, Metrics/MethodLength
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
@@ -165,7 +165,7 @@ def tiered_percent_as_string(value)
   # rubocop:enable Metrics/MethodLength
 
   # We sometimes insert <wbr> after sequences of these characters.
-  WORD_BREAK_DIVIDERS = /([,_\-.]+)/.freeze
+  WORD_BREAK_DIVIDERS = /([,_\-.]+)/
 
   # This text is considered safe, so we can directly mark it as such.
   # rubocop:disable Rails/OutputSafety
diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb
@@ -10,7 +10,7 @@ module SessionsHelper
   RESET_SESSION_TIMER = 1.hour # Active sessions older than this reset timer
   GITHUB_PATTERN = %r{
     \Ahttps://github.com/([A-Za-z0-9_.-]+)/([A-Za-z0-9_.-]+)/?\Z
-  }x.freeze
+  }x
   require 'uri'
 
   # Remove the "locale=value", if any, from the url_query provided
diff --git a/app/lib/chief.rb b/app/lib/chief.rb
@@ -13,8 +13,6 @@
 # the Detective INPUTS and OUTPUTS to determine what order to run, and
 # run them in parallel in an appropriate order.
 
-require 'set'
-
 # rubocop:disable Metrics/ClassLength
 class Chief
   # Confidence level (1..5) where automation result will *override*
diff --git a/app/lib/how_access_repo_files_detective.rb b/app/lib/how_access_repo_files_detective.rb
@@ -13,7 +13,7 @@ class HowAccessRepoFilesDetective < Detective
   INPUTS = [:repo_url].freeze
   OUTPUTS = [:repo_files].freeze # Ask :repo_files.get("FILENAME") for files.
 
-  GITHUB_REPO = %r{https?://github.com/([\w\.-]*)/([\w\.-]*)(.git|/)?}.freeze
+  GITHUB_REPO = %r{https?://github.com/([\w\.-]*)/([\w\.-]*)(.git|/)?}
   def analyze(_evidence, current)
     repo_url = current[:repo_url]
     return {} if repo_url.blank?
diff --git a/app/models/badge.rb b/app/models/badge.rb
@@ -4,8 +4,6 @@
 # OpenSSF Best Practices badge contributors
 # SPDX-License-Identifier: MIT
 
-require 'set'
-
 # rubocop: disable Metrics/ClassLength
 class Badge
   ACCEPTABLE_PERCENTAGES = (0..99).to_a.freeze
diff --git a/app/models/project.rb b/app/models/project.rb
@@ -48,7 +48,7 @@ class Project < ApplicationRecord
 
   # All badge levels as IDs. Useful for enumerating "all levels" as:
   # Project::LEVEL_IDS.each do |level| ... end
-  LEVEL_ID_NUMBERS = (0..(COMPLETED_BADGE_LEVELS.length - 1)).freeze
+  LEVEL_ID_NUMBERS = (0..(COMPLETED_BADGE_LEVELS.length - 1))
   LEVEL_IDS = LEVEL_ID_NUMBERS.map(&:to_s)
 
   PROJECT_OTHER_FIELDS = %i[
@@ -65,25 +65,13 @@ class Project < ApplicationRecord
 
   default_scope { order(:created_at) }
 
-  scope :created_since, (
-    lambda do |time|
-      where(Project.arel_table[:created_at].gteq(time))
-    end
-  )
+  scope :created_since, ->(time) { where(created_at: time..) }
 
-  scope :gteq, (
-    lambda do |floor|
-      where(Project.arel_table[:tiered_percentage].gteq(floor.to_i))
-    end
-  )
+  scope :gteq, ->(floor) { where(tiered_percentage: floor.to_i..) }
 
   scope :in_progress, -> { lteq(99) }
 
-  scope :lteq, (
-    lambda do |ceiling|
-      where(Project.arel_table[:tiered_percentage].lteq(ceiling.to_i))
-    end
-  )
+  scope :lteq, ->(ceiling) { where(tiered_percentage: ..ceiling.to_i) }
 
   scope :passing, -> { gteq(100) }
 
@@ -145,11 +133,7 @@ class Project < ApplicationRecord
     # using: { tsearch: { any_word: true } }
   )
 
-  scope :updated_since, (
-    lambda do |time|
-      where(Project.arel_table[:updated_at].gteq(time))
-    end
-  )
+  scope :updated_since, ->(time) { where(updated_at: time..) }
 
   # Record information about a project.
   # We'll also record previous versions of information:
@@ -189,7 +173,7 @@ class Project < ApplicationRecord
   # We have to allow embedded spaces, e.g., "Jupyter Notebook".
   VALID_LANGUAGE_LIST =
     %r{\A(|-| ([A-Za-z0-9!\#$%'()*+.\/\:;=?@\[\]^~ -]+
-        (,\ ?[A-Za-z0-9!\#$%'()*+.\/\:;=?@\[\]^~ -]+)*))\Z}x.freeze
+        (,\ ?[A-Za-z0-9!\#$%'()*+.\/\:;=?@\[\]^~ -]+)*))\Z}x
   validates :implementation_languages,
             length: { maximum: MAX_SHORT_STRING_LENGTH },
             format: {
@@ -552,8 +536,7 @@ def self.projects_to_remind
       .where('badge_percentage_0 < 100')
       .where('lost_passing_at IS NULL OR lost_passing_at < ?',
              LOST_PASSING_REMINDER.days.ago)
-      .where('projects.updated_at < ?',
-             LAST_UPDATED_REMINDER.days.ago)
+      .where(projects: { updated_at: ...LAST_UPDATED_REMINDER.days.ago })
       .where('last_reminder_at IS NULL OR last_reminder_at < ?',
              LAST_SENT_REMINDER.days.ago)
       .joins(:user).references(:user) # Need this to check email address
@@ -592,7 +575,7 @@ def self.recently_reminded
       .select('projects.*, users.encrypted_email as user_encrypted_email')
       .joins(:user).references(:user) # Need this to check email address
       .where('last_reminder_at IS NOT NULL')
-      .where('last_reminder_at >= ?', 14.days.ago)
+      .where(last_reminder_at: 14.days.ago..)
       .reorder('last_reminder_at')
   end
 
diff --git a/app/models/project_stat.rb b/app/models/project_stat.rb
@@ -44,15 +44,9 @@ def stamp
         next if completion.to_i.zero? # Don't record percentage "0" > level 0
 
         public_send :"percent_1_ge_#{completion}=",
-                    Project.where(
-                      'badge_percentage_1 >= ?',
-                      completion.to_i
-                    ).count
+                    Project.where(badge_percentage_1: completion.to_i..).count
         public_send :"percent_2_ge_#{completion}=",
-                    Project.where(
-                      'badge_percentage_2 >= ?',
-                      completion.to_i
-                    ).count
+                    Project.where(badge_percentage_2: completion.to_i..).count
       end
       self.projects_edited = Project.where('created_at < updated_at').count
 
@@ -142,8 +136,8 @@ def stamp
   # Note that created_at is an index, so this should be extremely fast.
   def self.last_in_month(query_date)
     ProjectStat
-      .where('created_at >= ?', query_date.beginning_of_month)
-      .where('created_at <= ?', query_date.end_of_month)
+      .where(created_at: query_date.beginning_of_month..)
+      .where(created_at: ..query_date.end_of_month)
       .reorder(:created_at).last
   end
 
diff --git a/app/validators/text_validator.rb b/app/validators/text_validator.rb
@@ -5,7 +5,7 @@
 # SPDX-License-Identifier: MIT
 
 class TextValidator < ActiveModel::EachValidator
-  INVALID_CONTROL = /[\x01-\x08\x0b\x0c\x0e-\x1f]/.freeze
+  INVALID_CONTROL = /[\x01-\x08\x0b\x0c\x0e-\x1f]/
   def text_acceptable?(value)
     return true if value.nil?
     return false unless value.valid_encoding?
diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb
@@ -30,7 +30,7 @@ class UrlValidator < ActiveModel::EachValidator
         (/
           ([-A-Za-z0-9_.:/+!,#@~]|    # allow these ASCII chars.
            %(20|[89A-Ea-e][0-9A-Fa-f]|[Ff][0-7]))*  # Allow some %-encoded
-        )?)\z}x.freeze
+        )?)\z}x
 
   # Unescape but do *not* force an encoding (so we can force it separately
   # and check for validity).
diff --git a/config/routes.rb b/config/routes.rb
@@ -11,16 +11,16 @@
 # See how all your routes lay out with "rake routes".
 
 # This regex defines all legal locale values:
-LEGAL_LOCALE ||= /(?:#{I18n.available_locales.join('|')})/.freeze
+LEGAL_LOCALE ||= /(?:#{I18n.available_locales.join('|')})/
 
 # This regex is used to verify criteria levels in routes:
-VALID_CRITERIA_LEVEL ||= /[0-2]/.freeze
+VALID_CRITERIA_LEVEL ||= /[0-2]/
 
 # Confirm that number-only id is provided
-VALID_ID ||= /[1-9][0-9]*/.freeze
+VALID_ID ||= /[1-9][0-9]*/
 
 # Valid values for static badge display
-VALID_STATIC_VALUE ||= /0|[1-9]{1,2}|passing|silver|gold/.freeze
+VALID_STATIC_VALUE ||= /0|[1-9]{1,2}|passing|silver|gold/
 
 Rails.application.routes.draw do
   # First, handle routing of special cases.
diff --git a/lib/tasks/default.rake b/lib/tasks/default.rake
@@ -684,7 +684,6 @@ def link_okay?(link)
   fetchable?(link)
 end
 
-require 'set'
 def validate_links_in_string(translation, from, seen)
   translation.scan(/href=["'][^"']+["']/).each do |snippet|
     link = snippet[6..-2]
diff --git a/test/models/criteria_test.rb b/test/models/criteria_test.rb
@@ -5,7 +5,6 @@
 # SPDX-License-Identifier: MIT
 
 require 'test_helper'
-require 'set'
 
 # rubocop:disable Metrics/ClassLength
 class CriteriaTest < ActiveSupport::TestCase
diff --git a/test/models/project_test.rb b/test/models/project_test.rb
diff --git a/test/system/login_test.rb b/test/system/login_test.rb
