FindBugs 3.0.1 released

The new version of FindBugs is available for download on the official website. Despite the fact that only the third digit in the version number has changed, you will find many new interesting detectors, as well as the improvement of the old ones. If the main feature 3.0.0 was to support Java 8 and there were practically no new detectors, then in 3.0.1 the emphasis was on functionality. Here I want to briefly highlight some of the new detectors developed by me personally.

In this article we confine ourselves to detectors of useless code. I like to develop such detectors: they are not just looking for a specific pattern (for example, calling a method with a knowingly incorrect parameter), but they prove the futility of certain code fragments. Sometimes this way you can find very unexpected errors.

UC_USELESS_CONDITION

I wrote about this detector separately , but since the time of that article it has been significantly improved. Now fully processed variable variables, register reuse, transfer of values ​​from one variable to another, calls to String.length (), unboxing, checking the lengths of arrays. Also partially supported fields. Improved priorities. This detector allows you to find very interesting bugs from the series “How did it happen that we didn’t notice this for years?” For example, this code was found in PMD 5.2.3 (this is also a tool to check the quality of the code!):

protected boolean isHexCharacter(char c) { return isLatinDigit(c) || ('A' <= c || c <= 'F') || ('a' <= c || c <= 'f'); } 

Due to erroneous use || instead of &&, the condition is true for any characters (the method always returns true). The bug exists at least from version 5.0.0 (no further digging) - almost three years.

I note, by the way, that some do not see such mistakes, even if they look directly at them. Here another developer FindBugs thought that on the condition c != '\n' || c != '\r' c != '\n' || c != '\r' there was a false positive and even came up with a clever theory about the coincidence of the character code and the register number with the variable. But in fact, the triggering is not false, the code is really with an error.
')
Sometimes there is an error in copy-paste. For example, this was found in Lucene 4.10.3:

 final int bitsPerStoredFields = fieldsStream.readVInt(); if (bitsPerStoredFields == 0) { ... } else if (bitsPerStoredFields > 31) { throw new CorruptIndexException("bitsPerStoredFields=" + bitsPerStoredFields + " (resource=" + fieldsStream + ")"); } else { ... } final int bitsPerLength = fieldsStream.readVInt(); if (bitsPerLength == 0) { ... } else if (bitsPerStoredFields > 31) { //  UC_USELESS_CONDITION throw new CorruptIndexException("bitsPerLength=" + bitsPerLength + " (resource=" + fieldsStream + ")"); } else { ... } 

FindBugs now sees that with bitsPerStoredFields> 31, we should have already dropped out with an exception, so the second time such a check is meaningless. Apparently, the authors copied a piece of code and forgot to fix it at bitsPerLength.

UC_USELESS_OBJECT

This message is displayed if an object or an array is created in the method and some manipulations are performed with it that do not give any side effects, except for a change in the state of this object. There is a similar detector in the third-party plugin FB-contrib (WOC_WRITE_ONLY_COLLECTION_LOCAL) - it finds collections that only record in. Our detector can find more tricky situations and handle some user objects. Usually there is an unnecessary code that remains after refactoring and can simply be deleted. But sometimes the operation of the detector signals some deep problems in the algorithm. A simple example with arrays from IDEA:

 final int[] dx = new int[componentCount]; final int[] dy = new int[componentCount]; for (int i = 0; i < componentCount; i++) { final RadComponent component = myDraggedComponentsCopy.get(i); dx[i] = component.getX() - dropX; dy[i] = component.getY() - dropY; } 

The dx and dy arrays are filled, but they are not used anywhere else (probably used before). This whole cycle is not needed and can be deleted.

And here, in Eclipse, the algorithmic error is more likely:

 public void setProjectSettings(IJavaProject project, String destination, String antpath, String[] hrefs) { ProjectData data= fPerProjectSettings.get(project); if (data == null) { data= new ProjectData(); // UC_USELESS_OBJECT } data.setDestination(destination); data.setAntpath(antpath); StringBuffer refs= new StringBuffer(); ... data.setHRefs(refs.toString()); } 

ProjectData is a POJO class, and FindBugs is able to see that its setters do not change the global state of the system, modifying only the object itself. The standard execution path (without calling in if) does not cause questions. But if we went into if, then the further part of the method does not make sense, since the new object is not saved anywhere. Apparently, it was supposed after creation to save it in Map fPerProjectSettings.

RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT

The third of my serious development in this release. Maybe someday I'll write about it separately. A warning is issued when a method is called that has no side effects and its result is not used. False alarms are possible here, although they are quite rare and do not reduce the overall value of the warning.

Here is an example from IDEA:

 protected List<File> getFiles(@Nullable AntDomPattern pattern, Set<AntFilesProvider> processed) { ... if (singleFile != null && singleFile.isDirectory()) { Collections.singletonList(singleFile); // RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT } return Collections.emptyList(); } 

Obviously, inside the condition forgot to write return. If the return type of the called method is compatible with the return type of the current one, the priority of the warning is increased.

Another example from IDEA (not because there are a lot of mistakes, but because JetBrains employees read this post):

 protected void doOKAction() { SimpleNode node = getSelectedNode(); if (node instanceof NullNode) node = null; if (node != null) { if (!(node instanceof MavenProjectsStructure.ProjectNode)) { // RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT ((MavenProjectsStructure.MavenSimpleNode)node).findParent(MavenProjectsStructure.ProjectNode.class); } } myResult = node != null ? ((MavenProjectsStructure.ProjectNode)node).getMavenProject() : null; super.doOKAction(); } 

It seems they wanted to sign node = on the left.

A forgotten return or assignment is perhaps the most frequent reasons for this message. It also happens that a programmer calls a method, thinking that he is changing something, but he is not changing anything. For example, in Apache POI , the setBoolean method is called, reasonably suggesting that it installs something. And it simply calculates the new value and returns it (and who called the method just like that!).

It happens that the programmer called the wrong method that he wanted. So in FindBugs itself, we found and fixed the erroneous use of BitSet.intersects () (checks if two sets intersect) instead of BitSet.and () (crosses two sets, changing the first one).

UC_USELESS_VOID_METHOD

This is a consequence of the previous detector: since we learned quite well to determine whether a method has a side effect, we can report non-empty void methods that do not have such effects. Most often this is some unfinished code with a TODO in the middle, but you can find some really strange things. Here, for example, the useless method from Eclipse:

 private void pruneEmptyCategories(CheatSheetCollectionElement parent) { Object[] children = parent.getChildren(); for (int nX = 0; nX < children.length; nX++) { CheatSheetCollectionElement child = (CheatSheetCollectionElement) children[nX]; pruneEmptyCategories(child); } } 

A recursive traversal of the tree structure is performed and nothing else.

And here is another merge method of the SortedSet class in Eclipse:

Can you see that this 26-line method with three cycles and three if'ami doesn’t really do any good? FindBugs can now.

Conclusion

Update FindBugs, and you may find problems in your projects that you were not aware of before. The plugin for Eclipse is available on our website. The plugin for IDEA has also been updated. Do not forget to inform us about false positives and other problems found.