Support unbound generic types in 'nameof' operator. (#75368)

Co-authored-by: Julien Couvreur <[email protected]>

src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs

@@ -1373,14 +1373,32 @@ private NamedTypeSymbol ConstructNamedTypeUnlessTypeArgumentOmitted(SyntaxNode t
         {
             if (typeArgumentsSyntax.Any(SyntaxKind.OmittedTypeArgument))
             {
-                // Note: lookup won't have reported this, since the arity was correct.
-                // CONSIDER: the text of this error message makes sense, but we might want to add a separate code.
-                Error(diagnostics, ErrorCode.ERR_BadArity, typeSyntax, type, MessageID.IDS_SK_TYPE.Localize(), typeArgumentsSyntax.Count);
+                if (this.IsInsideNameof)
+                {
+                    // Inside a nameof an open-generic type is acceptable.  Fall through and bind the remainder accordingly.
+                    CheckFeatureAvailability(typeSyntax, MessageID.IDS_FeatureUnboundGenericTypesInNameof, diagnostics);
+
+                    // From the spec:
+                    //
+                    // Member lookup on an unbound type expression will be performed the same way as for a `this`
+                    // expression within that type declaration.
+                    //
+                    // So we want to just return the originating type symbol as is (e.g. List<T> in nameof(List<>)).
+                    // This is distinctly different than how typeof(List<>) works, where it returns an unbound generic
+                    // type.
+                }
+                else
+                {
+                    // Note: lookup won't have reported this, since the arity was correct.
+                    // CONSIDER: the text of this error message makes sense, but we might want to add a separate code.
+
+                    // If the syntax looks like an unbound generic type, then they probably wanted the definition.
+                    // Give an error indicating that the syntax is incorrect and then use the definition.
+                    // CONSIDER: we could construct an unbound generic type symbol, but that would probably be confusing
+                    // outside a typeof.
+                    Error(diagnostics, ErrorCode.ERR_BadArity, typeSyntax, type, MessageID.IDS_SK_TYPE.Localize(), typeArgumentsSyntax.Count);
+                }
 
-                // If the syntax looks like an unbound generic type, then they probably wanted the definition.
-                // Give an error indicating that the syntax is incorrect and then use the definition.
-                // CONSIDER: we could construct an unbound generic type symbol, but that would probably be confusing
-                // outside a typeof.
                 return type;
             }
             else

src/Compilers/CSharp/Portable/Binder/NameofBinder.cs

@@ -2,7 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections.Generic;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp
@@ -19,19 +21,25 @@ namespace Microsoft.CodeAnalysis.CSharp
     /// </summary>
     internal sealed class NameofBinder : Binder
     {
-        private readonly SyntaxNode _nameofArgument;
+        private readonly ExpressionSyntax _nameofArgument;
         private readonly WithTypeParametersBinder? _withTypeParametersBinder;
         private readonly Binder? _withParametersBinder;
         private ThreeState _lazyIsNameofOperator;
 
-        internal NameofBinder(SyntaxNode nameofArgument, Binder next, WithTypeParametersBinder? withTypeParametersBinder, Binder? withParametersBinder)
+        private readonly Dictionary<GenericNameSyntax, bool>? _allowedMap;
+
+        internal NameofBinder(ExpressionSyntax nameofArgument, Binder next, WithTypeParametersBinder? withTypeParametersBinder, Binder? withParametersBinder)
             : base(next)
         {
             _nameofArgument = nameofArgument;
             _withTypeParametersBinder = withTypeParametersBinder;
             _withParametersBinder = withParametersBinder;
+            OpenTypeVisitor.Visit(nameofArgument, out _allowedMap);
         }
 
+        protected override bool IsUnboundTypeAllowed(GenericNameSyntax syntax)
+            => _allowedMap != null && _allowedMap.TryGetValue(syntax, out bool allowed) && allowed;
+
         private bool IsNameofOperator
         {
             get

src/Compilers/CSharp/Portable/Binder/OpenTypeVisitor.cs

@@ -0,0 +1,99 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace Microsoft.CodeAnalysis.CSharp;
+
+internal partial class Binder
+{
+    /// <summary>
+    /// This visitor walks over a type expression looking for open types.
+    /// 
+    /// Open types are allowed if an only if:
+    /// <list type="number">
+    /// <item>There is no constructed generic type elsewhere in the visited syntax; and</item>
+    /// <item>The open type is not used as a type argument or array/pointer/nullable element type.</item>
+    /// </list>
+    /// 
+    /// Open types can be used both in <c>typeof(...)</c> and <c>nameof(...)</c> expressions.
+    /// </summary>
+    protected sealed class OpenTypeVisitor : CSharpSyntaxVisitor
+    {
+        private Dictionary<GenericNameSyntax, bool>? _allowedMap;
+        private bool _seenConstructed;
+
+        /// <param name="typeSyntax">The argument to typeof.</param>
+        /// <param name="allowedMap">
+        /// Keys are GenericNameSyntax nodes representing unbound generic types.
+        /// Values are false if the node should result in an error and true otherwise.
+        /// </param>
+        public static void Visit(ExpressionSyntax typeSyntax, out Dictionary<GenericNameSyntax, bool>? allowedMap)
+        {
+            OpenTypeVisitor visitor = new OpenTypeVisitor();
+            visitor.Visit(typeSyntax);
+            allowedMap = visitor._allowedMap;
+        }
+
+        public override void VisitGenericName(GenericNameSyntax node)
+        {
+            SeparatedSyntaxList<TypeSyntax> typeArguments = node.TypeArgumentList.Arguments;
+            if (node.IsUnboundGenericName)
+            {
+                _allowedMap ??= new Dictionary<GenericNameSyntax, bool>();
+                _allowedMap[node] = !_seenConstructed;
+            }
+            else
+            {
+                _seenConstructed = true;
+                foreach (TypeSyntax arg in typeArguments)
+                {
+                    Visit(arg);
+                }
+            }
+        }
+
+        public override void VisitQualifiedName(QualifiedNameSyntax node)
+        {
+            bool seenConstructedBeforeRight = _seenConstructed;
+
+            // Visit Right first because it's smaller (to make backtracking cheaper).
+            Visit(node.Right);
+
+            bool seenConstructedBeforeLeft = _seenConstructed;
+
+            Visit(node.Left);
+
+            // If the first time we saw a constructed type was in Left, then we need to re-visit Right
+            if (!seenConstructedBeforeRight && !seenConstructedBeforeLeft && _seenConstructed)
+            {
+                Visit(node.Right);
+            }
+        }
+
+        public override void VisitAliasQualifiedName(AliasQualifiedNameSyntax node)
+        {
+            Visit(node.Name);
+        }
+
+        public override void VisitArrayType(ArrayTypeSyntax node)
+        {
+            _seenConstructed = true;
+            Visit(node.ElementType);
+        }
+
+        public override void VisitPointerType(PointerTypeSyntax node)
+        {
+            _seenConstructed = true;
+            Visit(node.ElementType);
+        }
+
+        public override void VisitNullableType(NullableTypeSyntax node)
+        {
+            _seenConstructed = true;
+            Visit(node.ElementType);
+        }
+    }
+}

src/Compilers/CSharp/Portable/Binder/TypeofBinder.cs

@@ -36,92 +36,5 @@ protected override bool IsUnboundTypeAllowed(GenericNameSyntax syntax)
             bool allowed;
             return _allowedMap != null && _allowedMap.TryGetValue(syntax, out allowed) && allowed;
         }
-
-        /// <summary>
-        /// This visitor walks over a type expression looking for open types.
-        /// Open types are allowed if an only if:
-        ///   1) There is no constructed generic type elsewhere in the visited syntax; and
-        ///   2) The open type is not used as a type argument or array/pointer/nullable
-        ///        element type.
-        /// </summary>
-        private class OpenTypeVisitor : CSharpSyntaxVisitor
-        {
-            private Dictionary<GenericNameSyntax, bool> _allowedMap;
-            private bool _seenConstructed;
-
-            /// <param name="typeSyntax">The argument to typeof.</param>
-            /// <param name="allowedMap">
-            /// Keys are GenericNameSyntax nodes representing unbound generic types.
-            /// Values are false if the node should result in an error and true otherwise.
-            /// </param>
-            public static void Visit(ExpressionSyntax typeSyntax, out Dictionary<GenericNameSyntax, bool> allowedMap)
-            {
-                OpenTypeVisitor visitor = new OpenTypeVisitor();
-                visitor.Visit(typeSyntax);
-                allowedMap = visitor._allowedMap;
-            }
-
-            public override void VisitGenericName(GenericNameSyntax node)
-            {
-                SeparatedSyntaxList<TypeSyntax> typeArguments = node.TypeArgumentList.Arguments;
-                if (node.IsUnboundGenericName)
-                {
-                    if (_allowedMap == null)
-                    {
-                        _allowedMap = new Dictionary<GenericNameSyntax, bool>();
-                    }
-                    _allowedMap[node] = !_seenConstructed;
-                }
-                else
-                {
-                    _seenConstructed = true;
-                    foreach (TypeSyntax arg in typeArguments)
-                    {
-                        Visit(arg);
-                    }
-                }
-            }
-
-            public override void VisitQualifiedName(QualifiedNameSyntax node)
-            {
-                bool seenConstructedBeforeRight = _seenConstructed;
-
-                // Visit Right first because it's smaller (to make backtracking cheaper).
-                Visit(node.Right);
-
-                bool seenConstructedBeforeLeft = _seenConstructed;
-
-                Visit(node.Left);
-
-                // If the first time we saw a constructed type was in Left, then we need to re-visit Right
-                if (!seenConstructedBeforeRight && !seenConstructedBeforeLeft && _seenConstructed)
-                {
-                    Visit(node.Right);
-                }
-            }
-
-            public override void VisitAliasQualifiedName(AliasQualifiedNameSyntax node)
-            {
-                Visit(node.Name);
-            }
-
-            public override void VisitArrayType(ArrayTypeSyntax node)
-            {
-                _seenConstructed = true;
-                Visit(node.ElementType);
-            }
-
-            public override void VisitPointerType(PointerTypeSyntax node)
-            {
-                _seenConstructed = true;
-                Visit(node.ElementType);
-            }
-
-            public override void VisitNullableType(NullableTypeSyntax node)
-            {
-                _seenConstructed = true;
-                Visit(node.ElementType);
-            }
-        }
     }
 }

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -4932,6 +4932,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="IDS_FeatureNameof" xml:space="preserve">
     <value>nameof operator</value>
   </data>
+  <data name="IDS_FeatureUnboundGenericTypesInNameof" xml:space="preserve">
+    <value>unbound generic types in nameof operator</value>
+  </data>
   <data name="IDS_FeatureDictionaryInitializer" xml:space="preserve">
     <value>dictionary initializer</value>
   </data>

src/Compilers/CSharp/Portable/Errors/MessageID.cs

@@ -294,6 +294,7 @@ internal enum MessageID
         IDS_OverloadResolutionPriority = MessageBase + 12848,
 
         IDS_FeatureFirstClassSpan = MessageBase + 12849,
+        IDS_FeatureUnboundGenericTypesInNameof = MessageBase + 12850,
     }
 
     // Message IDs may refer to strings that need to be localized.
@@ -476,6 +477,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
                 // C# preview features.
                 case MessageID.IDS_FeatureFieldKeyword:
                 case MessageID.IDS_FeatureFirstClassSpan:
+                case MessageID.IDS_FeatureUnboundGenericTypesInNameof:
                     return LanguageVersion.Preview;
 
                 // C# 13.0 features.

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

@@ -6052,6 +6052,15 @@ private ScanTypeFlags ScanPossibleTypeArgumentList(
                     return result;
                 }
 
+                // Allow for any chain of errant commas in the generic name.  like `Dictionary<,int>` or
+                // `Dictionary<int,,>` We still want to think of these as generics, just with missing type-arguments, vs
+                // some invalid tree-expression that we would otherwise form.
+                if (this.CurrentToken.Kind == SyntaxKind.CommaToken)
+                {
+                    lastScannedType = default;
+                    continue;
+                }
+
                 lastScannedType = this.ScanType(out _);
                 switch (lastScannedType)
                 {