danger icon indicating copy to clipboard operation
danger copied to clipboard

Danger crashes if multiple inline comments have same severity, file and line number

Open pahnev opened this issue 2 years ago • 3 comments

Report

What did you do?

While implementing support for SwiftFormat plugin's inline_mode, I noticed that if multiple plugins are trying to add inline_comment with the same severity to the same file and line, danger crashes with lib/danger/request_sources/bitbucket_server.rb:158:in 'sort': comparison of Danger::Violation with Danger::Violation failed (ArgumentError). Github seems to have the same sort.

https://github.com/danger/danger/blob/master/lib/danger/request_sources/bitbucket_server.rb#L158

What did you expect to happen?

Expected danger not to crash, and all comments to be sorted out in any order.

What happened instead?

💥

Your Environment

Dangerfile

swiftlint.lint_files(fail_on_error: true, inline_mode: true, additional_swiftlint_args: '--strict')
swiftformat.check_format(fail_on_error: true, inline_mode: true)

pahnev avatar Oct 25 '21 13:10 pahnev

Fair, yeah, seems reasonable

orta avatar Nov 10 '21 12:11 orta

I wrote the following spec to reproduce this issue but it passes.

https://github.com/manicmaniac/danger/commit/92630ffd147a8676f3c5a809a991bae840903c79

@pahnev Could you tell me what values in file and line caused this error? I suppose they were invalid values that is not possible to compare.

manicmaniac avatar Feb 03 '22 04:02 manicmaniac

Hi, @manicmaniac, so here's my Dangerfile:

swiftformat.check_format(fail_on_error: true, inline_mode: true)
swiftlint.lint_files(fail_on_error: true, inline_mode: true)

And here's the output:

Results:
Errors:
- [ ] Path/To/File/ActivationTokenTests.swift#L11: warning: (consecutiveBlankLines) Replace consecutive blank lines with a single blank line. `ActivationTokenTests.swift:11`
- [ ] SwiftFormat found issues
Warnings:
- [ ] Path/To/File/ActivationTokenTests.swift#L12: Limit vertical whitespace to a single empty line. Currently 3.
`vertical_whitespace` `ActivationTokenTests.swift:12`

And if I make the SwiftLint elevate the message to an error: swiftlint.lint_files(fail_on_error: true, inline_mode: true, additional_swiftlint_args: '--strict') it results in a crash I mentioned above.

Both Swiftformat and SwiftLint work with the desired command alone, but not together.

pahnev avatar Feb 23 '22 07:02 pahnev