Proposal: Extended property patterns

[alrz]

Extended property patterns

• Proposed
• Prototype: Not Started
• Implementation: Not Started
• Specification: extended-property-patterns.md
  

Summary

Allow property subpatterns to reference nested members.

Motivation

As discussed in #4114 when you want to match a child property, nesting another recursive pattern adds too much noise which will hurt readability with no real advantage.

Detailed design

Semantics

A pattern of the form { Property1.Property2: pattern } is exactly equivalent to { Property1: { Property2: pattern } }.

This will include the null check for Nullable<T> values as it is the case for the expanded form, so we only see the underlying type's members when we dot off of a property pattern.

Repeated member paths are allowed. Under the hood, such member accesses are simplified to be evaluated once.

Syntax

Currently a SubpatternSyntax uses NameColonSyntax which is not able to hold a chain of identifiers.

There are several avenues we could take:

1. Accept a generic ExpressoinSyntax in place of the name
   This would particularly help with parsing and avoiding lookaheads, since patterns and expressions have a common parsing path, we can start with the pattern and if we get to a colon, we just adjust it as the name and continue to parse the actual pattern.
2. Accept a generic NameSyntax in place of the name
   This seems to be the "correct" node to use, but is more expensive to parse. Also. if and when we introduce indexer patterns we probably want to relax property patterns to enable nesting those as well e.g. { Property1[0].Property2[1]: pattern }. In that case we'll need to go with option (1) for forward compatibility.
3. Accept a new QualifiedNameColonSyntax in addition to NameColonSyntax
   This option doesn't quite help with anything other than keeping the API consistent.

In the first two options, we're able to hold a simple identifier so we might want to consider deprecating NameColon API.

Alternatives

We could use { P1?.P2: p } syntax to make the implicit null check more apparent. But then we still want to consider P1.P2 in case ?. doesn't apply e.g. for structs. Since patterns shouldn't throw in regular usage and we should emit the null-check anyways, that distinction would seem to be unnecessary.

Design meetings

• https://github.com/dotnet/csharplang/blob/main/meetings/2021/LDM-2021-02-10.md#extended-property-patterns

[alrz]

cc @CyrusNajmabadi

[canton7]

My 2c: if I saw { Property1.Property2: { pattern } } I would intuitively expect that to be accessing Property1.Property2, and therefore throw an NRE if Property1 is null.

Given that x is C { Property1: > 0 } desugars to x is C c && c.Property1 > 0, I would expect x is C { Property1.Property2: > 0 } to desugar to x is C c && x.Property1.Property2 > 0.

[alrz]

Given that x is C { Property1: > 0 } desugars to x is C c && c.Property1 > 0

Not exactly. The compiler actually consider x != null first, but optimize it away because it's following by a type check.

Take {P1: {P2: 0}} for example, each property is checked for null and the simplified form should not deviate from that semantics.

would intuitively expect that to be accessing Property1.Property2, and therefore throw an NRE if Property1 is null.

Patterns never emit code that might throw an NRE. It's possible that a Deconstruct method or a property getter throw in a pattern match, but that's user code. So I'd say intuitively you should expect that it doesn't, because otherwise it is in fact unexpected.

[canton7]

Not exactly. The compiler actually consider x != null first, but optimize it away because it's following by a type check.

I simply stated what the compiler does.

So I'd say intuitively you should expect that it doesn't, because otherwise it is in fact unexpected.

Eh? You can have whatever expectations you like, but I'm stating my expectations. Different people come at these things from different angles, and I'm giving my intuition here. You would do well to at least consider viewpoints other than your own.

[333fred]

I'll champion this

[CyrusNajmabadi]

Accept a generic ExpressoinSyntax in place of the name

I would go this route.

We have precedence for this sort of attach as well. An initializer is just a list of expressions. So { x = y } is an initializer with an assignment expression in it. So you have to see if the assignment is to an identifier, in which case this is a property initializer.

[alrz]

@canton7 Different people come at these things from different angles

Certainly. I didn't mean to say otherwise and I didn't mean you personally. I apologies if that wasn't clear in my comment.

[CyrusNajmabadi]

I strongly support this proposal. This will help simplify and clarify a lot of patterns that check nested values today.

[CyrusNajmabadi]

Eh? You can have whatever expectations you like, but I'm stating my expectations. Different people come at these things from different angles, and I'm giving my intuition here. You would do well to at least consider viewpoints other than your own.

Fwiw, I agree with @alrz. Patterns themselves should not throw. The pattern is a declarative way to express what shape the value needs to have. So if you say x.y: 0. You're saying: the x prop needs to have a y prop with value 0. If x is null, then it doesn't and the pattern evaluates to false. It doesn't throw or crash.

[alrz]

Accept a generic ExpressoinSyntax in place of the name

@CyrusNajmabadi I would go this route.

If breaking the public API is an option I'd prefer that one too (that might include IOp as well). We just need to filter out invalid expressions which doesn't seem to be too expensive.

[333fred]

If breaking the public API is an option I'd prefer that one too (that might include IOp as well). We just need to filter out invalid expressions which doesn't seem to be too expensive.

I would say let's not drive language design based on the public API of Roslyn :).

[alrz]

I would say let's not drive language design based on the public API of Roslyn :).

Does that mean the language design can or cannot dictate breaking changes? I mean, as far as the language concerns, the grammar is straightforward. The only remaining issue is that how it fits into the current implementation and I just wanted to call it out.

[333fred]

The only remaining issue is that how it fits into the current implementation and I just wanted to call it out.

If we need to make a breaking change in the roslyn API to reflect the reality of the language, then that's what we need to do. We don't want to let the language be hamstrung by the implementation details of the public API of the compiler.

[CyrusNajmabadi]

Note: we would not need to make any breaking changes here. We'd just take one of our standard paths to update an existing langauge grammar rule when it comes to modeling it in teh syntax model. We're well versed on ways to accomplish that.

[bernd5]

To not break the existing API, we could add a simple "compatibility property" which checks if the Expression is an assignment expression ...
Another option would be similar to the situation we have in MethodDeclarationSyntax which has now "two Bodies", an "ExpressionBody" and a "Body".
But that should not be a reason to reject this pattern extension.

[CyrusNajmabadi]

@bernd5 this was already triaged into the 10.0 working set.

[jcouv]

Merged the feature into roslyn's main branch (still under "preview" LangVer at the moment) :-)
Thanks @alrz for the contribution to C# 10!