[java-inspection] IDEA-364384 'Non-constant string as argument to ...' logging call has no quick fix if logger is returned from another method

GitOrigin-RevId: f33aa6b47a420fe610a440fd4321826a7a43452f

java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties

@@ -1996,6 +1996,8 @@ shared.thread.local.random.problem.descriptor='ThreadLocalRandom' instance might
 native.method.naming.convention.element.description='native' method
 use.of.obsolete.date.time.api.display.name=Use of obsolete date-time API
 use.of.obsolete.date.time.api.problem.descriptor=Obsolete date-time type <code>#ref</code> used #loc
+log4j.use.parameterized.logger=For Log4j 2 use fixes as for parameterized logger
+log4j.use.parameterized.logger.description=For Log4j 2 use fixes as for parameterized logger if the type cannot be inferred, otherwise fixes will not be proposed
 warn.on.label=Warn on:
 all.levels.option=all log levels
 warn.level.and.lower.option=warn level and lower

java/java-impl/src/com/siyeh/ig/logging/StringConcatenationArgumentToLogCallInspection.java

@@ -15,6 +15,7 @@ import com.intellij.psi.util.InheritanceUtil;
 import com.intellij.psi.util.PsiLiteralUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.psi.util.TypeConversionUtil;
+import com.intellij.util.ThreeState;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.BaseInspection;
 import com.siyeh.ig.BaseInspectionVisitor;
@@ -56,13 +57,18 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn
   );
   private static final String LOG4J_LOGGER = "org.apache.logging.log4j.Logger";
   private static final String LOG4J_BUILDER = "org.apache.logging.log4j.LogBuilder";
-  private static final String GET_LOGGER = "getLogger";
+  private static final CallMatcher GET_FORMATTER_LOGGER = staticCall("org.apache.logging.log4j.LogManager", "getFormatterLogger") ;
+  private static final CallMatcher GET_LOGGER =  staticCall("org.apache.logging.log4j.LogManager", "getLogger");
   private static final CallMatcher MESSAGE_FORMAT_FORMAT = anyOf(
     staticCall("java.text.MessageFormat", "format").parameterCount(2)
   );
   private static final String SLF4J_LOGGER = "org.slf4j.Logger";
 
   @SuppressWarnings("PublicField") public int warnLevel = 0;
+  /**
+   * @noinspection PublicField
+   */
+  public boolean isLog4JParameterizedLogger = true;
 
   @Override
   public @NotNull OptPane getOptionsPane() {
@@ -76,7 +82,10 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn
     return pane(
       dropdown("warnLevel", InspectionGadgetsBundle.message("warn.on.label"),
                EntryStream.of(options).mapKeyValue((idx, name) -> option(String.valueOf(idx), name))
-                 .toArray(OptDropdown.Option.class))
+                 .toArray(OptDropdown.Option.class)),
+               checkbox("isLog4JParameterizedLogger",
+                        InspectionGadgetsBundle.message("log4j.use.parameterized.logger"))
+                 .description(InspectionGadgetsBundle.message("log4j.use.parameterized.logger.description"))
     );
   }
 
@@ -90,6 +99,10 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn
     if (warnLevel != 0) {
       node.addContent(new Element("option").setAttribute("name", "warnLevel").setAttribute("value", String.valueOf(warnLevel)));
     }
+    if (!isLog4JParameterizedLogger) {
+      node.addContent(new Element("option").setAttribute("name", "isLog4JParameterizedLogger")
+                        .setAttribute("value", "false"));
+    }
   }
 
   @Override
@@ -105,9 +118,9 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn
     if (!(infos[2] instanceof PsiExpression targetExpression)) {
       return null;
     }
-
-    if (isFormattedLog4J(logCall)) return null;
-
+    ThreeState formattedLog4J = isFormattedLog4J(logCall);
+    if (formattedLog4J == ThreeState.YES) return null;
+    if (!isLog4JParameterizedLogger && formattedLog4J == ThreeState.UNSURE) return null;
     return getQuickFix(problemType, targetExpression);
   }
 
@@ -120,10 +133,10 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn
     };
   }
 
-  private static boolean isFormattedLog4J(@NotNull PsiMethodCallExpression logCall) {
+  private static ThreeState isFormattedLog4J(@NotNull PsiMethodCallExpression logCall) {
     PsiExpression qualifierExpression = logCall.getMethodExpression().getQualifierExpression();
     if (qualifierExpression == null) {
-      return false;
+      return ThreeState.NO;
     }
 
     boolean isLogBuilder = InheritanceUtil.isInheritor(qualifierExpression.getType(), LOG4J_BUILDER);
@@ -142,7 +155,6 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn
       }
 
       if (qualifierExpression != null) {
-        boolean isFormatted = true;
         if (qualifierExpression instanceof PsiMethodCallExpression callExpression) {
           PsiMethod method = callExpression.resolveMethod();
           if (method != null &&
@@ -159,24 +171,28 @@ public final class StringConcatenationArgumentToLogCallInspection extends BaseIn
 
         if (qualifierExpression instanceof PsiReferenceExpression referenceExpression &&
             referenceExpression.resolve() instanceof PsiVariable loggerVariable) {
-          if (!loggerVariable.isPhysical() ||
-              (loggerVariable.getInitializer() instanceof PsiMethodCallExpression callExpression &&
-               GET_LOGGER.equals(callExpression.getMethodExpression().getReferenceName()))) {
-            isFormatted = false;
+          if (loggerVariable.getInitializer() instanceof PsiMethodCallExpression callExpression) {
+            if (GET_FORMATTER_LOGGER.test(callExpression)) {
+              return ThreeState.YES;
+            }
+            if (GET_LOGGER.test(callExpression)) {
+              return ThreeState.NO;
+            }
           }
         }
 
-        if (qualifierExpression instanceof PsiMethodCallExpression callExpression &&
-            GET_LOGGER.equals(callExpression.getMethodExpression().getReferenceName())) {
-          isFormatted = false;
-        }
-
-        if (isFormatted) {
-          return true;
+        if (qualifierExpression instanceof PsiMethodCallExpression callExpression) {
+          if (GET_FORMATTER_LOGGER.test(callExpression)) {
+            return ThreeState.YES;
+          }
+          if (GET_LOGGER.test(callExpression)) {
+            return ThreeState.NO;
+          }
         }
       }
+      return ThreeState.UNSURE;
     }
-    return false;
+    return ThreeState.NO;
   }
 
   @Override

java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousDefaultLogger.after.java

@@ -0,0 +1,10 @@
+import org.apache.logging.log4j.*;
+
+class Log4jAmbitiousDefaultLogger {
+
+  public static void m(String a) {
+      getLogger().i<caret>nfo("12{}", a);
+  }
+
+  public native Logger getLogger();
+}

java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousDefaultLogger.java

@@ -0,0 +1,10 @@
+import org.apache.logging.log4j.*;
+
+class Log4jAmbitiousDefaultLogger {
+
+  public static void m(String a) {
+    getLogger().i<caret>nfo("12" + a);
+  }
+
+  public native Logger getLogger();
+}

java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousUnknownLogger.after.java

@@ -0,0 +1,10 @@
+import org.apache.logging.log4j.*;
+
+class Log4jAmbitiousDefaultLogger {
+
+  public static void m(String a) {
+    getLogger().i<caret>nfo("12" + a);
+  }
+
+  public native Logger getLogger();
+}

java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousUnknownLogger.java

@@ -0,0 +1,10 @@
+import org.apache.logging.log4j.*;
+
+class Log4jAmbitiousDefaultLogger {
+
+  public static void m(String a) {
+    getLogger().i<caret>nfo("12" + a);
+  }
+
+  public native Logger getLogger();
+}

java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jFormattedUnknownLogger.java

@@ -0,0 +1,10 @@
+import org.apache.logging.log4j.*;
+
+class Log4JFormatted {
+
+  private static final Logger logger = LogManager.getFormatterLogger(Log4JFormatted.class);
+
+  public static void m(String a) {
+    logger.i<caret>nfo("12" + a);
+  }
+}

java/java-tests/testSrc/com/siyeh/ig/fixes/logging/StringConcatenationArgumentToLogCallFixTest.java

@@ -3,6 +3,7 @@ package com.siyeh.ig.fixes.logging;
 
 import com.intellij.codeInspection.InspectionsBundle;
 import com.siyeh.InspectionGadgetsBundle;
+import com.siyeh.ig.BaseInspection;
 import com.siyeh.ig.IGQuickFixesTestCase;
 import com.siyeh.ig.logging.StringConcatenationArgumentToLogCallInspection;
 
@@ -26,7 +27,7 @@ public class StringConcatenationArgumentToLogCallFixTest extends IGQuickFixesTes
                            public static Logger getLogger(Class clazz) {
                              return null;
                            }
-                           public static Logger getFormattedLogger(Class clazz) {
+                           public static Logger getFormatterLogger(Class clazz) {
                              return null;
                            }
                          }""");
@@ -38,7 +39,12 @@ public class StringConcatenationArgumentToLogCallFixTest extends IGQuickFixesTes
       }"""
     );
 
-    myFixture.enableInspections(new StringConcatenationArgumentToLogCallInspection());
+    StringConcatenationArgumentToLogCallInspection inspection = new StringConcatenationArgumentToLogCallInspection();
+    myFixture.enableInspections(inspection);
+    String name = getTestName(false);
+    if (name.endsWith("UnknownLogger")) {
+      inspection.isLog4JParameterizedLogger = false;
+    }
   }
 
   public void testUseOfConstant() { doTest(); }
@@ -47,6 +53,22 @@ public class StringConcatenationArgumentToLogCallFixTest extends IGQuickFixesTes
   public void testLog4jFormatted() {
     assertQuickfixNotAvailable(InspectionGadgetsBundle.message("string.concatenation.argument.to.log.call.quickfix"));
   }
+  public void testLog4jFormattedUnknownLogger() {
+    BaseInspection inspection = getInspection();
+    if (inspection instanceof StringConcatenationArgumentToLogCallInspection stringConcatenationArgumentToLogCallInspection) {
+      stringConcatenationArgumentToLogCallInspection.isLog4JParameterizedLogger = false;
+    }
+    assertQuickfixNotAvailable(InspectionGadgetsBundle.message("string.concatenation.argument.to.log.call.quickfix"));
+  }
+
+  public void testLog4jAmbitiousDefaultLogger() {
+    doTest();
+  }
+
+  public void testLog4jAmbitiousUnknownLogger() {
+    assertQuickfixNotAvailable(InspectionGadgetsBundle.message("string.concatenation.argument.to.log.call.quickfix"));
+  }
+
   public void testLog4JLogBuilder() { doTest(); }
   public void testTextBlocks() {
     doTest(