(GH-1417) Fix code folding on CRLF documents (#1418)

* (GH-1417) Tokenize a document per line to avoid line endings

Previously the folding provider would tokenize the entire document at a time.
While this mostly works, it causes issues with regular expressions which use the
`$` anchor.  This anchor will interpret CRLF vs LF differently. While it may be
possible to munge the document prior to tokenization, the prior art within the
VS Code codebase shows that this is not the intended usage, i.e. lines should be
tokenized, not an entire document.  This commit changes the tokenization process
to tokenize per line but still preserves the original folding behaviour.

* (GH-1417) Add tests for CRLF documents

Previously the test fixtures were failing on some Windows systems due to git
changing the line endings.  This commit adds a new test fixture, and runs the
same tests on the same content document, but each with a different line-ending
(CRLF and LF).

* (maint) Update folding test fixtures

Previously some of the text in the test fixtures was confusing e.g. Text that
says 'cannot fold' should read 'should fold'.  This commit updates the text in
the fixture to describe the expected behaviour for humans.

* (GH-1417) Add folding tests for double quote here strings

Previously only single quoted here strings were tested for folding.  This commit
adds a test for the double quoted herestrings as well.

* (maint) Refactor folding test fixtures

Previously there was some duplication in the folder test fixtures.  This commit
refactors the expectation to be more DRY.  This commit also adds an expectation
on the line endings as we're depending on git to clone correctly.  Without this
we may get false positive results.

Author: glennsarti

src/features/Folding.ts

@@ -8,6 +8,7 @@ import * as vscode from "vscode";
 import {
     DocumentSelector,
     LanguageClient,
+    Position,
 } from "vscode-languageclient";
 import { IFeature } from "../feature";
 import { ILogger } from "../logging";
@@ -193,8 +194,24 @@ export class FoldingProvider implements vscode.FoldingRangeProvider {
         // If the grammar hasn't been setup correctly, return empty result
         if (this.powershellGrammar == null) { return []; }
 
-        // Convert the document text into a series of grammar tokens
-        const tokens: ITokenList = this.powershellGrammar.tokenizeLine(document.getText(), null).tokens;
+        // Tokenize each line and build up an array of document-wide tokens
+        // Note that line endings (CRLF/LF/CR) have interpolation issues so don't
+        // tokenize an entire document if the line endings are variable.
+        const tokens: ITokenList = new Array<IToken>();
+        let tokenizationState = null;
+        for (let i = 0; i < document.lineCount; i++) {
+            const result = this.powershellGrammar.tokenizeLine(document.lineAt(i).text, tokenizationState);
+            const offset = document.offsetAt(new vscode.Position(i, 0)) ;
+
+            for (const item of result.tokens) {
+                // Add the offset of the line to translate a character offset into
+                // a document based index
+                item.startIndex += offset;
+                item.endIndex += offset;
+                tokens.push(item);
+            }
+            tokenizationState = result.ruleStack;
+        }
 
         // Parse the token list looking for matching tokens and return
         // a list of LineNumberRange objects.  Then filter the list and only return matches

test/features/folding.test.ts

@@ -12,14 +12,14 @@ import { MockLogger } from "../test_utils";
 const fixturePath = path.join(__dirname, "..", "..", "..", "test", "fixtures");
 
 function assertFoldingRegions(result, expected): void {
-    assert.equal(result.length, expected.length);
-
     for (let i = 0; i < expected.length; i++) {
         const failMessage = `expected ${JSON.stringify(expected[i])}, actual ${JSON.stringify(result[i])}`;
         assert.equal(result[i].start, expected[i].start, failMessage);
         assert.equal(result[i].end, expected[i].end, failMessage);
         assert.equal(result[i].kind, expected[i].kind, failMessage);
     }
+
+    assert.equal(result.length, expected.length);
 }
 
 suite("Features", () => {
@@ -36,26 +36,47 @@ suite("Features", () => {
             assert.notEqual(psGrammar, null);
         });
 
-        test("Can detect all of the foldable regions in a document", async () => {
-            // Integration test against the test fixture 'folding.ps1' that contains
-            // all of the different types of folding available
-            const uri = vscode.Uri.file(path.join(fixturePath, "folding.ps1"));
-            const document = await vscode.workspace.openTextDocument(uri);
-            const result = await provider.provideFoldingRanges(document, null, null);
-
-            const expected = [
+        suite("For a single document", async () => {
+            const expectedFoldingRegions = [
                 { start: 1,  end: 6,  kind: 1 },
-                { start: 7,  end: 46, kind: 3 },
+                { start: 7,  end: 51, kind: 3 },
                 { start: 8,  end: 13, kind: 1 },
                 { start: 14, end: 17, kind: 3 },
-                { start: 21, end: 23, kind: 1 },
-                { start: 25, end: 35, kind: 3 },
-                { start: 27, end: 31, kind: 3 },
-                { start: 37, end: 39, kind: 3 },
-                { start: 42, end: 45, kind: 3 },
+                { start: 19, end: 22, kind: 3 },
+                { start: 26, end: 28, kind: 1 },
+                { start: 30, end: 40, kind: 3 },
+                { start: 32, end: 36, kind: 3 },
+                { start: 42, end: 44, kind: 3 },
+                { start: 47, end: 50, kind: 3 },
             ];
 
-            assertFoldingRegions(result, expected);
+            test("Can detect all of the foldable regions in a document with CRLF line endings", async () => {
+                // Integration test against the test fixture 'folding-crlf.ps1' that contains
+                // all of the different types of folding available
+                const uri = vscode.Uri.file(path.join(fixturePath, "folding-crlf.ps1"));
+                const document = await vscode.workspace.openTextDocument(uri);
+                const result = await provider.provideFoldingRanges(document, null, null);
+
+                // Ensure we have CRLF line endings as we're depending on git
+                // to clone the test fixtures correctly
+                assert.notEqual(document.getText().indexOf("\r\n"), -1);
+
+                assertFoldingRegions(result, expectedFoldingRegions);
+            });
+
+            test("Can detect all of the foldable regions in a document with LF line endings", async () => {
+                // Integration test against the test fixture 'folding-lf.ps1' that contains
+                // all of the different types of folding available
+                const uri = vscode.Uri.file(path.join(fixturePath, "folding-lf.ps1"));
+                const document = await vscode.workspace.openTextDocument(uri);
+                const result = await provider.provideFoldingRanges(document, null, null);
+
+                // Ensure we do not CRLF line endings as we're depending on git
+                // to clone the test fixtures correctly
+                assert.equal(document.getText().indexOf("\r\n"), -1);
+
+                assertFoldingRegions(result, expectedFoldingRegions);
+            });
         });
     });
 });

test/fixtures/.gitattributes

@@ -0,0 +1,8 @@
+# Set the default behavior, in case people don't have core.autocrlf set.
+* text=auto
+
+# These test fixtures require crlf
+folding-crlf.ps1  text eol=crlf
+
+# These test fixtures require lf
+folding-lf.ps1    text eol=lf

test/fixtures/folding-crlf.ps1

@@ -0,0 +1,52 @@
+function short-func-not-fold {};
+<#
+.SYNOPSIS
+  This whole comment block should fold, not just the SYNOPSIS
+.EXAMPLE
+  This whole comment block should fold, not just the EXAMPLE
+#>
+function New-VSCodeShouldFold {
+<#
+.SYNOPSIS
+  This whole comment block should fold, not just the SYNOPSIS
+.EXAMPLE
+  This whole comment block should fold, not just the EXAMPLE
+#>
+  $I = @'
+herestrings should fold
+
+'@
+
+$I = @"
+double quoted herestrings should also fold
+
+"@
+
+  # this won't be folded
+
+  # This block of comments should be foldable as a single block
+  # This block of comments should be foldable as a single block
+  # This block of comments should be foldable as a single block
+
+  #region This fools the indentation folding.
+  Write-Host "Hello"
+    # region Nested regions should be foldable
+    Write-Host "Hello"
+    # comment1
+    Write-Host "Hello"
+    #endregion
+    Write-Host "Hello"
+    # comment2
+    Write-Host "Hello"
+    # endregion
+
+  $c = {
+    Write-Host "Script blocks should be foldable"
+  }
+
+  # Array fools indentation folding
+  $d = @(
+  'should fold1',
+  'should fold2'
+  )
+}

test/fixtures/folding-lf.ps1

@@ -0,0 +1,52 @@
+function short-func-not-fold {};
+<#
+.SYNOPSIS
+  This whole comment block should fold, not just the SYNOPSIS
+.EXAMPLE
+  This whole comment block should fold, not just the EXAMPLE
+#>
+function New-VSCodeShouldFold {
+<#
+.SYNOPSIS
+  This whole comment block should fold, not just the SYNOPSIS
+.EXAMPLE
+  This whole comment block should fold, not just the EXAMPLE
+#>
+  $I = @'
+herestrings should fold
+
+'@
+
+$I = @"
+double quoted herestrings should also fold
+
+"@
+
+  # this won't be folded
+
+  # This block of comments should be foldable as a single block
+  # This block of comments should be foldable as a single block
+  # This block of comments should be foldable as a single block
+
+  #region This fools the indentation folding.
+  Write-Host "Hello"
+    # region Nested regions should be foldable
+    Write-Host "Hello"
+    # comment1
+    Write-Host "Hello"
+    #endregion
+    Write-Host "Hello"
+    # comment2
+    Write-Host "Hello"
+    # endregion
+
+  $c = {
+    Write-Host "Script blocks should be foldable"
+  }
+
+  # Array fools indentation folding
+  $d = @(
+  'should fold1',
+  'should fold2'
+  )
+}