Fix Issue in Select-Object where Hashtable members (e.g., `Keys`) cannot be used with -Property or -ExpandProperty by vexx32 · Pull Request #11097 · PowerShell/PowerShell

Rain Sallow (/u/ta11ow) (vexx32) · 2019-11-18T05:39:32Z

PR Summary

Prior to this PR, a hashtable piped into Select-Object -[Expand]Property Keys where the user expected Keys to retrieve the property value from the hashtable rather than the dictionary entry would return no value (in the case of -ExpandProperty, an error would be emitted, stating that the property could not be found).

This PR fixes this issue, and examines hashtable members as well as their key/value entries. The logic is designed such that hashtable key/values are still prioritised over hashtable members, so if a user creates a hashtable such as @{ Keys = "None" } and then selects the Keys property via Select-Object, it will have the same result as using $ht.Keys already does: the returned value will always prioritise an actual key/value entry over the hashtable members.

Note that this means creating a hashtable with a key named Keys will prevent access to the Keys member on the hashtable in this manner. This new behaviour is consistent with existing behaviour with retrieval of hashtable keys, and also consistent with dot-property access behaviour with hashtables -- you can access $ht.Keys up until you add a key with the name Keys, at which point you will always get that key value back.

Tests have been added to cover these cases explicitly.

PR Context

Users should be able to utilise hashtables as actual hashtable objects as well as pseudo-PSObjects where appropriate. It is disingenuous for these commands to pretend hashtables don't have other members which may be desired in some circumstances.

This PR fixes #11094

PR Checklist

Rain Sallow (/u/ta11ow) (vexx32) · 2019-11-18T05:45:48Z

Note for reviewers -- the Select-Object test file hasn't been updated in a long time (still using tabs instead of spaces) so I included a format-only commit 6544c98 to sort that separately. You'll probably want to review that commit individually and then the following one for added tests.

No existing tests were modified in this PR apart from the formatting fixes. I only did a quick tabs-to-spaces and then auto-format commit, so if there are specific style issues you also want to fix while I'm there let me know, we can fix them either here or in a follow up PR as necessary. 🙂

Rain Sallow (/u/ta11ow) (vexx32) · 2019-11-18T05:49:57Z

Also, I think the Codacy suggestion to make IfHashtableWrapAsPSCustomObject() a static method can be ignored. 😄

Rain Sallow (/u/ta11ow) (vexx32) · 2019-11-18T06:07:29Z

Hmm, interesting. Didn't expect to break that, but there you have it.

Rain Sallow (/u/ta11ow) (vexx32) · 2019-11-18T06:09:07Z

PoshChan-Bot (@PoshChan) please remind me in 17 hours

Ilya (iSazonov) · 2019-11-18T08:52:38Z

Rain Sallow (/u/ta11ow) (@vexx32) The PR description is more complex than the fix :-)

PoshChan-Bot (PoshChan) · 2019-11-18T23:09:17Z

Rain Sallow (/u/ta11ow) (@vexx32), this is the reminder you requested 17 hours ago

Rain Sallow (/u/ta11ow) (vexx32) · 2019-11-18T23:24:34Z

That was simpler than I expected! 🎉

Rain Sallow (/u/ta11ow) (vexx32) · 2019-11-19T00:06:14Z

PoshChan-Bot (@PoshChan) please retry macos

PoshChan-Bot (PoshChan) · 2019-11-19T00:06:53Z

Rain Sallow (/u/ta11ow) (@vexx32), successfully started retry of PowerShell-CI-macOS

Deleted user (ghost) · 2020-05-27T02:02:26Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

- Fix an issue where Select-Object and similar could not resolve member names from hashtables due to them carelessly converting them. - Allows both hashtable keys/values and actual members to be referenced. Precedence is given to the hashtable keys first to avoid collisions.

- Add test for Count & Length members - Use base target object instead of wrapping hashtable

Ilya (iSazonov) · 2020-06-11T07:25:22Z

Rain Sallow (/u/ta11ow) (@vexx32) Please look CI failures.

- Still had tab stops in some places, replaced everything with spaces. - removed some accidentally duplicated code - 'Format Document' to ensure consistent indentation. Sorry about the diff, this file was a bit of a mess 🙏

Dongbo Wang (daxian-dbw) · 2020-06-12T23:52:12Z

Rain Sallow (/u/ta11ow) (@vexx32) The test failure in mac CI is a known issue. Can you please check on the Windows failure?

Rain Sallow (/u/ta11ow) (vexx32) · 2020-06-13T00:05:42Z

That's one I haven't seen before. Something to do with remoting? Not sure what to make of it 😅

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at System.Management.Automation.Remoting.PrioritySendDataCollection.ReadOrRegisterCallback(OnDataAvailableCallback callback, DataPriorityType& priorityType) in D:\a\1\s\src\System.Management.Automation\engine\remoting\fanin\PriorityCollection.cs:line 240
   at System.Management.Automation.Remoting.Client.WSManClientCommandTransportManager.SendOneItem() in D:\a\1\s\src\System.Management.Automation\engine\remoting\fanin\WSManTransportManager.cs:line 3975

   at System.Management.Automation.Remoting.Client.WSManClientCommandTransportManager.OnRemoteCmdSendCompleted(IntPtr operationContext, Int32 flags, IntPtr error, IntPtr shellOperationHandle, IntPtr commandOperationHandle, IntPtr operationHandle, IntPtr data) in D:\a\1\s\src\System.Management.Automation\engine\remoting\fanin\WSManTransportManager.cs:line 3681
An error has occurred that was not properly handled. Additional information is shown below. The PowerShell process will exit.
Execution of {& $powershell $PSFlags -c $command} by build.psm1: line 1332 failed with exit code -1073741819
At D:\a\1\s\build.psm1:2191 char:17
+                 throw $errorMessage
+                 ~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : OperationStopped: (Execution of {& $po…it code -1073741819:String) [], RuntimeException
+ FullyQualifiedErrorId : Execution of {& $powershell $PSFlags -c $command} by build.psm1: line 1332 failed with exit code -1073741819
##[error]PowerShell exited with code '1'.

Dongbo Wang (daxian-dbw) · 2020-06-15T17:41:10Z

PoshChan-Bot (@PoshChan) please retry windows

PoshChan-Bot (PoshChan) · 2020-06-15T17:41:34Z

Dongbo Wang (@daxian-dbw), did not find any matching pull request checks

Ilya (iSazonov)

~~Tests contain many style fixes - please move it in separate PR.~~

Ilya (iSazonov) · 2020-06-15T18:33:16Z

+                {
+                    members = target.Members.Match(
+                        _stringValue,
+                        PSMemberTypes.Properties | PSMemberTypes.PropertySet | PSMemberTypes.Dynamic);


We evaluate this again. Maybe put this in a variable before line 172?

This is evaluated against the wrapped target object in the prior case.

I mean PSMemberTypes.Properties | PSMemberTypes.PropertySet | PSMemberTypes.Dynamic

Ilya (iSazonov) · 2020-06-15T18:38:41Z

+        $result = $(Select-Object -InputObject $dirObject -Last $TestLength).Length
+        $expected = $dirObject.Length
+        { $dirObject | Select-Object } | Should -Not -Throw


Should -Not -Throw is not good test.
Why do we remove one test below? And why do we need $expected above? Why do we change the tests at all?

Dongbo Wang (daxian-dbw) · 2020-06-15T20:31:20Z

Ilya (@iSazonov) It was called out at #11097 (comment) that there is a format-only commit included.
I'm fine to keep that commit in this PR because this test file was seldom touched.

BTW Ilya (@iSazonov), it's easier to review after ignoring the white space characters: https://github.com/PowerShell/PowerShell/pull/11097/files?w=1

Dongbo Wang (daxian-dbw)

I agree with Ilya (@iSazonov), something is wrong in the changes of these couple of tests.

Dongbo Wang (daxian-dbw) · 2020-06-16T17:16:00Z

-	$result   = $(Select-Object -InputObject $dirObject -Last $TestLength).Length
-	$expected = $dirObject.Length
-
-	$result | Should -Be $expected


$result | Should -Be $expected shouldn't be removed here.

Dongbo Wang (daxian-dbw) · 2020-06-16T17:16:31Z

-	{ $dirObject | select } | Should -Not -Throw
+        $result = $(Select-Object -InputObject $dirObject -Last $TestLength).Length
+        $expected = $dirObject.Length
+        { $dirObject | Select-Object } | Should -Not -Throw


You need to keep the It "Should be able to use the alias" { here.

Dongbo Wang (daxian-dbw) · 2020-06-16T17:22:07Z

+        $dirObject[$TestLength].Size | Should -Be ($orig2 + 1)
+    }

-	It "Select-Object with Property First Last Overlap should work"{


Why is this test case removed?

Weird, not sure, will add it back.

Dongbo Wang (daxian-dbw) · 2020-06-16T17:22:48Z

        $obj.psobject.TypeNames[0] | Should -BeLike "Selected*"
        $obj.psobject.TypeNames[1] | Should -Not -BeLike "Selected*"
+        $p = Get-Process -Id $pid | Select-Object -Property Process* -ExcludeProperty ProcessorAffinity -ExpandProperty Modules
+        $p[0].psobject.Properties.Item("ProcessorAffinity") | Should -BeNullOrEmpty


The new code should be moved after $obj.psobject.TypeNames[2] | Should .. line below.

Looks like this got duplicated from above (L#328-329) somehow. Sorry about that, really not sure what on earth happened here. Will remove.

Some of the Select-Object tests got a bit mangled during format. This commit should restore them all back to proper order.

Rain Sallow (/u/ta11ow) (vexx32) · 2020-06-19T16:17:20Z

Apologies about the mangled tests, not sure how that happened. Should be all fixed up now. 💖 😊

Deleted user (ghost) · 2020-06-25T19:07:17Z

🎉v7.1.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

… cannot be used with `-Property` or `-ExpandProperty` (PowerShell#11097)

Rain Sallow (/u/ta11ow) (vexx32) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Nov 18, 2019

PoshChan-Bot (PoshChan) reviewed Nov 18, 2019

View reviewed changes

Comment thread src/System.Management.Automation/FormatAndOutput/common/Utilities/Mshexpression.cs

PoshChan-Bot (PoshChan) reviewed Nov 18, 2019

View reviewed changes

Comment thread src/System.Management.Automation/FormatAndOutput/common/Utilities/Mshexpression.cs

PoshChan-Bot (PoshChan) reviewed Nov 18, 2019

View reviewed changes

Comment thread src/System.Management.Automation/FormatAndOutput/common/Utilities/Mshexpression.cs

Ilya (iSazonov) changed the title ~~Select-Object - 🐛 Fix Issue where Hashtable members (e.g., Keys) cannot be used with -Property or -ExpandProperty~~ Fix Issue in Select-Object where Hashtable members (e.g., Keys) cannot be used with -Property or -ExpandProperty Nov 18, 2019

Ilya (iSazonov) added this to the 7.1.0-preview.1 milestone Nov 19, 2019

Ilya (iSazonov) assigned Dongbo Wang (daxian-dbw) Nov 19, 2019

Rain Sallow (/u/ta11ow) (vexx32) mentioned this pull request Nov 21, 2019

Q&A for PowerShell Community Call for Nov 21, 2019 PowerShell/PowerShell-RFC#235

Closed

Dongbo Wang (daxian-dbw) modified the milestones: 7.1.0-preview.1, 7.1.0-preview.2 Apr 1, 2020

Travis Plunk (TravisEz13) modified the milestones: 7.1.0-preview.2, 7.1.0-preview.3 Apr 24, 2020

Travis Plunk (TravisEz13) modified the milestones: 7.1.0-preview.3, 7.1.0-preview.4 May 14, 2020

ghost added the Review - Needed The PR is being reviewed label May 27, 2020

Dongbo Wang (daxian-dbw) requested changes Jun 10, 2020

View reviewed changes

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jun 10, 2020

ghost removed this from the 7.1.0-preview.4 milestone Jun 10, 2020

ghost removed the Review - Needed The PR is being reviewed label Jun 10, 2020

♻️ Address @daxian-dbw review comments

74e066c

- Add test for Count & Length members - Use base target object instead of wrapping hashtable

Rain Sallow (/u/ta11ow) (vexx32) force-pushed the SelectObjectCommand-DictionaryProperties branch from 6de53fe to 74e066c Compare June 11, 2020 02:23

♻️ un-break test file

cfea794

- Still had tab stops in some places, replaced everything with spaces. - removed some accidentally duplicated code - 'Format Document' to ensure consistent indentation. Sorry about the diff, this file was a bit of a mess 🙏

Rain Sallow (/u/ta11ow) (vexx32) force-pushed the SelectObjectCommand-DictionaryProperties branch from 8beeb21 to cfea794 Compare June 11, 2020 15:59

Dongbo Wang (daxian-dbw) requested changes Jun 12, 2020

View reviewed changes

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Jun 12, 2020

✅ Resolve test issues

f7845eb

Dongbo Wang (daxian-dbw) approved these changes Jun 15, 2020

View reviewed changes

Ilya (iSazonov) reviewed Jun 15, 2020

View reviewed changes

♻️ Address review comments

3d83000

Dongbo Wang (daxian-dbw) requested changes Jun 16, 2020

View reviewed changes

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Jun 16, 2020

✅ Restore mangled tests

f0802de

Some of the Select-Object tests got a bit mangled during format. This commit should restore them all back to proper order.

Ilya (iSazonov) approved these changes Jun 19, 2020

View reviewed changes

Dongbo Wang (daxian-dbw) approved these changes Jun 19, 2020

View reviewed changes

Dongbo Wang (daxian-dbw) merged commit 02af121 into PowerShell:master Jun 19, 2020

Dongbo Wang (daxian-dbw) added this to the 7.1.0-preview.4 milestone Jun 19, 2020

Rain Sallow (/u/ta11ow) (vexx32) deleted the SelectObjectCommand-DictionaryProperties branch June 20, 2020 00:46

ghost mentioned this pull request Jun 25, 2020

Select-Object -ExpandProperty does not work with Hashtable members #11094

Closed

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026

Fix issue in Select-Object where Hashtable members (e.g., Keys)…

eee99a2

… cannot be used with `-Property` or `-ExpandProperty` (PowerShell#11097)

Uh oh!

Conversation

Rain Sallow (/u/ta11ow) (vexx32) commented Nov 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Nov 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Nov 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Nov 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Nov 18, 2019

Uh oh!

Ilya (iSazonov) commented Nov 18, 2019

Uh oh!

PoshChan-Bot (PoshChan) commented Nov 18, 2019

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Nov 18, 2019

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Nov 19, 2019

Uh oh!

PoshChan-Bot (PoshChan) commented Nov 19, 2019

Uh oh!

Deleted user (ghost) commented May 27, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ilya (iSazonov) commented Jun 11, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Dongbo Wang (daxian-dbw) commented Jun 12, 2020

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Jun 13, 2020

Uh oh!

Dongbo Wang (daxian-dbw) commented Jun 15, 2020

Uh oh!

PoshChan-Bot (PoshChan) commented Jun 15, 2020

Uh oh!

Ilya (iSazonov) left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dongbo Wang (daxian-dbw) commented Jun 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dongbo Wang (daxian-dbw) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Nov 18, 2019 •

edited

Loading

Rain Sallow (/u/ta11ow) (vexx32) commented Nov 18, 2019 •

edited

Loading

Rain Sallow (/u/ta11ow) (vexx32) commented Nov 18, 2019 •

edited

Loading

Ilya (iSazonov) left a comment •

edited

Loading

Rain Sallow (/u/ta11ow) (vexx32) Jun 16, 2020 •

edited

Loading

Dongbo Wang (daxian-dbw) commented Jun 15, 2020 •

edited

Loading