Skip to content
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

Setting property values with the .ForEach() array method doesn't work with a single [pscustomobject] instance. #16666

Closed
5 tasks done
mklement0 opened this issue Dec 22, 2021 · 10 comments · Fixed by #17213
Closed
5 tasks done
Labels
Issue-Bug Issue has been identified as a bug in the product Resolution-Fixed The issue is fixed. WG-Engine core PowerShell engine, interpreter, and runtime

Comments

@mklement0
Copy link
Contributor

mklement0 commented Dec 22, 2021

Prerequisites

Steps to reproduce

.ForEach() has an overload that allows setting property values by name and (array of) property values.

This works in general, but unexpectedly not with a single [pscustomobject] instance (as later discovered, it does work with arrays of [pscustomobject] instances).

($obj = [pscustomobject] @{ p = 1 }).ForEach('p', 42)

Expected behavior

No output, and $obj.p should report 42 afterwards.

Actual behavior

1 - the current property value - is output, and the attempt to assign a new value is ignored.

Error details

No response

Environment data

PowerShell Core 7.3.0-preview.1

Visuals

No response

@mklement0 mklement0 added the Needs-Triage The issue is new and needs to be triaged by a work group. label Dec 22, 2021
@iSazonov iSazonov added Issue-Bug Issue has been identified as a bug in the product WG-Engine core PowerShell engine, interpreter, and runtime labels Dec 23, 2021
@scriptingstudio
Copy link

strange, with single brackets does not work but double do

@(($obj = [pscustomobject] @{ p = 1 })).ForEach('p', 42)
$obj

 p
 -
42

@mklement0
Copy link
Contributor Author

@scriptingstudio, good find; it's the use of @(....) that makes the difference here - the inner (...) are only needed in order to pass the value of the $obj assignment through.

In other words: it's only when calling .ForEach() on a scalar [pscustomobject] instance that the bug surfaces.
It works fine on an array of [pscustomobject] instances.

@jhoneill
Copy link

I thought .foreach() and .where() were for arrays only. Documentation of the .foreach() method is pretty thin - it's not mentioned in About_foreach for example. It certainly does work for some scalars as described above (so I was wrong :-) ) , but I wonder if it has a low profile because has never reliably implemented some features ? Maybe I'm just not finding the right search terms, but I can't find any examples of that syntax in use...

@scriptingstudio
Copy link

scriptingstudio commented Dec 23, 2021

@jhoneill just a couple of doc examples https://mcpmag.com/articles/2015/12/02/where-method-in-powershell.aspx, https://powershellmagazine.com/2014/10/22/foreach-and-where-magic-methods

Microsoft Certified Professional Magazine Online
This feature, new to version 4 of PowerShell, will be a tool you use every day.
ForEach and Where are two frequently used concepts that have been available in PowerShell since version 1 came out in 2006.

@iSazonov
Copy link
Collaborator

I thought .foreach() and .where() were for arrays only.

Yes, but Count works for scalar and foreach()/where() too.

@mklement0
Copy link
Contributor Author

mklement0 commented Dec 23, 2021

They're (now) documented in about_Arrays and briefly mentioned in about_Intrinsic_Members

In the former topic they're under the heading "Methods of Arrays", so I've been calling them array methods on Stack Overflow for a while.

Yes, they work on scalars too - as they should, given PowerShell's unified handling of scalars and collections (just like you can send a scalar as-is through the pipeline):

(1).ForEach({ $_ })  # -> 1
(1).Where({ $true }) # -> 1

(What makes for a slightly awkward asymmetry, however, is that - unlike in the pipeline - the return value is always a collection, namely of type [System.Collections.ObjectModel.Collection[PSObject]])

@SeeminglyScience
Copy link
Collaborator

Looks like the binder is calling InvokeAdaptedMember instead of EnumerableOps.ForEach.

Here's the Bind result for a scalar int:

if (((target is int) && (_version == 0)) && ((member is string) && (value is string)))
{
    return EnumerableOps.ForEach(
        Fake.Dynamic<Func<CallSite, IEnumerator, IEnumerator>>(PSEnumerableBinder.Get())(
            Fake.Dynamic<Func<CallSite, object[], IEnumerator>>(PSEnumerableBinder.Get())(
                new object[] { target })),
        member,
        new object[] { value });
}

(side note, dunno what's up with the double enumerate there)

And here it is for a pscustomobject:

if ((((_version == 0) && (target is PSObject)) && ((PSObject.Base(target) == target) && (member is string))) &&
    (value is string))
{
    return PSInvokeMemberBinder.InvokeAdaptedMember(PSObject.Base(target), "ForEach", new object[] { member, value });
}

@jhoneill
Copy link

@scriptingstudio

@jhoneill just a couple of doc examples

Yes I'd found Boe Prox's one which basically says "this is not for scalars", and Kirk's one says ", two new “magic” methods were introduced for collection types" So not scalars either. These are community experts explaining it, not information from the authors.

I thought .foreach() and .where() were for arrays only.

Yes, but Count works for scalar and foreach()/where() too.

Yes, @iSazonov I've relied on that for years... I use .where() heavily and I'm now thinking "Do I only use it on what I know will be an array, or do I trust it to do the right thing with a scalar."

The syntax Mike's using is in about_arrays (not About_foreach) and there is a note in about_Intrinsic_Members

There are certainly some things that are not right ...

ps > $s = "one two three"
ps > $s.where({$_.length -gt 10 } )
one two three

ps > $s.where({$_.length -gt 10 } ) += "four"
InvalidArgument: Cannot convert value "one two three" to type "System.Management.Automation.WhereOperatorSelectionMode".  Error: "Unable to match the identifier name one two three to a valid enumerator name. Specify one of the following enumerator names and try again:
Default, First, Last, SkipUntil, Until, Split"

ps > > $s.where({$_.length -gt 10 } ) + "four"
one two three
four

The second has turned the scalar into an array. But the first one seems have turned the whole thing inside out.

@SeeminglyScience
Copy link
Collaborator

Aside from the second one (which appears to be a very bizarre but separate binding bug) that all looks right. Where and ForEach always return a Collection<> regardless of the input.

@mklement0 mklement0 changed the title Setting property values with the .ForEach() array method doesn't work with [pscustomobject] instances. Setting property values with the .ForEach() array method doesn't work with a single [pscustomobject] instance. Dec 23, 2021
@ghost ghost added the In-PR Indicates that a PR is out for the issue label Apr 28, 2022
@ghost ghost added Resolution-Fixed The issue is fixed. and removed Needs-Triage The issue is new and needs to be triaged by a work group. In-PR Indicates that a PR is out for the issue labels May 2, 2022
@ghost
Copy link

ghost commented May 23, 2022

🎉This issue was addressed in #17213, which has now been successfully released as v7.3.0-preview.4.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug Issue has been identified as a bug in the product Resolution-Fixed The issue is fixed. WG-Engine core PowerShell engine, interpreter, and runtime
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants