Clean up comments.

src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/BatchFixAllProvider.cs

@@ -453,45 +453,68 @@ public virtual string GetFixAllTitle(FixAllState fixAllState)
             ArrayBuilder<TextChange> overlappingSpans,
             ArrayBuilder<TextChange> intersectingSpans)
         {
-            // Pure insertions (i.e. changes with empty spans) are complex to evaluate.  
-            // We have to make sure that 
-            return change.Span.IsEmpty
-                ? InsertionChangeCanBeApplied(change, overlappingSpans, intersectingSpans)
+            // We distinguish two types of changes that can happen.  'Pure Insertions' 
+            // and 'Overwrites'.  Pure-Insertions are those that are just inserting 
+            // text into a specific *position*.  They do not replace any existing text.
+            // 'Overwrites' end up replacing existing text with some other piece of 
+            // (possibly-empty) text.
+            //
+            // Overwrites of text tend to be easy to understand and merge.  It is very
+            // clear what code is being overwritten and how it should interact with
+            // other changes.  Pure-insertions are more ambiguous to deal with.  For
+            // example, say there are two pure-insertions at some position.  There is
+            // no way for us to know what to do with this.  For example, we could take
+            // one insertion then the other, or vice versa.  Because of this ambiguity
+            // we conservatively disallow cases like this.
+
+            return IsPureInsertion(change)
+                ? PureInsertionChangeCanBeApplied(change, overlappingSpans, intersectingSpans)
                 : OverwriteChangeCanBeApplied(change, overlappingSpans, intersectingSpans);
         }
 
-        private static bool InsertionChangeCanBeApplied(
+        private static bool IsPureInsertion(TextChange change)
+            => change.Span.IsEmpty;
+
+        private static bool PureInsertionChangeCanBeApplied(
             TextChange change,
             ArrayBuilder<TextChange> overlappingSpans,
             ArrayBuilder<TextChange> intersectingSpans)
         {
-            // Empty spans can't ever overlap anything.
-            Debug.Assert(change.Span.IsEmpty);
+            // Pure insertions can't ever overlap anything.  (They're just an insertion at a 
+            // single position, and overlaps can't occur with single-positions).
+            Debug.Assert(IsPureInsertion(change));
             Debug.Assert(overlappingSpans.Count == 0);
             if (intersectingSpans.Count == 0)
             {
-                // Our insertion didn't hit any other changes.  This is safe to apply.
+                // Our pure-insertion didn't hit any other changes.  This is safe to apply.
                 return true;
             }
 
             if (intersectingSpans.Count == 1)
             {
-                // Our insertion hit another change.  Thats safe when:
+                // Our pure-insertion hit another change.  Thats safe when:
                 //  1) if both changes are the same.
-                //  2) the change we're hitting is not empty and we're at the end of the other change.
+                //  2) the change we're hitting is an overwrite-change and we're at the end of it.
 
                 // Specifically, it is not safe for us to insert somewhere in start-to-middle of an 
-                // existing non-empty change.  And if we have another *empty* change, then it's 
+                // existing overwrite-change.  And if we have another pure-insertion change, then it's 
                 // not safe for both of us to be inserting at the same point (except when the 
                 // change is identical).
+
+                // Note: you may wonder why we don't support hitting an overwriting change at the
+                // start of the overwrite.  This is because it's now ambiguous as to which of these
+                // changes should be applied first.
+
                 var otherChange = intersectingSpans[0];
                 if (otherChange == change)
                 {
-                    // We're both inserting the same text at the same position.  This is safe.
+                    // We're both pure-inserting the same text at the same position.  
+                    // We assume this is a case of some provider making the same changes and
+                    // we allow this.
                     return true;
                 }
 
-                return !otherChange.Span.IsEmpty &&
+                return !IsPureInsertion(otherChange) &&
                        otherChange.Span.End == change.Span.Start;
             }
 
@@ -504,7 +527,7 @@ public virtual string GetFixAllTitle(FixAllState fixAllState)
             ArrayBuilder<TextChange> overlappingSpans,
             ArrayBuilder<TextChange> intersectingSpans)
         {
-            Debug.Assert(!change.Span.IsEmpty);
+            Debug.Assert(!IsPureInsertion(change));
 
             return !OverwriteChangeConflictsWithOverlappingSpans(change, overlappingSpans) &&
                    !OverwriteChangeConflictsWithIntersectingSpans(change, intersectingSpans);
@@ -514,39 +537,42 @@ public virtual string GetFixAllTitle(FixAllState fixAllState)
             TextChange change,
             ArrayBuilder<TextChange> overlappingSpans)
         {
-            Debug.Assert(!change.Span.IsEmpty);
+            Debug.Assert(!IsPureInsertion(change));
 
             if (overlappingSpans.Count == 0)
             {
-                // This change didn't overlap with any other changes.  This change is safe to make.
+                // This overwrite didn't overlap with any other changes.  This change is safe to make.
                 return false;
             }
 
             // The change we want to make overlapped an existing change we're making.  Only allow
             // this if there was a single overlap and we are exactly the same change as it.
             // Otherwise, this is a conflict.
-            return overlappingSpans.Count > 1 || overlappingSpans[0] != change;
+            var isSafe = overlappingSpans.Count == 1 && overlappingSpans[0] == change;
+
+            return !isSafe;
         }
 
         private static bool OverwriteChangeConflictsWithIntersectingSpans(
             TextChange change,
             ArrayBuilder<TextChange> intersectingSpans)
         {
-            Debug.Assert(!change.Span.IsEmpty);
+            Debug.Assert(!IsPureInsertion(change));
 
-            // We care about our intersections with *empty-span* changes.  Non-empty changes that
+            // We care about our intersections with pure-insertion changes.  Overwrite-changes that
             // we overlap are already handled in OverwriteChangeConflictsWithOverlappingSpans.
-            // And non-empty spans that we abut (i.e. which we're adjacent to) are totally safe 
+            // And overwrite spans that we abut (i.e. which we're adjacent to) are totally safe 
             // for both to be applied.
             //
-            // However, empty-span changes are extremely ambiguous. It is not possible to tell which
-            // change should be applied first.  So if we get any empty spans we have to bail
+            // However, pure-insertion changes are extremely ambiguous. It is not possible to tell which
+            // change should be applied first.  So if we get any pure-insertions we have to bail
             // on applying this span.
             foreach (var otherSpan in intersectingSpans)
             {
-                if (otherSpan.Span.IsEmpty)
+                if (IsPureInsertion(otherSpan))
                 {
-                    // If the other span is empty, then we definitely conflict with it.
+                    // Intersecting with a pure-insertion is too ambiguous, so we just consider
+                    // this a conflict.
                     return true;
                 }
             }