Fix some warnings and make them appear in Bazel

.bazelrc

@@ -13,37 +13,4 @@ test --jvmopt="-Duser.country=US"
 test --jvmopt="-Duser.timezone=America/Los_Angeles"
 test --jvmopt="-Dfile.encoding=UTF-8"
 
-# Default javac options
-build --javacopt="-encoding UTF-8"
-build --javacopt="-Xlint:deprecation"
-build --javacopt="-Xlint:unchecked"
-build --javacopt="-Xlint:-removal"
-test --javacopt="-Xlint:-deprecation"
-
-# Set up Errorprone for success
-build --javacopt="-Xep:JUnit4TestNotRun:OFF
-build --javacopt="-Xep:ReturnValueIgnored:OFF"
-build --javacopt="-Xep:IdentityBinaryExpression:OFF"
-build --javacopt="-Xep:ClassInitializationDeadlock:OFF"
-build --javacopt="-Xep:EffectivelyPrivate:OFF"
-build --javacopt="-Xep:BooleanLiteral:OFF"
-build --javacopt="-Xep:AssignmentExpression:OFF"
-build --javacopt="-Xep:DuplicateBranches:OFF"
-build --javacopt="-Xep:PatternMatchingInstanceof:OFF"
-build --javacopt="-Xep:StatementSwitchToExpressionSwitch:OFF"
-build --javacopt="-Xep:RedundantControlFlow:OFF"
-build --javacopt="-Xep:AlmostJavadoc:OFF"
-build --javacopt="-Xep:EmptyBlockTag:OFF"
-build --javacopt="-Xep:EscapedEntity:OFF"
-build --javacopt="-Xep:MissingSummary:OFF"
-build --javacopt="-Xep:InvalidBlockTag:OFF"
-build --javacopt="-Xep:NotJavadoc:OFF"
-build --javacopt="-Xep:UnicodeEscape:OFF"
-build --javacopt="-Xep:EmptyCatch:OFF"
-build --javacopt="-Xep:LabelledBreakTarget:OFF"
-build --javacopt="-Xep:JavaUtilDate:OFF"
-build --javacopt="-Xep:InlineMeSuggester:OFF"
-build --javacopt="-Xep:UnusedVariable:OFF"
-build --javacopt="-Xep:AnnotateFormatMethod:OFF"
-
 try-import %workspace%/.bazelrc.user

bazel/errorprone.bzl

@@ -0,0 +1,41 @@
+DEFAULT_OPTS = [
+    "-XepAllErrorsAsWarnings",
+]
+
+LIBRARY_OPTS = DEFAULT_OPTS + [
+    "-XepAllSuggestionsAsWarnings",
+    "-Xlint:deprecation",
+    "-Xlint:unchecked",
+    "-Xlint:-removal",
+    # Checks we aren't ready to fix yet
+    "-Xep:ClassInitializationDeadlock:OFF",
+    "-Xep:EffectivelyPrivate:OFF",
+    "-Xep:BooleanLiteral:OFF",
+    "-Xep:AssignmentExpression:OFF",
+    "-Xep:DuplicateBranches:OFF",
+    "-Xep:PatternMatchingInstanceof:OFF",
+    "-Xep:StatementSwitchToExpressionSwitch:OFF",
+    "-Xep:EmptyBlockTag:OFF",
+    "-Xep:EscapedEntity:OFF",
+    "-Xep:MissingSummary:OFF",
+    "-Xep:InvalidBlockTag:OFF",
+    "-Xep:UnicodeEscape:OFF",
+    "-Xep:EmptyCatch:OFF",
+    "-Xep:LabelledBreakTarget:OFF",
+    "-Xep:JavaUtilDate:OFF",
+    "-Xep:InlineMeSuggester:OFF",
+    "-Xep:UnusedVariable:OFF",
+    "-Xep:AnnotateFormatMethod:OFF",
+    "-Xep:ImmutableEnumChecker:OFF",
+    "-Xep:DoNotCallSuggester:OFF",
+    # Additional useful checks
+    "-Xep:RemoveUnusedImports:WARN",
+    "-Xep:RemoveWildcardImport:WARN",
+    "-Xep:UnusedMethod:WARN",
+    "-Xep:RedundantControlFlow:WARN",
+]
+
+TEST_OPTS = DEFAULT_OPTS + [
+    "-Xlint:-deprecation",
+    "-XepDisableAllChecks",
+]

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

@@ -31,15 +31,14 @@ tasks.withType(JavaCompile).configureEach {
                 "AssignmentExpression",
                 "DuplicateBranches",
                 "PatternMatchingInstanceof",
-                "RedundantControlFlow",
                 "StatementSwitchToExpressionSwitch",
+                "ImmutableEnumChecker",
+                "DoNotCallSuggester",
                 // JavaDoc stuff -- not going to fix unless we do a big JavaDoc update
-                "AlmostJavadoc",
                 "EmptyBlockTag",
                 "EscapedEntity",
                 "MissingSummary",
                 "InvalidBlockTag",
-                "NotJavadoc",
                 "UnicodeEscape",
                 // Stuff that we just love to do but should do less of eventually
                 "EmptyCatch",
@@ -51,7 +50,7 @@ tasks.withType(JavaCompile).configureEach {
                 // or spuriously for local variables in my opinion.
                 "UnusedVariable",
                 // This requires annotations that are difficult to incorporate
-                // without adding a dependency for everyon who uses Rhino.
+                // without adding a dependency for everyone who uses Rhino.
                 "AnnotateFormatMethod"
         )
         options.errorprone.enable(

examples/build.gradle

@@ -7,3 +7,8 @@ dependencies {
     implementation project(':rhino-tools')
     implementation project(':rhino-xml')
 }
+
+tasks.withType(Javadoc) {
+    failOnError = false
+    options.addStringOption('Xdoclint:all,-missing', '-quiet')
+}

examples/src/main/java/Shell.java

@@ -99,7 +99,7 @@ public static String[] processOptions(Context cx, String args[]) {
             if (arg.equals("-version")) {
                 if (++i == args.length) usage(arg);
                 double d = Context.toNumber(args[i]);
-                if (d != d) usage(arg);
+                if (Double.isNaN(d)) usage(arg);
                 cx.setLanguageVersion((int) d);
                 continue;
             }

rhino-engine/BUILD.bazel

@@ -1,9 +1,11 @@
 load("@contrib_rules_jvm//java:defs.bzl", "JUNIT5_DEPS", "java_test_suite")
 load("@rules_java//java:defs.bzl", "java_library")
+load("//bazel:errorprone.bzl", "LIBRARY_OPTS", "TEST_OPTS")
 
 java_library(
     name = "rhino-engine",
     srcs = glob(["src/main/java/**/*.java"]),
+    javacopts = LIBRARY_OPTS,
     resources = glob(["src/main/resources/**"]),
     visibility = ["//visibility:public"],
     deps = ["//rhino"],
@@ -17,8 +19,8 @@ java_test_suite(
         exclude = ["**/module-info.java"],
     ),
     data = ["//tests:testsrc"],
+    javacopts = TEST_OPTS,
     runner = "junit5",
-    tags = ["no-agent"],
     test_suffixes = ["Test.java"],
     deps = [
         ":rhino-engine",

rhino-kotlin/BUILD.bazel

@@ -1,10 +1,12 @@
 load("@contrib_rules_jvm//java:defs.bzl", "JUNIT5_DEPS", "java_junit5_test")
 load("@rules_java//java:defs.bzl", "java_library")
 load("@rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library")
+load("//bazel:errorprone.bzl", "LIBRARY_OPTS", "TEST_OPTS")
 
 java_library(
     name = "rhino-kotlin",
     srcs = glob(["src/main/java/**/*.java"]),
+    javacopts = LIBRARY_OPTS,
     resources = glob(["src/main/resources/**"]),
     visibility = ["//visibility:public"],
     deps = [
@@ -28,7 +30,7 @@ java_junit5_test(
     name = "rhino-kotlin_test",
     size = "small",
     srcs = ["src/test/java/org/mozilla/kotlin/tests/KotlinNullabilityDetectorTest.java"],
-    tags = ["no-agent"],
+    javacopts = TEST_OPTS,
     test_class = "org.mozilla.kotlin.tests.KotlinNullabilityDetectorTest",
     deps = [
         ":rhino-kotlin",

rhino-tools/BUILD.bazel

@@ -1,9 +1,11 @@
 load("@contrib_rules_jvm//java:defs.bzl", "JUNIT5_DEPS", "java_test_suite")
 load("@rules_java//java:defs.bzl", "java_binary", "java_library")
+load("//bazel:errorprone.bzl", "LIBRARY_OPTS", "TEST_OPTS")
 
 java_library(
     name = "rhino-tools",
     srcs = glob(["src/main/java/**/*.java"]),
+    javacopts = LIBRARY_OPTS,
     resources = glob(["src/main/resources/**"]),
     visibility = ["//visibility:public"],
     deps = [
@@ -15,6 +17,7 @@ java_library(
 
 java_binary(
     name = "shell",
+    javacopts = LIBRARY_OPTS,
     main_class = "org.mozilla.javascript.tools.shell.Main",
     visibility = ["//visibility:public"],
     runtime_deps = [
@@ -30,6 +33,7 @@ java_binary(
 
 java_binary(
     name = "debugger",
+    javacopts = LIBRARY_OPTS,
     main_class = "org.mozilla.javascript.tools.debugger.Main",
     visibility = ["//visibility:public"],
     runtime_deps = [
@@ -41,6 +45,7 @@ java_binary(
 
 java_binary(
     name = "jsc",
+    javacopts = LIBRARY_OPTS,
     main_class = "org.mozilla.javascript.tools.jsc.Main",
     visibility = ["//visibility:public"],
     runtime_deps = [
@@ -56,8 +61,8 @@ java_test_suite(
         ["src/test/java/**/*.java"],
         exclude = ["**/module-info.java"],
     ),
+    javacopts = TEST_OPTS,
     runner = "junit5",
-    tags = ["no-agent"],
     test_suffixes = ["Test.java"],
     deps = [
         ":rhino-tools",

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

@@ -74,9 +74,9 @@ public void removeCellEditorListener(CellEditorListener l) {
         listenerList.remove(CellEditorListener.class, l);
     }
 
-    /*
-     * Notify all listeners that have registered interest for
-     * notification on this event type.
+    /**
+     * Notify all listeners that have registered interest for notification on this event type.
+     *
      * @see EventListenerList
      */
     protected void fireEditingStopped() {
@@ -91,9 +91,9 @@ protected void fireEditingStopped() {
         }
     }
 
-    /*
-     * Notify all listeners that have registered interest for
-     * notification on this event type.
+    /**
+     * Notify all listeners that have registered interest for notification on this event type.
+     *
      * @see EventListenerList
      */
     protected void fireEditingCanceled() {

rhino-xml/BUILD.bazel

@@ -1,9 +1,11 @@
 load("@contrib_rules_jvm//java:defs.bzl", "JUNIT5_DEPS", "java_junit5_test")
 load("@rules_java//java:defs.bzl", "java_library")
+load("//bazel:errorprone.bzl", "LIBRARY_OPTS", "TEST_OPTS")
 
 java_library(
     name = "rhino-xml",
     srcs = glob(["src/main/java/**/*.java"]),
+    javacopts = LIBRARY_OPTS,
     resources = glob(["src/main/resources/**"]),
     visibility = ["//visibility:public"],
     deps = ["//rhino"],
@@ -12,6 +14,7 @@ java_library(
 java_library(
     name = "rhino-xml-tests",
     srcs = glob(["src/test/java/**/*.java"]),
+    javacopts = TEST_OPTS,
     deps = [
         ":rhino-xml",
         "//rhino",
@@ -30,6 +33,7 @@ java_junit5_test(
     name = "XMLSecureParserTest",
     size = "small",
     srcs = glob(["**/XMLSecureParserTest.java"]),
+    javacopts = TEST_OPTS,
     tags = ["no-agent"],
     test_class = "org.mozilla.javascript.tests.XMLSecureParserTest",
     deps = [
@@ -46,7 +50,7 @@ java_junit5_test(
     name = "E4XBasicTest",
     size = "small",
     srcs = glob(["**/E4XBasicTest.java"]),
-    tags = ["no-agent"],
+    javacopts = TEST_OPTS,
     test_class = "org.mozilla.javascript.xmlimpl.tests.E4XBasicTest",
     deps = [
         ":rhino-xml",
@@ -62,7 +66,7 @@ java_junit5_test(
     name = "XmlNonResettableDocumentBuilderTest",
     size = "small",
     srcs = glob(["**/XmlNonResettableDocumentBuilderTest.java"]),
-    tags = ["no-agent"],
+    javacopts = TEST_OPTS,
     test_class = "org.mozilla.javascript.xmlimpl.tests.XmlNonResettableDocumentBuilderTest",
     deps = [
         ":rhino-xml",

rhino/BUILD.bazel

@@ -1,9 +1,11 @@
 load("@contrib_rules_jvm//java:defs.bzl", "JUNIT5_DEPS", "java_test_suite")
 load("@rules_java//java:defs.bzl", "java_library")
+load("//bazel:errorprone.bzl", "LIBRARY_OPTS", "TEST_OPTS")
 
 java_library(
     name = "rhino",
     srcs = glob(["src/main/java/**/*.java"]),
+    javacopts = LIBRARY_OPTS,
     resources = glob(["src/main/resources/**"]),
     visibility = ["//visibility:public"],
 )
@@ -16,9 +18,9 @@ java_test_suite(
         "//:rhino-test.config",
         "//tests:testsrc",
     ],
+    javacopts = TEST_OPTS,
     resources = glob(["src/test/resources/**"]),
     runner = "junit5",
-    tags = ["no-agent"],
     test_suffixes = ["Test.java"],
     deps = [
         ":rhino",

rhino/src/main/java/org/mozilla/javascript/NativeObject.java

@@ -550,13 +550,11 @@ private static Object js_getOwnPropDescs(
         }
         for (Object key : ids) {
             var desc = obj.getOwnPropertyDescriptor(cx, key);
-            if (desc == null) {
-                continue;
-            } else if (key instanceof Symbol) {
+            if (key instanceof Symbol) {
                 descs.put((Symbol) key, descs, desc.toObject(s));
             } else if (key instanceof Integer) {
                 descs.put((Integer) key, descs, desc.toObject(s));
-            } else {
+            } else if (desc != null) {
                 descs.put(ScriptRuntime.toString(key), descs, desc.toObject(s));
             }
         }

rhino/src/main/java/org/mozilla/javascript/Parser.java

@@ -259,18 +259,6 @@ private MappedLocation(int line, String lineSource, int offset) {
             this.offset = offset;
         }
 
-        public int getLine() {
-            return line;
-        }
-
-        public String getLineSource() {
-            return lineSource;
-        }
-
-        public int getOffset() {
-            return offset;
-        }
-
         @Override
         public boolean equals(Object obj) {
             if (obj == this) return true;

rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java

@@ -4638,7 +4638,6 @@ private static boolean eqString(CharSequence x, Object y) {
                     }
                 }
                 y = toPrimitive(y);
-                continue;
             } else {
                 warnAboutNonJSObject(y);
                 return false;