Context Navigation

Modify ↓

#12472 closed enhancement (fixed)

Java 8: Use error-prone in build

Reported by:	simon04	Owned by:	Don-vip
Priority:	normal	Milestone:	16.07
Component:	Core	Version:
Keywords:	build error prone ant java8	Cc:	Don-vip, bastiK

Description

Error-prone aims at identifying issues at compile time. Running it on the current code base reveals:

compile:
    [javac] Compiling 434 source files to /home/simon/src/josm.svngit/build
    [javac] Note: /home/simon/src/josm.svngit/src/oauth/signpost/AbstractOAuthProvider.java uses or overrides a deprecated API.
    [javac] Note: Recompile with -Xlint:deprecation for details.
    [javac] Note: Some input files use unchecked or unsafe operations.
    [javac] Note: Recompile with -Xlint:unchecked for details.
    [javac] Compiling 60 source files to /home/simon/src/josm.svngit/build
    [javac] /home/simon/src/josm.svngit/src/org/openstreetmap/gui/jmapviewer/JMapViewer.java:305: warning: [SynchronizeOnNonFinalField] Synchronizing on non-final fields is not safe: if the field is ever updated, different threads may end up locking on different objects.
    [javac]             synchronized (mapMarkerList) {
    [javac]                          ^
    [javac]     (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
    [javac] /home/simon/src/josm.svngit/src/org/openstreetmap/gui/jmapviewer/JMapViewer.java:319: warning: [SynchronizeOnNonFinalField] Synchronizing on non-final fields is not safe: if the field is ever updated, different threads may end up locking on different objects.
    [javac]             synchronized (mapRectangleList) {
    [javac]                          ^
    [javac]     (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
    [javac] /home/simon/src/josm.svngit/src/org/openstreetmap/gui/jmapviewer/JMapViewer.java:334: warning: [SynchronizeOnNonFinalField] Synchronizing on non-final fields is not safe: if the field is ever updated, different threads may end up locking on different objects.
    [javac]             synchronized (mapPolygonList) {
    [javac]                          ^
    [javac]     (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
    [javac] /home/simon/src/josm.svngit/src/org/openstreetmap/gui/jmapviewer/JMapViewer.java:653: warning: [SynchronizeOnNonFinalField] Synchronizing on non-final fields is not safe: if the field is ever updated, different threads may end up locking on different objects.
    [javac]             synchronized (mapPolygonList) {
    [javac]                          ^
    [javac]     (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
    [javac] /home/simon/src/josm.svngit/src/org/openstreetmap/gui/jmapviewer/JMapViewer.java:662: warning: [SynchronizeOnNonFinalField] Synchronizing on non-final fields is not safe: if the field is ever updated, different threads may end up locking on different objects.
    [javac]             synchronized (mapRectangleList) {
    [javac]                          ^
    [javac]     (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
    [javac] /home/simon/src/josm.svngit/src/org/openstreetmap/gui/jmapviewer/JMapViewer.java:671: warning: [SynchronizeOnNonFinalField] Synchronizing on non-final fields is not safe: if the field is ever updated, different threads may end up locking on different objects.
    [javac]             synchronized (mapMarkerList) {
    [javac]                          ^
    [javac]     (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
    [javac] 6 warnings
    [javac] Compiling 1290 source files to /home/simon/src/josm.svngit/build
    [javac] /home/simon/src/josm.svngit/src/org/openstreetmap/josm/data/osm/Storage.java:414: warning: [NonAtomicVolatileUpdate] This update of a volatile variable is non-atomic
    [javac]             modCount++;
    [javac]                     ^
    [javac]     (see http://errorprone.info/bugpattern/NonAtomicVolatileUpdate)
    [javac] /home/simon/src/josm.svngit/src/org/openstreetmap/josm/gui/SideButton.java:59: warning: [StringEquality] String comparison using reference equality instead of value equality
    [javac]                     if (evt.getPropertyName() == javax.swing.Action.SMALL_ICON) {
    [javac]                                               ^
    [javac]     (see http://errorprone.info/bugpattern/StringEquality)
    [javac]   Did you mean 'if (javax.swing.Action.SMALL_ICON.equals(evt.getPropertyName())) {'?
    [javac] /home/simon/src/josm.svngit/src/org/openstreetmap/josm/io/NmeaReader.java:54: warning: [NonOverridingEquals] equals method doesn't override Object.equals; enum instances can safely be compared by reference equality, so please delete this
    [javac]         public boolean equals(String type) {
    [javac]                        ^
    [javac]     (see http://errorprone.info/bugpattern/NonOverridingEquals)
    [javac]   Did you mean to remove this line?
    [javac] 3 warnings
     [copy] Copying 1 file to /home/simon/src/josm.svngit/build

To integrate it, the following modifications on build.xml are required:

build.xml

diff --git a/build.xml b/build.xml
index bd7e95c..019fd16 100644

  Build-Date: ${build.tstamp}
             <exclude name="org/apache/commons/logging/impl/ServletContextCleaner.java"/>
         </javac>
         <!-- JMapViewer -->
+        <javac sourcepath="" srcdir="${src.dir}" excludes="com/**,oauth/**,org/apache/commons/**,org/glassfish/**,org/openstreetmap/gui/jmapviewer/Demo.java,org/openstreetmap/josm/**,JOSM.java,gnu/**"
+        <javac compiler="com.google.errorprone.ErrorProneAntCompilerAdapter"
+            sourcepath="" srcdir="${src.dir}" excludes="com/**,oauth/**,org/apache/commons/**,org/glassfish/**,org/openstreetmap/gui/jmapviewer/Demo.java,org/openstreetmap/josm/**,JOSM.java,gnu/**"
             destdir="build" target="1.7" source="1.7" debug="on" includeantruntime="false" createMissingPackageInfoClass="false" encoding="UTF-8">
+            <compilerclasspath>
+              <pathelement location="./error_prone_ant-2.0.8.jar"/>
+            </compilerclasspath>
             <compilerarg value="-Xlint:cast"/>
             <compilerarg value="-Xlint:deprecation"/>
             <compilerarg value="-Xlint:dep-ann"/>
-…
+ Build-Date: ${build.tstamp}
             <compilerarg value="-XDignore.symbol.file"/>
         </javac>
         <!-- JOSM -->
+        <javac sourcepath="" srcdir="${src.dir}" excludes="com/**,oauth/**,org/apache/commons/**,org/glassfish/**,org/openstreetmap/gui/jmapviewer/Demo.java"
+        <javac compiler="com.google.errorprone.ErrorProneAntCompilerAdapter"
+            sourcepath="" srcdir="${src.dir}" excludes="com/**,oauth/**,org/apache/commons/**,org/glassfish/**,org/openstreetmap/gui/jmapviewer/Demo.java"
             destdir="build" target="1.7" source="1.7" debug="on" includeantruntime="false" createMissingPackageInfoClass="false" encoding="UTF-8">
+            <compilerclasspath>
+              <pathelement location="./error_prone_ant-2.0.8.jar"/>
+            </compilerclasspath>
             <compilerarg value="-Xlint:cast"/>
             <compilerarg value="-Xlint:deprecation"/>
             <compilerarg value="-Xlint:dep-ann"/>

Shall we add this to our build process?

Attachments (0)

Change History (25)

comment:1 Changed 7 years ago by simon04

In 9703/josm:

see #12472 - Fix warnings identified by error-prone

comment:2 Changed 7 years ago by Don-vip

Keywords:	error prone ant added
Milestone:	→ 16.02

I'd say yes if you put jar file in tools.

comment:3 Changed 7 years ago by simon04

Keywords:	java8 added
Milestone:	16.02 → 16.04

Nowhere documented, but it requires JDK 8.

comment:4 follow-up: 5 Changed 7 years ago by stoecker

I think these are also in Coverity. Do we need another test for the same?

comment:5 in reply to: 4 Changed 7 years ago by stoecker

Replying to stoecker:

I think these are also in Coverity. Do we need another test for the same?

No. It seems I mixed something up. These are not in Coverity.

comment:6 Changed 7 years ago by Don-vip

Even if it were, it's always better to have an autonomous offline system. We have quotas with Coverity and sometimes the website can be slow, or even unavailable.