Enable strict compiler settings (#413)

This change enforces javac compiler warnings and linting as errors as a
part of #406.

Author: bpkroth

.devcontainer/devcontainer.json

@@ -40,11 +40,13 @@
             "settings": {},
             // Add the IDs of extensions you want installed when the container is created.
             "extensions": [
-                "github.vscode-github-actions",
+                "EditorConfig.EditorConfig",
                 "GitHub.copilot",
                 "eamodio.gitlens",
-                "vscjava.vscode-java-pack",
-                "EditorConfig.EditorConfig"
+                "github.vscode-github-actions",
+                "github.vscode-pull-request-github",
+                "huntertran.auto-markdown-toc",
+                "vscjava.vscode-java-pack"
             ]
         }
     },

.vscode/extensions.json

@@ -0,0 +1,8 @@
+{
+    "recommendations": [
+        "EditorConfig.EditorConfig",
+        "github.vscode-pull-request-github",
+        "huntertran.auto-markdown-toc",
+        "vscjava.vscode-java-pack"
+    ]
+}

CONTRIBUTING.md

@@ -0,0 +1,72 @@
+# Contributing
+
+We welcome all contributions! Please open a [pull request](https://github.com/cmu-db/benchbase/pulls). Common contributions may include:
+
+- Adding support for a new DBMS.
+- Adding more tests of existing benchmarks.
+- Fixing any bugs or known issues.
+
+## Contents
+
+<!-- TOC -->
+
+- [Contributing](#contributing)
+    - [Contents](#contents)
+    - [IDE](#ide)
+    - [Adding a new DBMS](#adding-a-new-dbms)
+    - [Java Development Notes](#java-development-notes)
+        - [Avoid var keyword](#avoid-var-keyword)
+        - [Compiler Warnings](#compiler-warnings)
+            - [Alternatives to arrays of generics](#alternatives-to-arrays-of-generics)
+
+<!-- /TOC -->
+
+## IDE
+
+Although you can use any IDE you prefer, there are some configurations for [VSCode](https://code.visualstudio.com/) that you may find useful included in the repository, including [Github Codespaces](https://github.com/features/codespaces) and [VSCode devcontainer](https://code.visualstudio.com/docs/remote/containers) support to automatically handle dependencies, environment setup, code formatting, and more.
+
+## Adding a new DBMS
+
+Please see the existing MySQL and PostgreSQL code for an example.
+
+## Java Development Notes
+
+### Compiler Warnings
+
+In an effort to enforce clean, safe, maintainable code, [PR #413](https://github.com/cmu-db/benchbase/pull/413) enabled the `-Werror` and `-Xlint:all` options for the `javac` compiler.
+
+This means that any compiler warnings will cause the build to fail.
+
+If you are seeing a build failure due to a compiler warning, please fix the warning or (on rare occassions) add an exception to the line causing the issue.
+
+### Avoid `var` keyword
+
+In general, we prefer to avoid the `var` keyword in favor of explicit types.
+
+#### Alternatives to arrays of generics
+
+Per the [Java Language Specification](https://docs.oracle.com/javase/tutorial/java/generics/restrictions.html#createArrays), arrays of generic types are not allowed and will cause compiler warnings (e.g., `unchecked cast`)
+
+In some cases, you can extend a generic type to create a non-generic type that can be used in an array.
+
+For instance,
+
+```java
+// Simple generic class overload to avoid some cast warnings.
+class SomeTypeList extends LinkedList<SomeType> {}
+
+SomeTypeList[] someTypeLists = new SomeTypeList[] {
+    new SomeTypeList(),
+    new SomeTypeList(),
+    new SomeTypeList(),
+};
+```
+
+#### `this-escape` warnings
+
+`possible 'this' escape before subclass is fully initialized`
+
+The `this-escape` warning above is caused by passing using `this.someOverridableMethod()` in a constructor.
+This could in theory cause problems with a subclass not being fully initialized when the method is called.
+
+Since many of our classes are not designed to be subclassed, we can safely ignore this warning by marking the class as `final` rather than completely rewrite the class initialization.

README.md

@@ -221,15 +221,17 @@ Please see the existing MySQL and PostgreSQL code for an example.
 
 ## Contributing
 
-We welcome all contributions! Please open a pull request. Common contributions may include:
+We welcome all contributions! Please open a [pull request](https://github.com/cmu-db/benchbase/pulls). Common contributions may include:
 
 - Adding support for a new DBMS.
 - Adding more tests of existing benchmarks.
 - Fixing any bugs or known issues.
 
+Please see the [CONTRIBUTING.md](./CONTRIBUTING.md) for addition notes.
+
 ## Known Issues
 
-Please use GitHub's issue tracker for all issues.
+Please use [GitHub's issue tracker](https://github.com/cmu-db/benchbase/issues) for all issues.
 
 ## Credits
 

pom.xml

@@ -336,6 +336,20 @@
                 <configuration>
                     <source>${maven.compiler.source}</source>
                     <target>${maven.compiler.target}</target>
+                    <showDeprecation>true</showDeprecation>
+                    <showWarnings>true</showWarnings>
+                    <compilerArgs>
+                        <!-- Enable all linter messages for the compiler. -->
+                        <!-- TODO: Remove annotations preprocess warning exception. -->
+                        <arg>-Xlint:all,-processing</arg>
+                        <!-- ignore annotation processing on generated class files -->
+                        <arg>-implicit:class</arg>
+                        <!-- TODO: Enable documentation linting.
+                        <arg>-Xdoclint:all</arg>
+                        -->
+                        <!-- Turn all warnings into errors to help keep the code clean. -->
+                        <arg>-Werror</arg>
+                    </compilerArgs>
                 </configuration>
             </plugin>
             <plugin>

src/main/java/com/oltpbenchmark/DistributionStatistics.java

@@ -27,6 +27,7 @@
 import java.util.concurrent.TimeUnit;
 
 public class DistributionStatistics {
+    @SuppressWarnings("unused")
     private static final Logger LOG = LoggerFactory.getLogger(DistributionStatistics.class);
 
     private static final double[] PERCENTILES = {0.0, 0.25, 0.5, 0.75, 0.9, 0.95, 0.99, 1.0};

src/main/java/com/oltpbenchmark/WorkloadState.java

@@ -42,6 +42,7 @@ public class WorkloadState {
     private final Iterator<Phase> phaseIterator;
 
     private int workersWaiting = 0;
+    @SuppressWarnings("unused") // never read
     private int workersWorking = 0;
     private int workerNeedSleep;
 
@@ -60,7 +61,7 @@ public WorkloadState(BenchmarkState benchmarkState, List<Phase> works, int num_t
      */
     public void addToQueue(int amount, boolean resetQueues) {
         int workAdded = 0;
-        
+
         synchronized (this) {
             if (resetQueues) {
                 workQueue.clear();
@@ -71,7 +72,7 @@ public void addToQueue(int amount, boolean resetQueues) {
                     || !currentPhase.isRateLimited() || currentPhase.isSerial()) {
                 return;
             }
-            
+
             // Add the specified number of procedures to the end of the queue.
             // If we can't keep up with current rate, truncate transactions
             for (int i = 0; i < amount && workQueue.size() <= RATE_QUEUE_LIMIT; ++i) {

src/main/java/com/oltpbenchmark/api/BenchmarkModule.java

@@ -77,14 +77,15 @@ public abstract class BenchmarkModule {
     public BenchmarkModule(WorkloadConfiguration workConf) {
         this.workConf = workConf;
         this.dialects = new StatementDialects(workConf);
-        initClassLoader();
+        // setClassLoader();
+        this.classLoader = ClassLoader.getSystemClassLoader();
     }
 
     /**
      * Instantiates the classLoader variable, needs to be overwritten if
      * benchmark uses a custom implementation.
      */
-    protected void initClassLoader() {
+    protected void setClassLoader() {
         this.classLoader = ClassLoader.getSystemClassLoader();
     }
 

src/main/java/com/oltpbenchmark/api/StatementDialects.java

@@ -20,7 +20,6 @@
 import com.oltpbenchmark.WorkloadConfiguration;
 import com.oltpbenchmark.api.dialects.*;
 import com.oltpbenchmark.types.DatabaseType;
-import com.oltpbenchmark.types.State;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.xml.sax.SAXException;
@@ -30,7 +29,6 @@
 import javax.xml.transform.stream.StreamSource;
 import javax.xml.validation.Schema;
 import javax.xml.validation.SchemaFactory;
-import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.StringWriter;
@@ -40,7 +38,7 @@
 /**
  * @author pavlo
  */
-public class StatementDialects {
+public final class StatementDialects {
     private static final Logger LOG = LoggerFactory.getLogger(StatementDialects.class);
 
     private static final DatabaseType DEFAULT_DB_TYPE = DatabaseType.MYSQL;

src/main/java/com/oltpbenchmark/benchmarks/auctionmark/AuctionMarkBenchmark.java

@@ -33,11 +33,10 @@
 import java.util.ArrayList;
 import java.util.List;
 
-public class AuctionMarkBenchmark extends BenchmarkModule {
-
+public final class AuctionMarkBenchmark extends BenchmarkModule {
+    @SuppressWarnings("unused")
     private static final Logger LOG = LoggerFactory.getLogger(AuctionMarkBenchmark.class);
 
-
     private final RandomGenerator rng = new RandomGenerator((int) System.currentTimeMillis());
 
     public AuctionMarkBenchmark(WorkloadConfiguration workConf) {
@@ -70,6 +69,4 @@ protected List<Worker<? extends BenchmarkModule>> makeWorkersImpl() {
     protected Loader<AuctionMarkBenchmark> makeLoaderImpl() {
         return new AuctionMarkLoader(this);
     }
-
-
 }

src/main/java/com/oltpbenchmark/benchmarks/auctionmark/AuctionMarkLoader.java

@@ -46,7 +46,7 @@
  * @author pavlo
  * @author visawee
  */
-public class AuctionMarkLoader extends Loader<AuctionMarkBenchmark> {
+public final class AuctionMarkLoader extends Loader<AuctionMarkBenchmark> {
 
     // -----------------------------------------------------------------
     // INTERNAL DATA MEMBERS
@@ -1195,6 +1195,7 @@ protected class ItemBidGenerator extends SubTableGenerator<LoaderItemInfo> {
         private LoaderItemInfo.Bid bid = null;
         private float currentBidPriceAdvanceStep;
         private long currentCreateDateAdvanceStep;
+        @SuppressWarnings("unused") // only ever assigned
         private float currentPrice;
         private boolean new_item;
 

src/main/java/com/oltpbenchmark/benchmarks/auctionmark/AuctionMarkProfile.java

@@ -76,6 +76,7 @@ public class AuctionMarkProfile {
     // ----------------------------------------------------------------
     // SERIALIZABLE DATA MEMBERS
     // ----------------------------------------------------------------
+    static final long serialVersionUID = 0;
 
     /**
      * Database Scale Factor
@@ -108,6 +109,14 @@ public class AuctionMarkProfile {
      */
     protected transient Histogram<Integer> items_per_category = new Histogram<>();
 
+    /**
+     * Simple generic class overload to avoid some cast warnings below.
+     */
+    class ItemInfoList extends LinkedList<ItemInfo> {
+        // Required serialization field.
+        static final long serialVersionUID = 0;
+    }
+
     /**
      * Three status types for an item:
      * (1) Available - The auction of this item is still open
@@ -117,13 +126,13 @@ public class AuctionMarkProfile {
      * (3) Complete (The auction is closed and (There is no bid winner or
      * the bid winner has already purchased the item)
      */
-    private transient final LinkedList<ItemInfo> items_available = new LinkedList<>();
-    private transient final LinkedList<ItemInfo> items_endingSoon = new LinkedList<>();
-    private transient final LinkedList<ItemInfo> items_waitingForPurchase = new LinkedList<>();
-    private transient final LinkedList<ItemInfo> items_completed = new LinkedList<>();
+    private transient final ItemInfoList items_available = new ItemInfoList();
+    private transient final ItemInfoList items_endingSoon = new ItemInfoList();
+    private transient final ItemInfoList items_waitingForPurchase = new ItemInfoList();
+    private transient final ItemInfoList items_completed = new ItemInfoList();
+
 
-    @SuppressWarnings("unchecked")
-    protected transient final LinkedList<ItemInfo>[] allItemSets = new LinkedList[]{
+    protected transient final ItemInfoList allItemSets[] = new ItemInfoList[]{
             this.items_available,
             this.items_endingSoon,
             this.items_waitingForPurchase,
@@ -286,7 +295,7 @@ private AuctionMarkProfile copyProfile(AuctionMarkWorker worker, AuctionMarkProf
         this.items_per_category = other.items_per_category;
         this.gag_ids = other.gag_ids;
 
-        // Initialize the UserIdGenerator so we can figure out whether our 
+        // Initialize the UserIdGenerator so we can figure out whether our
         // client should even have these ids
         this.initializeUserIdGenerator(this.client_id);
 
@@ -455,7 +464,6 @@ private static void loadItems(AuctionMarkProfile profile, List<Object[]> vt) {
 
     private static void loadPendingItemComments(AuctionMarkProfile profile, List<Object[]> vt) {
         for (Object[] row : vt) {
-            int col = 1;
             long ic_id = SQLUtil.getLong(row[0]);
             String ic_i_id = SQLUtil.getString(row[1]);
             String ic_u_id = SQLUtil.getString(row[2]);
@@ -712,6 +720,7 @@ private boolean addItem(LinkedList<ItemInfo> items, ItemInfo itemInfo) {
             // HACK: Always swap existing ItemInfos with our new one, since it will
             // more up-to-date information
             ItemInfo existing = items.set(idx, itemInfo);
+            assert existing != null;
 
             return (true);
         }
@@ -861,7 +870,7 @@ private ItemInfo getRandomItem(LinkedList<ItemInfo> itemSet, boolean needCurrent
                 continue;
             }
 
-            // If they want an item that is ending in the future, then we compare it with 
+            // If they want an item that is ending in the future, then we compare it with
             // the current timestamp
             if (needFutureEndDate) {
                 boolean compareTo = (temp.getEndDate().compareTo(currentTime) < 0);