SpotBugs Report

Project Information

Project: MicrosoftStreetside (spotbugsMain)

SpotBugs version: 4.0.3

Code analyzed:



Metrics

5424 lines of code analyzed, in 130 classes, in 17 packages.

Metric Total Density*
High Priority Warnings 8 1.47
Medium Priority Warnings 20 3.69
Low Priority Warnings 28 5.16
Total Warnings 56 10.32

(* Defects per Thousand lines of non-commenting source statements)



Contents

Summary

Warning Type Number
Bad practice Warnings 15
Correctness Warnings 5
Malicious code vulnerability Warnings 3
Multithreaded correctness Warnings 1
Performance Warnings 8
Dodgy code Warnings 24
Total 56

Warnings

Click on a warning row to see full context information.

Bad practice Warnings

Code Warning
Eq org.openstreetmap.josm.plugins.streetside.StreetsideCubemap defines compareTo(Object) and uses Object.equals()
HE org.openstreetmap.josm.plugins.streetside.StreetsideCubemap defines hashCode and uses Object.equals()
RV Exceptional return value of java.io.File.mkdirs() ignored in org.openstreetmap.josm.plugins.streetside.cache.Caches.getCacheDirectory()
RV Exceptional return value of java.util.concurrent.ExecutorService.submit(Runnable) ignored in org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.downloadCubemapImages(String)
Se org.openstreetmap.josm.plugins.streetside.StreetsideLayer$NearestImgToTargetComparator implements Comparator but not Serializable
Se Class org.openstreetmap.josm.plugins.streetside.actions.StreetsideWalkAction defines non-transient non-serializable instance field thread
Se Class org.openstreetmap.josm.plugins.streetside.gui.StreetsideImageDisplay defines non-transient non-serializable instance field detections
Se Class org.openstreetmap.josm.plugins.streetside.gui.StreetsideImageDisplay defines non-transient non-serializable instance field image
Se Class org.openstreetmap.josm.plugins.streetside.gui.StreetsideMainDialog defines non-transient non-serializable instance field image
Se Class org.openstreetmap.josm.plugins.streetside.gui.StreetsideMainDialog defines non-transient non-serializable instance field imageCache
Se Class org.openstreetmap.josm.plugins.streetside.gui.StreetsideMainDialog defines non-transient non-serializable instance field thumbnailCache
Se Class org.openstreetmap.josm.plugins.streetside.gui.StreetsideMainDialog$PauseAction defines non-transient non-serializable instance field thread
Se Class org.openstreetmap.josm.plugins.streetside.gui.StreetsideMainDialog$StopAction defines non-transient non-serializable instance field thread
Se Class org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ClipboardAction defines non-transient non-serializable instance field contents
Se org.openstreetmap.josm.plugins.streetside.model.UserProfile is Serializable but its superclass doesn't define an accessible void constructor

Correctness Warnings

Code Warning
NP Read of unwritten field singleImg in org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBox.loadSingleImageViewports()
RCN Nullcheck of reader at line 47 of value previously dereferenced in org.openstreetmap.josm.plugins.streetside.oauth.OAuthUtils.getWithHeader(URL)
UwF Unwritten field: org.openstreetmap.josm.plugins.streetside.StreetsideSequence.user
UwF Unwritten field: org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBox.singleImg
UwF Unwritten field: org.openstreetmap.josm.plugins.streetside.gui.StreetsideExportDialog$RewriteButtonAction.lastPath

Malicious code vulnerability Warnings

Code Warning
EI org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBox.getViews() may expose internal representation by returning CubemapBox.views
MS org.openstreetmap.josm.plugins.streetside.cubemap.CubemapUtils.directionConversion isn't final but should be
MS org.openstreetmap.josm.plugins.streetside.cubemap.CubemapUtils.rowCol2StreetsideCellAddressMap isn't final but should be refactored to be so

Multithreaded correctness Warnings

Code Warning
LI Incorrect lazy initialization of static field org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.instance in org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.getInstance()

Performance Warnings

Code Warning
Bx Primitive boxed just to call toString in org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.buildCubemapFaces()
Bx Primitive boxed just to call toString in org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.downloadCubemapImages(String)
Bx Primitive boxed just to call toString in org.openstreetmap.josm.plugins.streetside.cubemap.TileDownloadingTask.call()
Bx Primitive boxed just to call toString in org.openstreetmap.josm.plugins.streetside.io.download.SequenceDownloadRunnable.lambda$run$0(StreetsideImage, List, CubemapUtils$CubemapFaces)
UrF Unread field: org.openstreetmap.josm.plugins.streetside.oauth.StreetsideUser.imagesHash
UrF Unread field: org.openstreetmap.josm.plugins.streetside.oauth.StreetsideUser.imagesPolicy
UrF Unread field: org.openstreetmap.josm.plugins.streetside.oauth.StreetsideUser.isTokenValid
UrF Unread field: org.openstreetmap.josm.plugins.streetside.oauth.StreetsideUser.username

Dodgy code Warnings

Code Warning
BC Unchecked/unconfirmed cast from org.openstreetmap.josm.data.cache.CacheEntry to org.openstreetmap.josm.data.cache.BufferedImageCacheEntry in org.openstreetmap.josm.plugins.streetside.cache.StreetsideCache.isObjectLoadable()
BC Unchecked/unconfirmed cast from java.awt.Graphics to java.awt.Graphics2D in org.openstreetmap.josm.plugins.streetside.gui.StreetsideImageDisplay.paintComponent(Graphics)
DB org.openstreetmap.josm.plugins.streetside.utils.GraphicsUtils.cropMultiTiledImages(BufferedImage[], int) uses the same code for two branches
Eq org.openstreetmap.josm.plugins.streetside.model.ImageDetection doesn't override KeyIndexedObject.equals(Object)
Eq org.openstreetmap.josm.plugins.streetside.model.MapObject doesn't override KeyIndexedObject.equals(Object)
Eq org.openstreetmap.josm.plugins.streetside.model.SpecialImageArea doesn't override KeyIndexedObject.equals(Object)
Eq org.openstreetmap.josm.plugins.streetside.model.UserProfile doesn't override KeyIndexedObject.equals(Object)
NP Load of known null value in org.openstreetmap.josm.plugins.streetside.io.export.StreetsideExportWriterThread.run()
PZLA Should org.openstreetmap.josm.plugins.streetside.utils.api.JsonDecoder.decodeDoublePair(JsonArray) return a zero length array rather than null?
PZLA Should org.openstreetmap.josm.plugins.streetside.utils.api.JsonStreetsideDecoder.decodeDoublePair(JsonArray) return a zero length array rather than null?
RCN Redundant nullcheck of org.openstreetmap.josm.plugins.streetside.StreetsideImage.getId(), which is known to be non-null in org.openstreetmap.josm.plugins.streetside.cache.CacheUtils.downloadPicture(StreetsideImage, CacheUtils$PICTURE)
RCN Redundant nullcheck of StringBuilder.toString(), which is known to be non-null in org.openstreetmap.josm.plugins.streetside.gui.imageinfo.WebLinkAction.actionPerformed(ActionEvent)
RCN Redundant nullcheck of outputSet which is known to be null in org.openstreetmap.josm.plugins.streetside.io.export.StreetsideExportWriterThread.run()
RCN Redundant nullcheck of clientSocket, which is known to be non-null in org.openstreetmap.josm.plugins.streetside.oauth.OAuthPortListener.run()
RCN Redundant nullcheck of org.openstreetmap.josm.plugins.streetside.StreetsideImage.getId(), which is known to be non-null in org.openstreetmap.josm.plugins.streetside.utils.api.JsonStreetsideSequencesDecoder.decodeBubbleData(StreetsideImage)
REC Exception is caught when Exception is not thrown in org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.downloadCubemapImages(String)
RV Return value of org.openstreetmap.josm.plugins.streetside.gui.StreetsideViewerDialog.getStreetsideViewerPanel() ignored, but method has no side effect
ST Write to static field org.openstreetmap.josm.plugins.streetside.gui.imageinfo.StreetsideViewerPanel.threeSixtyDegreeViewerPanel from instance method org.openstreetmap.josm.plugins.streetside.gui.imageinfo.StreetsideViewerPanel.initializeAndStartGUI()
ST Write to static field org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel.camera from instance method org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel.initialize()
ST Write to static field org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel.root from instance method org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel.initialize()
ST Write to static field org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel.cubemapBox from instance method org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel.initialize()
ST Write to static field org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel.subGroup from instance method org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel.initialize()
UC Useless object stored in variable message of method org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.downloadCubemapImages(String)
UCF Useless control flow in org.openstreetmap.josm.plugins.streetside.model.MapObject.getIcon(String)

Details

BC_UNCONFIRMED_CAST: Unchecked/unconfirmed cast

This cast is unchecked, and not all instances of the type casted from can be cast to the type it is being cast to. Check that your program logic ensures that this cast will not fail.

DM_BOXED_PRIMITIVE_TOSTRING: Method allocates a boxed primitive just to call toString

A boxed primitive is allocated just to call toString(). It is more effective to just use the static form of toString which takes the primitive value. So,

Replace...With this...
new Integer(1).toString()Integer.toString(1)
new Long(1).toString()Long.toString(1)
new Float(1.0).toString()Float.toString(1.0)
new Double(1.0).toString()Double.toString(1.0)
new Byte(1).toString()Byte.toString(1)
new Short(1).toString()Short.toString(1)
new Boolean(true).toString()Boolean.toString(true)

DB_DUPLICATE_BRANCHES: Method uses the same code for two branches

This method uses the same code to implement two branches of a conditional branch. Check to ensure that this isn't a coding mistake.

EI_EXPOSE_REP: May expose internal representation by returning reference to mutable object

Returning a reference to a mutable object value stored in one of the object's fields exposes the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Returning a new copy of the object is better approach in many situations.

EQ_COMPARETO_USE_OBJECT_EQUALS: Class defines compareTo(...) and uses Object.equals()

This class defines a compareTo(...) method but inherits its equals() method from java.lang.Object. Generally, the value of compareTo should return zero if and only if equals returns true. If this is violated, weird and unpredictable failures will occur in classes such as PriorityQueue. In Java 5 the PriorityQueue.remove method uses the compareTo method, while in Java 6 it uses the equals method.

From the JavaDoc for the compareTo method in the Comparable interface:

It is strongly recommended, but not strictly required that (x.compareTo(y)==0) == (x.equals(y)). Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals."

EQ_DOESNT_OVERRIDE_EQUALS: Class doesn't override equals in superclass

This class extends a class that defines an equals method and adds fields, but doesn't define an equals method itself. Thus, equality on instances of this class will ignore the identity of the subclass and the added fields. Be sure this is what is intended, and that you don't need to override the equals method. Even if you don't need to override the equals method, consider overriding it anyway to document the fact that the equals method for the subclass just return the result of invoking super.equals(o).

HE_HASHCODE_USE_OBJECT_EQUALS: Class defines hashCode() and uses Object.equals()

This class defines a hashCode() method but inherits its equals() method from java.lang.Object (which defines equality by comparing object references).  Although this will probably satisfy the contract that equal objects must have equal hashcodes, it is probably not what was intended by overriding the hashCode() method.  (Overriding hashCode() implies that the object's identity is based on criteria more complicated than simple reference equality.)

If you don't think instances of this class will ever be inserted into a HashMap/HashTable, the recommended hashCode implementation to use is:

public int hashCode() {
    assert false : "hashCode not designed";
    return 42; // any arbitrary constant will do
}

LI_LAZY_INIT_STATIC: Incorrect lazy initialization of static field

This method contains an unsynchronized lazy initialization of a non-volatile static field. Because the compiler or processor may reorder instructions, threads are not guaranteed to see a completely initialized object, if the method can be called by multiple threads. You can make the field volatile to correct the problem. For more information, see the Java Memory Model web site.

MS_SHOULD_BE_FINAL: Field isn't final but should be

This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.

MS_SHOULD_BE_REFACTORED_TO_BE_FINAL: Field isn't final but should be refactored to be so

This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability. However, the static initializer contains more than one write to the field, so doing so will require some refactoring.

NP_LOAD_OF_KNOWN_NULL_VALUE: Load of known null value

The variable referenced at this point is known to be null due to an earlier check against null. Although this is valid, it might be a mistake (perhaps you intended to refer to a different variable, or perhaps the earlier check to see if the variable is null should have been a check to see if it was non-null).

NP_UNWRITTEN_FIELD: Read of unwritten field

The program is dereferencing a field that does not seem to ever have a non-null value written to it. Unless the field is initialized via some mechanism not seen by the analysis, dereferencing this value will generate a null pointer exception.

PZLA_PREFER_ZERO_LENGTH_ARRAYS: Consider returning a zero length array rather than null

It is often a better design to return a length zero array rather than a null reference to indicate that there are no results (i.e., an empty list of results). This way, no explicit check for null is needed by clients of the method.

On the other hand, using null to indicate "there is no answer to this question" is probably appropriate. For example, File.listFiles() returns an empty list if given a directory containing no files, and returns null if the file is not a directory.

RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE: Nullcheck of value previously dereferenced

A value is checked here to see whether it is null, but this value can't be null because it was previously dereferenced and if it were null a null pointer exception would have occurred at the earlier dereference. Essentially, this code and the previous dereference disagree as to whether this value is allowed to be null. Either the check is redundant or the previous dereference is erroneous.

RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE: Redundant nullcheck of value known to be non-null

This method contains a redundant check of a known non-null value against the constant null.

RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE: Redundant nullcheck of value known to be null

This method contains a redundant check of a known null value against the constant null.

REC_CATCH_EXCEPTION: Exception is caught when Exception is not thrown

This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

try {
    ...
} catch (RuntimeException e) {
    throw e;
} catch (Exception e) {
    ... deal with all non-runtime exceptions ...
}

RV_RETURN_VALUE_IGNORED_BAD_PRACTICE: Method ignores exceptional return value

This method returns a value that is not checked. The return value should be checked since it can indicate an unusual or unexpected function execution. For example, the File.delete() method returns false if the file could not be successfully deleted (rather than throwing an Exception). If you don't check the result, you won't notice if the method invocation signals unexpected behavior by returning an atypical return value.

RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT: Return value of method without side effect is ignored

This code calls a method and ignores the return value. However our analysis shows that the method (including its implementations in subclasses if any) does not produce any effect other than return value. Thus this call can be removed.

We are trying to reduce the false positives as much as possible, but in some cases this warning might be wrong. Common false-positive cases include:

- The method is designed to be overridden and produce a side effect in other projects which are out of the scope of the analysis.

- The method is called to trigger the class loading which may have a side effect.

- The method is called just to get some exception.

If you feel that our assumption is incorrect, you can use a @CheckReturnValue annotation to instruct SpotBugs that ignoring the return value of this method is acceptable.

ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD: Write to static field from instance method

This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice.

SE_NO_SUITABLE_CONSTRUCTOR: Class is Serializable but its superclass doesn't define a void constructor

This class implements the Serializable interface and its superclass does not. When such an object is deserialized, the fields of the superclass need to be initialized by invoking the void constructor of the superclass. Since the superclass does not have one, serialization and deserialization will fail at runtime.

SE_COMPARATOR_SHOULD_BE_SERIALIZABLE: Comparator doesn't implement Serializable

This class implements the Comparator interface. You should consider whether or not it should also implement the Serializable interface. If a comparator is used to construct an ordered collection such as a TreeMap, then the TreeMap will be serializable only if the comparator is also serializable. As most comparators have little or no state, making them serializable is generally easy and good defensive programming.

SE_BAD_FIELD: Non-transient non-serializable instance field in serializable class

This Serializable class defines a non-primitive instance field which is neither transient, Serializable, or java.lang.Object, and does not appear to implement the Externalizable interface or the readObject() and writeObject() methods.  Objects of this class will not be deserialized correctly if a non-Serializable object is stored in this field.

UC_USELESS_OBJECT: Useless object created

Our analysis shows that this object is useless. It's created and modified, but its value never go outside of the method or produce any side-effect. Either there is a mistake and object was intended to be used or it can be removed.

This analysis rarely produces false-positives. Common false-positive cases include:

- This object used to implicitly throw some obscure exception.

- This object used as a stub to generalize the code.

- This object used to hold strong references to weak/soft-referenced objects.

UCF_USELESS_CONTROL_FLOW: Useless control flow

This method contains a useless control flow statement, where control flow continues onto the same place regardless of whether or not the branch is taken. For example, this is caused by having an empty statement block for an if statement:

if (argv.length == 0) {
    // TODO: handle this case
}

URF_UNREAD_FIELD: Unread field

This field is never read.  Consider removing it from the class.

UWF_UNWRITTEN_FIELD: Unwritten field

This field is never written.  All reads of it will return the default value. Check for errors (should it have been initialized?), or remove it if it is useless.