Introduce warning supression for problematic pattern-matches

[MateuszKowalewski]

This is a temporary workaround! The long-term goal, of course, is to get rid of these nasty annotations again. But this will need support from the Scala team though as there seem to be issues with the current pattern matcher exhaustiveness checker.

The idea is to pick one of the annotated places, temporarily remove the warning suppression, based on the error create an upstream issue in Scala, wait for resolution or even help resolve it, and than kill the picked @nowarn annotation for good. Rinse and repeat until all annotations are gone…

It makes no sense, imho, to keep an ever growing pile of warnings in Libretto. Also, breaking the problem in small actionable issues will help to solve it. The current state is a little bit overwhelming to tackle all at once.

[MateuszKowalewski]

I didn't specify the warning suppression directly at the affected place as this would kill readability of the code. (I've tried!)

Also this allows to have small batches of issues to upstream. So the process of getting rid of the annotations again wouldn't take too long. (OK, it will take very long, as issues with the exhaustiveness checker are notorious in Scala. But one needs to start somewhere…)

[MateuszKowalewski]

The other thing is: The "Unchecked Warnings" look actually more dangerous as the pattern matching issues.

The pattern match checker is often very wrong in Scala. But the "Unchecked Warnings" point more than often to real issues. Should this get separately investigated? Does it need a dedicated ticket?

[TomasMikula]

I think we are on the same page with the ultimate goal being 0 exhaustiveness warnings and 0 warning suppressions.

We can't have both today, due to issues in the Scala compiler.

(Aside: Although it's IMO unlikely that things with the compiler will improve in the near future, reporting the issues is still about the best thing we can do to help.)

I do agree with you that eliminating noisy warnings from the build is valuable, so that in the development process we can focus on the parts of the code that were changed. A counterpoint being that exhaustiveness of a pattern match may break at a distance (without changes in the pattern match itself). But still, I think you would argue, we have a chance to catch at least some errors, as opposed to capitulate completely to the flood of false warnings.

This is all well and good. But I also want to see a path from here to the ultimate goal. Things might actually improve in scalac over the coming years. When something improves in the compiler, how do we find out

1. that something has improved;
2. which suppressions can then be removed?

I'd like to set some kind of a reminder.

(If each suppression was annotated with a link to a scalac ticket, we'd know 1. by subscribing to notifications on that ticket, and 2. by git grep <ticket>.)

What's the proposed way to go forward from suppressed warnings?

The idea is to pick one of the annotated places, temporarily remove the warning suppression, based on the error create an upstream issue in Scala, wait for resolution or even help resolve it, and than kill the picked @nowarn annotation for good. Rinse and repeat until all annotations are gone…

Do you mean that each @nowarn (without a link) to scalac ticket, would be a kind of reminder for action? Even if the action is just to link to an existing or create a new scalac ticket?

[MateuszKowalewski]

I think we are on the same page with the ultimate goal being 0 exhaustiveness warnings and 0 warning suppressions.

Exactly!

I actually hate @nowarn…

It's a measure of last resort.

But: Being warning free is the only sane thing in any code base that doesn't fit on a page.

Do you mean that each @nowarn (without a link) to scalac ticket, would be a kind of reminder for action? Even if the action is just to link to an existing or create a new scalac ticket?

In a sense, yes.

But let's try your approach first and see how far we can get: Let's try create upstream tickets for all the current @nowarns. (It that's not feasible for all cases, we will find out)

(If each suppression was annotated with a link to a scalac ticket, we'd know 1. by subscribing to notifications on that ticket, and 2. by git grep .)

Yes, this makes sense. Being involved directly will give the needed visibility.

I'm going to help here! Only, likely not before the weekend.

Thanks again for pushing this forward! :+1: