Fix issue: HLS HLint plugin doesn't preserve HLint's severities #3881

[IAmPara0x]

preserve severity from HLint

[michaelpj]

Code looks good. I think we need to think carefully about whether we definitely want. This means that most hlint hints will turn up as warnings now. Is that too strong? I guess not - that's what hlint says they are, and if a hint seems too strong at that level then arguably that's a hlint bug.

[IAmPara0x]

This means that most hlint hints will turn up as warnings now. Is that too strong? I guess not - that's what hlint says they are, and if a hint seems too strong at that level then arguably that's a hlint bug.

I agree with you here, mainly because if people are using hlint plugin then they are using hlint and hlint gives warnings most of the times than not for the hints. So, if we don't propagate the severity of hlint, it's not an expected behaviour?

[michaelpj]

I do think hlint uses "warning" for a bunch of things that don't seem worthy of it to me. I've opened an issue on the hlint repo to discuss: ndmitchell/hlint#1549

[fendor]

Code changes look good to me, but I am also unsure whether we want to treat hlint warnings as warnings.

[michaelpj]

@IAmPara0x I'm not sure how to progress really. We could demote all of hlint's diagnostics by one level, but that doesn't really address your original annoyance that a hlint error wasn't showing up as an error. And the only real difference it would make is that hlint errors would be warnings instead of infos.

I do think that passing through hlint's severities as they are is likely to seem worse to our users :/

[IAmPara0x]

@IAmPara0x I'm not sure how to progress really. We could demote all of hlint's diagnostics by one level, but that doesn't really address your original annoyance that a hlint error wasn't showing up as an error. And the only real difference it would make is that hlint errors would be warnings instead of infos.

I do think that passing through hlint's severities as they are is likely to seem worse to our users :/

Yes, I think you are right here. Propagating default errors and warnings would confuse the users.

Maybe an alternative, solution will be to only propagate severities of only those hints that are user defined? And passing the default as infos/suggestions.

[michaelpj]

I don't think we have that information from hlint though, do we?

[michaelpj]

@IAmPara0x it looks like we got a suggestion from Neil that might work: preserve error severity and downgrade the others to info. How does that sound to you?

[IAmPara0x]

@IAmPara0x it looks like we got a suggestion from Neil that might work: preserve error severity and downgrade the others to info. How does that sound to you?

Sounds, good. I will update the PR to reflect the changes

[IAmPara0x]

I have updated the PR as we have decided, we now only preserve error severity from Hlint and rest everything is info. Let me know if the PR needs any further updates.

Initially I had an idea of changing the severity levels if they were defined explicitly in .hlint.yaml in the project's root directory. Though I am not sure, how much work it will be.