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

Dot-Property Access for Hashtable/Dictionary keys HIDES property values #7758

Closed
vexx32 opened this issue Sep 11, 2018 · 7 comments
Closed
Labels
Issue-Discussion the issue may not have a clear classification yet. The issue may generate an RFC or may be reclassif Resolution-By Design The reported behavior is by design. WG-Engine core PowerShell engine, interpreter, and runtime

Comments

@vexx32
Copy link
Collaborator

vexx32 commented Sep 11, 2018

@PowerShell/powershell-committee
@rjmholt (I can't tag them, apparently, heh!~)

Per discourse in #7736, opening this for hashtable key > property precedence and dynamic binding issues.

Steps to reproduce

$Hash = @{
    Keys = "No keys for you!"
}

Expected behavior

When accessing .Keys property, the keys of the hashtable should be listed. Properties (statically-bound, part of the parent object) should have higher precedence than keys in the hashtable, which are d

PS> $Hash.Keys
Keys

Hashtable keys should be lower precedence, because the native key access syntax $Hash['Keys'] is already an alternative and should be preferred in most cases regardless. The key->dot-property mapping is syntactic sugar and should not override actual object properties.

Actual behavior

The key named 'keys' is accessed instead, preventing listing of the hashtable keys without some trickery in amongst .PSObject.Properties

PS> $Hash.Keys
No keys for you!

This behaviour is reflected the same way for all hashtable object properties -- specifying a key in the table that has the same name as a property will mask the property and make it inaccessible, with the sole exception of the hidden .psobject property.

Environment data

> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      6.1.0-rc.1
PSEdition                      Core
GitCommitId                    6.1.0-rc.1
OS                             Microsoft Windows 10.0.17134
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0
@mklement0
Copy link
Contributor

mklement0 commented Sep 11, 2018

Interestingly, the logic is reversed in the context of member enumeration (collection-level properties shadow element properties), which prompted #7445 (see #7445 (comment) in particular).

To spell out the - obscure - workarounds:

# Update: Via .psbase - as suggested by @SteveL-MSFT in #3176
#  and as now also documented in about_hash-tables
PS> @{ Keys = "No keys for you!"; Foo = 'bar' }.psbase.Keys
Keys
Foo

# Also: .get_Keys()
PS> @{ Keys = "No keys for you!"; Foo = 'bar' }.get_Keys()
Keys
Foo

# Via .psobject.properties
PS> @{ Keys = "No keys for you!"; Foo = 'bar' }.psobject.properties['Keys'].Value
Keys
Foo

@BrucePay
Copy link
Collaborator

The current behaviour is by design and has been this way since V1. And while the behaviour is problematic in some cases (e.g. accessing the Keys, Values and Count properties), changing it would be a significant breaking change. Also, the workarounds, while obscure, are not difficult: .get_Count(), .get_Keys(), .get_Values().

@mklement0

Interestingly, the logic is reversed in the context of member enumeration (collection-level properties shadow element properties),

These are two entirely unrelated things. The hashtable behaviour predates member enumeration by about a decade.

Historical note: this behaviour dates from the earliest iterations of the language, modeled on Perl's hashtable/object behaviour. In these very early versions, hashtables were written as

@{
    one => 1
    two => 2
    method => { ... }
}

At this point, the fact that there was a .NET class underlying hashtables was not really relevant (hence the hiding of the .NET properties). Hashtables were pseudo-objects which included "methods": assigning a scriptblock to a key allowed you to invoke that key as $table.method(...). Later on, where we introduced the ability to add instance members with Add-Member the "method" mechanism was removed. Around this time, the separator was also changed from => to = but the property hiding was never cleaned up.

@mklement0
Copy link
Contributor

Thanks for giving us the history, @BrucePay.

These are two entirely unrelated things. The hashtable behaviour predates member enumeration by about a decade.

Unfortunately, that still makes it an inconsistency that can trip people up, because conceptually these things are similar.

With a commitment to backward compatibility, there is no good solution. Yes, the workaround is simple, but it being obscure is bad enough in itself.

For member enumeration, the proposed @. offers a clean solution, but, due to the reversed logic, that is not an option for hashtables.

@iSazonov iSazonov added WG-Engine core PowerShell engine, interpreter, and runtime Resolution-By Design The reported behavior is by design. Issue-Discussion the issue may not have a clear classification yet. The issue may generate an RFC or may be reclassif labels Sep 12, 2018
@vexx32
Copy link
Collaborator Author

vexx32 commented Sep 12, 2018

@BrucePay it would be a 'significant breaking change' to reverse the logic in the case of a small handful of properties that are attached to hashtables?

That doesn't strike me as particularly significant. I'm not suggesting removing the property->key mapping, I'm suggesting querying the object's actual properties first, in the way that is likely to be expected by anyone who knows the barest basics of hashtables.

I'm suggesting that because we have the native key access syntax, we can just ensure properties are queried first when using the syntax that is designed to access properties, so if there is an overlap like this (property with the same name as a key), said property is not hidden. Accessing the key itself would simply be done by using the standard collection accessor $hash['keys']

Unless you make extensive use of the present state of things, where you deliberately hide the hashtable properties with keys, it would break... Absolutely nothing, no? Using it for non-colliding key names would still work just fine, and if you (foolishly, perhaps) had some code where you took user input to insert keys to a hashtable, the possibility of them breaking your code if you don't know about the broken behaviour is quietly removed, letting you enumerate keys as per normal.

@mklement0
Copy link
Contributor

Good point about index notation being an unambiguous way to target entries, @vexx32.

While I too wish we could change the current logic, I think the backward-compatibility problem is more serious than you think:

in the way that is likely to be expected by anyone who knows the barest basics of hashtables.

In the context of PowerShell, you may never need to access any of a hashtable's properties from your code, which means existing code may live happily - without symptoms - with hashtables such as the following:

$ht = @{ Values = 'values'; Keys = 'keys'; Count = 'chocula'  }

Only if the code uses dot notation and expects it to give precedence to the hashtable's own properties do you have a problem, and that code would currently be broken.
And you can definitely use hashtables without ever touching the type's instance properties directly.

Conversely, someone using $ht.Values in their code to target the entry will see their code break, if we reverse the logic.

Someone who knows that dot notation targets entries first would probably not construct such a hashtable (though may still be given one), but given that we're discussing this only now, the behavior is probably not widely known - and one of the nice things about PowerShell is that you don't have to be intimately familiar with the underlying .NET types.

Ultimately, only a dedicated entry-accessor syntax form solves the problem - which we do have in [...] for hashtables, and which is proposed as @. for list-like collections (member enumeration) - but taking away dot notation for entry access now can definitely break things.

You could argue that dot notation should never have been implemented for hashtable-entry access and member enumeration, but those ships have clearly sailed, and the flip sides are convenience and familiarity.

The member-enumeration case is less problematic, because it does give precedence to the list's own properties (and you can use enumeration to ensure that you're targeting entries), but it sounds like we'll have to live the .get_<propName>() workaround for hashtables, unless someone conducts thorough analysis to demonstrate that little code, if any, is likely to break.

@adamedx
Copy link

adamedx commented Jul 17, 2022

What a surprise in a bad way -- I can't believe the language would do something like this. I filed a doc bug to make this more visible, we probably need to add any property accessors like keys to the linter for PowerShell in tools like vscode if they don't already catch that (I have not seen that they do).

Languages should not have non-deterministic behavior, especially if it's not documented. At this point I understand the arguments about not just "fixing" it due to the fix itself breaking code, but if we can't invent a mode called "don't alter the behavior of .net types at runtime", then at the very least let's identify a syntax for accessing collections without strange workarounds. If I add get_Keys() to my code everywhere, someone else is likely to come along and change it back to Keys because they have no idea about this unexpected (and frankly crazy) behavior.

@rkeithhill
Copy link
Collaborator

Another option is to create a PSScriptAnalyzer rule that flags hashtable property names that correspond to underlying .NET properties like Keys and Values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Discussion the issue may not have a clear classification yet. The issue may generate an RFC or may be reclassif Resolution-By Design The reported behavior is by design. WG-Engine core PowerShell engine, interpreter, and runtime
Projects
None yet
Development

No branches or pull requests

7 participants