Rubycritic does not handle inline classes very well

[Aqualon]

Hi, I ran rubycritic on one of my projects and got the following complaint about a class:

Updated 8 days ago
107 complexity
6.7 complexity per method
74 churn
16 methods
17 duplication

The problem is, that ApplicationController::InvalidSid is just a error class defined via

class InvalidSid < StandardError; end

So all the offenses are actually from ApplicationController itself.

Reporting this here, since I don't know if this is something caused by rubycritic itself or which is misreported by one of the gems doing the analysis.

[troessner]

Acknowledged. That's a pretty bad bug.

Reproducibly via:

class Foo
  def bar(x,y,z); end
  class Wtf; end
end

and:

rubycritic foo.rb

This will report everything on Foo::Wtf.

We probably should fix that since I assume nested classes occur in almost every bigger code base 😆

[Aqualon]

Interestingly it reports always the first nested class inside Foo. If I add another class before Wtf this one gets reported.

[danmarcab]

First of all thanks for this awesome project 👍

We are having the same problem, so I started to take a look to this issue.

The problem is on https://github.com/whitesmith/rubycritic/blob/master/lib/rubycritic/analysers/attributes.rb#L17

Here we get the first module name as the name of the module.

The question is how to decide which name to pick? The fact that rubycritic analyses and report based on files (as far as I understood) I wonder if trying to parse the file to get the module names is an overkill.

I am happy to work on this if we can define a rule of what class/module should we use. I see two alternatives:

• Decide this is too hard or codebase specific so maybe the best thing is to report just based in the filename (use the filename instead of the module name).
• Check all module names against the filename and pick the one that matches the name of the file. Fallback to filename based.

I was playing around and the second option is feasible (I have a working prototype), I can create a PR if you think it makes sense.

Sorry if I missed a previous discussion on this. I just want to help to improve this project.

[rishijain]

This is an issue with the latest version of RC as well.