From db9f7bf66b859098da0241062a60917c81e51564 Mon Sep 17 00:00:00 2001 From: Oleg Marchuk Date: Tue, 19 Dec 2017 15:52:05 +0200 Subject: [PATCH 1/6] Updated version to 1.4.0-SNAPSHOT --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index bb696a80..b5461c8f 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ org.sonar sonar-stash-plugin - 1.3.0-SNAPSHOT + 1.4.0-SNAPSHOT sonar-plugin Integration between Atlassian Stash (BitBucket) and SonarQube Stash From fa6c32df23717571cfce9aab4262a347d7c43b36 Mon Sep 17 00:00:00 2001 From: Oleg Marchuk Date: Tue, 19 Dec 2017 15:52:16 +0200 Subject: [PATCH 2/6] Added logs --- .../sonar/plugins/stash/StashProjectBuilder.java | 5 +++++ .../sonar/plugins/stash/StashRequestFacade.java | 8 ++++++++ .../issue/collector/SonarQubeCollector.java | 16 +++++++++++++--- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/sonar/plugins/stash/StashProjectBuilder.java b/src/main/java/org/sonar/plugins/stash/StashProjectBuilder.java index 8aee8af8..4c63b4bb 100644 --- a/src/main/java/org/sonar/plugins/stash/StashProjectBuilder.java +++ b/src/main/java/org/sonar/plugins/stash/StashProjectBuilder.java @@ -1,15 +1,20 @@ package org.sonar.plugins.stash; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.api.batch.bootstrap.ProjectBuilder; import java.io.File; public class StashProjectBuilder extends ProjectBuilder { + private static final Logger LOGGER = LoggerFactory.getLogger(StashProjectBuilder.class); + private File projectBaseDir; @Override public void build(Context context) { projectBaseDir = context.projectReactor().getRoot().getBaseDir(); + LOGGER.debug("Project base dir is {}", projectBaseDir); } public File getProjectBaseDir() { diff --git a/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java b/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java index 62f271e8..d4bf032d 100644 --- a/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java +++ b/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java @@ -437,6 +437,9 @@ public void resetComments(PullRequestRef pr, @Override public String getIssuePath(PostJobIssue issue) { InputComponent ip = issue.inputComponent(); + + LOGGER.debug("Input component {}", ip); + if (ip == null || !ip.isFile()) { return null; } @@ -446,6 +449,11 @@ public String getIssuePath(PostJobIssue issue) { .getRepositoryRoot() .orElse(projectBaseDir); + LOGGER.debug("Base dir is {}", baseDir); + LOGGER.debug("ip file {}", inputFile.file()); + LOGGER.debug("ip abs path {}", inputFile.absolutePath()); + LOGGER.debug("ip path {}", inputFile.path()); + LOGGER.debug("ip relative path {}", inputFile.relativePath()); return new PathResolver().relativePath(baseDir, inputFile.file()); } diff --git a/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java b/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java index 741c3620..bcdb3f76 100644 --- a/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java +++ b/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java @@ -43,14 +43,18 @@ static boolean shouldIncludeIssue( ) { if (!includeExistingIssues && !issue.isNew()) { if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Issue {} is not a new issue and so, not added to the report", issue.key()); + LOGGER.debug("Issue {} is not a new issue and so, NOT ADDED to the report" + + ", issue.componentKey = {}, issue.key = {}, issue.ruleKey = {}, issue.message = {}, issue.line = {}", + issue, issue.componentKey(), issue.key(), issue.ruleKey(), issue.message(), issue.line()); } return false; } if (excludedRules.contains(issue.ruleKey())) { if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Issue {} is ignored, not added to the report", issue.key()); + LOGGER.debug("Issue {} is ignored, NOT ADDED to the report" + + ", issue.componentKey = {}, issue.key = {}, issue.ruleKey = {}, issue.message = {}, issue.line = {}", + issue, issue.componentKey(), issue.key(), issue.ruleKey(), issue.message(), issue.line()); } return false; } @@ -58,10 +62,16 @@ static boolean shouldIncludeIssue( String path = issuePathResolver.getIssuePath(issue); if (!isProjectWide(issue) && path == null) { if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Issue {} is not linked to a file, not added to the report", issue.key()); + LOGGER.debug("Issue {} is not linked to a file, NOT ADDED to the report" + + ", issue.componentKey = {}, issue.key = {}, issue.ruleKey = {}, issue.message = {}, issue.line = {}", + issue, issue.componentKey(), issue.key(), issue.ruleKey(), issue.message(), issue.line()); } return false; } + + LOGGER.debug("Issue {} is ADDED to the report" + + ", issue.componentKey = {}, issue.key = {}, issue.ruleKey = {}, issue.message = {}, issue.line = {}", + issue, issue.componentKey(), issue.key(), issue.ruleKey(), issue.message(), issue.line()); return true; } } From 78c761e445c77ac81d3f6d3d99d3cac5e1fad167 Mon Sep 17 00:00:00 2001 From: Oleg Marchuk Date: Thu, 21 Dec 2017 12:44:05 +0200 Subject: [PATCH 3/6] Added getDiff and hasPath to StashDiffReport Added toString to DiffComment --- .../sonar/plugins/stash/issue/StashDiff.java | 9 +++-- .../plugins/stash/issue/StashDiffReport.java | 20 ++++++++--- .../stash/issue/StashDiffReportTest.java | 34 +++++++++++-------- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/sonar/plugins/stash/issue/StashDiff.java b/src/main/java/org/sonar/plugins/stash/issue/StashDiff.java index 55da63fe..0f1f1766 100644 --- a/src/main/java/org/sonar/plugins/stash/issue/StashDiff.java +++ b/src/main/java/org/sonar/plugins/stash/issue/StashDiff.java @@ -1,9 +1,8 @@ package org.sonar.plugins.stash.issue; -import org.sonar.plugins.stash.StashPlugin; - import java.util.ArrayList; import java.util.List; + import org.sonar.plugins.stash.StashPlugin.IssueType; public class StashDiff { @@ -49,4 +48,10 @@ public List getComments() { public boolean containsComment(long commentId) { return comments.stream().anyMatch(c -> c.getId() == commentId); } + + @Override + public String toString() { + return "StashDiff [type=" + type + ", path=" + path + ", source=" + source + ", destination=" + destination + + ", comments=" + comments + "]"; + } } diff --git a/src/main/java/org/sonar/plugins/stash/issue/StashDiffReport.java b/src/main/java/org/sonar/plugins/stash/issue/StashDiffReport.java index 471552df..7be790b2 100644 --- a/src/main/java/org/sonar/plugins/stash/issue/StashDiffReport.java +++ b/src/main/java/org/sonar/plugins/stash/issue/StashDiffReport.java @@ -1,13 +1,13 @@ package org.sonar.plugins.stash.issue; -import com.google.common.collect.Range; -import java.util.Objects; -import org.sonar.plugins.stash.StashPlugin; - import java.util.ArrayList; import java.util.List; +import java.util.Objects; + import org.sonar.plugins.stash.StashPlugin.IssueType; +import com.google.common.collect.Range; + /** * This class is a representation of the Stash Diff view. *

@@ -104,4 +104,14 @@ public List getComments() { } return result; } -} + + public boolean hasPath(String path) { + for (StashDiff diff : diffs) { + if (Objects.equals(diff.getPath(), path)) { + return true; + } + } + + return false; + } +} \ No newline at end of file diff --git a/src/test/java/org/sonar/plugins/stash/issue/StashDiffReportTest.java b/src/test/java/org/sonar/plugins/stash/issue/StashDiffReportTest.java index 3274c39b..3bd3d32c 100755 --- a/src/test/java/org/sonar/plugins/stash/issue/StashDiffReportTest.java +++ b/src/test/java/org/sonar/plugins/stash/issue/StashDiffReportTest.java @@ -1,17 +1,15 @@ package org.sonar.plugins.stash.issue; -import org.hamcrest.core.Is; -import org.junit.Before; -import org.junit.Test; -import org.sonar.plugins.stash.StashPlugin; - -import java.util.List; -import org.sonar.plugins.stash.StashPlugin.IssueType; - import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.sonar.plugins.stash.StashPlugin.IssueType; + public class StashDiffReportTest { StashDiff diff1; @@ -28,13 +26,13 @@ public void setUp() { StashComment comment2 = mock(StashComment.class); when(comment2.getId()).thenReturn((long)54321); - diff1 = new StashDiff(IssueType.CONTEXT, "path/to/diff1", (long)10, (long)20); + diff1 = new StashDiff(IssueType.CONTEXT, "path/to/diff1", 10, 20); diff1.addComment(comment1); - diff2 = new StashDiff(IssueType.ADDED, "path/to/diff2", (long)20, (long)30); + diff2 = new StashDiff(IssueType.ADDED, "path/to/diff2", 20, 30); diff2.addComment(comment2); - diff3 = new StashDiff(IssueType.CONTEXT, "path/to/diff3", (long)30, (long)40); + diff3 = new StashDiff(IssueType.CONTEXT, "path/to/diff3", 30, 40); report1.add(diff1); report1.add(diff2); @@ -131,13 +129,13 @@ public void testGetCommentsWithoutAnyIssues() { @Test public void testGetCommentsWithDuplicatedComments() { - StashComment comment1 = new StashComment((long)12345, "message", "path", (long)1, mock(StashUser.class), (long)1); + StashComment comment1 = new StashComment(12345, "message", "path", (long)1, mock(StashUser.class), 1); diff1.addComment(comment1); - StashComment comment2 = new StashComment((long)12345, "message", "path", (long)1, mock(StashUser.class), (long)1); + StashComment comment2 = new StashComment(12345, "message", "path", (long)1, mock(StashUser.class), 1); diff2.addComment(comment2); - StashComment comment3 = new StashComment((long)54321, "message", "path", (long)1, mock(StashUser.class), (long)1); + StashComment comment3 = new StashComment(54321, "message", "path", (long)1, mock(StashUser.class), 1); diff3.addComment(comment3); List comments = report1.getComments(); @@ -147,4 +145,12 @@ public void testGetCommentsWithDuplicatedComments() { assertEquals(54321, comments.get(1).getId()); } + @Test + public void testHasPath() { + assertEquals(report1.hasPath("path/to/diff1"), true); + assertEquals(report1.hasPath("path/to/diff2"), true); + assertEquals(report1.hasPath("path/to/diff3"), true); + assertEquals(report1.hasPath("path/to/diff4"), false); + } + } From 7e1b3797c42a5c183ce21bb425374e3b4d1ef1de Mon Sep 17 00:00:00 2001 From: Oleg Marchuk Date: Thu, 21 Dec 2017 12:56:57 +0200 Subject: [PATCH 4/6] Do not include into issues not existent in diff paths --- .../stash/StashIssueReportingPostJob.java | 6 +- .../plugins/stash/StashRequestFacade.java | 9 +-- .../issue/collector/SonarQubeCollector.java | 14 ++++- .../stash/StashIssueReportingPostJobTest.java | 5 +- .../collector/SonarQubeCollectorTest.java | 55 +++++++++++++++---- 5 files changed, 67 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/sonar/plugins/stash/StashIssueReportingPostJob.java b/src/main/java/org/sonar/plugins/stash/StashIssueReportingPostJob.java index 3bf912fd..31fc595d 100644 --- a/src/main/java/org/sonar/plugins/stash/StashIssueReportingPostJob.java +++ b/src/main/java/org/sonar/plugins/stash/StashIssueReportingPostJob.java @@ -74,9 +74,6 @@ private void updateStashWithSonarInfo(StashClient stashClient, int issueThreshold = stashRequestFacade.getIssueThreshold(); PullRequestRef pr = stashRequestFacade.getPullRequest(); - // SonarQube objects - List issueReport = stashRequestFacade.extractIssueReport(issues); - StashUser stashUser = stashRequestFacade .getSonarQubeReviewer(stashCredentials.getUserSlug(), stashClient); @@ -92,6 +89,9 @@ private void updateStashWithSonarInfo(StashClient stashClient, "No Stash differential report available to process the SQ analysis"); } + // SonarQube objects + List issueReport = stashRequestFacade.extractIssueReport(issues, diffReport); + // if requested, reset all comments linked to the pull-request if (config.resetComments()) { stashRequestFacade.resetComments(pr, diffReport, stashUser, stashClient); diff --git a/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java b/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java index d4bf032d..0eaafd1f 100644 --- a/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java +++ b/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java @@ -1,12 +1,13 @@ package org.sonar.plugins.stash; import java.io.File; -import java.util.Collection; -import java.util.Optional; import java.text.MessageFormat; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.api.batch.BatchSide; @@ -47,9 +48,9 @@ public StashRequestFacade(StashPluginConfiguration stashPluginConfiguration, this.projectBaseDir = projectBuilder.getProjectBaseDir(); } - public List extractIssueReport(Iterable issues) { + public List extractIssueReport(Iterable issues, StashDiffReport diffReport) { return SonarQubeCollector.extractIssueReport( - issues, this, config.includeExistingIssues(), config.excludedRules() + issues, this, diffReport, config.includeExistingIssues(), config.excludedRules() ); } diff --git a/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java b/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java index bcdb3f76..29f74fa3 100644 --- a/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java +++ b/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java @@ -12,6 +12,7 @@ import org.sonar.api.batch.postjob.issue.PostJobIssue; import org.sonar.api.rule.RuleKey; import org.sonar.plugins.stash.IssuePathResolver; +import org.sonar.plugins.stash.issue.StashDiffReport; public final class SonarQubeCollector { @@ -24,14 +25,16 @@ private SonarQubeCollector() { /** * Create issue report according to issue list generated during SonarQube * analysis. + * @param diffReport */ public static List extractIssueReport( Iterable issues, IssuePathResolver issuePathResolver, - boolean includeExistingIssues, Set excludedRules) { + StashDiffReport diffReport, boolean includeExistingIssues, Set excludedRules) { return StreamSupport.stream( issues.spliterator(), false) .filter(issue -> shouldIncludeIssue( issue, issuePathResolver, + diffReport, includeExistingIssues, excludedRules )) .collect(Collectors.toList()); @@ -39,7 +42,7 @@ public static List extractIssueReport( static boolean shouldIncludeIssue( PostJobIssue issue, IssuePathResolver issuePathResolver, - boolean includeExistingIssues, Set excludedRules + StashDiffReport diffReport, boolean includeExistingIssues, Set excludedRules ) { if (!includeExistingIssues && !issue.isNew()) { if (LOGGER.isDebugEnabled()) { @@ -69,6 +72,13 @@ static boolean shouldIncludeIssue( return false; } + if (!diffReport.hasPath(path)) { + LOGGER.debug("Issue {} is not linked to a diff, NOT ADDED to the report" + + ", issue.componentKey = {}, issue.key = {}, issue.ruleKey = {}, issue.message = {}, issue.line = {}", + issue, issue.componentKey(), issue.key(), issue.ruleKey(), issue.message(), issue.line()); + return false; + } + LOGGER.debug("Issue {} is ADDED to the report" + ", issue.componentKey = {}, issue.key = {}, issue.ruleKey = {}, issue.message = {}, issue.line = {}", issue, issue.componentKey(), issue.key(), issue.ruleKey(), issue.message(), issue.line()); diff --git a/src/test/java/org/sonar/plugins/stash/StashIssueReportingPostJobTest.java b/src/test/java/org/sonar/plugins/stash/StashIssueReportingPostJobTest.java index d7dd4b97..2e18ff3f 100755 --- a/src/test/java/org/sonar/plugins/stash/StashIssueReportingPostJobTest.java +++ b/src/test/java/org/sonar/plugins/stash/StashIssueReportingPostJobTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; @@ -84,7 +85,7 @@ public void setUp() throws Exception { when(config.includeAnalysisOverview()).thenReturn(Boolean.TRUE); when(report.size()).thenReturn(10); - when(stashRequestFacade.extractIssueReport(eq(report))) + when(stashRequestFacade.extractIssueReport(eq(report), anyObject())) .thenReturn(report); when(context.issues()).thenReturn(report); @@ -130,7 +131,7 @@ public void testExecuteOnWithReachedThreshold() throws Exception { List report = mock(ArrayList.class); when(report.size()).thenReturn(55); - when(stashRequestFacade.extractIssueReport(eq(report))) + when(stashRequestFacade.extractIssueReport(eq(report), anyObject())) .thenReturn(report); myJob = new StashIssueReportingPostJob(config, stashRequestFacade, server); diff --git a/src/test/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollectorTest.java b/src/test/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollectorTest.java index 7f65f518..af2a548f 100755 --- a/src/test/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollectorTest.java +++ b/src/test/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollectorTest.java @@ -29,6 +29,7 @@ import org.sonar.plugins.stash.DefaultIssue; import org.sonar.plugins.stash.IssuePathResolver; import org.sonar.plugins.stash.fixtures.DummyIssuePathResolver; +import org.sonar.plugins.stash.issue.StashDiffReport; @RunWith(MockitoJUnitRunner.class) public class SonarQubeCollectorTest { @@ -48,6 +49,9 @@ public class SonarQubeCollectorTest { Set excludedRules; IssuePathResolver ipr = new DummyIssuePathResolver(); + + @Mock + StashDiffReport stashDiffReport; @Before public void setUp() throws Exception { @@ -90,7 +94,7 @@ public void setUp() throws Exception { public void testExtractEmptyIssueReport() { ArrayList issues = new ArrayList<>(); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, false, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); assertEquals(0, report.size()); } @@ -100,7 +104,10 @@ public void testExtractIssueReport() { issues.add(issue1); issues.add(issue2); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, false, excludedRules); + when(stashDiffReport.hasPath("project/path1")).thenReturn(true); + when(stashDiffReport.hasPath("project/path2")).thenReturn(true); + + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); assertEquals(2, report.size()); assertEquals(1, countIssuesBySeverity(report, Severity.BLOCKER)); assertEquals(1, countIssuesBySeverity(report, Severity.CRITICAL)); @@ -116,6 +123,16 @@ public void testExtractIssueReport() { assertEquals((Integer) 2, sqIssue2.line()); } + + @Test + public void testExtractIssueReportIssuesNotInDiff() { + ArrayList issues = new ArrayList<>(); + issues.add(issue1); + issues.add(issue2); + + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); + assertEquals(0, report.size()); + } @Test public void testExtractIssueReportWithNoLine() { @@ -123,8 +140,10 @@ public void testExtractIssueReportWithNoLine() { ArrayList issues = new ArrayList<>(); issues.add(issue1); + + when(stashDiffReport.hasPath("project/path1")).thenReturn(true); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, false, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); assertEquals(1, report.size()); assertEquals(1, countIssuesBySeverity(report, Severity.BLOCKER)); assertEquals(0, countIssuesBySeverity(report, Severity.CRITICAL)); @@ -143,8 +162,11 @@ public void testExtractIssueReportWithOldOption() { ArrayList issues = new ArrayList<>(); issues.add(issue1); issues.add(issue2); + + when(stashDiffReport.hasPath("project/path1")).thenReturn(true); + when(stashDiffReport.hasPath("project/path2")).thenReturn(true); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, false, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); assertEquals(1, report.size()); assertEquals(0, countIssuesBySeverity(report, Severity.BLOCKER)); assertEquals(1, countIssuesBySeverity(report, Severity.CRITICAL)); @@ -157,8 +179,11 @@ public void testExtractIssueReportWithOneIssueWithoutInputFile() { ArrayList issues = new ArrayList<>(); issues.add(issue1); issues.add(issue2); + + when(stashDiffReport.hasPath("project/path1")).thenReturn(true); + when(stashDiffReport.hasPath("project/path2")).thenReturn(true); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, false, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); assertEquals(1, report.size()); assertEquals(0, countIssuesBySeverity(report, Severity.BLOCKER)); assertEquals(1, countIssuesBySeverity(report, Severity.CRITICAL)); @@ -177,8 +202,11 @@ public void testExtractIssueReportWithIncludeExistingIssuesOption() { ArrayList issues = new ArrayList<>(); issues.add(issue1); issues.add(issue2); + + when(stashDiffReport.hasPath("project/path1")).thenReturn(true); + when(stashDiffReport.hasPath("project/path2")).thenReturn(true); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, true, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, true, excludedRules); assertEquals(2, report.size()); } @@ -190,10 +218,13 @@ public void testExtractIssueReportWithExcludedRules() { ArrayList issues = new ArrayList<>(); issues.add(issue1); issues.add(issue2); + + when(stashDiffReport.hasPath("project/path1")).thenReturn(true); + when(stashDiffReport.hasPath("project/path2")).thenReturn(true); excludedRules.add(RuleKey.of("foo", "bar")); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, true, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, true, excludedRules); assertEquals(1, report.size()); assertEquals("key2", report.get(0).key()); } @@ -202,25 +233,27 @@ public void testExtractIssueReportWithExcludedRules() { public void testShouldIncludeIssue() { Set er = new HashSet<>(); InputComponent ic = new DefaultInputFile("module", "some/path"); + + when(stashDiffReport.hasPath("some/path")).thenReturn(true); assertFalse( SonarQubeCollector.shouldIncludeIssue( - new DefaultIssue().setNew(false).setInputComponent(ic), ipr, false, er + new DefaultIssue().setNew(false).setInputComponent(ic), ipr, stashDiffReport, false, er ) ); assertTrue( SonarQubeCollector.shouldIncludeIssue( - new DefaultIssue().setNew(false).setInputComponent(ic), ipr, true, er + new DefaultIssue().setNew(false).setInputComponent(ic), ipr, stashDiffReport, true, er ) ); assertTrue( SonarQubeCollector.shouldIncludeIssue( - new DefaultIssue().setNew(true).setInputComponent(ic), ipr, false, er + new DefaultIssue().setNew(true).setInputComponent(ic), ipr, stashDiffReport, false, er ) ); assertTrue( SonarQubeCollector.shouldIncludeIssue( - new DefaultIssue().setNew(true).setInputComponent(ic), ipr, true, er + new DefaultIssue().setNew(true).setInputComponent(ic), ipr, stashDiffReport, true, er ) ); } From 892af5e8966b1b7877a1ab61c6210a40f3c2efa5 Mon Sep 17 00:00:00 2001 From: Oleg Marchuk Date: Thu, 21 Dec 2017 12:57:16 +0200 Subject: [PATCH 5/6] Fixed slf4j test bindings --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index b5461c8f..518ca5df 100644 --- a/pom.xml +++ b/pom.xml @@ -110,7 +110,7 @@ org.slf4j slf4j-jdk14 - [1.7.21,) + 1.7.5 test From db74102556a258384c0446eb90a97931c35bca4c Mon Sep 17 00:00:00 2001 From: Oleg Marchuk Date: Mon, 15 Jan 2018 19:58:47 +0200 Subject: [PATCH 6/6] Do not include into issues not existent in diff --- .../plugins/stash/StashRequestFacade.java | 2 +- .../issue/collector/SonarQubeCollector.java | 25 +++++++++-- .../collector/SonarQubeCollectorTest.java | 44 +++++++++++-------- 3 files changed, 48 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java b/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java index 0eaafd1f..a79bcf83 100644 --- a/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java +++ b/src/main/java/org/sonar/plugins/stash/StashRequestFacade.java @@ -50,7 +50,7 @@ public StashRequestFacade(StashPluginConfiguration stashPluginConfiguration, public List extractIssueReport(Iterable issues, StashDiffReport diffReport) { return SonarQubeCollector.extractIssueReport( - issues, this, diffReport, config.includeExistingIssues(), config.excludedRules() + issues, this, diffReport, config.includeExistingIssues(), config.excludedRules(), config.issueVicinityRange() ); } diff --git a/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java b/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java index 29f74fa3..462cf4e6 100644 --- a/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java +++ b/src/main/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollector.java @@ -12,6 +12,7 @@ import org.sonar.api.batch.postjob.issue.PostJobIssue; import org.sonar.api.rule.RuleKey; import org.sonar.plugins.stash.IssuePathResolver; +import org.sonar.plugins.stash.StashPlugin.IssueType; import org.sonar.plugins.stash.issue.StashDiffReport; public final class SonarQubeCollector { @@ -26,23 +27,24 @@ private SonarQubeCollector() { * Create issue report according to issue list generated during SonarQube * analysis. * @param diffReport + * @param i */ public static List extractIssueReport( Iterable issues, IssuePathResolver issuePathResolver, - StashDiffReport diffReport, boolean includeExistingIssues, Set excludedRules) { + StashDiffReport diffReport, boolean includeExistingIssues, Set excludedRules, int issueVicinityRange) { return StreamSupport.stream( issues.spliterator(), false) .filter(issue -> shouldIncludeIssue( issue, issuePathResolver, diffReport, - includeExistingIssues, excludedRules + includeExistingIssues, excludedRules, issueVicinityRange )) .collect(Collectors.toList()); } static boolean shouldIncludeIssue( PostJobIssue issue, IssuePathResolver issuePathResolver, - StashDiffReport diffReport, boolean includeExistingIssues, Set excludedRules + StashDiffReport diffReport, boolean includeExistingIssues, Set excludedRules, int issueVicinityRange ) { if (!includeExistingIssues && !issue.isNew()) { if (LOGGER.isDebugEnabled()) { @@ -73,12 +75,27 @@ static boolean shouldIncludeIssue( } if (!diffReport.hasPath(path)) { - LOGGER.debug("Issue {} is not linked to a diff, NOT ADDED to the report" + LOGGER.debug("Issue {} is not linked to a diff by path, NOT ADDED to the report" + ", issue.componentKey = {}, issue.key = {}, issue.ruleKey = {}, issue.message = {}, issue.line = {}", issue, issue.componentKey(), issue.key(), issue.ruleKey(), issue.message(), issue.line()); return false; } + Integer issueLine = issue.line(); + if (issueLine == null) { + issueLine = 0; + } + + // check if issue belongs to the Stash diff view + IssueType type = diffReport.getType(path, issueLine, issueVicinityRange); + if (type == null) { + LOGGER.debug("Issue {} is not linked to a diff, NOT ADDED to the report" + + ", issue.componentKey = {}, issue.key = {}, issue.ruleKey = {}, issue.message = {}, issue.line = {}", issue, + issue.componentKey(), issue.key(), issue.ruleKey(), issue.message(), issue.line()); + return false; + } + + LOGGER.debug("Issue {} is ADDED to the report" + ", issue.componentKey = {}, issue.key = {}, issue.ruleKey = {}, issue.message = {}, issue.line = {}", issue, issue.componentKey(), issue.key(), issue.ruleKey(), issue.message(), issue.line()); diff --git a/src/test/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollectorTest.java b/src/test/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollectorTest.java index af2a548f..86af350c 100755 --- a/src/test/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollectorTest.java +++ b/src/test/java/org/sonar/plugins/stash/issue/collector/SonarQubeCollectorTest.java @@ -1,25 +1,25 @@ package org.sonar.plugins.stash.issue.collector; -import java.util.HashSet; -import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.anyObject; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.plugins.stash.StashPluginUtils.countIssuesBySeverity; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import org.sonar.api.batch.fs.FilePredicate; -import org.sonar.api.batch.fs.FilePredicates; -import org.sonar.api.batch.fs.FileSystem; import org.sonar.api.batch.fs.InputComponent; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.internal.DefaultInputFile; @@ -28,6 +28,7 @@ import org.sonar.api.rule.RuleKey; import org.sonar.plugins.stash.DefaultIssue; import org.sonar.plugins.stash.IssuePathResolver; +import org.sonar.plugins.stash.StashPlugin.IssueType; import org.sonar.plugins.stash.fixtures.DummyIssuePathResolver; import org.sonar.plugins.stash.issue.StashDiffReport; @@ -94,7 +95,7 @@ public void setUp() throws Exception { public void testExtractEmptyIssueReport() { ArrayList issues = new ArrayList<>(); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules, 0); assertEquals(0, report.size()); } @@ -106,8 +107,9 @@ public void testExtractIssueReport() { when(stashDiffReport.hasPath("project/path1")).thenReturn(true); when(stashDiffReport.hasPath("project/path2")).thenReturn(true); + when(stashDiffReport.getType(anyString(), anyLong(), anyInt())).thenReturn(IssueType.CONTEXT); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules, 0); assertEquals(2, report.size()); assertEquals(1, countIssuesBySeverity(report, Severity.BLOCKER)); assertEquals(1, countIssuesBySeverity(report, Severity.CRITICAL)); @@ -130,7 +132,7 @@ public void testExtractIssueReportIssuesNotInDiff() { issues.add(issue1); issues.add(issue2); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules, 0); assertEquals(0, report.size()); } @@ -142,8 +144,9 @@ public void testExtractIssueReportWithNoLine() { issues.add(issue1); when(stashDiffReport.hasPath("project/path1")).thenReturn(true); + when(stashDiffReport.getType(anyString(), anyLong(), anyInt())).thenReturn(IssueType.CONTEXT); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules, 0); assertEquals(1, report.size()); assertEquals(1, countIssuesBySeverity(report, Severity.BLOCKER)); assertEquals(0, countIssuesBySeverity(report, Severity.CRITICAL)); @@ -165,8 +168,9 @@ public void testExtractIssueReportWithOldOption() { when(stashDiffReport.hasPath("project/path1")).thenReturn(true); when(stashDiffReport.hasPath("project/path2")).thenReturn(true); + when(stashDiffReport.getType(anyString(), anyLong(), anyInt())).thenReturn(IssueType.CONTEXT); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules, 0); assertEquals(1, report.size()); assertEquals(0, countIssuesBySeverity(report, Severity.BLOCKER)); assertEquals(1, countIssuesBySeverity(report, Severity.CRITICAL)); @@ -182,8 +186,9 @@ public void testExtractIssueReportWithOneIssueWithoutInputFile() { when(stashDiffReport.hasPath("project/path1")).thenReturn(true); when(stashDiffReport.hasPath("project/path2")).thenReturn(true); + when(stashDiffReport.getType(anyString(), anyLong(), anyInt())).thenReturn(IssueType.CONTEXT); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, false, excludedRules, 0); assertEquals(1, report.size()); assertEquals(0, countIssuesBySeverity(report, Severity.BLOCKER)); assertEquals(1, countIssuesBySeverity(report, Severity.CRITICAL)); @@ -205,8 +210,9 @@ public void testExtractIssueReportWithIncludeExistingIssuesOption() { when(stashDiffReport.hasPath("project/path1")).thenReturn(true); when(stashDiffReport.hasPath("project/path2")).thenReturn(true); + when(stashDiffReport.getType(anyString(), anyLong(), anyInt())).thenReturn(IssueType.CONTEXT); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, true, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, true, excludedRules, 0); assertEquals(2, report.size()); } @@ -221,10 +227,11 @@ public void testExtractIssueReportWithExcludedRules() { when(stashDiffReport.hasPath("project/path1")).thenReturn(true); when(stashDiffReport.hasPath("project/path2")).thenReturn(true); + when(stashDiffReport.getType(anyString(), anyLong(), anyInt())).thenReturn(IssueType.CONTEXT); excludedRules.add(RuleKey.of("foo", "bar")); - List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, true, excludedRules); + List report = SonarQubeCollector.extractIssueReport(issues, ipr, stashDiffReport, true, excludedRules, 0); assertEquals(1, report.size()); assertEquals("key2", report.get(0).key()); } @@ -235,25 +242,26 @@ public void testShouldIncludeIssue() { InputComponent ic = new DefaultInputFile("module", "some/path"); when(stashDiffReport.hasPath("some/path")).thenReturn(true); + when(stashDiffReport.getType(anyString(), anyLong(), anyInt())).thenReturn(IssueType.CONTEXT); assertFalse( SonarQubeCollector.shouldIncludeIssue( - new DefaultIssue().setNew(false).setInputComponent(ic), ipr, stashDiffReport, false, er + new DefaultIssue().setNew(false).setInputComponent(ic), ipr, stashDiffReport, false, er, 0 ) ); assertTrue( SonarQubeCollector.shouldIncludeIssue( - new DefaultIssue().setNew(false).setInputComponent(ic), ipr, stashDiffReport, true, er + new DefaultIssue().setNew(false).setInputComponent(ic), ipr, stashDiffReport, true, er, 0 ) ); assertTrue( SonarQubeCollector.shouldIncludeIssue( - new DefaultIssue().setNew(true).setInputComponent(ic), ipr, stashDiffReport, false, er + new DefaultIssue().setNew(true).setInputComponent(ic), ipr, stashDiffReport, false, er, 0 ) ); assertTrue( SonarQubeCollector.shouldIncludeIssue( - new DefaultIssue().setNew(true).setInputComponent(ic), ipr, stashDiffReport, true, er + new DefaultIssue().setNew(true).setInputComponent(ic), ipr, stashDiffReport, true, er, 0 ) ); }