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 has changed in the version number, you will find many new interesting detectors, as well as an improvement in the old ones. If the main feature of 3.0.0 was support for Java 8 and there were practically no new detectors, then in 3.0.1 the emphasis was placed on functionality. Here I want to briefly highlight some of the new detectors that I personally developed.

    In this article, we restrict ourselves to useless code detectors. I like to develop such detectors: they do not just look for a specific template (for example, calling a method with a deliberately incorrect parameter), but they prove the futility of certain fragments of code. Sometimes in 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 variable variables are fully processed, reuse of the register, transfer of values ​​from one variable to another, calls to String.length (), unboxing, checking the lengths of arrays. Fields are also partially supported. Improved priorities. This detector allows you to find very interesting bugs from the series "How did it happen that we have not noticed this for years?" For example, such a code was found in PMD 5.2.3 (this is also a tool for checking 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 will always return true). The bug exists at least from version 5.0.0 (did not dig further) - almost three years.

    By the way, I note that some people don’t see such errors, even if they look directly at them. Here then another developer FindBugs thought that the condition c != '\n' || c != '\r'was a false alarm, and even came up with a clever theory about the coincidence of the character code and the register number of variable. But in fact, the operation is not false, the code is really with an error.

    Sometimes there is an erroneous 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 fallen out with an exception, so the second time this check is pointless. Apparently, the authors copied a piece of code and forgot to fix it on 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 changing the state of this object. A similar detector is in the third-party plugin FB-contrib (WOC_WRITE_ONLY_COLLECTION_LOCAL) - it finds collections that are written only. Our detector can find more tricky situations and process some user objects. Usually there is unnecessary code that remains after refactoring and can be simply deleted. But sometimes a detector triggering 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 populated, but are not used anywhere else (probably used before). This whole cycle is not needed, and it can be deleted.

    And here in Eclipse is already an algorithmic error:
    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 going into if) does not raise 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, after creating it was supposed to save it in Map fPerProjectSettings.

    RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT


    The third of my serious developments in this release. Perhaps someday I will 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 positives 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 getFiles(@Nullable AntDomPattern pattern, Set processed) {
      ...
      if (singleFile != null && singleFile.isDirectory()) {
        Collections.singletonList(singleFile); // RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT
      }
      return Collections.emptyList();
    }

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

    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 like they wanted to sign node =on the left.

    A forgotten return or assignment is perhaps the most common reason 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 POIs call the setBoolean method, reasonably assuming that it is installing something. And he simply calculates the new value and returns it (and whoever just called the method that way!).

    It happens that the programmer did not call the 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).

    UC_USELESS_VOID_METHOD


    This is a consequence of the previous detector: since we learned how to determine if a method has a side effect, we can report non-empty void methods that do not have such effects. Most often this is some kind of incomplete code with TODO in the middle, but it is possible to find truly strange things. Here, for example, is a 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:
    public void merge (SortedSet other)
    public void merge(SortedSet other) {
        Object[] array = fKeyHolder.getKeys();
        Object[] otherarray = other.fKeyHolder.getKeys();
        if (otherarray == null)
            return;
        if (array == null) {
            array = otherarray;
            return;
        }
        int ithis = 0, iother = 0, i = 0;
        int mthis = array.length, mother = otherarray.length;
        Object[] tmp = new Object[mthis + mother];
        while (ithis < mthis && iother < mother) {
            int comp = fComp.compare(array[ithis], otherarray[iother]);
            if (comp <= 0) {
                tmp[i++] = array[ithis++];
            } else {
                tmp[i++] = otherarray[iother++];
            }
        }
        while (ithis < mthis) {
            tmp[i++] = array[ithis++];
        }
        while (iother < mother) {
            tmp[i++] = otherarray[iother++];
        }
    }

    Can you notice that this 26-line method with three loops and three ifs doesn't really do any good? FindBugs can now.

    Conclusion


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

    Disclaimer
    No, I did not report all these errors to the developers. In the process of developing FindBugs, I test about 50 open source projects and find a total of thousands of new bugs in them (compared to the previous version of FindBugs). The time that I could spend on creating bug reports is better spent on improving FindBugs. In some of the projects under consideration, FindBugs is definitely used, so developers themselves will see new bugs when they are updated. If you want to help the community, you can always check any open project yourself using FindBugs and inform the project developers about the bugs found.

    Only registered users can participate in the survey. Please come in.

    What version of FindBugs are you using?

    • 4.3% I use 3.0.1 14
    • 10.1% I 'm using 3.0.0 (but will update soon) 33
    • 0.6% I use 3.0.0 (I do not plan to update) 2
    • 8.9% I'm using an older version 29
    • 6.7% FindBugs not using, I have another static Java code analyzer 22
    • 30.2% FindBugs not using, but I think to try 98
    • 4.3% FindBugs do not use, too lazy to bother 14
    • 8.6% I don’t use FindBugs, there are no errors in my Java code 28
    • 25.9% FindBugs do not use, I do not write in Java 84

    Also popular now: