Adjust syntax equivalence to account for nullable directives (#39227)

Adjust syntax equivalence to account for nullable directives

src/Compilers/CSharp/Portable/Syntax/SyntaxEquivalence.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 
 namespace Microsoft.CodeAnalysis.CSharp.Syntax
@@ -43,19 +44,15 @@ public static bool AreEquivalent(SyntaxTokenList before, SyntaxTokenList after)
 
         public static bool AreEquivalent(SyntaxToken before, SyntaxToken after)
         {
-            return before.RawKind == after.RawKind && (before.Node == null || AreTokensEquivalent(before.Node, after.Node));
+            return before.RawKind == after.RawKind && (before.Node == null || AreTokensEquivalent(before.Node, after.Node, ignoreChildNode: null));
         }
 
-        private static bool AreTokensEquivalent(GreenNode before, GreenNode after)
+        private static bool AreTokensEquivalent(GreenNode before, GreenNode after, Func<SyntaxKind, bool> ignoreChildNode)
         {
             // NOTE(cyrusn): Do we want to drill into trivia?  Can documentation ever affect the
             // global meaning of symbols?  This can happen in java with things like the "@obsolete"
             // clause in doc comment.  However, i don't know if anything like that exists in C#. 
 
-            // NOTE(cyrusn): I don't believe we need to examine pp directives.  We actually want to
-            // intentionally ignore them as we're only paying attention to the trees that affect
-            // symbolic information.
-
             // NOTE(cyrusn): I don't believe we need to examine skipped text.  It isn't relevant from
             // the perspective of global symbolic information.
             Debug.Assert(before.RawKind == after.RawKind);
@@ -69,16 +66,24 @@ private static bool AreTokensEquivalent(GreenNode before, GreenNode after)
             switch ((SyntaxKind)before.RawKind)
             {
                 case SyntaxKind.IdentifierToken:
-                    return ((Green.SyntaxToken)before).ValueText == ((Green.SyntaxToken)after).ValueText;
+                    if (((Green.SyntaxToken)before).ValueText != ((Green.SyntaxToken)after).ValueText)
+                    {
+                        return false;
+                    }
+                    break;
 
                 case SyntaxKind.NumericLiteralToken:
                 case SyntaxKind.CharacterLiteralToken:
                 case SyntaxKind.StringLiteralToken:
                 case SyntaxKind.InterpolatedStringTextToken:
-                    return ((Green.SyntaxToken)before).Text == ((Green.SyntaxToken)after).Text;
+                    if (((Green.SyntaxToken)before).Text != ((Green.SyntaxToken)after).Text)
+                    {
+                        return false;
+                    }
+                    break;
             }
 
-            return true;
+            return AreNullableDirectivesEquivalent(before, after, ignoreChildNode);
         }
 
         private static bool AreEquivalentRecursive(GreenNode before, GreenNode after, Func<SyntaxKind, bool> ignoreChildNode, bool topLevel)
@@ -101,7 +106,7 @@ private static bool AreEquivalentRecursive(GreenNode before, GreenNode after, Fu
             if (before.IsToken)
             {
                 Debug.Assert(after.IsToken);
-                return AreTokensEquivalent(before, after);
+                return AreTokensEquivalent(before, after, ignoreChildNode);
             }
 
             if (topLevel)
@@ -112,7 +117,7 @@ private static bool AreEquivalentRecursive(GreenNode before, GreenNode after, Fu
                 {
                     case SyntaxKind.Block:
                     case SyntaxKind.ArrowExpressionClause:
-                        return true;
+                        return AreNullableDirectivesEquivalent(before, after, ignoreChildNode);
                 }
 
                 // If we're only checking top level equivalence, then we don't have to go down into
@@ -210,5 +215,46 @@ private static bool AreEquivalentRecursive(GreenNode before, GreenNode after, Fu
                 return true;
             }
         }
+
+        private static bool AreNullableDirectivesEquivalent(GreenNode before, GreenNode after, Func<SyntaxKind, bool> ignoreChildNode)
+        {
+            // Fast path for when the caller does not care about nullable directives. This can happen in some IDE refactorings.
+            if (ignoreChildNode is object && ignoreChildNode(SyntaxKind.NullableDirectiveTrivia))
+            {
+                return true;
+            }
+
+            using var beforeDirectivesEnumerator = ((Green.CSharpSyntaxNode)before).GetDirectives().GetEnumerator();
+            using var afterDirectivesEnumerator = ((Green.CSharpSyntaxNode)after).GetDirectives().GetEnumerator();
+            while (true)
+            {
+                Green.DirectiveTriviaSyntax beforeAnnotation = getNextNullableDirective(beforeDirectivesEnumerator, ignoreChildNode);
+                Green.DirectiveTriviaSyntax afterAnnotation = getNextNullableDirective(afterDirectivesEnumerator, ignoreChildNode);
+
+                if (beforeAnnotation == null || afterAnnotation == null)
+                {
+                    return beforeAnnotation == afterAnnotation;
+                }
+
+                if (!AreEquivalentRecursive(beforeAnnotation, afterAnnotation, ignoreChildNode, topLevel: false))
+                {
+                    return false;
+                }
+
+                static Green.DirectiveTriviaSyntax getNextNullableDirective(IEnumerator<Green.DirectiveTriviaSyntax> enumerator, Func<SyntaxKind, bool> ignoreChildNode)
+                {
+                    while (enumerator.MoveNext())
+                    {
+                        var current = enumerator.Current;
+                        if (current.Kind == SyntaxKind.NullableDirectiveTrivia)
+                        {
+                            return current;
+                        }
+                    }
+
+                    return null;
+                }
+            }
+        }
     }
 }

src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxEquivalenceTests.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
-using Microsoft.CodeAnalysis.CSharp.Symbols;
-using Microsoft.CodeAnalysis.CSharp.Syntax;
-using Microsoft.CodeAnalysis.Text;
+using System;
 using Roslyn.Test.Utilities;
 using Xunit;
 
@@ -28,6 +26,15 @@ private void VerifyNotEquivalent(SyntaxTree tree1, SyntaxTree tree2, bool topLev
             Assert.False(SyntaxFactory.AreEquivalent(tree1, tree3, topLevel));
         }
 
+        private void VerifyEquivalent(SyntaxNode node1, SyntaxNode node2, Func<SyntaxKind, bool> ignoreChildNode)
+        {
+            Assert.True(SyntaxFactory.AreEquivalent(node1, node2, ignoreChildNode));
+
+            // now try as if the second tree were created from scratch.
+            var tree3 = SyntaxFactory.ParseSyntaxTree(node2.GetText().ToString());
+            Assert.True(SyntaxFactory.AreEquivalent(node1, tree3.GetRoot(), ignoreChildNode));
+        }
+
         [Fact]
         public void TestEmptyTrees()
         {
@@ -448,5 +455,135 @@ public void TestExpressionBodiedMethod()
             VerifyEquivalent(tree1, tree2, topLevel: true);
             VerifyNotEquivalent(tree1, tree2, topLevel: false);
         }
+
+        [Theory, WorkItem(38694, "https://github.com/dotnet/roslyn/issues/38694")]
+        [InlineData("#nullable enable", "#nullable disable")]
+        [InlineData("#nullable enable", "#nullable restore")]
+        [InlineData("#nullable disable", "#nullable restore")]
+        [InlineData("#nullable enable", "#nullable enable warnings")]
+        [InlineData("#nullable enable", "#nullable enable annotations")]
+        [InlineData("#nullable enable annotations", "#nullable enable warnings")]
+        [InlineData("", "#nullable disable")]
+        [InlineData("", "#nullable enable")]
+        [InlineData("", "#nullable restore")]
+        [InlineData("#nullable disable", "")]
+        [InlineData("#nullable enable", "")]
+        [InlineData("#nullable restore", "")]
+        public void TestNullableDirectives_DifferentDirectives(string firstDirective, string secondDirective)
+        {
+            var tree1 = SyntaxFactory.ParseSyntaxTree($@"
+{firstDirective}
+class C
+{{
+}}");
+            var tree2 = SyntaxFactory.ParseSyntaxTree($@"
+{secondDirective}
+class C
+{{
+}}");
+
+            VerifyNotEquivalent(tree1, tree2, topLevel: true);
+            VerifyNotEquivalent(tree1, tree2, topLevel: false);
+            VerifyEquivalent(tree1.GetRoot(), tree2.GetRoot(), ignoreChildNode: k => k == SyntaxKind.NullableDirectiveTrivia);
+
+            var tree3 = SyntaxFactory.ParseSyntaxTree($@"
+class C
+{{
+    void M()
+    {{
+{firstDirective}
+    }}
+}}");
+            var tree4 = SyntaxFactory.ParseSyntaxTree($@"
+class C
+{{
+    void M()
+    {{
+{secondDirective}
+    }}
+}}");
+
+            VerifyNotEquivalent(tree3, tree4, topLevel: true);
+            VerifyNotEquivalent(tree3, tree4, topLevel: false);
+            VerifyEquivalent(tree3.GetRoot(), tree4.GetRoot(), ignoreChildNode: k => k == SyntaxKind.NullableDirectiveTrivia);
+        }
+
+        [Theory, WorkItem(38694, "https://github.com/dotnet/roslyn/issues/38694")]
+        [InlineData("#nullable enable")]
+        [InlineData("#nullable disable")]
+        [InlineData("#nullable restore")]
+        [InlineData("#nullable enable warnings")]
+        public void TestNullableDirectives_TopLevelIdentical(string directive)
+        {
+            var tree1 = SyntaxFactory.ParseSyntaxTree($@"
+class C
+{{
+    void M()
+    {{
+{directive}
+        Console.WriteLine(1234);
+    }}
+}}");
+            var tree2 = SyntaxFactory.ParseSyntaxTree($@"
+class C
+{{
+    void M()
+    {{
+{directive}
+    }}
+}}");
+
+            VerifyEquivalent(tree1, tree2, topLevel: true);
+            VerifyNotEquivalent(tree1, tree2, topLevel: false);
+        }
+
+        [Fact, WorkItem(38694, "https://github.com/dotnet/roslyn/issues/38694")]
+        public void TestNullableDirectives_InvalidDirective()
+        {
+            var tree1 = SyntaxFactory.ParseSyntaxTree(@"
+class C
+{
+    void M()
+    {
+#nullable invalid
+    }
+}");
+            var tree2 = SyntaxFactory.ParseSyntaxTree(@"
+class C
+{
+    void M()
+    {
+    }
+}");
+
+            VerifyNotEquivalent(tree1, tree2, topLevel: true);
+            VerifyNotEquivalent(tree1, tree2, topLevel: false);
+        }
+
+        [Fact, WorkItem(38694, "https://github.com/dotnet/roslyn/issues/38694")]
+        public void TestNullableDirectives_DifferentNumberOfDirectives()
+        {
+            var tree1 = SyntaxFactory.ParseSyntaxTree(@"
+class C
+{
+    void M()
+    {
+#nullable enable
+    }
+}");
+
+            var tree2 = SyntaxFactory.ParseSyntaxTree(@"
+class C
+{
+    void M()
+    {
+#nullable enable
+#nullable disable
+    }
+}");
+
+            VerifyNotEquivalent(tree1, tree2, topLevel: true);
+            VerifyNotEquivalent(tree1, tree2, topLevel: false);
+        }
     }
 }

src/Workspaces/CSharp/Portable/AddImports/CSharpAddImportsService.cs

@@ -73,7 +73,7 @@ protected override SyntaxList<ExternAliasDirectiveSyntax> GetExterns(SyntaxNode
 
         protected override bool IsEquivalentImport(SyntaxNode a, SyntaxNode b)
         {
-            return a.IsEquivalentTo(b, topLevel: false);
+            return SyntaxFactory.AreEquivalent(a, b, kind => kind == SyntaxKind.NullableDirectiveTrivia);
         }
 
         private class Rewriter : CSharpSyntaxRewriter