[
https://issues.apache.org/jira/browse/ACCUMULO-4158?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16320329#comment-16320329
]
Mark Owens commented on ACCUMULO-4158:
--------------------------------------
[~ctubbsii], I agree that it would best be used as a development tool. I was
interested in seeing what types of issues it would discover. I created a new
maven profile to run error-prone displaying all findings as warnings. I created
a couple of summary files displaying the results that I'll attach in case
anyone is interested in viewing them. There was no editing of bug-issues in
these runs. All issues that error-prone thinks should be examined are displayed.
Some of the warnings such are LongLiteralLowerCaseSuffix, i.e., using capital
'L' for longs, aren't critical but they do add to consistentcy in the look of
the code base. Additionally, for older guys such as myself, whose eyesight is
not what is used to be, using a capital does make parsing the code much easier
:)
If we decided to use an error-prone profile as a dev tool we could come to a
consensus as to how to manage the error-prone bug list. There are options to
display everything as a warning rather than an error, as well as options to
turn off any particular bug-check completely. You can also convert any warning
to error, or vice-versa. It can also be set to ignore issues in generated code.
There is also a feature that allows error-prone to create patches that can be
used to update the code with selected fixes. For instance, for testing
purposes, I ran a patch to add all missing Override annotations to the code. An
error-prone profile could be used to enforce this convention in future code as
this can be flagged as an error, see
(https://issues.apache.org/jira/browse/ACCUMULO-2537).
I've heard of Spotbugs but have not taken a look as of yet. I'll look into it.
> Investigate using Google's error-prone
> --------------------------------------
>
> Key: ACCUMULO-4158
> URL: https://issues.apache.org/jira/browse/ACCUMULO-4158
> Project: Accumulo
> Issue Type: Improvement
> Components: build
> Reporter: Michael Wall
> Assignee: Mark Owens
> Priority: Minor
>
> Google has a tool at http://errorprone.info. From that page
> {quote}
> Using Error Prone to augment the compiler’s type analysis, you can catch more
> mistakes before they cost you time, or end up as bugs in production.
> {quote}
> It requires java 1.8. In the top level pom, replacing
> {code:xml}
> <plugin>
> <artifactId>maven-compiler-plugin</artifactId>
> <configuration>
> <source>${java.ver}</source>
> <target>${java.ver}</target>
> <optimize>true</optimize>
> <showDeprecation>true</showDeprecation>
> <showWarnings>true</showWarnings>
> </configuration>
> </plugin>
> {code}
> with
> {code:xml}
> <plugin>
> <groupId>org.apache.maven.plugins</groupId>
> <artifactId>maven-compiler-plugin</artifactId>
> <version>3.3</version>
> <configuration>
> <compilerId>javac-with-errorprone</compilerId>
> <forceJavacCompilerUse>true</forceJavacCompilerUse>
> <!-- maven-compiler-plugin defaults to targeting Java 5,
> but our javac only supports >=6 -->
> <source>${java.ver}</source>
> <target>${java.ver}</target>
> <compilerArgs>
> <arg>-Xep:ChainingConstructorIgnoresParameter:WARN</arg>
> <arg>-Xep:LongLiteralLowerCaseSuffix:OFF</arg>
> <arg>-Xep:SizeGreaterThanOrEqualsZero:WARN</arg>
> <arg>-Xep:InvalidPatternSyntax:WARN</arg>
> <arg>-Xep:TryFailThrowable:WARN</arg>
> <arg>-Xep:NonOverridingEquals:OFF</arg>
> </compilerArgs>
> <showWarnings>true</showWarnings>
> </configuration>
> <dependencies>
> <!-- override plexus-compiler-javac-errorprone's dependency on
> Error Prone with the latest version -->
> <dependency>
> <groupId>com.google.errorprone</groupId>
> <artifactId>error_prone_core</artifactId>
> <version>2.0.8</version>
> </dependency>
> <dependency>
> <groupId>org.codehaus.plexus</groupId>
> <artifactId>plexus-compiler-javac-errorprone</artifactId>
> <version>2.5</version>
> </dependency>
> </dependencies>
> </plugin>
> {code}
> in the 1.6 branch and runing 'mvn compile' yielded the following.
> {noformat}
> [INFO] Scanning for projects...
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Reactor Build Order:
> [INFO]
> [INFO] Apache Accumulo Project
> [INFO] Apache Accumulo Trace
> [INFO] Apache Accumulo Fate
> [INFO] Apache Accumulo Start
> [INFO] Apache Accumulo Core
> [INFO] Apache Accumulo Simple Examples
> [INFO] Apache Accumulo Server Base
> [INFO] Apache Accumulo GC Server
> [INFO] Apache Accumulo Master Server
> [INFO] Apache Accumulo Monitor Server
> [INFO] Apache Accumulo Tracer Server
> [INFO] Apache Accumulo Tablet Server
> [INFO] Apache Accumulo MiniCluster
> [INFO] Apache Accumulo Native Libraries
> [INFO] Apache Accumulo Testing
> [INFO] Apache Accumulo Proxy
> [INFO] Apache Accumulo
> [INFO] Apache Accumulo Documentation
> [INFO] Apache Accumulo Maven Plugin
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Project 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Trace 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Fate 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Start 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Core 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO] --- maven-compiler-plugin:3.3:compile (default-compile) @
> accumulo-core ---
> [INFO] Changes detected - recompiling the module!
> [INFO] Compiling 595 source files to
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/target/classes
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/data/Value.java:80:
> warning: [ChainingConstructorIgnoresParameter] The called constructor
> accepts a parameter with the same name and type as one of its caller's
> parameters, but its caller doesn't pass that parameter to it. It's likely
> that it was intended to.
> this(toBytes(bytes), false);
> ^
> (see
> http://errorprone.info/bugpattern/ChainingConstructorIgnoresParameter)
> Did you mean 'this(toBytes(bytes), copy);'?
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/mapreduce/AccumuloOutputFormat.java:510:
> warning: [SizeGreaterThanOrEqualsZero] Comparison of a size >= 0 is always
> true, did you intend to check for non-emptiness?
> if (e.getAuthorizationFailuresMap().size() >= 0) {
> ^
> (see http://errorprone.info/bugpattern/SizeGreaterThanOrEqualsZero)
> Did you mean 'if (!e.getAuthorizationFailuresMap().isEmpty()) {'?
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/mapreduce/lib/impl/ConfiguratorBase.java:277:
> warning: [Finally] If you return or throw from a finally, then values
> returned or thrown from the try-catch block will be ignored. Consider using
> try-with-resources instead.
> throw new RuntimeException(fileScanner.ioException());
> ^
> (see http://errorprone.info/bugpattern/Finally)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java:447:
> warning: [WaitNotInLoop] Because of spurious wakeups, wait() must always be
> called in a loop
> wait();
> ^
> (see http://errorprone.info/bugpattern/WaitNotInLoop)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/volume/VolumeImpl.java:109:
> warning: [EqualsHashCode] Classes that override equals should also override
> hashCode.
> public boolean equals(Object o) {
> ^
> (see http://errorprone.info/bugpattern/EqualsHashCode)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/LruBlockCache.java:444:
> warning: [EqualsHashCode] Classes that override equals should also override
> hashCode.
> public boolean equals(Object that) {
> ^
> (see http://errorprone.info/bugpattern/EqualsHashCode)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/file/FileOperations.java:17:
> warning: [MultipleTopLevelClasses] Expected at most one top-level class
> declaration, instead found: DispatchingFileFactory, FileOperations
> package org.apache.accumulo.core.file;
> ^
> (see http://errorprone.info/bugpattern/MultipleTopLevelClasses)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/util/shell/commands/ConfigCommand.java:211:
> warning: [InvalidPatternSyntax] Invalid syntax used for a regular
> expression: "." is a valid but useless regex
> siteVal = sysVal = dfault = curVal = curVal.replaceAll(".", "*");
> ^
> (see http://errorprone.info/bugpattern/InvalidPatternSyntax)
> Did you mean 'siteVal = sysVal = dfault = curVal = curVal.replaceAll("\\.",
> "*");'?
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/mapred/AccumuloOutputFormat.java:509:
> warning: [SizeGreaterThanOrEqualsZero] Comparison of a size >= 0 is always
> true, did you intend to check for non-emptiness?
> if (e.getAuthorizationFailuresMap().size() >= 0) {
> ^
> (see http://errorprone.info/bugpattern/SizeGreaterThanOrEqualsZero)
> Did you mean 'if (!e.getAuthorizationFailuresMap().isEmpty()) {'?
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/impl/OfflineScanner.java:17:
> warning: [MultipleTopLevelClasses] Expected at most one top-level class
> declaration, instead found: OfflineIterator, OfflineScanner
> package org.apache.accumulo.core.client.impl;
> ^
> (see http://errorprone.info/bugpattern/MultipleTopLevelClasses)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java:211:
> 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.
> synchronized (serverQueues) {
> ^
> (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java:487:
> 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.
> synchronized (cachedSessionIDs) {
> ^
> (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java:505:
> 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.
> synchronized (cachedSessionIDs) {
> ^
> (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java:521:
> 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.
> synchronized (cachedSessionIDs) {
> ^
> (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/impl/ConditionalWriterImpl.java:528:
> 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.
> synchronized (cachedSessionIDs) {
> ^
> (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/core/src/main/java/org/apache/accumulo/core/client/impl/MultiTableBatchWriterImpl.java:218:
> 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.
> synchronized (tableWriters) {
> ^
> (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
> 16 warnings
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Simple Examples 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ..
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Server Base 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo GC Server 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Master Server 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Monitor Server 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Tracer Server 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Tablet Server 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo MiniCluster 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Native Libraries 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Testing 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
> [INFO] --- maven-compiler-plugin:3.3:compile (default-compile) @
> accumulo-test ---
> [INFO] Changes detected - recompiling the module!
> [INFO] Compiling 176 source files to
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/test/target/classes
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/test/src/main/java/org/apache/accumulo/test/EstimateInMemMapOverhead.java:17:
> warning: [MultipleTopLevelClasses] Expected at most one top-level class
> declaration, instead found: MemoryUsageTest, TextMemoryUsageTest,
> InMemoryMapMemoryUsageTest, MutationMemoryUsageTest,
> IntObjectMemoryUsageTest, EstimateInMemMapOverhead
> package org.apache.accumulo.test;
> ^
> (see http://errorprone.info/bugpattern/MultipleTopLevelClasses)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/test/src/main/java/org/apache/accumulo/test/IMMLGBenchmark.java:129:
> warning: [ElementsCountedInLoop] This code, which counts elements using a
> loop, can be replaced by a simpler library method
> for (@SuppressWarnings("unused")
> ^
> (see http://errorprone.info/bugpattern/ElementsCountedInLoop)
> Did you mean 'count += Iterables.size(scanner);'?
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/test/src/main/java/org/apache/accumulo/test/continuous/Histogram.java:17:
> warning: [MultipleTopLevelClasses] Expected at most one top-level class
> declaration, instead found: HistData, Histogram
> package org.apache.accumulo.test.continuous;
> ^
> (see http://errorprone.info/bugpattern/MultipleTopLevelClasses)
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/test/src/main/java/org/apache/accumulo/test/performance/metadata/MetadataBatchScanTest.java:225:
> warning: [ElementsCountedInLoop] This code, which counts elements using a
> loop, can be replaced by a simpler library method
> for (@SuppressWarnings("unused")
> ^
> (see http://errorprone.info/bugpattern/ElementsCountedInLoop)
> Did you mean 'count += Iterables.size(bs);'?
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/test/src/main/java/org/apache/accumulo/test/randomwalk/Module.java:402:
> 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.
> synchronized (timer) {
> ^
> (see http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
> 5 warnings
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Proxy 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Documentation 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
>
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Building Apache Accumulo Maven Plugin 1.6.6-SNAPSHOT
> [INFO]
> ------------------------------------------------------------------------
> ...
> [INFO]
> [INFO] --- maven-compiler-plugin:3.3:compile (default-compile) @
> accumulo-maven-plugin ---
> [INFO] Changes detected - recompiling the module!
> [INFO] Compiling 4 source files to
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/maven-plugin/target/classes
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/maven-plugin/target/generated-sources/plugin/org/apache/accumulo/maven/plugin/HelpMojo.java:100:
> warning: [Finally] If you return or throw from a finally, then values
> returned or thrown from the try-catch block will be ignored. Consider using
> try-with-resources instead.
> throw new MojoExecutionException( e.getMessage(), e );
> ^
> (see http://errorprone.info/bugpattern/Finally)
> 1 warning
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO]
> [INFO] Apache Accumulo Project ............................ SUCCESS [ 3.028
> s]
> [INFO] Apache Accumulo Trace .............................. SUCCESS [ 1.216
> s]
> [INFO] Apache Accumulo Fate ............................... SUCCESS [ 0.584
> s]
> [INFO] Apache Accumulo Start .............................. SUCCESS [ 2.946
> s]
> [INFO] Apache Accumulo Core ............................... SUCCESS [ 28.759
> s]
> [INFO] Apache Accumulo Simple Examples .................... SUCCESS [ 1.099
> s]
> [INFO] Apache Accumulo Server Base ........................ SUCCESS [ 1.251
> s]
> [INFO] Apache Accumulo GC Server .......................... SUCCESS [ 0.856
> s]
> [INFO] Apache Accumulo Master Server ...................... SUCCESS [ 0.993
> s]
> [INFO] Apache Accumulo Monitor Server ..................... SUCCESS [ 1.111
> s]
> [INFO] Apache Accumulo Tracer Server ...................... SUCCESS [ 0.808
> s]
> [INFO] Apache Accumulo Tablet Server ...................... SUCCESS [ 1.032
> s]
> [INFO] Apache Accumulo MiniCluster ........................ SUCCESS [ 2.017
> s]
> [INFO] Apache Accumulo Native Libraries ................... SUCCESS [ 1.591
> s]
> [INFO] Apache Accumulo Testing ............................ SUCCESS [ 3.486
> s]
> [INFO] Apache Accumulo Proxy .............................. SUCCESS [ 1.984
> s]
> [INFO] Apache Accumulo .................................... SUCCESS [ 2.274
> s]
> [INFO] Apache Accumulo Documentation ...................... SUCCESS [ 0.280
> s]
> [INFO] Apache Accumulo Maven Plugin ....................... SUCCESS [ 6.065
> s]
> [INFO]
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Total time: 01:02 min
> [INFO] Finished at: 2016-03-03T11:05:24-05:00
> [INFO] Final Memory: 142M/615M
> [INFO]
> ------------------------------------------------------------------------
> {noformat}
> Notice the LongLiteralLowerCaseSuffix and NonOverridingEquals are both off.
> There were lots of these. Also notice I only ran 'mvn compile', there are
> errors in the test classes as well like
> {noformat}
> /Users/mjwall/NetBeansProjects/accumulo-mjwall/fate/src/test/java/org/apache/accumulo/fate/zookeeper/DistributedReadWriteLockTest.java:74:
> warning: [NonAtomicVolatileUpdate] This update of a volatile variable is
> non-atomic
> ++counter;
> ^
> {noformat}
> Maybe this is useful. Thanks to [~drew.farris] for showing me error-prone.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
