Tab completion for Where-Object and Foreach-Object does not escape single quotes in property names

Prerequisites

Write a descriptive title.
Make sure you are able to repro it on the latest released version
Search the existing issues.
Refer to the FAQ.
Refer to Differences between Windows PowerShell 5.1 and PowerShell.

Steps to reproduce

For PSCustomObject's (and likely others) properties containing characters like spaces and quotes, tab completion automatically quotes and escapes them. As example payload, take:

> $employees = "ID,Employee's name`n1,Alice`n2,Bob"

While $employees[0].E<TAB> will correctly produce $employees[0].'Employee''s name', doubling single quote to escape it, it is not done for Where-Object and ForEach-Object.

Expected behavior

> $Employees | % E<TAB>
$Employees | % 'Employee''s name'

Actual behavior

> $Employees | % E<TAB>
$Employees | % 'Employee's name'

Error details

Pressing enter waits for input due to unclosed quote.

Environment data

Name                           Value
----                           -----
PSVersion                      7.4.5
PSEdition                      Core
GitCommitId                    7.4.5
OS                             Microsoft Windows 10.0.22631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visuals

No response

MartinGC94

Paging MartinGC94 who I believe is local expert on tab completion

This can be fixed here: https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs#L6882 by changing it from completionText = $"'{memberName}'"; to completionText = $"'{memberName.Replace("'", "''")}'"; which is how it's done in other places. Though like in those other places it won't work if the member name contains special single quote chars like ’.

GitHub
PowerShell/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs at master · PowerShell/PowerShell
PowerShell for every system! Contribute to PowerShell/PowerShell development by creating an account on GitHub.

Wouldn't it make more sense to try and centralise ilthe code, so that commands use the same escaping as [0].E<YAB>?Otherwise there will be more mismatches

Definitely. What I suggested before was just a quick fix. Ideally someone should do what this PR tried to do: #10226 which is to go through all the completers to make the escaping methods work consistently.
Personally though I don't feel like doing the work because:
1: I have a bunch of other pending PRs that I don't want to risk merge conflicts with.
2: It's rarely something that bothers me because member names are typically simple enough that the current escaping works well enough for me.

Sure thing, refactors are usually done when there is no other pending work.

This can be fixed here: https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs#L6882 by changing it from completionText = $"'{memberName}'"; to completionText = $"'{memberName.Replace("'", "''")}'"; which is how it's done in other places. Though like in those other places it won't work if the member name contains special single quote chars like ’.

Wouldn't it make sense to use CodeGeneration.EscapeSingleQuotedStringContent instead of .Replace ?

I didn't know about that method but I don't see any reason why it should be preferred over .Replace.

Well, main reason is for what you mentioned in the last comment:

Though like in those other places it won't work if the member name contains special single quote chars like ’

This method handles that:

[System.Management.Automation.Language.CodeGeneration]::EscapeSingleQuotedStringContent("foo’bar")
# Outputs: foo’’bar

Would something like this work?

var completion Text = (memberName.IndexOfAny(s_charactersRequiringQuotes) != 0)
    ? $"'{EscapeSingleQuotedStringContent(memberName)'"
    : memberName;
if (isMethod && addMethodParenthesis)
{
    completionText += "(";
}

Would something like this work?

var completion Text = (memberName.IndexOfAny(s_charactersRequiringQuotes) != 0)
    ? $"'{EscapeSingleQuotedStringContent(memberName)'"
    : memberName;
if (isMethod && addMethodParenthesis)
{
    completionText += "(";
}

I don't think methods can have a ' (might be mistaken) but also you shouldn't try to add ( when using the -MemberName + -ArgumentList parameter set. ForEach-Object will correctly bind the method arguments using spaces, i.e.:

class Test {
    [string] Method([string] $foo, [string] $bar) {
        return "$foo $bar"
    }
}

[Test]::new() | % Method hello world

Navigation Menu

Repository navigation

Tab completion for Where-Object and Foreach-Object does not escape single quotes in property names #24493

Prerequisites

Steps to reproduce

Expected behavior

Actual behavior

Error details

Environment data

Visuals

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Tab completion for Where-Object and Foreach-Object does not escape single quotes in property names #24493

Description

Prerequisites

Steps to reproduce

Expected behavior

Actual behavior

Error details

Environment data

Visuals

Activity

dkaszews commented on Oct 25, 2024

MartinGC94 commented on Oct 25, 2024

dkaszews commented on Oct 26, 2024

MartinGC94 commented on Oct 26, 2024

dkaszews commented on Oct 26, 2024

santisq commented on Oct 26, 2024

MartinGC94 commented on Oct 26, 2024

santisq commented on Oct 26, 2024

dkaszews commented on Oct 27, 2024

santisq commented on Oct 30, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions