From 5f382bc66268fba05304d45bce941e592e2a6069 Mon Sep 17 00:00:00 2001 From: Mikhail Pyltsin Date: Wed, 6 Mar 2024 17:56:16 +0100 Subject: [PATCH] [java-inspection] IDEA-337700 Improvements for logging inspections - new LogStatementNotGuardedByLogConditionInspection GitOrigin-RevId: e5cb767b24b33cb450597551dbbf4ece153a81e2 --- .../src/META-INF/InspectionGadgets.xml | 4 - ...tementGuardedByLogConditionInspection.java | 261 ---------- .../LogStatementGuardedByLogCondition.html | 43 -- .../Braces.after.java | 19 - .../Braces.java | 16 - .../Simple.after.java | 14 - .../Simple.java | 12 - ...StatementGuardedByLogConditionFixTest.java | 32 -- ...ntGuardedByLogConditionInspectionTest.java | 32 -- .../resources/META-INF/JvmAnalysisPlugin.xml | 6 + .../LogStatementNotGuardedByLogCondition.html | 32 ++ .../messages/JvmAnalysisBundle.properties | 18 + ...ementNotGuardedByLogConditionInspection.kt | 398 +++++++++++++++ ...otGuardedByLogConditionInspectionMerger.kt | 31 ++ ...ggingStringTemplateAsArgumentInspection.kt | 46 +- .../codeInspection/logging/LoggingUtil.kt | 35 +- ...GuardedByLogConditionInspectionTestBase.kt | 22 + .../testFramework/logging/LoggingTestUtils.kt | 1 + ...tNotGuardedByLogConditionInspectionTest.kt | 482 ++++++++++++++++++ ...tNotGuardedByLogConditionInspectionTest.kt | 329 ++++++++++++ ...gStringTemplateAsArgumentInspectionTest.kt | 11 +- .../generate/JavaUastCodeGenerationPlugin.kt | 4 +- 22 files changed, 1370 insertions(+), 478 deletions(-) delete mode 100644 java/java-impl/src/com/siyeh/ig/logging/LogStatementGuardedByLogConditionInspection.java delete mode 100644 java/java-impl/src/inspectionDescriptions/LogStatementGuardedByLogCondition.html delete mode 100644 java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Braces.after.java delete mode 100644 java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Braces.java delete mode 100644 java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Simple.after.java delete mode 100644 java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Simple.java delete mode 100644 java/java-tests/testSrc/com/siyeh/ig/fixes/logging/LogStatementGuardedByLogConditionFixTest.java delete mode 100644 java/java-tests/testSrc/com/siyeh/ig/logging/LogStatementGuardedByLogConditionInspectionTest.java create mode 100644 jvm/jvm-analysis-impl/resources/inspectionDescriptions/LogStatementNotGuardedByLogCondition.html create mode 100644 jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStatementNotGuardedByLogConditionInspection.kt create mode 100644 jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStatementNotGuardedByLogConditionInspectionMerger.kt create mode 100644 jvm/jvm-analysis-internal-testFramework/src/com/intellij/jvm/analysis/internal/testFramework/logging/LoggingStatementNotGuardedByLogConditionInspectionTestBase.kt create mode 100644 jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingStatementNotGuardedByLogConditionInspectionTest.kt create mode 100644 jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingStatementNotGuardedByLogConditionInspectionTest.kt diff --git a/java/java-impl/src/META-INF/InspectionGadgets.xml b/java/java-impl/src/META-INF/InspectionGadgets.xml index 922719575159..1e284c4403cb 100644 --- a/java/java-impl/src/META-INF/InspectionGadgets.xml +++ b/java/java-impl/src/META-INF/InspectionGadgets.xml @@ -1397,10 +1397,6 @@ key="logger.initialized.with.foreign.class.display.name" groupBundle="messages.InspectionsBundle" groupKey="group.names.logging.issues" enabledByDefault="true" level="WARNING" implementationClass="com.siyeh.ig.logging.LoggerInitializedWithForeignClassInspection"/> - diff --git a/java/java-impl/src/com/siyeh/ig/logging/LogStatementGuardedByLogConditionInspection.java b/java/java-impl/src/com/siyeh/ig/logging/LogStatementGuardedByLogConditionInspection.java deleted file mode 100644 index 1f6068e375b5..000000000000 --- a/java/java-impl/src/com/siyeh/ig/logging/LogStatementGuardedByLogConditionInspection.java +++ /dev/null @@ -1,261 +0,0 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. -package com.siyeh.ig.logging; - -import com.intellij.codeInsight.options.JavaClassValidator; -import com.intellij.codeInsight.options.JavaIdentifierValidator; -import com.intellij.modcommand.ModPsiUpdater; -import com.intellij.codeInspection.LocalQuickFix; -import com.intellij.modcommand.PsiUpdateModCommandQuickFix; -import com.intellij.codeInspection.options.OptPane; -import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.InvalidDataException; -import com.intellij.openapi.util.WriteExternalException; -import com.intellij.psi.*; -import com.intellij.psi.codeStyle.JavaCodeStyleManager; -import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.psi.util.PsiUtil; -import com.siyeh.InspectionGadgetsBundle; -import com.siyeh.ig.BaseInspection; -import com.siyeh.ig.BaseInspectionVisitor; -import com.siyeh.ig.psiutils.JavaLoggingUtils; -import com.siyeh.ig.psiutils.TypeUtils; -import org.jdom.Element; -import org.jetbrains.annotations.NonNls; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - -import java.util.ArrayList; -import java.util.List; - -import static com.intellij.codeInspection.options.OptPane.*; - -public final class LogStatementGuardedByLogConditionInspection extends BaseInspection { - - final List logMethodNameList = new ArrayList<>(); - final List logConditionMethodNameList = new ArrayList<>(); - @SuppressWarnings({"PublicField"}) - public String loggerClassName = JavaLoggingUtils.JAVA_LOGGING; - @NonNls - @SuppressWarnings({"PublicField"}) - public String loggerMethodAndconditionMethodNames = - "fine,isLoggable(java.util.logging.Level.FINE)," + - "finer,isLoggable(java.util.logging.Level.FINER)," + - "finest,isLoggable(java.util.logging.Level.FINEST)"; - @SuppressWarnings("PublicField") - public boolean flagAllUnguarded = false; - - public LogStatementGuardedByLogConditionInspection() { - parseString(loggerMethodAndconditionMethodNames, logMethodNameList, logConditionMethodNameList); - } - - @Override - public @NotNull OptPane getOptionsPane() { - return pane( - string("loggerClassName", InspectionGadgetsBundle.message("logger.name.option"), - new JavaClassValidator()), - table("", - column("logMethodNameList", InspectionGadgetsBundle.message("log.method.name"), new JavaIdentifierValidator()), - column("logConditionMethodNameList", InspectionGadgetsBundle.message("log.condition.text"))), - checkbox("flagAllUnguarded", InspectionGadgetsBundle.message("log.statement.guarded.by.log.condition.flag.all.unguarded.option")) - ); - } - - @Override - @NotNull - protected String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message("log.statement.guarded.by.log.condition.problem.descriptor"); - } - - @Override - @Nullable - protected LocalQuickFix buildFix(Object... infos) { - return new LogStatementGuardedByLogConditionFix(); - } - - @Override - public BaseInspectionVisitor buildVisitor() { - return new LogStatementGuardedByLogConditionVisitor(); - } - - @Override - public void readSettings(@NotNull Element element) throws InvalidDataException { - super.readSettings(element); - parseString(loggerMethodAndconditionMethodNames, logMethodNameList, logConditionMethodNameList); - } - - @Override - public void writeSettings(@NotNull Element element) throws WriteExternalException { - loggerMethodAndconditionMethodNames = formatString(logMethodNameList, logConditionMethodNameList); - super.writeSettings(element); - } - - private class LogStatementGuardedByLogConditionFix extends PsiUpdateModCommandQuickFix { - - @Override - @NotNull - public String getFamilyName() { - return InspectionGadgetsBundle.message("log.statement.guarded.by.log.condition.quickfix"); - } - - @Override - protected void applyFix(@NotNull Project project, @NotNull PsiElement element, @NotNull ModPsiUpdater updater) { - final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)element.getParent().getParent(); - final PsiStatement statement = PsiTreeUtil.getParentOfType(methodCallExpression, PsiStatement.class); - if (statement == null) { - return; - } - final List logStatements = new ArrayList<>(); - logStatements.add(statement); - final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); - final String referenceName = methodExpression.getReferenceName(); - if (referenceName == null) { - return; - } - PsiStatement previousStatement = PsiTreeUtil.getPrevSiblingOfType(statement, PsiStatement.class); - while (isSameLogMethodCall(previousStatement, referenceName)) { - logStatements.add(0, previousStatement); - previousStatement = PsiTreeUtil.getPrevSiblingOfType(previousStatement, PsiStatement.class); - } - PsiStatement nextStatement = PsiTreeUtil.getNextSiblingOfType(statement, PsiStatement.class); - while (isSameLogMethodCall(nextStatement, referenceName)) { - logStatements.add(nextStatement); - nextStatement = PsiTreeUtil.getNextSiblingOfType(nextStatement, PsiStatement.class); - } - final PsiElementFactory factory = JavaPsiFacade.getInstance(project).getElementFactory(); - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - if (qualifier == null) { - return; - } - final int index = logMethodNameList.indexOf(referenceName); - final String conditionMethodText = logConditionMethodNameList.get(index); - @NonNls final String ifStatementText = "if (" + qualifier.getText() + '.' + conditionMethodText + ") {}"; - final PsiIfStatement ifStatement = (PsiIfStatement)factory.createStatementFromText(ifStatementText, statement); - final PsiBlockStatement blockStatement = (PsiBlockStatement)ifStatement.getThenBranch(); - if (blockStatement == null) { - return; - } - final PsiCodeBlock codeBlock = blockStatement.getCodeBlock(); - for (PsiStatement logStatement : logStatements) { - codeBlock.add(logStatement); - } - final PsiStatement firstStatement = logStatements.get(0); - final PsiElement parent = firstStatement.getParent(); - final JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(project); - if (parent instanceof PsiIfStatement && ((PsiIfStatement)parent).getElseBranch() != null) { - final PsiBlockStatement newBlockStatement = (PsiBlockStatement)factory.createStatementFromText("{}", statement); - newBlockStatement.getCodeBlock().add(ifStatement); - final PsiElement result = firstStatement.replace(newBlockStatement); - codeStyleManager.shortenClassReferences(result); - return; - } - final PsiElement result = parent.addBefore(ifStatement, firstStatement); - codeStyleManager.shortenClassReferences(result); - for (PsiStatement logStatement : logStatements) { - logStatement.delete(); - } - } - - private boolean isSameLogMethodCall(PsiStatement statement, @NotNull String methodName) { - if (statement == null) { - return false; - } - if (!(statement instanceof PsiExpressionStatement expressionStatement)) { - return false; - } - final PsiExpression expression = expressionStatement.getExpression(); - if (!(expression instanceof PsiMethodCallExpression methodCallExpression)) { - return false; - } - final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); - final String referenceName = methodExpression.getReferenceName(); - if (!methodName.equals(referenceName)) { - return false; - } - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - return TypeUtils.expressionHasTypeOrSubtype(qualifier, loggerClassName); - } - } - - private class LogStatementGuardedByLogConditionVisitor extends BaseInspectionVisitor { - - @Override - public void visitMethodCallExpression(@NotNull PsiMethodCallExpression expression) { - super.visitMethodCallExpression(expression); - final PsiReferenceExpression methodExpression = expression.getMethodExpression(); - final String referenceName = methodExpression.getReferenceName(); - if (!logMethodNameList.contains(referenceName)) { - return; - } - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - if (!TypeUtils.expressionHasTypeOrSubtype(qualifier, loggerClassName)) { - return; - } - if (isSurroundedByLogGuard(expression, referenceName)) { - return; - } - final PsiExpressionList argumentList = expression.getArgumentList(); - final PsiExpression[] arguments = argumentList.getExpressions(); - if (arguments.length == 0) { - return; - } - if (!flagAllUnguarded) { - boolean constant = true; - for (PsiExpression argument : arguments) { - argument = PsiUtil.skipParenthesizedExprDown(argument); - if (argument instanceof PsiLambdaExpression || argument instanceof PsiMethodReferenceExpression) { - continue; - } - if (!PsiUtil.isConstantExpression(argument)) { - constant = false; - break; - } - } - if (constant) { - return; - } - } - registerMethodCallError(expression); - } - - private boolean isSurroundedByLogGuard(PsiElement element, String logMethodName) { - while (true) { - final PsiIfStatement ifStatement = PsiTreeUtil.getParentOfType(element, PsiIfStatement.class); - if (ifStatement == null) { - return false; - } - final PsiExpression condition = ifStatement.getCondition(); - if (isLogGuardCheck(condition, logMethodName)) { - return true; - } - element = ifStatement; - } - } - - private boolean isLogGuardCheck(@Nullable PsiExpression expression, String logMethodName) { - expression = PsiUtil.skipParenthesizedExprDown(expression); - if (expression instanceof PsiMethodCallExpression methodCallExpression) { - final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - if (!TypeUtils.expressionHasTypeOrSubtype(qualifier, loggerClassName)) { - return false; - } - final String referenceName = methodExpression.getReferenceName(); - if (referenceName == null) { - return false; - } - final int index = logMethodNameList.indexOf(logMethodName); - final String conditionName = logConditionMethodNameList.get(index); - return conditionName.startsWith(referenceName); - } - else if (expression instanceof PsiPolyadicExpression polyadicExpression) { - final PsiExpression[] operands = polyadicExpression.getOperands(); - for (PsiExpression operand : operands) { - if (isLogGuardCheck(operand, logMethodName)) { - return true; - } - } - } - return false; - } - } -} diff --git a/java/java-impl/src/inspectionDescriptions/LogStatementGuardedByLogCondition.html b/java/java-impl/src/inspectionDescriptions/LogStatementGuardedByLogCondition.html deleted file mode 100644 index 3e1b2cd1ae7f..000000000000 --- a/java/java-impl/src/inspectionDescriptions/LogStatementGuardedByLogCondition.html +++ /dev/null @@ -1,43 +0,0 @@ - - -Reports logging calls with non-constant arguments that are not surrounded by a guard condition. -The evaluation of the arguments of a logging call can be expensive. -Surrounding a logging call with a guard clause prevents that cost when logging -is disabled for the level used by the logging statement. This is especially useful for the -least serious level (trace, debug, finest) of logging calls, because those are -most often disabled in a production environment. -

Example:

-

-  public class Principal {
-    void bad(Object object) {
-      if (true) {
-        LOG.debug("log log log " + expensiveCalculation(object));
-      }
-      LOG.debug("some more logging " + expensiveCalculation(1));
-    }
-
-    void good(Object) {
-      if (LOG.isDebug()) {
-        LOG.debug("value: " + expensiveCalculation(object));
-      }
-    }
-  }
-
- -

- Configure the inspection: -

-
    -
  • - Use the Logger class name field to specify the logger class name used. -
  • -
  • -

    - Use the table to specify the logging methods this inspection should warn on, with the corresponding log condition text. -

  • -
  • - Use the Flag all unguarded logging calls option to have the inspection flag all unguarded log calls, not only those with non-constant arguments. -
  • -
- - \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Braces.after.java b/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Braces.after.java deleted file mode 100644 index eccd1ebcfe14..000000000000 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Braces.after.java +++ /dev/null @@ -1,19 +0,0 @@ -package com.siyeh.igfixes.logging.log_statement_guarded_by_log_condition; - - -import java.util.logging.Logger; - -class Braces { - - private static final Logger LOG = Logger.getLogger("log"); - - void m(boolean b) { - if (b) { - if (LOG.isLoggable(java.util.logging.Level.FINE)) { - LOG.fine("time: " + System.currentTimeMillis()); - } - } - else - System.out.println(); - } -} \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Braces.java b/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Braces.java deleted file mode 100644 index 093020844edb..000000000000 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Braces.java +++ /dev/null @@ -1,16 +0,0 @@ -package com.siyeh.igfixes.logging.log_statement_guarded_by_log_condition; - - -import java.util.logging.Logger; - -class Braces { - - private static final Logger LOG = Logger.getLogger("log"); - - void m(boolean b) { - if (b) - LOG.fine("time: " + System.currentTimeMillis()); - else - System.out.println(); - } -} \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Simple.after.java b/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Simple.after.java deleted file mode 100644 index 710b9475f9cc..000000000000 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Simple.after.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.siyeh.igfixes.logging.log_statement_guarded_by_log_condition; - -import java.util.logging.Logger; - -class Simple { - - private static final Logger LOG = Logger.getLogger("log"); - - void m() { - if (LOG.isLoggable(java.util.logging.Level.FINE)) { - LOG.fine("asdfasd" + System.currentTimeMillis()); - } - } -} \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Simple.java b/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Simple.java deleted file mode 100644 index 65db5ecbf509..000000000000 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/logging/log_statement_guarded_by_log_condition/Simple.java +++ /dev/null @@ -1,12 +0,0 @@ -package com.siyeh.igfixes.logging.log_statement_guarded_by_log_condition; - -import java.util.logging.Logger; - -class Simple { - - private static final Logger LOG = Logger.getLogger("log"); - - void m() { - LOG.fine("asdfasd" + System.currentTimeMillis()); - } -} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/siyeh/ig/fixes/logging/LogStatementGuardedByLogConditionFixTest.java b/java/java-tests/testSrc/com/siyeh/ig/fixes/logging/LogStatementGuardedByLogConditionFixTest.java deleted file mode 100644 index c5e56a5804a6..000000000000 --- a/java/java-tests/testSrc/com/siyeh/ig/fixes/logging/LogStatementGuardedByLogConditionFixTest.java +++ /dev/null @@ -1,32 +0,0 @@ -package com.siyeh.ig.fixes.logging; - -import com.siyeh.InspectionGadgetsBundle; -import com.siyeh.ig.IGQuickFixesTestCase; -import com.siyeh.ig.logging.LogStatementGuardedByLogConditionInspection; - -/** - * @author Bas Leijdekkers - */ -public class LogStatementGuardedByLogConditionFixTest extends IGQuickFixesTestCase { - - public void testSimple() { doTest(); } - public void testBraces() { doTest(); } - - @Override - protected void setUp() throws Exception { - super.setUp(); - myFixture.addClass("package java.util.logging;" + - "public class Logger {" + - " public void fine(String msg) {}" + - " public boolean isLoggable(Level level) { return true; }" + - " public static Logger getLogger(String log) { return new Logger(log); }" + - "}"); - myFixture.enableInspections(new LogStatementGuardedByLogConditionInspection()); - myDefaultHint = InspectionGadgetsBundle.message("log.statement.guarded.by.log.condition.quickfix"); - } - - @Override - protected String getRelativePath() { - return "logging/log_statement_guarded_by_log_condition"; - } -} diff --git a/java/java-tests/testSrc/com/siyeh/ig/logging/LogStatementGuardedByLogConditionInspectionTest.java b/java/java-tests/testSrc/com/siyeh/ig/logging/LogStatementGuardedByLogConditionInspectionTest.java deleted file mode 100644 index b5f6d3c87a93..000000000000 --- a/java/java-tests/testSrc/com/siyeh/ig/logging/LogStatementGuardedByLogConditionInspectionTest.java +++ /dev/null @@ -1,32 +0,0 @@ -package com.siyeh.ig.logging; - -import com.intellij.codeInspection.InspectionProfileEntry; -import com.intellij.testFramework.LightProjectDescriptor; -import com.siyeh.ig.LightJavaInspectionTestCase; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - -public class LogStatementGuardedByLogConditionInspectionTest extends LightJavaInspectionTestCase { - - public void testLogStatementGuardedByLogCondition() { - doTest(); - } - - @Nullable - @Override - protected InspectionProfileEntry getInspection() { - final LogStatementGuardedByLogConditionInspection inspection = new LogStatementGuardedByLogConditionInspection(); - inspection.loggerClassName = "com.siyeh.igtest.logging.log_statement_guarded_by_log_condition.LogStatementGuardedByLogCondition.Logger"; - inspection.logMethodNameList.clear(); - inspection.logMethodNameList.add("debug"); - inspection.logConditionMethodNameList.clear(); - inspection.logConditionMethodNameList.add("isDebug"); - return inspection; - } - - @NotNull - @Override - protected LightProjectDescriptor getProjectDescriptor() { - return JAVA_8; - } -} \ No newline at end of file diff --git a/jvm/jvm-analysis-impl/resources/META-INF/JvmAnalysisPlugin.xml b/jvm/jvm-analysis-impl/resources/META-INF/JvmAnalysisPlugin.xml index d3c507c70eff..72232258664e 100644 --- a/jvm/jvm-analysis-impl/resources/META-INF/JvmAnalysisPlugin.xml +++ b/jvm/jvm-analysis-impl/resources/META-INF/JvmAnalysisPlugin.xml @@ -76,6 +76,12 @@ implementationClass="com.intellij.codeInspection.logging.LoggingSimilarMessageInspection" runForWholeFile="true" /> + + + +Reports logging calls that are not surrounded by a guard condition. +The evaluation of a call's arguments can be expensive. +Surrounding a logging call with a guard clause prevents that cost when logging +is disabled for the level used by the logging statement. This is especially useful for the +least serious level (trace, debug, finest) of logging calls, because those are +most often disabled in a production environment. +

Example:

+

+  public class TestObject {
+    void test(Object object) {
+      LOG.debug("some logging " + expensiveCalculation(1));
+    }
+  }
+
+

After a quick-fix is applied:

+

+  public class TestObject {
+    void test(Object object) {
+      if(LOG.isDebugEnabled()){
+        LOG.debug("some logging " + expensiveCalculation(1));
+      }
+    }
+  }
+
+

This inspection supports Log4j2 and the SLF4J logging frameworks (except builders). + For Java classes, it is possible to configure for a custom framework (see settings bellow). + +

New in 2024.2

+ + \ No newline at end of file diff --git a/jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties b/jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties index 31fff518abda..9ae5fcaf998c 100644 --- a/jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties +++ b/jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties @@ -163,6 +163,24 @@ jvm.inspection.logging.string.template.as.argument.info.level.and.lower.option=i jvm.inspection.logging.string.template.as.argument.debug.level.and.lower.option=debug level and lower jvm.inspection.logging.string.template.as.argument.trace.level.option=trace level + +jvm.inspection.log.statement.not.guarded.display.name=Logging call not guarded by log condition +jvm.inspection.log.statement.not.guarded.warn.on.label=Warn on: +jvm.inspection.log.statement.not.guarded.all.levels.option=all log levels +jvm.inspection.log.statement.not.guarded.warn.level.and.lower.option=warn level and lower +jvm.inspection.log.statement.not.guarded.info.level.and.lower.option=info level and lower +jvm.inspection.log.statement.not.guarded.debug.level.and.lower.option=debug level and lower +jvm.inspection.log.statement.not.guarded.trace.level.option=trace level +jvm.inspection.log.statement.not.guarded.unguarded.constant.option=Process unguarded logging calls with constant messages +jvm.inspection.log.statement.not.guarded.unguarded.constant.option.comment=Process all unguarded log calls, not only those with non-constant arguments +jvm.inspection.log.statement.not.guarded.logger.name.option=Custom logger class (only for Java) +jvm.inspection.log.statement.not.guarded.logger.name.option.comment=Use this field to specify the logger class name used. This option is used only for Java classes. +jvm.inspection.log.statement.not.guarded.custom.table=Use the table to specify the logging methods this inspection should warn on, with the corresponding log condition text. This option is used only for Java classes. +jvm.inspection.log.statement.not.guarded.log.method.name=Custom Logging Method Name (Only for Java) +jvm.inspection.log.statement.not.guarded.log.condition.text=Custom Log Condition Text (Only for Java) +jvm.inspection.log.statement.not.guarded.log.fix.family.name=Surround with log condition +jvm.inspection.log.statement.not.guarded.log.problem.descriptor=Logging call not guarded by log condition #loc + jvm.inspection.logging.placeholder.count.matches.argument.count.display.name=Number of placeholders does not match number of arguments in logging call jvm.inspection.logging.placeholder.count.matches.argument.count.more.problem.descriptor=More arguments provided ({0}) than placeholders specified ({1}) #loc jvm.inspection.logging.placeholder.count.matches.argument.count.fewer.problem.partial.descriptor=Fewer arguments provided ({0}) than placeholders specified (at least {1}) #loc diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStatementNotGuardedByLogConditionInspection.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStatementNotGuardedByLogConditionInspection.kt new file mode 100644 index 000000000000..4b0fda585451 --- /dev/null +++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStatementNotGuardedByLogConditionInspection.kt @@ -0,0 +1,398 @@ +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.codeInspection.logging + +import com.intellij.analysis.JvmAnalysisBundle +import com.intellij.codeInsight.options.JavaClassValidator +import com.intellij.codeInsight.options.JavaIdentifierValidator +import com.intellij.codeInspection.AbstractBaseUastLocalInspectionTool +import com.intellij.codeInspection.LocalInspectionToolSession +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.codeInspection.logging.LoggingUtil.LimitLevelType +import com.intellij.codeInspection.options.OptDescribedComponent +import com.intellij.codeInspection.options.OptPane +import com.intellij.codeInspection.options.OptRegularComponent +import com.intellij.codeInspection.registerUProblem +import com.intellij.lang.java.JavaLanguage +import com.intellij.modcommand.ModPsiUpdater +import com.intellij.modcommand.PsiUpdateModCommandQuickFix +import com.intellij.openapi.project.Project +import com.intellij.openapi.util.NlsContexts +import com.intellij.openapi.util.NlsSafe +import com.intellij.profile.codeInspection.InspectionProjectProfileManager +import com.intellij.psi.* +import com.intellij.psi.codeStyle.JavaCodeStyleManager +import com.intellij.psi.impl.LanguageConstantExpressionEvaluator +import com.intellij.psi.util.PsiTreeUtil +import com.intellij.uast.UastHintedVisitorAdapter +import com.siyeh.ig.psiutils.CommentTracker +import com.siyeh.ig.psiutils.JavaLoggingUtils +import org.jetbrains.uast.* +import org.jetbrains.uast.generate.getUastElementFactory +import org.jetbrains.uast.generate.replace +import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor + +class LoggingStatementNotGuardedByLogConditionInspection : AbstractBaseUastLocalInspectionTool() { + + //for backward compatibility + @JvmField + var customLogMethodNameList: MutableList = mutableListOf("fine", "finer", "finest") + + //for backward compatibility + @JvmField + var customLogConditionMethodNameList: MutableList = mutableListOf( + "isLoggable(java.util.logging.Level.FINE)", + "isLoggable(java.util.logging.Level.FINER)", + "isLoggable(java.util.logging.Level.FINEST)", + ) + + //for backward compatibility + @JvmField + var customLoggerClassName: String = JavaLoggingUtils.JAVA_LOGGING + + @JvmField + var myLimitLevelType: LimitLevelType = LimitLevelType.DEBUG_AND_LOWER + + @JvmField + var flagUnguardedConstant: Boolean = false + + override fun getOptionsPane(): OptPane { + return OptPane.pane( + OptPane.dropdown( + "myLimitLevelType", + JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.warn.on.label"), + OptPane.option(LimitLevelType.ALL, + JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.all.levels.option")), + OptPane.option(LimitLevelType.WARN_AND_LOWER, + JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.warn.level.and.lower.option")), + OptPane.option(LimitLevelType.INFO_AND_LOWER, + JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.info.level.and.lower.option")), + OptPane.option(LimitLevelType.DEBUG_AND_LOWER, + JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.debug.level.and.lower.option")), + OptPane.option(LimitLevelType.TRACE, + JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.trace.level.option")), + ), + OptPane.checkbox("flagUnguardedConstant", JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.unguarded.constant.option")) + .comment(JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.unguarded.constant.option.comment")), + + OptPane.string("customLoggerClassName", JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.logger.name.option"), + JavaClassValidator()) + .comment(JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.logger.name.option.comment")), + OptPane.table("", + OptPane.column("customLogMethodNameList", JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.method.name"), JavaIdentifierValidator()), + OptPane.column("customLogConditionMethodNameList", JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.condition.text"))) + .comment(JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.custom.table")), + ) + } + + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor = + UastHintedVisitorAdapter.create( + holder.file.language, + LogStatementNotGuardedByLogConditionVisitor(holder, isOnTheFly), + arrayOf(UCallExpression::class.java), + directOnly = true + ) + + inner class LogStatementNotGuardedByLogConditionVisitor( + private val holder: ProblemsHolder, + private val isOnTheFly: Boolean, + ) : AbstractUastNonRecursiveVisitor() { + override fun visitCallExpression(node: UCallExpression): Boolean { + val isCustom: Boolean + val sourcePsi = node.sourcePsi + if (sourcePsi == null) return true + if (LoggingUtil.LOG_MATCHERS_WITHOUT_BUILDERS.uCallMatches(node)) { + isCustom = false + } + else if (node.isMethodNameOneOf(customLogMethodNameList.filterNotNull())) { + val uMethod = node.resolveToUElementOfType() ?: return true + if (uMethod.getContainingUClass()?.qualifiedName != customLoggerClassName) { + return true + } + isCustom = true + } + else { + return true + } + + //custom settings are supported only for java only for backward compatibility + if (isCustom && node.lang != JavaLanguage.INSTANCE) { + return true + } + + val isInformationLevel = isOnTheFly && InspectionProjectProfileManager.isInformationLevel(getShortName(), sourcePsi) + if (!isInformationLevel && !isCustom && LoggingUtil.skipAccordingLevel(node, myLimitLevelType)) { + return true + } + + if (isSurroundedByLogGuard(node, isCustom)) { + return true + } + + if (!isInformationLevel && skipIfOnlyConstantArguments(node)) return true + + val message = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.problem.descriptor") + + val loggerLevel = LoggingUtil.getLoggerLevel(node) + + val qualifiedExpression = if (node.uastParent is UQualifiedReferenceExpression) node.uastParent else node + var psiElement = qualifiedExpression?.sourcePsi + while (psiElement?.parent.toUElement() == qualifiedExpression) { + psiElement = psiElement?.parent + } + val before = PsiTreeUtil.skipWhitespacesAndCommentsBackward(psiElement).toUElement() as? UQualifiedReferenceExpression + val beforeCall = before.getUCallExpression(2) + val beforeLoggerLevel = LoggingUtil.getLoggerLevel(beforeCall) + if (beforeCall != null && + (LoggingUtil.LOG_MATCHERS_WITHOUT_BUILDERS.uCallMatches(beforeCall) || beforeCall.isMethodNameOneOf(customLogMethodNameList.filterNotNull())) + ) { + val receiverText = node.receiver?.sourcePsi?.text + if (receiverText != null && beforeCall.receiver?.sourcePsi?.text == receiverText && + !(skipIfOnlyConstantArguments(beforeCall)) && + beforeCall.methodName == node.methodName && + beforeLoggerLevel == loggerLevel) { + return true + } + } + + var parent = node.uastParent + if (parent is UQualifiedReferenceExpression) { + parent = parent.uastParent + } + + if (parent is ULambdaExpression || parent is UReturnExpression) { + return true + } + + val textCustomCondition = if (isCustom) { + val indexOfMethod = customLogMethodNameList.indexOf(node.methodName) + if (indexOfMethod == -1) return false + customLogConditionMethodNameList[indexOfMethod] + } + else { + null + } + if (node.uastParent is UQualifiedReferenceExpression) { + holder.registerUProblem(node.uastParent as UExpression, message, CreateGuardFix(textCustomCondition)) + } + else { + holder.registerUProblem(node, message, CreateGuardFix(textCustomCondition)) + } + return true + } + + private fun skipIfOnlyConstantArguments(node: UCallExpression): Boolean { + val arguments = node.valueArguments + if (arguments.isEmpty()) { + return true + } + + val expressionEvaluator = LanguageConstantExpressionEvaluator.INSTANCE.forLanguage(node.lang) + + if (expressionEvaluator != null && !flagUnguardedConstant) { + var constant = true + for (parenArgument in arguments) { + val argument = parenArgument.skipParenthesizedExprDown() + if (argument is ULambdaExpression) { + continue + } + + val sourcePsi = argument.sourcePsi + if (isConstantOrPolyadicWithConstants(argument)) { + continue + } + if (sourcePsi != null && expressionEvaluator.computeConstantExpression(sourcePsi, false) == null) { + constant = false + break + } + } + if (constant) { + return true + } + } + return false + } + + private fun isConstantOrPolyadicWithConstants(argument: UExpression): Boolean { + if (argument is ULiteralExpression) return true + if (argument is UBinaryExpression && argument.operands.all { isConstantOrPolyadicWithConstants(it) }) return true + if (argument is UPolyadicExpression && argument.operands.all { isConstantOrPolyadicWithConstants(it) }) return true + return false + } + + private fun isSurroundedByLogGuard(callExpression: UCallExpression, isCustom: Boolean): Boolean { + val guardedCondition = LoggingUtil.getGuardedCondition(callExpression) ?: return false + if (isCustom) { + val indexOfMethod = customLogMethodNameList.indexOf(callExpression.methodName) + if (indexOfMethod == -1) return false + val text = customLogConditionMethodNameList[indexOfMethod] + val expectedText = callExpression.receiver.toString() + "." + text + return guardedCondition.sourcePsi?.textMatches(expectedText) ?: return true + } + val loggerLevel = LoggingUtil.getLoggerLevel(callExpression) ?: return true + val levelFromCondition = LoggingUtil.getLevelFromCondition(guardedCondition) ?: return true + return LoggingUtil.isGuardedIn(levelFromCondition, loggerLevel) + } + } + + private inner class CreateGuardFix(private val textCustomCondition: String?) : PsiUpdateModCommandQuickFix() { + + override fun getFamilyName(): String { + return JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + } + + override fun applyFix(project: Project, element: PsiElement, updater: ModPsiUpdater) { + val uCallExpression: UCallExpression = element.toUElement().getUCallExpression(2) ?: return + + if (textCustomCondition != null) { + generateCustomForJava(uCallExpression) + return + } + + val qualifiedExpression = if (uCallExpression.uastParent is UQualifiedReferenceExpression) uCallExpression.uastParent else uCallExpression + if (qualifiedExpression !is UExpression) return + + var currentElement = qualifiedExpression.sourcePsi + while (currentElement?.parent.toUElement() == qualifiedExpression) { + currentElement = currentElement?.parent + } + + if (currentElement == null) return + + val calls = mutableListOf() + calls.add(qualifiedExpression) + val receiverText = uCallExpression.receiver?.sourcePsi?.text ?: return + + val loggerLevel = LoggingUtil.getLoggerLevel(uCallExpression) ?: return + + while (true) { + val nextElement = PsiTreeUtil.skipWhitespacesAndCommentsForward(currentElement) + val after = nextElement.toUElement() as? UQualifiedReferenceExpression + val afterCall = after.getUCallExpression(2) ?: break + val afterLevel = LoggingUtil.getLoggerLevel(afterCall) + if (after != null && + (LoggingUtil.LOG_MATCHERS_WITHOUT_BUILDERS.uCallMatches(afterCall) || afterCall.isMethodNameOneOf(customLogMethodNameList.filterNotNull())) && + receiverText == after.receiver.sourcePsi?.text && + uCallExpression.methodName == afterCall.methodName && + afterLevel == loggerLevel) { + calls.add(after) + currentElement = nextElement + continue + } + break + } + val conditionCall = LoggingUtil.GUARD_MAP.entries.firstOrNull { it.value == loggerLevel.name.lowercase() }?.key ?: return + + val elementFactory = uCallExpression.getUastElementFactory(project) ?: return + if (calls.isEmpty()) return + var condition: UExpression = + elementFactory.createQualifiedReference(qualifiedName = "${receiverText}.$conditionCall", + context = uCallExpression.sourcePsi) ?: return + + if (condition.uastParent is UQualifiedReferenceExpression) { + condition = condition.uastParent as UQualifiedReferenceExpression + } + val blockExpression = elementFactory.createBlockExpression(calls, uCallExpression.sourcePsi) ?: return + + val ifExpression = elementFactory.createIfExpression(condition, blockExpression, null, element) ?: return + + val uExpression = calls[0] + val newIfExpression = uExpression.replace(ifExpression) + val newCondition = newIfExpression?.condition + if (newCondition is UQualifiedReferenceExpression && newCondition.resolveToUElement() !is UMethod) { + val callCondition = elementFactory.createCallExpression(newCondition.receiver, conditionCall, emptyList(), PsiTypes.booleanType(), UastCallKind.METHOD_CALL, newCondition.sourcePsi) + if (callCondition != null) { + newCondition.replace(callCondition) + } + } + + if (calls.size > 1) { + for (i in 1 until calls.size) { + calls[i].sourcePsi?.delete() + } + } + } + + /** + * It is only used to support backward compatibility with the previous java inspections. + * The main generation is for UAST + */ + private fun generateCustomForJava(uCallExpression: UExpression) { + val methodCallExpression = uCallExpression.javaPsi + if (methodCallExpression !is PsiMethodCallExpression) { + return + } + val statement = PsiTreeUtil.getParentOfType(methodCallExpression, PsiStatement::class.java) ?: return + val logStatements = mutableListOf() + logStatements.add(statement) + val methodExpression = methodCallExpression.getMethodExpression() + var nextStatement = PsiTreeUtil.getNextSiblingOfType(statement, PsiStatement::class.java) + while (isSameLogMethodCall(nextStatement, methodExpression)) { + if (nextStatement != null) { + logStatements.add(nextStatement) + nextStatement = PsiTreeUtil.getNextSiblingOfType(nextStatement, PsiStatement::class.java) + } + else { + break + } + } + val factory = JavaPsiFacade.getInstance(methodExpression.project).elementFactory + val qualifier = methodExpression.qualifierExpression + if (qualifier == null) { + return + } + val ifStatementText = "if (" + qualifier.text + '.' + textCustomCondition + ") {}" + val ifStatement = factory.createStatementFromText(ifStatementText, statement) as PsiIfStatement + val blockStatement = (ifStatement.thenBranch as PsiBlockStatement?) ?: return + val codeBlock = blockStatement.codeBlock + for (logStatement in logStatements) { + codeBlock.add(logStatement) + } + val firstStatement = logStatements[0] + val parent = firstStatement.parent + val codeStyleManager = JavaCodeStyleManager.getInstance(methodCallExpression.project) + if (parent is PsiIfStatement && parent.elseBranch != null) { + val newBlockStatement = factory.createStatementFromText("{}", statement) as PsiBlockStatement + newBlockStatement.codeBlock.add(ifStatement) + val commentTracker = CommentTracker() + val result = commentTracker.replace(firstStatement, newBlockStatement) + codeStyleManager.shortenClassReferences(result) + return + } + val result = parent.addBefore(ifStatement, firstStatement) + codeStyleManager.shortenClassReferences(result) + for (logStatement in logStatements) { + logStatement.delete() + } + } + + private fun isSameLogMethodCall(current: PsiStatement?, targetReference: PsiReferenceExpression): Boolean { + if (current == null) { + return false + } + if (current !is PsiExpressionStatement) { + return false + } + val expression = current.expression + if (expression !is PsiMethodCallExpression) { + return false + } + val methodExpression = expression.methodExpression + val referenceName = methodExpression.referenceName + if (targetReference.referenceName != referenceName) { + return false + } + val qualifier = methodExpression.qualifierExpression + val qualifierText = qualifier?.text + return qualifierText != null && qualifierText == targetReference.qualifierExpression?.text + } + } +} + +private fun OptRegularComponent.comment(@NlsContexts.Tooltip @NlsSafe comment: String): OptRegularComponent { + if (this is OptDescribedComponent) { + val component = this.description(comment) + return component ?: this + } + return this +} \ No newline at end of file diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStatementNotGuardedByLogConditionInspectionMerger.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStatementNotGuardedByLogConditionInspectionMerger.kt new file mode 100644 index 000000000000..174a710db23e --- /dev/null +++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStatementNotGuardedByLogConditionInspectionMerger.kt @@ -0,0 +1,31 @@ +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.codeInspection.logging + +import com.intellij.codeInspection.ex.InspectionElementsMergerBase +import com.intellij.openapi.util.JDOMExternalizerUtil +import com.siyeh.ig.BaseInspection +import com.siyeh.ig.psiutils.JavaLoggingUtils +import org.jdom.Element + +class LoggingStatementNotGuardedByLogConditionInspectionMerger : InspectionElementsMergerBase() { + override fun getMergedToolName(): String = "LogStatementNotGuardedByLogCondition" + + override fun getSourceToolNames(): Array = arrayOf("LogStatementGuardedByLogCondition") + + + override fun transformElement(sourceToolName: String, sourceElement: Element, toolElement: Element): Element { + val inspection = LoggingStatementNotGuardedByLogConditionInspection() + inspection.customLoggerClassName = JDOMExternalizerUtil.readField(sourceElement, "loggerClassName") ?: JavaLoggingUtils.JAVA_LOGGING + inspection.flagUnguardedConstant = JDOMExternalizerUtil.readField(sourceElement, "flagAllUnguarded", "false").toBoolean() + val conditionMethodNames = JDOMExternalizerUtil.readField(sourceElement, "loggerMethodAndconditionMethodNames") + if (conditionMethodNames != null) { + val calls = mutableListOf() + val condition = mutableListOf() + BaseInspection.parseString(conditionMethodNames, calls, condition) + inspection.customLogMethodNameList = calls + inspection.customLogConditionMethodNameList = condition + } + inspection.writeSettings(toolElement) + return toolElement + } +} \ No newline at end of file diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStringTemplateAsArgumentInspection.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStringTemplateAsArgumentInspection.kt index 68b96bb12413..e753e7d2412a 100644 --- a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStringTemplateAsArgumentInspection.kt +++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingStringTemplateAsArgumentInspection.kt @@ -1,4 +1,4 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInspection.logging import com.intellij.analysis.JvmAnalysisBundle @@ -6,7 +6,6 @@ import com.intellij.codeInspection.* import com.intellij.codeInspection.logging.LoggingUtil.Companion import com.intellij.codeInspection.logging.LoggingUtil.Companion.LOG_MATCHERS import com.intellij.codeInspection.logging.LoggingUtil.Companion.countPlaceHolders -import com.intellij.codeInspection.logging.LoggingUtil.Companion.getLoggerLevel import com.intellij.codeInspection.logging.LoggingUtil.Companion.getLoggerType import com.intellij.codeInspection.logging.LoggingUtil.Companion.isGuarded import com.intellij.codeInspection.options.OptPane @@ -27,29 +26,25 @@ import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor class LoggingStringTemplateAsArgumentInspection : AbstractBaseUastLocalInspectionTool() { @JvmField - var myLimitLevelType: LimitLevelType = LimitLevelType.DEBUG_AND_LOWER + var myLimitLevelType: LoggingUtil.LimitLevelType = LoggingUtil.LimitLevelType.DEBUG_AND_LOWER @JvmField var mySkipPrimitives: Boolean = true - enum class LimitLevelType { - ALL, WARN_AND_LOWER, INFO_AND_LOWER, DEBUG_AND_LOWER, TRACE - } - override fun getOptionsPane(): OptPane { return OptPane.pane( OptPane.dropdown( "myLimitLevelType", JvmAnalysisBundle.message("jvm.inspection.logging.string.template.as.argument.warn.on.label"), - OptPane.option(LimitLevelType.ALL, + OptPane.option(LoggingUtil.LimitLevelType.ALL, JvmAnalysisBundle.message("jvm.inspection.logging.string.template.as.argument.all.levels.option")), - OptPane.option(LimitLevelType.WARN_AND_LOWER, + OptPane.option(LoggingUtil.LimitLevelType.WARN_AND_LOWER, JvmAnalysisBundle.message("jvm.inspection.logging.string.template.as.argument.warn.level.and.lower.option")), - OptPane.option(LimitLevelType.INFO_AND_LOWER, + OptPane.option(LoggingUtil.LimitLevelType.INFO_AND_LOWER, JvmAnalysisBundle.message("jvm.inspection.logging.string.template.as.argument.info.level.and.lower.option")), - OptPane.option(LimitLevelType.DEBUG_AND_LOWER, + OptPane.option(LoggingUtil.LimitLevelType.DEBUG_AND_LOWER, JvmAnalysisBundle.message("jvm.inspection.logging.string.template.as.argument.debug.level.and.lower.option")), - OptPane.option(LimitLevelType.TRACE, + OptPane.option(LoggingUtil.LimitLevelType.TRACE, JvmAnalysisBundle.message("jvm.inspection.logging.string.template.as.argument.trace.level.option")), ), OptPane.checkbox("mySkipPrimitives", @@ -71,7 +66,7 @@ class LoggingStringTemplateAsArgumentInspection : AbstractBaseUastLocalInspectio override fun visitCallExpression(node: UCallExpression): Boolean { if (!LOG_MATCHERS.uCallMatches(node)) return true - if (skipAccordingLevel(node)) return true + if (LoggingUtil.skipAccordingLevel(node, myLimitLevelType)) return true val valueArguments = node.valueArguments val uMethod = node.resolve().toUElement() as? UMethod ?: return true val uastParameters = uMethod.uastParameters @@ -104,7 +99,7 @@ class LoggingStringTemplateAsArgumentInspection : AbstractBaseUastLocalInspectio parts.addAll(stringExpression.operands) } else { - parts.addAll(stringExpression.operands.flatMap { operand-> + parts.addAll(stringExpression.operands.flatMap { operand -> if (isPattern(operand) && operand is UPolyadicExpression) { operand.operands } @@ -124,8 +119,8 @@ class LoggingStringTemplateAsArgumentInspection : AbstractBaseUastLocalInspectio if ((injected.size == 1 && InheritanceUtil.isInheritor(injected.first().getExpressionType(), CommonClassNames.JAVA_LANG_THROWABLE)) || ((valueArguments.lastIndex - indexStringExpression) == 1 && - InheritanceUtil.isInheritor(valueArguments.last().getExpressionType(), CommonClassNames.JAVA_LANG_THROWABLE)) - ) { + InheritanceUtil.isInheritor(valueArguments.last().getExpressionType(), CommonClassNames.JAVA_LANG_THROWABLE)) + ) { return true } @@ -146,25 +141,6 @@ class LoggingStringTemplateAsArgumentInspection : AbstractBaseUastLocalInspectio return operands.all { it.getExpressionType().isPrimitiveOrWrappers() } } - private fun skipAccordingLevel(node: UCallExpression): Boolean { - if (myLimitLevelType != LimitLevelType.ALL) { - val loggerLevel = getLoggerLevel(node) - if (loggerLevel == null) return true - val notSkip: Boolean = when (loggerLevel) { - Companion.LevelType.FATAL -> false - Companion.LevelType.ERROR -> false - Companion.LevelType.WARN -> myLimitLevelType.ordinal == LimitLevelType.WARN_AND_LOWER.ordinal - Companion.LevelType.INFO -> myLimitLevelType.ordinal <= LimitLevelType.INFO_AND_LOWER.ordinal - Companion.LevelType.DEBUG -> myLimitLevelType.ordinal <= LimitLevelType.DEBUG_AND_LOWER.ordinal - Companion.LevelType.TRACE -> myLimitLevelType.ordinal <= LimitLevelType.TRACE.ordinal - } - return !notSkip - } - else { - return false - } - } - /** * @param stringExpression The string expression to check. * @return True if the string expression consists of patterns or string only, false otherwise. diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingUtil.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingUtil.kt index dc9f7e6efb9f..73e1b8c86611 100644 --- a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingUtil.kt +++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingUtil.kt @@ -1,4 +1,4 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInspection.logging import com.intellij.psi.util.CachedValueProvider @@ -9,7 +9,7 @@ import com.siyeh.ig.callMatcher.CallMatcher import org.jetbrains.uast.* import org.jetbrains.uast.visitor.AbstractUastVisitor -internal class LoggingUtil { +class LoggingUtil { companion object { internal const val SLF4J_LOGGER = "org.slf4j.Logger" @@ -44,6 +44,11 @@ internal class LoggingUtil { SLF4J_BUILDER_MATCHER, ) + internal val LOG_MATCHERS_WITHOUT_BUILDERS: CallMatcher = CallMatcher.anyOf( + SLF4J_MATCHER, + LOG4J_MATCHER, + ) + internal val FORMATTED_LOG4J: CallMatcher = CallMatcher.staticCall("org.apache.logging.log4j.LogManager", "getFormatterLogger") internal const val LOG_4_J_LOGGER = "org.apache.logging.slf4j.Log4jLogger" @@ -66,6 +71,25 @@ internal class LoggingUtil { private val LEVEL_CLASSES = setOf("org.apache.logging.log4j.Level", "org.slf4j.event.Level") private val LEGACY_LEVEL_CLASSES = setOf("org.apache.logging.log4j.Level", "org.apache.log4j.Priority", "java.util.logging.Level") + fun skipAccordingLevel(node: UCallExpression, myLimitLevelType: LimitLevelType): Boolean { + if (myLimitLevelType != LimitLevelType.ALL) { + val loggerLevel = getLoggerLevel(node) + if (loggerLevel == null) return true + val notSkip: Boolean = when (loggerLevel) { + Companion.LevelType.FATAL -> false + Companion.LevelType.ERROR -> false + Companion.LevelType.WARN -> myLimitLevelType.ordinal == LimitLevelType.WARN_AND_LOWER.ordinal + Companion.LevelType.INFO -> myLimitLevelType.ordinal <= LimitLevelType.INFO_AND_LOWER.ordinal + Companion.LevelType.DEBUG -> myLimitLevelType.ordinal <= LimitLevelType.DEBUG_AND_LOWER.ordinal + Companion.LevelType.TRACE -> myLimitLevelType.ordinal <= LimitLevelType.TRACE.ordinal + } + return !notSkip + } + else { + return false + } + } + internal fun getLoggerType(uCall: UCallExpression?): LoggerType? { return if (SLF4J_MATCHER.uCallMatches(uCall)) { LoggerType.SLF4J_LOGGER_TYPE @@ -412,4 +436,9 @@ internal class LoggingUtil { Pair("isFatalEnabled", "fatal"), ) } -} \ No newline at end of file + + enum class LimitLevelType { + ALL, WARN_AND_LOWER, INFO_AND_LOWER, DEBUG_AND_LOWER, TRACE + } +} + diff --git a/jvm/jvm-analysis-internal-testFramework/src/com/intellij/jvm/analysis/internal/testFramework/logging/LoggingStatementNotGuardedByLogConditionInspectionTestBase.kt b/jvm/jvm-analysis-internal-testFramework/src/com/intellij/jvm/analysis/internal/testFramework/logging/LoggingStatementNotGuardedByLogConditionInspectionTestBase.kt new file mode 100644 index 000000000000..2788f42ca3af --- /dev/null +++ b/jvm/jvm-analysis-internal-testFramework/src/com/intellij/jvm/analysis/internal/testFramework/logging/LoggingStatementNotGuardedByLogConditionInspectionTestBase.kt @@ -0,0 +1,22 @@ +package com.intellij.jvm.analysis.internal.testFramework.logging + +import com.intellij.codeHighlighting.HighlightDisplayLevel +import com.intellij.codeInsight.daemon.HighlightDisplayKey +import com.intellij.codeInspection.logging.LoggingStatementNotGuardedByLogConditionInspection +import com.intellij.codeInspection.logging.LoggingUtil +import com.intellij.profile.codeInspection.ProjectInspectionProfileManager.Companion.getInstance + +abstract class LoggingStatementNotGuardedByLogConditionInspectionTestBase : LoggingInspectionTestBase() { + override val inspection = LoggingStatementNotGuardedByLogConditionInspection() + + override fun setUp() { + super.setUp() + inspection.run { + myLimitLevelType = LoggingUtil.LimitLevelType.INFO_AND_LOWER + val displayKey = HighlightDisplayKey.find(this.getShortName()) + val currentProfile = getInstance(project).currentProfile + currentProfile.setErrorLevel(displayKey!!, HighlightDisplayLevel.WARNING, project) + inspection.flagUnguardedConstant = getTestName(true).endsWith("flagUnguardedConstant") + } + } +} \ No newline at end of file diff --git a/jvm/jvm-analysis-internal-testFramework/src/com/intellij/jvm/analysis/internal/testFramework/logging/LoggingTestUtils.kt b/jvm/jvm-analysis-internal-testFramework/src/com/intellij/jvm/analysis/internal/testFramework/logging/LoggingTestUtils.kt index a72ce17e931b..7f6b86154a07 100644 --- a/jvm/jvm-analysis-internal-testFramework/src/com/intellij/jvm/analysis/internal/testFramework/logging/LoggingTestUtils.kt +++ b/jvm/jvm-analysis-internal-testFramework/src/com/intellij/jvm/analysis/internal/testFramework/logging/LoggingTestUtils.kt @@ -38,6 +38,7 @@ object LoggingTestUtils { return null; } public void warning(String msg) {} + public void fine(String msg) {} public boolean isLoggable(Level level) {} } """.trimIndent()) diff --git a/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingStatementNotGuardedByLogConditionInspectionTest.kt b/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingStatementNotGuardedByLogConditionInspectionTest.kt new file mode 100644 index 000000000000..8080caab0ef1 --- /dev/null +++ b/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingStatementNotGuardedByLogConditionInspectionTest.kt @@ -0,0 +1,482 @@ +package com.intellij.codeInspection.tests.java.logging + +import com.intellij.analysis.JvmAnalysisBundle +import com.intellij.jvm.analysis.internal.testFramework.logging.LoggingStatementNotGuardedByLogConditionInspectionTestBase +import com.intellij.jvm.analysis.testFramework.JvmLanguage + +class JavaLoggingStatementNotGuardedByLogConditionInspectionTest : LoggingStatementNotGuardedByLogConditionInspectionTestBase() { + fun `test slf4j`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + LOG.debug("test" + arg); + } + } + """.trimIndent()) + } + + + fun `test inside lambda slf4j`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + Runnable r = ()->LOG.debug("test" + arg); + } + } + """.trimIndent()) + } + + fun `test log4j2`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import org.apache.logging.log4j.*; + class X { + static final Logger LOG = LogManager.getLogger(); + void n(String arg) { + LOG.debug("test" + arg); + } + } + """.trimIndent()) + } + + fun `test custom logger`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + LOG.fine("test" + arg); + } + } + """.trimIndent()) + } + + fun `test skip according level for slf4j`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + LOG.warn("test" + arg); + } + } + """.trimIndent()) + } + + fun `test skip according level for custom logger`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + LOG.warning("test" + arg); + } + } + """.trimIndent()) + } + + fun `test is surrounded by guard for slf4j`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + if((LOG.isDebugEnabled())) { + LOG.debug("test" + arg); + } + + if(LOG.isInfoEnabled()) { + LOG.debug("test" + arg); //todo! + } + + if(true && LOG.isDebugEnabled()) { + LOG.debug("test" + arg); + } + + if(true && LOG.isDebugEnabled()) { + if(true) { + LOG.debug("test" + arg); + } + } + + if(true) { + if(true) { + LOG.debug("test" + arg); + } + } + } + } + """.trimIndent()) + } + + fun `test is surrounded by guard for custom logger`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + if(LOG.isLoggable(java.util.logging.Level.FINE)) { + LOG.fine("test" + arg); + } + + if(true && LOG.isLoggable(java.util.logging.Level.FINE)) { + LOG.fine("test" + arg); + } + } + } + """.trimIndent()) + } + + fun `test skip if only constant arguments for slf4j`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + LOG.debug("test"); + LOG.debug("test {} {}", "test" + "test", 1 + 1); + } + } + """.trimIndent()) + } + + fun `test don't skip if only constant arguments for slf4j flagUnguardedConstant`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n1(String arg) { + LOG.debug("test"); + } + void n2(String arg) { + LOG.debug("test"); + } + } + """.trimIndent()) + } + + fun `test skip with several log calls for slf4j`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n2(String arg) { + LOG.debug("test1" + arg); + LOG.debug("test2" + arg); + } + + void n3(String arg) { + LOG.debug("test1" + arg); + LOG.debug("test2" + arg); + LOG.debug("test2" + arg); + } + + void constantCall(String arg) { + LOG.debug("test1"); + LOG.debug("test2" + arg); + } + + void beforeNotLog(String arg) { + constantCall(arg); + LOG.debug("test2" + arg); + } + + void differentLevels(String arg) { + LOG.debug("test1" + arg); + LOG.warn("test2" + arg); + } + } + """.trimIndent()) + } + + fun `test skip with several log calls for custom logger`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + LOG.fine("test" + arg); + LOG.fine("test" + arg); + } + } + """.trimIndent()) + } + + fun `test lambda`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import org.apache.logging.log4j.*; + class X { + static final Logger LOG = LogManager.getLogger(); + void n(String arg) { + LOG.info("test {}", ()->"1"); + } + } + """.trimIndent()) + } + + fun `test fix simple slf4j`() { + myFixture.testQuickFix( + testPreview = true, + lang = JvmLanguage.JAVA, + before = """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + LOG.debug("test" + arg); + } + } + """.trimIndent(), + after = """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + if (LOG.isDebugEnabled()) { + LOG.debug("test" + arg); + } + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } + + fun `test fix simple nested slf4j`() { + myFixture.testQuickFix( + testPreview = false, + lang = JvmLanguage.JAVA, + before = """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + if(true){ + LOG.debug("test" + arg); + } + } + } + """.trimIndent(), + after = """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + if(true){ + if (LOG.isDebugEnabled()) { + LOG.debug("test" + arg); + } + } + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } + + fun `test fix simple custom`() { + myFixture.testQuickFix( + testPreview = true, + lang = JvmLanguage.JAVA, + before = """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + LOG.fine("test" + arg); + } + } + """.trimIndent(), + after = """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + if (LOG.isLoggable(Level.FINE)) { + LOG.fine("test" + arg); + } + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } + + fun `test fix simple nested custom`() { + myFixture.testQuickFix( + testPreview = true, + lang = JvmLanguage.JAVA, + before = """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + if(true){ + LOG.fine("test" + arg); + } + } + } + """.trimIndent(), + after = """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + if(true){ + if (LOG.isLoggable(Level.FINE)) { + LOG.fine("test" + arg); + } + } + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } + + fun `test fix several similar slf4j`() { + myFixture.testQuickFix( + testPreview = true, + lang = JvmLanguage.JAVA, + before = """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + LOG.debug("test1" + arg); + LOG.debug("test2" + arg); + LOG.debug("test3" + arg); + } + } + """.trimIndent(), + after = """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + if (LOG.isDebugEnabled()) { + LOG.debug("test1" + arg); + LOG.debug("test2" + arg); + LOG.debug("test3" + arg); + } + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } + + fun `test fix several similar custom`() { + myFixture.testQuickFix( + testPreview = true, + lang = JvmLanguage.JAVA, + before = """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + LOG.fine("test1" + arg); + LOG.fine("test2" + arg); + LOG.fine("test3" + arg); + } + } + """.trimIndent(), + after = """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + if (LOG.isLoggable(Level.FINE)) { + LOG.fine("test1" + arg); + LOG.fine("test2" + arg); + LOG.fine("test3" + arg); + } + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } + + fun `test fix several different slf4j`() { + myFixture.testQuickFix( + testPreview = true, + lang = JvmLanguage.JAVA, + before = """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + LOG.debug("test1" + arg); + LOG.debug("test2" + arg); + LOG.trace("test3" + arg); + } + } + """.trimIndent(), + after = """ + import org.slf4j.Logger; + import org.slf4j.LoggerFactory; + class X { + private static final Logger LOG = LoggerFactory.getLogger(X.class); + void n(String arg) { + if (LOG.isDebugEnabled()) { + LOG.debug("test1" + arg); + LOG.debug("test2" + arg); + } + LOG.trace("test3" + arg); + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } + + fun `test fix several different custom`() { + myFixture.testQuickFix( + testPreview = true, + lang = JvmLanguage.JAVA, + before = """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + LOG.fine("test1" + arg); + LOG.fine("test2" + arg); + LOG.finer("test3" + arg); + } + } + """.trimIndent(), + after = """ + import java.util.logging.*; + class X { + static final Logger LOG = Logger.getLogger(""); + void n(String arg) { + if (LOG.isLoggable(Level.FINE)) { + LOG.fine("test1" + arg); + LOG.fine("test2" + arg); + } + LOG.finer("test3" + arg); + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingStatementNotGuardedByLogConditionInspectionTest.kt b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingStatementNotGuardedByLogConditionInspectionTest.kt new file mode 100644 index 000000000000..e49f2860f463 --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingStatementNotGuardedByLogConditionInspectionTest.kt @@ -0,0 +1,329 @@ +package com.intellij.codeInspection.tests.kotlin.logging + +import com.intellij.analysis.JvmAnalysisBundle +import com.intellij.jvm.analysis.internal.testFramework.logging.LoggingStatementNotGuardedByLogConditionInspectionTestBase +import com.intellij.jvm.analysis.testFramework.JvmLanguage + +class KotlinLoggingStatementNotGuardedByLogConditionInspectionTest : LoggingStatementNotGuardedByLogConditionInspectionTestBase() { + fun `test slf4j`() { + myFixture.testHighlighting(JvmLanguage.KOTLIN, """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + private val LOG = LoggerFactory.getLogger() + + fun n(arg: String) { + LOG.debug("test {}", arg) + } + } + """.trimIndent()) + } + + fun `test log4j2`() { + myFixture.testHighlighting(JvmLanguage.KOTLIN, """ + import org.apache.logging.log4j.LogManager + import org.apache.logging.log4j.Logger + + internal class X { + fun n(arg: String) { + LOG.debug("test {}", arg) + } + + companion object { + val LOG: Logger = LogManager.getLogger() + } + } + """.trimIndent()) + } + + fun `test skip according level for slf4j`() { + myFixture.testHighlighting(JvmLanguage.KOTLIN, """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n(arg: String) { + LOG.warn("test " + arg) + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent()) + } + + fun `test is surrounded by guard for slf4j`() { + myFixture.testHighlighting(JvmLanguage.KOTLIN, """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n(arg: String) { + if (LOG.isDebugEnabled) { + LOG.debug("test " + arg) + } + + if (LOG.isInfoEnabled) { + LOG.debug("test" + arg) //todo! + } + + if (true && LOG.isDebugEnabled) { + LOG.debug("test {}", arg) + } + + if (true && LOG.isDebugEnabled) { + if (true) { + LOG.debug("test {}", arg) + } + } + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent()) + } + + fun `test skip if only constant arguments for slf4j`() { + myFixture.testHighlighting(JvmLanguage.KOTLIN, """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n() { + LOG.debug("test") + LOG.debug("test {} {}", "test" + "test", 1 + 1) + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent()) + } + + fun `test don't skip if only constant arguments for slf4j flagUnguardedConstant`() { + myFixture.testHighlighting(JvmLanguage.KOTLIN, """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n1() { + LOG.debug("test") + } + + fun n2() { + LOG.debug("test") + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent()) + } + + fun `test skip with several log calls for slf4j`() { + myFixture.testHighlighting(JvmLanguage.KOTLIN, """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n2(arg: String) { + LOG.debug("test1" + arg) + LOG.debug("test2" + arg) + } + + fun n3(arg: String) { + LOG.debug("test1" + arg) + LOG.debug("test2" + arg) + LOG.debug("test2" + arg) + } + + fun constantCall(arg: String) { + LOG.debug("test1") + LOG.debug("test2" + arg) + } + + fun beforeNotLog(arg: String) { + constantCall(arg) + LOG.debug("test2" + arg) + } + + fun differentLevels(arg: String) { + LOG.debug("test1" + arg) + LOG.warn("test2" + arg) + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent()) + } + + fun `test fix simple slf4j`() { + myFixture.testQuickFix( + testPreview = true, + lang = JvmLanguage.KOTLIN, + before = """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n(arg: String) { + LOG.debug("test" + arg) + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent(), + after = """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n(arg: String) { + if (LOG.isDebugEnabled) { + LOG.debug("test" + arg) + } + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } + + fun `test fix simple nested slf4j`() { + myFixture.testQuickFix( + testPreview = false, + lang = JvmLanguage.KOTLIN, + before = """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n(arg: String) { + if(true){ + LOG.debug("test" + arg) + } + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent(), + after = """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n(arg: String) { + if(true){ + if (LOG.isDebugEnabled) { + LOG.debug("test" + arg) + } + } + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } + + fun `test fix several similar slf4j`() { + myFixture.testQuickFix( + testPreview = true, + lang = JvmLanguage.KOTLIN, + before = """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n(arg: String) { + LOG.debug("test1" + arg) + LOG.debug("test2" + arg) + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent(), + after = """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n(arg: String) { + if (LOG.isDebugEnabled) { + LOG.debug("test1" + arg) + LOG.debug("test2" + arg) + } + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } + + fun `test fix several different slf4j`() { + myFixture.testQuickFix( + testPreview = true, + lang = JvmLanguage.KOTLIN, + before = """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n(arg: String) { + LOG.debug("test1" + arg) + LOG.debug("test2" + arg) + LOG.info("test3" + arg) + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent(), + after = """ + import org.slf4j.Logger + import org.slf4j.LoggerFactory + + internal class X { + fun n(arg: String) { + if (LOG.isDebugEnabled) { + LOG.debug("test1" + arg) + LOG.debug("test2" + arg) + } + LOG.info("test3" + arg) + } + + companion object { + private val LOG: Logger = LoggerFactory.getLogger() + } + } + """.trimIndent(), + hint = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name") + ) + } +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingStringTemplateAsArgumentInspectionTest.kt b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingStringTemplateAsArgumentInspectionTest.kt index 9fb15e1a96bb..20820fc862c1 100644 --- a/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingStringTemplateAsArgumentInspectionTest.kt +++ b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingStringTemplateAsArgumentInspectionTest.kt @@ -2,6 +2,7 @@ package com.intellij.codeInspection.tests.kotlin.logging import com.intellij.codeInspection.InspectionProfileEntry import com.intellij.codeInspection.logging.LoggingStringTemplateAsArgumentInspection +import com.intellij.codeInspection.logging.LoggingUtil import com.intellij.jvm.analysis.KotlinJvmAnalysisTestUtil import com.intellij.jvm.analysis.internal.testFramework.logging.LoggingStringTemplateAsArgumentInspectionTestBase import com.intellij.testFramework.TestDataPath @@ -18,7 +19,7 @@ class KotlinLoggingStringTemplateAsArgumentInspectionTest { override val inspection: InspectionProfileEntry get() { val loggingStringTemplateAsArgumentInspection = LoggingStringTemplateAsArgumentInspection() - loggingStringTemplateAsArgumentInspection.myLimitLevelType = LoggingStringTemplateAsArgumentInspection.LimitLevelType.ALL + loggingStringTemplateAsArgumentInspection.myLimitLevelType = LoggingUtil.LimitLevelType.ALL return loggingStringTemplateAsArgumentInspection } @@ -35,7 +36,7 @@ class KotlinLoggingStringTemplateAsArgumentInspectionTest { override val inspection: InspectionProfileEntry get() = LoggingStringTemplateAsArgumentInspection().apply { mySkipPrimitives = false - myLimitLevelType = LoggingStringTemplateAsArgumentInspection.LimitLevelType.INFO_AND_LOWER + myLimitLevelType = LoggingUtil.LimitLevelType.INFO_AND_LOWER } fun `test highlighting`() { @@ -48,7 +49,7 @@ class KotlinLoggingStringTemplateAsArgumentInspectionTest { override val inspection: InspectionProfileEntry get() = LoggingStringTemplateAsArgumentInspection().apply { mySkipPrimitives = false - myLimitLevelType = LoggingStringTemplateAsArgumentInspection.LimitLevelType.DEBUG_AND_LOWER + myLimitLevelType = LoggingUtil.LimitLevelType.DEBUG_AND_LOWER } fun `test highlighting`() { @@ -60,7 +61,7 @@ class KotlinLoggingStringTemplateAsArgumentInspectionTest { override val inspection: InspectionProfileEntry get() = LoggingStringTemplateAsArgumentInspection().apply { mySkipPrimitives = true - myLimitLevelType = LoggingStringTemplateAsArgumentInspection.LimitLevelType.ALL + myLimitLevelType = LoggingUtil.LimitLevelType.ALL } fun `test highlighting`() { @@ -72,7 +73,7 @@ class KotlinLoggingStringTemplateAsArgumentInspectionTest { override val inspection: InspectionProfileEntry get() = LoggingStringTemplateAsArgumentInspection().apply { mySkipPrimitives = false - myLimitLevelType = LoggingStringTemplateAsArgumentInspection.LimitLevelType.ALL + myLimitLevelType = LoggingUtil.LimitLevelType.ALL } fun `test highlighting`() { diff --git a/uast/uast-java-ide/src/org/jetbrains/uast/java/generate/JavaUastCodeGenerationPlugin.kt b/uast/uast-java-ide/src/org/jetbrains/uast/java/generate/JavaUastCodeGenerationPlugin.kt index 308388a8f1c6..4c69f2bfe6ce 100644 --- a/uast/uast-java-ide/src/org/jetbrains/uast/java/generate/JavaUastCodeGenerationPlugin.kt +++ b/uast/uast-java-ide/src/org/jetbrains/uast/java/generate/JavaUastCodeGenerationPlugin.kt @@ -1,4 +1,4 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package org.jetbrains.uast.java.generate import com.intellij.codeInsight.BlockUtils @@ -71,7 +71,7 @@ internal class JavaUastCodeGenerationPlugin : UastCodeGenerationPlugin { adjustChainStyleToMethodCalls(oldPsi, newPsi) val factory = JavaPsiFacade.getElementFactory(oldPsi.project) val updOldPsi = when { - (newPsi is PsiBlockStatement || newPsi is PsiCodeBlock) && oldPsi.parent is PsiExpressionStatement -> oldPsi.parent + (newPsi is PsiCodeBlock || newPsi is PsiStatement) && oldPsi.parent is PsiExpressionStatement -> oldPsi.parent else -> oldPsi } val updNewPsi = when {