Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GitHub/AvoidObjectSendWithDynamicMethod cop #145

Merged
merged 6 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

## v0.21.0

- Added new GitHub/AvoidObjectSendWithDynamicMethod cop to discourage use of methods like Object#send

## v0.20.0

- Updated minimum dependencies for "rubocop" (`>= 1.37`), "rubocop-performance" (`>= 1.15`), and "rubocop-rails", (`>= 2.17`).
Expand Down
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
rubocop-github (0.20.0)
rubocop-github (0.21.0)
rubocop (>= 1.37)
rubocop-performance (>= 1.15)
rubocop-rails (>= 2.17)
Expand Down
3 changes: 3 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ Gemspec/RequiredRubyVersion:
Gemspec/RubyVersionGlobalsUsage:
Enabled: false

GitHub/AvoidObjectSendWithDynamicMethod:
Enabled: true

GitHub/InsecureHashAlgorithm:
Enabled: true

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop-github.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@

RuboCop::GitHub::Inject.default_defaults!

require "rubocop/cop/github/avoid_object_send_with_dynamic_method"
require "rubocop/cop/github/insecure_hash_algorithm"
77 changes: 77 additions & 0 deletions lib/rubocop/cop/github/avoid_object_send_with_dynamic_method.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# frozen_string_literal: true

require "rubocop"

module RuboCop
module Cop
module GitHub
# Public: A Rubocop to discourage using methods like Object#send that allow you to dynamically call other
# methods on a Ruby object, when the method being called is itself completely dynamic. Instead, explicitly call
# methods by name.
#
# Examples:
#
# # bad
# foo.send(some_variable)
#
# # good
# case some_variable
# when "bar"
# foo.bar
# else
# foo.baz
# end
#
# # fine
# foo.send(:bar)
# foo.public_send("some_method")
# foo.__send__("some_#{variable}_method")
class AvoidObjectSendWithDynamicMethod < Base
MESSAGE_TEMPLATE = "Avoid using Object#%s with a dynamic method name."
SEND_METHODS = %i(send public_send __send__).freeze
CONSTANT_TYPES = %i(sym str const).freeze

def on_send(node)
return unless send_method?(node)
return if method_being_sent_is_constrained?(node)
add_offense(source_range_for_method_call(node), message: MESSAGE_TEMPLATE % node.method_name)
end

private

def send_method?(node)
SEND_METHODS.include?(node.method_name)
end

def method_being_sent_is_constrained?(node)
method_name_being_sent_is_constant?(node) || method_name_being_sent_is_dynamic_string_with_constants?(node)
end

def method_name_being_sent_is_constant?(node)
method_being_sent = node.arguments.first
# e.g., `worker.send(:perform)` or `base.send("extend", Foo)`
CONSTANT_TYPES.include?(method_being_sent.type)
end

def method_name_being_sent_is_dynamic_string_with_constants?(node)
method_being_sent = node.arguments.first
return false unless method_being_sent.type == :dstr

# e.g., `foo.send("can_#{action}?")`
method_being_sent.child_nodes.any? { |child_node| CONSTANT_TYPES.include?(child_node.type) }
end

def source_range_for_method_call(node)
begin_pos =
if node.receiver # e.g., for `foo.send(:bar)`, `foo` is the receiver
node.receiver.source_range.end_pos
else # e.g., `send(:bar)`
node.source_range.begin_pos
end
end_pos = node.loc.selector.end_pos
Parser::Source::Range.new(processed_source.buffer, begin_pos, end_pos)
end
end
end
end
end
2 changes: 1 addition & 1 deletion rubocop-github.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Gem::Specification.new do |s|
s.name = "rubocop-github"
s.version = "0.20.0"
s.version = "0.21.0"
s.summary = "RuboCop GitHub"
s.description = "Code style checking for GitHub Ruby repositories "
s.homepage = "https://github.com/github/rubocop-github"
Expand Down
80 changes: 80 additions & 0 deletions test/test_avoid_object_send_with_dynamic_method.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# frozen_string_literal: true

require_relative "./cop_test"
require "minitest/autorun"
require "rubocop/cop/github/avoid_object_send_with_dynamic_method"

class TestAvoidObjectSendWithDynamicMethod < CopTest
def cop_class
RuboCop::Cop::GitHub::AvoidObjectSendWithDynamicMethod
end

def test_offended_by_send_call
offenses = investigate cop, <<-RUBY
def my_method(foo)
foo.send(@some_ivar)
end
RUBY
assert_equal 1, offenses.size
assert_equal "Avoid using Object#send with a dynamic method name.", offenses.first.message
end

def test_offended_by_public_send_call
offenses = investigate cop, <<-RUBY
foo.public_send(bar)
RUBY
assert_equal 1, offenses.size
assert_equal "Avoid using Object#public_send with a dynamic method name.", offenses.first.message
end

def test_offended_by_call_to___send__
offenses = investigate cop, <<-RUBY
foo.__send__(bar)
RUBY
assert_equal 1, offenses.size
assert_equal "Avoid using Object#__send__ with a dynamic method name.", offenses.first.message
end

def test_offended_by_send_calls_without_receiver
offenses = investigate cop, <<-RUBY
send(some_method)
public_send(@some_ivar)
__send__(a_variable, "foo", "bar")
RUBY
assert_equal 3, offenses.size
assert_equal "Avoid using Object#send with a dynamic method name.", offenses[0].message
assert_equal "Avoid using Object#public_send with a dynamic method name.", offenses[1].message
assert_equal "Avoid using Object#__send__ with a dynamic method name.", offenses[2].message
end

def test_unoffended_by_other_method_calls
offenses = investigate cop, <<-RUBY
foo.bar(arg1, arg2)
case @some_ivar
when :foo
baz.foo
when :bar
baz.bar
end
puts "public_send" if send?
RUBY
assert_equal 0, offenses.size
end

def test_unoffended_by_send_calls_to_dynamic_methods_that_include_hardcoded_strings
offenses = investigate cop, <<-'RUBY'
foo.send("can_#{action}?")
foo.public_send("make_#{SOME_CONSTANT}")
RUBY
assert_equal 0, offenses.size
end

def test_unoffended_by_send_calls_without_dynamic_methods
offenses = investigate cop, <<-RUBY
base.send :extend, ClassMethods
foo.public_send(:bar)
foo.__send__("bar", arg1, arg2)
RUBY
assert_equal 0, offenses.size
end
end