At Smart Bear, we use both FindBugs and PMD for static analysis and we still review every line of code. There are certain things that static analysis tools are great at and some things that they just cannot do. They can detect most bad idioms (like your DateFormat example). They can detect inconsistent code (you checked for null before dereferencing a parameter once, but not every time within a method), which probably indicates a bug. They can detect complex code (by computing cyclomatic complexity) and flag it for further review or refactoring.
They cannot reason about whether the code works, conforms to the spec, and is maintainable. In my mind, this is the primary function of code review. Here are some questions that go on my mental code review checklist:
1) Do the
unit tests validate the specified behavior?
2) Are there missing test cases?
3) Did the unit tests change? If so, was this an intentional change or a "get the unit tests to pass" change?
4) Can I understand the code?
5) Are variables well-named?
6) Are the comments sufficient?
7) Is there a better way? Especially a better way that makes use of some common code. Static analysis can detect duplicate code, but that's not exactly what I'm looking for here. I'm considering alternative approaches that would simplify the code by using some common library code.
For more thoughts:
Can static code analysis replace peer code review?