Using the IDEA Static Analysis tools

Making Effective Use of the IDEA Static Analysis Tools

The IntelliJ IDEA IDE contains some of the best static analysis tools that I've seen. This is clearly an extremely valuable tool. The real trick is using it well. It is no use to try to run all 500+ across a large project because it would take forever and the vast majority of the results would be useless. It seems like most of the metrics are there to help companies conform to various arbitrary coding standards, most of which I don't think are terribly useful. There are however some metrics that are extremely helpful and have flagged many honest-to-god bugs. I performed a fairly exhaustive survey of the 500+ code analysis tools included in IDEA, looking for the very best ones that seem to give high-quality, useful advice on potential problems in your code.

My goal here was to collect a concise list of the most useful and pertinent ones to use as a basis for both tracking overall code quality and as an aid for each developer to use to improve the code that they individually own. I've looked at all of IDEA's metrics and ran nearly half of them over a large project and examined the results. Some gave somewhat useful results but flagged far too many cases to reasonably examine, so I rejected those. Others sounded good but found no problems, so I left those out too. The 50+ listed below are the real gems that I found to be both pertinent and immediately useful.

Running just the metrics below on a large project will easily generate thousands of hits. About a third will be flagged as errors, and the rest as warnings. I highly suggest that examining all of them which is not as difficult as it sounds. Lots of them will be trivial to fix. When you run the analysis over your code and click on an individual results link, it will take you to the code in question, give a description of the problem, offer suggestions for fixing it, and often will present you with a "Problem Resolution" link you can click on and get IDEA to make the change for you. Be careful though as sometimes you will definitely not want to fix it the way that IDEA suggests, and other times the result is a false positive and you will not want to change it at all. That's fine but I think you should at least consider each potential problem that IDEA comes up with.

So what is IDEA's opinion of *your* code, you wonder? Here's how to find out:

The first thing you may want to do is to increase the maximum amount of RAM that you allow IDEA to use since some metrics are memory intensive. You may want to do that regardless of whether you run the analysis just to allow your development environment to run more smoothly. to do that on Windows, you want to edit the file named "idea.exe.vmoptions" that you will find in
C:\Program Files\JetBrains\IntelliJ IDEA 5.0\bin\
In there you will want to change the "mx" line to read something like
-Xmx500m

Next, you will want to drop this inspection configuration file into the appropriate location which for me is
C:\Documents and Settings\Melinda Green\.IntelliJIdea50\config\inspection
You may want to move your existing Default.xml file out of the way first in case you want to restore it later. I think that IDEA's defaults are rather poor, and I think you can always hit the "reset" button in the config editor too, but it's always good to be safe.

Next, launch IDEA and hit Analyze->Inspect Code. Select the inspection scope you are interested in. I found it most useful to create a custom scope for just the client code I was interested in, and you may want to create one that includes just your packages. Just click the "Custom scope" radio button and then click the "..." button to name your custom scope and to select which packages to include and exclude. Hint: you normally want to use the "recursive" options.

Once you select your scope and click "OK", analysis will begin. If you use the attached defaults and run it over the whole project, this could
take 15 minutes or more which is why it is good to limit your scope. Once completed, it should be fairly obvious how to navigate the tree of results it returns and to examine and fix most of the problems it flags in your code.

I encourage you to play around with creating various inspection profiles of your own and adjusting their parameters. Just click the "..." button next to the "Default" profile drop-down to see the profile editor. This page is where I have recently spent a *lot* of time. Clicking on the metric names in the left hand tree will cause a helpful description to be displayed on the right. Just check and uncheck various analysis metrics to modify the current profile. Use the "Copy..." button to easily create custom profiles for various classes of problems you are interested in.

Here at last is the list of enabled metrics that you should get from the attached profile along with brief notes of mine in square braces. Problems, questions, or feedback? Email me.

Enjoy!
-Melinda

Class Metrics
    - Overly complex class [Tune the "limit" option to find the top offenders for your search scope]

Class Structure
    - Field can be local [Highly recommended! Just click "Convert to local" to fix, or look to remove variable altogether]
    - Missing @deprecated annotation
    - Singleton [Avoided when possible, especially as a lazy-man's global! Doesn't even work when multiple class loaders operate]
    - Utility class without private constructor [Important protective measure for classes that are truly collections of static utilities]

Code Maturity
    - TODO comment [Not a problem but obsolete ones should be removed and valid ones need author initials]

Code Style Issues
    - expression.equals("literal") rather than "literal".equals(expression) [Removes a common source of NPEs]
      (Optional: Unnecessary code block [Sometimes improve readability but probably not for case statements, etc.])
      (Optional: Unnecessary parentheses [Sometimes improve readability but sometimes not.])
    - Unnecessary semicolon [Probably a typo]

Control flow issues
    - Conditionals that can be simplified to && or ||
    - Fallthrough in 'switch' statement [Found some real bugs!! Document any code intentionally falling through]
    - 'if' statement with negated condition [Inverting logic will almost certainly make it easier to understand]
    - Redundant 'if' statement ["if(condition) return true else return false"?? how about just "return condition"]
    - Redundant conditional expression

Data flow issues
    - Redundant local variable [Just take IDEA's suggestion unless variable substantially improves readability]
    - Scope of variable is too broad [Just keep clicking the "Narrow scope of Var" link until IDEA is satisfied]

Declaration Redundancy
      (Optional: Declaration access can be weaker [Verbose but useful to browse for those few great suggestions])
    - Declaration can have static modifier [This is a great one. Strongly consider accepting recommendations]
    - Empty method [Sometimes OK as a placeholder but useful to check and perhaps simply by removing method]
    - Redundant throws clause

Encapsulation issues
    - Package-visible field [Common programmer oversight. Make these private or document heavily why not!!]
    - Package-visible inner class [Exactly the same as above]
    - Protected field [Obviously intentional but probably better made private, or document heavily why not]
    - Protected inner class [Ditto]

General
    - Deprecated API usage [OK if code documented why intentionally used. Add warning suppression too?]

Inheritance issues
    - Abstract method overrides abstract method [Just redundant]
    - Constructor not 'protected' in 'abstract' class [Seems reasonable to protect abstract constructors]

Initialization issues
    - Abstract method call in constructor [Asking for trouble as parent not fully initialized when subclass is called]
    - Overridden method call in constructor [Not asking for trouble but found it just the same]
    - 'this' reference escaped in object construction [Flags cases where class implements listener instead of anon]
    - Unsafe lazy initialization of static field [Not thread-safe. Could be cause of past or lurking problems]

Method Metrics
    - Overly complex method [Tune the limit option to find the top offenders for your search scope]

Numeric issues
    - Integer division in floating point context

Performance issues
    - Field may be 'static'
    - Inner class may be static [Great one! Note such classes can be moved to own file if parent getting bloated.]
    - Map replaceable by EnumMap
    - Method may be 'static' [Great one! Fix or document why not]
    - Set replaceable by EnumSet
    - Single character string concatenation [While fixing, watch for "\n" and "/" cases (See Portability Issues below)]
    - Single character string parameter in 'String.indexOf()' call
    - '.size() > 0' replaceable by '.isEmpty()'
    - String concatenation as argument to StringBuffer.append() [Either ditch the buffer or chain the concatenations]
    - 'String.equals("")'

Portability Issues
    - Hardcoded file separator [Use File.separator when file paths are intended]
    - Hardcoded line separator [Almost always wrong. Use System.getProperty("line.separator") instead]

Probable Bugs
    - Constant conditions & exceptions
    - 'equals()' between objects of inconvertible types
    - Mismatched query and update of collection
    - Result of method call ignored
    - String comparison using '==', instead of '.equals()'

Serialization issues
    - Serializable class without serialVersionUID

Threading issues
    - Synchronization on a non-final field
    - 'synchronized' method [Look to narrow scope, remove altogether, or document why needed]

Verbose or redundant code constructs
    - Reduntant array creation
    - Redundant type arguments
    - Redundant type cast

Visibility issues
    - Method overloads method of superclass