From 4ac8ce5fa6a364280dc3742e34447f90fdb2033d Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 14 Dec 2018 20:50:57 +0800 Subject: [PATCH 1/5] (maint) Add tests for code folding for PowerShell classes Previously there were no tests for PowerShell classes. This commit adds a simple test for this scenario to ensure future changes do not break folding. --- .../Language/TokenOperationsTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs index 2b45cf531..320fc5158 100644 --- a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs @@ -260,5 +260,31 @@ public void LaguageServiceFindsFoldablRegionsWithSameEndToken() { AssertFoldingReferenceArrays(expectedFolds, result); } + + // A simple PowerShell Classes test + [Fact] + public void LaguageServiceFindsFoldablRegionsWithClasses() { + string testString = +@"class TestClass { + [string[]] $TestProperty = @( + 'first', + 'second', + 'third') + + [string] TestMethod() { + return $this.TestProperty[0] + } +} +"; + FoldingReference[] expectedFolds = { + CreateFoldingReference(0, 16, 8, 1, null), + CreateFoldingReference(1, 31, 3, 16, null), + CreateFoldingReference(6, 26, 7, 5, null) + }; + + FoldingReference[] result = GetRegions(testString); + + AssertFoldingReferenceArrays(expectedFolds, result); + } } } From 63a8a7c70265f82ba778999e653514ac8b78f0b5 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 14 Dec 2018 21:18:45 +0800 Subject: [PATCH 2/5] (GH-824) Refactor the FoldingReference arrays and lists into it's own class Previously the folding provider created many intermediate arrays and lists and required post-processing. This commit changes the behaviour to use an accumlator patter with an extended Dictionary class. This new class adds a `SafeAdd` method to add FoldingRanges, which then has the logic to determine if the range should indeed be added, for example, passing nulls or pre-existing larger ranges. By passing around this list using ByReference we can avoid creating many objects which are just then thrown away. This commit also moves the ShowLastLine code from the FoldingProvider into the Language Server. This reduces the number of array enumerations to one. --- .../Server/LanguageServer.cs | 23 ++-- .../Language/FoldingReference.cs | 36 ++++++ .../Language/TokenOperations.cs | 108 +++++------------- .../Language/TokenOperationsTests.cs | 72 +++++------- 4 files changed, 108 insertions(+), 131 deletions(-) diff --git a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs index a13f8ee11..4e2234ff4 100644 --- a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs +++ b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs @@ -535,7 +535,7 @@ private async Task HandleGetCommandRequestAsync( { PSCommand psCommand = new PSCommand(); if (!string.IsNullOrEmpty(param)) - { + { psCommand.AddCommand("Microsoft.PowerShell.Core\\Get-Command").AddArgument(param); } else @@ -1267,7 +1267,7 @@ protected async Task HandleCodeActionRequest( } } - // Add "show documentation" commands last so they appear at the bottom of the client UI. + // Add "show documentation" commands last so they appear at the bottom of the client UI. // These commands do not require code fixes. Sometimes we get a batch of diagnostics // to create commands for. No need to create multiple show doc commands for the same rule. var ruleNamesProcessed = new HashSet(); @@ -1390,17 +1390,18 @@ private FoldingRange[] Fold(string documentUri) if (!editorSession.Workspace.TryGetFile(documentUri, out scriptFile)) { return null; } var result = new List(); - FoldingReference[] foldableRegions = - TokenOperations.FoldableRegions(scriptFile.ScriptTokens, this.currentSettings.CodeFolding.ShowLastLine); - foreach (FoldingReference fold in foldableRegions) + // If we're showing the last line, decrement the Endline of all regions by one. + int endLineOffset = this.currentSettings.CodeFolding.ShowLastLine ? -1 : 0; + + foreach (KeyValuePair entry in TokenOperations.FoldableRegions(scriptFile.ScriptTokens)) { result.Add(new FoldingRange { - EndCharacter = fold.EndCharacter, - EndLine = fold.EndLine, - Kind = fold.Kind, - StartCharacter = fold.StartCharacter, - StartLine = fold.StartLine + EndCharacter = entry.Value.EndCharacter, + EndLine = entry.Value.EndLine + endLineOffset, + Kind = entry.Value.Kind, + StartCharacter = entry.Value.StartCharacter, + StartLine = entry.Value.StartLine }); } @@ -1744,7 +1745,7 @@ await eventSender( }); } - // Generate a unique id that is used as a key to look up the associated code action (code fix) when + // Generate a unique id that is used as a key to look up the associated code action (code fix) when // we receive and process the textDocument/codeAction message. private static string GetUniqueIdFromDiagnostic(Diagnostic diagnostic) { diff --git a/src/PowerShellEditorServices/Language/FoldingReference.cs b/src/PowerShellEditorServices/Language/FoldingReference.cs index 54e3401df..041245965 100644 --- a/src/PowerShellEditorServices/Language/FoldingReference.cs +++ b/src/PowerShellEditorServices/Language/FoldingReference.cs @@ -4,6 +4,7 @@ // using System; +using System.Collections.Generic; namespace Microsoft.PowerShell.EditorServices { @@ -60,4 +61,39 @@ public int CompareTo(FoldingReference that) { return string.Compare(this.Kind, that.Kind); } } + + /// + /// A class that holds a list of FoldingReferences and ensures that when adding a reference that the + /// folding rules are obeyed, e.g. Only one fold per start line + /// + public class FoldingReferenceList : Dictionary + { + /// + /// Adds a FoldingReference to the list and enforces ordering rules e.g. Only one fold per start line + /// + public void SafeAdd(FoldingReference item) + { + if (item == null) { return; } + + // Only add the item if it hasn't been seen before or it's the largest range + if (TryGetValue(item.StartLine, out FoldingReference currentItem)) + { + if (currentItem.CompareTo(item) == 1) { this[item.StartLine] = item; } + } + else + { + this[item.StartLine] = item; + } + } + + /// + /// Helper method to easily convert the Dictionary Values into an array + /// + public FoldingReference[] ToArray() + { + var result = new FoldingReference[Count]; + Values.CopyTo(result, 0); + return result; + } + } } diff --git a/src/PowerShellEditorServices/Language/TokenOperations.cs b/src/PowerShellEditorServices/Language/TokenOperations.cs index 3003cd6e8..fab49ce12 100644 --- a/src/PowerShellEditorServices/Language/TokenOperations.cs +++ b/src/PowerShellEditorServices/Language/TokenOperations.cs @@ -39,85 +39,39 @@ internal static class TokenOperations /// /// Extracts all of the unique foldable regions in a script given the list tokens /// - internal static FoldingReference[] FoldableRegions( - Token[] tokens, - bool ShowLastLine) + internal static FoldingReferenceList FoldableRegions( + Token[] tokens) { - List foldableRegions = new List(); + var refList = new FoldingReferenceList(); // Find matching braces { -> } // Find matching hashes @{ -> } - foldableRegions.AddRange( - MatchTokenElements(tokens, s_openingBraces, TokenKind.RCurly, RegionKindNone) - ); + MatchTokenElements(tokens, s_openingBraces, TokenKind.RCurly, RegionKindNone, ref refList); // Find matching parentheses ( -> ) // Find matching array literals @( -> ) // Find matching subexpressions $( -> ) - foldableRegions.AddRange( - MatchTokenElements(tokens, s_openingParens, TokenKind.RParen, RegionKindNone) - ); + MatchTokenElements(tokens, s_openingParens, TokenKind.RParen, RegionKindNone, ref refList); // Find contiguous here strings @' -> '@ - foldableRegions.AddRange( - MatchTokenElement(tokens, TokenKind.HereStringLiteral, RegionKindNone) - ); + MatchTokenElement(tokens, TokenKind.HereStringLiteral, RegionKindNone, ref refList); // Find unopinionated variable names ${ \n \n } - foldableRegions.AddRange( - MatchTokenElement(tokens, TokenKind.Variable, RegionKindNone) - ); + MatchTokenElement(tokens, TokenKind.Variable, RegionKindNone, ref refList); // Find contiguous here strings @" -> "@ - foldableRegions.AddRange( - MatchTokenElement(tokens, TokenKind.HereStringExpandable, RegionKindNone) - ); + MatchTokenElement(tokens, TokenKind.HereStringExpandable, RegionKindNone, ref refList); // Find matching comment regions #region -> #endregion - foldableRegions.AddRange( - MatchCustomCommentRegionTokenElements(tokens, RegionKindRegion) - ); + MatchCustomCommentRegionTokenElements(tokens, RegionKindRegion, ref refList); // Find blocks of line comments # comment1\n# comment2\n... - foldableRegions.AddRange( - MatchBlockCommentTokenElement(tokens, RegionKindComment) - ); + MatchBlockCommentTokenElement(tokens, RegionKindComment, ref refList); // Find comments regions <# -> #> - foldableRegions.AddRange( - MatchTokenElement(tokens, TokenKind.Comment, RegionKindComment) - ); - - // Remove any null entries. Nulls appear if the folding reference is invalid - // or missing - foldableRegions.RemoveAll(item => item == null); - - // Sort the FoldingReferences, 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 to detect duplicate regions. The first in the list - // will be used and subsequent duplicates ignored. - foldableRegions.Sort(); - - // 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. - foldableRegions.RemoveAll( (FoldingReference item) => { - // Note - I'm not happy with searching here, but as the RemoveAll - // doesn't expose the index in the List, we need to calculate it. Fortunately the - // list is sorted at this point, so we can use BinarySearch. - int index = foldableRegions.BinarySearch(item); - if (index == 0) { return false; } - return (item.StartLine == foldableRegions[index - 1].StartLine); - }); - - // Some editors have different folding UI, sometimes the lastline should be displayed - // If we do want to show the last line, just change the region to be one line less - if (ShowLastLine) { - foldableRegions.ForEach( item => { item.EndLine--; }); - } + MatchTokenElement(tokens, TokenKind.Comment, RegionKindComment, ref refList); - return foldableRegions.ToArray(); + return refList; } /// @@ -163,13 +117,13 @@ static private FoldingReference CreateFoldingReference( /// /// Given an array of tokens, find matching regions which start (array of tokens) and end with a different TokenKind /// - static private List MatchTokenElements( + static private void MatchTokenElements( Token[] tokens, TokenKind[] startTokenKind, TokenKind endTokenKind, - string matchKind) + string matchKind, + ref FoldingReferenceList refList) { - List result = new List(); Stack tokenStack = new Stack(); foreach (Token token in tokens) { @@ -177,28 +131,26 @@ static private List MatchTokenElements( tokenStack.Push(token); } if ((tokenStack.Count > 0) && (token.Kind == endTokenKind)) { - result.Add(CreateFoldingReference(tokenStack.Pop(), token, matchKind)); + refList.SafeAdd(CreateFoldingReference(tokenStack.Pop(), token, matchKind)); } } - return result; } /// /// Given an array of token, finds a specific token /// - static private List MatchTokenElement( + static private void MatchTokenElement( Token[] tokens, TokenKind tokenKind, - string matchKind) + string matchKind, + ref FoldingReferenceList refList) { - List result = new List(); foreach (Token token in tokens) { if ((token.Kind == tokenKind) && (token.Extent.StartLineNumber != token.Extent.EndLineNumber)) { - result.Add(CreateFoldingReference(token, token, matchKind)); + refList.SafeAdd(CreateFoldingReference(token, token, matchKind)); } } - return result; } /// @@ -230,11 +182,11 @@ static private bool IsBlockComment(int index, Token[] tokens) { /// classed as comments. To workaround this we search for valid block comments (See IsBlockCmment) /// and then determine contiguous line numbers from there /// - static private List MatchBlockCommentTokenElement( + static private void MatchBlockCommentTokenElement( Token[] tokens, - string matchKind) + string matchKind, + ref FoldingReferenceList refList) { - List result = new List(); Token startToken = null; int nextLine = -1; for (int index = 0; index < tokens.Length; index++) @@ -243,7 +195,7 @@ static private List MatchBlockCommentTokenElement( if (IsBlockComment(index, tokens) && s_nonRegionLineCommentRegex.IsMatch(thisToken.Text)) { int thisLine = thisToken.Extent.StartLineNumber - 1; if ((startToken != null) && (thisLine != nextLine)) { - result.Add(CreateFoldingReference(startToken, nextLine - 1, matchKind)); + refList.SafeAdd(CreateFoldingReference(startToken, nextLine - 1, matchKind)); startToken = thisToken; } if (startToken == null) { startToken = thisToken; } @@ -253,9 +205,8 @@ static private List MatchBlockCommentTokenElement( // If we exit the token array and we're still processing comment lines, then the // comment block simply ends at the end of document if (startToken != null) { - result.Add(CreateFoldingReference(startToken, nextLine - 1, matchKind)); + refList.SafeAdd(CreateFoldingReference(startToken, nextLine - 1, matchKind)); } - return result; } /// @@ -263,9 +214,10 @@ static private List MatchBlockCommentTokenElement( /// the comment text is either `# region` or `# endregion`, and then use a stack to determine /// the ranges they span /// - static private List MatchCustomCommentRegionTokenElements( + static private void MatchCustomCommentRegionTokenElements( Token[] tokens, - string matchKind) + string matchKind, + ref FoldingReferenceList refList) { // These regular expressions are used to match lines which mark the start and end of region comment in a PowerShell // script. They are based on the defaults in the VS Code Language Configuration at; @@ -273,7 +225,6 @@ static private List MatchCustomCommentRegionTokenElements( string startRegionText = @"^\s*#region\b"; string endRegionText = @"^\s*#endregion\b"; - List result = new List(); Stack tokenStack = new Stack(); for (int index = 0; index < tokens.Length; index++) { @@ -283,11 +234,10 @@ static private List MatchCustomCommentRegionTokenElements( tokenStack.Push(token); } if ((tokenStack.Count > 0) && (Regex.IsMatch(token.Text, endRegionText, RegexOptions.IgnoreCase))) { - result.Add(CreateFoldingReference(tokenStack.Pop(), token, matchKind)); + refList.SafeAdd(CreateFoldingReference(tokenStack.Pop(), token, matchKind)); } } } - return result; } } } diff --git a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs index 320fc5158..c7788a53a 100644 --- a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs @@ -13,13 +13,17 @@ public class TokenOperationsTests /// /// Helper method to create a stub script file and then call FoldableRegions /// - private FoldingReference[] GetRegions(string text, bool showLastLine = true) { + private FoldingReference[] GetRegions(string text) { ScriptFile scriptFile = new ScriptFile( "testfile", "clienttestfile", text, Version.Parse("5.0")); - return Microsoft.PowerShell.EditorServices.TokenOperations.FoldableRegions(scriptFile.ScriptTokens, showLastLine); + + var result = Microsoft.PowerShell.EditorServices.TokenOperations.FoldableRegions(scriptFile.ScriptTokens).ToArray(); + // The foldable regions need to be deterministic for testing so sort the array. + Array.Sort(result); + return result; } /// @@ -122,22 +126,22 @@ double quoted herestrings should also fold valid} = 5 "; private FoldingReference[] expectedAllInOneScriptFolds = { - CreateFoldingReference(0, 0, 3, 10, "region"), - CreateFoldingReference(1, 0, 2, 2, "comment"), - CreateFoldingReference(10, 0, 14, 2, "comment"), - CreateFoldingReference(16, 30, 62, 1, null), - CreateFoldingReference(17, 0, 21, 2, "comment"), - CreateFoldingReference(23, 7, 25, 2, 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), + CreateFoldingReference(0, 0, 4, 10, "region"), + CreateFoldingReference(1, 0, 3, 2, "comment"), + CreateFoldingReference(10, 0, 15, 2, "comment"), + CreateFoldingReference(16, 30, 63, 1, null), + CreateFoldingReference(17, 0, 22, 2, "comment"), + CreateFoldingReference(23, 7, 26, 2, null), + CreateFoldingReference(31, 5, 34, 2, null), + CreateFoldingReference(38, 2, 40, 0, "comment"), + CreateFoldingReference(42, 2, 52, 14, "region"), + CreateFoldingReference(44, 4, 48, 14, "region"), + CreateFoldingReference(54, 7, 56, 3, null), + CreateFoldingReference(59, 7, 62, 3, null), + CreateFoldingReference(67, 0, 69, 0, "comment"), + CreateFoldingReference(70, 0, 75, 26, "region"), + CreateFoldingReference(71, 0, 73, 0, "comment"), + CreateFoldingReference(78, 0, 80, 6, null), }; /// @@ -180,20 +184,6 @@ public void LaguageServiceFindsFoldablRegionsWithCRLF() { AssertFoldingReferenceArrays(expectedAllInOneScriptFolds, result); } - [Trait("Category", "Folding")] - [Fact] - public void LaguageServiceFindsFoldablRegionsWithoutLastLine() { - FoldingReference[] result = GetRegions(allInOneScript, false); - // Incrememnt the end line of the expected regions by one as we will - // be hiding the last line - FoldingReference[] expectedFolds = expectedAllInOneScriptFolds.Clone() as FoldingReference[]; - for (int index = 0; index < expectedFolds.Length; index++) - { - expectedFolds[index].EndLine++; - } - AssertFoldingReferenceArrays(expectedFolds, result); - } - [Trait("Category", "Folding")] [Fact] public void LaguageServiceFindsFoldablRegionsWithMismatchedRegions() { @@ -207,7 +197,7 @@ public void LaguageServiceFindsFoldablRegionsWithMismatchedRegions() { #region should not fold - mismatched "; FoldingReference[] expectedFolds = { - CreateFoldingReference(2, 0, 3, 10, "region") + CreateFoldingReference(2, 0, 4, 10, "region") }; FoldingReference[] result = GetRegions(testString); @@ -225,8 +215,8 @@ public void LaguageServiceFindsFoldablRegionsWithDuplicateRegions() { }) "; FoldingReference[] expectedFolds = { - CreateFoldingReference(1, 64, 1, 27, null), - CreateFoldingReference(2, 35, 3, 2, null) + CreateFoldingReference(1, 64, 2, 27, null), + CreateFoldingReference(2, 35, 4, 2, null) }; FoldingReference[] result = GetRegions(testString); @@ -251,9 +241,9 @@ public void LaguageServiceFindsFoldablRegionsWithSameEndToken() { ) "; FoldingReference[] expectedFolds = { - CreateFoldingReference(0, 19, 4, 1, null), - CreateFoldingReference(2, 9, 3, 5, null), - CreateFoldingReference(7, 5, 8, 1, null) + CreateFoldingReference(0, 19, 5, 1, null), + CreateFoldingReference(2, 9, 4, 5, null), + CreateFoldingReference(7, 5, 9, 1, null) }; FoldingReference[] result = GetRegions(testString); @@ -277,9 +267,9 @@ [string] TestMethod() { } "; FoldingReference[] expectedFolds = { - CreateFoldingReference(0, 16, 8, 1, null), - CreateFoldingReference(1, 31, 3, 16, null), - CreateFoldingReference(6, 26, 7, 5, null) + CreateFoldingReference(0, 16, 9, 1, null), + CreateFoldingReference(1, 31, 4, 16, null), + CreateFoldingReference(6, 26, 8, 5, null) }; FoldingReference[] result = GetRegions(testString); From 607274b693ab2ea7fc16a8f9f7d47d41866440c5 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 14 Dec 2018 21:41:55 +0800 Subject: [PATCH 3/5] (GH-824) Refactor Token Folding Operations Previously each token type detection was separated into discrete blocks to make reading the code easier. However this meant there were many enumerations of the Tokens array as well as passing around intermediate arrays/lists. This commit takes the individual methods and overlaps them to reduce the number of enumerations and regular expression matching to a minimum. Note that there are considerable code comments here due to the code now being more complex on initial review. --- .../Server/LanguageServer.cs | 12 +- .../Language/FoldingReference.cs | 25 +- .../Language/TokenOperations.cs | 265 ++++++++---------- .../Language/TokenOperationsTests.cs | 2 +- 4 files changed, 145 insertions(+), 159 deletions(-) diff --git a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs index 4e2234ff4..9f00ac7fa 100644 --- a/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs +++ b/src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs @@ -1394,14 +1394,14 @@ private FoldingRange[] Fold(string documentUri) // If we're showing the last line, decrement the Endline of all regions by one. int endLineOffset = this.currentSettings.CodeFolding.ShowLastLine ? -1 : 0; - foreach (KeyValuePair entry in TokenOperations.FoldableRegions(scriptFile.ScriptTokens)) + foreach (FoldingReference fold in TokenOperations.FoldableReferences(scriptFile.ScriptTokens).References) { result.Add(new FoldingRange { - EndCharacter = entry.Value.EndCharacter, - EndLine = entry.Value.EndLine + endLineOffset, - Kind = entry.Value.Kind, - StartCharacter = entry.Value.StartCharacter, - StartLine = entry.Value.StartLine + EndCharacter = fold.EndCharacter, + EndLine = fold.EndLine + endLineOffset, + Kind = fold.Kind, + StartCharacter = fold.StartCharacter, + StartLine = fold.StartLine }); } diff --git a/src/PowerShellEditorServices/Language/FoldingReference.cs b/src/PowerShellEditorServices/Language/FoldingReference.cs index 041245965..2b8a14502 100644 --- a/src/PowerShellEditorServices/Language/FoldingReference.cs +++ b/src/PowerShellEditorServices/Language/FoldingReference.cs @@ -66,8 +66,21 @@ public int CompareTo(FoldingReference that) { /// A class that holds a list of FoldingReferences and ensures that when adding a reference that the /// folding rules are obeyed, e.g. Only one fold per start line /// - public class FoldingReferenceList : Dictionary + public class FoldingReferenceList { + private readonly Dictionary references = new Dictionary(); + + /// + /// Return all references in the list + /// + public IEnumerable References + { + get + { + return references.Values; + } + } + /// /// Adds a FoldingReference to the list and enforces ordering rules e.g. Only one fold per start line /// @@ -76,13 +89,13 @@ public void SafeAdd(FoldingReference item) if (item == null) { return; } // Only add the item if it hasn't been seen before or it's the largest range - if (TryGetValue(item.StartLine, out FoldingReference currentItem)) + if (references.TryGetValue(item.StartLine, out FoldingReference currentItem)) { - if (currentItem.CompareTo(item) == 1) { this[item.StartLine] = item; } + if (currentItem.CompareTo(item) == 1) { references[item.StartLine] = item; } } else { - this[item.StartLine] = item; + references[item.StartLine] = item; } } @@ -91,8 +104,8 @@ public void SafeAdd(FoldingReference item) /// public FoldingReference[] ToArray() { - var result = new FoldingReference[Count]; - Values.CopyTo(result, 0); + var result = new FoldingReference[references.Count]; + references.Values.CopyTo(result, 0); return result; } } diff --git a/src/PowerShellEditorServices/Language/TokenOperations.cs b/src/PowerShellEditorServices/Language/TokenOperations.cs index fab49ce12..683ef7afb 100644 --- a/src/PowerShellEditorServices/Language/TokenOperations.cs +++ b/src/PowerShellEditorServices/Language/TokenOperations.cs @@ -21,55 +21,139 @@ internal static class TokenOperations 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 - }; + // These regular expressions are used to match lines which mark the start and end of region comment in a PowerShell + // script. They are based on the defaults in the VS Code Language Configuration at; + // https://github.com/Microsoft/vscode/blob/64186b0a26/extensions/powershell/language-configuration.json#L26-L31 + static private readonly Regex s_startRegionTextRegex = new Regex( + @"^\s*#region\b", + RegexOptions.IgnoreCase | RegexOptions.Compiled); + static private readonly Regex s_endRegionTextRegex = new Regex( + @"^\s*#endregion\b", + RegexOptions.IgnoreCase | RegexOptions.Compiled); /// /// Extracts all of the unique foldable regions in a script given the list tokens /// - internal static FoldingReferenceList FoldableRegions( + internal static FoldingReferenceList FoldableReferences( Token[] tokens) { var refList = new FoldingReferenceList(); - // Find matching braces { -> } - // Find matching hashes @{ -> } - MatchTokenElements(tokens, s_openingBraces, TokenKind.RCurly, RegionKindNone, ref refList); - - // Find matching parentheses ( -> ) - // Find matching array literals @( -> ) - // Find matching subexpressions $( -> ) - MatchTokenElements(tokens, s_openingParens, TokenKind.RParen, RegionKindNone, ref refList); + Stack tokenCurlyStack = new Stack(); + Stack tokenParenStack = new Stack(); + foreach (Token token in tokens) + { + switch (token.Kind) + { + // Find matching braces { -> } + // Find matching hashes @{ -> } + case TokenKind.LCurly: + case TokenKind.AtCurly: + tokenCurlyStack.Push(token); + break; + + case TokenKind.RCurly: + if (tokenCurlyStack.Count > 0) + { + refList.SafeAdd(CreateFoldingReference(tokenCurlyStack.Pop(), token, RegionKindNone)); + } + break; + + // Find matching parentheses ( -> ) + // Find matching array literals @( -> ) + // Find matching subexpressions $( -> ) + case TokenKind.LParen: + case TokenKind.AtParen: + case TokenKind.DollarParen: + tokenParenStack.Push(token); + break; + + case TokenKind.RParen: + if (tokenParenStack.Count > 0) + { + refList.SafeAdd(CreateFoldingReference(tokenParenStack.Pop(), token, RegionKindNone)); + } + break; + + // Find contiguous here strings @' -> '@ + // Find unopinionated variable names ${ \n \n } + // Find contiguous expandable here strings @" -> "@ + case TokenKind.HereStringLiteral: + case TokenKind.Variable: + case TokenKind.HereStringExpandable: + if (token.Extent.StartLineNumber != token.Extent.EndLineNumber) + { + refList.SafeAdd(CreateFoldingReference(token, token, RegionKindNone)); + } + break; + } + } - // Find contiguous here strings @' -> '@ - MatchTokenElement(tokens, TokenKind.HereStringLiteral, RegionKindNone, ref refList); + // Find matching comment regions #region -> #endregion + // Given a list of tokens, find the tokens that are comments and + // the comment text is either `#region` or `#endregion`, and then use a stack to determine + // the ranges they span + // + // Find blocks of line comments # comment1\n# comment2\n... + // Finding blocks of comment tokens is more complicated as the newline characters are not + // classed as comments. To workaround this we search for valid block comments (See IsBlockCmment) + // and then determine contiguous line numbers from there + // + // Find comments regions <# -> #> + // Match the token start and end of kind TokenKind.Comment + var tokenCommentRegionStack = new Stack(); + Token blockStartToken = null; + int blockNextLine = -1; - // Find unopinionated variable names ${ \n \n } - MatchTokenElement(tokens, TokenKind.Variable, RegionKindNone, ref refList); + for (int index = 0; index < tokens.Length; index++) + { + Token token = tokens[index]; + if (token.Kind != TokenKind.Comment) { continue; } + + // Processing for comment regions <# -> #> + if (token.Extent.StartLineNumber != token.Extent.EndLineNumber) + { + refList.SafeAdd(CreateFoldingReference(token, token, RegionKindComment)); + continue; + } - // Find contiguous here strings @" -> "@ - MatchTokenElement(tokens, TokenKind.HereStringExpandable, RegionKindNone, ref refList); + if (!IsBlockComment(index, tokens)) { continue; } - // Find matching comment regions #region -> #endregion - MatchCustomCommentRegionTokenElements(tokens, RegionKindRegion, ref refList); + // Regex's are very expensive. Use them sparingly! + // Processing for #region -> #endregion + if (s_startRegionTextRegex.IsMatch(token.Text)) + { + tokenCommentRegionStack.Push(token); + continue; + } + if (s_endRegionTextRegex.IsMatch(token.Text)) + { + // Mismatched regions in the script can cause bad stacks. + if (tokenCommentRegionStack.Count > 0) + { + refList.SafeAdd(CreateFoldingReference(tokenCommentRegionStack.Pop(), token, RegionKindRegion)); + } + continue; + } - // Find blocks of line comments # comment1\n# comment2\n... - MatchBlockCommentTokenElement(tokens, RegionKindComment, ref refList); + // If it's neither a start or end region then it could be block line comment + // Processing for blocks of line comments # comment1\n# comment2\n... + int thisLine = token.Extent.StartLineNumber - 1; + if ((blockStartToken != null) && (thisLine != blockNextLine)) + { + refList.SafeAdd(CreateFoldingReference(blockStartToken, blockNextLine - 1, RegionKindComment)); + blockStartToken = token; + } + if (blockStartToken == null) { blockStartToken = token; } + blockNextLine = thisLine + 1; + } - // Find comments regions <# -> #> - MatchTokenElement(tokens, TokenKind.Comment, RegionKindComment, ref refList); + // If we exit the token array and we're still processing comment lines, then the + // comment block simply ends at the end of document + if (blockStartToken != null) + { + refList.SafeAdd(CreateFoldingReference(blockStartToken, blockNextLine - 1, RegionKindComment)); + } return refList; } @@ -114,45 +198,6 @@ static private FoldingReference CreateFoldingReference( }; } - /// - /// Given an array of tokens, find matching regions which start (array of tokens) and end with a different TokenKind - /// - static private void MatchTokenElements( - Token[] tokens, - TokenKind[] startTokenKind, - TokenKind endTokenKind, - string matchKind, - ref FoldingReferenceList refList) - { - Stack tokenStack = new Stack(); - foreach (Token token in tokens) - { - if (Array.IndexOf(startTokenKind, token.Kind) != -1) { - tokenStack.Push(token); - } - if ((tokenStack.Count > 0) && (token.Kind == endTokenKind)) { - refList.SafeAdd(CreateFoldingReference(tokenStack.Pop(), token, matchKind)); - } - } - } - - /// - /// Given an array of token, finds a specific token - /// - static private void MatchTokenElement( - Token[] tokens, - TokenKind tokenKind, - string matchKind, - ref FoldingReferenceList refList) - { - foreach (Token token in tokens) - { - if ((token.Kind == tokenKind) && (token.Extent.StartLineNumber != token.Extent.EndLineNumber)) { - refList.SafeAdd(CreateFoldingReference(token, token, matchKind)); - } - } - } - /// /// Returns true if a Token is a block comment; /// - Must be a TokenKind.comment @@ -167,77 +212,5 @@ static private bool IsBlockComment(int index, Token[] tokens) { if (tokens[index - 1].Kind != TokenKind.NewLine) { return false; } return thisToken.Text.StartsWith("#"); } - - // This regular expressions is used to detect a line comment (as opposed to an inline comment), that is not a region - // block directive i.e. - // - No text between the beginning of the line and `#` - // - Comment does start with region - // - Comment does start with endregion - static private readonly Regex s_nonRegionLineCommentRegex = new Regex( - @"\s*#(?!region\b|endregion\b)", - RegexOptions.IgnoreCase | RegexOptions.Compiled); - - /// - /// Finding blocks of comment tokens is more complicated as the newline characters are not - /// classed as comments. To workaround this we search for valid block comments (See IsBlockCmment) - /// and then determine contiguous line numbers from there - /// - static private void MatchBlockCommentTokenElement( - Token[] tokens, - string matchKind, - ref FoldingReferenceList refList) - { - Token startToken = null; - int nextLine = -1; - for (int index = 0; index < tokens.Length; index++) - { - Token thisToken = tokens[index]; - if (IsBlockComment(index, tokens) && s_nonRegionLineCommentRegex.IsMatch(thisToken.Text)) { - int thisLine = thisToken.Extent.StartLineNumber - 1; - if ((startToken != null) && (thisLine != nextLine)) { - refList.SafeAdd(CreateFoldingReference(startToken, nextLine - 1, matchKind)); - startToken = thisToken; - } - if (startToken == null) { startToken = thisToken; } - nextLine = thisLine + 1; - } - } - // If we exit the token array and we're still processing comment lines, then the - // comment block simply ends at the end of document - if (startToken != null) { - refList.SafeAdd(CreateFoldingReference(startToken, nextLine - 1, matchKind)); - } - } - - /// - /// Given a list of tokens, find the tokens that are comments and - /// the comment text is either `# region` or `# endregion`, and then use a stack to determine - /// the ranges they span - /// - static private void MatchCustomCommentRegionTokenElements( - Token[] tokens, - string matchKind, - ref FoldingReferenceList refList) - { - // These regular expressions are used to match lines which mark the start and end of region comment in a PowerShell - // script. They are based on the defaults in the VS Code Language Configuration at; - // https://github.com/Microsoft/vscode/blob/64186b0a26/extensions/powershell/language-configuration.json#L26-L31 - string startRegionText = @"^\s*#region\b"; - string endRegionText = @"^\s*#endregion\b"; - - Stack tokenStack = new Stack(); - for (int index = 0; index < tokens.Length; index++) - { - if (IsBlockComment(index, tokens)) { - Token token = tokens[index]; - if (Regex.IsMatch(token.Text, startRegionText, RegexOptions.IgnoreCase)) { - tokenStack.Push(token); - } - if ((tokenStack.Count > 0) && (Regex.IsMatch(token.Text, endRegionText, RegexOptions.IgnoreCase))) { - refList.SafeAdd(CreateFoldingReference(tokenStack.Pop(), token, matchKind)); - } - } - } - } } } diff --git a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs index c7788a53a..b1950054a 100644 --- a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs @@ -20,7 +20,7 @@ private FoldingReference[] GetRegions(string text) { text, Version.Parse("5.0")); - var result = Microsoft.PowerShell.EditorServices.TokenOperations.FoldableRegions(scriptFile.ScriptTokens).ToArray(); + var result = Microsoft.PowerShell.EditorServices.TokenOperations.FoldableReferences(scriptFile.ScriptTokens).ToArray(); // The foldable regions need to be deterministic for testing so sort the array. Array.Sort(result); return result; From d7e335a3366059acf36209c51e0a24f4878a8991 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 14 Dec 2018 21:56:46 +0800 Subject: [PATCH 4/5] (GH-812) Update folder for DSC style scripts Previously the code folding was not tested against DSC configuration scripts. This commit adds tests for a sample DSC script to ensure the folding occurs at the correct places --- .../Language/TokenOperationsTests.cs | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs index b1950054a..3008d3206 100644 --- a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs @@ -276,5 +276,57 @@ [string] TestMethod() { AssertFoldingReferenceArrays(expectedFolds, result); } + + // This tests DSC style keywords and param blocks + [Fact] + public void LaguageServiceFindsFoldablRegionsWithDSC() { + string testString = +@"Configuration Example +{ + param + ( + [Parameter()] + [System.String[]] + $NodeName = 'localhost', + + [Parameter(Mandatory = $true)] + [ValidateNotNullorEmpty()] + [System.Management.Automation.PSCredential] + $Credential + ) + + Import-DscResource -Module ActiveDirectoryCSDsc + + Node $AllNodes.NodeName + { + WindowsFeature ADCS-Cert-Authority + { + Ensure = 'Present' + Name = 'ADCS-Cert-Authority' + } + + AdcsCertificationAuthority CertificateAuthority + { + IsSingleInstance = 'Yes' + Ensure = 'Present' + Credential = $Credential + CAType = 'EnterpriseRootCA' + DependsOn = '[WindowsFeature]ADCS-Cert-Authority' + } + } +} +"; + FoldingReference[] expectedFolds = { + CreateFoldingReference(1, 0, 33, 1, null), + CreateFoldingReference(3, 4, 12, 5, null), + CreateFoldingReference(17, 4, 32, 5, null), + CreateFoldingReference(19, 8, 22, 9, null), + CreateFoldingReference(25, 8, 31, 9, null) + }; + + FoldingReference[] result = GetRegions(testString); + + AssertFoldingReferenceArrays(expectedFolds, result); + } } } From 4ce90d9e8c33febe56355f9f1771f56746ba8aba Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Sun, 16 Dec 2018 20:15:17 +0800 Subject: [PATCH 5/5] (GH-824) More strict block comment folding detection Previously the folder would search for the region markers without case sensitivity. This commit modifies the regular expressions to be more strict on the what is a region marker and adds a negative test to ensure that regions that are not cased correctly are not folded . --- src/PowerShellEditorServices/Language/TokenOperations.cs | 7 +++---- .../Language/TokenOperationsTests.cs | 8 ++++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/PowerShellEditorServices/Language/TokenOperations.cs b/src/PowerShellEditorServices/Language/TokenOperations.cs index 683ef7afb..e62dca68c 100644 --- a/src/PowerShellEditorServices/Language/TokenOperations.cs +++ b/src/PowerShellEditorServices/Language/TokenOperations.cs @@ -24,12 +24,11 @@ internal static class TokenOperations // These regular expressions are used to match lines which mark the start and end of region comment in a PowerShell // script. They are based on the defaults in the VS Code Language Configuration at; // https://github.com/Microsoft/vscode/blob/64186b0a26/extensions/powershell/language-configuration.json#L26-L31 + // https://github.com/Microsoft/vscode/issues/49070 static private readonly Regex s_startRegionTextRegex = new Regex( - @"^\s*#region\b", - RegexOptions.IgnoreCase | RegexOptions.Compiled); + @"^\s*#[rR]egion\b", RegexOptions.Compiled); static private readonly Regex s_endRegionTextRegex = new Regex( - @"^\s*#endregion\b", - RegexOptions.IgnoreCase | RegexOptions.Compiled); + @"^\s*#[eE]nd[rR]egion\b", RegexOptions.Compiled); /// /// Extracts all of the unique foldable regions in a script given the list tokens diff --git a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs index 3008d3206..38a62993b 100644 --- a/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs +++ b/test/PowerShellEditorServices.Test/Language/TokenOperationsTests.cs @@ -43,11 +43,11 @@ private static FoldingReference CreateFoldingReference(int startLine, int startC // folding regions and regions which should not be // detected. Due to file encoding this could be CLRF or LF line endings private const string allInOneScript = -@"#RegIon This should fold +@"#Region This should fold <# Nested different comment types. This should fold #> -#EnDReGion +#EndRegion # region This should not fold due to whitespace $shouldFold = $false @@ -124,6 +124,10 @@ double quoted herestrings should also fold ${this is valid} = 5 + +#RegIon This should fold due to casing +$foo = 'bar' +#EnDReGion "; private FoldingReference[] expectedAllInOneScriptFolds = { CreateFoldingReference(0, 0, 4, 10, "region"),