AmadeusITGroup / sonar-stash

Stash (BitBucket) plugin, a pull-request decorator which allows to integrate SonarQube violations directly into your pull-request
MIT License
165 stars 82 forks source link

Issue comment on the wrong line #194

Closed adhikasp closed 5 years ago

adhikasp commented 5 years ago

I was trying to use sonar-stash plugin, but found that the sonar bot was commenting the pull request on the wrong line.

image

As you can see, the comment was on line no 106 of old diff instead of line 106 new diff.

Version used:

adhikasp commented 5 years ago

I did a little debugging and found out that:

After I set vicinity range as 0, it post at the addition diff correctly

t-8ch commented 5 years ago

Hi @adhikasp , thanks for the report! Could you provide me with the raw JSON response from bitbucket? (/rest/api/1.0/projects/xxx/repos/xxx/pull-requests/xxx/diff?withComments=true, replace xxx with the correct values). Please obfuscate all names you don't want to be public. This will make it much easier to test and fix the issue.

adhikasp commented 5 years ago

@t-8ch sure, here is the json dump https://gist.github.com/adhikasp/1920791cc2436af2f8145863861f9b0e

I was messing around with the code, and this modification fix the problem

public IssueType getType(String path, long destination, int vicinityRange) {
    IssueType tmpType = null;
    for (StashDiff diff : diffs) {
      if (Objects.equals(diff.getPath(), path)) {
        // Line 0 never belongs to Stash Diff view.
        // It is a global comment with a type set to CONTEXT.
        if (destination == 0) {
          return IssueType.CONTEXT;
        } else if (destination == diff.getDestination()) {
          return diff.getType();
        } else if (includeVicinityIssuesForDiff(diff, destination, vicinityRange)) {
          tmpType = diff.getType();
        }
      }
    }
    return tmpType;
  }

It seems that sonar-stash couldn't differentiate between issues that was raised from code change and issues from existing code, and thus pick the wrong lineType. In this particular case, sonar-stash pick the CONTEXT diff because it contains the problematic line in its vicinity and it appear first in the StashDiff iteration. This hack works but the logic seems unreliable for other conditions.

t-8ch commented 5 years ago

Could you also provide the line number, sonarqube thinks this issue belongs to? (Try to put a print into the StashIssueReportingPostJob)

adhikasp commented 5 years ago

@t-8ch here it is

[INFO] Issue key 01673981387EDCCDB0, ruleKey ModifiersOrderCheck, componentKey com.gdn.package:module-api-client:src/main/java/com/gdn/package/module/client/ModuleClient.java, line 56
[INFO] Issue key 01673981387EDCCDB1, ruleKey S106, componentKey com.gdn.package:module-api-client:src/main/java/com/gdn/package/module/client/ModuleClient.java, line 109
[INFO] Issue key 01673981387EDCCDB2, ruleKey S106, componentKey com.gdn.package:module-api-client:src/main/java/com/gdn/package/module/client/ModuleClient.java, line 120
[INFO] Issue key 01673981387EDCCDB3, ruleKey S2583, componentKey com.gdn.package:module-api-client:src/main/java/com/gdn/package/module/client/ModuleClient.java, line 108
[INFO] Issue key 01673981387EDCCDB4, ruleKey S1481, componentKey com.gdn.package:module-api-client:src/main/java/com/gdn/package/module/client/ModuleClient.java, line 106
[INFO] Issue key 01673981387EDCCDB5, ruleKey S1185, componentKey com.gdn.package:module-api-client:src/main/java/com/gdn/package/module/client/ModuleClient.java, line 207
[INFO] Issue key 01673981387EDCCDB6, ruleKey S00117, componentKey com.gdn.package:module-api-client:src/main/java/com/gdn/package/module/client/ModuleClient.java, line 106
[INFO] Issue key 01673981387EDCCDB7, ruleKey S00117, componentKey com.gdn.package:module-api-client:src/main/java/com/gdn/package/module/client/ModuleClient.java, line 119

This is the result with vicinity=0

image

t-8ch commented 5 years ago

I am building a complete test case, but the file in question from the log is ModuleClient.java while in the JSON it is aModuleClient.java.