Merge pull request #21829 from CyrusNajmabadi/disabledTextOutlining

Properly collapse the right range when encountering nested #if regions.

src/EditorFeatures/CSharpTest/Structure/DisabledTextStructureTests.cs

@@ -111,6 +111,77 @@ class P {
     {
 #if Bar
        M();
+#endif
+        }|}
+#endif
+    }
+";
+
+            await VerifyBlockSpansAsync(code,
+                Region("span", CSharpStructureHelpers.Ellipsis, autoCollapse: true));
+        }
+
+        [WorkItem(459257, "https://devdiv.visualstudio.com/DevDiv/_workitems?id=459257")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Outlining)]
+        public async Task NestedDisabledCodePreProcessorDirectivesWithElseShouldCollapseEntireDisabledRegion()
+        {
+            const string code = @"
+class P {
+#if Goo
+{|span:    void $$M()
+    {
+#if Bar
+       M();
+#else
+
+#endif
+        }|}
+#endif
+    }
+";
+
+            await VerifyBlockSpansAsync(code,
+                Region("span", CSharpStructureHelpers.Ellipsis, autoCollapse: true));
+        }
+
+        [WorkItem(459257, "https://devdiv.visualstudio.com/DevDiv/_workitems?id=459257")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Outlining)]
+        public async Task NestedDisabledCodePreProcessorDirectivesWithElifShouldCollapseEntireDisabledRegion()
+        {
+            const string code = @"
+class P {
+#if Goo
+{|span:    void $$M()
+    {
+#if Bar
+       M();
+#elif Baz
+
+#endif
+        }|}
+#endif
+    }
+";
+
+            await VerifyBlockSpansAsync(code,
+                Region("span", CSharpStructureHelpers.Ellipsis, autoCollapse: true));
+        }
+
+        [WorkItem(459257, "https://devdiv.visualstudio.com/DevDiv/_workitems?id=459257")]
+        [Fact, Trait(Traits.Feature, Traits.Features.Outlining)]
+        public async Task NestedDisabledCodePreProcessorDirectivesWithElseAndElifShouldCollapseEntireDisabledRegion()
+        {
+            const string code = @"
+class P {
+#if Goo
+{|span:    void $$M()
+    {
+#if Bar
+       M();
+#else
+
+#elif Baz
+
 #endif
         }|}
 #endif

src/Features/CSharp/Portable/Structure/Providers/DisabledTextTriviaStructureProvider.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.Collections.Immutable;
 using System.Threading;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Structure;
@@ -25,17 +24,7 @@ internal class DisabledTextTriviaStructureProvider : AbstractSyntaxTriviaStructu
         {
             // We'll always be leading trivia of some token.
             var startPos = trivia.FullSpan.Start;
-            var endPos = trivia.FullSpan.End;
 
-            // Look through our parent token's trivia, to:
-            // 1. See if we're the first disabled trivia attached to the token.
-            // 2. To extend the span to the end of the last disabled trivia.
-            //
-            // The issue is that if there are other pre-processor directives (like #regions or
-            // #lines) mixed in the disabled code, they will be interleaved.  Keep walking past
-            // them to the next thing that will actually end a disabled block. When we encounter
-            // one, we must also consider which opening block they end. In case of nested pre-processor
-            // directives, the inner most end block should match the inner most open block and so on.
             var parentTriviaList = trivia.Token.LeadingTrivia;
             var indexInParent = parentTriviaList.IndexOf(trivia);
 
@@ -48,56 +37,95 @@ internal class DisabledTextTriviaStructureProvider : AbstractSyntaxTriviaStructu
                 indexInParent = parentTriviaList.IndexOf(trivia);
             }
 
-            if (indexInParent <= 0 ||
-                (!parentTriviaList[indexInParent - 1].IsKind(SyntaxKind.IfDirectiveTrivia) &&
+            if (indexInParent <= 0)
+            {
+                return;
+            }
+
+            if (!parentTriviaList[indexInParent - 1].IsKind(SyntaxKind.IfDirectiveTrivia) &&
                 !parentTriviaList[indexInParent - 1].IsKind(SyntaxKind.ElifDirectiveTrivia) &&
-                 !parentTriviaList[indexInParent - 1].IsKind(SyntaxKind.ElseDirectiveTrivia)))
+                !parentTriviaList[indexInParent - 1].IsKind(SyntaxKind.ElseDirectiveTrivia))
             {
                 return;
             }
 
+            var endTrivia = GetCorrespondingEndTrivia(trivia, parentTriviaList, indexInParent);
+            var endPos = GetEndPositionExludingLastNewLine(syntaxTree, endTrivia, cancellationToken);
+
+            var span = TextSpan.FromBounds(startPos, endPos);
+            spans.Add(new BlockSpan(
+                isCollapsible: true,
+                textSpan: span,
+                type: BlockTypes.PreprocessorRegion,
+                bannerText: CSharpStructureHelpers.Ellipsis,
+                autoCollapse: true));
+        }
+
+        private static int GetEndPositionExludingLastNewLine(SyntaxTree syntaxTree, SyntaxTrivia trivia, CancellationToken cancellationToken)
+        {
+            var endPos = trivia.FullSpan.End;
+            var text = syntaxTree.GetText(cancellationToken);
+            return endPos >= 2 && text[endPos - 1] == '\n' && text[endPos - 2] == '\r' ? endPos - 2 :
+                   endPos >= 1 && SyntaxFacts.IsNewLine(text[endPos - 1])              ? endPos - 1 : endPos;
+        }
+
+        private SyntaxTrivia GetCorrespondingEndTrivia(
+            SyntaxTrivia trivia, SyntaxTriviaList triviaList, int index)
+        {
+            // Look through our parent token's trivia, to extend the span to the end of the last
+            // disabled trivia.
+            //
+            // The issue is that if there are other pre-processor directives (like #regions or
+            // #lines) mixed in the disabled code, they will be interleaved.  Keep walking past
+            // them to the next thing that will actually end a disabled block. When we encounter
+            // one, we must also consider which opening block they end. In case of nested pre-processor
+            // directives, the inner most end block should match the inner most open block and so on.
+
             var nestedIfDirectiveTrivia = 0;
-            for (var i = indexInParent; i < parentTriviaList.Count; i++)
+            for (var i = index; i < triviaList.Count; i++)
             {
-                if (parentTriviaList[i].IsKind(SyntaxKind.IfDirectiveTrivia))
+                var currentTrivia = triviaList[i];
+                switch (currentTrivia.Kind())
                 {
+                    case SyntaxKind.IfDirectiveTrivia:
+                        // Hit a nested #if directive.  Keep track of this so we can ensure
+                        // that our actual disabled region reached the right end point.
                         nestedIfDirectiveTrivia++;
-                }
+                        continue;
 
-                if (parentTriviaList[i].IsKind(SyntaxKind.EndIfDirectiveTrivia) ||
-                    parentTriviaList[i].IsKind(SyntaxKind.ElifDirectiveTrivia) ||
-                    parentTriviaList[i].IsKind(SyntaxKind.ElseDirectiveTrivia))
-                {
+                    case SyntaxKind.EndIfDirectiveTrivia:
                         if (nestedIfDirectiveTrivia > 0)
                         {
+                            // This #endif corresponded to a nested #if, pop our stack
+                            // and keep searching.
                             nestedIfDirectiveTrivia--;
+                            continue;
                         }
-                    else
+
+                        // Found an #endif corresponding to our original #if/#elif/#else region we
+                        // started with. Mark up to the trivia before this as the range to collapse.
+                        return triviaList[i - 1];
+
+                    case SyntaxKind.ElseDirectiveTrivia:
+                    case SyntaxKind.ElifDirectiveTrivia:
+                        if (nestedIfDirectiveTrivia > 0)
                         {
-                        endPos = parentTriviaList[i - 1].FullSpan.End;
-                        break;
-                    }
-                }
+                            // This #else/#elif corresponded to a nested #if, ignore as
+                            // they're not relevant to the original construct we started 
+                            // on.
+                            continue;
                         }
 
-            // Now, exclude the last newline if there is one.
-            var text = syntaxTree.GetText(cancellationToken);
-            if (endPos > 1 && text[endPos - 1] == '\n' && text[endPos - 2] == '\r')
-            {
-                endPos -= 2;
+                        // We found the next #else/#elif corresponding to our original #if/#elif/#else
+                        // region we started with. Mark up to the trivia before this as the range
+                        // to collapse.
+                        return triviaList[i - 1];
                 }
-            else if (endPos > 0 && SyntaxFacts.IsNewLine(text[endPos - 1]))
-            {
-                endPos--;
             }
 
-            var span = TextSpan.FromBounds(startPos, endPos);
-            spans.Add(new BlockSpan(
-                isCollapsible: true,
-                textSpan: span,
-                type: BlockTypes.PreprocessorRegion,
-                bannerText: CSharpStructureHelpers.Ellipsis,
-                autoCollapse: true));
+            // Couldn't find a future trivia to collapse up to.  Just collapse the original 
+            // disabled text trivia we started with.
+            return trivia;
         }
     }
 }