diff --git a/src/Analyzers/CSharp/Analyzers/RemoveUnnecessaryLambdaExpression/CSharpRemoveUnnecessaryLambdaExpressionDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/RemoveUnnecessaryLambdaExpression/CSharpRemoveUnnecessaryLambdaExpressionDiagnosticAnalyzer.cs index 9183a35a5ad1a..bb2ae9b569e48 100644 --- a/src/Analyzers/CSharp/Analyzers/RemoveUnnecessaryLambdaExpression/CSharpRemoveUnnecessaryLambdaExpressionDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/RemoveUnnecessaryLambdaExpression/CSharpRemoveUnnecessaryLambdaExpressionDiagnosticAnalyzer.cs @@ -176,6 +176,26 @@ private void AnalyzeSyntax(SyntaxNodeAnalysisContext context, INamedTypeSymbol? if (invokedMethod.GetAttributes().Any(a => Equals(a.AttributeClass, conditionalAttributeType))) return; + // In the case where we have `() => expr.m()`, check if `expr` is overwritten anywhere. If so then we do not + // want to remove the lambda, as that will bind eagerly to the original `expr` and will not see the write + // that later happens + if (invokedExpression is MemberAccessExpressionSyntax { Expression: var accessedExpression }) + { + // Limit the search space to the outermost code block that could contain references to this expr (or + // fall back to compilation unit for top level statements). + var outermostBody = invokedExpression.AncestorsAndSelf().Last( + n => n is BlockSyntax or ArrowExpressionClauseSyntax or AnonymousFunctionExpressionSyntax or CompilationUnitSyntax); + foreach (var candidate in outermostBody.DescendantNodes().OfType()) + { + if (candidate != accessedExpression && + SemanticEquivalence.AreEquivalent(semanticModel, candidate, accessedExpression) && + candidate.IsWrittenTo(semanticModel, cancellationToken)) + { + return; + } + } + } + // Semantically, this looks good to go. Now, do an actual speculative replacement to ensure that the // non-invoked method reference refers to the same method symbol, and that it converts to the same type that // the lambda was. diff --git a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryLambdaExpression/RemoveUnnecessaryLambdaExpressionTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryLambdaExpression/RemoveUnnecessaryLambdaExpressionTests.cs index 3511e248f7b0f..d0405c2424f8e 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryLambdaExpression/RemoveUnnecessaryLambdaExpressionTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryLambdaExpression/RemoveUnnecessaryLambdaExpressionTests.cs @@ -21,18 +21,29 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.RemoveUnnecessaryLambda [Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryLambdaExpression)] public class RemoveUnnecessaryLambdaExpressionTests { - private static async Task TestInRegularAndScriptAsync(string testCode, string fixedCode, LanguageVersion version = LanguageVersion.Preview) + private static async Task TestInRegularAndScriptAsync( + string testCode, + string fixedCode, + LanguageVersion version = LanguageVersion.Preview, + OutputKind? outputKind = null) { await new VerifyCS.Test { TestCode = testCode, FixedCode = fixedCode, LanguageVersion = version, + TestState = + { + OutputKind = outputKind, + } }.RunAsync(); } - private static Task TestMissingInRegularAndScriptAsync(string testCode, LanguageVersion version = LanguageVersion.Preview) - => TestInRegularAndScriptAsync(testCode, testCode, version); + private static Task TestMissingInRegularAndScriptAsync( + string testCode, + LanguageVersion version = LanguageVersion.Preview, + OutputKind? outputKind = null) + => TestInRegularAndScriptAsync(testCode, testCode, version, outputKind); [Fact] public async Task TestMissingInCSharp10() @@ -1722,5 +1733,242 @@ private static void M2(Action a) { } } """); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69094")] + public async Task TestNotWithAssignmentOfInvokedExpression1() + { + await TestMissingInRegularAndScriptAsync(""" + using System; + using System.Threading.Tasks; + + TaskCompletionSource valueSet = new(); + Helper helper = new(v => valueSet.SetResult(v)); + helper.Set(true); + valueSet = new(); + helper.Set(false); + + class Helper + { + private readonly Action action; + internal Helper(Action action) + { + this.action = action; + } + + internal void Set(bool value) => action(value); + } + + """, outputKind: OutputKind.ConsoleApplication); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69094")] + public async Task TestWithoutAssignmentOfInvokedExpression1() + { + await TestInRegularAndScriptAsync(""" + using System; + using System.Threading.Tasks; + + TaskCompletionSource valueSet = new(); + Helper helper = new([|v => |]valueSet.SetResult(v)); + helper.Set(true); + helper.Set(false); + + class Helper + { + private readonly Action action; + internal Helper(Action action) + { + this.action = action; + } + + internal void Set(bool value) => action(value); + } + + """, """ + using System; + using System.Threading.Tasks; + + TaskCompletionSource valueSet = new(); + Helper helper = new(valueSet.SetResult); + helper.Set(true); + helper.Set(false); + + class Helper + { + private readonly Action action; + internal Helper(Action action) + { + this.action = action; + } + + internal void Set(bool value) => action(value); + } + + """, + outputKind: OutputKind.ConsoleApplication); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69094")] + public async Task TestNotWithAssignmentOfInvokedExpression2() + { + await TestMissingInRegularAndScriptAsync(""" + using System; + using System.Threading.Tasks; + + class C + { + void M() + { + TaskCompletionSource valueSet = new(); + Helper helper = new(v => valueSet.SetResult(v)); + helper.Set(true); + valueSet = new(); + helper.Set(false); + } + } + + class Helper + { + private readonly Action action; + internal Helper(Action action) + { + this.action = action; + } + + internal void Set(bool value) => action(value); + } + + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69094")] + public async Task TestWithoutAssignmentOfInvokedExpression2() + { + await TestInRegularAndScriptAsync(""" + using System; + using System.Threading.Tasks; + + class C + { + void M() + { + TaskCompletionSource valueSet = new(); + Helper helper = new([|v => |]valueSet.SetResult(v)); + helper.Set(true); + helper.Set(false); + } + } + + class Helper + { + private readonly Action action; + internal Helper(Action action) + { + this.action = action; + } + + internal void Set(bool value) => action(value); + } + + """, """ + using System; + using System.Threading.Tasks; + + class C + { + void M() + { + TaskCompletionSource valueSet = new(); + Helper helper = new(valueSet.SetResult); + helper.Set(true); + helper.Set(false); + } + } + + class Helper + { + private readonly Action action; + internal Helper(Action action) + { + this.action = action; + } + + internal void Set(bool value) => action(value); + } + + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69094")] + public async Task TestWithoutAssignmentOfInvokedExpression3() + { + await TestInRegularAndScriptAsync(""" + using System; + using System.Threading.Tasks; + + class C + { + void M() + { + TaskCompletionSource valueSet = new(); + Helper helper = new([|v => |]valueSet.SetResult(v)); + helper.Set(true); + helper.Set(false); + + var v = () => + { + // this is a different local. it should not impact the outer simplification + TaskCompletionSource valueSet = new(); + valueSet = new(); + }; + } + } + + class Helper + { + private readonly Action action; + internal Helper(Action action) + { + this.action = action; + } + + internal void Set(bool value) => action(value); + } + + """, """ + using System; + using System.Threading.Tasks; + + class C + { + void M() + { + TaskCompletionSource valueSet = new(); + Helper helper = new(valueSet.SetResult); + helper.Set(true); + helper.Set(false); + + var v = () => + { + // this is a different local. it should not impact the outer simplification + TaskCompletionSource valueSet = new(); + valueSet = new(); + }; + } + } + + class Helper + { + private readonly Action action; + internal Helper(Action action) + { + this.action = action; + } + + internal void Set(bool value) => action(value); + } + + """); + } } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems index 1a5bd0607eb49..15a0a736d7f2e 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems @@ -534,6 +534,7 @@ + diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/SemanticEquivalence.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/SemanticEquivalence.cs similarity index 100% rename from src/Workspaces/Core/Portable/Shared/Extensions/SemanticEquivalence.cs rename to src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/SemanticEquivalence.cs