fix: NullPointerException in SignOnlyFormat (#195)

`CompileTimeConstantExpressionMatcher` can match expressions that return
null from `ASTHelpers.constValue`.
So instead we directly rely on the latter for determining constants.

According to the error-prone team, this combination of helper methods
was not guaranteed to play together nicely and it just happened to work
before error-prone v2.14.0.

closes #193

src/main/java/jp/skypencil/errorprone/slf4j/FormatShouldBeConst.java

@@ -8,9 +8,9 @@
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
-import com.google.errorprone.matchers.CompileTimeConstantExpressionMatcher;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
 import com.sun.tools.javac.util.List;
@@ -26,9 +26,6 @@
 public class FormatShouldBeConst extends BugChecker implements MethodInvocationTreeMatcher {
   private static final long serialVersionUID = 3271269614137732880L;
 
-  private static final CompileTimeConstantExpressionMatcher IS_CONST =
-      new CompileTimeConstantExpressionMatcher();
-
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
     if (!Consts.IS_LOGGING_METHOD.matches(tree, state)) {
@@ -42,14 +39,16 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
             ? 1
             : 0;
 
-    if (IS_CONST.matches(tree.getArguments().get(formatIndex), state)) {
+    ExpressionTree expression = tree.getArguments().get(formatIndex);
+    Object constValue = ASTHelpers.constValue(expression);
+    if (constValue != null) {
       return Description.NO_MATCH;
     }
 
     String message =
         String.format(
             "SLF4J logging format should be constant value, but it is \'%s\'",
-            state.getSourceForNode(tree.getArguments().get(formatIndex)));
+            state.getSourceForNode(expression));
     return Description.builder(
             tree,
             "Slf4jFormatShouldBeConst",

src/main/java/jp/skypencil/errorprone/slf4j/SignOnlyFormat.java

@@ -8,9 +8,9 @@
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
-import com.google.errorprone.matchers.CompileTimeConstantExpressionMatcher;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.tools.javac.code.Symbol.VarSymbol;
 import com.sun.tools.javac.util.List;
@@ -26,9 +26,6 @@
 public class SignOnlyFormat extends BugChecker implements MethodInvocationTreeMatcher {
   private static final long serialVersionUID = 3271269614137732880L;
 
-  private static final CompileTimeConstantExpressionMatcher IS_CONST =
-      new CompileTimeConstantExpressionMatcher();
-
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
     if (!Consts.IS_LOGGING_METHOD.matches(tree, state)) {
@@ -42,11 +39,12 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
             ? 1
             : 0;
 
-    if (!IS_CONST.matches(tree.getArguments().get(formatIndex), state)) {
+    ExpressionTree expression = tree.getArguments().get(formatIndex);
+    Object constValue = ASTHelpers.constValue(expression);
+    if (constValue == null) {
       return Description.NO_MATCH;
     }
-
-    String format = ASTHelpers.constValue(tree.getArguments().get(formatIndex)).toString();
+    String format = constValue.toString();
     if (verifyFormat(format)) {
       return Description.NO_MATCH;
     }

src/test/java/jp/skypencil/errorprone/slf4j/FormatShouldBeConstTest.java

@@ -51,4 +51,26 @@ public void testMarker() {
                 + "}")
         .doTest();
   }
+
+  @Test
+  public void testTernaryInStaticBlock() {
+    helper
+        .addSourceLines(
+            "TernaryInStaticBlock.java",
+            "import org.slf4j.Logger;\n"
+                + "import org.slf4j.LoggerFactory;\n"
+                + "\n"
+                + "public class TernaryInStaticBlock {\n"
+                + "   public static boolean DEBUG = false;\n"
+                + "   public static final boolean DEBUG_FINAL = false;\n"
+                + "   private static final Logger logger = LoggerFactory.getLogger(TernaryInStaticBlock.class);\n"
+                + "\n"
+                + "   static {\n"
+                + "     // BUG: Diagnostic contains: SLF4J logging format should be constant value, but it is '\"Debug mode \" + (DEBUG ? \"enabled.\" : \"disabled.\")'\n"
+                + "     logger.info(\"Debug mode \" + (DEBUG ? \"enabled.\" : \"disabled.\"));\n"
+                + "     logger.info(\"Debug mode \" + (DEBUG_FINAL ? \"enabled.\" : \"disabled.\"));\n"
+                + "   }\n"
+                + "}\n")
+        .doTest();
+  }
 }

src/test/java/jp/skypencil/errorprone/slf4j/SignOnlyFormatTest.java

@@ -16,11 +16,11 @@ public void setup() {
   public void testPlaceholderOnly() {
     helper
         .addSourceLines(
-            "NonConstantFormat.java",
+            "PlaceholderOnly.java",
             "import org.slf4j.Logger;\n"
                 + "import org.slf4j.LoggerFactory;\n"
                 + "\n"
-                + "public class NonConstantFormat {\n"
+                + "public class PlaceholderOnly {\n"
                 + "    private final Logger logger = LoggerFactory.getLogger(getClass());\n"
                 + "    void method() {\n"
                 + "        // BUG: Diagnostic contains: SLF4J logging format should contain non-sign text, but it is \'{}, {}\'\n"
@@ -68,4 +68,26 @@ public void testMarker() {
                 + "}")
         .doTest();
   }
+
+  @Test
+  public void testTernaryInStaticBlock() {
+    helper
+        .addSourceLines(
+            "TernaryInStaticBlock.java",
+            "import org.slf4j.Logger;\n"
+                + "import org.slf4j.LoggerFactory;\n"
+                + "\n"
+                + "public class TernaryInStaticBlock {\n"
+                + "   public static boolean INDENT = true;\n"
+                + "   public static final boolean INDENT_FINAL = true;\n"
+                + "   private static final Logger logger = LoggerFactory.getLogger(TernaryInStaticBlock.class);\n"
+                + "\n"
+                + "   static {\n"
+                + "     logger.info((INDENT ? \"  \" : \"\") + \"{}\", 1);\n"
+                + "     // BUG: Diagnostic contains: SLF4J logging format should contain non-sign text, but it is '  {}'\n"
+                + "     logger.info((INDENT_FINAL ? \"  \" : \"\") + \"{}\", 1);\n"
+                + "   }\n"
+                + "}\n")
+        .doTest();
+  }
 }