Skip to content

Commit 1f9678a

Browse files
committed
Add cop for unscope AR class methods
1 parent 5db0a50 commit 1f9678a

File tree

2 files changed

+116
-0
lines changed

2 files changed

+116
-0
lines changed
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# frozen_string_literal: true
2+
3+
require "rubocop"
4+
5+
module RuboCop
6+
module Cop
7+
module GitHub
8+
# Public: A Rubocop to discourage unscoped calls to potentially dangerous ActiveRecord class methods.
9+
#
10+
# Examples:
11+
#
12+
# # bad
13+
# class Widget < ActiveRecord::Base
14+
# def self.do_class_business
15+
# widget_count = ids.size
16+
# end
17+
# end
18+
#
19+
# # good
20+
# class Widget < ActiveRecord::Base
21+
# def self.do_class_business(user)
22+
# user_widget_count = where(user: user).ids.size
23+
# end
24+
# end
25+
class AvoidUnscopedActiveRecordClassMethodCalls < Base
26+
MESSAGE_TEMPLATE = "Avoid using ActiveModel.%s without a scope."
27+
DANGEROUS_METHODS = %i(ids).freeze
28+
29+
def_node_matcher :active_record?, <<~PATTERN
30+
{
31+
(const {nil? cbase} :ApplicationRecord)
32+
(const (const {nil? cbase} :ActiveRecord) :Base)
33+
}
34+
PATTERN
35+
36+
def on_send(node)
37+
return unless dangerous_method?(node)
38+
return unless nil_receiver?(node)
39+
return unless used_in_class_method?(node)
40+
return unless class_is_active_record?(node)
41+
42+
add_offense(node, message: MESSAGE_TEMPLATE % node.method_name)
43+
end
44+
45+
private
46+
47+
def dangerous_method?(node)
48+
DANGEROUS_METHODS.include?(node.method_name)
49+
end
50+
51+
def nil_receiver?(node)
52+
node.receiver.nil?
53+
end
54+
55+
def used_in_class_method?(node)
56+
return false if node.nil?
57+
return true if node.defs_type?
58+
return false if node.def_type?
59+
used_in_class_method?(node.parent)
60+
end
61+
62+
def class_is_active_record?(node)
63+
node.each_ancestor(:class).any? { |class_node| active_record?(class_node.parent_class) }
64+
end
65+
end
66+
end
67+
end
68+
end
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# frozen_string_literal: true
2+
3+
require_relative "cop_test"
4+
require "minitest/autorun"
5+
require "rubocop/cop/github/avoid_unscoped_active_record_class_method_calls"
6+
7+
class TestAvoidUnscopedActiveRecordClassMethodCalls < CopTest
8+
def cop_class
9+
RuboCop::Cop::GitHub::AvoidUnscopedActiveRecordClassMethodCalls
10+
end
11+
12+
def test_offended_by_unscoped_call_from_active_record_class_method
13+
offenses = investigate cop, <<-RUBY
14+
class Widget < ApplicationRecord
15+
def self.do_class_business
16+
count = ids.size
17+
end
18+
end
19+
RUBY
20+
21+
assert_equal 1, offenses.size
22+
assert_equal "#{cop.name}: Avoid using ActiveModel.ids without a scope.", offenses.first.message
23+
end
24+
25+
def test_unoffended_by_unscoped_call_from_non_active_record_class_method
26+
offenses = investigate cop, <<-RUBY
27+
class Widget < OtherClass
28+
def self.do_class_business
29+
count = ids.size
30+
end
31+
end
32+
RUBY
33+
34+
assert_equal 0, offenses.size
35+
end
36+
37+
def test_unoffended_by_unscoped_call_from_active_record_instance_method
38+
offenses = investigate cop, <<-RUBY
39+
class Widget < ApplicationRecord::Thing
40+
def do_instance_business
41+
count = ids.size
42+
end
43+
end
44+
RUBY
45+
46+
assert_equal 0, offenses.size
47+
end
48+
end

0 commit comments

Comments
 (0)