Handle known-matches collisions

src/Workspaces/Core/Portable/Differencing/Match.cs

@@ -31,9 +31,7 @@ internal Match(TNode root1, TNode root2, TreeComparer<TNode> comparer, IEnumerab
             _root1 = root1;
             _root2 = root2;
             _comparer = comparer;
-            _oneToTwo = new Dictionary<TNode, TNode>();
-            _twoToOne = new Dictionary<TNode, TNode>();
-
+            
             int labelCount = comparer.LabelCount;
 
             // Calculate chains (not including root node):
@@ -42,6 +40,12 @@ internal Match(TNode root1, TNode root2, TreeComparer<TNode> comparer, IEnumerab
             CategorizeNodesByLabels(comparer, root1, labelCount, out nodes1, out count1);
             CategorizeNodesByLabels(comparer, root2, labelCount, out nodes2, out count2);
 
+            _oneToTwo = new Dictionary<TNode, TNode>();
+            _twoToOne = new Dictionary<TNode, TNode>();
+
+            // Root nodes always match. Add them before adding known matches to make sure we always have root mapping.
+            TryAdd(root1, root2);
+
             if (knownMatches != null)
             {
                 foreach (var knownMatch in knownMatches)
@@ -61,10 +65,8 @@ internal Match(TNode root1, TNode root2, TreeComparer<TNode> comparer, IEnumerab
                         throw new ArgumentException(string.Format(WorkspacesResources.NodeMustBeContainedInTheNewTree, knownMatch.Value), nameof(knownMatches));
                     }
 
-                    if (!_oneToTwo.ContainsKey(knownMatch.Key))
-                    {
-                        Add(knownMatch.Key, knownMatch.Value);
-                    }
+                    // skip pairs whose key or value is already mapped:
+                    TryAdd(knownMatch.Key, knownMatch.Value);
                 }
             }
 
@@ -111,12 +113,6 @@ private void ComputeMatch(List<TNode>[] nodes1, List<TNode>[] nodes2)
         {
             Debug.Assert(nodes1.Length == nodes2.Length);
 
-            // Root nodes always match but they might have been added as knownMatches
-            if (!HasPartnerInTree2(_root1))
-            {
-                Add(_root1, _root2);
-            }
-
             // --- The original FastMatch algorithm ---
             // 
             // For each leaf label l, and then for each internal node label l do:
@@ -257,7 +253,11 @@ private void ComputeMatchForLabel(List<TNode> s1, List<TNode> s2, int tiedToAnce
 
                 if (matched && bestDistance <= maxAcceptableDistance)
                 {
-                    Add(node1, bestMatch);
+                    bool added = TryAdd(node1, bestMatch);
+
+                    // We checked above that node1 doesn't have a partner. 
+                    // The map is a bijection by construction, so we should be able to add the mapping.
+                    Debug.Assert(added);
 
                     // If we exactly matched to firstNonMatch2 we can advance it.
                     if (i2 == firstNonMatch2)
@@ -268,13 +268,19 @@ private void ComputeMatchForLabel(List<TNode> s1, List<TNode> s2, int tiedToAnce
             }
         }
 
-        internal void Add(TNode node1, TNode node2)
+        internal bool TryAdd(TNode node1, TNode node2)
         {
             Debug.Assert(_comparer.TreesEqual(node1, _root1));
             Debug.Assert(_comparer.TreesEqual(node2, _root2));
 
+            if (_oneToTwo.ContainsKey(node1) || _twoToOne.ContainsKey(node2))
+            {
+                return false;
+            }
+
             _oneToTwo.Add(node1, node2);
             _twoToOne.Add(node2, node1);
+            return true;
         }
 
         internal bool TryGetPartnerInTree1(TNode node2, out TNode partner1)

src/Workspaces/CoreTest/Differencing/MatchTests.cs

@@ -30,5 +30,58 @@ public void KnownMatches()
 
             Assert.Throws<ArgumentException>(() => TestTreeComparer.Instance.ComputeMatch(oldRoot, newRoot, new[] { KeyValuePair.Create(x1, x2), KeyValuePair.Create(x1, new TestNode(0, 0)) }));
         }
+
+        [Fact]
+        public void KnownMatchesDups()
+        {
+            TestNode x1, x2, y1, y2, n;
+
+            var oldRoot = new TestNode(0, 1,
+                x1 = new TestNode(1, 1),
+                y1 = new TestNode(1, 4));
+
+            var newRoot = new TestNode(0, 1,
+                x2 = new TestNode(1, 2),
+                y2 = new TestNode(1, 3));
+
+            var m = TestTreeComparer.Instance.ComputeMatch(oldRoot, newRoot, new[] 
+            {
+                KeyValuePair.Create(x1, x2),
+                KeyValuePair.Create(y1, x2),
+            });
+
+            // the first one wins:
+            Assert.True(m.TryGetNewNode(x1, out n));
+            Assert.Equal(x2, n);
+            Assert.True(m.TryGetOldNode(x2, out n));
+            Assert.Equal(x1, n);
+            Assert.True(m.TryGetNewNode(y1, out n)); // matched
+            Assert.Equal(y2, n);
+        }
+
+        [Fact]
+        public void KnownMatchesRootMatch()
+        {
+            TestNode x1, x2, n;
+
+            var oldRoot = new TestNode(0, 1,
+                x1 = new TestNode(0, 1));
+
+            var newRoot = new TestNode(0, 1,
+                x2 = new TestNode(0, 2));
+
+            var m = TestTreeComparer.Instance.ComputeMatch(oldRoot, newRoot, new[]
+            {
+                KeyValuePair.Create(x1, newRoot),
+            });
+
+            // the root wins:
+            Assert.True(m.TryGetNewNode(x1, out n)); // matched
+            Assert.Equal(x2, n);
+            Assert.True(m.TryGetOldNode(newRoot, out n));
+            Assert.Equal(oldRoot, n);
+            Assert.True(m.TryGetNewNode(oldRoot, out n));
+            Assert.Equal(newRoot, n);
+        }
     }
 }