[java-inspection] IDEA-337700 Improvements for logging inspections

- new inspection LoggingGuardedByConditionInspection

GitOrigin-RevId: 4dc52a70105fb7bdbbf3d6ddbed5db2e6640fd0d
This commit is contained in:
Mikhail Pyltsin
2024-03-08 16:23:29 +01:00
committed by intellij-monorepo-bot
parent d856b5a54c
commit d4b2f1f75d
8 changed files with 669 additions and 4 deletions

View File

@@ -83,6 +83,12 @@
implementationClass="com.intellij.codeInspection.logging.LoggingStatementNotGuardedByLogConditionInspection"/>
<inspectionElementsMerger implementation="com.intellij.codeInspection.logging.LoggingStatementNotGuardedByLogConditionInspectionMerger"/>
<localInspection language="UAST" enabledByDefault="true" level="INFORMATION"
groupBundle="messages.JvmAnalysisBundle" bundle="messages.JvmAnalysisBundle"
groupPathKey="jvm.inspections.group.name" groupKey="jvm.inspections.logging.frameworks.group.name"
key="jvm.inspection.log.guarded.display.name"
implementationClass="com.intellij.codeInspection.logging.LoggingGuardedByConditionInspection"/>
<!-- Performance -->
<localInspection language="UAST" enabledByDefault="true" level="WARNING" shortName="UrlHashCode"
groupBundle="messages.JvmAnalysisBundle" bundle="messages.JvmAnalysisBundle"

View File

@@ -0,0 +1,27 @@
<html>
<body>
Reports logging calls that are surrounded by a guard condition.
This inspection can be used to adjust with a custom code style.
<p><b>Example:</b></p>
<pre><code lang="java">
public class TestObject {
void test(Object object) {
if(LOG.isDebugEnabled()){
LOG.debug("some logging " + expensiveCalculation(1));
}
}
}
</code></pre>
<p>After a quick-fix is applied:</p>
<pre><code lang="java">
public class TestObject {
void test(Object object) {
LOG.debug("some logging " + expensiveCalculation(1));
}
}
</code></pre>
<p>This inspection supports <em>Log4j2</em> and the <em>SLF4J</em> logging frameworks (except builders).
<!-- tooltip end -->
<p><small>New in 2024.2</small></p>
</body>
</html>

View File

@@ -181,6 +181,11 @@ jvm.inspection.log.statement.not.guarded.log.condition.text=Custom Log Condition
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.log.guarded.display.name=Logging calls guarded by log condition
jvm.inspection.log.guarded.warn.if.fix.possible=Warn only if a fix is available
jvm.inspection.log.guarded.problem.descriptor=Logging call guarded by log condition #loc
jvm.inspection.log.guarded.fix.family.name=Unwrap log guard condition
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

View File

@@ -0,0 +1,171 @@
// 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.codeInspection.AbstractBaseUastLocalInspectionTool
import com.intellij.codeInspection.LocalInspectionToolSession
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.codeInspection.options.OptPane
import com.intellij.codeInspection.registerUProblem
import com.intellij.modcommand.ModPsiUpdater
import com.intellij.modcommand.PsiUpdateModCommandQuickFix
import com.intellij.openapi.project.Project
import com.intellij.profile.codeInspection.InspectionProjectProfileManager
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiElementVisitor
import com.intellij.psi.PsiWhiteSpace
import com.intellij.psi.util.PsiTreeUtil
import com.intellij.uast.UastHintedVisitorAdapter
import org.jetbrains.uast.*
import org.jetbrains.uast.generate.replace
import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor
import org.jetbrains.uast.visitor.AbstractUastVisitor
class LoggingGuardedByConditionInspection : AbstractBaseUastLocalInspectionTool() {
@JvmField
var showOnlyIfFixPossible: Boolean = true
override fun getOptionsPane(): OptPane {
return OptPane.pane(
OptPane.checkbox("showOnlyIfFixPossible",
JvmAnalysisBundle.message("jvm.inspection.log.guarded.warn.if.fix.possible"))
)
}
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor =
UastHintedVisitorAdapter.create(
holder.file.language,
LoggingGuardedByLogConditionVisitor(holder, isOnTheFly),
arrayOf(UIfExpression::class.java),
directOnly = true
)
inner class LoggingGuardedByLogConditionVisitor(
private val holder: ProblemsHolder,
private val isOnTheFly: Boolean,
) : AbstractUastNonRecursiveVisitor() {
override fun visitIfExpression(node: UIfExpression): Boolean {
val condition = node.condition
val visitor = object : AbstractUastVisitor() {
val conditions = mutableListOf<UCallExpression>()
override fun visitCallExpression(node: UCallExpression): Boolean {
val guardedCondition = LoggingUtil.getGuardedCondition(node)
if (guardedCondition == null) {
conditions.add(node)
}
return true
}
}
condition.accept(visitor)
if (visitor.conditions.size != 1) return true
val guardedCondition = visitor.conditions[0]
val uIfExpression = guardedCondition.getParentOfType<UIfExpression>() ?: return true
if (node.sourcePsi != uIfExpression.sourcePsi) return true
val levelFromCondition = LoggingUtil.getLevelFromCondition(guardedCondition) ?: return true
val calls: List<UCallExpression> = LoggingUtil.getLoggerCalls(guardedCondition)
if (calls.isEmpty()) return true
val uCallExpression = calls.find {
val currentLoggerLevel = LoggingUtil.getLoggerLevel(it) ?: return@find false
LoggingUtil.isGuardedIn(levelFromCondition, currentLoggerLevel) &&
guardedCondition.receiver?.sourcePsi?.text == it.receiver?.sourcePsi?.text &&
LoggingUtil.LOG_MATCHERS_WITHOUT_BUILDERS.uCallMatches(it)
}
if (uCallExpression == null) return true
val fixPossible = checkIfFixPossible(guardedCondition, uCallExpression, calls)
if (!fixPossible && showOnlyIfFixPossible) return true
val sourcePsiGuardedCondition = guardedCondition.sourcePsi ?: return true
val fixes = if (fixPossible) arrayOf(UnguardedFix()) else arrayOf()
val isInformationLevel = isOnTheFly && InspectionProjectProfileManager.isInformationLevel(getShortName(), sourcePsiGuardedCondition)
val toHighlight = if (isInformationLevel) node else guardedCondition
val message = JvmAnalysisBundle.message("jvm.inspection.log.guarded.problem.descriptor")
holder.registerUProblem(toHighlight, message, *fixes)
return true
}
private fun checkIfFixPossible(guardedCondition: UExpression, node: UCallExpression, calls: List<UCallExpression>): Boolean {
val uIfExpression = guardedCondition.getParentOfType<UIfExpression>() ?: return false
if (!(uIfExpression.condition.sourcePsi == guardedCondition.sourcePsi ||
(guardedCondition.uastParent is UQualifiedReferenceExpression &&
uIfExpression.condition.sourcePsi == guardedCondition.uastParent?.sourcePsi) ||
(guardedCondition.uastParent is USimpleNameReferenceExpression &&
guardedCondition.uastParent?.uastParent is UQualifiedReferenceExpression &&
uIfExpression.condition.sourcePsi == guardedCondition.uastParent?.uastParent?.sourcePsi)
)
) {
return false
}
val thenExpression = uIfExpression.thenExpression
if (thenExpression?.sourcePsi != node.sourcePsi &&
!(thenExpression is UQualifiedReferenceExpression &&
node.uastParent?.sourcePsi == thenExpression.sourcePsi)) {
if (thenExpression !is UBlockExpression) {
return false
}
val nestedExpressions = thenExpression.expressions
if (nestedExpressions.size != calls.size) {
return false
}
if (nestedExpressions.zip(calls).any { (nested, call) ->
nested.sourcePsi != call.sourcePsi &&
(call.uastParent is UQualifiedReferenceExpression && call.uastParent?.sourcePsi != nested.sourcePsi)
}) {
return false
}
}
return true
}
}
private inner class UnguardedFix : PsiUpdateModCommandQuickFix() {
override fun getFamilyName(): String {
return JvmAnalysisBundle.message("jvm.inspection.log.guarded.fix.family.name")
}
override fun applyFix(project: Project, element: PsiElement, updater: ModPsiUpdater) {
val uIfExpression: UIfExpression = element.toUElement()?.getParentOfType<UIfExpression>(strict = false) ?: return
val thenExpression = uIfExpression.thenExpression
if (thenExpression !is UExpression) return
if (thenExpression !is UBlockExpression) {
uIfExpression.replace(thenExpression)
return
}
val ifStatementSourcePsi = uIfExpression.sourcePsi ?: return
val expressions = thenExpression.expressions
if (expressions.isEmpty()) return
var currentParent = ifStatementSourcePsi.parent
var after = ifStatementSourcePsi
var nextExpression = expressions[0].sourcePsi
val lastExpression = expressions.last().sourcePsi ?: return
while (nextExpression?.parent.toUElement()?.sourcePsi == expressions[0].sourcePsi) {
nextExpression = nextExpression?.parent
}
if (nextExpression == null) return
while (true) {
if (nextExpression == null) break
var newAdded: PsiElement = currentParent.addAfter(nextExpression.copy(), after)
if (nextExpression is PsiWhiteSpace) {
while (newAdded.nextSibling !is PsiWhiteSpace) {
newAdded = newAdded.parent ?: break
}
after = newAdded.nextSibling
}
else {
after = newAdded
}
currentParent = newAdded.parent
if (PsiTreeUtil.isAncestor(nextExpression, lastExpression, false)) break
nextExpression = nextExpression.nextSibling ?: break
}
ifStatementSourcePsi.delete()
}
}
}

View File

@@ -387,12 +387,16 @@ class LoggingUtil {
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>()) {
val guarded = sourcePsi.toUElementOfType<UExpression>()
val qualifier = when (guarded) {
is USimpleNameReferenceExpression -> {
((guarded.uastParent as? UQualifiedReferenceExpression)?.receiver as? UResolvable)?.resolveToUElement()
}
is UQualifiedReferenceExpression -> {
(guarded.receiver as? UResolvable)?.resolveToUElement() as? UVariable
(guarded.receiver as? UResolvable)?.resolveToUElement()
}
is UCallExpression -> {
(guarded.receiver as? UResolvable)?.resolveToUElement() as? UVariable
(guarded.receiver as? UResolvable)?.resolveToUElement()
}
else -> {
null
@@ -401,7 +405,7 @@ class LoggingUtil {
if (qualifier == null) {
return@CachedValueProvider emptyResult
}
val uIfExpression = guardedCondition.getParentOfType<UIfExpression>()
val uIfExpression = guarded?.getParentOfType<UIfExpression>()
if (uIfExpression == null) {
return@CachedValueProvider emptyResult
}

View File

@@ -0,0 +1,20 @@
package com.intellij.jvm.analysis.internal.testFramework.logging
import com.intellij.codeHighlighting.HighlightDisplayLevel
import com.intellij.codeInsight.daemon.HighlightDisplayKey
import com.intellij.codeInspection.logging.LoggingGuardedByConditionInspection
import com.intellij.profile.codeInspection.ProjectInspectionProfileManager.Companion.getInstance
abstract class LoggingGuardedByConditionInspectionTestBase : LoggingInspectionTestBase() {
override val inspection = LoggingGuardedByConditionInspection()
override fun setUp() {
super.setUp()
inspection.run {
val displayKey = HighlightDisplayKey.find(this.getShortName())
val currentProfile = getInstance(project).currentProfile
currentProfile.setErrorLevel(displayKey!!, HighlightDisplayLevel.WARNING, project)
inspection.showOnlyIfFixPossible = getTestName(true).endsWith("showOnlyIfFixPossible")
}
}
}

View File

@@ -0,0 +1,191 @@
package com.intellij.codeInspection.tests.java.logging
import com.intellij.analysis.JvmAnalysisBundle
import com.intellij.jvm.analysis.internal.testFramework.logging.LoggingGuardedByConditionInspectionTestBase
import com.intellij.jvm.analysis.testFramework.JvmLanguage
class JavaLoggingGuardedByConditionInspectionTest : LoggingGuardedByConditionInspectionTestBase() {
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) {
if(<warning descr="Logging call guarded by log condition">LOG.isDebugEnabled()</warning>) {
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) {
if(<warning descr="Logging call guarded by log condition">LOG.isDebugEnabled()</warning>) {
LOG.debug("test1" + arg);
}
}
}
""".trimIndent())
}
fun `test several calls 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(<warning descr="Logging call guarded by log condition">LOG.isDebugEnabled()</warning>) {
LOG.debug("test1" + arg);
LOG.debug("test2" + arg);
}
}
}
""".trimIndent())
}
fun `test several different calls 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(<warning descr="Logging call guarded by log condition">LOG.isDebugEnabled()</warning>) {
LOG.info("test1" + arg);
LOG.debug("test2" + arg);
}
}
}
""".trimIndent())
}
fun `test several calls with another call 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(<warning descr="Logging call guarded by log condition">LOG.isDebugEnabled()</warning>) {
LOG.debug("test1" + arg);
arg.toString();
}
}
}
""".trimIndent())
}
fun `test several calls with another call slf4j showOnlyIfFixPossible`() {
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("test1" + arg);
arg.toString();
}
}
}
""".trimIndent())
}
fun `test slf4j fix`() {
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) {
if(<caret>LOG.isDebugEnabled()) {
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) {
LOG.debug("test" + arg);
}
}
""".trimIndent(),
hint = JvmAnalysisBundle.message("jvm.inspection.log.guarded.fix.family.name")
)
}
fun `test slf4j without block fix`() {
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) {
if(<caret>LOG.isDebugEnabled())
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) {
LOG.debug("test" + arg);
}
}
""".trimIndent(),
hint = JvmAnalysisBundle.message("jvm.inspection.log.guarded.fix.family.name")
)
}
fun `test several calls slf4j fix`() {
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) {
if(<caret>LOG.isDebugEnabled()) {
LOG.debug("test1" + arg);
LOG.debug("test2" + 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) {
LOG.debug("test1" + arg);
LOG.debug("test2" + arg);
}
}
""".trimIndent(),
hint = JvmAnalysisBundle.message("jvm.inspection.log.guarded.fix.family.name")
)
}
}

View File

@@ -0,0 +1,241 @@
package com.intellij.codeInspection.tests.kotlin.logging
import com.intellij.analysis.JvmAnalysisBundle
import com.intellij.jvm.analysis.internal.testFramework.logging.LoggingGuardedByConditionInspectionTestBase
import com.intellij.jvm.analysis.testFramework.JvmLanguage
class KotlinLoggingGuardedByConditionInspectionTest : LoggingGuardedByConditionInspectionTestBase() {
fun `test slf4j`() {
myFixture.testHighlighting(JvmLanguage.KOTLIN, """
import org.slf4j.Logger
import org.slf4j.LoggerFactory
internal class X {
fun n(arg: String) {
if (LOG.<warning descr="Logging call guarded by log condition">isDebugEnabled</warning>) {
LOG.debug("test" + arg)
}
}
companion object {
private val LOG: Logger = LoggerFactory.getLogger()
}
}
""".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) {
if (LOG.<warning descr="Logging call guarded by log condition">isDebugEnabled()</warning>) {
LOG.debug("test1" + arg)
}
}
companion object {
val LOG: Logger = LogManager.getLogger()
}
}
""".trimIndent())
}
fun `test several calls slf4j`() {
myFixture.testHighlighting(JvmLanguage.KOTLIN, """
import org.slf4j.Logger
import org.slf4j.LoggerFactory
internal class X {
fun n(arg: String) {
if (LOG.<warning descr="Logging call guarded by log condition">isDebugEnabled</warning>) {
LOG.debug("test1" + arg)
LOG.debug("test2" + arg)
}
}
companion object {
private val LOG: Logger = LoggerFactory.getLogger()
}
}
""".trimIndent())
}
fun `test several different calls slf4j`() {
myFixture.testHighlighting(JvmLanguage.KOTLIN, """
import org.slf4j.Logger
import org.slf4j.LoggerFactory
internal class X {
fun n(arg: String) {
if (LOG.<warning descr="Logging call guarded by log condition">isDebugEnabled</warning>) {
LOG.info("test1" + arg)
LOG.debug("test2" + arg)
}
}
companion object {
private val LOG: Logger = LoggerFactory.getLogger()
}
}
""".trimIndent())
}
fun `test several calls with another call slf4j`() {
myFixture.testHighlighting(JvmLanguage.KOTLIN, """
import org.slf4j.Logger
import org.slf4j.LoggerFactory
internal class X {
fun n(arg: String) {
if (LOG.<warning descr="Logging call guarded by log condition">isDebugEnabled</warning>) {
LOG.debug("test1" + arg)
arg.toString()
}
}
companion object {
private val LOG: Logger = LoggerFactory.getLogger()
}
}
""".trimIndent())
}
fun `test several calls with another call slf4j showOnlyIfFixPossible`() {
myFixture.testHighlighting(JvmLanguage.KOTLIN, """
import org.slf4j.Logger
import org.slf4j.LoggerFactory
internal class X {
fun n(arg: String) {
if (LOG.isDebugEnabled) {
LOG.debug("test1" + arg)
arg.toString()
}
}
companion object {
private val LOG: Logger = LoggerFactory.getLogger()
}
}
""".trimIndent())
}
fun `test slf4j fix`() {
myFixture.testQuickFix(
testPreview = true,
lang = JvmLanguage.KOTLIN,
before = """
import org.slf4j.Logger
import org.slf4j.LoggerFactory
internal class X {
fun n(arg: String) {
if (LOG.<caret>isDebugEnabled) {
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) {
LOG.debug("test" + arg)
}
companion object {
private val LOG: Logger = LoggerFactory.getLogger()
}
}
""".trimIndent(),
hint = JvmAnalysisBundle.message("jvm.inspection.log.guarded.fix.family.name")
)
}
fun `test slf4j without block fix`() {
myFixture.testQuickFix(
testPreview = true,
lang = JvmLanguage.KOTLIN,
before = """
import org.slf4j.Logger
import org.slf4j.LoggerFactory
internal class X {
fun n(arg: String) {
if (LOG.<caret>isDebugEnabled)
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) {
LOG.debug("test" + arg)
}
companion object {
private val LOG: Logger = LoggerFactory.getLogger()
}
}""".trimIndent(),
hint = JvmAnalysisBundle.message("jvm.inspection.log.guarded.fix.family.name")
)
}
fun `test several calls slf4j fix`() {
myFixture.testQuickFix(
testPreview = true,
lang = JvmLanguage.KOTLIN,
before = """
import org.slf4j.Logger
import org.slf4j.LoggerFactory
internal class X {
fun n(arg: String) {
if (LOG.<caret>isDebugEnabled) {
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) {
LOG.debug("test1" + arg)
LOG.debug("test2" + arg)
}
companion object {
private val LOG: Logger = LoggerFactory.getLogger()
}
}
""".trimIndent(),
hint = JvmAnalysisBundle.message("jvm.inspection.log.guarded.fix.family.name")
)
}
}