default nonnull + spotbugs + fix warnings

pom.xml

@@ -1,4 +1,6 @@
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
   <modelVersion>4.0.0</modelVersion>
 
   <artifactId>logging</artifactId>
@@ -93,10 +95,11 @@
       <version>4.11</version>
     </dependency>
 
+    <!-- Annotations-->
     <dependency>
-      <groupId>com.intellij</groupId>
-      <artifactId>annotations</artifactId>
-      <version>9.0.4</version>
+      <groupId>io.norberg</groupId>
+      <artifactId>some</artifactId>
+      <version>1.0.0</version>
       <scope>provided</scope>
     </dependency>
 
@@ -168,6 +171,25 @@
           </execution>
         </executions>
       </plugin>
+      <plugin>
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-maven-plugin</artifactId>
+        <version>4.1.4</version>
+        <configuration>
+          <effort>Max</effort>
+          <threshold>Low</threshold>
+          <failOnError>true</failOnError>
+          <xmlOutput>true</xmlOutput>
+        </configuration>
+        <executions>
+          <execution>
+            <phase>verify</phase>
+            <goals>
+              <goal>check</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
     </plugins>
   </build>
 </project>

src/main/java/com/spotify/logging/LoggingConfigurator.java

@@ -56,6 +56,7 @@
 import java.io.File;
 import java.lang.management.ManagementFactory;
 import java.nio.charset.StandardCharsets;
+import javax.annotation.Nullable;
 import net.logstash.logback.composite.loggingevent.ArgumentsJsonProvider;
 import org.slf4j.LoggerFactory;
 
@@ -91,6 +92,7 @@ public static String getMsgPattern(final ReplaceNewLines replaceNewLines) {
   public static final String SPOTIFY_SYSLOG_PORT = "SPOTIFY_SYSLOG_PORT";
   private static final String USE_JSON_LOGGING = "USE_JSON_LOGGING";
 
+  @SuppressWarnings("unused")
   public enum Level {
     OFF(ch.qos.logback.classic.Level.OFF),
     ERROR(ch.qos.logback.classic.Level.ERROR),
@@ -189,7 +191,7 @@ public static void configureDefaults(
     // Call configureSyslogDefaults if the SPOTIFY_SYSLOG_HOST or SPOTIFY_SYSLOG_PORT env var is
     // set. If this causes a problem, we could introduce a configureConsoleDefaults method which
     // users could call instead to avoid this behavior.
-    final String syslogHost = getSyslogHost();
+    final @Nullable String syslogHost = getSyslogHost();
     final int syslogPort = getSyslogPort();
     if (syslogHost != null || syslogPort != -1) {
       configureSyslogDefaults(ident, level, syslogHost, syslogPort, replaceNewLines);
@@ -229,7 +231,7 @@ public static void configureLogstashEncoderDefaults(final Level level) {
     encoder.addProvider(new ArgumentsJsonProvider());
     encoder.start();
 
-    final ConsoleAppender<ILoggingEvent> appender = new ConsoleAppender<ILoggingEvent>();
+    final ConsoleAppender<ILoggingEvent> appender = new ConsoleAppender<>();
     appender.setTarget("System.out");
     appender.setName("stdout");
     appender.setEncoder(encoder);
@@ -262,7 +264,7 @@ public static void configureSyslogDefaults(
       final String ident, final Level level, final ReplaceNewLines replaceNewLines) {
     final String syslogHost = getenv(SPOTIFY_SYSLOG_HOST);
     final String port = getenv(SPOTIFY_SYSLOG_PORT);
-    final int syslogPort = port == null ? -1 : Integer.valueOf(port);
+    final int syslogPort = port == null ? -1 : Integer.parseInt(port);
     configureSyslogDefaults(ident, level, syslogHost, syslogPort, replaceNewLines);
   }
 
@@ -278,7 +280,7 @@ public static void configureSyslogDefaults(
   public static void configureSyslogDefaults(
       final String ident,
       final Level level,
-      final String host,
+      final @Nullable String host,
       final int port,
       final ReplaceNewLines replaceNewLines) {
     configureSyslogDefaults(ident, level, host, port, Logger.ROOT_LOGGER_NAME, replaceNewLines);
@@ -297,7 +299,7 @@ public static void configureSyslogDefaults(
   public static void configureSyslogDefaults(
       final String ident,
       final Level level,
-      final String host,
+      final @Nullable String host,
       final int port,
       final String loggerName,
       final ReplaceNewLines replaceNewLines) {
@@ -374,7 +376,7 @@ private static Appender<ILoggingEvent> getStdErrAppender(
     encoder.start();
 
     // Setup stderr appender
-    final ConsoleAppender<ILoggingEvent> appender = new ConsoleAppender<ILoggingEvent>();
+    final ConsoleAppender<ILoggingEvent> appender = new ConsoleAppender<>();
     appender.setTarget("System.err");
     appender.setName("stderr");
     appender.setEncoder(encoder);
@@ -395,7 +397,7 @@ private static Appender<ILoggingEvent> getStdErrAppender(
    */
   static Appender<ILoggingEvent> getSyslogAppender(
       final LoggerContext context,
-      final String host,
+      final @Nullable String host,
       final int port,
       final ReplaceNewLines replaceNewLines) {
     final String h = (host == null || host.isEmpty()) ? "localhost" : host;
@@ -430,6 +432,7 @@ static Appender<ILoggingEvent> getSyslogAppender(
    *
    * @deprecated Don't use, see docs.
    */
+  @Deprecated
   static void configure(final JewelCliLoggingOptions opts) {
     // Use logback config file to setup logging if specified, discarding any other logging options.
     if (!opts.logFileName().isEmpty()) {
@@ -448,7 +451,7 @@ static void configure(final JewelCliLoggingOptions opts) {
     // See if syslog host was specified via command line or environment variable.
     // The command line value takes precedence, which defaults to an empty string.
     String syslogHost = opts.syslogHost();
-    if (syslogHost == null || syslogHost.isEmpty()) {
+    if (syslogHost.isEmpty()) {
       syslogHost = getSyslogHost();
     }
 
@@ -561,7 +564,7 @@ private static String getMyPid() {
     return pid;
   }
 
-  private static String getSyslogHost() {
+  private static @Nullable String getSyslogHost() {
     final String host = System.getenv().getOrDefault(SPOTIFY_SYSLOG_HOST, "");
     return host.isEmpty() ? null : host;
   }
@@ -571,7 +574,7 @@ private static int getSyslogPort() {
     return port.isEmpty() ? -1 : Integer.parseInt(port);
   }
 
-  private static String getSpotifyHostname() {
+  private static @Nullable String getSpotifyHostname() {
     final String hostname = System.getenv().getOrDefault(SPOTIFY_HOSTNAME, "");
     return hostname.isEmpty() ? null : hostname;
   }

src/main/java/com/spotify/logging/LoggingSupport.java

@@ -37,6 +37,7 @@
 package com.spotify.logging;
 
 import java.util.concurrent.atomic.AtomicLong;
+import javax.annotation.Nullable;
 import org.slf4j.Logger;
 
 /** Utility for emitting log messages in the Spotify log format. */
@@ -109,7 +110,7 @@ public static void debug(
       final Logger logger,
       final String type,
       final int version,
-      final Ident ident,
+      final @Nullable Ident ident,
       final Object... args) {
 
     logger.debug(buildLogLine(type, version, ident, args));
@@ -124,7 +125,7 @@ public static void info(
       final Logger logger,
       final String type,
       final int version,
-      final Ident ident,
+      final @Nullable Ident ident,
       final Object... args) {
 
     logger.info(buildLogLine(type, version, ident, args));
@@ -139,7 +140,7 @@ public static void warn(
       final Logger logger,
       final String type,
       final int version,
-      final Ident ident,
+      final @Nullable Ident ident,
       final Object... args) {
 
     logger.warn(buildLogLine(type, version, ident, args));
@@ -154,14 +155,14 @@ public static void error(
       final Logger logger,
       final String type,
       final int version,
-      final Ident ident,
+      final @Nullable Ident ident,
       final Object... args) {
 
     logger.error(buildLogLine(type, version, ident, args));
   }
 
   protected static String buildLogLine(
-      final String type, final int version, final Ident ident, final Object... args) {
+      final String type, final int version, @Nullable final Ident ident, final Object... args) {
     final StringBuilder line = new StringBuilder();
     line.append(LoggingSupport.rid.getAndIncrement()).append(' ');
 
@@ -182,7 +183,7 @@ protected static String buildLogLine(
     return line.toString();
   }
 
-  protected static void appendEscaped(final Object o, final StringBuilder out) {
+  protected static void appendEscaped(final @Nullable Object o, final StringBuilder out) {
     if (o == null) {
       return;
     }

src/main/java/com/spotify/logging/UncaughtExceptionLogger.java

@@ -45,11 +45,6 @@ public class UncaughtExceptionLogger {
 
   public static void setDefaultUncaughtExceptionHandler() {
     Thread.setDefaultUncaughtExceptionHandler(
-        new Thread.UncaughtExceptionHandler() {
-          @Override
-          public void uncaughtException(final Thread th, final Throwable t) {
-            logger.error("Uncaught exception in thread " + th, t);
-          }
-        });
+        (th, t) -> logger.error("Uncaught exception in thread " + th, t));
   }
 }

src/main/java/com/spotify/logging/logback/LevelRangeFilter.java

@@ -40,6 +40,7 @@
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.filter.Filter;
 import ch.qos.logback.core.spi.FilterReply;
+import javax.annotation.Nullable;
 
 /**
  * Filters events within the levels levelMin and levelMax, inclusive.
@@ -50,8 +51,8 @@
  */
 public class LevelRangeFilter extends Filter<ILoggingEvent> {
 
-  private Level levelMax;
-  private Level levelMin;
+  private @Nullable Level levelMax;
+  private @Nullable Level levelMin;
 
   @Override
   public FilterReply decide(final ILoggingEvent event) {

src/main/java/com/spotify/logging/logback/LoggerThresholdFilter.java

@@ -40,6 +40,7 @@
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.filter.Filter;
 import ch.qos.logback.core.spi.FilterReply;
+import javax.annotation.Nullable;
 
 /**
  * Comparable to ch.qos.logback.classic.filter.ThresholdFilter, but for a specific logger. Filters
@@ -48,20 +49,27 @@
  */
 public class LoggerThresholdFilter extends Filter<ILoggingEvent> {
 
-  private String logger;
-  private Level level;
-  private String exceptLogger;
+  private @Nullable String logger;
+  private @Nullable Level level;
+  private @Nullable String exceptLogger;
 
   @Override
   public FilterReply decide(ILoggingEvent event) {
-    if (!isStarted()) return FilterReply.NEUTRAL;
+    if (!isStarted()) {
+      return FilterReply.NEUTRAL;
+    }
 
-    if (logger != null && !event.getLoggerName().startsWith(logger)) return FilterReply.NEUTRAL;
+    if (logger != null && !event.getLoggerName().startsWith(logger)) {
+      return FilterReply.NEUTRAL;
+    }
 
-    if (exceptLogger != null && event.getLoggerName().startsWith(exceptLogger))
+    if (exceptLogger != null && event.getLoggerName().startsWith(exceptLogger)) {
       return FilterReply.NEUTRAL;
+    }
 
-    if (level != null && !event.getLevel().isGreaterOrEqual(level)) return FilterReply.DENY;
+    if (level != null && !event.getLevel().isGreaterOrEqual(level)) {
+      return FilterReply.DENY;
+    }
 
     return FilterReply.NEUTRAL;
   }

src/main/java/com/spotify/logging/logback/MillisecondPrecisionSyslogAppender.java

@@ -49,12 +49,13 @@
 import java.lang.reflect.Field;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import javax.annotation.Nullable;
 
 /** A {@link SyslogAppender} with millisecond timestamp precision. */
 public class MillisecondPrecisionSyslogAppender extends SyslogAppender {
   private Charset charset = StandardCharsets.UTF_8;
-  PatternLayout stackTraceLayout = new PatternLayout();
-  private OutputStream sos;
+  private final PatternLayout stackTraceLayout = new PatternLayout();
+  private @Nullable OutputStream sos;
 
   @Override
   public void start() {
@@ -82,6 +83,7 @@ protected void append(ILoggingEvent eventObject) {
       if (msg.length() > getMaxMessageSize()) {
         msg = msg.substring(0, getMaxMessageSize());
       }
+      assert sos != null;
       sos.write(msg.getBytes(charset));
       sos.flush();
       postProcess(eventObject, sos);
@@ -112,7 +114,7 @@ private void recursiveWrite(
       final String stackTracePrefix,
       final IThrowableProxy tp,
       final int indent,
-      final String firstLinePrefix) {
+      final @Nullable String firstLinePrefix) {
     final StackTraceElementProxy[] stepArray = tp.getStackTraceElementProxyArray();
     try {
       handleThrowableFirstLine(sw, tp, stackTracePrefix, indent, firstLinePrefix);
@@ -121,7 +123,7 @@ private void recursiveWrite(
         sb.append(stackTracePrefix);
         addIndent(sb, indent);
         sb.append(step);
-        sw.write(sb.toString().getBytes());
+        sw.write(sb.toString().getBytes(StandardCharsets.UTF_8));
         sw.flush();
       }
     } catch (IOException e) {
@@ -153,15 +155,15 @@ private void handleThrowableFirstLine(
       final IThrowableProxy tp,
       final String stackTracePrefix,
       final int indent,
-      final String prefix)
+      final @Nullable String prefix)
       throws IOException {
     StringBuilder sb = new StringBuilder().append(stackTracePrefix);
     addIndent(sb, indent);
     if (prefix != null) {
       sb.append(prefix);
     }
     sb.append(tp.getClassName()).append(": ").append(tp.getMessage());
-    sw.write(sb.toString().getBytes());
+    sw.write(sb.toString().getBytes(StandardCharsets.UTF_8));
     sw.flush();
   }