SpotBugs Report

Project Information

Project:

SpotBugs version: 3.1.1

Code analyzed:



Metrics

5844 lines of code analyzed, in 141 classes, in 17 packages.

Metric Total Density*
High Priority Warnings 10 1.71
Medium Priority Warnings 20 3.42
Low Priority Warnings 33 5.65
Total Warnings 63 10.78

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



Contents

Summary

Warning Type Number
Bad practice Warnings 15
Correctness Warnings 2
Malicious code vulnerability Warnings 3
Multithreaded correctness Warnings 4
Performance Warnings 10
Dodgy code Warnings 29
Total 63

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()
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.imageinfo.ClipboardAction defines non-transient non-serializable instance field contents
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 org.openstreetmap.josm.plugins.streetside.model.UserProfile is Serializable but its superclass doesn't define an accessible void constructor
Se org.openstreetmap.josm.plugins.streetside.StreetsideLayer$NearestCbToTargetComparator implements Comparator but not Serializable
Se org.openstreetmap.josm.plugins.streetside.StreetsideLayer$NearestImgToTargetComparator implements Comparator but not Serializable

Correctness Warnings

Code Warning
UwF Unwritten field: org.openstreetmap.josm.plugins.streetside.StreetsideImage.he
UwF Unwritten field: org.openstreetmap.josm.plugins.streetside.StreetsideSequence.user

Malicious code vulnerability Warnings

Code Warning
EI org.openstreetmap.josm.plugins.streetside.utils.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
IS Inconsistent synchronization of org.openstreetmap.josm.plugins.streetside.gui.StreetsideMainDialog.streetsideViewerHelp; locked 50% of time
LI Incorrect lazy initialization of static field org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.instance in org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.getInstance()
UG org.openstreetmap.josm.plugins.streetside.gui.StreetsideMainDialog.getStreetsideViewerHelp() is unsynchronized, org.openstreetmap.josm.plugins.streetside.gui.StreetsideMainDialog.setStreetsideViewerHelp(StreetsideViewerHelpPopup) is synchronized
UG org.openstreetmap.josm.plugins.streetside.StreetsideAbstractImage.getCd() is unsynchronized, org.openstreetmap.josm.plugins.streetside.StreetsideAbstractImage.setCd(long) is synchronized

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.CubemapBuilder.tileAdded(String)
Bx Boxing/unboxing to parse a primitive org.openstreetmap.josm.plugins.streetside.cubemap.CubemapUtils.convertQuaternary2Decimal(String)
Bx Primitive boxed just to call toString in org.openstreetmap.josm.plugins.streetside.io.download.SequenceDownloadRunnable.lambda$run$0(StreetsideImage, List, CubemapUtils$CubemapFaces)
UPM Private method org.openstreetmap.josm.plugins.streetside.actions.WalkThread.preDownloadCubemaps(StreetsideImage, int, CacheUtils$PICTURE) is never called
UPM Private method org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel.createDefaultScene() is never called
UrF Unread field: org.openstreetmap.josm.plugins.streetside.cubemap.TileDownloadingTask.cancelled
UrF Unread field: org.openstreetmap.josm.plugins.streetside.io.download.ImageDetailsDownloadRunnable.data
UuF Unused field: org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.streetsideViewerHelp

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.cubemap.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)
FE Test for floating point equality in org.openstreetmap.josm.plugins.streetside.utils.CubemapBox.loadSingleImageViewports()
NP Load of known null value in org.openstreetmap.josm.plugins.streetside.io.download.SequenceDownloadRunnable.run(URLConnection)
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 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)
RI Class org.openstreetmap.josm.plugins.streetside.StreetsideCubemap implements same interface as superclass
SF Switch statement found in org.openstreetmap.josm.plugins.streetside.gui.StreetsideMainDialog.setMode(StreetsideMainDialog$MODE) where default case is missing
SF Switch statement found in org.openstreetmap.josm.plugins.streetside.utils.StreetsideURL.string2URLs(String, String, String) where one case falls through to the next case
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()
ST Write to static field org.openstreetmap.josm.plugins.streetside.StreetsideCubemap.face2TilesMap from instance method new org.openstreetmap.josm.plugins.streetside.StreetsideCubemap(String, LatLon, double)
ST Write to static field org.openstreetmap.josm.plugins.streetside.StreetsideCubemap.face2TilesMap from instance method org.openstreetmap.josm.plugins.streetside.StreetsideCubemap.resetFaces2TileMap()
UCF Useless control flow in org.openstreetmap.josm.plugins.streetside.io.download.SequenceDownloadRunnable.run(URLConnection)
UCF Useless control flow in org.openstreetmap.josm.plugins.streetside.StreetsideLayer.setVisible(boolean)
UrF Unread public/protected field: org.openstreetmap.josm.plugins.streetside.cubemap.CubemapUtils.directionConversion
UuF Unused public or protected field: org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.cancelled
UwF CubemapBuilder.cubemap not initialized in constructor and dereferenced in org.openstreetmap.josm.plugins.streetside.cubemap.CubemapBuilder.buildCubemapFaces()
UwF JoinMode.lastPos not initialized in constructor and dereferenced in org.openstreetmap.josm.plugins.streetside.mode.JoinMode.paint(Graphics2D, MapView, Bounds)

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_FOR_PARSING: Boxing/unboxing to parse a primitive

A boxed primitive is created from a String, just to extract the unboxed primitive value. It is more efficient to just call the static parseXXX method.

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).

FE_FLOATING_POINT_EQUALITY: Test for floating point equality

This operation compares two floating point values for equality. Because floating point calculations may involve rounding, calculated float and double values may not be accurate. For values that must be precise, such as monetary values, consider using a fixed-precision type such as BigDecimal. For values that need not be precise, consider comparing for equality within some range, for example: if ( Math.abs(x - y) < .0000001 ). See the Java Language Specification, section 4.2.4.

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
}

IS2_INCONSISTENT_SYNC: Inconsistent synchronization

The fields of this class appear to be accessed inconsistently with respect to synchronization.  This bug report indicates that the bug pattern detector judged that

A typical bug matching this bug pattern is forgetting to synchronize one of the methods in a class that is intended to be thread-safe.

You can select the nodes labeled "Unsynchronized access" to show the code locations where the detector believed that a field was accessed without synchronization.

Note that there are various sources of inaccuracy in this detector; for example, the detector cannot statically detect all situations in which a lock is held.  Also, even when the detector is accurate in distinguishing locked vs. unlocked accesses, the code in question may still be correct.

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).

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_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.

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 ...
}

RI_REDUNDANT_INTERFACES: Class implements same interface as superclass

This class declares that it implements an interface that is also implemented by a superclass. This is redundant because once a superclass implements an interface, all subclasses by default also implement this interface. It may point out that the inheritance hierarchy has changed since this class was created, and consideration should be given to the ownership of the interface's implementation.

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.

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.

SF_SWITCH_NO_DEFAULT: Switch statement found where default case is missing

This method contains a switch statement where default case is missing. Usually you need to provide a default case.

Because the analysis only looks at the generated bytecode, this warning can be incorrect triggered if the default case is at the end of the switch statement and the switch statement doesn't contain break statements for other cases.

SF_SWITCH_FALLTHROUGH: Switch statement found where one case falls through to the next case

This method contains a switch statement where one case branch will fall through to the next case. Usually you need to end this case with a break or return.

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.

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
}

UG_SYNC_SET_UNSYNC_GET: Unsynchronized get method, synchronized set method

This class contains similarly-named get and set methods where the set method is synchronized and the get method is not.  This may result in incorrect behavior at runtime, as callers of the get method will not necessarily see a consistent state for the object.  The get method should be made synchronized.

UPM_UNCALLED_PRIVATE_METHOD: Private method is never called

This private method is never called. Although it is possible that the method will be invoked through reflection, it is more likely that the method is never used, and should be removed.

URF_UNREAD_FIELD: Unread field

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

URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD: Unread public/protected field

This field is never read.  The field is public or protected, so perhaps it is intended to be used with classes not seen as part of the analysis. If not, consider removing it from the class.

UUF_UNUSED_FIELD: Unused field

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

UUF_UNUSED_PUBLIC_OR_PROTECTED_FIELD: Unused public or protected field

This field is never used.  The field is public or protected, so perhaps it is intended to be used with classes not seen as part of the analysis. If not, consider removing it from the class.

UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR: Field not initialized in constructor but dereferenced without null check

This field is never initialized within any constructor, and is therefore could be null after the object is constructed. Elsewhere, it is loaded and dereferenced without a null check. This could be a either an error or a questionable design, since it means a null pointer exception will be generated if that field is dereferenced before being initialized.

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.