MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-01-05 01:50:56 +07:00
Code Issues Packages Projects Releases Wiki Activity

[java-inspections] IDEA-326706 Filter guards for logs if any of calls are accepted

Browse Source 
GitOrigin-RevId: 201056e9b3ea856df391fc4aba8d01605cdb053f
This commit is contained in:
Mikhail Pyltsin
2023-08-02 19:14:48 +02:00
committed by intellij-monorepo-bot
parent d6b4ea495e
commit c80e4efe26
4 changed files with 247 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

75
jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingConditionDisagreesWithLogLevelStatementInspection.kt
View File

@@ -7,10 +7,14 @@ import com.intellij.codeInspection.LocalInspectionToolSession
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.codeInspection.registerUProblem
import com.intellij.psi.PsiElementVisitor
import com.intellij.psi.util.CachedValueProvider
import com.intellij.psi.util.CachedValuesManager
import com.intellij.psi.util.PsiModificationTracker
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.visitor.AbstractUastNonRecursiveVisitor
class LoggingConditionDisagreesWithLogLevelStatementInspection : AbstractBaseUastLocalInspectionTool() {
@@ -28,6 +32,7 @@ class LoggingConditionDisagreesWithLogLevelStatementInspection : AbstractBaseUas
) : AbstractUastNonRecursiveVisitor() {
override fun visitCallExpression(node: UCallExpression): Boolean {
//unfortunately it is impossible to use guarded as a start point, because it can be really complex
if (LoggingUtil.LOG_MATCHERS.uCallMatches(node)) {
processActualLoggers(node)
}
@@ -37,24 +42,70 @@ class LoggingConditionDisagreesWithLogLevelStatementInspection : AbstractBaseUas
return true
}
private fun processActualLoggers(call: UCallExpression) {
val guardedCondition = LoggingUtil.getGuardedCondition(call) ?: return
val loggerLevel = LoggingUtil.getLoggerLevel(call) ?: return
val levelFromCondition = LoggingUtil.getLevelFromCondition(guardedCondition) ?: return
if (!LoggingUtil.isGuardedIn(levelFromCondition, loggerLevel)) {
registerProblem(guardedCondition, levelFromCondition.name, loggerLevel.name)
private fun processActualLoggers(callExpression: UCallExpression) {
val guardedCondition = LoggingUtil.getGuardedCondition(callExpression) ?: return
if (!guardIsUsed(guardedCondition)) {
val loggerLevel = LoggingUtil.getLoggerLevel(callExpression) ?: return
val levelFromCondition = LoggingUtil.getLevelFromCondition(guardedCondition) ?: return
if (!LoggingUtil.isGuardedIn(levelFromCondition, loggerLevel)) {
registerProblem(guardedCondition, levelFromCondition.name, loggerLevel.name)
}
}
}
private fun processLegacyLoggers(call: UCallExpression) {
val guardedCondition = LoggingUtil.getGuardedCondition(call) ?: return
val loggerLevel = LoggingUtil.getLegacyLoggerLevel(call) ?: return
val levelFromCondition = LoggingUtil.getLegacyLevelFromCondition(guardedCondition) ?: return
if (!LoggingUtil.isLegacyGuardedIn(levelFromCondition, loggerLevel)) {
registerProblem(guardedCondition, levelFromCondition.name, loggerLevel.name)
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)
}
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 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) {
val guardedCondition = LoggingUtil.getGuardedCondition(callExpression) ?: return
if (!guardIsUsedLegacy(guardedCondition)) {
val loggerLevel = LoggingUtil.getLegacyLoggerLevel(callExpression) ?: return
val levelFromCondition = LoggingUtil.getLegacyLevelFromCondition(guardedCondition) ?: return
if (!LoggingUtil.isLegacyGuardedIn(levelFromCondition, loggerLevel)) {
registerProblem(guardedCondition, levelFromCondition.name, loggerLevel.name)
}
}
}
private fun guardIsUsedLegacy(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)
}
val calls: List<UCallExpression> = LoggingUtil.getLoggerCalls(guardedCondition)
val levelFromCondition = LoggingUtil.getLegacyLevelFromCondition(guardedCondition)
?: return@CachedValueProvider CachedValueProvider.Result.create(false,
PsiModificationTracker.MODIFICATION_COUNT)
return@CachedValueProvider CachedValueProvider.Result.create(calls.any { call ->
val condition = LoggingUtil.getGuardedCondition(call)
if ((guardedCondition.sourcePsi?.isEquivalentTo(condition?.sourcePsi)) != true) return@any false
val loggerLevel = LoggingUtil.getLegacyLoggerLevel(call) ?: return@any false
LoggingUtil.isLegacyGuardedIn(levelFromCondition, loggerLevel)
}, PsiModificationTracker.MODIFICATION_COUNT)
})
}
private fun registerProblem(call: UExpression,
levelFromCondition: String,
loggerLevel: String) {

65
jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingUtil.kt
View File

@@ -1,9 +1,13 @@
// 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
internal class LoggingUtil {
companion object {
@@ -129,7 +133,15 @@ internal class LoggingUtil {
internal fun getGuardedCondition(call: UCallExpression?): UExpression? {
if (call == null) return null
val loggerSource = getLoggerQualifier(call) ?: return null
val ifExpression = call.getParentOfType<UIfExpression>() ?: return null
var ifExpression: UIfExpression? = call.getParentOfType<UIfExpression>() ?: return null
while (ifExpression != null) {
if (getReferencesForVariable(loggerSource, ifExpression.condition).isEmpty()) {
ifExpression = ifExpression.getParentOfType<UIfExpression>() ?: return null
continue
}
break
}
if (ifExpression == null) return null
var condition = ifExpression.condition.skipParenthesizedExprDown()
if (condition is UPrefixExpression) {
if (condition.operator != UastPrefixOperator.LOGICAL_NOT) return null
@@ -201,7 +213,7 @@ internal class LoggingUtil {
}
}
if (resolvedReceiver is UMethod) {
if(!resolvedReceiver.uastParameters.isEmpty()) return null
if (!resolvedReceiver.uastParameters.isEmpty()) return null
val methodType = resolvedReceiver.returnType?.canonicalText ?: return null
if (LOGGER_CLASSES.contains(methodType) || LEGACY_LOGGER_CLASSES.contains(methodType)) {
return resolvedReceiver
@@ -324,6 +336,38 @@ internal class LoggingUtil {
return count
}
fun getLoggerCalls(guardedCondition: UExpression): List<UCallExpression> {
val sourcePsi = guardedCondition.sourcePsi ?: return emptyList()
return CachedValuesManager.getManager(sourcePsi.project).getCachedValue(sourcePsi, CachedValueProvider {
val qualifier = when (val guarded = sourcePsi.toUElementOfType<UExpression>()) {
is UQualifiedReferenceExpression -> {
(guarded.receiver as? UResolvable)?.resolveToUElement() as? UVariable
}
is UCallExpression -> {
(guarded.receiver as? UResolvable)?.resolveToUElement() as? UVariable
}
else -> {
null
}
}
if (qualifier == null) {
return@CachedValueProvider CachedValueProvider.Result.create(emptyList<UCallExpression>(),
PsiModificationTracker.MODIFICATION_COUNT)
}
val uIfExpression = guardedCondition.getParentOfType<UIfExpression>()
if (uIfExpression == null) {
return@CachedValueProvider CachedValueProvider.Result.create(emptyList<UCallExpression>(),
PsiModificationTracker.MODIFICATION_COUNT)
}
val referencesForVariable = getReferencesForVariable(qualifier, uIfExpression)
return@CachedValueProvider CachedValueProvider.Result.create(
referencesForVariable.mapNotNull { it.selector as? UCallExpression }
.filter { it.sourcePsi?.containingFile != null }
.filter { LOG_MATCHERS.uCallMatches(it) || LEGACY_LOG_MATCHERS.uCallMatches(it) },
PsiModificationTracker.MODIFICATION_COUNT)
})
}
enum class LoggerType {
SLF4J_LOGGER_TYPE, SLF4J_BUILDER_TYPE, LOG4J_LOGGER_TYPE, LOG4J_BUILDER_TYPE
}
@@ -337,4 +381,21 @@ internal class LoggingUtil {
FATAL, ERROR, SEVERE, WARN, WARNING, INFO, DEBUG, TRACE, CONFIG, FINE, FINER, FINEST
}
}
}
internal fun getReferencesForVariable(variable: UElement, context: UElement): List<UQualifiedReferenceExpression> {
val sourcePsi = variable.sourcePsi ?: return emptyList()
val result: MutableList<UQualifiedReferenceExpression> = mutableListOf()
val visitor: AbstractUastVisitor = object : AbstractUastVisitor() {
override fun visitQualifiedReferenceExpression(node: UQualifiedReferenceExpression): Boolean {
val selector = node.receiver
val resolveToUElement = (selector as? UResolvable)?.resolveToUElement() ?: return true
if (sourcePsi.isEquivalentTo(resolveToUElement.sourcePsi)) {
result.add(node)
}
return true
}
}
context.accept(visitor)
return result
}

73
jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingConditionDisagreesWithLogLevelStatementInspectionTest.kt
View File

@@ -22,6 +22,79 @@ class JavaLoggingConditionDisagreesWithLogLevelStatementInspectionTest : Logging
""".trimIndent())
}
fun `test double guarded`() {
myFixture.testHighlighting(JvmLanguage.JAVA, """
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
class X {
private static final Logger LOG = LoggerFactory.getLogger(X.class);
private static void test2() {
if (LOG.isDebugEnabled()) {
try {
something();
if (LOG.<warning descr="Level of condition 'INFO' does not match level of logging call 'DEBUG'">isInfoEnabled</warning>()) {
if (problem()) {
LOG.debug("test3");
}
}
} catch (Exception ignore) {
}
}
}
private static boolean something() {
return false;
}
private static boolean problem() {
return false;
}
}
""".trimIndent())
}
fun `test several logs`() {
myFixture.testHighlighting(JvmLanguage.JAVA, """
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
class X {
private static final Logger logger = LoggerFactory.getLogger(X.class);
public static void test1() {
if (logger.isDebugEnabled()) {
try {
something();
logger.debug("a");
} catch (Exception e) {
logger.error("a");
}
}
}
private static void something() {
}
}
""".trimIndent())
}
fun `test several logs 2`() {
myFixture.testHighlighting(JvmLanguage.JAVA, """
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
class X {
private static final Logger logger = LoggerFactory.getLogger(X.class);
public static void test1() {
if (logger.<warning descr="Level of condition 'DEBUG' does not match level of logging call 'ERROR'"><warning descr="Level of condition 'DEBUG' does not match level of logging call 'INFO'">isDebugEnabled</warning></warning>()) {
try {
something();
logger.info("a");
} catch (Exception e) {
logger.error("a");
}
}
}
private static void something() {
}
}
""".trimIndent())
}
fun `test log4j2`() {
myFixture.testHighlighting(JvmLanguage.JAVA, """
import org.apache.logging.log4j.*;

48
jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingConditionDisagreesWithLogLevelStatementInspectionTest.kt
View File

@@ -54,6 +54,54 @@ class KotlinLoggingConditionDisagreesWithLogLevelStatementInspectionTest : Loggi
""".trimIndent())
}
fun `test several logs`() {
myFixture.testHighlighting(JvmLanguage.KOTLIN, """
import org.slf4j.Logger
import org.slf4j.LoggerFactory
internal object X {
private val logger = LoggerFactory.getLogger()
fun test1() {
if (logger.isDebugEnabled) {
try {
something()
logger.debug("a")
} catch (e: Exception) {
logger.error("a")
}
}
if (logger.isDebugEnabled()) {
try {
something()
logger.debug("a")
} catch (e: Exception) {
logger.error("a")
}
}
if (logger.<warning descr="Level of condition 'INFO' does not match level of logging call 'DEBUG'"><warning descr="Level of condition 'INFO' does not match level of logging call 'ERROR'">isInfoEnabled</warning></warning>) {
try {
something()
logger.debug("a")
} catch (e: Exception) {
logger.error("a")
}
}
if (logger.<warning descr="Level of condition 'INFO' does not match level of logging call 'DEBUG'"><warning descr="Level of condition 'INFO' does not match level of logging call 'ERROR'">isInfoEnabled</warning></warning>()) {
try {
something()
logger.debug("a")
} catch (e: Exception) {
logger.error("a")
}
}
}
private fun something() {}
}
""".trimIndent())
}
fun `test java util logging`() {
myFixture.testHighlighting(JvmLanguage.KOTLIN, """
import java.util.logging.Level