From 1f5842ee24c4b544acb2c8dea81754809e70dab9 Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Tue, 29 Nov 2022 03:49:09 -0600 Subject: [PATCH] SONAR-17592 Manage formattings for truncation of messages --- .../org/sonar/core/issue/DefaultIssue.java | 3 +- .../sonar/core/issue/DefaultIssueTest.java | 6 --- .../issue/internal/DefaultIssueLocation.java | 20 ++++++-- .../internal/DefaultIssueLocationTest.java | 48 ++++++++++++++++++- 4 files changed, 62 insertions(+), 15 deletions(-) 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 5a4da59435b..3b7dffb95b6 100644 --- 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 9f16990058b..78ba1d218dd 100644 --- 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 89f1c9550f3..2dc92187466 100644 --- 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 newMessageFormattings) { + public DefaultIssueLocation message(String message, List 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 128ed060392..1c26aaa4400 100644 --- 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 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 -- 2.39.5