[uast-inspections] IDEA-134025 Quickfix for "Log condition does not match logging call"

GitOrigin-RevId: 9bf9f012525aab45167daf84dd13ebd0a8c135fa
2025-12-16 22:51:17 +07:00 · 2024-01-08 18:32:03 +01:00
parent e6597bc5a8
commit cb7187787c
5 changed files with 354 additions and 50 deletions
--- a/jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties
+++ b/jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties
@@ -178,6 +178,8 @@ jvm.inspection.logging.similar.message.problem.descriptor=Similar log messages

 jvm.inspection.logging.condition.disagrees.with.log.statement.display.name=Log condition does not match logging call
 jvm.inspection.logging.condition.disagrees.with.log.statement.problem.descriptor=Level of condition ''{0}'' does not match level of logging call ''{1}''
+jvm.inspection.logging.condition.disagrees.with.log.statement.fix.name=Change level of the {0, choice, 0#condition|1#call}
+jvm.inspection.logging.condition.disagrees.with.log.statement.fix.family.name=Change log level

 jvm.inspection.test.failed.line.display.name=Failed line in test

--- a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingConditionDisagreesWithLogLevelStatementInspection.kt
+++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingConditionDisagreesWithLogLevelStatementInspection.kt
@@ -2,19 +2,22 @@
 package com.intellij.codeInspection.logging

 import com.intellij.analysis.JvmAnalysisBundle
-import com.intellij.codeInspection.AbstractBaseUastLocalInspectionTool
-import com.intellij.codeInspection.LocalInspectionToolSession
-import com.intellij.codeInspection.ProblemsHolder
-import com.intellij.codeInspection.registerUProblem
+import com.intellij.codeInspection.*
+import com.intellij.modcommand.ModPsiUpdater
+import com.intellij.modcommand.PsiUpdateModCommandQuickFix
+import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiElementVisitor
+import com.intellij.psi.SmartPointerManager
+import com.intellij.psi.SmartPsiElementPointer
 import com.intellij.psi.util.CachedValueProvider
 import com.intellij.psi.util.CachedValuesManager
 import com.intellij.psi.util.PsiModificationTracker
+import com.intellij.psi.util.PsiTreeUtil
 import com.intellij.uast.UastHintedVisitorAdapter
-import org.jetbrains.uast.UCallExpression
-import org.jetbrains.uast.UExpression
-import org.jetbrains.uast.UReferenceExpression
-import org.jetbrains.uast.toUElementOfType
+import org.jetbrains.uast.*
+import org.jetbrains.uast.generate.getUastElementFactory
+import org.jetbrains.uast.generate.replace
 import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor

 class LoggingConditionDisagreesWithLogLevelStatementInspection : AbstractBaseUastLocalInspectionTool() {
@@ -48,29 +51,24 @@ class LoggingConditionDisagreesWithLogLevelStatementInspection : AbstractBaseUas
      val loggerLevel = LoggingUtil.getLoggerLevel(callExpression) ?: return
      val levelFromCondition = LoggingUtil.getLevelFromCondition(guardedCondition) ?: return
      if (!LoggingUtil.isGuardedIn(levelFromCondition, loggerLevel)) {
-        registerProblem(guardedCondition, levelFromCondition.name, loggerLevel.name)
+        registerProblem(guardedCondition, levelFromCondition.name, loggerLevel.name, callExpression, guardedCondition)
      }
    }

    private fun guardIsUsed(guarded: UExpression): Boolean {
      val sourcePsi = guarded.sourcePsi ?: return false
-      return CachedValuesManager.getManager(sourcePsi.project).getCachedValue(sourcePsi, CachedValueProvider {
      val guardedCondition = sourcePsi.toUElementOfType<UExpression>()
      if (guardedCondition == null) {
-          return@CachedValueProvider CachedValueProvider.Result.create(false,
-                                                                       PsiModificationTracker.MODIFICATION_COUNT)
+        return false
      }
      val calls: List<UCallExpression> = LoggingUtil.getLoggerCalls(guardedCondition)
-        val levelFromCondition = LoggingUtil.getLevelFromCondition(guardedCondition)
-                                 ?: return@CachedValueProvider CachedValueProvider.Result.create(false,
-                                                                                                 PsiModificationTracker.MODIFICATION_COUNT)
-        return@CachedValueProvider CachedValueProvider.Result.create(calls.any { call ->
+      val levelFromCondition = LoggingUtil.getLevelFromCondition(guardedCondition) ?: return false
+      return calls.any { call ->
        val condition = LoggingUtil.getGuardedCondition(call)
        if ((guardedCondition.sourcePsi?.isEquivalentTo(condition?.sourcePsi)) != true) return@any false
        val loggerLevel = LoggingUtil.getLoggerLevel(call) ?: return@any false
        LoggingUtil.isGuardedIn(levelFromCondition, loggerLevel)
-        }, PsiModificationTracker.MODIFICATION_COUNT)
-      })
+      }
    }

    private fun processLegacyLoggers(callExpression: UCallExpression) {
@@ -79,7 +77,7 @@ class LoggingConditionDisagreesWithLogLevelStatementInspection : AbstractBaseUas
      val loggerLevel = LoggingUtil.getLegacyLoggerLevel(callExpression) ?: return
      val levelFromCondition = LoggingUtil.getLegacyLevelFromCondition(guardedCondition) ?: return
      if (!LoggingUtil.isLegacyGuardedIn(levelFromCondition, loggerLevel)) {
-        registerProblem(guardedCondition, levelFromCondition.name, loggerLevel.name)
+        registerProblem(guardedCondition, levelFromCondition.name, loggerLevel.name, callExpression, guardedCondition)
      }
    }

@@ -106,15 +104,120 @@ class LoggingConditionDisagreesWithLogLevelStatementInspection : AbstractBaseUas

    private fun registerProblem(call: UExpression,
                                levelFromCondition: String,
-                                loggerLevel: String) {
+                                loggerLevel: String,
+                                callExpression: UCallExpression? = null,
+                                guardedCondition: UExpression? = null) {
+      val fixes = createFixes(callExpression, guardedCondition)
      val message = JvmAnalysisBundle.message("jvm.inspection.logging.condition.disagrees.with.log.statement.problem.descriptor",
                                              levelFromCondition, loggerLevel)
      if (call is UCallExpression) {
-        holder.registerUProblem(call, message)
+        holder.registerUProblem(call, message, *fixes)
      }
      if (call is UReferenceExpression) {
-        holder.registerUProblem(call, message)
+        holder.registerUProblem(call, message, *fixes)
+      }
+    }
+
+    private fun createFixes(callExpression: UCallExpression?, guardedCondition: UExpression?): Array<LocalQuickFix> {
+      if (callExpression == null || guardedCondition == null) {
+        return emptyArray()
+      }
+      val result = mutableListOf<LocalQuickFix>()
+      val logFix = createChangeLog(callExpression, guardedCondition)
+      if (logFix != null) {
+        result.add(logFix)
+      }
+      val guardFix = createChangeGuard(guardedCondition, callExpression)
+      if (guardFix != null) {
+        result.add(guardFix)
+      }
+      return result.toTypedArray()
+    }
+
+    private fun createChangeGuard(guard: UExpression, callExpression: UCallExpression): LocalQuickFix? {
+      val guardName = getGuardName(guard) ?: return null
+      LoggingUtil.GUARD_MAP[guardName] ?: return null
+      if (LoggingUtil.getLoggerCalls(guard).size != 1) return null
+      val callToGuard = LoggingUtil.GUARD_MAP.entries.firstOrNull { it.value == callExpression.methodName }?.key ?: return null
+      return ChangeCallNameFix(FixType.GUARD, guard, callToGuard)
+    }
+
+    private fun createChangeLog(callExpression: UCallExpression, guard: UExpression): LocalQuickFix? {
+      val guardName = getGuardName(guard) ?: return null
+      val guardToCall = LoggingUtil.GUARD_MAP[guardName] ?: return null
+      if (!LoggingUtil.GUARD_MAP.values.contains(callExpression.methodName)) return null
+      return ChangeCallNameFix(FixType.CALL, callExpression, guardToCall)
+    }
+
+    private fun getGuardName(guard: UExpression): String? {
+      return when (guard) {
+        is UCallExpression -> {
+          guard.methodName
+        }
+        is UQualifiedReferenceExpression -> {
+          guard.resolvedName
+        }
+        else -> {
+          null
+        }
      }
    }
  }
 }
+
+private fun ChangeCallNameFix(fixType: FixType, expression: UExpression, newName: String): ChangeCallNameFix? {
+  val sourcePsi = expression.sourcePsi ?: return null
+  val pointer = SmartPointerManager.createPointer(sourcePsi)
+  return ChangeCallNameFix(fixType, pointer, newName)
+}
+
+private enum class FixType { CALL, GUARD }
+private class ChangeCallNameFix(private val myFixType: FixType,
+                                private val myExpression: SmartPsiElementPointer<PsiElement?>,
+                                private val newName: String) : PsiUpdateModCommandQuickFix() {
+
+  override fun getName(): String {
+    return JvmAnalysisBundle.message("jvm.inspection.logging.condition.disagrees.with.log.statement.fix.name",
+                                     if (myFixType == FixType.GUARD) 0 else 1)
+  }
+
+  override fun getFamilyName(): String {
+    return JvmAnalysisBundle.message("jvm.inspection.logging.condition.disagrees.with.log.statement.fix.family.name")
+  }
+
+  override fun applyFix(project: Project, element: PsiElement, updater: ModPsiUpdater) {
+    val uCallExpression = if (myFixType == FixType.GUARD) {
+      val referenceExpression = myExpression.element.toUElementOfType<UCallExpression>()
+                                ?: myExpression.element.toUElementOfType<UQualifiedReferenceExpression>() ?: return
+      if (referenceExpression is UCallExpression) {
+        element.getUastParentOfType<UCallExpression>() ?: return
+      }
+      else {
+        element.getUastParentOfType<UQualifiedReferenceExpression>() ?: return
+      }
+    }
+    else {
+      val uCallExpression = myExpression.element.toUElementOfType<UCallExpression>() ?: return
+      val psiElement = PsiTreeUtil.findSameElementInCopy(uCallExpression.sourcePsi, element.containingFile) ?: return
+      psiElement.toUElementOfType<UCallExpression>() ?: return
+    }
+    val elementFactory = uCallExpression.getUastElementFactory(project) ?: return
+
+    if (uCallExpression is UCallExpression) {
+      val newCall = elementFactory.createCallExpression(uCallExpression.receiver, newName, uCallExpression.valueArguments,
+                                                        uCallExpression.returnType,
+                                                        uCallExpression.kind, uCallExpression.sourcePsi
+      ) ?: return
+      val oldCall = uCallExpression.getQualifiedParentOrThis()
+      oldCall.replace(newCall)
+    }
+    else if (uCallExpression is UQualifiedReferenceExpression) {
+      val receiver = uCallExpression.receiver.sourcePsi?.text ?: return
+      val newReference: UQualifiedReferenceExpression =
+        elementFactory.createQualifiedReference(qualifiedName = "$receiver.$newName",
+                                                context = uCallExpression.sourcePsi) ?: return
+      uCallExpression.replace(newReference)
+    }
+    return
+  }
+}
--- 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,7 +1,10 @@
 // Copyright 2000-2023 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
+import com.intellij.psi.util.CachedValuesManager
 import com.intellij.psi.util.InheritanceUtil
+import com.intellij.psi.util.PsiModificationTracker
 import com.siyeh.ig.callMatcher.CallMatcher
 import org.jetbrains.uast.*
 import org.jetbrains.uast.visitor.AbstractUastVisitor
@@ -357,6 +360,8 @@ internal class LoggingUtil {

    fun getLoggerCalls(guardedCondition: UExpression): List<UCallExpression> {
      val sourcePsi = guardedCondition.sourcePsi ?: return emptyList()
+      return CachedValuesManager.getManager(sourcePsi.project).getCachedValue(sourcePsi, CachedValueProvider {
+        val emptyResult = CachedValueProvider.Result.create(listOf<UCallExpression>(), PsiModificationTracker.MODIFICATION_COUNT)
        val qualifier = when (val guarded = sourcePsi.toUElementOfType<UExpression>()) {
          is UQualifiedReferenceExpression -> {
            (guarded.receiver as? UResolvable)?.resolveToUElement() as? UVariable
@@ -369,16 +374,19 @@ internal class LoggingUtil {
          }
        }
        if (qualifier == null) {
-        return emptyList()
+          return@CachedValueProvider emptyResult
        }
        val uIfExpression = guardedCondition.getParentOfType<UIfExpression>()
        if (uIfExpression == null) {
-        return emptyList()
+          return@CachedValueProvider emptyResult
        }
        val referencesForVariable = getReferencesForVariable(qualifier, uIfExpression)
-      return referencesForVariable.mapNotNull { it.selector as? UCallExpression }
+        val filtered = referencesForVariable.mapNotNull { it.selector as? UCallExpression }
          .filter { it.sourcePsi?.containingFile != null }
          .filter { LOG_MATCHERS.uCallMatches(it) || LEGACY_LOG_MATCHERS.uCallMatches(it) }
+        return@CachedValueProvider CachedValueProvider.Result.create(filtered,
+                                                                     PsiModificationTracker.MODIFICATION_COUNT)
+      })
    }

    enum class LoggerType {
@@ -393,5 +401,14 @@ internal class LoggingUtil {
    enum class LegacyLevelType {
      FATAL, ERROR, SEVERE, WARN, WARNING, INFO, DEBUG, TRACE, CONFIG, FINE, FINER, FINEST
    }
+
+    internal val GUARD_MAP = mapOf(
+      Pair("isTraceEnabled", "trace"),
+      Pair("isDebugEnabled", "debug"),
+      Pair("isInfoEnabled", "info"),
+      Pair("isWarnEnabled", "warn"),
+      Pair("isErrorEnabled", "error"),
+      Pair("isFatalEnabled", "fatal"),
+    )
  }
 }
--- a/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingConditionDisagreesWithLogLevelStatementInspectionTest.kt
+++ b/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingConditionDisagreesWithLogLevelStatementInspectionTest.kt
@@ -1,5 +1,6 @@
 package com.intellij.codeInspection.tests.java.logging

+import com.intellij.analysis.JvmAnalysisBundle
 import com.intellij.jvm.analysis.internal.testFramework.logging.LoggingConditionDisagreesWithLogLevelStatementInspectionTestBase
 import com.intellij.jvm.analysis.testFramework.JvmLanguage

@@ -125,4 +126,112 @@ class JavaLoggingConditionDisagreesWithLogLevelStatementInspectionTest : Logging
      }
      """.trimIndent())
  }
+
+  fun `test fixes log4j2 change calls`() {
+      myFixture.testQuickFix(
+        testPreview = true,
+        lang = JvmLanguage.JAVA,
+        before = """
+      import org.apache.logging.log4j.LogManager;
+      import org.apache.logging.log4j.Logger;
+      
+      class Logging {
+          private static final Logger LOG = LogManager.getLogger();
+      
+          private static void request1(String i) {
+              String msg = "log messages2: {}";
+              if (LOG.is<caret>DebugEnabled()) {
+                  LOG.info(msg, i);
+              }
+          }
+      }""".trimIndent(),
+        after = """
+      import org.apache.logging.log4j.LogManager;
+      import org.apache.logging.log4j.Logger;
+      
+      class Logging {
+          private static final Logger LOG = LogManager.getLogger();
+      
+          private static void request1(String i) {
+              String msg = "log messages2: {}";
+              if (LOG.isDebugEnabled()) {
+                  LOG.debug(msg, i);
+              }
+          }
+      }""".trimIndent(),
+      hint = JvmAnalysisBundle.message("jvm.inspection.logging.condition.disagrees.with.log.statement.fix.name", 1)
+      )
+  }
+
+  fun `test fixes log4j2 change guards`() {
+      myFixture.testQuickFix(
+        testPreview = true,
+        lang = JvmLanguage.JAVA,
+        before = """
+      import org.apache.logging.log4j.LogManager;
+      import org.apache.logging.log4j.Logger;
+      
+      class Logging {
+          private static final Logger LOG = LogManager.getLogger();
+      
+          private static void request1(String i) {
+              String msg = "log messages2: {}";
+              if (LOG.is<caret>DebugEnabled()) {
+                  LOG.info(msg, i);
+              }
+          }
+      }""".trimIndent(),
+        after = """
+      import org.apache.logging.log4j.LogManager;
+      import org.apache.logging.log4j.Logger;
+      
+      class Logging {
+          private static final Logger LOG = LogManager.getLogger();
+      
+          private static void request1(String i) {
+              String msg = "log messages2: {}";
+              if (LOG.isInfoEnabled()) {
+                  LOG.info(msg, i);
+              }
+          }
+      }""".trimIndent(),
+      hint = JvmAnalysisBundle.message("jvm.inspection.logging.condition.disagrees.with.log.statement.fix.name", 0)
+      )
+  }
+
+  fun `test fixes slf4j change guards`() {
+      myFixture.testQuickFix(
+        testPreview = true,
+        lang = JvmLanguage.JAVA,
+        before = """
+      import org.slf4j.Logger;
+      import org.slf4j.LoggerFactory;
+      
+      class Slf4J {
+        private final static Logger log = LoggerFactory.getLogger(Slf4J.class);
+        
+        private static void request1(String i) {
+            String msg = "log messages2: {}";
+            if (log.is<caret>DebugEnabled()) {
+                log.info(msg, i);
+            }
+        }
+      }""".trimIndent(),
+        after = """
+      import org.slf4j.Logger;
+      import org.slf4j.LoggerFactory;
+      
+      class Slf4J {
+        private final static Logger log = LoggerFactory.getLogger(Slf4J.class);
+        
+        private static void request1(String i) {
+            String msg = "log messages2: {}";
+            if (log.isInfoEnabled()) {
+                log.info(msg, i);
+            }
+        }
+      }""".trimIndent(),
+      hint = JvmAnalysisBundle.message("jvm.inspection.logging.condition.disagrees.with.log.statement.fix.name", 0)
+      )
+  }
 }
--- a/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingConditionDisagreesWithLogLevelStatementInspectionTest.kt
+++ b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingConditionDisagreesWithLogLevelStatementInspectionTest.kt
@@ -1,5 +1,6 @@
 package com.intellij.codeInspection.tests.kotlin.logging

+import com.intellij.analysis.JvmAnalysisBundle
 import com.intellij.jvm.analysis.internal.testFramework.logging.LoggingConditionDisagreesWithLogLevelStatementInspectionTestBase
 import com.intellij.jvm.analysis.testFramework.JvmLanguage

@@ -121,4 +122,76 @@ class KotlinLoggingConditionDisagreesWithLogLevelStatementInspectionTest : Loggi
      }
    """.trimIndent())
  }
+
+  fun `test fixes slf4j change guards`() {
+    myFixture.testQuickFix(
+      testPreview = true,
+      lang = JvmLanguage.KOTLIN,
+      before = """
+      import org.slf4j.Logger
+      import org.slf4j.LoggerFactory
+      
+      internal object Slf4J {
+          private val log: Logger = LoggerFactory.getLogger(Slf4J::class.java)
+      
+          private fun request1(i: String) {
+              val msg = "log messages2: {}"
+              if (log.isDeb<caret>ugEnabled) {
+                  log.info(msg, i)
+              }
+          }
+      }""".trimIndent(),
+      after = """
+      import org.slf4j.Logger
+      import org.slf4j.LoggerFactory
+      
+      internal object Slf4J {
+          private val log: Logger = LoggerFactory.getLogger(Slf4J::class.java)
+      
+          private fun request1(i: String) {
+              val msg = "log messages2: {}"
+              if (log.isInfoEnabled) {
+                  log.info(msg, i)
+              }
+          }
+      }""".trimIndent(),
+      hint = JvmAnalysisBundle.message("jvm.inspection.logging.condition.disagrees.with.log.statement.fix.name", 0)
+    )
+  }
+
+  fun `test fixes slf4j change guards as methods`() {
+    myFixture.testQuickFix(
+      testPreview = true,
+      lang = JvmLanguage.KOTLIN,
+      before = """
+      import org.slf4j.Logger
+      import org.slf4j.LoggerFactory
+      
+      internal object Slf4J {
+          private val log: Logger = LoggerFactory.getLogger(Slf4J::class.java)
+      
+          private fun request1(i: String) {
+              val msg = "log messages2: {}"
+              if (log.isDeb<caret>ugEnabled()) {
+                  log.info(msg, i)
+              }
+          }
+      }""".trimIndent(),
+      after = """
+      import org.slf4j.Logger
+      import org.slf4j.LoggerFactory
+      
+      internal object Slf4J {
+          private val log: Logger = LoggerFactory.getLogger(Slf4J::class.java)
+      
+          private fun request1(i: String) {
+              val msg = "log messages2: {}"
+              if (log.isInfoEnabled()) {
+                  log.info(msg, i)
+              }
+          }
+      }""".trimIndent(),
+      hint = JvmAnalysisBundle.message("jvm.inspection.logging.condition.disagrees.with.log.statement.fix.name", 0)
+    )
+  }
 }