Modify

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#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
    a b Build-Date: ${build.tstamp}  
    225225            <exclude name="org/apache/commons/logging/impl/ServletContextCleaner.java"/>
    226226        </javac>
    227227        <!-- JMapViewer -->
    228         <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/**"
     228        <javac compiler="com.google.errorprone.ErrorProneAntCompilerAdapter"
     229            sourcepath="" srcdir="${src.dir}" excludes="com/**,oauth/**,org/apache/commons/**,org/glassfish/**,org/openstreetmap/gui/jmapviewer/Demo.java,org/openstreetmap/josm/**,JOSM.java,gnu/**"
    229230            destdir="build" target="1.7" source="1.7" debug="on" includeantruntime="false" createMissingPackageInfoClass="false" encoding="UTF-8">
     231            <compilerclasspath>
     232              <pathelement location="./error_prone_ant-2.0.8.jar"/>
     233            </compilerclasspath>
    230234            <compilerarg value="-Xlint:cast"/>
    231235            <compilerarg value="-Xlint:deprecation"/>
    232236            <compilerarg value="-Xlint:dep-ann"/>
    Build-Date: ${build.tstamp}  
    242246            <compilerarg value="-XDignore.symbol.file"/>
    243247        </javac>
    244248        <!-- JOSM -->
    245         <javac sourcepath="" srcdir="${src.dir}" excludes="com/**,oauth/**,org/apache/commons/**,org/glassfish/**,org/openstreetmap/gui/jmapviewer/Demo.java"
     249        <javac compiler="com.google.errorprone.ErrorProneAntCompilerAdapter"
     250            sourcepath="" srcdir="${src.dir}" excludes="com/**,oauth/**,org/apache/commons/**,org/glassfish/**,org/openstreetmap/gui/jmapviewer/Demo.java"
    246251            destdir="build" target="1.7" source="1.7" debug="on" includeantruntime="false" createMissingPackageInfoClass="false" encoding="UTF-8">
     252            <compilerclasspath>
     253              <pathelement location="./error_prone_ant-2.0.8.jar"/>
     254            </compilerclasspath>
    247255            <compilerarg value="-Xlint:cast"/>
    248256            <compilerarg value="-Xlint:deprecation"/>
    249257            <compilerarg value="-Xlint:dep-ann"/>

Shall we add this to our build process?

Attachments (0)

Change History (25)

comment:1 by simon04, 8 years ago

In 9703/josm:

see #12472 - Fix warnings identified by error-prone

comment:2 by Don-vip, 8 years ago

Keywords: error prone ant added
Milestone: 16.02

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

comment:3 by simon04, 8 years ago

Keywords: java8 added
Milestone: 16.0216.04

Nowhere documented, but it requires JDK 8.

comment:4 by stoecker, 8 years ago

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

in reply to:  4 comment:5 by stoecker, 8 years ago

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 by Don-vip, 8 years ago

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.

comment:7 by Don-vip, 8 years ago

Milestone: 16.0416.05

comment:8 by Don-vip, 8 years ago

Milestone: 16.0516.06

comment:9 by Don-vip, 8 years ago

Milestone: 16.0616.07

comment:10 by Don-vip, 8 years ago

Owner: changed from team to Don-vip
Status: newassigned
Summary: Use error-prone in build[PATCH] Java 8: Use error-prone in build

comment:11 by Don-vip, 8 years ago

Resolution: fixed
Status: assignedclosed

In 10581/josm:

see #11390, fix #12472 - Use error-prone in build (patch by simon04, modified) - requires java 8

comment:12 by Don-vip, 8 years ago

In 10589/josm:

see #12472 - fix "Non-exhaustive switch" compilation warning

comment:13 by Don-vip, 8 years ago

In 10614/josm:

see #11390, see #12472 - Update to error-prone 2.0.11

comment:14 by Don-vip, 8 years ago

In 10628/josm:

see #12472 - error-prone causes problems with coverity and java 9, introduce an ant property to override it

comment:15 by Don-vip, 8 years ago

Summary: [PATCH] Java 8: Use error-prone in buildJava 8: Use error-prone in build

comment:16 by Don-vip, 8 years ago

In 10653/josm:

see #12472 - fix warning "ClassCanBeStatic"

comment:17 by Don-vip, 8 years ago

In 10655/josm:

see #12472 - fix warning "OperatorPrecedence"

comment:18 by Don-vip, 8 years ago

In 10656/josm:

see #12472 - fix warning "ReferenceEquality"

comment:19 by Don-vip, 8 years ago

In 10659/josm:

see #12472 - fix more warnings, increase maximum number of reported warnings from 100 to 1000

comment:20 by Don-vip, 8 years ago

In 10662/josm:

see #12472, fix #13230, fix #13225, fix #13228 - disable ReferenceEquality warning + partial revert of r10656 + r10659, causes too much problems

comment:21 by Don-vip, 8 years ago

In 10670/josm:

see #12472 - fix "OperatorPrecedence" warnings

comment:22 by Don-vip, 8 years ago

In 10671/josm:

see #12472 - fix "MissingCasesInEnumSwitch" warnings

comment:23 by Don-vip, 8 years ago

In 10672/josm:

see #12472 - fix "UnsynchronizedOverridesSynchronized" warnings

comment:24 by Don-vip, 8 years ago

In 10678/josm:

see #12472 - fix "OperatorPrecedence" warnings

comment:25 by Don-vip, 8 years ago

In 10704/josm:

see #12472 - fix build when error-prone is not used

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.