simplabs / excellent
- Source
- Commits
- Network (3)
- Issues (10)
- Downloads (11)
- Wiki (31)
- Graphs
-
Branch:
master
-
more formatters should be added (HTML etc). There should also be a documented API for custom formatters.
Comments
-
0 comments Created 5 months ago by marcoowchecksxChecks for use of params[:x] or session[:y] in views/templatesmarcoowx -
0 comments Created 5 months ago by marcoowchecksxCheck for custom initializers in ActiveRecordmarcoowx -
1 comment Created 4 months ago by marcoowchecksxLook over which tests are executed by defaultmarcoowxMaybe the complexity checks should not be executed by default since they produce tons of output that is usually of little help. Maybe other checks should be excluded from the list of default checks too
Comments
-
RDoc should explicitly describe the extension API that is available for adding more/ custom checks
Comments
-
1 comment Created 2 months ago by marcoowadd highlighted source to the HTML formatteroutputx -
I think it would be great, if programmers mark specific lines of code to exclude from the check. So they can at e.g. "# excellent:ignore" to a method to skip flog test or to a model to skip validation check.
Of course, everybody has to check for himself, if he use this to "make the checker silent", but also it provides the possibility to reach zero warnings. For example checkstyle in the java world does this, too.Comments
This is something I don't really want to do. Excellent was made to make the code better, not worse ;) "Pragmas" like
# excellent:ignorein the source would be something that Excellent would actually have to report as code smells.There is a ticket that requests configuration files (#2), but I don't think it makes sense to go down to the class or even method level to enable/disable certain checks. This would soon become a configuration hell every time you refactor something, want to disable or re-enable checks, forget to do so after refactorings etc.
Yes, but think of a bigger project with maybe 1000 warnings. The team want's to build better code, but some warnings always be there as false positives. Now someone commits something and the warnings climb to 1010. We have to look, which of the 1010 are the new error, which are real warnings and which are false positives. After a few commits this number always will get bigger and bigger - this is not really satisfying for the programmers. But if you leave the number near zero, the programmer gets feedback like "Hey, there are THESE 10 new warnings from your code".
Of course you can simply add# excellent:ignoreto all your errors, but programmers, who are pragmatic enough, to use tools like Excellent, will think twice to do so.Yes, I see your point. But for situations like that I'm thinking about some "CI like thing" (it's still very vague) that only checks the files that have actually changed in the commit and will report warnings for that commit only. Then you might get maybe 30-40 warnings max for a bigger commit which is a number you can handle well even if you know 10 are false positives since you e.g. really don't want to validate anything in a specific model.
Again, see #2. I'll add config files to make it possible to configure which checks to run for
- all files in a directory where config files in subdirectories override configs in parent directories
- specific files
So if you really wanted to disable a check for a specific model, you would disable it for the file the model is defined in. This does not really have high prio right now however since there are some other tickets that are just more important (fixing bugs, adding specs etc). Patches welcome of course!
-
2 comments Created 7 months ago by simplabschecksxinternalsxAdd configuration filesmarcoowxConfiguration files should be added that let the user control which checks to apply for which elements. Maybe that should be similar to the way reek does it.
Comments
I think more than one option to configure what checks are applied should be implemented. Actually the following make sense in my opinion:
- placing a config file in the directories that are scanned (where options in lower directories would override those in higher ones)
- having switches in the executable
- making it configurable in the rake task
-
1 comment Created 3 months ago by marcoowchecksxinternalsxDebug checks and parsermarcoowx -
It would be really cool to see the offending code fore each warning. Highlighting that code would also be very cool.
Comments
Since only Ruby syntax highlighting is needed, the syntax gem might be pretty good for this task: http://syntax.rubyforge.org
-
1 comment Created 3 months ago by carloModel checks complain about inherited `initialize` methodsinternalsxRunning Excellent over my Rails models yields the following results:
app/models/source.rb * Line 1: Source defines initialize method. app/models/thing.rb * Line 1: Thing defines initialize method. app/models/tweet.rb * Line 1: Tweet defines initialize method.These models are plain vanilla
ActiveRecord::Basedecendants. They don't have their owninitializemethods.Comments
-
Looks like all output is threshold based. It would be nice if there were also a way to get a report of the actual numbers.
I was about to release my own basic loc (lines of code) project when I discovered Excellent. Rather the release yet another narrow ruby metrics gem, I'd much rather contribute my work to Excellent. I don't see any overall loc check in Excellent now. I could make it so the threshold would be reasonable ratios between lines of comments, lines of code and (where possible) lines of test code.
Also I'd love to add a charting capability to the html formatter. Bar graphs could be used for most metrics, and other figures, like sloc figures, make for great pie charts. Beyond that, maybe history graphs like metric_fu has which can graph changes over time, which is pretty cool.
So if you don't mind I would like to contribute to the project.
BTW, it's very nice to see such well written code.
Comments
Cool, of course you're very welcome to contribute!
It woulc require some refactoring though to add a loc counter that counts ALL lines (e.g. sum of all lines in all files that are parsed) since the current implementation is really file and element based (class, method). It should still be possible though to extend Excellent in that way.
I already thought about graphs etc. A good way to do that might be something like http://raphaeljs.com/ where the complete graph creation could take place in the HTML formatter.
Great to have someone willing to contribute!
Okay, I did some research into the possibility of counting things like comments. Ideally it looks like the solution would be to switch from RubyParser to Ripper --the official parser shipped with Ruby 1.9. Ripper has a scanner mode which can be user to analyze source character by character. Unfortunately it doesn't look like Ripper can be made to work with 1.8. If that is indeed the case, I suppose the only option right now is to add a separate loc parser. Thoughts?
Love raphaeljs.com btw, I was thinking google chart api, but raphael is much cooler.
-
0 comments Created 2 months ago by marcoowmarcoowxcombine current parser and LOCParserparserxIt should be possible to combine the parser, e.g. parse line counts of the file first, then parse the actual code.
Comments
-
1 comment Created 2 months ago by marcoowinternalsxAdd specs for the code processor and the conextsspecsxThe contexts and the code processor need some specs. They are not really reliable and produce some false findings, bad
#full_names etc.Comments
-
To be able to cleanly produce statistics besides the mere warnings, I think it would make sense to refactor the warnings so that
FileInfo(or whatever they may be called) objects are returned by the runner, which have a list of warnings as well as a list of stats (line count etc.) These descriptors would then be passed to the formatters.Comments
-
It could make sense to switch to sydparse (http://gemcutter.org/search?query=sydparse&commit=Search).
sydparse is the parser used by rubinius. I assume that sydparse will or already does support Ruby 1.9 since Rubinius of coruse wants to support 1.9 syntax. It's (for the most part) implemented in C, so it will be significantly faster than ruby_parser. The big question is how much refactoring would be neccessary to make Excellent work with sydparse.
Comments
Would also be great to reuse Rubinius' Melbourne parser (http://github.com/evanphx/rubinius/blob/master/lib/melbourne.rb) to generate an AST from sydparse's output. That would remove the need for excellent's own implementation (which is actually the most important and most fragile part at the same time).
Have to check if that's possible and if it could be done without license issues.
-
0 comments Created 2 months ago by marcoowmarcoowxMethod that take blocks should not count the block as a parameterparserxBlock parameters are currently counted as regular parameters. A method like this
def method(param, &block) endwould currently be treated as having 2 parameters. I think it should count as 1 parameter only.
Comments





done, text and html formatters should be enough for now