SONAR-17592 Manage formattings for truncation of messages

diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java
index 5a4da59435bb3bee81d03a2413cf103b5db83436..3b7dffb95b69d9fbe2eda0de9f957e4e06b6e7b1 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java
@@ -262,8 +262,7 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure.
   }
 
   public DefaultIssue setMessage(@Nullable String s) {
-    //TODO trim messageFormattings?
-    this.message = StringUtils.abbreviate(StringUtils.trim(s), MESSAGE_MAX_SIZE);
+    this.message = StringUtils.abbreviate(s, MESSAGE_MAX_SIZE);
     return this;
   }
 

diff --git a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java
index 9f16990058b8689c07e753b9636fa47621941cb9..78ba1d218dd956b384ce1dd41d25077a0b369a03 100644 (file)
--- a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java
@@ -147,12 +147,6 @@ public class DefaultIssueTest {
     assertThat(issue.message()).hasSize(1_333);
   }
 
-  @Test
-  public void message_should_be_trimmed() {
-    issue.setMessage("    foo     ");
-    assertThat(issue.message()).isEqualTo("foo");
-  }
-
   @Test
   public void message_could_be_null() {
     issue.setMessage(null);

diff --git a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java
index 89f1c9550f3e9fd10a21c3e4ecf1287a9803b03f..2dc921874667fc01ab7c0c4e4d11626dd5838af8 100644 (file)
--- a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java
+++ b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocation.java
@@ -29,10 +29,10 @@ import org.sonar.api.batch.sensor.issue.IssueLocation;
 import org.sonar.api.batch.sensor.issue.MessageFormatting;
 import org.sonar.api.batch.sensor.issue.NewIssueLocation;
 import org.sonar.api.batch.sensor.issue.NewMessageFormatting;
+import org.sonar.api.issue.Issue;
 
 import static java.util.Objects.requireNonNull;
 import static org.apache.commons.lang.StringUtils.abbreviate;
-import static org.apache.commons.lang.StringUtils.trim;
 import static org.sonar.api.utils.Preconditions.checkArgument;
 import static org.sonar.api.utils.Preconditions.checkState;
 
@@ -64,18 +64,28 @@ public class DefaultIssueLocation implements NewIssueLocation, IssueLocation {
   @Override
   public DefaultIssueLocation message(String message) {
     validateMessage(message);
-    this.message = abbreviate(trim(message), MESSAGE_MAX_SIZE);
+    this.message = abbreviate(message, Issue.MESSAGE_MAX_SIZE);
     return this;
   }
 
   @Override
-  public NewIssueLocation message(String message, List<NewMessageFormatting> newMessageFormattings) {
+  public DefaultIssueLocation message(String message, List<NewMessageFormatting> newMessageFormattings) {
     validateMessage(message);
     validateFormattings(newMessageFormattings, message);
-    this.message = abbreviate(trim(message), MESSAGE_MAX_SIZE);
+    this.message = abbreviate(message,  Issue.MESSAGE_MAX_SIZE);
 
     for (NewMessageFormatting newMessageFormatting : newMessageFormattings) {
-      messageFormattings.add((MessageFormatting) newMessageFormatting);
+      DefaultMessageFormatting messageFormatting = (DefaultMessageFormatting) newMessageFormatting;
+      if (messageFormatting.start() >  Issue.MESSAGE_MAX_SIZE) {
+        continue;
+      }
+      if (messageFormatting.end() > Issue.MESSAGE_MAX_SIZE) {
+        messageFormatting = new DefaultMessageFormatting()
+          .start(messageFormatting.start())
+          .end( Issue.MESSAGE_MAX_SIZE)
+          .type(messageFormatting.type());
+      }
+      messageFormattings.add(messageFormatting);
     }
     return this;
   }

diff --git a/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java b/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java
index 128ed0603927a8c2c836ed6fb41630102c10d533..1c26aaa44008f6be9cb1bfe05949dafb9845f0bf 100644 (file)
--- a/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java
+++ b/sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueLocationTest.java
@@ -19,13 +19,17 @@
  */
 package org.sonar.api.batch.sensor.issue.internal;
 
+import java.util.List;
 import org.apache.commons.lang.StringUtils;
 import org.junit.Test;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
+import org.sonar.api.batch.sensor.issue.MessageFormatting;
+import org.sonar.api.batch.sensor.issue.NewMessageFormatting;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.AssertionsForClassTypes.tuple;
 
 public class DefaultIssueLocationTest {
 
@@ -56,11 +60,51 @@ public class DefaultIssueLocationTest {
   public void prevent_too_long_messages() {
     assertThat(new DefaultIssueLocation()
       .on(inputFile)
-      .message(StringUtils.repeat("a", 4000)).message()).hasSize(4000);
+      .message(StringUtils.repeat("a", 1333)).message()).hasSize(1333);
 
     assertThat(new DefaultIssueLocation()
       .on(inputFile)
-      .message(StringUtils.repeat("a", 4001)).message()).hasSize(4000);
+      .message(StringUtils.repeat("a", 1334)).message()).hasSize(1333);
+  }
+
+  @Test
+  public void should_ignore_messageFormatting_if_message_is_trimmed() {
+    DefaultMessageFormatting messageFormatting = new DefaultMessageFormatting()
+      .start(1500)
+      .end(1501)
+      .type(MessageFormatting.Type.CODE);
+
+    DefaultIssueLocation location = new DefaultIssueLocation()
+      .message(StringUtils.repeat("a", 2000), List.of(messageFormatting));
+
+    assertThat(location.messageFormattings()).isEmpty();
+  }
+
+  @Test
+  public void should_truncate_messageFormatting_if_necessary() {
+    DefaultMessageFormatting messageFormatting = new DefaultMessageFormatting()
+      .start(1300)
+      .end(1501)
+      .type(MessageFormatting.Type.CODE);
+
+    DefaultIssueLocation location = new DefaultIssueLocation()
+      .message(StringUtils.repeat("a", 2000), List.of(messageFormatting));
+
+    assertThat(location.messageFormattings())
+      .extracting(MessageFormatting::start, MessageFormatting::end)
+      .containsOnly(tuple(1300, 1333));
+  }
+
+  @Test
+  public void should_validate_message_formatting() {
+    List<NewMessageFormatting> messageFormattings = List.of(new DefaultMessageFormatting()
+      .start(1)
+      .end(3)
+      .type(MessageFormatting.Type.CODE));
+    DefaultIssueLocation location = new DefaultIssueLocation();
+
+    assertThatThrownBy(() -> location.message("aa", messageFormattings))
+      .isInstanceOf(IllegalArgumentException.class);
   }
 
   @Test