Writing and testing a custom RuboCop cop

Solving a problem is great — but keeping it from coming back is even better. As we resolve issues in our code base, we often consider how to keep that classification of issue out of the code base entirely. Sometimes we reach for RuboCop to help us police certain patterns. This also helps to document the originating issue and educates teammates on why these patterns are undesirable.

RuboCop is more than just a linter. It is highly extensible and allows you to write custom cops to enforce specific behavior. These cops can be used to create better code practices, prevent bad patterns from sneaking into a legacy code base, and provide training for other engineers. But it can be tricky to know how to create a new cop and if it will work long-term.

We can write unit tests to ensure the success of our custom cops, just as we would with any application code.

Let's explore this with an example to show how testing could be done.

Testing custom cops

With the Aha! engineering team, every model has an account_id attribute present and for security reasons, we never want this to be set via mass-assignment. To avoid this, we want to prevent certain attributes from being added to attr_accessible.

# bad
class Foo
  attr_accessible :name, :account_id
end
Foo.create(account_id: 1, name: "foo")

# good
class Foo
  attr_accessible :name
end
foo = Foo.new(name: "foo")
foo.account_id = 1
foo.save

We have a custom cop that analyzes the arguments to that method and will error if any protected attribute is present. The custom cop we have ends up looking something like this:

class RuboCop::Cop::ProtectedAttrAccessibleFields < RuboCop::Cop::Cop
  # We can define a list of attributes we want to protect
  PROTECTED_ATTRIBUTES = [
    :account_id,
  ].freeze

  # We can define an error message that is displayed when an offense is detected.
  # This can be helpful to communicate information back to other engineers
  ERROR_MESSAGE = <<~ERROR.freeze
    Only permit attributes that are safe to be completely user controlled. Typically any *_id field could be problematic.
    Instead perform direct assignment of the field after doing a scoped lookup. This is the safest way to handle user input.
    Some fields such as #{PROTECTED_ATTRIBUTES.inspect} should never be used as part of attr_accessible.
  ERROR

  # We want to examine method calls. Particularly those that are calling the attr_accessible method
  # and also have arguments we care about
  def on_send(node)
    if receiver_attr_accessible?(node) && protected_arguments?(node)
      # If we do detect an attr_accessible call with arguments we care about, we can record an offense
      add_offense(node, message: ERROR_MESSAGE)
    end
  end

  private

  def receiver_attr_accessible?(node)
    node.method_name == :attr_accessible
  end

  def protected_arguments?(node)
    node.arguments.any? do |argument|
      if argument.sym_type? || argument.str_type?
        PROTECTED_ATTRIBUTES.include?(argument.value.to_sym)
      end
    end
  end
end

This custom cop does the trick. Adding a test for it ensures that it won't break in the future when we update RuboCop or extend the functionality. In order to write a test, we need to understand how the custom cops are set up and run.

Instantiate a custom cop

RuboCop::Cop::Cop inherits from RuboCop::Cop::Base and that allows the instantiation without any arguments. So it turns out this isn't anything special — creating a new instance of our cop is really as simple as: RuboCop::Cop::ProtectedAttrAccessibleFields.new

If the cop requires some kind of configuration, it can be passed to the instance via a RuboCop::Config object. The RuboCop::Config takes two arguments. RuboCop can provide configuration via YML files. You can use the first argument of RuboCop::Config to pass this configuration with various values from the test. The second argument is the path of the loaded YML file, which can be ignored in the tests.

config = RuboCop::Config.new({ RuboCop::Cop::ProtectedAttrAccessibleFields.badge.to_s => {} }, "/")
cop = RuboCop::Cop::ProtectedAttrAccessibleFields.new(config)

Process, execute, examine

As it turns out, there is a method available, RuboCop::Cop::Base#parse , that accepts a string as input and will return something the cop can process.

This allows us to have something like:

source = <<~CODE
  attr_accessible :account_id
CODE
processed_source = cop.parse(source)

There is a class from within RuboCop, RuboCop::Cop::Commissioner , that is responsible for taking a list of cops and using those to investigate the processed source code. In order to run our cop, we can run this method.

commissioner = RuboCop::Cop::Commissioner.new([cop])
investigation_report = commissioner.investigate(processed_source)

The RuboCop::Cop::Commissioner#investigate method will return an instance of RuboCop::Cop::Commissioner::InvestigationReport which is a simple struct class that has a list of offenses that have been recorded.

Put it all together

We end up with a test file that looks something like this:

describe RuboCop::Cop::ProtectedAttrAccessibleFields do
  let(:config) { RuboCop::Config.new({ described_class.badge.to_s => {} }, "/") }
  let(:cop) { described_class.new(config) }
  let(:commissioner) { RuboCop::Cop::Commissioner.new([cop]) }

  it "records an offense if we use allow account_id as a string" do
    source = <<~CODE
      attr_accessible :foo, 'account_id'
    CODE
    investigation_report = commissioner.investigate(cop.parse(source))

    expect(investigation_report.offenses).to_not be_blank
    expect(investigation_report.offenses.first.message).to eql described_class::ERROR_MESSAGE
  end

  it "records an offense if we use allow account_id as symbol" do
    source = <<~CODE
      attr_accessible :foo, :account_id
    CODE
    investigation_report = commissioner.investigate(cop.parse(source))

    expect(investigation_report.offenses).to_not be_blank
    expect(investigation_report.offenses.first.message).to eql described_class::ERROR_MESSAGE
  end

  it "doesn't record an offense if no protected attribute is used" do
    source = <<~CODE
      attr_accessible :foo
    CODE
    investigation_report = commissioner.investigate(cop.parse(source))

    expect(investigation_report.offenses).to be_blank
  end

end

Now that we know how to write tests, we can use them as a starting point for building new cops, extending existing cops, and ensuring that things continue to function as our application grows and evolves. These little investments into project-specific cops can end up being a large investment in the future health of the projects.

Sign up for a free trial of Aha! Develop

Aha! Develop is a fully extendable agile development tool. Prioritize the backlog, estimate work, and plan sprints. If you are interested in an integrated product development approach, use Aha! Roadmaps and Aha! Develop together. Sign up for a free 30-day trial or join a live demo to see why more than 5,000 companies trust our software to build lovable products and be happy doing it. -->