Double-checking if a method is unused in Ruby

A real life example for Ruby’s Prepend

Have you ever met a method in the codebase that seemed very much unused, but you were not 100% sure and didn’t dare to remove it?

In this tutorial we are gonna build a tool that sends a notification if a marked method gets invoked. This helps us to double-check if the method in question is indeed unused before removing it.

A real life example for Ruby’s Prepend

The final product in action

GitHub
config/invoked_method_reporter.yml

methods_to_report: [
  'TargetModule#unused_method_from_module',
  'TargetModule.method_of_module',
  'User.unused_method_from_class',
  'User#unused_method_from_class'
]

target_module.rb

module TargetModule
  def self.method_of_module
  end

  def unused_method_from_module
  end
end

Initialiser

InvokedMethodReporter.setup

user.rb

class User
  include TargetModule
  extend TargetModule

  def self.unused_method_from_class
  end

  def unused_method_from_class
  end
end

As a result we will receive a notification in any of the following cases.

  • TargetModule.method_of_module
  • User.unused_method_from_class
  • User.new.unused_method_from_class
  • User.unused_method_from_module
  • User.new.unused_method_from_module

The story

One day at work I found a funny looking method. I did a quick research and I saw no visible reference neither to it nor its name as a text in the codebase.

Most probably this method was unused. But given that the project is an old legacy application where everything is possible and which deals with money, I was somewhat reluctant to just remove it.

If only there were an option to get a slack notification when the method was called to double-check things before removing it.

Well, why not to build one as an exercise?

The theory

Ruby has a handy little tool we can leverage: prepend. This article describes it quite thoroughly. But in short: we can override any existing method without inheritance or touching them at all.
We only need:

  • a new module
  • override the target method in the new module (and call super)
  • prepend the new module into the module or class that contains the target method

The first playful approach

GitHub

describe UserInvokedMethodReporter do
  it 'invokes the reporter then the original impl' do
    expect(STDOUT).to receive(:puts).with('User#unused_method_from_class was invoked').ordered
    expect(STDOUT).to receive(:puts).with('Original impl with: Param1').ordered

    User.new.unused_method_from_class('Param1')
  end
end

class User
  def unused_method_from_class(param1)
    puts "Original impl with: #{param1}"
  end
end

module UserInvokedMethodReporter
  def unused_method_from_class(*)
    puts 'User#unused_method_from_class was invoked'
    super
  end
end

User.prepend(UserInvokedMethodReporter)

To look like professionals, we start with a test to describe the desired final behaviour. Then we have the User class with a suspected unused method. And finally we perform the override using a new module and the magic prepend method.

This way we can seamlessly inject our rebel reporter functionality before the original implementation. We don’t need to touch the original code. Which can even change we will break nothing, this is what I call loosely coupling.

One inconvenience with this solution is we need to create a module for every class or module we wish to monitor and do the prepend manually.

Second approach: automate

GitHub
Wouldn’t it be cool if we could just call a method and it would bind a reporter to the target method? This would allow us to read a list of the target methods from a config file and automate the whole process.

Metaprogramming is here to the rescue to write code that will write code for us.

Let’s start with the API design

To bind a reporter:

InvokedMethodReporter.bind_to('User#unused_method_from_class')

To send a notification or log what happened:

InvokedMethodReporter.report('User#unused_method_from_class')

Remember we are decent engineers? Meaning thorough tests! (the sooner the better)

describe InvokedMethodReporter do
  describe '.bind_to' do
    it 'invokes the reporter then the original impl' do
      described_class.bind_to('User#unused_method_from_class')

      expect(STDOUT).to receive(:puts).with('User#unused_method_from_class was invoked').ordered
      expect(STDOUT).to receive(:puts).with('Original impl with: Param1').ordered

      User.new.unused_method_from_class('Param1')
    end
  end
end

Basically in this test we expect that the reporter will be called and then the original implementation, in this exact order.

I have to admit this test doesn’t meet 100% the requirements of the unit testing theory. After the next section you will see it is more like an integration test. I like integration tests and I’m not really a mockist.

The design

The interface InvokedMethodReporter.bind_to looks pretty neat, but observe that we don’t call the bind_to method on an instance of the InvokedMethodReporter class. Which means bind_to is only a helper method. The question arises: should we pack all the functionality there or somewhere else? Well, the answer is as usual: it depends. I’d say if we have only a few lines of code, then don’t bother and put them into the helper method. Maybe it is not visible from here, but we will have so much functionality that it seems an object is about to be born.

Following the single responsible principle, let’s put the binding functionality into a Binder class and let its object do the job. On the other hand InvokedMethodReporter can stay as a Module which contains two helper methods. If needed we can convert it into a class later.

The implementation

# invoked_method_reporter.rb
module InvokedMethodReporter
  def self.bind_to(method_definition)
    Binder.bind_to(method_definition)
  end

  def self.report(method_definition)
    puts "#{method_definition} was invoked"
  end
end

The module is quite self-explanatory. It proxies the binding functionality and defines a basic reporter, which we will pimp up later.

# invoked_method_reporter/binder.rb
module InvokedMethodReporter
  class Binder
    attr_reader :namespace, :method_name, :method_definition

    def self.bind_to(method_definition)
      new(method_definition).bind
    end

    def initialize(method_definition)
      @method_definition       = method_definition
      @namespace, @method_name = method_definition.split('#')
    end

    def bind
      target_const.prepend(module_to_prepend)
    end

    private

    def target_const
      Object.const_get(namespace)
    end

    def module_to_prepend
      local_method_name       = method_name
      local_method_definition = method_definition

      Module.new do
        define_method(local_method_name) do |*args|
          InvokedMethodReporter.report(local_method_definition)
          super(*args)
        end
      end
    end
  end
end

At the beginning we grab the string User#unused_method_from_class in the constructor. This contains the namespace and the method separated by a # sign. This is a convention to imply the method is defined on the object level (even if everything is an object in Ruby). First we separate the string into well-defined variables.

Then we continue with the binding. For this we need two things:

  • get the class constant
  • send the prepend message to it with a module that contains the new method definition of the target method

The first one is relatively easy, just use Object.const_get, and once more Ruby’s magic explodes into action. namespace.constantize is also available in the Rails world.

The second part is a bit trickier because we need that special module. Thanks to Ruby’s flexibility we can create one using the Module class. Then we define the target method on it with the define_method method and we are good to go.

But wait, something strange is going on, why are there two local variables copying values from the objet level fields? Unfortunately the object level fields are not accessible inside the block given to Module.new.

So far so good, but one might ask: What about the class level methods and all the other things with the target_module?

Breath easy young padawan, we will get there soon enough.

Next step: attach reporter to class level methods

GitHub

In this case we have a minor problem with the target_const method.

User.prepend is good enough to override object level methods, but insufficient for the class level. We need to sort of level up. We can use the singleton_class method to reach the higher level.

def target_const
  Object.const_get(namespace).singleton_class
end

Another task is to detect if we operate on the object or the class level. The difference comes from the character that separates the namespace from the method name, it can be a . or a #.

With this knowledge secured, let’s review the current design.

We could extend the Binder class with more code and with more if statements, but I really like replacing conditionals with polymorphism. So let’s separate matters into their own place.

A little tip to leverage identifying sameness or difference. As Sandi Metz writes in her 99bottles book:

However, despite the fact that sameness is easier to identify, difference is more useful because it has more meaning.

We can identify two differences:

  • the target_const implementation
  • using different character to separate the namespace from the method name

Everything else seems to be the same, so they can stay in the Binder class and we can move the differences into separate child classes.

Sounds like a plan, let’s materialise it in the code.

Tests Tests Tests

describe InvokedMethodReporter do
  describe '.bind_to' do
    context 'class level' do
      it 'invokes the reporter then the original impl' do
        described_class.bind_to('User.unused_method_from_class')

        expect(STDOUT).to receive(:puts).with('User.unused_method_from_class was invoked').ordered
        expect(STDOUT).to receive(:puts).with('Original impl with: Param1').ordered

        User.unused_method_from_class('Param1')
      end
    end

    context 'object level' do
      it 'invokes the reporter then the original impl' do
        described_class.bind_to('User#unused_method_from_class')

        expect(STDOUT).to receive(:puts).with('User#unused_method_from_class was invoked').ordered
        expect(STDOUT).to receive(:puts).with('Original impl with: Param1').ordered

        User.new.unused_method_from_class('Param1')
      end
    end
  end
end

This test only extends the previous one with the class level binding.

The implementation

# invoked_method_reporter/binder.rb
module InvokedMethodReporter
  class Binder
    attr_reader :namespace, :method_name, :method_definition

    def self.bind_to(method_definition)
      if method_definition.include?('.')
        ClassLevelBinder.new(method_definition).bind
      else
        ObjectLevelBinder.new(method_definition).bind
      end
    end

    def initialize(method_definition)
      @method_definition       = method_definition
      @namespace, @method_name = method_definition.split(self.class::SEPARATOR)
    end

    def bind
      target_const.prepend(module_to_prepend)
    end

    private

    def module_to_prepend
      local_method_name       = method_name
      local_method_definition = method_definition

      Module.new do
        define_method(local_method_name) do |*args|
          InvokedMethodReporter.report(local_method_definition)
          super(*args)
        end
      end
    end
  end
end
# invoked_method_reporter/class_level_binder.rb
module InvokedMethodReporter
  class ClassLevelBinder < Binder
    SEPARATOR = '.'

    private

    def target_const
      Object.const_get(namespace).singleton_class
    end
  end
end
# invoked_method_reporter/object_level_binder.rb
module InvokedMethodReporter
  class ObjectLevelBinder < Binder
    SEPARATOR = '#'

    private

    def target_const
      Object.const_get(namespace)
    end
  end
end

I was hesitating where to put this part:

if method_definition.include?('.')
  ClassLevelBinder.new(method_definition).bind
else
  ObjectLevelBinder.new(method_definition).bind
end

The other option would be InvokedMethodReporter.bind_to. But I feel this responsibility belongs more to the Binder class. But then the child classes inherit this method. Le me know your opinion in the comment section.

Config and helper methods

# invoked_method_reporter/config.rb
require 'yaml'

module InvokedMethodReporter
  class Config
    def initialize
      @config = YAML.load_file('config/invoked_method_reporter.yml')
    end

    def methods_to_report
      @config['methods_to_report']
    end
  end
end
# invoked_method_reporter.rb
module InvokedMethodReporter
  def self.config
    @config ||= Config.new
  end

  def self.setup
    bind_to(config.methods_to_report)
  end

  def self.bind_to(method_definitions)
    Array(method_definitions).each do |method_definition|
      Binder.bind_to(method_definition)
    end
  end
end

A neat trick in the bind_to method: Array(method_definitions) this makes possible to call the method with a single item or with an array, the outcome will be the same.

Advanced testing

GitHub
So far we have used an existing class for testing. This has a few drawbacks in a bigger system:

  • the target method can be removed from the class
  • the whole class can be removed
  • it is not threatening us right now, but this type of low level code can cause unwanted side effects in the tests running after

So our test is tightly coupled to an existing class and can develop hostile activities later on.

Why not to create our own “mock” class? Things to look after:

  • choose a class name that most probably won’t appear in real code
  • remove the class after the test to avoid side effects
describe InvokedMethodReporter do
  class TargetClassForInvokedMethodReporter
    def original_impl; end

    def self.original_impl; end

    def unused_method_from_class
      original_impl
    end

    def self.unused_method_from_class
      original_impl
    end
  end

  described_class.bind_to('TargetClassForInvokedMethodReporter#unused_method_from_class')
  described_class.bind_to('TargetClassForInvokedMethodReporter.unused_method_from_class')

  after(:all) do
    Object.send(:remove_const, :TargetClassForInvokedMethodReporter)
  end

  describe '.bind_to' do
    context 'object level' do
      it 'invokes the reporter then the original implementation' do
        target_class_object = TargetClassForInvokedMethodReporter.new

        expect(described_class).to receive(:report).ordered
        expect(target_class_object).to receive(:original_impl).ordered

        target_class_object.unused_method_from_class
      end
    end

    context 'class level' do
      it 'invokes the reporter then the original implementation' do
        expect(described_class).to receive(:report).ordered
        expect(TargetClassForInvokedMethodReporter).to receive(:original_impl).ordered

        TargetClassForInvokedMethodReporter.unused_method_from_class
      end
    end
  end
end

Methods defined in modules

At this point we are good with methods defined in classes. We have three more cases to discuss:

  • method defined in module A
    • the module A included into a class
    • class extended by the module A
  • module A defines a “class” level method

Better illustration in code:

module TargetModule
  def unused_method_from_module
  end

  def self.unused_method_in_module
  end
end

class TargetClass
  include TargetModule
  extend TargetModule
end

This gives us three method call options:

TargetClass.new.unused_method_from_module
TargetClass.unused_method_from_module
TargetModule.unused_method_in_module

The good news is our current implementation covers those cases. And naturally there is a but: the reporter for modules has to be called before include TargetModule or extend TargetModule happens.

The reason? Run the following code snippets and examine the console output.

module InvokedMethodReporter
end

module TargetModule
  def unused_method_from_module
  end

  def self.unused_method_in_module
  end
end

class TargetClass
  include TargetModule
  extend TargetModule
end

TargetModule.prepend(InvokedMethodReporter)

p TargetClass.ancestors
module InvokedMethodReporter
end

module TargetModule
  def unused_method_from_module
  end

  def self.unused_method_in_module
  end
end

TargetModule.prepend(InvokedMethodReporter)

class TargetClass
  include TargetModule
  extend TargetModule
end

p TargetClass.ancestors

The console output of the first case

[TargetClass, TargetModule, Object, Kernel, BasicObject]

The console output of the second case

[TargetClass, InvokedMethodReporter, TargetModule, Object, Kernel, BasicObject]

The significant difference is the InvokedMethodReporter is injected into the inheritance chain of TargetClass so it can override methods defined by TargetModule.

Nothing is left except to extend our test suite for modules.

The reporter

After all the hard work, finally we can have the well-deserved fun with reporting. At work we use Rollbar, hooked into a dedicated Slack channel so we can have live notifications.

require 'active_support/backtrace_cleaner'

module InvokedMethodReporter
  def self.report(method_definition)
    message = "[InvokedMethodReporter] #{method_definition} was invoked"

    report_params = {
      sender: sender_trace
    }

    Rollbar.info(message, report_params)
  rescue StandardError => e
    Rollbar.error('Error in InvokedMethodReporter.report', e)
    # we should raise an exception here in case of test env
  end

  private_class_method

  def self.sender_trace
    backtrace_cleaner = ActiveSupport::BacktraceCleaner.new

    filters = [
      'invoked_method_reporter/binder.rb',
      'invoked_method_reporter.rb',
      'gems'
    ]

    backtrace_cleaner.add_silencer do |line|
      filters.any? { |filter| line.include?(filter) }
    end

    backtrace_cleaner.clean(caller).first
  end
end

It is interesting to know who sends the message to our “unused_method”. The backtrace is a good start, but it can contain a lot of non-project-related files, like files from gems etc. The ActiveSupport::BacktraceCleaner is a very useful class to remove unrelated paths from the backtrace.

Rails integration

To make sure we bind the reporter to modules before they get included, we wanna make sure that our initialiser runs first. Not the nicest solution but it will do the work:

# config/initializers/01_invoked_method_reporter.rb
InvokedMethodReporter.setup

The rest of the files can go into the lib folder just like in the final version.

Final thoughts

We have this solution in production for weeks by now. We set up the time limit for one month, meaning if a method doesn’t get invoked for a month that’s good enough to remove it. Soon we can get rid of some dead and unclean part of our codebase. The feeling will be close to indescribable.

Please let me know your opinion.

Comments