Sort nodes by location before determining contiguous spans encompassed by these nodes.

CommonSyntaxNodeExtensions.GetContiguousSpans() was incorrectly assuming that the supplied nodes will be in the order in which they appear in source. This helper is used in the analyzer that produces diagnostics to grey out unused usings (i.e. to produce one diagnostic for a set of contiguous unused using directives).

Due to the above incorrect assumption, we would end up producing a single diagnostic spanning multiple unused usings sometimes and multiple diagnostics at other times. Often different invocations of the analyzer for the same source file would produce different set of unused usings diagnostics.

While this does not cause observable problems in regular usage - this is a problem for tests and led to flaky behavior where test would pass sometimes and fail at other times.

src/EditorFeatures/CSharpTest/Squiggles/ErrorSquiggleProducerTests.cs

@@ -74,15 +74,20 @@ public void SuggestionTagsForUnnecessaryCode()
 @"<Workspace>
     <Project Language=""C#"" CommonReferences=""true"">
         <Document FilePath = ""Test.cs"" >
-            using System.Collections; // Unused using.
-            class Program
-            {
+// System is used - rest are unused.
+using System.Collections;
+using System;
+using System.Diagnostics;
+using System.Collections.Generic;
+
+class Program
+{
     void Test()
     {
-                    System.Int32 x = 2; // Simplify type name.
+        Int32 x = 2; // Int32 can be simplified.
         x += 1;
     }
-            }
+}
         </Document>
     </Project>
 </Workspace>";
@@ -94,18 +99,30 @@ void Test()
                     ImmutableArray.Create<DiagnosticAnalyzer>(
                         new CSharpSimplifyTypeNamesDiagnosticAnalyzer(),
                         new CSharpRemoveUnnecessaryImportsDiagnosticAnalyzer()));
-                var spans = GetErrorSpans(workspace, analyzerMap.ToImmutable());
 
-                spans = spans.OrderBy(s => s.Span.Span.Start);
+                var spans = 
+                    GetErrorSpans(workspace, analyzerMap.ToImmutable())
+                        .OrderBy(s => s.Span.Span.Start).ToImmutableArray();
 
-                Assert.Equal(2, spans.Count());
-                var first = spans.First();
-                var second = spans.Last();
+                Assert.Equal(3, spans.Length);
+                var first = spans[0];
+                var second = spans[1];
+                var third = spans[2];
 
                 Assert.Equal(PredefinedErrorTypeNames.Suggestion, first.Tag.ErrorType);
                 Assert.Equal(CSharpFeaturesResources.RemoveUnnecessaryUsingsDiagnosticTitle, first.Tag.ToolTipContent);
+                Assert.Equal(40, first.Span.Start);
+                Assert.Equal(25, first.Span.Length);
+
                 Assert.Equal(PredefinedErrorTypeNames.Suggestion, second.Tag.ErrorType);
-                Assert.Equal(WorkspacesResources.NameCanBeSimplified, second.Tag.ToolTipContent);
+                Assert.Equal(CSharpFeaturesResources.RemoveUnnecessaryUsingsDiagnosticTitle, second.Tag.ToolTipContent);
+                Assert.Equal(82, second.Span.Start);
+                Assert.Equal(60, second.Span.Length);
+
+                Assert.Equal(PredefinedErrorTypeNames.Suggestion, third.Tag.ErrorType);
+                Assert.Equal(WorkspacesResources.NameCanBeSimplified, third.Tag.ToolTipContent);
+                Assert.Equal(196, third.Span.Start);
+                Assert.Equal(5, third.Span.Length);
             }
         }
 

src/EditorFeatures/VisualBasicTest/Squiggles/ErrorSquiggleProducerTests.vb

@@ -78,23 +78,32 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Squiggles
                         New VisualBasicRemoveUnnecessaryImportsDiagnosticAnalyzer()))
 
             Dim spans = ProduceSquiggles(analyzerMap.ToImmutable(),
-"Imports System.Collections ' Unused import.
+"
+' System.Diagnostics is used - rest are unused.
+Imports System.Diagnostics
+Imports System.Collections
+Imports System.Collections.Generic
+Imports System.Linq
+
 Class C1
     Sub Foo()
-        Dim x as System.Int32 = 2 ' Simplify type name.
-        x = x + 1
+        Process.Start(GetType(Int32).ToString()) 'Int32 can be simplified.
     End Sub
-End Class")
-            spans = spans.OrderBy(Function(s) s.Span.Span.Start)
+End Class").OrderBy(Function(s) s.Span.Span.Start).ToImmutableArray()
 
-            Assert.Equal(2, spans.Count())
-            Dim first = spans.First()
-            Dim second = spans.Last()
+            Assert.Equal(2, spans.Length)
+            Dim first = spans(0)
+            Dim second = spans(1)
 
             Assert.Equal(PredefinedErrorTypeNames.Suggestion, first.Tag.ErrorType)
             Assert.Equal(VBFeaturesResources.RemoveUnnecessaryImportsDiagnosticTitle, first.Tag.ToolTipContent)
+            Assert.Equal(79, first.Span.Start)
+            Assert.Equal(83, first.Span.Length)
+
             Assert.Equal(PredefinedErrorTypeNames.Suggestion, second.Tag.ErrorType)
             Assert.Equal(WorkspacesResources.NameCanBeSimplified, second.Tag.ToolTipContent)
+            Assert.Equal(221, second.Span.Start)
+            Assert.Equal(5, second.Span.Length)
         End Sub
     End Class
 End Namespace

src/Workspaces/Core/Portable/Shared/Extensions/CommonSyntaxNodeExtensions.cs

@@ -266,7 +266,9 @@ public static TextSpan GetContainedSpan(this IEnumerable<SyntaxNode> nodes)
         {
             SyntaxNode lastNode = null;
             TextSpan? textSpan = null;
-            foreach (var node in nodes)
+
+            // Sort the nodes in source location order.
+            foreach (var node in nodes.OrderBy(n => n.SpanStart))
             {
                 if (lastNode == null)
                 {