[uast-inspection] IDEA-331308 Create inspection highlighting non-distinguishable logging sites in a single class

- skip error log levels with stacktrace

GitOrigin-RevId: 5662e1f1a5e43fdb371d6597693c0ab875040204
This commit is contained in:
Mikhail Pyltsin
2024-01-17 13:46:03 +00:00
committed by intellij-monorepo-bot
parent 06f55eb8cd
commit dcc38e5eb7
9 changed files with 118 additions and 20 deletions
@@ -15,8 +15,14 @@ These calls can be non-distinguishable from each other, and this introduces diff
}
</code></pre>
<!-- tooltip end -->
<ul>
<li>
Use the <b>Do not report calls with `error` log level</b> option to ignore messages with `error` log level and when there is an exception.
It can be useful, because places of calls can be found with stacktraces
</li>
</ul>
<p><small>New in 2024.1</small></p>
</body>
</html>
@@ -175,6 +175,7 @@ jvm.inspection.logging.placeholder.count.matches.argument.count.slf4j.throwable.
jvm.inspection.logging.similar.message.display.name=Non-distinguishable logging calls
jvm.inspection.logging.similar.message.problem.descriptor=Similar log messages
jvm.inspection.logging.similar.message.problem.skip.on.error=Do not report calls with `error` log level
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}''
@@ -296,16 +296,6 @@ class LoggingPlaceholderCountMatchesArgumentCountInspection : AbstractBaseUastLo
return throwable.isConvertibleFrom(functionalReturnType)
}
private fun hasThrowableType(lastArgument: UExpression): Boolean {
val type = lastArgument.getExpressionType()
if (type is UastErrorType) {
return false
}
if (type is PsiDisjunctionType) {
return type.disjunctions.all { InheritanceUtil.isInheritor(it, CommonClassNames.JAVA_LANG_THROWABLE) }
}
return InheritanceUtil.isInheritor(type, CommonClassNames.JAVA_LANG_THROWABLE)
}
}
private enum class ResultType {
@@ -322,6 +312,17 @@ class LoggingPlaceholderCountMatchesArgumentCountInspection : AbstractBaseUastLo
private data class Result(val argumentCount: Int, val placeholderCount: Int, val result: ResultType)
}
internal fun hasThrowableType(lastArgument: UExpression): Boolean {
val type = lastArgument.getExpressionType()
if (type is UastErrorType) {
return false
}
if (type is PsiDisjunctionType) {
return type.disjunctions.all { InheritanceUtil.isInheritor(it, CommonClassNames.JAVA_LANG_THROWABLE) }
}
return InheritanceUtil.isInheritor(type, CommonClassNames.JAVA_LANG_THROWABLE)
}
private fun UCallableReferenceExpression.getMethodReferenceReturnType(): PsiType? {
val method = this.resolveToUElement() as? UMethod ?: return null
if (method.isConstructor) {
@@ -33,7 +33,7 @@ private val SLF4J_HOLDER = object : LoggerTypeSearcher {
}
}
private val LOG4J_LOG_BUILDER_HOLDER = object : LoggerTypeSearcher {
internal val LOG4J_LOG_BUILDER_HOLDER = object : LoggerTypeSearcher {
override fun findType(expression: UCallExpression, context: LoggerContext): PlaceholderLoggerType? {
var qualifierExpression = getImmediateLoggerQualifier(expression)
if (qualifierExpression is UReferenceExpression) {
@@ -109,7 +109,7 @@ private val AKKA_PLACEHOLDERS = object : LoggerTypeSearcher {
}
}
private val IDEA_PLACEHOLDERS = object : LoggerTypeSearcher {
internal val IDEA_PLACEHOLDERS = object : LoggerTypeSearcher {
override fun findType(expression: UCallExpression, context: LoggerContext): PlaceholderLoggerType? {
return null
}
@@ -6,6 +6,7 @@ import com.intellij.codeInspection.AbstractBaseUastLocalInspectionTool
import com.intellij.codeInspection.LocalInspectionToolSession
import com.intellij.codeInspection.ProblemDescriptor
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.codeInspection.options.OptPane
import com.intellij.java.JavaBundle
import com.intellij.java.library.JavaLibraryUtil
import com.intellij.modcommand.ModCommand
@@ -24,8 +25,21 @@ import org.jetbrains.uast.visitor.AbstractUastVisitor
private const val MIN_TEXT_LENGTH = 3
private const val MAX_PART_COUNT = 10
private const val WITH_THROWABLE = "withThrowable"
private const val SET_CAUSE = "setCause"
class LoggingSimilarMessageInspection : AbstractBaseUastLocalInspectionTool() {
@JvmField
var mySkipErrorLogLevel: Boolean = true
override fun getOptionsPane(): OptPane {
return OptPane.pane(
OptPane.checkbox("mySkipErrorLogLevel",
JvmAnalysisBundle.message("jvm.inspection.logging.similar.message.problem.skip.on.error"))
)
}
//otherwise results will be inconsistent
override fun runForWholeFile(): Boolean {
return true
@@ -126,7 +140,17 @@ class LoggingSimilarMessageInspection : AbstractBaseUastLocalInspectionTool() {
val result = mutableSetOf<UCallExpression>()
file.accept(object : AbstractUastVisitor() {
override fun visitCallExpression(node: UCallExpression): Boolean {
LOGGER_TYPE_SEARCHERS.mapFirst(node) ?: return false
val loggerTypeSearcher = LOGGER_TYPE_SEARCHERS.mapFirst(node) ?: return false
if (mySkipErrorLogLevel) {
val hasSetMessage = hasSetThrowable(node, loggerTypeSearcher)
if (hasSetMessage) return false
if (LoggingUtil.getLoggerLevel(node) == LoggingUtil.Companion.LevelType.ERROR) {
if (loggerTypeSearcher == IDEA_PLACEHOLDERS) return false
val valueArguments = node.valueArguments
if (loggerTypeSearcher != SLF4J_BUILDER_HOLDER && loggerTypeSearcher != LOG4J_LOG_BUILDER_HOLDER &&
!valueArguments.isEmpty() && hasThrowableType(valueArguments.last())) return false
}
}
result.add(node)
return true
}
@@ -135,6 +159,32 @@ class LoggingSimilarMessageInspection : AbstractBaseUastLocalInspectionTool() {
}
}
private fun hasSetThrowable(node: UCallExpression,
loggerType: LoggerTypeSearcher?): Boolean {
if (loggerType == null) {
return false
}
if (!(loggerType == SLF4J_BUILDER_HOLDER || loggerType == LOG4J_LOG_BUILDER_HOLDER)) {
return false
}
var currentCall = node.receiver
for (ignore in 0..MAX_BUILDER_LENGTH) {
if (currentCall is UQualifiedReferenceExpression) {
currentCall = currentCall.selector
continue
}
if (currentCall !is UCallExpression) {
return false
}
val methodName = currentCall.methodName ?: return false
if (methodName == WITH_THROWABLE || methodName == SET_CAUSE) {
return true
}
currentCall = currentCall.receiver
}
return false
}
private fun collectParts(node: UCallExpression, searcher: LoggerTypeSearcher?): List<LoggingStringPartEvaluator.PartHolder>? {
if (searcher == null) return null
val arguments = node.valueArguments
@@ -262,13 +262,13 @@ internal class LoggingUtil {
return findLevelTypeByName(methodName, LEGACY_LEVEL_MAP)
}
internal fun getLoggerLevel(uCall: UCallExpression?): LevelType? {
internal fun getLoggerLevel(uCall: UCallExpression?, isLog: Boolean = false): LevelType? {
if (uCall == null) {
return null
}
var levelName = uCall.methodName
if ("log" == levelName) {
if (isLog || "log" == levelName) {
val levelTypeFromLog = findLevelTypeByFirstArgument(uCall, LEVEL_CLASSES, LEVEL_MAP)
if (levelTypeFromLog != null) {
return levelTypeFromLog
@@ -288,8 +288,9 @@ internal class LoggingUtil {
return levelTypeFromAtLevel
}
}
val loggerLevel = getLoggerLevel(nextCall, true)
if (loggerLevel != null) return loggerLevel
levelName = nextCall?.methodName
}
if (levelName == null) {
return null
@@ -127,6 +127,7 @@ public interface Logger {
public interface LoggingEventBuilder {
LoggingEventBuilder addArgument(Object object);
LoggingEventBuilder setMessage(String message);
LoggingEventBuilder setCause(Throwable cause);
LoggingEventBuilder addKeyValue(String key, Object object);
void log(String format, Object... arguments);
void log();
@@ -16,11 +16,15 @@ class JavaLoggingSimilarMessageInspectionTest : LoggingSimilarMessageInspectionT
private static void request1(String i) {
String msg = "log messages: " + i;
<weak_warning descr="Similar log messages">L<caret>OG.info(msg)</weak_warning>;
<weak_warning descr="Similar log messages">LOG.error(msg)</weak_warning>;
LOG.error(msg, new RuntimeException());
}
private static void request2(int i) {
String msg = "log messages: " + i;
<weak_warning descr="Similar log messages">LOG.info(msg)</weak_warning>;
<weak_warning descr="Similar log messages">LOG.error(msg)</weak_warning>;
LOG.error(msg, new RuntimeException());
}
}
""".trimIndent())
@@ -35,11 +39,15 @@ class JavaLoggingSimilarMessageInspectionTest : LoggingSimilarMessageInspectionT
private static void request1(String i) {
String msg = "log messages: " + i;
LOG.info(msg);
LOG.error(msg);
LOG.error(msg, new RuntimeException());
}
private static void request2(int i) {
String msg = "log messages: " + i;
<selection><caret>LOG.info(msg)</selection>;
LOG.error(msg);
LOG.error(msg, new RuntimeException());
}
}
""".trimIndent())
@@ -151,11 +159,15 @@ class JavaLoggingSimilarMessageInspectionTest : LoggingSimilarMessageInspectionT
private static void request1(String i) {
String msg = "log messages: " + i;
<weak_warning descr="Similar log messages">LOG.info(msg)</weak_warning>;
<weak_warning descr="Similar log messages">LOG.error(msg)</weak_warning>;
LOG.error(msg, new RuntimeException());
}
private static void request2(int i) {
String msg = "log messages: " + i;
<weak_warning descr="Similar log messages">LOG.info(msg)</weak_warning>;
<weak_warning descr="Similar log messages">LOG.error(msg)</weak_warning>;
LOG.error(msg, new RuntimeException());
}
}
""".trimIndent())
@@ -170,12 +182,14 @@ class JavaLoggingSimilarMessageInspectionTest : LoggingSimilarMessageInspectionT
private static void request1(String i) {
String msg = "log messages: " + i;
<weak_warning descr="Similar log messages">LOG.info(msg)</weak_warning>;
<weak_warning descr="Similar log messages">LOG.atInfo().setMessage(msg).log()</weak_warning>;
LOG.atInfo().setCause(new RuntimeException("1234")).setMessage(msg).log();
}
private static void request2(int i) {
String msg = "log messages: " + i;
<weak_warning descr="Similar log messages">LOG.info(msg)</weak_warning>;
<weak_warning descr="Similar log messages">LOG.atInfo().setMessage(msg).log()</weak_warning>;
LOG.atInfo().setCause(new RuntimeException("1234")).setMessage(msg).log();
}
}
""".trimIndent())
@@ -529,11 +543,13 @@ class JavaLoggingSimilarMessageInspectionTest : LoggingSimilarMessageInspectionT
private static void request1(String i) {
String msg = "log messages: " + i;
<weak_warning descr="Similar log messages">L<caret>OG.info(msg)</weak_warning>;
LOG.error(msg);
}
private static void request2(int i) {
String msg = "log messages: " + i;
<weak_warning descr="Similar log messages">LOG.info(msg)</weak_warning>;
<weak_warning descr="Similar log messages">L<caret>OG.info(msg)</weak_warning>;
LOG.error(msg);
}
}
""".trimIndent())
@@ -47,5 +47,27 @@ class KotlinLoggingSimilarMessageInspectionTest : LoggingSimilarMessageInspectio
}
""".trimIndent())
}
fun `test setMessage slf4j`() {
myFixture.testHighlighting(JvmLanguage.KOTLIN, """
import org.slf4j.Logger
import org.slf4j.LoggerFactory
internal class Logging {
private val LOG: Logger = LoggerFactory.getLogger(Logging::class.<error descr="[UNRESOLVED_REFERENCE] Unresolved reference: java">java</error>)
private fun request1(i: String) {
val msg = "log messages: {}" + i
LOG.atInfo().setCause(RuntimeException()).setMessage(msg).log()
LOG.atInfo().setMessage(msg).<weak_warning descr="Similar log messages">log()</weak_warning>
}
private fun request2(i: Int) {
val msg = "log messages: {}" + i
LOG.atInfo().setCause(RuntimeException()).setMessage(msg).log()
LOG.atInfo().setMessage(msg).<weak_warning descr="Similar log messages">log()</weak_warning>
}
}
""".trimIndent())
}
}