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.

Sign up for GitHub

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

Jump to bottom

Trying to assign a value to an .Item(int index) method call is quietly ignored; equivalent indexer syntax (e.g. [0]) does report an error, albeit a misleading one. #24074

Open
5 tasks done
mklement0 opened this issue Jul 23, 2024 · 6 comments
Open
5 tasks done

Trying to assign a value to an .Item(int index) method call is quietly ignored; equivalent indexer syntax (e.g. [0]) does report an error, albeit a misleading one. #24074

mklement0 opened this issue Jul 23, 2024 · 6 comments
Assignees
@SeeminglyScience
Labels
Issue-Enhancement the issue is more of a feature request than a bug Needs-Investigation The behavior reported in the issue is unexpected and needs further investigation. WG-Engine core PowerShell engine, interpreter, and runtime

Comments

@mklement0
Copy link
Contributor

mklement0 commented Jul 23, 2024 •
edited
Loading

Prerequisites

Steps to reproduce

PowerShell surfaces even .Item(int index) methods (e.g. XmlNodeList.Item) via indexer syntax, not just parameterized .Item properties.

For instance:

$nodeList = ([xml] '<foo><bar>1</bar><bar>2</bar></foo>').SelectNodes('//bar')

# -> $true, because these syntax forms are equivalent.
$nodeList.Item(0) -eq $nodeList[0]

By definition, such an .Item method is conceptually read-only; that is, a value can be retrieved, but not assigned:

# !! PROBLEMATIC: Misleading error message: 
#  'InvalidOperation: Unable to index into an object of type "System.Xml.XPathNodeList".'
$nodeList[0] = $null

'---'

# !! BROKEN: The equivalent syntax form is QUIETLY IGNORED.
$nodeList.Item(0) = $null

Conceptually related:

Expected behavior

InvalidOperation: Cannot assign a value to an element of a "System.Xml.XPathNodeList" instance.
---
InvalidOperation: Cannot assign a value to an element of a "System.Xml.XPathNodeList" instance.

Note:

  • The error message is a hypothetical one and is meant to be syntax-neutral (see discussion below).
  • In C# code, the compile-time error that would occur is Property or indexer 'XmlNodeList.this[int]' cannot be assigned to -- it is read only

Actual behavior

InvalidOperation: Unable to index into an object of type "System.Xml.XPathNodeList".
---

That is:

Error details

No response

Environment data

PowerShell 7.5.0-preview.3

Visuals

No response
@mklement0 mklement0 added the Needs-Triage The issue is new and needs to be triaged by a work group. label Jul 23, 2024
@mklement0 mklement0 changed the title Trying to assign a value to an .Item(int index) method call is quietly ignored. Trying to assign a value to an .Item(int index) method call is quietly ignored; equivalent indexer syntax (e.g. [0]) does report an error, but a misleading one. Jul 23, 2024
@237dmitry
Copy link

237dmitry commented Jul 23, 2024 •
edited
Loading

!! BROKEN: The equivalent syntax form is QUIETLY IGNORED.

But this work:

$nodeList.Item(0).'#text' = $null

@mklement0 mklement0 changed the title Trying to assign a value to an .Item(int index) method call is quietly ignored; equivalent indexer syntax (e.g. [0]) does report an error, but a misleading one. Trying to assign a value to an .Item(int index) method call is quietly ignored; equivalent indexer syntax (e.g. [0]) does report an error, albeit a misleading one. Jul 23, 2024
@mklement0
Copy link
Contributor Author

@237dmitry, it does, but that isn't relevant to this issue (you're simply performing an assignment to a read/write property).

@rhubarb-geek-nz
Copy link

Property or indexer 'XmlNodeList.this[int]' cannot be assigned to -- it is read only

The issue isn't that it is read-only. The fact that you can read from it is irrelevant to the assignment operation. The issue is there is no set accessor

@mklement0
Copy link
Contributor Author

mklement0 commented Jul 23, 2024 •
edited
Loading

That is the error message emitted by the C# compiler in this scenario, which I agree is also not ideal, especially with the explicit .Item() call.

As noted, the .Item() member at hand is a method, to which property accessors do not apply. Typically, what underlies indexers is a parameterized property. Because we're dealing with a method, I called it conceptually read-only in the initial post.

Ideally, the error message should reflect the syntax used in the source code (indexer vs. method syntax); not sure if that is possible / worth the effort.

@mklement0
Copy link
Contributor Author

On further reflection:

  • As the current error message / lack thereof shows, PowerShell can choose its own error message.

  • A syntax-neutral error message would be something like:

    • InvalidOperation: Cannot assign a value to an element of a "System.Xml.XPathNodeList" instance.

I've updated the initial post accordingly.

@vexx32 vexx32 added Issue-Enhancement the issue is more of a feature request than a bug WG-Engine core PowerShell engine, interpreter, and runtime labels Jul 25, 2024
@SeeminglyScience SeeminglyScience self-assigned this Sep 16, 2024
@SeeminglyScience SeeminglyScience added Needs-Investigation The behavior reported in the issue is unexpected and needs further investigation. and removed Needs-Triage The issue is new and needs to be triaged by a work group. labels Sep 16, 2024
@SeeminglyScience
Copy link
Collaborator

The Engine WG discussed this and agree that it should not silently fail. I will investigate further

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@SeeminglyScience SeeminglyScience

Labels
Issue-Enhancement the issue is more of a feature request than a bug Needs-Investigation The behavior reported in the issue is unexpected and needs further investigation. WG-Engine core PowerShell engine, interpreter, and runtime
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mklement0 @SeeminglyScience @vexx32 @237dmitry @rhubarb-geek-nz