Check for one line methods, simplify assessment EN14960 bits

Author: stefan-burke

.rubocop.yml

@@ -4,6 +4,7 @@ inherit_gem: { rubocop-rails-omakase: rubocop.yml }
 # Load custom cops
 require:
   - ./lib/rubocop/cop/custom/ternary_line_breaks
+  - ./lib/rubocop/cop/custom/one_line_methods
 
 # Load plugins
 plugins:
@@ -13,6 +14,9 @@ plugins:
 Custom/TernaryLineBreaks:
   Enabled: true
 
+Custom/OneLineMethods:
+  Enabled: true
+
 # Enable Sorbet cops
 Sorbet/BlockMethodDefinition:
   Enabled: true

app/models/assessments/anchorage_assessment.rb

@@ -58,9 +58,7 @@ def meets_anchor_requirements?
   end
 
   sig { returns(Integer) }
-  def total_anchors
-    (num_low_anchors || 0) + (num_high_anchors || 0)
-  end
+  def total_anchors = (num_low_anchors || 0) + (num_high_anchors || 0)
 
   sig { returns(Integer) }
   def required_anchors

app/models/assessments/user_height_assessment.rb

@@ -50,24 +50,31 @@ class UserHeightAssessment < ApplicationRecord
 
     sig { returns(T::Hash[Symbol, T.untyped]) }
     def validate_play_area
-      return {valid: false, errors: ["Inspection not found"]} unless inspection
-
       unit_length = inspection.length
       unit_width = inspection.width
 
-      # Check if we have all required measurements
-      if [unit_length, unit_width, play_area_length, play_area_width].any?(&:nil?)
-        return {
-          valid: false,
-          errors: ["Missing required measurements for play area validation"],
-          measurements: {}
-        }
-      end
+      measurements = [
+        unit_length,
+        unit_width,
+        play_area_length,
+        play_area_width
+      ]
+      return missing_measurements_result if measurements.any?(&:nil?)
+
+      call_en14960_validator(unit_length, unit_width)
+    end
 
-      # Use the negative_adjustment value, defaulting to 0 if nil
+    private
+
+    sig {
+      params(
+        unit_length: T.nilable(BigDecimal),
+        unit_width: T.nilable(BigDecimal)
+      ).returns(T::Hash[Symbol, T.untyped])
+    }
+    def call_en14960_validator(unit_length, unit_width)
       adjustment = negative_adjustment || 0
 
-      # Call the EN14960 validator - convert BigDecimal to Float
       EN14960.validate_play_area(
         unit_length: unit_length.to_f,
         unit_width: unit_width.to_f,
@@ -77,9 +84,13 @@ def validate_play_area
       )
     end
 
-    sig { returns(T::Boolean) }
-    def play_area_valid?
-      validate_play_area[:valid]
+    sig { returns(T::Hash[Symbol, T.untyped]) }
+    def missing_measurements_result
+      {
+        valid: false,
+        errors: [I18n.t("assessments.user_height.errors.missing_measurements")],
+        measurements: {}
+      }
     end
   end
 end

lib/rubocop/cop/custom/one_line_methods.rb

@@ -0,0 +1,70 @@
+# typed: false
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Custom
+      # Detects one-line methods that are simple aliases
+      # (passing same arguments through)
+      # These should call the original method directly instead
+      #
+      # @example
+      #   # bad - unnecessary wrapper method
+      #   def user_name(id)
+      #     fetch_user_name(id)
+      #   end
+      #
+      #   # good - just call fetch_user_name directly
+      #
+      #   # good - performs calculation
+      #   def total_anchors
+      #     (num_low_anchors || 0) + (num_high_anchors || 0)
+      #   end
+      #
+      #   # good - calls with different arguments
+      #   def full_name
+      #     format_name(first_name, last_name)
+      #   end
+      #
+      class OneLineMethods < Base
+        MSG = "Call the original method directly instead of creating " \
+              "an aliasing wrapper method"
+
+        def on_def(node)
+          return unless aliasing_method?(node)
+
+          add_offense(node)
+        end
+
+        def on_defs(node)
+          return unless aliasing_method?(node)
+
+          add_offense(node)
+        end
+
+        private
+
+        def aliasing_method?(node)
+          body = node.body
+          return false unless body
+          return false unless body.send_type?
+          return false if body.receiver
+
+          method_args = node.arguments
+          call_args = body.arguments
+
+          return false unless method_args.size == call_args.size
+          return false if method_args.empty?
+
+          arguments_match?(method_args, call_args)
+        end
+
+        def arguments_match?(method_args, call_args)
+          method_args.zip(call_args).all? do |method_arg, call_arg|
+            call_arg.lvar_type? && call_arg.children.first == method_arg.name
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/rubocop/cop/custom/one_line_methods_spec.rb

@@ -0,0 +1,163 @@
+# typed: false
+# frozen_string_literal: true
+
+require "rails_helper"
+require "rubocop"
+require "rubocop/rspec/support"
+
+# Load the custom cop using Rails.root
+require Rails.root.join("lib/rubocop/cop/custom/one_line_methods")
+
+RSpec.describe RuboCop::Cop::Custom::OneLineMethods, type: :rubocop do
+  include RuboCop::RSpec::ExpectOffense
+
+  let(:config) { RuboCop::Config.new }
+  let(:cop) { described_class.new(config) }
+
+  context "when method is a simple alias with same arguments" do
+    it "registers an offense for single argument pass-through" do
+      message = "Custom/OneLineMethods: Call the original method directly " \
+                "instead of creating an aliasing wrapper method"
+      expect_offense(<<~RUBY)
+        def fetch_user(id)
+        ^^^^^^^^^^^^^^^^^^ #{message}
+          get_user(id)
+        end
+      RUBY
+    end
+
+    it "registers an offense for multiple arguments pass-through" do
+      message = "Custom/OneLineMethods: Call the original method directly " \
+                "instead of creating an aliasing wrapper method"
+      expect_offense(<<~RUBY)
+        def find_record(type, id)
+        ^^^^^^^^^^^^^^^^^^^^^^^^^ #{message}
+          lookup_record(type, id)
+        end
+      RUBY
+    end
+
+    it "registers an offense for class methods" do
+      message = "Custom/OneLineMethods: Call the original method directly " \
+                "instead of creating an aliasing wrapper method"
+      expect_offense(<<~RUBY)
+        def self.fetch_user(id)
+        ^^^^^^^^^^^^^^^^^^^^^^^ #{message}
+          get_user(id)
+        end
+      RUBY
+    end
+  end
+
+  context "when method is not a simple alias" do
+    it "does not register offense for methods with different arguments" do
+      expect_no_offenses(<<~RUBY)
+        def full_name
+          format_name(first_name, last_name)
+        end
+      RUBY
+    end
+
+    it "does not register offense for methods without arguments" do
+      expect_no_offenses(<<~RUBY)
+        def complete?
+          incomplete_fields.empty?
+        end
+      RUBY
+    end
+
+    it "does not register offense for methods calling on a receiver" do
+      expect_no_offenses(<<~RUBY)
+        def user_name
+          user.name
+        end
+      RUBY
+    end
+
+    it "does not register offense for comparison operations" do
+      expect_no_offenses(<<~RUBY)
+        def triggered_by?(check_user)
+          user == check_user
+        end
+      RUBY
+    end
+
+    it "does not register offense for calculations" do
+      expect_no_offenses(<<~RUBY)
+        def total_anchors
+          (num_low_anchors || 0) + (num_high_anchors || 0)
+        end
+      RUBY
+    end
+
+    it "does not register offense for transformations" do
+      expect_no_offenses(<<~RUBY)
+        def column_name_syms
+          column_names.map(&:to_sym).sort
+        end
+      RUBY
+    end
+
+    it "does not register offense for multi-line methods" do
+      expect_no_offenses(<<~RUBY)
+        def some_method(arg)
+          result = process(arg)
+          result
+        end
+      RUBY
+    end
+
+    # Endless methods are handled differently and won't be parsed as regular
+    # methods so they naturally won't trigger this cop
+  end
+
+  context "edge cases" do
+    it "does not register offense for methods with blocks" do
+      expect_no_offenses(<<~RUBY)
+        def process_items(items)
+          handle_items(items) { |item| item.process }
+        end
+      RUBY
+    end
+
+    it "does not register offense for methods with keyword arguments" do
+      expect_no_offenses(<<~RUBY)
+        def create_user(name:, email:)
+          build_user(name: name, email: email)
+        end
+      RUBY
+    end
+
+    it "does not register offense for methods with splat arguments" do
+      expect_no_offenses(<<~RUBY)
+        def forward_args(*args)
+          process(*args)
+        end
+      RUBY
+    end
+
+    it "does not register offense when arguments are reordered" do
+      expect_no_offenses(<<~RUBY)
+        def swap_args(a, b)
+          process(b, a)
+        end
+      RUBY
+    end
+
+    it "does not register offense when fewer arguments are passed" do
+      expect_no_offenses(<<~RUBY)
+        def simplified_call(a, b)
+          complex_call(a)
+        end
+      RUBY
+    end
+
+    it "does not register offense for delegations without arguments" do
+      expect_no_offenses(<<~RUBY)
+        def current_user
+          fetch_current_user
+        end
+      RUBY
+    end
+  end
+end