From 6c14711cba4dccaa5191ed648cb6013af9affb8d Mon Sep 17 00:00:00 2001
From: Tomas Bjerre <tomas.bjerre85@gmail.com>
Date: Tue, 2 Apr 2024 17:09:38 +0200
Subject: [PATCH] fix: avoid repeated message in the Coverity parser (refs
 #188)

---
 build.gradle                                  |  2 +-
 .../lib/parsers/CoverityParser.java           | 29 ++++++++++++++++---
 .../se/bjurr/violations/lib/CoverityTest.java |  2 +-
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/build.gradle b/build.gradle
index 9531f6b..6557e96 100644
--- a/build.gradle
+++ b/build.gradle
@@ -64,6 +64,6 @@ dependencies {
  testImplementation 'junit:junit:4.13.2'
  testImplementation 'org.assertj:assertj-core:3.25.3'
  testImplementation 'uk.co.jemos.podam:podam:8.0.1.RELEASE'
- testImplementation 'com.approvaltests:approvaltests:23.0.0'
+ testImplementation 'com.approvaltests:approvaltests:23.0.1'
  testImplementation 'com.networknt:json-schema-validator:1.4.0'
 }
diff --git a/src/main/java/se/bjurr/violations/lib/parsers/CoverityParser.java b/src/main/java/se/bjurr/violations/lib/parsers/CoverityParser.java
index 90f2080..ff95e64 100644
--- a/src/main/java/se/bjurr/violations/lib/parsers/CoverityParser.java
+++ b/src/main/java/se/bjurr/violations/lib/parsers/CoverityParser.java
@@ -11,9 +11,11 @@
 import se.bjurr.violations.lib.ViolationsLogger;
 import se.bjurr.violations.lib.model.SEVERITY;
 import se.bjurr.violations.lib.model.Violation;
+import se.bjurr.violations.lib.model.generated.coverity.CheckerProperty;
 import se.bjurr.violations.lib.model.generated.coverity.CoveritySchema;
 import se.bjurr.violations.lib.model.generated.coverity.Issue;
 import se.bjurr.violations.lib.reports.Parser;
+import se.bjurr.violations.lib.util.Utils;
 
 public class CoverityParser implements ViolationsParser {
 
@@ -32,10 +34,7 @@ public Set<Violation> parseReportOutput(
       violations.add(
           violationBuilder() //
               .setFile(issue.getMainEventFilePathname()) //
-              .setMessage(
-                  issue.getCheckerProperties().getSubcategoryLocalEffect()
-                      + "\n"
-                      + issue.getCheckerProperties().getSubcategoryLocalEffect()) //
+              .setMessage(this.getMessage(issue.getCheckerProperties())) //
               .setParser(Parser.COVERITY) //
               .setCategory(issue.getCheckerProperties().getCategory())
               .setRule(issue.getType() + "/" + issue.getSubtype()) //
@@ -46,6 +45,28 @@ public Set<Violation> parseReportOutput(
     return violations;
   }
 
+  private String getMessage(final CheckerProperty checkerProperty) {
+    final boolean hasLongDescription =
+        !Utils.isNullOrEmpty(checkerProperty.getSubcategoryLongDescription());
+    final boolean hasLocalEffect =
+        !Utils.isNullOrEmpty(checkerProperty.getSubcategoryLocalEffect());
+    if (hasLongDescription && hasLocalEffect) {
+      if (checkerProperty
+          .getSubcategoryLongDescription()
+          .contains(checkerProperty.getSubcategoryLocalEffect())) {
+        return checkerProperty.getSubcategoryLongDescription();
+      }
+      return checkerProperty.getSubcategoryLongDescription()
+          + ".\n"
+          + checkerProperty.getSubcategoryLocalEffect();
+    } else if (hasLongDescription) {
+      return checkerProperty.getSubcategoryLongDescription();
+    } else if (hasLocalEffect) {
+      return checkerProperty.getSubcategoryLocalEffect();
+    }
+    return checkerProperty.getImpactDescription();
+  }
+
   private SEVERITY toSeverity(final String from) {
     if (from.equalsIgnoreCase("Medium")) {
       return WARN;
diff --git a/src/test/java/se/bjurr/violations/lib/CoverityTest.java b/src/test/java/se/bjurr/violations/lib/CoverityTest.java
index 7d9351f..29e6517 100644
--- a/src/test/java/se/bjurr/violations/lib/CoverityTest.java
+++ b/src/test/java/se/bjurr/violations/lib/CoverityTest.java
@@ -30,7 +30,7 @@ public void testThatViolationsCanBeParsed() {
     final Violation violation0 = new ArrayList<>(actual).get(0);
     assertThat(violation0.getMessage()) //
         .isEqualTo(
-            "The expression's value is always zero; construct may indicate an inadvertent logic error.\nThe expression's value is always zero; construct may indicate an inadvertent logic error.");
+            "Bitwise-and ('&amp;') operation applied to zero always produces zero.\nThe expression's value is always zero; construct may indicate an inadvertent logic error.");
     assertThat(violation0.getFile()) //
         .isEqualTo("C:/Workspace/workspace/Build_jenkins_development/somefile.cs");
     assertThat(violation0.getSeverity()) //