From 92a5324114cbcebbe9e4c2dda6404f1c683e798c Mon Sep 17 00:00:00 2001
From: Glenn Sarti <glennsarti@users.noreply.github.com>
Date: Wed, 12 Sep 2018 09:45:12 +0800
Subject: [PATCH] (GH-1523) Remove duplicate/overlapping folding regions

Previously the syntax folder returned an ordered list of folding ranges which
VS Code would "ignore" overlapping or duplicate ranges.  However on manual
testing, it showed that duplicate region did exist and could be folded/unfolded
using the "Fold All" and "Unfold All" commands, but could NOT be manipulated
manually in the editor using the +/- indicator.

This commit uses a filter which removes any duplicate or overlapping regions
which is easily detected via similar region start lines.  This commit also adds
a test for this scenario.
---
 src/features/Folding.ts             | 16 +++++++++-------
 test/features/folding.test.ts       | 15 +++++++++++++++
 test/fixtures/folding-duplicate.ps1 |  5 +++++
 3 files changed, 29 insertions(+), 7 deletions(-)
 create mode 100644 test/fixtures/folding-duplicate.ps1

diff --git a/src/features/Folding.ts b/src/features/Folding.ts
index 1a68b8c741..1e68f3b33b 100644
--- a/src/features/Folding.ts
+++ b/src/features/Folding.ts
@@ -238,8 +238,8 @@ export class FoldingProvider implements vscode.FoldingRangeProvider {
         // Sort the list of matched tokens, starting at the top of the document,
         // and ensure that, in the case of multiple ranges starting the same line,
         // that the largest range (i.e. most number of lines spanned) is sorted
-        // first.  This is needed as vscode will just ignore any duplicate folding
-        // ranges.
+        // first.  This is needed to detect duplicate regions. The first in the list
+        // will be used and subsequent duplicates ignored.
         foldableRegions.sort((a: LineNumberRange, b: LineNumberRange) => {
             // Initially look at the start line
             if (a.startline > b.startline) { return 1; }
@@ -252,11 +252,13 @@ export class FoldingProvider implements vscode.FoldingRangeProvider {
             return 0;
         });
 
-        // Convert the matched token list into a FoldingRange[]
-        const foldingRanges = [];
-        foldableRegions.forEach((item) => { foldingRanges.push(item.toFoldingRange()); });
-
-        return foldingRanges;
+        return foldableRegions
+            // It's possible to have duplicate or overlapping ranges, that is, regions which have the same starting
+            // line number as the previous region. Therefore only emit ranges which have a different starting line
+            // than the previous range.
+            .filter((item, index, src) => index === 0 || item.startline !== src[index - 1].startline)
+            // Convert the internal representation into the VSCode expected type
+            .map((item) => item.toFoldingRange());
     }
 
     /**
diff --git a/test/features/folding.test.ts b/test/features/folding.test.ts
index 583abfb85d..8f94f96c65 100644
--- a/test/features/folding.test.ts
+++ b/test/features/folding.test.ts
@@ -96,6 +96,21 @@ suite("Features", () => {
 
                 assertFoldingRegions(result, expectedMismatchedFoldingRegions);
             });
+
+            test("Does not return duplicate or overlapping regions", async () => {
+                const expectedMismatchedFoldingRegions = [
+                    { start: 1,  end: 2,  kind: null },
+                    { start: 2,  end: 4,  kind: null },
+                ];
+
+                // Integration test against the test fixture 'folding-mismatch.ps1' that contains
+                // duplicate/overlapping ranges due to the `(` and `{` characters
+                const uri = vscode.Uri.file(path.join(fixturePath, "folding-duplicate.ps1"));
+                const document = await vscode.workspace.openTextDocument(uri);
+                const result = await provider.provideFoldingRanges(document, null, null);
+
+                assertFoldingRegions(result, expectedMismatchedFoldingRegions);
+            });
         });
     });
 });
diff --git a/test/fixtures/folding-duplicate.ps1 b/test/fixtures/folding-duplicate.ps1
new file mode 100644
index 0000000000..0d2a62797a
--- /dev/null
+++ b/test/fixtures/folding-duplicate.ps1
@@ -0,0 +1,5 @@
+# This script causes duplicate/overlapping ranges due to the `(` and `{` characters
+$AnArray = @(Get-ChildItem -Path C:\ -Include *.ps1 -File).Where({
+    $_.FullName -ne 'foo'}).ForEach({
+        # Do Something
+})