Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upIntroduce Get-EditorServicesParserAst to expand on ExpandAlias #1199
Conversation
|
Issues
======
- Added 1
Complexity increasing per file
==============================
- src/PowerShellEditorServices.Hosting/Commands/GetEditorServicesParserAst.cs 3
See the complete overview on Codacy |
|
|
||
| protected override void EndProcessing() | ||
| { | ||
| var errors = new Collection<PSParseError>(); |
| { | ||
| if (PSTokenType == PSTokenType.Command) | ||
| { | ||
| var result = SessionState.InvokeCommand.GetCommand(token.Content, CommandType); |
TylerLeonhardt
Feb 18, 2020
Member
You've done a great job of converting the code from PowerShell to C# that I'm wondering if you could put this code in the Handler below instead of in its own cmdlet... then the only thing you'd need to do in script is the call to Get-Command (by using _powerShellContextService.ExecuteCommandAsync) and that would be ConstrainedLanguage mode compliant.
Also, Get-Command takes in an array for -Name so you should be able to get the names in a foreach, throw them into an array, and then pass that into Get-Command so we only invoke PowerShell once.
You've done a great job of converting the code from PowerShell to C# that I'm wondering if you could put this code in the Handler below instead of in its own cmdlet... then the only thing you'd need to do in script is the call to Get-Command (by using _powerShellContextService.ExecuteCommandAsync) and that would be ConstrainedLanguage mode compliant.
Also, Get-Command takes in an array for -Name so you should be able to get the names in a foreach, throw them into an array, and then pass that into Get-Command so we only invoke PowerShell once.
rjmholt
Feb 18, 2020
•
Member
Basically rather than implementing the cmdlet, you can do something like this:
ScriptBlockAst scriptAst = Parser.ParseInput(request.Text, out Token[] _, out ParseError[] _);
var commandsToExpand = new List<string>();
foreach (Ast foundAst in scriptAst.FindAll(ast => ast is CommandAst, true))
{
if (!(foundAst is CommandAst commandAst))
{
continue;
}
string commandName = commandAst.GetCommandName();
if (commandName != null)
{
commandsToExpand.Add(commandName);
}
}
var psCommand = new PSCommand()
.AddCommand("Get-Command")
.AddParameter("Name", commandsToExpand)
.AddParameter("ErrorAction", "Ignore");
IEnumerable<CommandInfo> expandedCommands = _powerShellContextService.InvokeCommandAsync<CommandInfo>(psCommand);
var expandedCommands = new List<string>(commandsToExpand.Count);
foreach (CommandInfo command in expandedCommands)
{
if (command is AliasInfo alias)
{
expandedCommands.Add(alias.ReferencedCommand.Name);
continue;
}
expandedCommands.Add(command.Name);
}
Basically rather than implementing the cmdlet, you can do something like this:
ScriptBlockAst scriptAst = Parser.ParseInput(request.Text, out Token[] _, out ParseError[] _);
var commandsToExpand = new List<string>();
foreach (Ast foundAst in scriptAst.FindAll(ast => ast is CommandAst, true))
{
if (!(foundAst is CommandAst commandAst))
{
continue;
}
string commandName = commandAst.GetCommandName();
if (commandName != null)
{
commandsToExpand.Add(commandName);
}
}
var psCommand = new PSCommand()
.AddCommand("Get-Command")
.AddParameter("Name", commandsToExpand)
.AddParameter("ErrorAction", "Ignore");
IEnumerable<CommandInfo> expandedCommands = _powerShellContextService.InvokeCommandAsync<CommandInfo>(psCommand);
var expandedCommands = new List<string>(commandsToExpand.Count);
foreach (CommandInfo command in expandedCommands)
{
if (command is AliasInfo alias)
{
expandedCommands.Add(alias.ReferencedCommand.Name);
continue;
}
expandedCommands.Add(command.Name);
}
rjmholt
Feb 18, 2020
Member
Posted this to document the thought, but it's non-trivial at this point.
You now need to be able to insert the expanded command names back into the original AST while not modifying anything else.
That will require some amount of custom logic to put the script back together from the AST, and there's not currently a nice API for that beyond writing a new AST visitor...
We could try this on tokens instead, but I'm not sure that's the best idea.
Posted this to document the thought, but it's non-trivial at this point.
You now need to be able to insert the expanded command names back into the original AST while not modifying anything else.
That will require some amount of custom logic to put the script back together from the AST, and there's not currently a nice API for that beyond writing a new AST visitor...
We could try this on tokens instead, but I'm not sure that's the best idea.
romero126
Feb 18, 2020
•
Author
Originally I had thought we can do much more with the tokenizer in order to remove repetitious code that will come up. Like Expand Alias, Expand Command.. etc.. However I don't really see much more of a use case other than those two.
So moving forward..
We can do something similar to this in the pipeline. It looks like we just need to create a pipeline object and chain commands to it.
get-command -Name gci | ? { $_ -is [System.Management.Automation.AliasInfo] } | % { $_.ResolvedCommand.ModuleName + "\" + $_.ResolvedCommand.Name }
Originally I had thought we can do much more with the tokenizer in order to remove repetitious code that will come up. Like Expand Alias, Expand Command.. etc.. However I don't really see much more of a use case other than those two.
So moving forward..
We can do something similar to this in the pipeline. It looks like we just need to create a pipeline object and chain commands to it.
get-command -Name gci | ? { $_ -is [System.Management.Automation.AliasInfo] } | % { $_.ResolvedCommand.ModuleName + "\" + $_.ResolvedCommand.Name }
rjmholt
Feb 18, 2020
Member
The problem with using the pipeline is that the more you use the pipeline the longer you exclude other services from using it. So basically you only want to use the pipeline for things you absolutely need its context for, which in this case is alias resolution.
Beyond that, we want to do as much as we can in off-thread C# code.
The problem is that the old function rebuilt the whole script one command at a time by rebuilding the string for each command. The new cmdlet doesn't do that and my code above doesn't have an implementation for that either.
We need to be able to reconstruct the whole script with the expanded commands given in.
The problem with using the pipeline is that the more you use the pipeline the longer you exclude other services from using it. So basically you only want to use the pipeline for things you absolutely need its context for, which in this case is alias resolution.
Beyond that, we want to do as much as we can in off-thread C# code.
The problem is that the old function rebuilt the whole script one command at a time by rebuilding the string for each command. The new cmdlet doesn't do that and my code above doesn't have an implementation for that either.
We need to be able to reconstruct the whole script with the expanded commands given in.
SeeminglyScience
Feb 19, 2020
•
Collaborator
We need to be able to reconstruct the whole script with the expanded commands given in.
Ideally the ExpandAliasResult would be changed to take a WorkspaceEdit. Then you could do something like this:
ScriptBlockAst ast = Parser.ParseInput(request.Text, out _, out _);
CommandAst[] commands = ast
.FindAll(a => a is CommandAst, searchNestedScriptBlocks: true)
.Cast<CommandAst>()
.ToArray();
var commandNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
foreach (CommandAst command in commands)
{
string commandName = command.GetCommandName();
if (string.IsNullOrEmpty(commandName))
{
continue;
}
commandNames.Add(commandName);
}
var psCommand = new PSCommand()
.AddCommand("Microsoft.PowerShell.Core\\Get-Command")
.AddParameter("CommandTypes", CommandTypes.Alias)
.AddParameter("ErrorAction", ActionPreference.Ignore)
.AddParameter("Name", commandNames.ToArray());
AliasInfo[] aliases = (await _powerShellContextService
.ExecuteCommandAsync<AliasInfo>(psCommand, sendErrorToHost: false).ConfigureAwait(false))
.ToArray();
var aliasMap = new Dictionary<string, string>(
capacity: aliases.Length,
StringComparer.OrdinalIgnoreCase);
foreach (AliasInfo alias in aliases)
{
aliasMap.Add(alias.Name, alias.Definition);
}
var edits = new List<TextEdit>(capacity: commands.Length);
foreach (CommandAst command in commands)
{
string commandName = command.GetCommandName();
if (string.IsNullOrEmpty(commandName))
{
continue;
}
if (!aliasMap.TryGetValue(commandName, out string definition))
{
continue;
}
IScriptExtent extent = command.CommandElements[0].Extent;
var edit = new TextEdit()
{
NewText = definition,
Range = new Range(
new Position(extent.StartLineNumber - 1, extent.StartColumnNumber - 1),
new Position(extent.EndLineNumber - 1, extent.EndColumnNumber - 1))
};
edits.Add(edit);
}
// make and return WorkspaceEdit
We need to be able to reconstruct the whole script with the expanded commands given in.
Ideally the ExpandAliasResult would be changed to take a WorkspaceEdit. Then you could do something like this:
ScriptBlockAst ast = Parser.ParseInput(request.Text, out _, out _);
CommandAst[] commands = ast
.FindAll(a => a is CommandAst, searchNestedScriptBlocks: true)
.Cast<CommandAst>()
.ToArray();
var commandNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
foreach (CommandAst command in commands)
{
string commandName = command.GetCommandName();
if (string.IsNullOrEmpty(commandName))
{
continue;
}
commandNames.Add(commandName);
}
var psCommand = new PSCommand()
.AddCommand("Microsoft.PowerShell.Core\\Get-Command")
.AddParameter("CommandTypes", CommandTypes.Alias)
.AddParameter("ErrorAction", ActionPreference.Ignore)
.AddParameter("Name", commandNames.ToArray());
AliasInfo[] aliases = (await _powerShellContextService
.ExecuteCommandAsync<AliasInfo>(psCommand, sendErrorToHost: false).ConfigureAwait(false))
.ToArray();
var aliasMap = new Dictionary<string, string>(
capacity: aliases.Length,
StringComparer.OrdinalIgnoreCase);
foreach (AliasInfo alias in aliases)
{
aliasMap.Add(alias.Name, alias.Definition);
}
var edits = new List<TextEdit>(capacity: commands.Length);
foreach (CommandAst command in commands)
{
string commandName = command.GetCommandName();
if (string.IsNullOrEmpty(commandName))
{
continue;
}
if (!aliasMap.TryGetValue(commandName, out string definition))
{
continue;
}
IScriptExtent extent = command.CommandElements[0].Extent;
var edit = new TextEdit()
{
NewText = definition,
Range = new Range(
new Position(extent.StartLineNumber - 1, extent.StartColumnNumber - 1),
new Position(extent.EndLineNumber - 1, extent.EndColumnNumber - 1))
};
edits.Add(edit);
}
// make and return WorkspaceEdit
Get-EditorServicesParserAst will allow for ExpandAlias or other ast features to look at comment blocks or Functions defined.
This also addresses an issue with running the Module in Constrained Language Mode.