Lambda and local function suppressions

[sbomer]

This adds support for RequiresUnreferencedCode and UnconditionalSuppressMessage on lambdas and local functions, relying on heuristics and knowledge of compiler implementation details to detect lambdas and local functions.

This approach scans the code for IL references to lambdas and local functions, which has some limitations.

• Unused local functions aren't referenced by the containing method, so warnings from these will not be suppressed by suppressions on the containing method. Lambdas don't seem to have this problem, because they contain a reference to the generated method as part of the delegate conversion.
• The IL doesn't in general contain enough information to determine the nesting of the scopes of lambdas and local functions, so we make no attempt to do this. We only try to determine to which user method a lambda or local function belongs. So suppressions on a lambda or local function will not silence warnings from a nested lambda or local function in the same scope.

This also adds warnings for reflection access to compiler-generated state machine members, and to lambdas or local functions. For these, the analyzer makes no attempt to determine what the actual IL corresponding to the user code will be, so it produces fewer warnings (see tests for some examples). The linker will warn for what is actually in IL.

One way to work around the first limitation would be to rely on the compiler-generated names to determine the name of the user method, but this is fragile due to the issue @vitek-karas pointed out.

[jtschuster]

Is there a reason we don't want this while loop in IsMethodInRequiresUnreferencedCodeScope?

[sbomer]

I considered it, but think eventually we might want to pull this out so that it applies not only to methods but also to fields and other members - so I left it here for now, and when I add dataflow for nested functions I'll see what makes more sense.

[vitek-karas]

Since we can recognize local/lambda functions - couldn't we associate the inner lambda with just the outer one? We know they're related (parent/child) since we see the usage from one to the other.

This would have to be different for state machines though. Lambdas/local functions can have attributes on them specified by the user, so we should "stop" the association on the first level which can have user defined attributes. State machines are not like that and we need to go all the way up to the user defined method.

Note that we would still walk upward to determine suppressions as we already do (we have while loops in the code to handle that).

But it's possible I'm missing something.
Also - this can be lower priority - we can merge as-is and look into this afterwards.

[sbomer]

The issue is that with local functions the IL reference (if there is one at all) doesn't tell us where it is defined. So M1 and M2 produce effectively the same IL:

public class C {
    void M1() {
        void Outer() {
            Inner();
            
            void Inner() {
            }
        }
    }
    
    void M2() {
        void Outer() {
            Inner();
        }
        
        void Inner() {
        }
    }
}

It might be possible to do better for cases where they capture enclosing state - there we could potentially infer the nesting from what got captured - but this would mean more reliance on compiler implementation details.

This is only really an issue for local functions though. You're right that we could do better for lambdas since the IL will always have an actual reference to the generated lambda method at the point where it is defined (I'll update the comment to reflect this). It's debatable whether we should implement better understanding of nested lambdas, or leave lambdas with the same limitation for consistency with local functions, what do you think?

[vitek-karas]

I didn't realize that with local functions you can have them "cross scope" like this. You're right, this is probably not easily solvable. I think in that case, we just need to make sure that we don't omit warnings which should be there. I don't think we will with the current implementation (in this PR), but maybe give that a thought. It's better if we produce warnings which should not be there, than the other way round.