diff --git a/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties b/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties index 6c3bf90cbb7e..9a30b4952e76 100644 --- a/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties +++ b/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 #ref 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 diff --git a/java/java-impl/src/com/siyeh/ig/logging/StringConcatenationArgumentToLogCallInspection.java b/java/java-impl/src/com/siyeh/ig/logging/StringConcatenationArgumentToLogCallInspection.java index 75618e4f2cf6..992757685a15 100644 --- a/java/java-impl/src/com/siyeh/ig/logging/StringConcatenationArgumentToLogCallInspection.java +++ b/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 diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousDefaultLogger.after.java b/java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousDefaultLogger.after.java new file mode 100644 index 000000000000..1f110740511b --- /dev/null +++ b/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().info("12{}", a); + } + + public native Logger getLogger(); +} \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousDefaultLogger.java b/java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousDefaultLogger.java new file mode 100644 index 000000000000..ece3b7672f67 --- /dev/null +++ b/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().info("12" + a); + } + + public native Logger getLogger(); +} \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousUnknownLogger.after.java b/java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousUnknownLogger.after.java new file mode 100644 index 000000000000..ece3b7672f67 --- /dev/null +++ b/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().info("12" + a); + } + + public native Logger getLogger(); +} \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousUnknownLogger.java b/java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jAmbitiousUnknownLogger.java new file mode 100644 index 000000000000..ece3b7672f67 --- /dev/null +++ b/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().info("12" + a); + } + + public native Logger getLogger(); +} \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jFormattedUnknownLogger.java b/java/java-tests/testData/ig/com/siyeh/igfixes/logging/string_concatenation_argument_to_log_call/Log4jFormattedUnknownLogger.java new file mode 100644 index 000000000000..f7428bfae9d7 --- /dev/null +++ b/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.info("12" + a); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/siyeh/ig/fixes/logging/StringConcatenationArgumentToLogCallFixTest.java b/java/java-tests/testSrc/com/siyeh/ig/fixes/logging/StringConcatenationArgumentToLogCallFixTest.java index a693054b4c47..bab2b638c19e 100644 --- a/java/java-tests/testSrc/com/siyeh/ig/fixes/logging/StringConcatenationArgumentToLogCallFixTest.java +++ b/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(