Implement DoNotIgnoreAttribute analyzer for method return values

[jeffhandley]

Contributes to dotnet/runtime#34098, adding support for method return values that are ignored.

Notable details from the approved API:

• The analyzer should only look at the specific method declaration, and not try to infer the behavior by walking the virtual hierarchy
• For return values, discard assignment (_ = obj.Method()) should count as "not ignoring"

I will submit a separate PR with another analyzer that adds supprot for out param values that are ignored (as that one is more involved).

[codecov]

Codecov Report

Merging #6546 (58659d0) into main (916b9a6) will increase coverage by 0.00%.
The diff coverage is 98.53%.

Additional details and impacted files

@@            Coverage Diff            @@
##             main    #6546     +/-   ##
=========================================
  Coverage   96.43%   96.44%             
=========================================
  Files        1372     1377      +5     
  Lines      320265   321768   +1503     
  Branches    10293    10302      +9     
=========================================
+ Hits       308844   310323   +1479     
- Misses       8967     8988     +21     
- Partials     2454     2457      +3     

[Youssef1313]

I think there are no tests for a non-async method that are awaitable (e.g, returns a Task, Task<T>, etc)

[Youssef1313]

nit: The cast here might be unnecessary.

[Youssef1313]

Consider testing the attribute on lambda and local functions. e.g:

Func<string> x = [return: DoNotIgnore] () => "";        
x();

I'm not sure if this will work with the current implementation, or whether it's an important scenario for now.

Other than that LGTM.

[Youssef1313]

public class C
{
    public void M()
    {
        for (int i = 0; i < 100; DoNotIgnore(i++))
        {
        }
    }

    [return: DoNotIgnore]
    public string DoNotIgnore(int i) => i.ToString();
}

Quite unrealistic scenario. So feel free to not handle it :)

[Youssef1313]

[return: DoNotIgnore]
public string M1() => "";

// Do we want to produce a diagnostic (with a different ID) suggesting M2 to be annotated with DoNotIgnore?
// If yes, do we want to take it a step further and do flow analysis to flag more cases?
public string M2() => M1();

[buyaa-n]

Just curious when we are going to add the attribute in runtime and apply it to methods? It would be helpful to test the analyzer with real usages

[buyaa-n]

NIT: sounds like its need an action after the bug fixed, might want to add TODO here or wherever else is appropriate

Suggested change

	* See https://github.com/dotnet/roslyn/issues/67616
	* TODO: https://github.com/dotnet/roslyn/issues/67616

[buyaa-n]

NIT: More test idea if it makes sense to add: Tuple/ValueTuple

class C
{
    [return: System.Diagnostics.CodeAnalysis.DoNotIgnore]
    string AnnotatedMethod() => null;
    void M()
    {
        (int a, string x) value = (1, AnnotatedMethod());
     }
}

[buyaa-n]

Overall LGTM, thanks!

[jeffhandley]

We had some offline discussions about this analyzer and its potential relationship to IDE0058: Remove unnecessary expression value - .NET | Microsoft Learn and CA1806: Do not ignore method results (code analysis) - .NET | Microsoft Learn. We have not had the capacity to move that design discussion forward enough to have confidence in landing in a good place during .NET 8.

I'm going to close this PR, and when we're able to resume those discussions we will update dotnet/runtime#34098.