[OPEN-ILS-DEV] Integrating automated code analysis tools into regular practice?

[Dan Scott]

On 22/11/2007, Scott McKellar <mck9 at swbell.net> wrote:
>
> --- Dan Scott <denials at gmail.com> wrote:
>
> > I was pondering a few possibilities for helping to tighten up our
> > code
> > in a more automated fashion today. I thought I would throw the ideas
> > out there to see if there's interest or support for them...
>
> <snip: info about code-scanning tools>
>
> Mod parent up.
>
> I have stumbled across various problems (security vulnerabilities,
> memory leaks, etc.) in the course of doing other things, but I
> haven't been looking for them systematically.  I've probably
> overlooked more than I've found, even if you don't count the code
> that I haven't looked at yet.  No doubt the automated tools can find
> more stuff faster.  It's the difference between working with a pick
> and shovel and working with a backhoe.
>
> Don't forget good old lint.  I haven't used it yet on this project
> because I've been finding enough to keep me busy with the pick-and-
> shovel approach.  However my experience is that lint usually finds
> at least a few forehead-slappers.
>
> My one reservation is about the idea of posting the results on the
> web.  The point is not that I don't want to air our dirty linens
> in public -- it's all open source, after all -- but I wouldn't want
> to erect needless barriers to the code scans.  If it takes ten
> minutes to run the scans, and three hours to update the website, then
> we probably won't run the scans as often as we should.  I'd rather
> have more scans than prettier web pages.
>
> Hence I suggest that any publication of the scan results involve
> minimal work, because the reporting is rudimentary, automated,
> or both.
>
> I note that Coverity's website already publishes defect counts on the
> projects it covers.

Hi Scott:

I wouldn't propose something that would take a lot of work - I'm lazy
that way :)

For tools like rats and Perl::Critic, we could simply run the tools
over the entire tree of OpenSRF and Evergreen, pipe the output into a
file, and post the file to the Web site. If we had an automated build
machine, we could make that a standard part of each build. This is
also where we would store the results of automated regression test
runs, if we, well, had regression tests :)

A moderately sophisticated approach would diff the output between
builds and flag new warnings, to focus attention on regressions or new
code that had been introduced.

A more sophisticated approach would shove each possible violation into
a database row, then let human eyes determine whether the warning was
valid or not; if not, then a brief rationale could be entered and the
warning could automatically be ignored in future runs (with some
degree of fuzziness for line number drift) and future developers would
be able to find a record of why that particular "violation" is
actually quite intentional and necessary.

But I would be happy with just the least sophisticated approach. The
hard part is where things like Perl::Critic flag coding practices that
verge towards style, e.g. coming down hard on the use of unless() and
postfix conditionals ($foo = $bar unless $bar = 0; and the like).  If
there is violent disagreement with cleaning up these "moderately
severe" violations, we can always just turn that particular rule off
in our profile.

Perl::Critic also provides nice summary statistics. Here's an example
of the summary for Evergreen trunk, with only the most severe
violations flagged:

190 files.
1,558 subroutines/methods.
43,568 statements.
55,421 lines of code.

Average McCabe score of subroutines was 4.37.

537 violations.
Violations per line of code was 0.010.

537 severity 5 violations.

21 violations of BuiltinFunctions::ProhibitStringyEval.
48 violations of InputOutput::ProhibitBarewordFileHandles.
47 violations of InputOutput::ProhibitTwoArgOpen.
12 violations of Modules::RequireBarewordIncludes.
6 violations of Modules::RequireFilenameMatchesPackage.
314 violations of Subroutines::ProhibitExplicitReturnUndef.
1 violations of Subroutines::ProhibitSubroutinePrototypes.
7 violations of TestingAndDebugging::ProhibitNoStrict.
47 violations of TestingAndDebugging::RequireUseStrict.
1 violations of ValuesAndExpressions::ProhibitLeadingZeros.
30 violations of Variables::ProhibitConditionalDeclarations.
3 violations of Variables::RequireLexicalLoopIterators.

(I know you're not a Perl hacker, but for those Perl hackers reading
this who haven't been exposed to Perl::Critic, more info on the
particular violations and the rationale for why they are violations
can be found for each violation at
http://search.cpan.org/dist/Perl-Critic/ and in even greater depth in
Damian Conway's "Perl Best Practices").

I have attached the RATS output in HTML format for the Evergreen tree
as well, for anyone interested in what it has to say.

-- 
Dan Scott
Laurentian University
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://list.georgialibraries.org/pipermail/open-ils-dev/attachments/20071122/1a84e345/rats-evergreen-0001.html