From 2acd3aaddd70d9001746a6e8d40ae6a72db8f097 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 20 Mar 2018 16:17:57 -0700 Subject: [PATCH 1/4] Refactor pester script detection --- .../Symbols/PesterDocumentSymbolProvider.cs | 156 +++++++++++------- 1 file changed, 97 insertions(+), 59 deletions(-) diff --git a/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs b/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs index 32bd199ff..8ccc51778 100644 --- a/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs +++ b/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs @@ -27,63 +27,103 @@ IEnumerable IDocumentSymbolProvider.ProvideDocumentSymbols( return Enumerable.Empty(); } - var commandAsts = scriptFile.ScriptAst.FindAll(ast => - { - CommandAst commandAst = ast as CommandAst; + // Find plausible Pester commands + IEnumerable commandAsts = scriptFile.ScriptAst.FindAll(IsNamedCommandWithArguments, true); + + return commandAsts.OfType() + .Where(IsPesterCommand) + .Select(ast => ConvertPesterAstToSymbolReference(scriptFile, ast)) + .Where(pesterSymbol => pesterSymbol?.TestName != null); + } + + /// + /// Test if the given Ast is a regular CommandAst with arguments + /// + /// the PowerShell Ast to test + /// true if the Ast represents a PowerShell command with arguments, false otherwise + private static bool IsNamedCommandWithArguments(Ast ast) + { return - commandAst != null && + ast is CommandAst commandAst && commandAst.InvocationOperator != TokenKind.Dot && PesterSymbolReference.GetCommandType(commandAst.GetCommandName()).HasValue && commandAst.CommandElements.Count >= 2; - }, - true); + } - return commandAsts.Select( - ast => - { - // By this point we know the Ast is a CommandAst with 2 or more CommandElements - int testNameParamIndex = 1; - CommandAst testAst = (CommandAst)ast; + /// + /// Test whether the given CommandAst represents a Pester command + /// + /// the CommandAst to test + /// true if the CommandAst represents a Pester command, false otherwise + private static bool IsPesterCommand(CommandAst commandAst) + { + if (commandAst == null) + { + return false; + } - // The -Name parameter - for (int i = 1; i < testAst.CommandElements.Count; i++) - { - CommandParameterAst paramAst = testAst.CommandElements[i] as CommandParameterAst; - if (paramAst != null && - paramAst.ParameterName.Equals("Name", StringComparison.OrdinalIgnoreCase)) - { - testNameParamIndex = i + 1; - break; - } - } + // Ensure the first word is a Pester keyword + if (!(commandAst.CommandElements[0] is StringConstantExpressionAst pesterKeywordAst && + PesterSymbolReference.PesterKeywords.ContainsKey(pesterKeywordAst.Value))) + { + return false; + } - if (testNameParamIndex > testAst.CommandElements.Count - 1) - { - return null; - } + // Ensure that the last argument of the command is a scriptblock + if (!(commandAst.CommandElements[commandAst.CommandElements.Count-1] is ScriptBlockExpressionAst)) + { + return false; + } - StringConstantExpressionAst stringAst = - testAst.CommandElements[testNameParamIndex] as StringConstantExpressionAst; + return true; + } + + /// + /// Convert a CommandAst known to represent a Pester command and a reference to the scriptfile + /// it is in into symbol representing a Pester call for code lens + /// + /// the scriptfile the Pester call occurs in + /// the CommandAst representing the Pester call + /// a symbol representing the Pester call containing metadata for CodeLens to use + private static PesterSymbolReference ConvertPesterAstToSymbolReference(ScriptFile scriptFile, CommandAst pesterCommandAst) + { + string testLine = scriptFile.GetLine(pesterCommandAst.Extent.StartLineNumber); + string commandName = (pesterCommandAst.CommandElements[0] as StringConstantExpressionAst)?.Value; - if (stringAst == null) + // Search for a name for the test + string testName = null; + for (int i = 1; i < pesterCommandAst.CommandElements.Count; i++) + { + CommandElementAst currentCommandElement = pesterCommandAst.CommandElements[i]; + + // Check for an explicit "-Name" parameter + if (currentCommandElement is CommandParameterAst parameterAst) + { + i++; + if (parameterAst.ParameterName == "Name" && i < pesterCommandAst.CommandElements.Count) { - return null; + testName = (pesterCommandAst.CommandElements[i] as StringConstantExpressionAst)?.Value; + break; } + continue; + } - string testDefinitionLine = - scriptFile.GetLine( - ast.Extent.StartLineNumber); - - return - new PesterSymbolReference( - scriptFile, - testAst.GetCommandName(), - testDefinitionLine, - stringAst.Value, - ast.Extent); + // Otherwise, if an argument is given with no parameter, we assume it's the name + if (pesterCommandAst.CommandElements[i] is StringConstantExpressionAst testNameStrAst) + { + testName = testNameStrAst.Value; + break; + } + } - }).Where(s => s != null); + return new PesterSymbolReference( + scriptFile, + commandName, + testLine, + testName, + pesterCommandAst.Extent + ); } } @@ -114,6 +154,17 @@ public enum PesterCommandType /// public class PesterSymbolReference : SymbolReference { + /// + /// Lookup for Pester keywords we support. Ideally we could extract these from Pester itself + /// + internal static readonly IReadOnlyDictionary PesterKeywords = + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "Describe", PesterCommandType.Describe }, + { "Context", PesterCommandType.Context }, + { "It", PesterCommandType.It } + }; + private static char[] DefinitionTrimChars = new char[] { ' ', '{' }; /// @@ -145,25 +196,12 @@ internal PesterSymbolReference( internal static PesterCommandType? GetCommandType(string commandName) { - if (commandName == null) + PesterCommandType pesterCommandType; + if (!PesterKeywords.TryGetValue(commandName, out pesterCommandType)) { return null; } - - switch (commandName.ToLower()) - { - case "describe": - return PesterCommandType.Describe; - - case "context": - return PesterCommandType.Context; - - case "it": - return PesterCommandType.It; - - default: - return null; - } + return pesterCommandType; } } } From 872a5c6890507ff823032758d85cc25cca79199a Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 22 Mar 2018 11:07:04 -0700 Subject: [PATCH 2/4] Add null check --- .../Symbols/PesterDocumentSymbolProvider.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs b/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs index 8ccc51778..674b2f2cd 100644 --- a/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs +++ b/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs @@ -43,9 +43,10 @@ IEnumerable IDocumentSymbolProvider.ProvideDocumentSymbols( /// true if the Ast represents a PowerShell command with arguments, false otherwise private static bool IsNamedCommandWithArguments(Ast ast) { + CommandAst commandAst = ast as CommandAst; return - ast is CommandAst commandAst && + commandAst != null && commandAst.InvocationOperator != TokenKind.Dot && PesterSymbolReference.GetCommandType(commandAst.GetCommandName()).HasValue && commandAst.CommandElements.Count >= 2; From 6f5fdc3a0c907e10aeedf8f4a6581fb8b7955ce1 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 22 Mar 2018 20:17:31 -0700 Subject: [PATCH 3/4] Clean up command AST parsing --- .../Symbols/PesterDocumentSymbolProvider.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs b/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs index 674b2f2cd..8ef87f8ce 100644 --- a/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs +++ b/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs @@ -90,7 +90,7 @@ private static bool IsPesterCommand(CommandAst commandAst) private static PesterSymbolReference ConvertPesterAstToSymbolReference(ScriptFile scriptFile, CommandAst pesterCommandAst) { string testLine = scriptFile.GetLine(pesterCommandAst.Extent.StartLineNumber); - string commandName = (pesterCommandAst.CommandElements[0] as StringConstantExpressionAst)?.Value; + string commandName = pesterCommandAst.GetCommandName(); // Search for a name for the test string testName = null; @@ -159,12 +159,9 @@ public class PesterSymbolReference : SymbolReference /// Lookup for Pester keywords we support. Ideally we could extract these from Pester itself /// internal static readonly IReadOnlyDictionary PesterKeywords = - new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { "Describe", PesterCommandType.Describe }, - { "Context", PesterCommandType.Context }, - { "It", PesterCommandType.It } - }; + Enum.GetValues(typeof(PesterCommandType)) + .Cast() + .ToDictionary(pct => pct.ToString(), pct => pct); private static char[] DefinitionTrimChars = new char[] { ' ', '{' }; From 1e89761336fbd0290af75d5b66f5655313e1e55c Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 23 Mar 2018 14:48:08 -0700 Subject: [PATCH 4/4] Clean up AST usage, validate pester syntax better --- .../Symbols/PesterDocumentSymbolProvider.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs b/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs index 8ef87f8ce..eebb6095c 100644 --- a/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs +++ b/src/PowerShellEditorServices/Symbols/PesterDocumentSymbolProvider.cs @@ -65,8 +65,7 @@ private static bool IsPesterCommand(CommandAst commandAst) } // Ensure the first word is a Pester keyword - if (!(commandAst.CommandElements[0] is StringConstantExpressionAst pesterKeywordAst && - PesterSymbolReference.PesterKeywords.ContainsKey(pesterKeywordAst.Value))) + if (!PesterSymbolReference.PesterKeywords.ContainsKey(commandAst.GetCommandName())) { return false; } @@ -93,7 +92,9 @@ private static PesterSymbolReference ConvertPesterAstToSymbolReference(ScriptFil string commandName = pesterCommandAst.GetCommandName(); // Search for a name for the test + // If the test has more than one argument for names, we set it to null string testName = null; + bool alreadySawName = false; for (int i = 1; i < pesterCommandAst.CommandElements.Count; i++) { CommandElementAst currentCommandElement = pesterCommandAst.CommandElements[i]; @@ -104,17 +105,18 @@ private static PesterSymbolReference ConvertPesterAstToSymbolReference(ScriptFil i++; if (parameterAst.ParameterName == "Name" && i < pesterCommandAst.CommandElements.Count) { - testName = (pesterCommandAst.CommandElements[i] as StringConstantExpressionAst)?.Value; - break; + testName = alreadySawName ? null : (pesterCommandAst.CommandElements[i] as StringConstantExpressionAst)?.Value; + alreadySawName = true; } continue; } // Otherwise, if an argument is given with no parameter, we assume it's the name + // If we've already seen a name, we set the name to null if (pesterCommandAst.CommandElements[i] is StringConstantExpressionAst testNameStrAst) { - testName = testNameStrAst.Value; - break; + testName = alreadySawName ? null : testNameStrAst.Value; + alreadySawName = true; } }