Skip to content

Introduce Get-EditorServicesParserAst to expand on ExpandAlias #1199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

romero126
Copy link

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.

@romero126 romero126 requested a review from rjmholt as a code owner February 18, 2020 18:17
@TylerLeonhardt
Copy link
Member

Codacy Here is an overview of what got changed by this pull request:

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>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
if (PSTokenType == PSTokenType.Command)
{
var result = SessionState.InvokeCommand.GetCommand(token.Content, CommandType);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@rjmholt rjmholt Feb 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@romero126 romero126 Feb 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

@SeeminglyScience SeeminglyScience Feb 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@andyleejordan
Copy link
Member

Sorry, just going to close this, it's been 4 years. Still up for grabs if someone wants to improve the way that expand alias function works. It's still a TODO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants