Undefined method `split' for nil:nilClass (NoMethodError)

[midwire]

I am getting the following error consistently when running rubycritic on a particular project:

.../gems/2.1.0/gems/rubycritic-1.1.0/lib/rubycritic/analysers/smells/flog.rb:48:in `smell_locations': undefined method `split' for nil:NilClass (NoMethodError)

I will dig further but wanted to submit this to see if anyone else has seen it.

[guilhermesimoes]

Can you figure out what file is causing it? That would be great.

[midwire]

It happens when running against the FFI gem in 'vendor': ffi-1.9.3. The method is in flog.rb, line 48:

  def smell_locations(context)
    line = @flog.method_locations[context]
    file_path, file_line = line.split(":") # <-- Raises here, when 'line' is nil
    [Location.new(file_path, file_line)]
  end

I can certainly submit a PR to ignore lines if they are nil but I'm not sure that is the best way to handle this. Thoughts?

The 'context' is "describe(Callback)::LibTest#Callback", and so line = @flog.method_locations[context] is not finding that context within the collection.

Looks like the file is: vendor/bundle/ruby/1.9.1/gems/ffi-1.9.3/spec/ffi/callback_spec.rb

[midwire]

PS: This may just go away if we can ignore 'vendor'. I don't think anyone will want to critique their 'vendor' directory.

[guilhermesimoes]

Yeah, we don't want to ignore that. We want a location for every smell, otherwise we can't show it in the file report.

Like I said on issue #11, running rubycritic app lib should be enough for Rails apps. Any reason you can't do that?

[midwire]

Unfortunately rubycritic app lib does not ignore the vendor directory. I get the same error when doing that command line.

Also, it is not a Rails app that I'm testing against. It is Sinatra, but I also get the same error on the same ffi gem in another gem that I'm testing against. As I'm sure you know, Rails apps are not the only Ruby projects that bundle gems under 'vendor'. :)

[midwire]

I also see a lot of errors like this, with a command line of rubycritic app lib:

ERROR: parsing ruby file vendor/bundle/ruby/1.9.1/gems/backports-3.6.0/spec/tags/1.9.2/core/io/write_spec.rb
Racc::ParseError: vendor/bundle/ruby/1.9.1/gems/backports-3.6.0/spec/tags/1.9.2/core/io/write_spec.rb:2 :: parse error on value "option" (tIDENTIFIER) at:
  /Users/midwire/.rbenv/versions/2.1.1/lib/ruby/2.1.0/racc/parser.rb:529:in `on_error'
  /Users/midwire/.rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/gems/ruby_parser-3.4.1/lib/ruby_parser_extras.rb:1095:in `on_error'
  /Users/midwire/.rbenv/versions/2.1.1/lib/ruby/2.1.0/racc/parser.rb:258:in `_racc_do_parse_c'
  /Users/midwire/.rbenv/versions/2.1.1/lib/ruby/2.1.0/racc/parser.rb:258:in `do_parse'
  /Users/midwire/.rbenv/versions/2.1.1/lib/ruby/gems/2.1.0/gems/ruby_parser-3.4.1/lib/ruby_parser_extras.rb:1019:in `block in process'

[guilhermesimoes]

I still haven't looked into that fii gem file that's causing this issue, but I just pushed a new version that should allow you to run rubycritic app lib. Sorry for the trouble.

Also, I've never vendored my gems, so when you spoke of a vendor folder, I immediately thought of Rails.

As for those errors, they occur when files are unparsable. Those errors get reported multiple times by the various gems wrapped by RubyCritic. I know that Flog has a quiet options that we can use, but Flay and Reek don't. We'll have to figure out a way to solve that.

[guilhermesimoes]

This has somewhat been fixed upstream.

Given the following code:

describe "Callback" do
  module LibTest
    extend FFI::Library
  end
end

Running flog --all callback_spec.rb yields:

1.0: describe#Bad                     callback_spec.rb:1
1.0: describe(Bad)::Helper#none

That none method does not exist and so obviously it's missing its file and line, which is far from ideal.

However, using Flog's option to skip code outside of methods, the none method is omitted.

Running flog --all --methods-only callback_spec.rb yields:

1.0: describe#Bad                     callback_spec.rb:1

And there you have it. Luckily, RubyCritic already uses this option so all Flog smells should have files and lines.

I consider this fixed for now, but if this type of error ever comes up again, please feel free to reopen.