21 bugs in 21st version of Apache NetBeans
Tags: java, programming
Published: 2024-04-19 08:34:49

Apache NetBeans is one of the first IDEs for Java\. It has been supported for nearly 30 years\. Recently, Apache NetBeans 21 was released\. We've decided to check the source code of such a long\-lived project and have chosen the most curious errors\. Let's follow them up in this article\.

![](https://import.viva64.com/docx/blog/1117_NetBeans_check/image1.png)

## What are we checking and how?

One of the ways to check the program source code is to use a static analyzer\. This is a special tool that takes source code files as input and outputs the result of the check — warnings about code fragments that may contain errors\. Our company develops such a tool called PVS\-Studio\.

Using the analyzer on large projects with a vast code base is particularly interesting, especially when we can't fully review the code\. Apache NetBeans is an open\-source IDE for Java that was first released in 1997\. The latest version of this IDE, the 21st, was released recently\. 

So, we've decided to check [Apache NetBeans](https://github.com/apache/netbeans/tree/21?tab=readme-ov-file) and break down 21 errors that the analyzer found in the source code\. We've divided all the errors into small groups based on their specifics\.

## Copy but check

Let's start with copy\-paste errors\. When we copy the code, it's easy to forget to change just single character\. As a result, it becomes the reason why the program doesn't operate correctly\.



```cpp
private void refresh(Node[] newSelection) {
  ....
  if(entry1 != null && entry2 != null && file1 != null && file2 != null) {
    if(entry1.getDateTime().getTime() > entry1.getDateTime().getTime()) {
      refreshRevisionDiffPanel(entry1, entry2, file1, file2);
    } else {
      refreshRevisionDiffPanel(entry2, entry1, file2, file1);
    }
    return;
  }
  ....
}
```



Look at the second *if* condition: it compares two identical objects with each other:



```cpp
entry1.getDateTime().getTime() > entry1.getDateTime().getTime().
```

The analyzer issues the following warning for the code fragment: 

[V6001](https://pvs-studio.com/en/docs/warnings/v6001/) There are identical sub\-expressions 'entry1\.getDateTime\(\)\.getTime\(\)' to the left and to the right of the '\>' operator\. HistoryDiffView\.java\(130\)

Here's another error of the same type:

```cpp
private RevisionInterval getResortedRevisionInterval(SVNRevision revision1, 
                                                     SVNRevision revision2) {
  RevisionInterval ret; 
  if(revision1.equals(SVNRevision.HEAD) && 
     revision1.equals(SVNRevision.HEAD)) {
    ....
  }
  return ret;
}
```

The method is used with the SVN version control system to evaluate the interval between two revisions: *revision1 *and *revision2*\. The logical condition checks if the revision is one of the most recent \(*SVNRevision\.HEAD*\)\. However, developers may mess up and compare only the first revision instead of both revisions\.

The PVS\-Studio warning:

[V6001](https://pvs-studio.com/en/docs/warnings/v6001/) There are identical sub\-expressions 'revision1\.equals\(SVNRevision\.HEAD\)' to the left and to the right of the '&&' operator\. RevertModifications\.java\(387\), RevertModifications\.java\(387\)

Let's move on:

```cpp
public boolean paintDragFeedback(....)
{
  ....
  if (x1 >= x2) {
    x1 = contInsets.left;
    x2 = contSize.width - contInsets.right;
    if (x1 >= x2) return true; // container is too small
  }
  if (y1 >= y2) {
    y1 = contInsets.top;
    x2 = contSize.height - contInsets.bottom;                // <=
    if (y1 >= y2) return true; // container is too small
  }
  ....
}
```

Take a closer look at the two *if* blocks: the first handles the *x* coordinate, the second handles the *y* coordinate\. However, in the second block, devs forgot to change *x2* to *y2*\. As a result, the entire block operates incorrectly\. The analyzer issues the following warning:

[V6080](https://pvs-studio.com/en/docs/warnings/v6080/) Consider checking for misprints\. It's possible that an assigned variable should be checked in the next condition\. BorderLayoutSupport\.java\(264\), BorderLayoutSupport\.java\(263\)

And the next one:

```cpp
public void preferenceChange(PreferenceChangeEvent evt) {
  String k = evt == null ? null : evt.getKey();  
  ....
  try {
    if (k == null || k.equals(FoldUtilitiesImpl.PREF_OVERRIDE_DEFAULTS)) {
     ....
    }
    if (k == null || k.equals(SimpleValueNames.CODE_FOLDING_ENABLE)) {
     ....
    } 
    if (k == null || FoldUtilitiesImpl.PREF_CONTENT_PREVIEW.equals
                              (FoldUtilitiesImpl.PREF_CONTENT_PREVIEW)) {
      ....
    }
    if (k == null || FoldUtilitiesImpl.PREF_CONTENT_SUMMARY.equals
                              (FoldUtilitiesImpl.PREF_CONTENT_SUMMARY)) {
      ....
    } 
    ....
  } 
  ....
}
```

Here, in the third and fourth *if* blocks in the condition, the *FoldUtilitiesImpl\.PREF\_CONTENT\_PREVIEW* and *FoldUtilitiesImpl*\.*PREF\_CONTENT\_SUMMARY* strings are compared to themselves, not to the *k* string\. It means that the written actions in these blocks will always be performed, even when not required\. There are analyzer warnings for these two lines:

[V6007](https://pvs-studio.com/en/docs/warnings/v6007/) Expression 'FoldUtilitiesImpl\.PREF\_CONTENT\_PREVIEW\.equals\(FoldUtilitiesImpl\.PREF\_CONTENT\_PREVIEW\)' is always true\. FoldOptionsPanel\.java\(177\)

[V6007](https://pvs-studio.com/en/docs/warnings/v6007/) Expression 'FoldUtilitiesImpl\.PREF\_CONTENT\_SUMMARY\.equals\(FoldUtilitiesImpl\.PREF\_CONTENT\_SUMMARY\)' is always true\. FoldOptionsPanel\.java\(180\)

Let's take a look at another error:

```cpp
static void createJfxExtension(Project p, FileObject dirFO, 
                               WizardType type) throws IOException {
  FileObject templateFO = FileUtil.getConfigFile("Templates/JFX/jfx-impl.xml"); 
  if (templateFO != null) {
    ....
    if (type == JavaFXProjectWizardIterator.WizardType.SWING) {
      ....
      FileObject swingTemplateFO1 = FileUtil.getConfigFile(....); 
      if (swingTemplateFO1 != null) {
        FileUtil.copyFile(swingTemplateFO1, 
                          templatesFO, "FXSwingTemplate"); 
      }
      FileObjectswingTemplateFO2=FileUtil.getConfigFile(....); 
      if (swingTemplateFO1 != null) {
        FileUtil.copyFile(swingTemplateFO2, 
                          templatesFO, "FXSwingTemplateApplet"); 
      }
      FileObject swingTemplateFO3 = FileUtil.getConfigFile(....); // NOI18N
      if (swingTemplateFO1 != null) {
        FileUtil.copyFile(swingTemplateFO3, 
                          templatesFO,"FXSwingTemplateApplication"); 
      }
    }
    JFXProjectUtils.addExtension(p);
  }
}
```

In this example, the method describes instructions for opening three different configuration files and then copying them into one\. It may seem like everything is OK: before each copy, the file is checked to see whether it's opened or not\. But only *swingTemplateF01* is checked all the time\. If *swingTemplateF02* or *swingTemplateF03* can't be opened, it may cause an error and the program crash\. 

Here's the analyzer warning: 

[V6080](https://pvs-studio.com/en/docs/warnings/v6080/) Consider checking for misprints\. It's possible that an assigned variable should be checked in the next condition\. JFXProjectUtils\.java\(768\), JFXProjectUtils\.java\(769\)

That's not all\.

```cpp
public Object getElementAt(int index) {
  ....
  int myMinIndex = getExternal (minIndex) + 1; 
  // one after the index of the first non-1 index
  int myMaxIndex = getExternal (maxIndex);

  assert myMaxIndex >= myMaxIndex : "Must be greater"; // NOI18N
  ....
}
```

A copy\-paste error may impede not only program operation but also the debugging process at the development stage\. In this part of the code, the *myMaxIndex \>= myMaxIndex* condition is written in the assert instead of *myMaxIndex \>= myMinIndex*\. 

These indexes are used to fill the array range with a specific value\. Since the wrong expression is written in *assert*, an error in the program logic that causes *myMaxIndex* to be less than *myMinIndex* won't be detected in advance*\.* Correctly written assert conditions reduce the time needed to find and debug errors\. 

The PVS\-Studio warning:

[V6001](https://pvs-studio.com/en/docs/warnings/v6001/) There are identical sub\-expressions 'myMaxIndex' to the left and to the right of the '\>=' operator\. FilteredListModel\.java\(319\)

Let's go to the next fragment:

```cpp
private void mergeParallelInclusions(List<IncludeDesc> inclusions, 
                                     IncludeDesc original,
                                     boolean preserveOriginal) {
  IncludeDesc best = null; 
  ....
  // 2nd remove incompatible inclusions, move compatible ones to same level
  for (Iterator it=inclusions.iterator(); it.hasNext(); ) {
    IncludeDesc iDesc = (IncludeDesc) it.next();
    if (iDesc != best) {
      if (!compatibleInclusions(iDesc, best, dimension)) {
        it.remove();
      } else if (iDesc.parent == best.parent && 
                 iDesc.neighbor == best.neighbor && 
                (iDesc.neighbor != null || iDesc.index == iDesc.index)) {  // <=
        it.remove(); // same inclusion twice (detect for better robustness)
      }
      ....
    }
    ....
  }
  ....
  if (unifyGaps != null) {
    // unify resizability of the border gaps collected for individual inclusions
    for (LayoutInterval[] gaps : unifyGaps) {
      int preferredFixedSide = 
                         fixedSideGaps[LEADING] >= fixedSideGaps[TRAILING] ? 
                                                   LEADING : TRAILING;
      for (int i=LEADING; i <= TRAILING; i++) {
        if (LayoutInterval.canResize(gaps[i]) && !anyResizingNeighbor[i]
            && (anyResizingNeighbor[i^1] || preferredFixedSide == i)) {
          operations.setIntervalResizing(gaps[i], false);
          if (!LayoutInterval.canResize(gaps[i^1])) {
            operations.setIntervalResizing(gaps[i^i], true);       // <=
          }
          break;
        }
      }
    }
  }
}
```

This method has almost 1500 lines of code\. This size spells out why some bugs haven't been detected and fixed right away\. 

First, let's look at the conditions described in the first marked loop on the collection\. Inside the body, the received *iDesk* object is compared with the *best* object\. In one of the comparison conditions, all fields of the object are used\. However, a developer was slightly wrong and wrote *iDesc\.index == iDesc\.index* instead of *iDesc\.index == best\.index*\.

Now let's look at the code fragments from the second loop\. Pay attention to the index definition of the array element in the *if \(\!LayoutInterval\.canResize\(gaps\[i^1\]\)\)* line\. As an index in the *gaps* array, the result of the exclusive OR operation of entities with the *i* index is used\. However, the next line contains an error in using this binary operation: *operations\.setIntervalResizing\(gaps**\[i^i\]**, true\)*\. Here, the expression results in zero because the exclusive OR operator is performed on the same element\. If zero was meant, it would have been more logical to write it immediately\.

The analyzer warning:

[V6001](https://pvs-studio.com/en/docs/warnings/v6001/) There are identical sub\-expressions 'iDesc\.index' to the left and to the right of the '==' operator\. LayoutFeeder\.java\(3660\)

[V6001](https://pvs-studio.com/en/docs/warnings/v6001/) There are identical sub\-expressions 'i' to the left and to the right of the '^' operator\. LayoutFeeder\.java\(3897\)

## Too idle to write methods from scratch

All previous examples were about the copy\-paste of code lines one by one\. In the next fragments, devs copied whole methods\!

```cpp
public final class EditorMimeTypesImpl 
             implements EditorMimeTypesImplementation {

  private final PropertyChangeSupport listeners;

  @Override
  public void addPropertyChangeListener(@NonNull final 
                                    PropertyChangeListener listener) {
    Parameters.notNull("listener", listener);   //NOI18N
    listeners.addPropertyChangeListener(listener);
  }

  @Override
  public void removePropertyChangeListener(@NonNull final 
                                    PropertyChangeListener listener) {
    Parameters.notNull("listener", listener);   //NOI18N
    listeners.addPropertyChangeListener(listener);
  }
}
```

Take a look at these two methods\. They interact with the *listeners* object, and inside it, a collection is organized\. The first method is used to add an item and the second method is used to delete an item\. However, inside the *removePropertyChangeListener* method, its body completely duplicates the *addPropertyChangeListener* method\. Deleting items from this collection is impossible in this case\. 

The PVS\-Studio analyzer issues the following warning: 

[V6032](https://pvs-studio.com/en/docs/warnings/v6032/) It is odd that the body of method 'addPropertyChangeListener' is fully equivalent to the body of another method 'removePropertyChangeListener'\. EditorMimeTypesImpl\.java\(63\), EditorMimeTypesImpl\.java\(69\)

This is not the first case:

```cpp
public class MethodParamsTipPaintComponent extends JToolTip {
  protected int getWidth(String s, Font font) {
    if (font == null) return fontMetrics.stringWidth(s);
      return getFontMetrics(font).stringWidth(s);
  }

  protected int getHeight(String s, Font font) {
    if (font == null) return fontMetrics.stringWidth(s);
      return getFontMetrics(font).stringWidth(s);
  }
}
```

There are two other methods, and one of them has been completely copied\. In the *getHeight* method, we get the same value as in the *getWeight* method\.

The PVS\-Studio warning:

[V6032](https://pvs-studio.com/en/docs/warnings/v6032/) It is odd that the body of method 'getWidth' is fully equivalent to the body of another method 'getHeight'\. MethodParamsTipPaintComponent\.java\(121\), MethodParamsTipPaintComponent\.java\(126\)

## Shoot yourself in foot with null reference 

Here we'll wrap copy\-paste errors up and move on to another type of errors: incorrect reference handling\.

Sometimes a dev accesses a reference before checking it for *null*\. Look at this case:

```cpp
public void setAllData(String key, String[] data) {
  ....
  // remove superfluous data
  if (getEntryCount() > data.length/3) {
    for (int j=0; j < getEntryCount(); j++) {
      PropertiesFileEntry pfe = getNthEntry(j);
      PropertiesStructure ps = pfe.getHandler().getStructure();
      if (pfe == null || ps == null) continue;
      ....
    }
  }
}
```

Here, the *pfe* object is obtained, its method is called, and only then we see the check for *null*\.

The analyzer issues the following warning:

[V6060](https://pvs-studio.com/en/docs/warnings/v6060/) The 'pfe' reference was utilized before it was verified against null\. BundleStructure\.java\(423\), BundleStructure\.java\(424\)

Let's continue:

```cpp
public void run() {
  for (SourceJavadocAttacherImplementation.Definer d : plugins) {
    ....
    List<? extends URL> s = source ?
      d.getSources(root, this) : d.getJavadoc(root, this);
    if (s != null || s.isEmpty()) {
    ....
    }
  }
}
```

Here, a programmer incorrectly used logical operations, using *\|\|* instead of *&&*\. The *s* list will be checked for missing items in it only if there is no reference to the list\. Unlike the previous example, the static analyzer identifies the error as the following warning:

[V6008](https://pvs-studio.com/en/docs/warnings/v6008/) Null dereference of 's'\. SourceJavadocAttacherUtil\.java\(141\)\.

Here's another similar error in the reference logic:

```cpp
public void propertyChange(PropertyChangeEvent evt) {
  ....
  synchronized (this) {
    artifacts = null;
    if (listeners == null && listeners.isEmpty()) {
      return;
    }
    ....
  }
}
```

If the reference to *listeners* is *null*, the *listeners* list is checked to ensure that there are no items inside\.

The analyzer issues the warning:

[V6008](https://pvs-studio.com/en/docs/warnings/v6008/) Null dereference of 'listeners'\. MavenArtifactsImplementation\.java\(613\)

Let's keep going:

```cpp
private SourcesModel getModel () {
  SourcesModel tm = model.get ();
  if (tm == null) {
    tm.sourcePath.removePropertyChangeListener (this);
    tm.debugger.getSmartSteppingFilter ().
    removePropertyChangeListener (this);
  }
  return tm;
}
```

In this code fragment, the null reference error reaches a new level\. The null reference is dereferenced when we're sure that the reference is exactly *null*\. Moreover, this function is used in 136 places\! Obviously, the developer didn't plan to implement exactly this behavior of the program\. If such an error isn't noticed immediately, the *tm* reference is almost never null\. However, we can't deny that some planned functionality isn't still executed\.

The PVS\-Studio warning:

[V6008](https://pvs-studio.com/en/docs/warnings/v6008/) Null dereference of 'tm'\. SourcesModel\.java\(713\)

The project has another exactly the same error:

```cpp
void unsetStepSuspendedByBpIn(JPDAThreadImpl thread) {
  JPDABreakpoint oldBp;
  synchronized (stepBreakpointLock) {
  ....
    if (this.suspendedSteppingThreads == null) {
      this.suspendedSteppingThreads.remove(thread);
      ....
    }
  }
  ....
}
```

In this fragment, the *thread* reference is deleted from a collection that doesn't exist\.

The analyzer issues the warning:

[V6008](https://pvs-studio.com/en/docs/warnings/v6008/) Null dereference of 'this\.suspendedSteppingThreads'\. JPDAThreadImpl\.java\(2329\)

Null dereference errors are pretty common in this project\. So, let's break down a few more\.

```cpp
public @Override Element getElement() {
  if (view != null) {
    return view.getElement();
  } else {
    return view.getDocument().getDefaultRootElement();
  }
}
```

Something odd occurs here: the *view* reference will be dereferenced, no matter if it's *null* or not\. 

The analyzer warning:

[V6008](https://pvs-studio.com/en/docs/warnings/v6008/) Null dereference of 'view'\. CollapsedView\.java\(627\)

The next error is more riveting\. Here, devs didn't forget to check for *null*, but unfortunately, they made another mistake:

```cpp
public ModulePathsProblemsProvider(@NonNull final Lookup baseLkp) {
  this.project = baseLkp.lookup(J2SEProject.class);
  if (this.project == null) {
    throw new IllegalArgumentException(String.format(
       "Unsupported project: %s of type: %s",  //NOI18N
        project,
        project.getClass()
       ));
  }
  this.moduleInfoListeners = new HashSet<>();
  this.listeners = new PropertyChangeSupport(this);
}
```

The *project* reference is dereferenced when we try to get the information to throw an exception that the *project* reference is null\. They wanted the best of their code but not today\.

The PVS\-Studio warning:

[V6008](https://pvs-studio.com/en/docs/warnings/v6008/) Null dereference of 'project'\. ModulePathsProblemsProvider\.java\(93\)

The code fragments that guaranteeing to throw a *NullPointerException* do not end here\.

```cpp
public static void main(String args[]) throws Exception {
  ....
  Bridge lex = null; //new XMLSyntaxTokenManager(input);
  int i = 25; //token count
  int id;
  int toks = 0;
  long time = System.currentTimeMillis();
  while (i/*--*/>0) {

    lex.next();
    ....
  }
}
```

A null reference to an object of the *Bridge* class is created, and then it's dereferenced\. The most surprising thing is that the code snippet that initializes this reference has been commented out\. As we can see by the flood of comments, the method has damaged a lot after the edits\.

The analyzer resents and issues a warning:

[V6008](https://pvs-studio.com/en/docs/warnings/v6008/) Null dereference of 'lex'\. XMLSyntaxTokenManager\.java\(216\)

## Error in reverse

Now let's move on to the more unusual bugs for this project\.

```cpp
private void changeIgnoreStatus (File f) throws IOException {
  File parent = f;
  boolean isDirectory = f.isDirectory() && (! Files.isSymbolicLink(f.toPath()));
  StringBuilder sb = new StringBuilder('/');
  if (isDirectory) {
    sb.append('/');
  }
  boolean cont = true;
  while (cont) {
    sb.insert(0, parent.getName()).insert(0, '/');
    parent = parent.getParentFile();
    String path = sb.toString();
    if (parent.equals(getRepository().getWorkTree())) {
      if (addStatement(new File(parent, Constants.DOT_GIT_IGNORE), 
          path, isDirectory, false) &&
          handleAdditionalIgnores(path, isDirectory)) {
        addStatement(new File(parent, Constants.DOT_GIT_IGNORE),
                              path, isDirectory, true);
      }
      cont = false;
    } else {
      cont = addStatement(new File(parent, Constants.DOT_GIT_IGNORE), 
                                   path, isDirectory, false);
   }
}
```

The '*/'* character is passed to the *StringBuilder* constructor\. There doesn't seem to be anything wrong here: devs create the StringBuilder instance and immediately add a character to it\. However, Java sees this character as a number\. As a result, instead of receiving the initial character as a slash, *sb *reserves memory space for 47 characters, and 47 corresponds to the '*/'* character in the ASCII table\.

Now we're coming to the best part\. This code generates the absolute path to the *f* file\. If the slash is specified as a string in the *sb* constructor, it'll be the initial character\. Then the path is written to the beginning of this string, thus leaving a slash at the end\. Based on this algorithm, we'd get the following paths:

* "*/folder1/folder2/f/*" for file;
* "*/folder1/folder2/f//*" for folder\.

It isn't hard to see that in both cases there would be an extra slash at the end\. However, this doesn't happen, because the slash doesn't influence anything in the *StringBuilder* constructor\. It turns out that the code isn't properly executing, and that's why the string is generated correctly\! The PVS\-Studio analyzer issues the following warning: 

[V6009](https://pvs-studio.com/en/docs/warnings/v6009/) Buffer capacity is set to '47' using a char value\. Most likely, the '/' symbol was supposed to be placed in the buffer\. IgnoreUnignoreCommand\.java\(107\)

## Danger code\. Crash is inevitable

I suggest you look at the "unstoppable" code: 

```cpp
private class BrowserOutlineView extends OutlineView {
  ....
  public void startEditingAtPath(TreePath path) {
    startEditingAtPath(path);
  }
  ....
}
```

If you want the program to crash all the time, write recursion without the exit condition\. 

The analyzer warning:

[V6062](https://pvs-studio.com/en/docs/warnings/v6062/) Possible infinite recursion inside the 'startEditingAtPath' method\. BrowserPanel\.java\(165\), BrowserPanel\.java\(166\)

Besides, such recursion occurs more than once in the source code:

```cpp
public class SchemaReferenceProxy extends SchemaReference 
                                  implements AXIComponentProxy {
  ....
  private SchemaReference getShared() {
    return (SchemaReference)getShared();
  }
  ....
}
```

If you examine the source code more closely, you'll find that the *getShared* method is called in all methods of the *SchemaReferenceProxy* class\. The class is used when working with the XML files\. So, if you're actively working in Apache NetBeans with the XML files, and you suddenly have a program crash, you know the reason\.

The analyzer issues the warning:

[V6062](https://pvs-studio.com/en/docs/warnings/v6062/) Possible infinite recursion inside the 'getShared' method\. SchemaReferenceProxy\.java\(40\), SchemaReferenceProxy\.java\(41\)

## Sum it up

We've checked 21 bugs in the Apache NetBeans source code\. We stop here, but many suspicious places are still left unexplored\. There were some simply weird things happened\. For example, there are many places where triple negation has been used, like in *\!\!\!it\.hasNext\(\)*\. It's not clear why this would be done in Java :\)

You should understand that Apache NetBeans is a project with the huge source code base — it's simply impossible to find and fix all the errors manually\. That's why when developing a project at such a high level, it'd be better resort to the help of various tools, such as a static analyzer\.

If you're interested in checking other IDEs, we recommend reading the related articles:

* [Operation K\. Looking for bugs in the IntelliJ IDEA code](https://pvs-studio.com/en/blog/posts/java/1089/) 
* [Qt Creator\* in search of Qt Creator bugs](https://pvs-studio.com/en/blog/posts/cpp/1099/)
* [PVS\-Studio vs CodeLite: a battle for the perfect code](https://pvs-studio.com/en/blog/posts/cpp/1065/)
* [Any bugs in your IDE? Checking AvalonStudio with PVS\-Studio](https://pvs-studio.com/en/blog/posts/csharp/0966/) 

If you're interested in the PVS\-Studio static analyzer and would like to check your project, here's a special trial version for you\. Try it [here](https://pvs-studio.com/en/pvs-studio/try-free/?utm_source=website&utm_medium=devto&utm_campaign=article&utm_content=1117)\.