Adopt "errorprone" tool for static analysis

Compared to spotbugs, errorprone seems to run faster, finds a number of
useful things, is easier to configure, and is updated often.

* Add errorprone to the build for all non-test code
* Suppress or disable warnings that are too much for us to fix now
* Fix other errors and warnings
* Remove spotbugs (it and errorprone don't always agree)

buildSrc/build.gradle

@@ -8,5 +8,5 @@ repositories {
 
 dependencies {
     implementation 'com.diffplug.spotless:spotless-plugin-gradle:6.25.0'
-    implementation 'com.github.spotbugs.snom:spotbugs-gradle-plugin:6.0.15'
+    implementation 'net.ltgt.gradle:gradle-errorprone-plugin:4.0.0'
 }

buildSrc/src/main/groovy/rhino.library-conventions.gradle

@@ -1,13 +1,14 @@
 plugins {
     id 'rhino.java-conventions'
-    id 'com.github.spotbugs'
     id 'maven-publish'
     id 'checkstyle'
     id 'jacoco'
+    id 'net.ltgt.errorprone'
 }
 
-import com.github.spotbugs.snom.Confidence
-import com.github.spotbugs.snom.Effort
+dependencies {
+    errorprone "com.google.errorprone:error_prone_core:2.28.0"
+}
 
 version = project.version
 
@@ -26,20 +27,30 @@ tasks.withType(Jar).configureEach {
     }
 }
 
-spotbugs {
-    effort = Effort.valueOf('LESS')
-    reportLevel = Confidence.valueOf('MEDIUM')
-    excludeFilter = file("${projectDir}/../config/spotbugs/spotbugs-exclude.xml")
-}
-
-spotbugsMain {
-    reports {
-        html {
-            required = true
-            outputLocation = file("$buildDir/reports/spotbugs/main/spotbugs.html")
-            stylesheet = 'fancy-hist.xsl'
-        }
-    }
+tasks.withType(JavaCompile).configureEach {
+    options.errorprone.disable(
+        // JavaDoc stuff
+        "AlmostJavadoc",
+        "EmptyBlockTag",
+        "EscapedEntity",
+        "MissingSummary",
+        "InvalidBlockTag",
+        "InvalidLink",
+        "InvalidParam",
+        "NotJavadoc",
+        "UnicodeEscape",
+        // Great ideas for when minimum version is more than Java 11
+        "PatternMatchingInstanceof",
+        // Stuff that we just love to do but should do less of eventually
+        "EmptyCatch",
+        "LabelledBreakTarget",
+        "JavaUtilDate",
+        // Less important for now, more stylistic than bug
+        "InlineMeSuggester",
+        "NonApiType",
+        "UnnecessaryParentheses",
+        "UnusedVariable")
+    options.errorprone.excludedPaths = '.+/src/test/java/.+'
 }
 
 jacocoTestReport {

rhino-engine/src/main/java/org/mozilla/javascript/engine/Builtins.java

@@ -7,6 +7,7 @@
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
+import java.nio.charset.StandardCharsets;
 import javax.script.ScriptContext;
 import org.mozilla.javascript.Context;
 import org.mozilla.javascript.Function;
@@ -30,7 +31,7 @@ public class Builtins {
 
     void register(Context cx, ScriptableObject scope, ScriptContext sc) {
         if (sc.getWriter() == null) {
-            stdout = new OutputStreamWriter(System.out);
+            stdout = new OutputStreamWriter(System.out, StandardCharsets.UTF_8);
         } else {
             stdout = sc.getWriter();
         }

rhino-tools/src/main/java/org/mozilla/javascript/tools/debugger/Dim.java

@@ -11,6 +11,7 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -78,10 +79,10 @@ public class Dim {
     /**
      * Synchronization object used to allow script evaluations to happen when a thread is resumed.
      */
-    private Object monitor = new Object();
+    private final Object monitor = new Object();
 
     /** Synchronization object used to wait for valid {@link #interruptedContextData}. */
-    private Object eventThreadMonitor = new Object();
+    private final Object eventThreadMonitor = new Object();
 
     /** The action to perform to end the interruption loop. */
     private volatile int returnValue = -1;
@@ -258,7 +259,7 @@ private String loadSource(String sourceUrl) {
             }
 
             try {
-                source = Kit.readReader(new InputStreamReader(is));
+                source = Kit.readReader(new InputStreamReader(is, StandardCharsets.UTF_8));
             } finally {
                 is.close();
             }
@@ -1130,6 +1131,9 @@ public static class SourceInfo {
         /** Array indicating whether a breakpoint is set on the line. */
         private boolean[] breakpoints;
 
+        /** Lock for same */
+        private final Object breakpointsLock = new Object();
+
         /** Array of FunctionSource objects for the functions in the script. */
         private FunctionSource[] functionSources;
 
@@ -1260,7 +1264,7 @@ public boolean breakpoint(int line, boolean value) {
                 throw new IllegalArgumentException(String.valueOf(line));
             }
             boolean changed;
-            synchronized (breakpoints) {
+            synchronized (breakpointsLock) {
                 if (breakpoints[line] != value) {
                     breakpoints[line] = value;
                     changed = true;
@@ -1273,7 +1277,7 @@ public boolean breakpoint(int line, boolean value) {
 
         /** Removes all breakpoints from the script. */
         public void removeAllBreakpoints() {
-            synchronized (breakpoints) {
+            synchronized (breakpointsLock) {
                 for (int line = 0; line != breakpoints.length; ++line) {
                     breakpoints[line] = false;
                 }