From b1e46690121cceeff64f83612342fa649e6ef208 Mon Sep 17 00:00:00 2001 From: ayoub elouahili Date: Wed, 18 Oct 2023 03:35:49 +0100 Subject: [PATCH 1/6] fix Inline temporary variable adds unnecessary cast --- .../InlineTemporaryCodeRefactoringProvider.cs | 4 +-- .../InlineTemporary/InlineTemporaryTests.cs | 29 ++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs index 497e5049139cf..fe9147f3da09d 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs @@ -178,7 +178,7 @@ private static async Task InlineTemporaryAsync(Document document, Vari newScope = GetScope(declarator); // Note that we only remove the local declaration if there weren't any conflicts, - if (conflictReferences.Count == 0) + if (conflictReferences.Count == 0) { // No semantic conflicts, we can remove the definition. document = await document.ReplaceNodeAsync( @@ -414,7 +414,7 @@ ExpressionSyntax CreateExpressionToInline() return SyntaxFactory.ArrayCreationExpression(arrayType, arrayInitializer); } - else if (isVar && expression is ObjectCreationExpressionSyntax or ArrayCreationExpressionSyntax or CastExpressionSyntax) + else if (isVar && expression is ObjectCreationExpressionSyntax or ArrayCreationExpressionSyntax or CastExpressionSyntax or InvocationExpressionSyntax) { // if we have `var x = new Y();` there's no need to do any casting as the type is indicated // directly in the existing code. The same holds for `new Y[]` or `(Y)...` diff --git a/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs b/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs index c780c545bb5e4..631de049f0bb3 100644 --- a/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs +++ b/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs @@ -6,11 +6,11 @@ using Microsoft.CodeAnalysis.CodeRefactorings; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.CodeRefactorings.InlineTemporary; -using Microsoft.CodeAnalysis.CSharp.Shared.Extensions; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.Test.Utilities; using Roslyn.Test.Utilities; using Xunit; +using static ICSharpCode.Decompiler.IL.Transforms.Stepper; namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeRefactorings.InlineTemporary { @@ -5835,5 +5835,32 @@ void SomeMethod(string _) { } await TestInRegularAndScriptAsync(code, expected); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69869")] + public async Task InlineTemporaryNoNeededVariable() + { + await TestInRegularAndScriptAsync( + """ + using System; + class A + { + void M(string[] args) + { + var [||]a = Math.Round(1.1D); + var b = a; + } + } + """, + """ + using System; + class A + { + void M(string[] args) + { + var b = Math.Round(1.1D); + } + } + """); + } } } From eaac957b9e28710422d7aff0d5f099530c4ad1d1 Mon Sep 17 00:00:00 2001 From: ayoub elouahili Date: Wed, 18 Oct 2023 03:39:34 +0100 Subject: [PATCH 2/6] undo space remove unused using --- .../CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs | 1 - .../InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/EditorFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs b/src/EditorFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs index 2177c6f371c5d..3dcc475f22bd0 100644 --- a/src/EditorFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs +++ b/src/EditorFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs @@ -22,7 +22,6 @@ using Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics; using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; using Microsoft.CodeAnalysis.Host; -using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Remote.Testing; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; diff --git a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs index fe9147f3da09d..f5b50a36ad653 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs @@ -178,7 +178,7 @@ private static async Task InlineTemporaryAsync(Document document, Vari newScope = GetScope(declarator); // Note that we only remove the local declaration if there weren't any conflicts, - if (conflictReferences.Count == 0) + if (conflictReferences.Count == 0) { // No semantic conflicts, we can remove the definition. document = await document.ReplaceNodeAsync( From 400c42e6018556a6b7da83db1deaffe2f644cbee Mon Sep 17 00:00:00 2001 From: ayoub elouahili Date: Mon, 23 Oct 2023 17:50:28 +0100 Subject: [PATCH 3/6] undo change add comment --- .../CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs | 1 + .../InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/EditorFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs b/src/EditorFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs index 3dcc475f22bd0..2177c6f371c5d 100644 --- a/src/EditorFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs +++ b/src/EditorFeatures/DiagnosticsTestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs @@ -22,6 +22,7 @@ using Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics; using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; using Microsoft.CodeAnalysis.Host; +using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Remote.Testing; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; diff --git a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs index f5b50a36ad653..f3a4f2d9b0481 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs @@ -416,6 +416,7 @@ ExpressionSyntax CreateExpressionToInline() } else if (isVar && expression is ObjectCreationExpressionSyntax or ArrayCreationExpressionSyntax or CastExpressionSyntax or InvocationExpressionSyntax) { + // if we have an InvocationExpressionSyntax `var x = Math.Round(1.1D);` there's no need to do any casting. // if we have `var x = new Y();` there's no need to do any casting as the type is indicated // directly in the existing code. The same holds for `new Y[]` or `(Y)...` return expression; From 10e9aed822b320a29c98f238580911150c4ce80e Mon Sep 17 00:00:00 2001 From: ELouahili Ayoub Date: Tue, 24 Oct 2023 19:23:04 +0100 Subject: [PATCH 4/6] Update InlineTemporaryCodeRefactoringProvider.cs add explication --- .../InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs index f3a4f2d9b0481..6b040eec479ad 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs @@ -416,7 +416,7 @@ ExpressionSyntax CreateExpressionToInline() } else if (isVar && expression is ObjectCreationExpressionSyntax or ArrayCreationExpressionSyntax or CastExpressionSyntax or InvocationExpressionSyntax) { - // if we have an InvocationExpressionSyntax `var x = Math.Round(1.1D);` there's no need to do any casting. + // if we have an InvocationExpressionSyntax `var x = Math.Round(1.1D);` there's no need to do any casting as x is var and the method return type is known. // if we have `var x = new Y();` there's no need to do any casting as the type is indicated // directly in the existing code. The same holds for `new Y[]` or `(Y)...` return expression; From 952ee2fda7749c6a2ed56d36f4022c0cce589d44 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 24 Oct 2023 11:54:49 -0700 Subject: [PATCH 5/6] Update src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs --- .../InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs index 6b040eec479ad..c7dbe4b499a97 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs @@ -416,7 +416,8 @@ ExpressionSyntax CreateExpressionToInline() } else if (isVar && expression is ObjectCreationExpressionSyntax or ArrayCreationExpressionSyntax or CastExpressionSyntax or InvocationExpressionSyntax) { - // if we have an InvocationExpressionSyntax `var x = Math.Round(1.1D);` there's no need to do any casting as x is var and the method return type is known. + // if we have an InvocationExpressionSyntax `var x = Math.Round(1.1D);` there's no need to do any casting + // as x's type will be entirely determined by the return type of the invoked expression. // if we have `var x = new Y();` there's no need to do any casting as the type is indicated // directly in the existing code. The same holds for `new Y[]` or `(Y)...` return expression; From dc541923507181b6d0d0646e3fe0ab8e4b5b6a54 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 22 Nov 2024 21:08:15 -0800 Subject: [PATCH 6/6] Reapply --- .../InlineTemporaryCodeRefactoringProvider.cs | 8 ++++-- .../InlineTemporary/InlineTemporaryTests.cs | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs index af667cac512ef..624dfbb5bf2e4 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs @@ -408,10 +408,14 @@ ExpressionSyntax CreateExpressionToInline() return SyntaxFactory.ArrayCreationExpression(arrayType, arrayInitializer); } - else if (isVar && expression is ObjectCreationExpressionSyntax or ArrayCreationExpressionSyntax or CastExpressionSyntax) + else if (isVar && expression is + ObjectCreationExpressionSyntax or + ArrayCreationExpressionSyntax or + CastExpressionSyntax or + InvocationExpressionSyntax) { // if we have `var x = new Y();` there's no need to do any casting as the type is indicated - // directly in the existing code. The same holds for `new Y[]` or `(Y)...` + // directly in the existing code. The same holds for `new Y[]` or `(Y)...` or `Y(...)` return expression; } else diff --git a/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs b/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs index c73157fd375d2..33b14d1cc7e88 100644 --- a/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs +++ b/src/Features/CSharpTest/InlineTemporary/InlineTemporaryTests.cs @@ -5884,4 +5884,31 @@ public string M() await TestInRegularAndScriptAsync(code, expected); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/69869")] + public async Task InlineTemporaryNoNeededVariable() + { + await TestInRegularAndScriptAsync( + """ + using System; + class A + { + void M(string[] args) + { + var [||]a = Math.Round(1.1D); + var b = a; + } + } + """, + """ + using System; + class A + { + void M(string[] args) + { + var b = Math.Round(1.1D); + } + } + """); + } }