Fix completion that relies on pseudobinding in script blocks #25122
Conversation
iSazonov
added
the
CL-General
| { | ||
| try | ||
| { | ||
| commandName = GetSafeValueVisitor.GetSafeValue(commandAst.CommandElements[0], null, GetSafeValueVisitor.SafeValueContext.ModuleAnalysis) as string; |
There was a problem hiding this comment.
Could you please add a code comment to document the scenario, maybe with examples of command names we expect here?
| string commandName = commandAst.GetCommandName(); | ||
| if (commandName is null) | ||
| { | ||
| // GetCommandName only works if the name is a string constant. GetSafeValueVistor can evaluate some safe dynamic expressions |
There was a problem hiding this comment.
GetCommandName? Comment for the method says about scripts, modules, aliases. So I'd like to understand what could be going on here.
There was a problem hiding this comment.
GetCommandName is the method that was run on line 280. It failed to get the name because it only handles constants but there's also this GetSafeValueVisitor that can handle a bit more dynamic expressions but still be safe. When this visitor sees a scriptblock expression, it tries to create a new scriptblock using the text but that fails when there's any parse errors in the script text. This is why: & {(New-Guid).<Tab> fails, but & {(New-Guid).N<Tab> does not, because the N removes the parser error from the missing name after the member access token.
There was a problem hiding this comment.
Thanks! I see GetSafeValue() process StringConstantExpressionAst too so we could not call commandAst.GetCommandName(). Perhaps it was added as fast way vs visitor.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
📣 Hey @MartinGC94, how did we do? We would love to hear your feedback with the link below! 🗣️ 🔗 https://aka.ms/PSRepoFeedback |
3 participants
PR Summary
Fixes an issue in the pseudobinding code where a parse exception would prevent the pseudobinding from working when attempting to parse an incomplete script. This would prevent this:
& {(New-Guid).<Tab>from working.It would fail because the script text has errors (due to incomplete input) and the safe value visitor would try to create a scriptblock based on the incomplete text.
PR Context
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.- [ ] Issue filed:
(which runs in a different PS Host).