[uast-inspection] IDEA-343298 LoggingSimilarMessageInspection should skip empty messages

- skip messages without certain beginnings and ends

GitOrigin-RevId: eb9ef8be4c8629712a0915a539fc7e9dac4c1238

jvm/jvm-analysis-impl/resources/inspectionDescriptions/LoggingSimilarMessage.html

@@ -17,6 +17,9 @@ These calls can be non-distinguishable from each other, and this introduces diff
 
 <!-- tooltip end -->
 <ul>
+  <li>
+    Use the <b>Minimal length of a similar sequence</b> option to set minimal length of similar sequences after which calls will be reported
+  </li>
   <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

jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties

@@ -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.min.similar.length=Minimal length of a similar sequence
 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/jvm-analysis-impl/src/com/intellij/codeInspection/logging/LoggingSimilarMessageInspection.kt

@@ -22,7 +22,6 @@ import org.jetbrains.uast.*
 import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor
 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"
@@ -33,8 +32,14 @@ class LoggingSimilarMessageInspection : AbstractBaseUastLocalInspectionTool() {
   @JvmField
   var mySkipErrorLogLevel: Boolean = true
 
+  @JvmField
+  var myMinTextLength: Int = 5
+
   override fun getOptionsPane(): OptPane {
     return OptPane.pane(
+      OptPane.number("myMinTextLength",
+                     JvmAnalysisBundle.message("jvm.inspection.logging.similar.message.problem.min.similar.length"),
+                     3, 100),
       OptPane.checkbox("mySkipErrorLogLevel",
                        JvmAnalysisBundle.message("jvm.inspection.logging.similar.message.problem.skip.on.error"))
     )
@@ -55,12 +60,13 @@ class LoggingSimilarMessageInspection : AbstractBaseUastLocalInspectionTool() {
       return PsiElementVisitor.EMPTY_VISITOR
     }
 
-    return UastHintedVisitorAdapter.create(holder.file.language, PlaceholderCountMatchesArgumentCountVisitor(holder),
+    return UastHintedVisitorAdapter.create(holder.file.language, PlaceholderCountMatchesArgumentCountVisitor(holder, myMinTextLength),
                                            arrayOf(UFile::class.java), directOnly = true)
   }
 
   inner class PlaceholderCountMatchesArgumentCountVisitor(
     private val holder: ProblemsHolder,
+    private val myMinTextLength: Int,
   ) : AbstractUastNonRecursiveVisitor() {
 
     override fun visitFile(node: UFile): Boolean {
@@ -81,17 +87,17 @@ class LoggingSimilarMessageInspection : AbstractBaseUastLocalInspectionTool() {
         if (group.size > 5) {
           var firstIsTaken = true
           var minLength: Int? = group
-                            .mapNotNull { messageLog -> messageLog.parts }
-                            .filter { parts -> firstIsText(parts) }
-                            .minOfOrNull { parts -> parts[0].text?.length ?: 0 }
+            .mapNotNull { messageLog -> messageLog.parts }
+            .filter { parts -> firstIsText(parts) }
+            .minOfOrNull { parts -> parts[0].text?.length ?: 0 }
 
 
           if (minLength == null || minLength == 0) {
             firstIsTaken = false
             minLength = group
-                          .mapNotNull { messageLog -> messageLog.parts }
-                          .filter { parts -> lastIsText(parts) }
-                          .minOfOrNull { parts -> parts.last().text?.length ?: 0 }
+              .mapNotNull { messageLog -> messageLog.parts }
+              .filter { parts -> lastIsText(parts) }
+              .minOfOrNull { parts -> parts.last().text?.length ?: 0 }
           }
 
           if (minLength == null || minLength == 0) {
@@ -102,7 +108,7 @@ class LoggingSimilarMessageInspection : AbstractBaseUastLocalInspectionTool() {
             .groupBy {
               val parts = it.parts ?: return@groupBy ""
               if (parts.isEmpty()) return@groupBy ""
-              val part0 = if(firstIsTaken) parts[0] else parts.last()
+              val part0 = if (firstIsTaken) parts[0] else parts.last()
               if (!part0.isConstant || part0.text == null || part0.text.length < minLength) return@groupBy ""
               part0.text.substring(0, minLength)
             }
@@ -114,7 +120,7 @@ class LoggingSimilarMessageInspection : AbstractBaseUastLocalInspectionTool() {
           val alreadyHasWarning = mutableSetOf<Int>()
           for (firstIndex in 0..currentGroup.lastIndex) {
             for (secondIndex in firstIndex + 1..currentGroup.lastIndex) {
-              if (similar(currentGroup[firstIndex].parts, currentGroup[secondIndex].parts)) {
+              if (similar(currentGroup[firstIndex].parts, currentGroup[secondIndex].parts, myMinTextLength)) {
                 if (alreadyHasWarning.add(firstIndex)) {
                   registerProblem(holder, currentGroup[firstIndex].call, currentGroup[secondIndex].call)
                 }
@@ -130,12 +136,6 @@ class LoggingSimilarMessageInspection : AbstractBaseUastLocalInspectionTool() {
       return super.visitFile(node)
     }
 
-    private fun firstIsText(parts: List<LoggingStringPartEvaluator.PartHolder>) =
-      parts.isNotEmpty() && parts[0].isConstant && parts[0].text?.isNotBlank() == true
-
-    private fun lastIsText(parts: List<LoggingStringPartEvaluator.PartHolder>) =
-      parts.isNotEmpty() && parts.last().isConstant && parts.last().text?.isNotBlank() == true
-
     private fun collectCalls(file: UFile): Set<UCallExpression> {
       val result = mutableSetOf<UCallExpression>()
       file.accept(object : AbstractUastVisitor() {
@@ -301,13 +301,24 @@ private class PartHolderIterator(private val parts: List<LoggingStringPartEvalua
 private data class MessageLog(val call: UCallExpression, val parts: List<LoggingStringPartEvaluator.PartHolder>?)
 
 private fun similar(first: List<LoggingStringPartEvaluator.PartHolder>?,
-                    second: List<LoggingStringPartEvaluator.PartHolder>?): Boolean {
+                    second: List<LoggingStringPartEvaluator.PartHolder>?,
+                    minTextLength: Int): Boolean {
   if (first == null || second == null) return false
   if (first.isEmpty() || second.isEmpty()) return false
   if (first.size >= MAX_PART_COUNT || second.size >= MAX_PART_COUNT) return false
   val firstIterator = PartHolderIterator(first)
   val secondIterator = PartHolderIterator(second)
   var intersection = 0
+
+  val firstFirstIsText = firstIsText(first)
+  val firstLastIsText = lastIsText(first)
+
+  val secondFirstIsText = firstIsText(second)
+  val secondLastIsText = lastIsText(second)
+
+  if (!firstFirstIsText && !firstLastIsText && (secondFirstIsText || secondLastIsText)) return false
+  if (!secondFirstIsText && !secondLastIsText && (firstFirstIsText || firstLastIsText)) return false
+
   while (firstIterator.hasNext() && secondIterator.hasNext()) {
     //example: "something {} something", `{}` is skipped here
     if (!firstIterator.isText()) {
@@ -412,5 +423,11 @@ private fun similar(first: List<LoggingStringPartEvaluator.PartHolder>?,
     return false
   }
 
-  return intersection >= MIN_TEXT_LENGTH
-}
+  return intersection >= minTextLength
+}
+
+private fun firstIsText(parts: List<LoggingStringPartEvaluator.PartHolder>) =
+  parts.isNotEmpty() && parts[0].isConstant && parts[0].text?.isNotBlank() == true
+
+private fun lastIsText(parts: List<LoggingStringPartEvaluator.PartHolder>) =
+  parts.isNotEmpty() && parts.last().isConstant && parts.last().text?.isNotBlank() == true

jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/logging/JavaLoggingSimilarMessageInspectionTest.kt

@@ -205,11 +205,11 @@ class JavaLoggingSimilarMessageInspectionTest : LoggingSimilarMessageInspectionT
           Logger logger = LoggerFactory.getLogger(X.class);
                   
           <weak_warning descr="Similar log messages">logger.atError()
-          .setMessage("aaa {}")
+          .setMessage("aaaaa {}")
           .log()</weak_warning>;
       
           <weak_warning descr="Similar log messages">logger.atError()
-          .setMessage("aaa 2{}")
+          .setMessage("aaaaa 2{}")
           .log()</weak_warning>;
         }        
       }

jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/logging/KotlinLoggingSimilarMessageInspectionTest.kt

@@ -69,5 +69,26 @@ class KotlinLoggingSimilarMessageInspectionTest : LoggingSimilarMessageInspectio
       }
     """.trimIndent())
   }
+
+  fun `test skip in the middle parts`() {
+    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 = "${"\${i}"}1234356${"\${i}"}"
+              LOG.info(msg)
+          }
+      
+          private fun request2(i: Int) {
+              val msg = "something 1234356${"\${i}"}"
+              LOG.info(msg)
+          }
+      }
+    """.trimIndent())
+  }
 }