Add warning if semantics may change

src/EditorFeatures/CSharpTest/CodeActions/InlineTemporary/InlineTemporaryTests.cs

@@ -4962,6 +4962,142 @@ static void Bar(AwaitExpressionSyntax awaitExpression)
         if (!(awaitExpression.Expression is ParenthesizedExpressionSyntax parenthesizedExpression))
             return;
     }
+}");
+        }
+
+        [WorkItem(42835, "https://github.com/dotnet/roslyn/issues/42835")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineTemporary)]
+        public async Task WarnWhenPossibleChangeInSemanticMeaning()
+        {
+            await TestInRegularAndScriptAsync(@"
+class C
+{
+    int P { get; set; }
+
+    void M()
+    {
+        var [||]c = new C();
+        c.P = 1;
+        var c2 = c;
+    }
+}",
+@"
+class C
+{
+    int P { get; set; }
+
+    void M()
+    {
+        {|Warning:new C().P = 1|};
+        var c2 = new C();
+    }
+}");
+        }
+
+        [WorkItem(42835, "https://github.com/dotnet/roslyn/issues/42835")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineTemporary)]
+        public async Task WarnWhenPossibleChangeInSemanticMeaning_IgnoreParentheses()
+        {
+            await TestInRegularAndScriptAsync(@"
+class C
+{
+    int P { get; set; }
+
+    void M()
+    {
+        var [||]c = (new C());
+        c.P = 1;
+        var c2 = c;
+    }
+}",
+@"
+class C
+{
+    int P { get; set; }
+
+    void M()
+    {
+        {|Warning:(new C()).P = 1|};
+        var c2 = (new C());
+    }
+}");
+        }
+
+        [WorkItem(42835, "https://github.com/dotnet/roslyn/issues/42835")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineTemporary)]
+        public async Task WarnWhenPossibleChangeInSemanticMeaning_MethodInvocation()
+        {
+            await TestInRegularAndScriptAsync(@"
+class C
+{
+    int P { get; set; }
+
+    void M()
+    {
+        var [||]c = M2();
+        c.P = 1;
+        var c2 = c;
+    }
+
+    C M2()
+    {
+        return new C();
+    }
+}",
+@"
+class C
+{
+    int P { get; set; }
+
+    void M()
+    {
+        {|Warning:M2().P = 1|};
+        var c2 = M2();
+    }
+
+    C M2()
+    {
+        return new C();
+    }
+}");
+        }
+
+        [WorkItem(42835, "https://github.com/dotnet/roslyn/issues/42835")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineTemporary)]
+        public async Task WarnWhenPossibleChangeInSemanticMeaning_MethodInvocation2()
+        {
+            await TestInRegularAndScriptAsync(@"
+class C
+{
+    int P { get; set; }
+
+    void M()
+    {
+        var [||]c = new C();
+        c.M2();
+        var c2 = c;
+    }
+
+    void M2()
+    {
+        P = 1;
+    }
+}",
+@"
+class C
+{
+    int P { get; set; }
+
+    void M()
+    {
+        {|Warning:new C().M2()|};
+        var c2 = new C();
+    }
+
+    void M2()
+    {
+        P = 1;
+    }
 }");
         }
     }

src/Features/CSharp/Portable/CSharpFeaturesResources.resx

@@ -588,6 +588,9 @@
   <data name="Warning_Inlining_temporary_into_conditional_method_call" xml:space="preserve">
     <value>Warning: Inlining temporary into conditional method call.</value>
   </data>
+  <data name="Warning_Inlining_temporary_variable_may_change_semantic_meaning" xml:space="preserve">
+    <value>Warning: Inlining temporary variable may change semantic meaning.</value>
+  </data>
   <data name="local_variable_declaration" xml:space="preserve">
     <value>local variable declaration</value>
   </data>

src/Features/CSharp/Portable/CodeRefactorings/InlineTemporary/InlineTemporaryCodeRefactoringProvider.cs

@@ -184,7 +184,9 @@ private async Task<Document> InlineTemporaryAsync(Document document, VariableDec
             // Collect the identifier names for each reference.
             var local = (ILocalSymbol)semanticModel.GetDeclaredSymbol(variableDeclarator, cancellationToken);
             var symbolRefs = await SymbolFinder.FindReferencesAsync(local, updatedDocument.Project.Solution, cancellationToken).ConfigureAwait(false);
-            var references = symbolRefs.Single(r => Equals(r.Definition, local)).Locations;
+            var referencedSymbol = symbolRefs.SingleOrDefault(r => Equals(r.Definition, local));
+            var references = referencedSymbol == null ? SpecializedCollections.EmptyEnumerable<ReferenceLocation>() : referencedSymbol.Locations;
+
             var syntaxRoot = await updatedDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
 
             // Collect the topmost parenting expression for each reference.
@@ -210,6 +212,10 @@ private async Task<Document> InlineTemporaryAsync(Document document, VariableDec
 
             var originalInitializerSymbolInfo = semanticModel.GetSymbolInfo(variableDeclarator.Initializer.Value, cancellationToken);
 
+            syntaxRoot = await updatedDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+            var hasPossibleSideEffects = references.Count() > 1 &&
+                HasPossibleSideEffects(variableDeclarator.Initializer?.Value, syntaxRoot, references);
+
             // Make each topmost parenting statement or Equals Clause Expressions semantically explicit.
             updatedDocument = await updatedDocument.ReplaceNodesAsync(topmostParentingExpressions, (o, n) =>
             {
@@ -221,6 +227,14 @@ private async Task<Document> InlineTemporaryAsync(Document document, VariableDec
                     node = node.WithAdditionalAnnotations(
                         WarningAnnotation.Create(CSharpFeaturesResources.Warning_Inlining_temporary_into_conditional_method_call));
                 }
+
+                // If the refactoring may potentially change the code's semantics, display a warning message to the user.
+                if (hasPossibleSideEffects)
+                {
+                    node = node.WithAdditionalAnnotations(
+                        WarningAnnotation.Create(CSharpFeaturesResources.Warning_Inlining_temporary_variable_may_change_semantic_meaning));
+                }
+
                 return node;
             }, cancellationToken).ConfigureAwait(false);
 
@@ -254,7 +268,8 @@ private async Task<Document> InlineTemporaryAsync(Document document, VariableDec
                 if (updatedDocument == newDocument)
                 {
                     // No semantic conflicts, we can remove the definition.
-                    updatedDocument = await updatedDocument.ReplaceNodeAsync(newScope, RemoveDeclaratorFromScope(variableDeclarator, newScope), cancellationToken).ConfigureAwait(false);
+                    updatedDocument = await updatedDocument.ReplaceNodeAsync(
+                        newScope, RemoveDeclaratorFromScope(variableDeclarator, newScope), cancellationToken).ConfigureAwait(false);
                 }
                 else
                 {
@@ -266,6 +281,65 @@ private async Task<Document> InlineTemporaryAsync(Document document, VariableDec
             return updatedDocument;
         }
 
+        private static bool HasPossibleSideEffects(SyntaxNode expression, SyntaxNode syntaxRoot, IEnumerable<ReferenceLocation> references)
+        {
+            // Checks to see if inlining the temporary variable may change the code's semantics. 
+
+            if (expression == null || syntaxRoot == null)
+            {
+                return true;
+            }
+
+            // Semantics changes may occur if we have references that access properties or invoke methods (see below examples).
+            var propertyOrMethodAccess = false;
+            foreach (var reference in references)
+            {
+                var node = syntaxRoot.FindNode(reference.Location.SourceSpan);
+                if (node.IsParentKind(SyntaxKind.SimpleMemberAccessExpression, SyntaxKind.InvocationExpression))
+                {
+                    propertyOrMethodAccess = true;
+                    break;
+                }
+            }
+
+            if (!propertyOrMethodAccess)
+            {
+                return false;
+            }
+
+            if (expression.IsKind(SyntaxKind.ParenthesizedExpression, out ParenthesizedExpressionSyntax parenthesizedExpression))
+            {
+                expression = parenthesizedExpression.WalkDownParentheses();
+            }
+
+            // e.g.:
+            //     var [||]c = new C();
+            //     c.P = 1;
+            //     var x = c;
+            // After refactoring:
+            //     new C().P = 1;
+            //     var x = new C();
+            if (expression.IsKind(SyntaxKind.ObjectCreationExpression))
+            {
+                return true;
+            }
+
+            // e.g. - let method M return a new instance of an object containing property P:
+            //     var [||]c = M();
+            //     c.P = 0;
+            //     var x = c;
+            // After refactoring:
+            //     M().P = 0;
+            //     var x = M();
+            if (expression.IsKind(SyntaxKind.InvocationExpression))
+            {
+                return true;
+            }
+
+            // Assume if we reach here that the refactoring won't cause side effects.
+            return false;
+        }
+
         private static async Task<VariableDeclaratorSyntax> FindDeclaratorAsync(Document document, CancellationToken cancellationToken)
             => await FindNodeWithAnnotationAsync<VariableDeclaratorSyntax>(document, DefinitionAnnotation, cancellationToken).ConfigureAwait(false);
 

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.cs.xlf

@@ -187,6 +187,11 @@
         <target state="translated">Upozornění: dočasné vkládání do volání podmíněné metody.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">{0} tady není null.</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.de.xlf

@@ -187,6 +187,11 @@
         <target state="translated">Warnung: temporäres Inlining in bedingtem Methodenaufruf.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">"{0}" ist hier nicht NULL.</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.es.xlf

@@ -187,6 +187,11 @@
         <target state="translated">Advertencia: Inserción de una llamada temporal en otra de método condicional.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">"{0}" no es NULL aquí.</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.fr.xlf

@@ -187,6 +187,11 @@
         <target state="translated">Avertissement : Inlining temporaire dans un appel de méthode conditionnel.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">'{0}' n'a pas une valeur null ici.</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.it.xlf

@@ -187,6 +187,11 @@
         <target state="translated">Avviso: incorporamento dell'elemento temporaneo nella chiamata a un metodo condizionale.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">'{0}' non è Null in questo punto.</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.ja.xlf

@@ -187,6 +187,11 @@
         <target state="translated">警告: 一時メソッド呼び出しを条件付きメソッド呼び出しにインライン展開しています。</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">ここでは、'{0}' は null ではありません。</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.ko.xlf

@@ -187,6 +187,11 @@
         <target state="translated">경고: 임시 작업을 조건부 메서드 호출로 인라인 처리합니다.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">'{0}'은(는) 여기에서 null이 아닙니다.</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.pl.xlf

@@ -187,6 +187,11 @@
         <target state="translated">Ostrzeżenie: tymczasowe wbudowywanie do wywołania metody warunkowej.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">Element „{0}” nie ma wartości null w tym miejscu.</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.pt-BR.xlf

@@ -187,6 +187,11 @@
         <target state="translated">Aviso: embutindo a chamada de método temporária na condicional.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">'{0}' não é nulo aqui.</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.ru.xlf

@@ -187,6 +187,11 @@
         <target state="translated">Предупреждение: встраивание временных элементов в условный вызов метода.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">Здесь "{0}" имеет значение, отличное от NULL.</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.tr.xlf

@@ -187,6 +187,11 @@
         <target state="translated">Uyarı: Koşullu yöntem çağrısında geçici öğe satır içinde kullanılıyor.</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">'{0}' burada null değil.</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.zh-Hans.xlf

@@ -187,6 +187,11 @@
         <target state="translated">警告: 即将在条件方法调用中内联临时内容。</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">“{0}”在此处不为 null。</target>

src/Features/CSharp/Portable/xlf/CSharpFeaturesResources.zh-Hant.xlf

@@ -187,6 +187,11 @@
         <target state="translated">警告: 內嵌臨時加入條件式方法呼叫。</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_Inlining_temporary_variable_may_change_semantic_meaning">
+        <source>Warning: Inlining temporary variable may change semantic meaning.</source>
+        <target state="new">Warning: Inlining temporary variable may change semantic meaning.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="_0_is_not_null_here">
         <source>'{0}' is not null here.</source>
         <target state="translated">'{0}' 在此不是 null。</target>

src/VisualStudio/Core/Def/Implementation/TableDataSource/AbstractTableControlEventProcessorProvider.cs

@@ -45,6 +45,8 @@ public override void PreprocessNavigate(ITableEntryHandle entryHandle, TableEntr
                 // that can happen if error is staled build error or user used #line pragma in C#
                 // to point to some random file in error or more.
                 e.Handled = roslynSnapshot.TryNavigateTo(index, e.IsPreview);
+
+                e.Handled = false;
             }
         }
     }