From 92f74f7e0761eaa441b65d8e0f7efbf78ed9ab36 Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Tue, 13 Aug 2024 21:31:09 +0100 Subject: [PATCH] #945: Gather statistics for issues fixed in a pull request Sonarqube currently reports a fixed issues metric for pull requests, but the plugin isn't providing the data to allow that value to be calculated. To resolve this an additional IssueVisitor has been introduced that compares the issues from the target branch with the findings on the source branch and finds any target code blocks that no longer exists - implying the issue line has been removed - or any code that still exists but is now reporting the issue as fixed, and reports them to the PullRequestFixedIssuesRepository which is used within Sonarqube to gather the count of issues fixed in the current analysis. --- ...munityReportAnalysisComponentProvider.java | 6 +- .../PullRequestFixedIssuesIssueVisitor.java | 88 ++++++++++++ ...tyReportAnalysisComponentProviderTest.java | 39 +++++- ...ullRequestFixedIssuesIssueVisitorTest.java | 126 ++++++++++++++++++ 4 files changed, 250 insertions(+), 9 deletions(-) create mode 100644 src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/PullRequestFixedIssuesIssueVisitor.java create mode 100644 src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/PullRequestFixedIssuesIssueVisitorTest.java diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityReportAnalysisComponentProvider.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityReportAnalysisComponentProvider.java index e6158a1..39f367a 100644 --- a/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityReportAnalysisComponentProvider.java +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityReportAnalysisComponentProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2019-2022 Michael Clarke + * Copyright (C) 2019-2024 Michael Clarke * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -28,6 +28,7 @@ import com.github.mc1arke.sonarqube.plugin.almclient.github.v3.RestApplicationAu import com.github.mc1arke.sonarqube.plugin.almclient.github.v4.DefaultGraphqlProvider; import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.DefaultGitlabClientFactory; import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.PostAnalysisIssueVisitor; +import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.PullRequestFixedIssuesIssueVisitor; import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.PullRequestPostAnalysisTask; import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.azuredevops.AzureDevOpsPullRequestDecorator; import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.bitbucket.BitbucketPullRequestDecorator; @@ -53,7 +54,8 @@ public class CommunityReportAnalysisComponentProvider implements ReportAnalysisC DefaultGithubClientFactory.class, RestApplicationAuthenticationProvider.class, GithubPullRequestDecorator.class, HttpClientBuilderFactory.class, DefaultBitbucketClientFactory.class, BitbucketPullRequestDecorator.class, DefaultGitlabClientFactory.class, GitlabMergeRequestDecorator.class, - DefaultAzureDevopsClientFactory.class, AzureDevOpsPullRequestDecorator.class); + DefaultAzureDevopsClientFactory.class, AzureDevOpsPullRequestDecorator.class, + PullRequestFixedIssuesIssueVisitor.class); } } diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/PullRequestFixedIssuesIssueVisitor.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/PullRequestFixedIssuesIssueVisitor.java new file mode 100644 index 0000000..c7490b4 --- /dev/null +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/PullRequestFixedIssuesIssueVisitor.java @@ -0,0 +1,88 @@ +/* + * Copyright (C) 2024 Michael Clarke + * + * 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 com.github.mc1arke.sonarqube.plugin.ce.pullrequest; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; + +import org.sonar.api.issue.IssueStatus; +import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.issue.DefaultTrackingInput; +import org.sonar.ce.task.projectanalysis.issue.IssueVisitor; +import org.sonar.ce.task.projectanalysis.issue.fixedissues.PullRequestFixedIssueRepository; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.tracking.Input; +import org.sonar.core.issue.tracking.NonClosedTracking; +import org.sonar.core.issue.tracking.Tracker; +import org.sonar.core.issue.tracking.Tracking; + +public class PullRequestFixedIssuesIssueVisitor extends IssueVisitor { + + private static final List NON_CLOSED_ISSUE_STATUSES = List.of(IssueStatus.OPEN, IssueStatus.ACCEPTED, IssueStatus.CONFIRMED); + + private final PullRequestFixedIssueRepository pullRequestFixedIssueRepository; + private final AnalysisMetadataHolder analysisMetadataHolder; + private final Tracker tracker; + + public PullRequestFixedIssuesIssueVisitor(PullRequestFixedIssueRepository pullRequestFixedIssueRepository, + AnalysisMetadataHolder analysisMetadataHolder, + Tracker tracker) { + this.pullRequestFixedIssueRepository = pullRequestFixedIssueRepository; + this.analysisMetadataHolder = analysisMetadataHolder; + this.tracker = tracker; + } + + @Override + public void onRawIssues(Component component, Input rawIssues, Input baseIssues) { + if (!analysisMetadataHolder.isPullRequest() || baseIssues == null) { + return; + } + + for (DefaultIssue fixedIssue : findFixedIssues(rawIssues, baseIssues)) { + pullRequestFixedIssueRepository.addFixedIssue(fixedIssue); + } + } + + private List findFixedIssues(Input rawIssues, Input baseIssues) { + List nonClosedBaseIssues = baseIssues.getIssues().stream() + .filter(issue -> Optional.ofNullable(issue.issueStatus()) + .map(NON_CLOSED_ISSUE_STATUSES::contains) + .orElse(false)) + .collect(Collectors.toList()); + Input trackingIssues = new DefaultTrackingInput(nonClosedBaseIssues, baseIssues.getLineHashSequence(), baseIssues.getBlockHashSequence()); + NonClosedTracking nonClosedTrackedBaseIssues = tracker.trackNonClosed(rawIssues, trackingIssues); + + List fixedIssues = new ArrayList<>(); + fixedIssues.addAll(findFixedIssues(nonClosedTrackedBaseIssues)); + fixedIssues.addAll(nonClosedTrackedBaseIssues.getUnmatchedBases().collect(Collectors.toList())); + return fixedIssues; + } + + private static List findFixedIssues(Tracking nonClosedIssues) { + return nonClosedIssues.getMatchedRaws().entrySet().stream() + .filter(issueEntry -> issueEntry.getKey().issueStatus() == IssueStatus.FIXED) + .map(Map.Entry::getValue) + .collect(Collectors.toList()); + } + +} diff --git a/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityReportAnalysisComponentProviderTest.java b/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityReportAnalysisComponentProviderTest.java index f9e0d9c..8feeed4 100644 --- a/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityReportAnalysisComponentProviderTest.java +++ b/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/CommunityReportAnalysisComponentProviderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2019-2022 Michael Clarke + * Copyright (C) 2019-2024 Michael Clarke * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -18,21 +18,46 @@ */ package com.github.mc1arke.sonarqube.plugin.ce; -import org.junit.Test; +import static org.assertj.core.api.Assertions.assertThat; import java.util.List; -import static org.junit.Assert.assertEquals; +import org.junit.jupiter.api.Test; + +import com.github.mc1arke.sonarqube.plugin.almclient.DefaultLinkHeaderReader; +import com.github.mc1arke.sonarqube.plugin.almclient.azuredevops.DefaultAzureDevopsClientFactory; +import com.github.mc1arke.sonarqube.plugin.almclient.bitbucket.DefaultBitbucketClientFactory; +import com.github.mc1arke.sonarqube.plugin.almclient.bitbucket.HttpClientBuilderFactory; +import com.github.mc1arke.sonarqube.plugin.almclient.github.DefaultGithubClientFactory; +import com.github.mc1arke.sonarqube.plugin.almclient.github.v3.DefaultUrlConnectionProvider; +import com.github.mc1arke.sonarqube.plugin.almclient.github.v3.RestApplicationAuthenticationProvider; +import com.github.mc1arke.sonarqube.plugin.almclient.github.v4.DefaultGraphqlProvider; +import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.DefaultGitlabClientFactory; +import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.PostAnalysisIssueVisitor; +import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.PullRequestFixedIssuesIssueVisitor; +import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.PullRequestPostAnalysisTask; +import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.azuredevops.AzureDevOpsPullRequestDecorator; +import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.bitbucket.BitbucketPullRequestDecorator; +import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.github.GithubPullRequestDecorator; +import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.GitlabMergeRequestDecorator; +import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.markup.MarkdownFormatterFactory; +import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.report.ReportGenerator; /** * @author Michael Clarke */ -public class CommunityReportAnalysisComponentProviderTest { +class CommunityReportAnalysisComponentProviderTest { @Test - public void testGetComponents() { + void shouldReturnAllRegisteredReportComponents() { List result = new CommunityReportAnalysisComponentProvider().getComponents(); - assertEquals(18, result.size()); - assertEquals(CommunityBranchLoaderDelegate.class, result.get(0)); + assertThat(result).containsExactly(CommunityBranchLoaderDelegate.class, PullRequestPostAnalysisTask.class, + PostAnalysisIssueVisitor.class, DefaultLinkHeaderReader.class, ReportGenerator.class, + MarkdownFormatterFactory.class, DefaultGraphqlProvider.class, DefaultUrlConnectionProvider.class, + DefaultGithubClientFactory.class, RestApplicationAuthenticationProvider.class, GithubPullRequestDecorator.class, + HttpClientBuilderFactory.class, DefaultBitbucketClientFactory.class, BitbucketPullRequestDecorator.class, + DefaultGitlabClientFactory.class, GitlabMergeRequestDecorator.class, + DefaultAzureDevopsClientFactory.class, AzureDevOpsPullRequestDecorator.class, + PullRequestFixedIssuesIssueVisitor.class); } } diff --git a/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/PullRequestFixedIssuesIssueVisitorTest.java b/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/PullRequestFixedIssuesIssueVisitorTest.java new file mode 100644 index 0000000..42e6ace --- /dev/null +++ b/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/PullRequestFixedIssuesIssueVisitorTest.java @@ -0,0 +1,126 @@ +/* + * Copyright (C) 2024 Michael Clarke + * + * 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 com.github.mc1arke.sonarqube.plugin.ce.pullrequest; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.sonar.api.issue.IssueStatus; +import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.issue.fixedissues.PullRequestFixedIssueRepository; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.tracking.Input; +import org.sonar.core.issue.tracking.NonClosedTracking; +import org.sonar.core.issue.tracking.Tracker; + +class PullRequestFixedIssuesIssueVisitorTest { + + @Test + void shouldSkipVisitIfNotAPullRequest() { + AnalysisMetadataHolder analysisMetadataHolder = mock(); + PullRequestFixedIssueRepository pullRequestFixedIssuesRepository = mock(); + Tracker tracker = mock(); + + when(analysisMetadataHolder.isPullRequest()).thenReturn(false); + + Component component = mock(); + Input rawIssues = mock(); + Input baseIssues = mock(); + + PullRequestFixedIssuesIssueVisitor underTest = new PullRequestFixedIssuesIssueVisitor(pullRequestFixedIssuesRepository, analysisMetadataHolder, tracker); + underTest.onRawIssues(component, rawIssues, baseIssues); + + verify(analysisMetadataHolder).isPullRequest(); + + verifyNoInteractions(pullRequestFixedIssuesRepository, tracker); + } + + @Test + void shouldSkipVisitIfPullRequestButNoBaseIssuesToFix() { + AnalysisMetadataHolder analysisMetadataHolder = mock(); + PullRequestFixedIssueRepository pullRequestFixedIssuesRepository = mock(); + Tracker tracker = mock(); + + when(analysisMetadataHolder.isPullRequest()).thenReturn(true); + + Component component = mock(); + Input rawIssues = mock(); + Input baseIssues = null; + + PullRequestFixedIssuesIssueVisitor underTest = new PullRequestFixedIssuesIssueVisitor(pullRequestFixedIssuesRepository, analysisMetadataHolder, tracker); + underTest.onRawIssues(component, rawIssues, baseIssues); + + verify(analysisMetadataHolder).isPullRequest(); + + verifyNoInteractions(pullRequestFixedIssuesRepository, tracker); + } + + @Test + void shouldPassFixedAndRemovedIssuesToRepository() { + AnalysisMetadataHolder analysisMetadataHolder = mock(); + PullRequestFixedIssueRepository pullRequestFixedIssuesRepository = mock(); + Tracker tracker = mock(); + + when(analysisMetadataHolder.isPullRequest()).thenReturn(true); + + Component component = mock(); + Input rawIssues = mock(); + List baseIssueList = List.of(mockIssue(IssueStatus.FALSE_POSITIVE), mockIssue(IssueStatus.OPEN), mockIssue(IssueStatus.FIXED), mockIssue(IssueStatus.OPEN), mockIssue(IssueStatus.FALSE_POSITIVE), mockIssue(null)); + Input baseIssues = mock(); + when(baseIssues.getIssues()).thenReturn(baseIssueList); + + NonClosedTracking nonClosedTracking = mock(); + Map matchedRaws = Map.of(mockIssue(IssueStatus.OPEN), baseIssueList.get(1), mockIssue(IssueStatus.FIXED), baseIssueList.get(3)); + when(nonClosedTracking.getMatchedRaws()).thenReturn(matchedRaws); + Stream unmatchedBaseIssues = Stream.of(baseIssueList.get(0), baseIssueList.get(2)); + when(nonClosedTracking.getUnmatchedBases()).thenReturn(unmatchedBaseIssues); + when(tracker.trackNonClosed(any(), any())).thenReturn(nonClosedTracking); + + PullRequestFixedIssuesIssueVisitor underTest = new PullRequestFixedIssuesIssueVisitor(pullRequestFixedIssuesRepository, analysisMetadataHolder, tracker); + underTest.onRawIssues(component, rawIssues, baseIssues); + + verify(analysisMetadataHolder).isPullRequest(); + ArgumentCaptor> trackingIssuesCaptor = ArgumentCaptor.captor(); + verify(tracker).trackNonClosed(eq(rawIssues), trackingIssuesCaptor.capture()); + assertThat(trackingIssuesCaptor.getValue().getIssues()).containsExactly(baseIssueList.get(1), baseIssueList.get(3)); + verify(nonClosedTracking).getUnmatchedBases(); + + verify(pullRequestFixedIssuesRepository).addFixedIssue(baseIssueList.get(0)); + verify(pullRequestFixedIssuesRepository).addFixedIssue(baseIssueList.get(2)); + verify(pullRequestFixedIssuesRepository).addFixedIssue(baseIssueList.get(3)); + } + + private static DefaultIssue mockIssue(IssueStatus issueStatus) { + DefaultIssue issue = mock(DefaultIssue.class); + when(issue.issueStatus()).thenReturn(issueStatus); + return issue; + } +}