Skip to content
This repository has been archived by the owner on Jan 7, 2021. It is now read-only.

Report issues with no direct line in the diff view #161

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/main/java/org/sonar/plugins/stash/StashRequestFacade.java
Original file line number Diff line number Diff line change
Expand Up @@ -193,36 +193,37 @@ private void postIssueComment(PullRequestRef pr,
String issueKey = issue.key();
String path = getIssuePath(issue);
StashCommentReport comments = commentsByFile.get(path);
String commentContent = getMarkdownPrinter().printIssueMarkdown(issue);
Integer issueLine = issue.line();
if (issueLine == null) {
issueLine = 0;
}
// Surprisingly this syntax does not trigger the squid:NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE rule
// but it does if you transform that into a ternary operator at the assignment level :/

// if comment not already pushed to Stash
if (comments != null && comments.contains(commentContent, path, issueLine)) {
LOGGER.debug("Comment \"{}\" already pushed on file {} ({})", issueKey, path, issueLine);
return;
}
IssueType type = diffReport.getType(path, issueLine, config.issueVicinityRange());

// check if issue belongs to the Stash diff view
IssueType type = diffReport.getType(path, issueLine, config.issueVicinityRange());
if (type == null) {
LOGGER.info(
"Comment \"{}\" cannot be pushed to Stash like it does not belong to diff view - {} (line: {})",
issueKey, path, issueLine);
return;
}

long line = diffReport.getLine(path, issueLine);
long diffLine = diffReport.getLine(path, issueLine);
String commentContent = getMarkdownPrinter().printIssueMarkdown(issue, diffLine == 0);

// if comment not already pushed to Stash
if (comments != null && comments.contains(commentContent, path, diffLine)) {
LOGGER.debug("Comment \"{}\" already pushed on file {} ({})", issueKey, path, diffLine);
return;
}

StashComment comment = stashClient
.postCommentLineOnPullRequest(pr, commentContent, path, line, type);
.postCommentLineOnPullRequest(pr, commentContent, path, diffLine, type);

LOGGER
.debug("Comment \"{}\" has been created ({}) on file {} ({})", issueKey, type, path, line);
.debug("Comment \"{}\" has been created ({}) on file {} ({})", issueKey, type, path, diffLine);

// Create task linked to the comment if configured

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ public static String printIssueNumberBySeverityMarkdown(Collection<PostJobIssue>
}

public String printIssueMarkdown(PostJobIssue issue) {
return printIssueMarkdown(issue, false);
}

public String printIssueMarkdown(PostJobIssue issue, boolean includeLineNumber) {
StringBuilder sb = new StringBuilder();
String message = issue.message();

Expand All @@ -76,6 +80,10 @@ public String printIssueMarkdown(PostJobIssue issue) {
sonarQubeURL + "/" + CODING_RULES_RULE_KEY + issue.ruleKey()))
.append("]");

if (includeLineNumber) {
sb.append(" (Line ").append(issue.line()).append(")");
}

return sb.toString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,11 @@ private static boolean includeVicinityIssuesForDiff(StashDiff diff, long destina
}

public IssueType getType(String path, long destination, int vicinityRange) {
boolean foundFile = false;

for (StashDiff diff : diffs) {
if (Objects.equals(diff.getPath(), path)) {
foundFile = true;
// Line 0 never belongs to Stash Diff view.
// It is a global comment with a type set to CONTEXT.
if (destination == 0) {
Expand All @@ -56,7 +59,12 @@ public IssueType getType(String path, long destination, int vicinityRange) {
}
}
}
return null;

if (foundFile) {
return IssueType.CONTEXT;
} else {
return null;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ public void testGetType() {
assertEquals(IssueType.CONTEXT, report1.getType("path/to/diff1", 20, StashDiffReport.VICINITY_RANGE_NONE));
assertEquals(IssueType.ADDED, report1.getType("path/to/diff2", 30, StashDiffReport.VICINITY_RANGE_NONE));

assertEquals(null, report1.getType("path/to/diff2", 20, StashDiffReport.VICINITY_RANGE_NONE));
assertEquals(null, report1.getType("path/to/diff1", 30, StashDiffReport.VICINITY_RANGE_NONE));
assertEquals(IssueType.CONTEXT, report1.getType("path/to/diff2", 20, StashDiffReport.VICINITY_RANGE_NONE));
assertEquals(IssueType.CONTEXT, report1.getType("path/to/diff1", 30, StashDiffReport.VICINITY_RANGE_NONE));
assertEquals(null, report1.getType("path/to/diff4", 60, StashDiffReport.VICINITY_RANGE_NONE));
}

Expand Down