# 21 bugs in 21st version of Apache NetBeans 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\. ![1117_NetBeans_check/image1.png](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 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 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/)\.