Merge pull request #9505 from gafter/patterns-9371

Revise the shape of the syntax trees for property patterns

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

@@ -1450,12 +1450,24 @@
   </Node>
 
   <Node Name="BoundSubPropertyPattern" Base="BoundNode">
-    <Field Name="Type" Type="TypeSymbol" Null="disallow"/>
-    <Field Name="Property" Type="Symbol" Null="allow"/>
-    <Field Name="ResultKind" Type="LookupResultKind"/>
+    <!-- Member is either a BoundPropertyPatternMember or an error expression -->
+    <Field Name="Member" Type="BoundExpression" Null="disallow"/>
     <Field Name="Pattern" Type="BoundPattern" Null="disallow"/>
   </Node>
 
+  <Node Name="BoundPropertyPatternMember" Base="BoundExpression">
+    <!-- Non-null type is required for this node kind -->
+    <Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
+
+    <Field Name="MemberSymbol" Type="Symbol" Null="allow"/>
+    <!--<Field Name="Arguments" Type="ImmutableArray&lt;BoundExpression&gt;"/>-->
+    <!--<Field Name="ArgumentNamesOpt" Type="ImmutableArray&lt;string&gt;" Null="allow"/>-->
+    <!--<Field Name="ArgumentRefKindsOpt" Type="ImmutableArray&lt;RefKind&gt;" Null="allow"/>-->
+    <!--<Field Name="Expanded" Type="bool"/>-->
+    <!--<Field Name="ArgsToParamsOpt" Type="ImmutableArray&lt;int&gt;" Null="allow"/>-->
+    <Field Name="ResultKind" PropertyOverrides="true" Type="LookupResultKind"/>
+  </Node>
+
   <Node Name="BoundRecursivePattern" Base="BoundPattern">
     <Field Name="Type" Type="TypeSymbol" Null="disallow"/>
     <Field Name="IsOperator" Type="MethodSymbol" Null="allow"/>

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

@@ -2772,14 +2772,8 @@ public override TResult Accept<TArgument, TResult>(OperationVisitor<TArgument, T
             return visitor.VisitNoneOperation(this, argument);
         }
 
-        protected override OperationKind ExpressionKind
-        {
-            get
-            {
-                // TODO: implement IOperation for pattern-matching constructs (https://github.com/dotnet/roslyn/issues/8699)
-                return OperationKind.None;
-            }
-        }
+        // TODO: implement IOperation for pattern-matching constructs (https://github.com/dotnet/roslyn/issues/8699)
+        protected override OperationKind ExpressionKind => OperationKind.None;
     }
 
     partial class BoundMatchExpression
@@ -2796,14 +2790,8 @@ public override TResult Accept<TArgument, TResult>(OperationVisitor<TArgument, T
             return visitor.VisitNoneOperation(this, argument);
         }
 
-        protected override OperationKind ExpressionKind
-        {
-            get
-            {
-                // TODO: implement IOperation for pattern-matching constructs (https://github.com/dotnet/roslyn/issues/8699)
-                return OperationKind.None;
-            }
-        }
+        // TODO: implement IOperation for pattern-matching constructs (https://github.com/dotnet/roslyn/issues/8699)
+        protected override OperationKind ExpressionKind => OperationKind.None;
     }
 
     partial class BoundThrowExpression
@@ -2820,13 +2808,27 @@ public override TResult Accept<TArgument, TResult>(OperationVisitor<TArgument, T
             return visitor.VisitNoneOperation(this, argument);
         }
 
-        protected override OperationKind ExpressionKind
+        // TODO: implement IOperation for pattern-matching constructs (https://github.com/dotnet/roslyn/issues/8699)
+        protected override OperationKind ExpressionKind => OperationKind.None;
+    }
+
+    partial class BoundPropertyPatternMember
+    {
+        public override void Accept(OperationVisitor visitor)
         {
-            get
-            {
-                // TODO: implement IOperation for pattern-matching constructs (https://github.com/dotnet/roslyn/issues/8699)
-                return OperationKind.None;
-            }
+            // TODO: implement IOperation for pattern-matching constructs (https://github.com/dotnet/roslyn/issues/8699)
+            visitor.VisitNoneOperation(this);
         }
+
+        public override TResult Accept<TArgument, TResult>(OperationVisitor<TArgument, TResult> visitor, TArgument argument)
+        {
+            // TODO: implement IOperation for pattern-matching constructs (https://github.com/dotnet/roslyn/issues/8699)
+            return visitor.VisitNoneOperation(this, argument);
+        }
+
+        // TODO: implement IOperation for pattern-matching constructs (https://github.com/dotnet/roslyn/issues/8699)
+        protected override OperationKind ExpressionKind => OperationKind.None;
+
+        public override Symbol ExpressionSymbol => this.MemberSymbol;
     }
 }

src/Compilers/CSharp/Portable/CSharpExtensions.cs

@@ -430,22 +430,6 @@ public static Conversion ClassifyConversion(this Compilation compilation, ITypeS
             }
         }
 
-        /// <summary>
-        /// Gets the symbol information for the property of a sub-property pattern.
-        /// </summary>
-        public static SymbolInfo GetSymbolInfo(this SemanticModel semanticModel, SubPropertyPatternSyntax node, CancellationToken cancellationToken = default(CancellationToken))
-        {
-            var csmodel = semanticModel as CSharpSemanticModel;
-            if (csmodel != null)
-            {
-                return csmodel.GetSymbolInfo(node, cancellationToken);
-            }
-            else
-            {
-                return SymbolInfo.None;
-            }
-        }
-
         /// <summary>
         /// Returns what symbol(s), if any, the given expression syntax bound to in the program.
         /// 

src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs

@@ -495,11 +495,6 @@ internal virtual IOperation GetOperationWorker(CSharpSyntaxNode node, GetOperati
         /// </summary>
         public abstract SymbolInfo GetSymbolInfo(SelectOrGroupClauseSyntax node, CancellationToken cancellationToken = default(CancellationToken));
 
-        /// <summary>
-        /// Gets the symbol information for the property of a sub-property pattern.
-        /// </summary>
-        public abstract SymbolInfo GetSymbolInfo(SubPropertyPatternSyntax node, CancellationToken cancellationToken = default(CancellationToken));
-
         /// <summary>
         /// Returns what symbol(s), if any, the given expression syntax bound to in the program.
         /// 
@@ -4351,12 +4346,6 @@ private SymbolInfo GetSymbolInfoFromNode(SyntaxNode node, CancellationToken canc
                 return this.GetSymbolInfo(orderingSyntax, cancellationToken);
             }
 
-            var subPropertyPattern = node as SubPropertyPatternSyntax;
-            if (subPropertyPattern != null)
-            {
-                return this.GetSymbolInfo(subPropertyPattern, cancellationToken);
-            }
-
             return SymbolInfo.None;
         }
 

src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs

@@ -948,23 +948,6 @@ internal override Optional<object> GetConstantValueWorker(CSharpSyntaxNode node,
             return GetTypeInfoForQuery(bound);
         }
 
-        /// <summary>
-        /// Gets the symbol information for the property of a sub-property pattern.
-        /// </summary>
-        public override SymbolInfo GetSymbolInfo(SubPropertyPatternSyntax node, CancellationToken cancellationToken)
-        {
-            var boundNode = GetLowerBoundNode(node) as BoundSubPropertyPattern;
-            if (boundNode != null)
-            {
-                var property = boundNode.Property;
-                return new SymbolInfo(property, boundNode.ResultKind == LookupResultKind.Viable ? CandidateReason.None : boundNode.ResultKind.ToCandidateReason());
-            }
-            else
-            {
-                return default(SymbolInfo);
-            }
-        }
-
         private void GetBoundNodes(CSharpSyntaxNode node, out CSharpSyntaxNode bindableNode, out BoundNode lowestBoundNode, out BoundNode highestBoundNode, out BoundNode boundParent)
         {
             bindableNode = this.GetBindableSyntaxNode(node);
@@ -1637,8 +1620,7 @@ internal protected virtual CSharpSyntaxNode GetBindableSyntaxNode(CSharpSyntaxNo
                             !(node is OrderingSyntax) &&
                             !(node is JoinIntoClauseSyntax) &&
                             !(node is QueryContinuationSyntax) &&
-                            !(node is ArrowExpressionClauseSyntax) &&
-                            !(node is SubPropertyPatternSyntax))
+                            !(node is ArrowExpressionClauseSyntax))
                         {
                             return GetBindableSyntaxNode(parent);
                         }

src/Compilers/CSharp/Portable/Compilation/SyntaxTreeSemanticModel.cs

@@ -423,13 +423,6 @@ internal override Optional<object> GetConstantValueWorker(CSharpSyntaxNode node,
             return (model == null) ? default(TypeInfo) : model.GetTypeInfo(node, cancellationToken);
         }
 
-        public override SymbolInfo GetSymbolInfo(SubPropertyPatternSyntax node, CancellationToken cancellationToken)
-        {
-            CheckSyntaxNode(node);
-            var model = this.GetMemberModel(node);
-            return (model == null) ? default(SymbolInfo) : model.GetSymbolInfo(node, cancellationToken);
-        }
-
         public override IPropertySymbol GetDeclaredSymbol(AnonymousObjectMemberDeclaratorSyntax declaratorSyntax, CancellationToken cancellationToken = default(CancellationToken))
         {
             CheckSyntaxNode(declaratorSyntax);

src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass_Switch.cs

@@ -241,6 +241,11 @@ public virtual void VisitPattern(BoundExpression expression, BoundPattern patter
             }
         }
 
+        public override BoundNode VisitPropertyPatternMember(BoundPropertyPatternMember node)
+        {
+            throw ExceptionUtilities.Unreachable;
+        }
+
         /// <summary>
         /// Check if the given expression is known to *always* match, or *always* fail against the given pattern.
         /// Return true for known match, false for known fail, and null otherwise.
@@ -270,13 +275,12 @@ public virtual void VisitPattern(BoundExpression expression, BoundPattern patter
                         {
                             // so far so good: the expression is known to match the *type* of the property pattern.
                             // Now check if each subpattern is irrefutable.
-                            int n = propPattern.Subpatterns.Length;
-                            for (int i = 0; i < n; i++)
+                            foreach (var subpattern in propPattern.Subpatterns)
                             {
-                                var prop = propPattern.Subpatterns[i].Property;
-                                var pat = propPattern.Subpatterns[i].Pattern;
+                                var prop = (subpattern.Member as BoundPropertyPatternMember)?.MemberSymbol;
+                                var pat = subpattern.Pattern;
                                 BoundExpression subExpr;
-                                switch (prop.Kind)
+                                switch (prop?.Kind)
                                 {
                                     case SymbolKind.Property:
                                         var propSymbol = (PropertySymbol)prop;
@@ -286,6 +290,7 @@ public virtual void VisitPattern(BoundExpression expression, BoundPattern patter
                                         var fieldSymbol = (FieldSymbol)prop;
                                         subExpr = new BoundFieldAccess(pat.Syntax, null, fieldSymbol, null);
                                         break;
+                                    // TODO: what about events?
                                     default:
                                         return false;
                                 }

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs

@@ -46,14 +46,32 @@ BoundExpression TranslatePattern(BoundExpression input, BoundPattern pattern)
                         var temp = _factory.SynthesizedLocal(pat.Type);
                         var matched = DeclPattern(syntax, input, temp);
                         input = _factory.Local(temp);
-                        for (int i = 0; i < pat.Subpatterns.Length; i++)
+                        foreach (var subpattern in pat.Subpatterns)
                         {
-                            var subProperty = pat.Subpatterns[i].Property;
-                            var subPattern = pat.Subpatterns[i].Pattern;
-                            var subExpression =
-                                subProperty.Kind == SymbolKind.Field
-                                    ? (BoundExpression)_factory.Field(input, (FieldSymbol)subProperty)
-                                    : _factory.Call(input, ((PropertySymbol)subProperty).GetMethod);
+                            // TODO: review and test this code path.
+                            // https://github.com/dotnet/roslyn/issues/9542
+                            // e.g. Can the `as` below result in `null`?
+                            var subProperty = (subpattern.Member as BoundPropertyPatternMember)?.MemberSymbol;
+                            var subPattern = subpattern.Pattern;
+                            BoundExpression subExpression;
+                            switch (subProperty?.Kind)
+                            {
+                                case SymbolKind.Field:
+                                    subExpression = _factory.Field(input, (FieldSymbol)subProperty);
+                                    break;
+                                case SymbolKind.Property:
+                                    // TODO: review and test this code path.
+                                    // https://github.com/dotnet/roslyn/issues/9542
+                                    // e.g. https://github.com/dotnet/roslyn/pull/9505#discussion_r55320220
+                                    subExpression = _factory.Call(input, ((PropertySymbol)subProperty).GetMethod);
+                                    break;
+                                case SymbolKind.Event:
+                                    // TODO: should a property pattern be capable of referencing an event?
+                                    // https://github.com/dotnet/roslyn/issues/9515
+                                default:
+                                    throw ExceptionUtilities.Unreachable;
+                            }
+
                             var partialMatch = this.TranslatePattern(subExpression, subPattern);
                             matched = _factory.LogicalAnd(matched, partialMatch);
                         }
@@ -128,7 +146,9 @@ BoundExpression DeclPattern(CSharpSyntaxNode syntax, BoundExpression input, Loca
                 var tmpType = _factory.SpecialType(SpecialType.System_Nullable_T).Construct(type);
                 var tmp = _factory.SynthesizedLocal(tmpType, syntax);
                 var asg1 = _factory.AssignmentExpression(_factory.Local(tmp), _factory.As(input, tmpType));
-                var value = _factory.Call(_factory.Local(tmp), GetNullableMethod(syntax, tmpType, SpecialMember.System_Nullable_T_GetValueOrDefault));
+                var value = _factory.Call(
+                    _factory.Local(tmp),
+                    GetNullableMethod(syntax, tmpType, SpecialMember.System_Nullable_T_GetValueOrDefault));
                 var asg2 = _factory.AssignmentExpression(_factory.Local(target), value);
                 var result = MakeNullableHasValue(syntax, _factory.Local(tmp));
                 return _factory.Sequence(tmp, asg1, asg2, result);
@@ -143,7 +163,10 @@ BoundExpression DeclPattern(CSharpSyntaxNode syntax, BoundExpression input, Loca
                 //     return s;
                 // }
                 return _factory.Conditional(_factory.Is(input, type),
-                    _factory.Sequence(_factory.AssignmentExpression(_factory.Local(target), _factory.Convert(type, input)), _factory.Literal(true)),
+                    _factory.Sequence(_factory.AssignmentExpression(
+                        _factory.Local(target),
+                        _factory.Convert(type, input)),
+                        _factory.Literal(true)),
                     _factory.Literal(false),
                     _factory.SpecialType(SpecialType.System_Boolean));
             }