Merge pull request #2295 from mattwar/Bug463

Change sibling check to use similarity instead of exact match

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

@@ -321,7 +321,7 @@ private void TestQualifyWithThisCore(SyntaxNode root, int index)
             Assert.Equal("this.", changes[0].NewText);
         }
 
-        [Fact(Skip = "463"), WorkItem(463, "https://github.com/dotnet/roslyn/issues/463")]
+        [Fact, WorkItem(463, "https://github.com/dotnet/roslyn/issues/463")]
         public void TestReplaceWithBuiltInType()
         {
             var original = @"
@@ -365,8 +365,6 @@ private void TestReplaceWithBuiltInTypeCore(SyntaxNode root, int index)
         {
             var oldTree = root.SyntaxTree;
 
-            var nodesToReplace = root.DescendantNodes().Where(n => n is SimpleNameSyntax && n.ToString() == "Object");
-
             var span = new TextSpan(index, 6);
             var node = root.FindNode(span, getInnermostNodeForTie: true) as SimpleNameSyntax;
             Assert.NotNull(node);

src/Compilers/Core/Portable/Syntax/SyntaxDiffer.cs

@@ -242,12 +242,17 @@ private DiffAction GetNextAction()
                     // either there is no match for the first new-node in the old-list or the 
                     // the similarity of the first old-node in the new-list is much greater
 
+                    // if we find a match for the old node in the new list, that probably means nodes were inserted before it.
                     if (indexOfOldInNew > 0)
                     {
                         // look ahead to see if the old node also appears again later in its own list
-                        var oldHasIdenticalSibling = FindExactMatch(_oldNodes, _oldNodes.Peek(), 1) >= 1;
+                        int indexOfOldInOld;
+                        int similarityOfOldInOld;
+                        FindBestMatch(_oldNodes, _oldNodes.Peek(), out indexOfOldInOld, out similarityOfOldInOld, 1);
 
-                        if (!oldHasIdenticalSibling)
+                        // don't declare an insert if the node also appeared later in the original list
+                        var oldHasSimilarSibling = (indexOfOldInOld >= 1 && similarityOfOldInOld >= similarityOfOldInNew);
+                        if (!oldHasSimilarSibling)
                         {
                             return new DiffAction(DiffOp.InsertNew, indexOfOldInNew);
                         }
@@ -331,28 +336,7 @@ private static void ReplaceFirstWithChildren(Stack<SyntaxNodeOrToken> stack)
             }
         }
 
-        private static int FindExactMatch(Stack<SyntaxNodeOrToken> stack, SyntaxNodeOrToken node, int startIndex)
-        {
-            int i = 0;
-            foreach (var stackNode in stack)
-            {
-                if (i >= MaxSearchLength)
-                {
-                    break;
-                }
-
-                if (i >= startIndex && AreIdentical(stackNode, node))
-                {
-                    return i;
-                }
-
-                i++;
-            }
-
-            return -1;
-        }
-
-        private void FindBestMatch(Stack<SyntaxNodeOrToken> stack, SyntaxNodeOrToken node, out int index, out int similarity)
+        private void FindBestMatch(Stack<SyntaxNodeOrToken> stack, SyntaxNodeOrToken node, out int index, out int similarity, int startIndex = 0)
         {
             index = -1;
             similarity = -1;
@@ -365,64 +349,67 @@ private void FindBestMatch(Stack<SyntaxNodeOrToken> stack, SyntaxNodeOrToken nod
                     break;
                 }
 
-                if (AreIdentical(stackNode, node))
-                {
-                    var sim = node.FullSpan.Length;
-                    if (sim > similarity)
-                    {
-                        index = i;
-                        similarity = sim;
-                        return;
-                    }
-                }
-                else if (AreSimilar(stackNode, node))
+                if (i >= startIndex)
                 {
-                    var sim = GetSimilarity(stackNode, node);
-
-                    // Are these really the same? This may be expensive so only check this if 
-                    // similarity is rated equal to them being identical.
-                    if (sim == node.FullSpan.Length && node.IsToken)
+                    if (AreIdentical(stackNode, node))
                     {
-                        if (stackNode.ToFullString() == node.ToFullString())
+                        var sim = node.FullSpan.Length;
+                        if (sim > similarity)
                         {
                             index = i;
                             similarity = sim;
                             return;
                         }
                     }
-
-                    if (sim > similarity)
+                    else if (AreSimilar(stackNode, node))
                     {
-                        index = i;
-                        similarity = sim;
-                    }
-                }
-                else
-                {
-                    // check one level deep inside list node's children
-                    int j = 0;
-                    foreach (var child in stackNode.ChildNodesAndTokens())
-                    {
-                        if (j >= MaxSearchLength)
+                        var sim = GetSimilarity(stackNode, node);
+
+                        // Are these really the same? This may be expensive so only check this if 
+                        // similarity is rated equal to them being identical.
+                        if (sim == node.FullSpan.Length && node.IsToken)
                         {
-                            break;
+                            if (stackNode.ToFullString() == node.ToFullString())
+                            {
+                                index = i;
+                                similarity = sim;
+                                return;
+                            }
                         }
 
-                        j++;
-
-                        if (AreIdentical(child, node))
+                        if (sim > similarity)
                         {
                             index = i;
-                            similarity = node.FullSpan.Length;
-                            return;
+                            similarity = sim;
                         }
-                        else if (AreSimilar(child, node))
+                    }
+                    else
+                    {
+                        // check one level deep inside list node's children
+                        int j = 0;
+                        foreach (var child in stackNode.ChildNodesAndTokens())
                         {
-                            var sim = GetSimilarity(child, node);
-                            if (sim > similarity)
+                            if (j >= MaxSearchLength)
+                            {
+                                break;
+                            }
+
+                            j++;
+
+                            if (AreIdentical(child, node))
                             {
                                 index = i;
-                                similarity = sim;
+                                similarity = node.FullSpan.Length;
+                                return;
+                            }
+                            else if (AreSimilar(child, node))
+                            {
+                                var sim = GetSimilarity(child, node);
+                                if (sim > similarity)
+                                {
+                                    index = i;
+                                    similarity = sim;
+                                }
                             }
                         }
                     }