 Inspired by Josh Bloch and Bill Pugh's Java Puzzlers talk at Google, Java Puzzlers, episode VI, I decided to use FindBugs and analyze some core Java libraries we wrote and used at one of my previous employments. Here are some of the findings:  
Commons: 675 classes, 505 bugs (98 bad practice, 27 correctness, 96 malicious code vulnerability, 14 multithreaded correctness, 207 performance, 63 dodgy).
Messaging: 31 classes, 21 bugs (9 bad practice, 8 malicious code vulnerability, 3 performance, 1 dodgy)
Services: 239 classes, 78 bugs (5 bad practice, 4 correctness, 35 malicious code vulnerability, 1 multithreaded correctness, 26 performance, 7 dodgy)
Content management: 637 classes, 577 bugs (38 bad practice, 14 correctness, 72 malicious code vulnerability, 3 multithreaded correctness, 382 performance, 68 dodgy)
Inspired by Josh Bloch and Bill Pugh's Java Puzzlers talk at Google, Java Puzzlers, episode VI, I decided to use FindBugs and analyze some core Java libraries we wrote and used at one of my previous employments. Here are some of the findings:  
Commons: 675 classes, 505 bugs (98 bad practice, 27 correctness, 96 malicious code vulnerability, 14 multithreaded correctness, 207 performance, 63 dodgy).
Messaging: 31 classes, 21 bugs (9 bad practice, 8 malicious code vulnerability, 3 performance, 1 dodgy)
Services: 239 classes, 78 bugs (5 bad practice, 4 correctness, 35 malicious code vulnerability, 1 multithreaded correctness, 26 performance, 7 dodgy)
Content management: 637 classes, 577 bugs (38 bad practice, 14 correctness, 72 malicious code vulnerability, 3 multithreaded correctness, 382 performance, 68 dodgy)
indexPrimary +=  Math.abs(this.getStoragePrimary()
  .hashCode()) + SEP;Integer.minValue() then the result will be negative as well.
Impossible cast (ouch!):ArrayList list=new ArrayList();
setChoices((DateDatum[])list.toArray());
internalConnetion:try {
  if(internalConnetion == null) {
    throw new TransactionManagerException("...");
  }
  ...
  } catch(Exception e) {
    throw new TransactionManagerException(
      e.getMessage(), e);
  } finally {
    if (internalConnetion.getDepth() < 1) {
if (accountId.equals(internalAccount)) {
  permissions.add(new AllPermission());
  return permissions;
}
if (accountId == null) {
  return permissions;
}
return new Boolean(false);Boolean.valueOf(...) instead.
Method invokes inefficient new String(String) constructor:
String path = new String("");
for(int i = 0; i < (hash.length / 2); i++) {
  rtnValue += Integer.toHexString(x);
}
Set keySet = headers.keySet();
Iterator iterator = keySet.iterator();
while(iterator.hasNext()) {
  String value = (String) headers.get(key);
}
public void setMethods(Hashtable methods) {
  this.methods = methods;
  FunctionsContainer.getLogger().info("Loaded " +
    methods.size()+" methods");
}
methods object into the internal representation of the object. Storing a copy of the object would have been much safer. 
It's always a good idea to statically analyse your code once you're done with it.  It's not going to render it bug free but it certainly helps. Oh, and while I'm at it, go get yourself a copy of Java Puzzlers book; it helps avoiding some very dark corners you might not have been aware of.
 

