From 1f9678aa96c51476faea7c7e6cbb4fbfc5160da4 Mon Sep 17 00:00:00 2001 From: Nick Hengeveld Date: Tue, 14 Jan 2025 02:06:28 +0000 Subject: [PATCH] Add cop for unscope AR class methods --- ...scoped_active_record_class_method_calls.rb | 68 +++++++++++++++++++ ...scoped_active_record_class_method_calls.rb | 48 +++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 lib/rubocop/cop/github/avoid_unscoped_active_record_class_method_calls.rb create mode 100644 test/test_avoid_unscoped_active_record_class_method_calls.rb diff --git a/lib/rubocop/cop/github/avoid_unscoped_active_record_class_method_calls.rb b/lib/rubocop/cop/github/avoid_unscoped_active_record_class_method_calls.rb new file mode 100644 index 00000000..8897d648 --- /dev/null +++ b/lib/rubocop/cop/github/avoid_unscoped_active_record_class_method_calls.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require "rubocop" + +module RuboCop + module Cop + module GitHub + # Public: A Rubocop to discourage unscoped calls to potentially dangerous ActiveRecord class methods. + # + # Examples: + # + # # bad + # class Widget < ActiveRecord::Base + # def self.do_class_business + # widget_count = ids.size + # end + # end + # + # # good + # class Widget < ActiveRecord::Base + # def self.do_class_business(user) + # user_widget_count = where(user: user).ids.size + # end + # end + class AvoidUnscopedActiveRecordClassMethodCalls < Base + MESSAGE_TEMPLATE = "Avoid using ActiveModel.%s without a scope." + DANGEROUS_METHODS = %i(ids).freeze + + def_node_matcher :active_record?, <<~PATTERN + { + (const {nil? cbase} :ApplicationRecord) + (const (const {nil? cbase} :ActiveRecord) :Base) + } + PATTERN + + def on_send(node) + return unless dangerous_method?(node) + return unless nil_receiver?(node) + return unless used_in_class_method?(node) + return unless class_is_active_record?(node) + + add_offense(node, message: MESSAGE_TEMPLATE % node.method_name) + end + + private + + def dangerous_method?(node) + DANGEROUS_METHODS.include?(node.method_name) + end + + def nil_receiver?(node) + node.receiver.nil? + end + + def used_in_class_method?(node) + return false if node.nil? + return true if node.defs_type? + return false if node.def_type? + used_in_class_method?(node.parent) + end + + def class_is_active_record?(node) + node.each_ancestor(:class).any? { |class_node| active_record?(class_node.parent_class) } + end + end + end + end +end diff --git a/test/test_avoid_unscoped_active_record_class_method_calls.rb b/test/test_avoid_unscoped_active_record_class_method_calls.rb new file mode 100644 index 00000000..f7c628e8 --- /dev/null +++ b/test/test_avoid_unscoped_active_record_class_method_calls.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require_relative "cop_test" +require "minitest/autorun" +require "rubocop/cop/github/avoid_unscoped_active_record_class_method_calls" + +class TestAvoidUnscopedActiveRecordClassMethodCalls < CopTest + def cop_class + RuboCop::Cop::GitHub::AvoidUnscopedActiveRecordClassMethodCalls + end + + def test_offended_by_unscoped_call_from_active_record_class_method + offenses = investigate cop, <<-RUBY + class Widget < ApplicationRecord + def self.do_class_business + count = ids.size + end + end + RUBY + + assert_equal 1, offenses.size + assert_equal "#{cop.name}: Avoid using ActiveModel.ids without a scope.", offenses.first.message + end + + def test_unoffended_by_unscoped_call_from_non_active_record_class_method + offenses = investigate cop, <<-RUBY + class Widget < OtherClass + def self.do_class_business + count = ids.size + end + end + RUBY + + assert_equal 0, offenses.size + end + + def test_unoffended_by_unscoped_call_from_active_record_instance_method + offenses = investigate cop, <<-RUBY + class Widget < ApplicationRecord::Thing + def do_instance_business + count = ids.size + end + end + RUBY + + assert_equal 0, offenses.size + end +end