Rubocop: RuboCop counts lines incorrectly when run as a rake task

[karmix]

RuboCop is reporting offenses in our code detected by the Metrics/BlockLength cop on module-like objects. We disable the cop on these blocks and try to move on, but Lint/RedundantCopDisableDirective later starts reporting that we disabled the cop on these blocks unnecessarily.  

I found that I was getting different results when running RuboCop with its rake task than running it on the command line.  I have tracked down the problem and it looks like a bug.  If RuboCop is expected to run in the same VM as other unrelated gems, as is the case when running as a rake task alongside tasks for other tools, it should not rely on storing code outside its own namespace where it might interact with other gems.

Specifically, this is what I found:

Besides RuboCop, our Rakefile uses code analysis tools provided by Chef.  Those tools use a gem called Treetop.  Treetop provides a very simple implementation of String#blank?.  RuboCop expects a more complex implementation of String#blank?.  When a Rakefile imports Treetop before RuboCop, RuboCop ends up calling Treetop's implementation of String#blank? instead of its own.

Many of RuboCop's Metrics cops use the CodeLengthCalculator class to count lines of code.  The CodeLengthCalculator class relies on String#blank? when determining whether a it should count a line as code.  Lines of code extracted by RuboCop end with an embedded new-line character, which TreeTop's simplistic definition of String#blank? will never match ("\n" != "").  As a result, RuboCop counts all lines as code when Treetop defines String#blank?.

---

Expected behavior

RuboCop should find the same results whether run on the command line or via a Rake task.

Actual behavior

When Treetop is loaded in the Rakefile before RuboCop, RuboCop's metrics cops count blank lines as lines of code, causing .

Steps to reproduce the problem

1. Create .rubocop.yml containing:

AllCops:
  DisabledByDefault: true
Metrics/BlockLength:
  Max: 2

1. Create test.rb containing:

[].each do
  puts 'line 1'

  puts 'line 2'
end

1. Create Rakefile containing:

require 'treetop'
require 'rubocop/rake_task'
RuboCop::RakeTask.new

1. Run rubocop

$ rubocop
Inspecting 2 files
..

2 files inspected, no offenses detected

and observe different results when running rake

$ rake rubocop
Running RuboCop...
Inspecting 2 files
.C

Offenses:

test.rb:1:1: C: Metrics/BlockLength: Block has too many lines. [3/2]
[].each do ...
^^^^^^^^^^

2 files inspected, 1 offense detected
RuboCop failed!

RuboCop version

I was using rubocop 0.89.1 (using Parser 2.7.1.4, rubocop-ast 0.3.0, running on ruby 2.7.1 x86_64-linux) when I discovered this bug. I have confirmed that it is still present in master.

[karmix]

When searching for open issues, I ran across #2134, which appears to be a slightly different issue (RuboCop is not failing without listing offenses, in this case).  However, @tibileo and @tmsanrinsha described this issue later in the dialogue. @tmsanrinsha even narrowed the problem down to a conflict with Treetop (which I wish I had seen sooner).

[marcandre]

Good investigating!

I'm sorry it's causing you grief.

My personal opinion is that treetop is really in the wrong here. As a gem that is meant to be used by others, it should not monkeypatch base classes. Moreover, it should definitely not monkeypatch them in a way that is:
1) quite different from Rails' extension with the same name
2) isn't much faster than a much better version (like ours)
3) is completely unnecessary (why don't they use empty???)

That being said, let me open #8637 to address this... 😆

[marcandre]

I didn't add a test, but I'm pretty sure the PR I just merged should do the trick.

Let us know if that's not the case.

[cjheath]

Fixed in Treetop 1.6.11

[karmix]

I'd like to thank everyone for the amazingly fast turn around on this issue. You all rock!