[java-inspection] IJ-CR-128589 IDEA-349655 Improvements for logging inspections. new LogStatementNotGuardedByLogCondition

- delete custom settings

GitOrigin-RevId: dc3625006922b3db74c6f478a3b1ac8bf5e5ddad
This commit is contained in:
Mikhail Pyltsin
2024-03-22 13:24:15 +01:00
committed by intellij-monorepo-bot
parent 96408c28cf
commit 59f94d7232
6 changed files with 10 additions and 354 deletions

View File

@@ -9,11 +9,6 @@
beanClass="com.intellij.lang.LanguageExtensionPoint">
<with attribute="implementationClass" implements="com.intellij.codeInspection.sourceToSink.SourceToSinkProvider"/>
</extensionPoint>
<extensionPoint qualifiedName="com.intellij.codeInspection.customLoggingStatementNotGuardedByLogFix"
dynamic="true"
beanClass="com.intellij.lang.LanguageExtensionPoint">
<with attribute="implementationClass" implements="com.intellij.codeInspection.logging.LoggingStatementNotGuardedByLogCustomFix"/>
</extensionPoint>
</extensionPoints>
<extensions defaultExtensionNs="com.intellij">
<!--Test frameworks-->
@@ -207,9 +202,6 @@
<extensions defaultExtensionNs="com.intellij.codeInspection">
<sourceToSinkProvider language="JAVA" implementationClass="com.intellij.codeInspection.sourceToSink.JavaSourceToSinkProvider"/>
</extensions>
<extensions defaultExtensionNs="com.intellij.codeInspection">
<customLoggingStatementNotGuardedByLogFix language="JAVA" implementationClass="com.intellij.codeInspection.logging.JavaLoggingStatementNotGuardedByLogCustomFix"/>
</extensions>
<actions>
<group id="UastInternal" text="UAST" internal="true" popup="true">
<action id="DumpUastLog" internal="true" class="com.intellij.analysis.internal.DumpUastTreeAction" text="Dump UAST Tree"/>

View File

@@ -173,11 +173,6 @@ jvm.inspection.log.statement.not.guarded.debug.level.and.lower.option=debug leve
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

View File

@@ -2,8 +2,6 @@
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
@@ -20,7 +18,6 @@ import com.intellij.psi.PsiTypes
import com.intellij.psi.impl.LanguageConstantExpressionEvaluator
import com.intellij.psi.util.PsiTreeUtil
import com.intellij.uast.UastHintedVisitorAdapter
import com.siyeh.ig.psiutils.JavaLoggingUtils
import org.jetbrains.uast.*
import org.jetbrains.uast.generate.getUastElementFactory
import org.jetbrains.uast.generate.replace
@@ -28,22 +25,6 @@ import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor
class LoggingStatementNotGuardedByLogConditionInspection : AbstractBaseUastLocalInspectionTool() {
//for backward compatibility
@JvmField
var customLogMethodNameList: MutableList<String?> = mutableListOf("fine", "finer", "finest")
//for backward compatibility
@JvmField
var customLogConditionMethodNameList: MutableList<String?> = 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
@@ -68,15 +49,7 @@ class LoggingStatementNotGuardedByLogConditionInspection : AbstractBaseUastLocal
),
OptPane.checkbox("flagUnguardedConstant", JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.unguarded.constant.option"))
.description(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())
.description(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")))
.description(JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.custom.table")),
)
)
}
@@ -94,32 +67,16 @@ class LoggingStatementNotGuardedByLogConditionInspection : AbstractBaseUastLocal
) : AbstractUastNonRecursiveVisitor() {
override fun visitCallExpression(node: UCallExpression): Boolean {
val sourcePsi = node.sourcePsi ?: return true
val isCustom = if (LoggingUtil.LOG_MATCHERS_WITHOUT_BUILDERS.uCallMatches(node)) {
false
}
else if (node.isMethodNameOneOf(customLogMethodNameList.filterNotNull())) {
val uMethod = node.resolveToUElementOfType<UMethod>() ?: return true
if (uMethod.getContainingUClass()?.qualifiedName != customLoggerClassName) {
return true
}
true
}
else {
return true
}
val fix = LoggingStatementNotGuardedByLogCustomFix.fixProvider.forLanguage(sourcePsi.getLanguage())
if (isCustom && (fix == null || !fix.isAvailable(sourcePsi))) {
if (!LoggingUtil.LOG_MATCHERS_WITHOUT_BUILDERS.uCallMatches(node)) {
return true
}
val isInformationLevel = isOnTheFly && InspectionProjectProfileManager.isInformationLevel(getShortName(), sourcePsi)
if (!isInformationLevel && !isCustom && LoggingUtil.skipAccordingLevel(node, myLimitLevelType)) {
if (!isInformationLevel && LoggingUtil.skipAccordingLevel(node, myLimitLevelType)) {
return true
}
if (isSurroundedByLogGuard(node, isCustom)) {
if (isSurroundedByLogGuard(node)) {
return true
}
@@ -135,7 +92,7 @@ class LoggingStatementNotGuardedByLogConditionInspection : AbstractBaseUastLocal
val beforeCall = before.getUCallExpression(2)
val beforeLoggerLevel = LoggingUtil.getLoggerLevel(beforeCall)
if (beforeCall != null &&
(LoggingUtil.LOG_MATCHERS_WITHOUT_BUILDERS.uCallMatches(beforeCall) || beforeCall.isMethodNameOneOf(customLogMethodNameList.filterNotNull()))
LoggingUtil.LOG_MATCHERS_WITHOUT_BUILDERS.uCallMatches(beforeCall)
) {
val receiverText = node.receiver?.sourcePsi?.text
val loggerLevel = LoggingUtil.getLoggerLevel(node)
@@ -156,22 +113,13 @@ class LoggingStatementNotGuardedByLogConditionInspection : AbstractBaseUastLocal
return true
}
val textCustomCondition = if (isCustom) {
val indexOfMethod = customLogMethodNameList.indexOf(node.methodName)
if (indexOfMethod == -1) return false
customLogConditionMethodNameList[indexOfMethod]
}
else {
null
}
val message = JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.problem.descriptor")
if (nodeParent is UQualifiedReferenceExpression) {
holder.registerUProblem(nodeParent as UExpression, message, CreateGuardFix(textCustomCondition))
holder.registerUProblem(nodeParent as UExpression, message, CreateGuardFix())
}
else {
holder.registerUProblem(node, message, CreateGuardFix(textCustomCondition))
holder.registerUProblem(node, message, CreateGuardFix())
}
return true
}
@@ -215,22 +163,15 @@ class LoggingStatementNotGuardedByLogConditionInspection : AbstractBaseUastLocal
return false
}
private fun isSurroundedByLogGuard(callExpression: UCallExpression, isCustom: Boolean): Boolean {
private fun isSurroundedByLogGuard(callExpression: UCallExpression): 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() {
private inner class CreateGuardFix : PsiUpdateModCommandQuickFix() {
override fun getFamilyName(): String {
return JvmAnalysisBundle.message("jvm.inspection.log.statement.not.guarded.log.fix.family.name")
@@ -239,12 +180,6 @@ class LoggingStatementNotGuardedByLogConditionInspection : AbstractBaseUastLocal
override fun applyFix(project: Project, element: PsiElement, updater: ModPsiUpdater) {
val uCallExpression: UCallExpression = element.toUElement().getUCallExpression(2) ?: return
if (textCustomCondition != null) {
val fix = LoggingStatementNotGuardedByLogCustomFix.fixProvider.forLanguage(element.getLanguage()) ?: return
fix.fix(uCallExpression, textCustomCondition)
return
}
val qualifiedExpression = if (uCallExpression.uastParent is UQualifiedReferenceExpression) uCallExpression.uastParent else uCallExpression
if (qualifiedExpression !is UExpression) return
@@ -266,7 +201,7 @@ class LoggingStatementNotGuardedByLogConditionInspection : AbstractBaseUastLocal
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())) &&
(LoggingUtil.LOG_MATCHERS_WITHOUT_BUILDERS.uCallMatches(afterCall)) &&
receiverText == after.receiver.sourcePsi?.text &&
uCallExpression.methodName == afterCall.methodName &&
afterLevel == loggerLevel) {

View File

@@ -3,8 +3,6 @@ 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() {
@@ -15,16 +13,7 @@ class LoggingStatementNotGuardedByLogConditionInspectionMerger : InspectionEleme
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<String?>()
val condition = mutableListOf<String?>()
BaseInspection.parseString(conditionMethodNames, calls, condition)
inspection.customLogMethodNameList = calls
inspection.customLogConditionMethodNameList = condition
}
inspection.writeSettings(toolElement)
return toolElement
}

View File

@@ -1,106 +0,0 @@
// 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.lang.LanguageExtension
import com.intellij.lang.java.JavaLanguage
import com.intellij.openapi.extensions.ExtensionPointName
import com.intellij.openapi.extensions.ExtensionPointName.Companion.create
import com.intellij.psi.*
import com.intellij.psi.codeStyle.JavaCodeStyleManager
import com.intellij.psi.util.PsiTreeUtil
import com.siyeh.ig.psiutils.CommentTracker
import org.jetbrains.uast.UCallExpression
private val EP_NAME: ExtensionPointName<LoggingStatementNotGuardedByLogCustomFix> = create("com.intellij.codeInspection.customLoggingStatementNotGuardedByLogFix")
/**
* Provides custom fix for LoggingStatementNotGuardedByLogConditionInspection
*/
interface LoggingStatementNotGuardedByLogCustomFix {
companion object {
@JvmField
val fixProvider = LanguageExtension<LoggingStatementNotGuardedByLogCustomFix>(EP_NAME.name)
}
fun fix(call: UCallExpression, textCustomCondition: String)
fun isAvailable(element: PsiElement): Boolean
}
class JavaLoggingStatementNotGuardedByLogCustomFix : LoggingStatementNotGuardedByLogCustomFix {
override fun fix(call: UCallExpression, textCustomCondition: String) {
val methodCallExpression = call.javaPsi
if (methodCallExpression !is PsiMethodCallExpression) {
return
}
val statement = PsiTreeUtil.getParentOfType(methodCallExpression, PsiStatement::class.java) ?: return
val logStatements = mutableListOf(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
}
override fun isAvailable(element: PsiElement): Boolean {
return element.language == JavaLanguage.INSTANCE
}
}

View File

@@ -44,18 +44,6 @@ class JavaLoggingStatementNotGuardedByLogConditionInspectionTest : LoggingStatem
""".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) {
<warning descr="Logging call not guarded by log condition">LOG.fine("test" + arg)</warning>;
}
}
""".trimIndent())
}
fun `test skip according level for slf4j`() {
myFixture.testHighlighting(JvmLanguage.JAVA, """
import org.slf4j.Logger;
@@ -199,19 +187,6 @@ class JavaLoggingStatementNotGuardedByLogConditionInspectionTest : LoggingStatem
""".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) {
<warning descr="Logging call not guarded by log condition">LOG.fine("test" + arg)</warning>;
LOG.fine("test" + arg);
}
}
""".trimIndent())
}
fun `test lambda`() {
myFixture.testHighlighting(JvmLanguage.JAVA, """
import org.apache.logging.log4j.*;
@@ -288,66 +263,6 @@ class JavaLoggingStatementNotGuardedByLogConditionInspectionTest : LoggingStatem
)
}
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.<caret>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.<caret>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,
@@ -382,38 +297,6 @@ class JavaLoggingStatementNotGuardedByLogConditionInspectionTest : LoggingStatem
)
}
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<caret>.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,
@@ -447,36 +330,4 @@ class JavaLoggingStatementNotGuardedByLogConditionInspectionTest : LoggingStatem
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<caret>.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")
)
}
}