Consider running debug builds of analyzers/source generators in some build leg

[vitek-karas]

Analyzers are compiled against a specific version of the C# compiler. As such they can only recognize the shapes of the code that version of the compiler supports. With a new version of the compiler new shapes become possible, sometimes these are subtle changes, sometimes they mean adding new node types to representation or new enum values to properties. Since we can't update analyzers in sync with the compiler, the analyzer has to be written such that it can survive most of these changes without too much harm (typically it would simply ignore the new shape).

But that means that we don't get to be notified if such a new shape start occurring and can't fix the analyzer unless we have some other reason to look into that case. There will be cases where the analyzer doesn't work the way it should simply because it wasn't updated to match the latest compiler behavior.

We considered adding a warning into the analyzer to ask the user to file an issue if this happens, but that comes with rather severe downsides:

• If this happens, it's not an actionable warning (other than filing the bug), the only solution is to suppress the warning code
• Lot of users treat warnings as errors, so this becomes a blocking problem for them if it happens
• We're putting burden on our users due to our inability to keep things in sync

(See the discussion in #88836 for some more detail)

For the illink analyzer we decided to change these cases into assertions. But that means we would like to run the debug build of the analyzer on as much code as possible so that we catch these things internally when they happen.

I think it would make sense to do this for all the analyzers where it's possible - there are assertions in their code bases as well which are only exercised on targeted unit tests, but basically never on real-world code.

The same thing applies to source generators as well obviously.

/cc @sbomer @agocke @jkoritzinsky

[ghost]

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Analyzers are compiled against a specific version of the C# compiler. As such they can only recognize the shapes of the code that version of the compiler supports. With a new version of the compiler new shapes become possible, sometimes these are subtle changes, sometimes they mean adding new node types to representation or new enum values to properties. Since we can't update analyzers in sync with the compiler, the analyzer has to be written such that it can survive most of these changes without too much harm (typically it would simply ignore the new shape).

But that means that we don't get to be notified if such a new shape start occurring and can't fix the analyzer unless we have some other reason to look into that case. There will be cases where the analyzer doesn't work the way it should simply because it wasn't updated to match the latest compiler behavior.

We considered adding a warning into the analyzer to ask the user to file an issue if this happens, but that comes with rather severe downsides:

• If this happens, it's not an actionable warning (other than filing the bug), the only solution is to suppress the warning code
• Lot of users treat warnings as errors, so this becomes a blocking problem for them if it happens
• We're putting burden on our users due to our inability to keep things in sync

(See the discussion in #88836 for some more detail)

For the illink analyzer we decided to change these cases into assertions. But that means we would like to run the debug build of the analyzer on as much code as possible so that we catch these things internally when they happen.

I think it would make sense to do this for all the analyzers where it's possible - there are assertions in their code bases as well which are only exercised on targeted unit tests, but basically never on real-world code.

The same thing applies to source generators as well obviously.

/cc @sbomer @agocke @jkoritzinsky

Author:	vitek-karas
Assignees:	-
Labels:	untriaged, area-Tools-ILLink, needs-area-label
Milestone:	-

[sbomer]

This was done for the ILLink analyzers in #93259. @agocke @jkoritzinsky are there other analyzers or source generators we should consider?

For the illink analyzer we decided to change these cases into assertions.

Note that in #94888 (comment) we settled on throwing, not asserting (but throwing in Debug builds only). That's important because throwing will surface as an AD0001 warning that can be silenced, whereas asserts aren't easily silenced (see #94858).

[jkoritzinsky]

We moved all of our assertions into throws (in both debug and release) quite a while ago, so this doesn't apply for the interop generators. Since generally our generators produce code required for a successful compilation (or at least for any meaningful test case to pass), I'm not concerned about suppressions of AD0001.