From b7d3abc5227dc7492b52c4e43fc2d071d352221f Mon Sep 17 00:00:00 2001 From: Mikhail Pyltsin Date: Thu, 15 Aug 2024 12:54:33 +0200 Subject: [PATCH] [uast-inspections] IDEA-357019 Inspection warning 'Number of placeholders do not match number of arguments in logging call' not reported when logger not initialized inline GitOrigin-RevId: e601f8f8c94233af84856b7be2626f71b57325eb --- .../logging/LoggingPlaceholderUtil.kt | 107 ++++++++++++++---- ...CountMatchesArgumentCountInspectionTest.kt | 85 ++++++++++++++ ...entCountInspectionPlaceholderNumberTest.kt | 61 ++++++++++ 3 files changed, 230 insertions(+), 23 deletions(-) diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingPlaceholderUtil.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingPlaceholderUtil.kt index 47bec07866d8..5ad80abaf72d 100644 --- a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingPlaceholderUtil.kt +++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingPlaceholderUtil.kt @@ -3,13 +3,17 @@ package com.intellij.codeInspection.logging import com.intellij.openapi.util.TextRange import com.intellij.psi.* +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.intellij.util.containers.addIfNotNull import com.siyeh.ig.callMatcher.CallMapper import com.siyeh.ig.callMatcher.CallMatcher import com.siyeh.ig.format.FormatDecode import com.siyeh.ig.psiutils.TypeUtils import org.jetbrains.uast.* +import org.jetbrains.uast.UastBinaryOperator.Companion.ASSIGN import org.jetbrains.uast.visitor.AbstractUastVisitor const val MAX_BUILDER_LENGTH = 20 @@ -32,28 +36,74 @@ private val SLF4J_HOLDER = object : LoggerTypeSearcher { internal val LOG4J_LOG_BUILDER_HOLDER = object : LoggerTypeSearcher { override fun findType(expression: UCallExpression, context: LoggerContext): PlaceholderLoggerType? { + var initializers: List? var qualifierExpression = getImmediateLoggerQualifier(expression) if (qualifierExpression is UReferenceExpression) { val target: UVariable = qualifierExpression.resolveToUElement() as? UVariable ?: return null if (!target.isFinal) { return PlaceholderLoggerType.LOG4J_EQUAL_PLACEHOLDERS //formatted builder is really rare } - qualifierExpression = target.uastInitializer?.skipParenthesizedExprDown() - } - if (qualifierExpression is UQualifiedReferenceExpression) { - qualifierExpression = qualifierExpression.selector - } - if (qualifierExpression is UCallExpression) { - return when (LOG4J_HOLDER.findType(qualifierExpression, context)) { - PlaceholderLoggerType.LOG4J_FORMATTED_STYLE -> PlaceholderLoggerType.LOG4J_FORMATTED_STYLE - PlaceholderLoggerType.LOG4J_OLD_STYLE -> PlaceholderLoggerType.LOG4J_EQUAL_PLACEHOLDERS - else -> null + qualifierExpression = target.uastInitializer + if (qualifierExpression == null) { + initializers = tryToFindInitializers(target, expression) + } + else { + initializers = listOf(qualifierExpression) } } - return PlaceholderLoggerType.LOG4J_EQUAL_PLACEHOLDERS + else { + initializers = listOf(qualifierExpression) + } + if (initializers == null) return null + return initializers.map { + it?.skipParenthesizedExprDown() + }.map { + if (it is UQualifiedReferenceExpression) { + it.selector + } + else { + it + } + }.map { + if (it is UCallExpression) { + when (LOG4J_HOLDER.findType(it, context)) { + PlaceholderLoggerType.LOG4J_FORMATTED_STYLE -> PlaceholderLoggerType.LOG4J_FORMATTED_STYLE + PlaceholderLoggerType.LOG4J_OLD_STYLE -> PlaceholderLoggerType.LOG4J_EQUAL_PLACEHOLDERS + else -> null + } + } + else { + PlaceholderLoggerType.LOG4J_EQUAL_PLACEHOLDERS + } + }.distinct().singleOrNull() } } +private fun tryToFindInitializers(originalVariable: UVariable, expression: UCallExpression): List? { + val variableText = originalVariable.name + if (variableText == null) return null + if (originalVariable.getContainingUFile()?.sourcePsi != expression.getContainingUFile()?.sourcePsi) return null + val element = originalVariable.sourcePsi ?: return null + return CachedValuesManager.getManager(element.project).getCachedValue(element, CachedValueProvider { + val variable = element.toUElementOfType() + val initializers = mutableListOf() + val uastParent = variable?.uastParent + uastParent?.accept(object : AbstractUastVisitor() { + override fun visitBinaryExpression(node: UBinaryExpression): Boolean { + if (node.operator == ASSIGN) { + val leftOperand = node.leftOperand + if (leftOperand is UReferenceExpression && leftOperand.sourcePsi?.text == variableText && leftOperand.resolveToUElement() == variable) { + initializers.add(node.rightOperand) + } + } + return super.visitBinaryExpression(node) + } + }) + return@CachedValueProvider CachedValueProvider.Result.create(initializers, + PsiModificationTracker.MODIFICATION_COUNT) + }) +} + internal val SLF4J_BUILDER_HOLDER = object : LoggerTypeSearcher { override fun findType(expression: UCallExpression, context: LoggerContext): PlaceholderLoggerType { if (context.log4jAsImplementationForSlf4j) { @@ -66,7 +116,7 @@ internal val SLF4J_BUILDER_HOLDER = object : LoggerTypeSearcher { private val LOG4J_HOLDER = object : LoggerTypeSearcher { override fun findType(expression: UCallExpression, context: LoggerContext): PlaceholderLoggerType? { val qualifierExpression = getImmediateLoggerQualifier(expression) - var initializer: UExpression? = null + var initializers: List? = null if (qualifierExpression is UReferenceExpression) { var resolvedToUElement = qualifierExpression.resolveToUElement() if (resolvedToUElement is UMethod) { @@ -82,20 +132,31 @@ private val LOG4J_HOLDER = object : LoggerTypeSearcher { if (!target.isFinal) { return null } - initializer = target.uastInitializer - if (initializer == null) return null + val uastInitializer = target.uastInitializer + initializers = if (uastInitializer != null) listOf(uastInitializer) else null + if (initializers == null) { + initializers = tryToFindInitializers(target, expression) + } } else if (qualifierExpression is UCallExpression) { - initializer = qualifierExpression + initializers = listOf(qualifierExpression) } - initializer = initializer?.skipParenthesizedExprDown() - if (initializer is UQualifiedReferenceExpression) { - initializer = initializer.selector - } - return if (initializer is UCallExpression && LoggingUtil.FORMATTED_LOG4J.uCallMatches(initializer)) { - PlaceholderLoggerType.LOG4J_FORMATTED_STYLE - } - else PlaceholderLoggerType.LOG4J_OLD_STYLE + if (initializers == null || initializers.isEmpty()) return null + return initializers.map { + it.skipParenthesizedExprDown() + }.map { + if (it is UQualifiedReferenceExpression) { + it.selector + } + else { + it + } + }.map { + if (it is UCallExpression && LoggingUtil.FORMATTED_LOG4J.uCallMatches(it)) { + PlaceholderLoggerType.LOG4J_FORMATTED_STYLE + } + else PlaceholderLoggerType.LOG4J_OLD_STYLE + }.distinct().singleOrNull() } } diff --git a/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingPlaceholderCountMatchesArgumentCountInspectionTest.kt b/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingPlaceholderCountMatchesArgumentCountInspectionTest.kt index 88d0c41c9a20..c7d10f0b1636 100644 --- a/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingPlaceholderCountMatchesArgumentCountInspectionTest.kt +++ b/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingPlaceholderCountMatchesArgumentCountInspectionTest.kt @@ -614,6 +614,91 @@ class JavaLoggingPlaceholderCountMatchesArgumentCountInspectionTest : LoggingPla """.trimIndent()) } + fun `test lazy init`() { + myFixture.testHighlighting(JvmLanguage.JAVA, """ + import org.apache.logging.log4j.LogBuilder; + import org.apache.logging.log4j.LogManager; + import org.apache.logging.log4j.Logger; + + class LazyInitializer { + + static class StaticInitializer { + private static final Logger log; + + static { + log = LogManager.getLogger(); + } + + public StaticInitializer() { + log.info("{}"); + } + } + + static class StaticInitializerBuilder { + private static final LogBuilder log; + + static { + log = LogManager.getLogger().atDebug(); + } + + public StaticInitializerBuilder() { + log.log("{}"); + } + } + + static class StaticInitializerBuilder2 { + private static final LogBuilder log; + + static { + if (1 == 1) { + log = LogManager.getLogger().atDebug(); + } else { + log = LogManager.getFormatterLogger().atDebug(); + } + } + + public StaticInitializerBuilder2() { + log.log("{}"); + } + } + + static class ConstructorInitializer { + private final Logger log; + + + public ConstructorInitializer() { + log = LogManager.getLogger(); + } + + public ConstructorInitializer(int i) { + log = LogManager.getLogger(); + } + + public void test() { + log.info("{}"); + } + } + + static class ConstructorInitializer2 { + private final Logger log; + + + public ConstructorInitializer2() { + log = LogManager.getFormatterLogger(); + } + + public ConstructorInitializer2(int i) { + log = LogManager.getLogger(); + } + + public void test() { + log.info("{}"); + } + } + } + """.trimIndent()) + } + fun `test slf4j structured logging`() { inspection.slf4jToLog4J2Type = LoggingPlaceholderCountMatchesArgumentCountInspection.Slf4jToLog4J2Type.NO myFixture.testHighlighting(JvmLanguage.JAVA, """ diff --git a/jvm/jvm-analysis-kotlin-tests-shared/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingPlaceholderCountMatchesArgumentCountInspectionPlaceholderNumberTest.kt b/jvm/jvm-analysis-kotlin-tests-shared/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingPlaceholderCountMatchesArgumentCountInspectionPlaceholderNumberTest.kt index e52f177588aa..ef950d94a658 100644 --- a/jvm/jvm-analysis-kotlin-tests-shared/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingPlaceholderCountMatchesArgumentCountInspectionPlaceholderNumberTest.kt +++ b/jvm/jvm-analysis-kotlin-tests-shared/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingPlaceholderCountMatchesArgumentCountInspectionPlaceholderNumberTest.kt @@ -203,4 +203,65 @@ abstract class KotlinLoggingPlaceholderCountMatchesArgumentCountInspectionPlaceh } """.trimIndent()) } + + fun `test lazy init`() { + myFixture.testHighlighting(JvmLanguage.KOTLIN, """ + import org.apache.logging.log4j.LogBuilder + import org.apache.logging.log4j.LogManager + import org.apache.logging.log4j.Logger + + class LazyInitializer { + + internal class StaticInitializerBuilder2 { + init { + log.log("{}") + } + + companion object { + private val log: LogBuilder + + init { + if (1 == 1) { + log = LogManager.getLogger().atDebug() + } else { + log = LogManager.getFormatterLogger().atDebug() + } + } + } + } + + internal class ConstructorInitializer { + private val log: Logger + + constructor() { + log = LogManager.getLogger() + } + + constructor(i: Int) { + log = LogManager.getLogger() + } + + fun test() { + log.info("{}") + } + } + + internal class ConstructorInitializer2 { + private val log: Logger + + constructor() { + log = LogManager.getFormatterLogger() + } + + constructor(i: Int) { + log = LogManager.getLogger() + } + + fun test() { + log.info("{}") + } + } + } + """.trimIndent()) + } } \ No newline at end of file