search

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

Invalid range issue with include.existing.issues=true #189

Closed hosszubalazs closed 5 years ago

hosszubalazs commented 5 years ago

Hey!

Environment: sonar-stash 1.4.0 SonarQube 6.7 LTS sonar.stash.include.existing.issues=true sonar.stash.include.vicinity.issues.range=4 // This is probably not interesting here

Issue: I get the following issue when doing PR analysis: Invalid range: [162..136]

I have a lot of lines with each of them having issues(testing 1.4 of the plugin). The PR removes lines 132-165, and adds a new line on 132.

What I expect to happen is

I think the issue is thrown by Range, probably from this line: return Range.closed(diff.getSource() - range, diff.getDestination() + range).contains(destination); https://google.github.io/guava/releases/19.0/api/docs/com/google/common/collect/Range.html

My guess is removing code kills it, needs another check to make sure the left side of closed() is the smaller number.

What do you think? Cheers and thanks, Balázs

t-8ch commented 5 years ago

Hi @hosszubalazs , thanks for the report! I will take a look at it.

daconstenla commented 5 years ago

Do we know something about this issue? we are facing it frequently in our projects 💃

t-8ch commented 5 years ago

@daconstenla Can you check the raw json API response returned from Bitbucket? (Should be doable in a browser). /rest/api/1.0/projects/xxx/repos/xxx/pull-requests/xxx/diff?withComments=true It seems the source field of the hunk is after the destination field. Feel free to clean up the JSON and post it here.

t-8ch commented 5 years ago

Can you try https://github.com/AmadeusITGroup/sonar-stash/tree/issues/189 ?

daconstenla commented 5 years ago

I'll try the version from the branch and I'll let you know, also I'll get a clean failing pull-request json string, thanks @t-8ch !

daconstenla commented 5 years ago

We have been running for one week without this issue using the mentioned version 👍 Thank you for the fix @t-8ch !

Will it be merged to master soon?

t-8ch commented 5 years ago

Done