From 02273eb1ce4c2677609dd93dcbe6731fb1408135 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 23 Nov 2018 09:00:11 +0800 Subject: [PATCH 1/4] (GH-811) Fix folding for regions with same end token Previously the token matching was broken; ``` foreach ($1 in $2) { <----- STARTS MATCH HERE (1) $x = @{ <----- STARTS MATCH HERE (2) 'abc' = 'def' } <----- ENDS MATCH HERE (1) (2) } ``` This was caused by two or more different token pairs sharing the same end token. This commit modifies the token pair matching to take an array of Start Tokens instead of a single. This has the added benefit of performance increase too. This commit also adds tests for this scenario. --- .../Language/TokenOperations.cs | 28 ++++++++----------- .../Language/TokenOperationsTests.cs | 28 +++++++++++++++++++ 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/PowerShellEditorServices/Language/TokenOperations.cs b/src/PowerShellEditorServices/Language/TokenOperations.cs index 236c05f16..6ed140c9e 100644 --- a/src/PowerShellEditorServices/Language/TokenOperations.cs +++ b/src/PowerShellEditorServices/Language/TokenOperations.cs @@ -3,6 +3,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. // +using System; using System.Collections.Generic; using System.Management.Automation.Language; using System.Text.RegularExpressions; @@ -28,24 +29,17 @@ internal static FoldingReference[] FoldableRegions( { List foldableRegions = new List(); - // Find matching braces { -> } - foldableRegions.AddRange( - MatchTokenElements(tokens, TokenKind.LCurly, TokenKind.RCurly, RegionKindNone) - ); - - // Find matching braces ( -> ) - foldableRegions.AddRange( - MatchTokenElements(tokens, TokenKind.LParen, TokenKind.RParen, RegionKindNone) - ); - - // Find matching arrays @( -> ) + // Find matching braces { -> } + // Find matching hashes @{ -> } foldableRegions.AddRange( - MatchTokenElements(tokens, TokenKind.AtParen, TokenKind.RParen, RegionKindNone) + MatchTokenElements(tokens, new TokenKind[] { TokenKind.LCurly, TokenKind.AtCurly }, TokenKind.RCurly, RegionKindNone) ); - // Find matching hashes @{ -> } + // Find matching parentheses ( -> ) + // Find matching array literals @( -> ) + // Find matching subexpressions $( -> ) foldableRegions.AddRange( - MatchTokenElements(tokens, TokenKind.AtCurly, TokenKind.RParen, RegionKindNone) + MatchTokenElements(tokens, new TokenKind[] { TokenKind.LParen, TokenKind.AtParen, TokenKind.DollarParen }, TokenKind.RParen, RegionKindNone) ); // Find contiguous here strings @' -> '@ @@ -146,11 +140,11 @@ static private FoldingReference CreateFoldingReference( } /// - /// Given an array of tokens, find matching regions which start and end with a different TokenKind + /// Given an array of tokens, find matching regions which start (array of tokens) and end with a different TokenKind /// static private List MatchTokenElements( Token[] tokens, - TokenKind startTokenKind, + TokenKind[] startTokenKind, TokenKind endTokenKind, string matchKind) { @@ -158,7 +152,7 @@ static private List MatchTokenElements( Stack tokenStack = new Stack(); foreach (Token token in tokens) { - if (token.Kind == startTokenKind) { + if (Array.IndexOf(startTokenKind, token.Kind) != -1) { tokenStack.Push(token); } if ((tokenStack.Count > 0) && (token.Kind == endTokenKind)) { diff --git a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs index 9a192a666..ce16015a6 100644 --- a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs @@ -217,5 +217,33 @@ public void LaguageServiceFindsFoldablRegionsWithDuplicateRegions() { FoldingReference[] result = GetRegions(testString); AssertFoldingReferenceArrays(expectedFolds, result); } + + // This tests that token matching { -> }, @{ -> } and + // ( -> ), @( -> ) and $( -> ) does not confuse the folder + [Fact] + public void LaguageServiceFindsFoldablRegionsWithSameEndToken() { + string testString = +@"foreach ($1 in $2) { + + $x = @{ + 'abc' = 'def' + } +} + +$y = $( + $arr = @('1', '2'); Write-Host ($arr) +) +"; + FoldingReference[] expectedFolds = { + CreateFoldingReference(0, 19, 4, 1, null), + CreateFoldingReference(2, 9, 3, 5, null), + CreateFoldingReference(7, 5, 8, 1, null) + }; + + FoldingReference[] result = GetRegions(testString); + + AssertFoldingReferenceArrays(expectedFolds, result); + } + } } From 8ba0ed489b46b1c82426abdb896fe5b5f60cffed Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Sat, 8 Dec 2018 20:53:47 +0800 Subject: [PATCH 2/4] (GH-811) Add support for unopinionated variable name in folder Previously the code folder was not aware of unopinionated variable name assignments. This commit adds detection and an appropriate test. --- .../Language/TokenOperations.cs | 5 +++++ .../Language/TokenOperationsTests.cs | 12 +++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/PowerShellEditorServices/Language/TokenOperations.cs b/src/PowerShellEditorServices/Language/TokenOperations.cs index 6ed140c9e..42365e8ee 100644 --- a/src/PowerShellEditorServices/Language/TokenOperations.cs +++ b/src/PowerShellEditorServices/Language/TokenOperations.cs @@ -47,6 +47,11 @@ internal static FoldingReference[] FoldableRegions( MatchTokenElement(tokens, TokenKind.HereStringLiteral, RegionKindNone) ); + // Find unopinionated variable names ${ \n \n } + foldableRegions.AddRange( + MatchTokenElement(tokens, TokenKind.Variable, RegionKindNone) + ); + // Find contiguous here strings @" -> "@ foldableRegions.AddRange( MatchTokenElement(tokens, TokenKind.HereStringExpandable, RegionKindNone) diff --git a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs index ce16015a6..415e9af4a 100644 --- a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs @@ -111,7 +111,13 @@ double quoted herestrings should also fold # Comment Block 2 # Comment Block 2 $something = $true -#endregion Comment Block 3"; +#endregion Comment Block 3 + +# What about anonymous variable assignment +${this +is +valid} = 5 +"; private FoldingReference[] expectedAllInOneScriptFolds = { CreateFoldingReference(0, 0, 3, 10, "region"), CreateFoldingReference(1, 0, 2, 2, "comment"), @@ -127,7 +133,8 @@ double quoted herestrings should also fold CreateFoldingReference(56, 7, 58, 3, null), CreateFoldingReference(64, 0, 65, 0, "comment"), CreateFoldingReference(67, 0, 71, 26, "region"), - CreateFoldingReference(68, 0, 69, 0, "comment") + CreateFoldingReference(68, 0, 69, 0, "comment"), + CreateFoldingReference(75, 0, 76, 6, null), }; /// @@ -244,6 +251,5 @@ public void LaguageServiceFindsFoldablRegionsWithSameEndToken() { AssertFoldingReferenceArrays(expectedFolds, result); } - } } From af1213586166ed6bad30d8fa8cce7cc1a40e4cdc Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Sat, 8 Dec 2018 21:19:38 +0800 Subject: [PATCH 3/4] (GH-811) Add negative folding test for splatting This commit adds a splat command to the folding scenario to ensure that the folder is not confused by the (at) character for splatting. --- .../Language/TokenOperationsTests.cs | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs index 415e9af4a..3ce157837 100644 --- a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs @@ -67,6 +67,9 @@ herestrings should fold '@ +# This won't confuse things +Get-Command -Param @I + $I = @"" double quoted herestrings should also fold @@ -122,19 +125,19 @@ double quoted herestrings should also fold CreateFoldingReference(0, 0, 3, 10, "region"), CreateFoldingReference(1, 0, 2, 2, "comment"), CreateFoldingReference(10, 0, 14, 2, "comment"), - CreateFoldingReference(16, 30, 59, 1, null), + CreateFoldingReference(16, 30, 62, 1, null), CreateFoldingReference(17, 0, 21, 2, "comment"), CreateFoldingReference(23, 7, 25, 2, null), - CreateFoldingReference(28, 5, 30, 2, null), - CreateFoldingReference(35, 2, 36, 0, "comment"), - CreateFoldingReference(39, 2, 48, 14, "region"), - CreateFoldingReference(41, 4, 44, 14, "region"), - CreateFoldingReference(51, 7, 52, 3, null), - CreateFoldingReference(56, 7, 58, 3, null), - CreateFoldingReference(64, 0, 65, 0, "comment"), - CreateFoldingReference(67, 0, 71, 26, "region"), - CreateFoldingReference(68, 0, 69, 0, "comment"), - CreateFoldingReference(75, 0, 76, 6, null), + CreateFoldingReference(31, 5, 33, 2, null), + CreateFoldingReference(38, 2, 39, 0, "comment"), + CreateFoldingReference(42, 2, 51, 14, "region"), + CreateFoldingReference(44, 4, 47, 14, "region"), + CreateFoldingReference(54, 7, 55, 3, null), + CreateFoldingReference(59, 7, 61, 3, null), + CreateFoldingReference(67, 0, 68, 0, "comment"), + CreateFoldingReference(70, 0, 74, 26, "region"), + CreateFoldingReference(71, 0, 72, 0, "comment"), + CreateFoldingReference(78, 0, 79, 6, null), }; /// From c76e134f751aee3496c8852dbf3f3fbd4e9c6a1e Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 11 Dec 2018 21:50:36 -0800 Subject: [PATCH 4/4] Move arrays to static --- .../Language/TokenOperations.cs | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/PowerShellEditorServices/Language/TokenOperations.cs b/src/PowerShellEditorServices/Language/TokenOperations.cs index 42365e8ee..3003cd6e8 100644 --- a/src/PowerShellEditorServices/Language/TokenOperations.cs +++ b/src/PowerShellEditorServices/Language/TokenOperations.cs @@ -16,10 +16,26 @@ namespace Microsoft.PowerShell.EditorServices /// internal static class TokenOperations { + // Region kinds to align with VSCode's region kinds private const string RegionKindComment = "comment"; private const string RegionKindRegion = "region"; private const string RegionKindNone = null; + // Opening tokens for { } and @{ } + private static readonly TokenKind[] s_openingBraces = new [] + { + TokenKind.LCurly, + TokenKind.AtCurly + }; + + // Opening tokens for ( ), @( ), $( ) + private static readonly TokenKind[] s_openingParens = new [] + { + TokenKind.LParen, + TokenKind.AtParen, + TokenKind.DollarParen + }; + /// /// Extracts all of the unique foldable regions in a script given the list tokens /// @@ -32,14 +48,14 @@ internal static FoldingReference[] FoldableRegions( // Find matching braces { -> } // Find matching hashes @{ -> } foldableRegions.AddRange( - MatchTokenElements(tokens, new TokenKind[] { TokenKind.LCurly, TokenKind.AtCurly }, TokenKind.RCurly, RegionKindNone) + MatchTokenElements(tokens, s_openingBraces, TokenKind.RCurly, RegionKindNone) ); // Find matching parentheses ( -> ) // Find matching array literals @( -> ) // Find matching subexpressions $( -> ) foldableRegions.AddRange( - MatchTokenElements(tokens, new TokenKind[] { TokenKind.LParen, TokenKind.AtParen, TokenKind.DollarParen }, TokenKind.RParen, RegionKindNone) + MatchTokenElements(tokens, s_openingParens, TokenKind.RParen, RegionKindNone) ); // Find contiguous here strings @' -> '@