From 7c11a6c444316903bac5fa05e3f986fed20af723 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 19 Feb 2019 23:52:57 +0100 Subject: [PATCH] SONAR-11741 fix NPE when importing issues from external report Incomplete definition of issue offsets should not make the import fail. --- .../externalissue/ExternalIssueImporter.java | 17 +- .../ExternalIssueImporterTest.java | 189 ++++++++++++++++++ 2 files changed, 205 insertions(+), 1 deletion(-) create mode 100644 sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/ExternalIssueImporterTest.java diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/ExternalIssueImporter.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/ExternalIssueImporter.java index f555959044e..468648a3235 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/ExternalIssueImporter.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/ExternalIssueImporter.java @@ -119,7 +119,22 @@ public class ExternalIssueImporter { if (location.textRange != null) { if (location.textRange.startColumn != null) { TextPointer start = file.newPointer(location.textRange.startLine, location.textRange.startColumn); - TextPointer end = file.newPointer(location.textRange.endLine, location.textRange.endColumn); + int endLine; + int endColumn; + + if (location.textRange.endLine == null) { + // assume it's on a single line + endLine = location.textRange.startLine; + } else { + endLine = location.textRange.endLine; + } + if (location.textRange.endColumn == null) { + // assume it's until the last character of the end line + endColumn = file.selectLine(endLine).end().lineOffset(); + } else { + endColumn = location.textRange.endColumn; + } + TextPointer end = file.newPointer(endLine, endColumn); newLocation.at(file.newRange(start, end)); } else { newLocation.at(file.selectLine(location.textRange.startLine)); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/ExternalIssueImporterTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/ExternalIssueImporterTest.java new file mode 100644 index 00000000000..155e08b98c4 --- /dev/null +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/ExternalIssueImporterTest.java @@ -0,0 +1,189 @@ +/* + * SonarQube + * Copyright (C) 2009-2019 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.scanner.externalissue; + +import java.io.File; +import javax.annotation.Nullable; +import org.apache.commons.lang.math.RandomUtils; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.sonar.api.batch.fs.TextRange; +import org.sonar.api.batch.fs.internal.DefaultInputFile; +import org.sonar.api.batch.fs.internal.TestInputFileBuilder; +import org.sonar.api.batch.rule.Severity; +import org.sonar.api.batch.sensor.internal.SensorContextTester; +import org.sonar.api.batch.sensor.issue.ExternalIssue; +import org.sonar.api.utils.log.LogTester; +import org.sonar.api.utils.log.LoggerLevel; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.commons.lang.ObjectUtils.defaultIfNull; +import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; +import static org.assertj.core.api.Assertions.assertThat; + +public class ExternalIssueImporterTest { + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + @Rule + public LogTester logs = new LogTester(); + + private DefaultInputFile sourceFile; + private SensorContextTester context; + + @Before + public void prepare() throws Exception { + File baseDir = temp.newFolder(); + context = SensorContextTester.create(baseDir); + sourceFile = new TestInputFileBuilder("foo", "src/Foo.java") + .setModuleBaseDir(baseDir.toPath()) + .initMetadata("the first line\nthe second line") + .setCharset(UTF_8) + .setLanguage("java") + .build(); + context.fileSystem().add(sourceFile); + } + + @Test + public void import_zero_issues() { + ReportParser.Report report = new ReportParser.Report(); + report.issues = new ReportParser.Issue[0]; + + ExternalIssueImporter underTest = new ExternalIssueImporter(this.context, report); + underTest.execute(); + + assertThat(context.allExternalIssues()).isEmpty(); + assertThat(context.allIssues()).isEmpty(); + assertThat(logs.logs(LoggerLevel.INFO)).contains("Imported 0 issues in 0 files"); + } + + @Test + public void import_issue_with_minimal_info() { + ReportParser.Report report = new ReportParser.Report(); + ReportParser.Issue input = new ReportParser.Issue(); + input.engineId = "findbugs"; + input.ruleId = "123"; + input.severity = "CRITICAL"; + input.type = "BUG"; + input.primaryLocation = new ReportParser.Location(); + input.primaryLocation.filePath = sourceFile.getProjectRelativePath(); + input.primaryLocation.message = randomAlphabetic(5); + report.issues = new ReportParser.Issue[] {input}; + + ExternalIssueImporter underTest = new ExternalIssueImporter(this.context, report); + underTest.execute(); + + assertThat(context.allExternalIssues()).hasSize(1); + ExternalIssue output = context.allExternalIssues().iterator().next(); + assertThat(output.engineId()).isEqualTo(input.engineId); + assertThat(output.ruleId()).isEqualTo(input.ruleId); + assertThat(output.severity()).isEqualTo(Severity.valueOf(input.severity)); + assertThat(output.remediationEffort()).isNull(); + assertThat(logs.logs(LoggerLevel.INFO)).contains("Imported 1 issue in 1 file"); + } + + @Test + public void import_issue_with_complete_primary_location() { + ReportParser.TextRange input = new ReportParser.TextRange(); + input.startLine = 1; + input.startColumn = 4; + input.endLine= 2; + input.endColumn= 3; + + runOn(newIssue(input)); + + assertThat(context.allExternalIssues()).hasSize(1); + ExternalIssue output = context.allExternalIssues().iterator().next(); + assertSameRange(input, output.primaryLocation().textRange()); + } + + /** + * If columns are not defined, then issue is assumed to be on the full first line. + * The end line is ignored. + */ + @Test + public void import_issue_with_no_columns() { + ReportParser.TextRange input = new ReportParser.TextRange(); + input.startLine = 1; + input.startColumn = null; + input.endLine= 2; + input.endColumn= null; + + runOn(newIssue(input)); + + assertThat(context.allExternalIssues()).hasSize(1); + TextRange got = context.allExternalIssues().iterator().next().primaryLocation().textRange(); + assertThat(got.start().line()).isEqualTo(input.startLine); + assertThat(got.start().lineOffset()).isEqualTo(0); + assertThat(got.end().line()).isEqualTo(input.startLine); + assertThat(got.end().lineOffset()).isEqualTo(sourceFile.selectLine(input.startLine).end().lineOffset()); + } + + /** + * If end column is not defined, then issue is assumed to be until the last character of the end line. + */ + @Test + public void import_issue_with_start_but_not_end_column() { + ReportParser.TextRange input = new ReportParser.TextRange(); + input.startLine = 1; + input.startColumn = 3; + input.endLine= 2; + input.endColumn= null; + + runOn(newIssue(input)); + + assertThat(context.allExternalIssues()).hasSize(1); + TextRange got = context.allExternalIssues().iterator().next().primaryLocation().textRange(); + assertThat(got.start().line()).isEqualTo(input.startLine); + assertThat(got.start().lineOffset()).isEqualTo(3); + assertThat(got.end().line()).isEqualTo(input.endLine); + assertThat(got.end().lineOffset()).isEqualTo(sourceFile.selectLine(input.endLine).end().lineOffset()); + } + + private void assertSameRange(ReportParser.TextRange expected, TextRange got) { + assertThat(got.start().line()).isEqualTo(expected.startLine); + assertThat(got.start().lineOffset()).isEqualTo(defaultIfNull(expected.startColumn, 0)); + assertThat(got.end().line()).isEqualTo(expected.endLine); + assertThat(got.end().lineOffset()).isEqualTo(defaultIfNull(expected.endColumn, 0)); + } + + private void runOn(ReportParser.Issue input) { + ReportParser.Report report = new ReportParser.Report(); + report.issues = new ReportParser.Issue[] {input}; + + ExternalIssueImporter underTest = new ExternalIssueImporter(this.context, report); + underTest.execute(); + } + + private ReportParser.Issue newIssue(@Nullable ReportParser.TextRange textRange) { + ReportParser.Issue input = new ReportParser.Issue(); + input.engineId = randomAlphabetic(5); + input.ruleId = randomAlphabetic(5); + input.severity = "CRITICAL"; + input.type = "BUG"; + input.effortMinutes = RandomUtils.nextInt(); + input.primaryLocation = new ReportParser.Location(); + input.primaryLocation.filePath = sourceFile.getProjectRelativePath(); + input.primaryLocation.message = randomAlphabetic(5); + input.primaryLocation.textRange = textRange; + return input; + } +} -- 2.39.5