Not sure about the name, could not think about anything better though. If you have any better ideas, let me know!

I think this option should be on by default, and developers would need to turn it off if they don't want it.

I think for now we should start with opt-in. There are rough edges we need to figure out first. But eventually I definitely agree - the value is there for sure :-)

Could we make it opt-in by adding the new warning code(s) to NoWarn for now, or use a new warning wave that won't be on by default until .NET 8? I don't see why this warning should get its own flag to disable it.

This would also give us additional validation by at least running the relevant code, even if the warnings are ultimately suppressed.

For now we're going with producing a new warning - in which case I agree with @sbomer that using either NoWarn or warning levels would be a great way to do this.

I think we should leave out the exact mechanics our of this document - we'll figure them out later on. It obviously has to be a toggle - and if it's on by default is a discussion for a time when we have much better understanding of the impact/UX of this.

Could we make it opt-in by adding the new warning code(s) to NoWarn for now, or use a new warning wave that won't be on by default until .NET 8? I don't see why this warning should get its own flag to disable it.

This would also give us additional validation by at least running the relevant code, even if the warnings are ultimately suppressed.

I think we should eventually do both, since each tool has its own strengths.

I would be worried that the analyzer can recommend removing a suppression just because doesn't have the full picture. Non-actionable code and code that either way is going to be removed seems less worrying to me.
On the linker side of things, I would be worried if implementing the linker tooling is going to recommend removing a suppression because of something we annotated using a feature that the analyzer knows best (like backtracking).

Great point, we will need to be very careful about the redundant suppression warning in areas where we don't have parity between the liker and analyzer.

Related, maybe we should specify that the "redundant suppression warning" suppression itself will never be reported as a redundant suppression - that way it can be used to temporarily work around any cases where we get it wrong? 😆 Although if this becomes common it will recreate the original problem.

Since we are already talking about things that are blurry in the document here:
What's the scope of this feature? my guess is that it should be restricted to a certain amount of warnings and attributes. For example, if it's implemented in the linker we wouldn't be able to detect a redundant UnconditionalSuppressMessage originated by RequiresDynamicCode or RequiresAssemblyFiles, or what is worse it will be flagged always as redundant just because Linker doesn't analyze it.
Also, is this feature going to be supported by NativeAOT?

All good questions. My take:

• We should scope the linker implementation to "linker" - just like we do with Requires* stuff. So this should only react to linker produced warnings (and their respective suppressions). @jkurdek could you please add this to the doc? - only suppressions for warnings produced by linker should be checked - other suppressions should be completely ignored.
• I imagine we would port this to NativeAOT (once we have proper sharing of linker->AOT it should almost come for free), but it's not a priority. In that case the set of recognized warnings codes would grow to the set which can be produced by NativeAOT.

I think a guiding principal of this (at least for now) should be "best effort" and "reduce noise". Unfortunately this feature comes with some amount of noise which is effectively by-design and there's little we can do about it. So eventually we should start thinking about an escape hatch - like suppressing the new warning itself :-), but as @sbomer notes this can lead to problems on its own.

I see this as a good starting point - also why it should be opt-in by default.

Added a note that only suppressions produced by the linker will be checked.

Illink also does a bunch of optimizations that may render a suppression that was necessary at source compilation time look unnecessary. E.g. if constant propagation figures out a codepath is unreachable and suddenly the suppression looks unnecessary. but it's just unnecessary for that specific deployment mode.

The analyzer has a better view of the code from this perspective.

We're running into this in the framework (around feature switches). Overall I agree that the tool should be able to "See" this (analyzer will, it's really hard to make that work for linker).
That said, especially in framework I would tend to refactor the code to avoid this altogether. Basically what this points to is the inability to target a specific line of code with the suppression because it has to be on IL level (so no pragmas). A way to workaround that limitation is to extract the offending call into a local function and suppress at the local function level. This has other benefits:

• The suppression is very specific and self-documenting from that point of view - otherwise it's actually rather difficult to tell what exact method call the suppression is targeted at, unless it mentions it in comment.
• If the code is changed to add a new call which causes the same warning code again, it will get suppressed automagically due to the presence of the suppression on the method - this can lead to trimming holes.
• It allows even linker to reason abut unnecessary suppressions.

It's not ideal, but for framework the benefits of very targeted suppressions outweighs the added complexity.

I would remove this - or maybe just say "The way to turn this on is TBD" or something like that.
Mainly because it's actually unlikely we will do it as a command line switch after the discussion.

done

For now we're going with producing a new warning - in which case I agree with @sbomer that using either NoWarn or warning levels would be a great way to do this.

I think we should leave out the exact mechanics our of this document - we'll figure them out later on. It obviously has to be a toggle - and if it's on by default is a discussion for a time when we have much better understanding of the impact/UX of this.

I would probably remove this - just say, running the linker with the detection enabled will produce a new warning.

done