CR feedback

src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Types.cs

@@ -418,7 +418,7 @@ private bool CanUseTupleTypeName(INamedTypeSymbol tupleSymbol)
 
         private static bool HasNonDefaultTupleElements(INamedTypeSymbol tupleSymbol)
         {
-           return tupleSymbol.TupleElements.Any(e=>(object)e != e.CorrespondingTupleField);
+            return tupleSymbol.TupleElements.Any(e => !e.IsDefaultTupleElement());
         }
 
         private void AddTupleTypeName(INamedTypeSymbol symbol)

src/Compilers/CSharp/Portable/Symbols/Tuples/TupleErrorFieldSymbol.cs

@@ -50,7 +50,7 @@ internal sealed class TupleErrorFieldSymbol : SynthesizedFieldSymbolBase
             _tupleElementIndex = (object)correspondingDefaultFieldOpt == null ? tupleElementIndex << 1 : (tupleElementIndex << 1) + 1;
             _isImplicitlyDeclared = isImplicitlyDeclared;
 
-            Debug.Assert(correspondingDefaultFieldOpt != null || this.IsDefaultTupleElement);
+            Debug.Assert((correspondingDefaultFieldOpt == null) == this.IsDefaultTupleElement);
             Debug.Assert(correspondingDefaultFieldOpt == null || correspondingDefaultFieldOpt.IsDefaultTupleElement);
 
             _correspondingDefaultField = correspondingDefaultFieldOpt ?? this;
@@ -126,6 +126,14 @@ public override bool IsImplicitlyDeclared
             }
         }
 
+        public override FieldSymbol CorrespondingTupleField
+        {
+            get
+            {
+                return _correspondingDefaultField;
+            }
+        }
+
         internal override bool SuppressDynamicAttribute
         {
             get

src/Compilers/CSharp/Portable/Symbols/Tuples/TupleFieldSymbol.cs

@@ -165,7 +165,7 @@ internal class TupleElementFieldSymbol : TupleFieldSymbol
             _locations = location == null ? ImmutableArray<Location>.Empty : ImmutableArray.Create(location);
             _isImplicitlyDeclared = isImplicitlyDeclared;
 
-            Debug.Assert(correspondingDefaultFieldOpt != null || this.IsDefaultTupleElement);
+            Debug.Assert((correspondingDefaultFieldOpt == null) == this.IsDefaultTupleElement);
             Debug.Assert(correspondingDefaultFieldOpt == null || correspondingDefaultFieldOpt.IsDefaultTupleElement);
 
             _correspondingDefaultField = correspondingDefaultFieldOpt ?? this;

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenTupleTest.cs

@@ -4843,7 +4843,7 @@ private static ImmutableArray<string> GetTupleElementNames(INamedTypeSymbol tupl
                 return default(ImmutableArray<string>);
             }
 
-            return elements.SelectAsArray(e => e.IsImplicitlyDeclared?  null: e.Name);
+            return elements.SelectAsArray(e => e.ProvidedTupleElementNameOrNull());
         }
 
         [Fact]
@@ -5116,10 +5116,11 @@ public void CreateTupleTypeSymbol_BadNames()
 
             NamedTypeSymbol intType = comp.GetSpecialType(SpecialType.System_Int32);
             var vt2 = comp.GetWellKnownType(WellKnownType.System_ValueTuple_T2).Construct(intType, intType);
+            var vt3 = comp.GetWellKnownType(WellKnownType.System_ValueTuple_T3).Construct(intType, intType, intType);
 
-            // illegal C# identifiers and blank
-            var tuple2 = comp.CreateTupleTypeSymbol(vt2, ImmutableArray.Create("123", " "));
-            Assert.Equal(new[] { "123", " " }, GetTupleElementNames(tuple2));
+            // illegal C# identifiers, space and null
+            var tuple2 = comp.CreateTupleTypeSymbol(vt3, ImmutableArray.Create("123", " ", null));
+            Assert.Equal(new[] { "123", " ", null }, GetTupleElementNames(tuple2));
 
             // reserved identifiers
             var tuple3 = comp.CreateTupleTypeSymbol(vt2, ImmutableArray.Create("return", "class"));
@@ -5137,6 +5138,28 @@ public void CreateTupleTypeSymbol_BadNames()
             }
         }
 
+        [Fact]
+        public void CreateTupleTypeSymbol_EmptyNames()
+        {
+            var comp = CSharpCompilation.Create("test", references: new[] { MscorlibRef });
+
+            NamedTypeSymbol intType = comp.GetSpecialType(SpecialType.System_Int32);
+            var vt2 = comp.GetWellKnownType(WellKnownType.System_ValueTuple_T2).Construct(intType, intType);
+
+            // not tuple-compatible underlying type
+            try
+            {
+                // illegal C# identifiers and blank
+                var tuple2 = comp.CreateTupleTypeSymbol(vt2, ImmutableArray.Create("123", ""));
+                Assert.True(false);
+            }
+            catch (ArgumentException e)
+            {
+                Assert.Contains(CodeAnalysisResources.TupleElementNameEmpty, e.Message);
+                Assert.Contains("1", e.Message);
+            }
+        }
+
         [Fact]
         public void CreateTupleTypeSymbol_VisualBasicElements()
         {

src/Compilers/Core/Portable/CodeAnalysisResources.Designer.cs

@@ -1333,6 +1333,15 @@ internal class CodeAnalysisResources {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Tuple element name cannot be an empty string..
+        /// </summary>
+        internal static string TupleElementNameEmpty {
+            get {
+                return ResourceManager.GetString("TupleElementNameEmpty", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Tuples must have at least two elements..
         /// </summary>

src/Compilers/Core/Portable/CodeAnalysisResources.resx

@@ -540,6 +540,9 @@
   <data name="TupleElementNameCountMismatch" xml:space="preserve">
     <value>If tuple element names are specified, the number of element names must match the cardinality of the tuple.</value>
   </data>
+  <data name="TupleElementNameEmpty" xml:space="preserve">
+    <value>Tuple element name cannot be an empty string.</value>
+  </data>
   <data name="TupleElementLocationCountMismatch" xml:space="preserve">
     <value>If tuple element locations are specified, the number of locations must match the cardinality of the tuple.</value>
   </data>

src/Compilers/Core/Portable/Compilation/Compilation.cs

@@ -878,6 +878,14 @@ protected static ImmutableArray<string> CheckTupleElementNames(int cardinality,
                     throw new ArgumentException(CodeAnalysisResources.TupleElementNameCountMismatch, nameof(elementNames));
                 }
 
+                for (int i = 0; i < elementNames.Length; i++)
+                {
+                    if (elementNames[i] == "")
+                    {
+                        throw new ArgumentException(CodeAnalysisResources.TupleElementNameEmpty, $"{nameof(elementNames)}[{i}]");
+                    }
+                }
+
                 if (elementNames.All(n => n == null))
                 {
                     return default(ImmutableArray<string>);

src/Compilers/Core/Portable/Symbols/ISymbolExtensions.cs

@@ -58,5 +58,35 @@ public static IMethodSymbol GetConstructedReducedFrom(this IMethodSymbol method)
 
             return reducedFrom.Construct(typeArgs);
         }
+
+        /// <summary>
+        /// Returns true if a given field is a nondefault tuple element
+        /// </summary>
+        internal static bool IsDefaultTupleElement(this IFieldSymbol field)
+        {
+            return (object)field == field.CorrespondingTupleField;
+        }
+
+        /// <summary>
+        /// Returns true if a given field is a tuple element
+        /// </summary>
+        internal static bool IsTupleElement(this IFieldSymbol field)
+        {
+            return (object)field.CorrespondingTupleField != null;
+        }
+
+        /// <summary>
+        /// Return the name of the field if the field is an explicitly named tuple element.
+        /// Otherwise returns null.
+        /// </summary>
+        /// <remarks>
+        /// Note that it is possible for an element to be both "Default" and to have a user provided name.
+        /// That could happen if the provided name matches the default name such as "Item10"
+        /// </remarks>
+        internal static string ProvidedTupleElementNameOrNull(this IFieldSymbol field)
+        {
+            return field.IsTupleElement() && !field.IsImplicitlyDeclared ? field.Name : null;
+        }
+
     }
 }

src/Compilers/VisualBasic/Portable/SymbolDisplay/SymbolDisplayVisitor.Types.vb

@@ -322,7 +322,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
         End Function
 
         Private Shared Function HasNonDefaultTupleElements(tupleSymbol As INamedTypeSymbol) As Boolean
-            Return tupleSymbol.TupleElements.Any(Function(e) e IsNot e.CorrespondingTupleField)
+            Return tupleSymbol.TupleElements.Any(Function(e) Not e.IsDefaultTupleElement)
         End Function
 
         Private Sub AddTupleTypeName(symbol As INamedTypeSymbol)

src/Compilers/VisualBasic/Portable/Symbols/Tuples/TupleErrorFieldSymbol.vb

@@ -47,7 +47,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
             Me._tupleElementIndex = If(correspondingDefaultFieldOpt Is Nothing, tupleElementIndex << 1, (tupleElementIndex << 1) + 1)
             Me._isImplicitlyDeclared = isImplicitlyDeclared
 
-            Debug.Assert(correspondingDefaultFieldOpt IsNot Nothing OrElse Me.IsDefaultTupleElement)
+            Debug.Assert(correspondingDefaultFieldOpt Is Nothing = Me.IsDefaultTupleElement)
             Debug.Assert(correspondingDefaultFieldOpt Is Nothing OrElse correspondingDefaultFieldOpt.IsDefaultTupleElement)
 
             _correspondingDefaultField = If(correspondingDefaultFieldOpt, Me)
@@ -107,6 +107,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
             End Get
         End Property
 
+        Public Overrides ReadOnly Property CorrespondingTupleField As FieldSymbol
+            Get
+                Return _correspondingDefaultField
+            End Get
+        End Property
+
         Public Overrides ReadOnly Property Type As TypeSymbol
             Get
                 Return Me._type

src/Compilers/VisualBasic/Portable/Symbols/Tuples/TupleFieldSymbol.vb

@@ -143,7 +143,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
             Me._locations = If((location Is Nothing), ImmutableArray(Of Location).Empty, ImmutableArray.Create(Of Location)(location))
             Me._isImplicitlyDeclared = isImplicitlyDeclared
 
-            Debug.Assert(correspondingDefaultFieldOpt IsNot Nothing OrElse Me.IsDefaultTupleElement)
+            Debug.Assert(correspondingDefaultFieldOpt Is Nothing = Me.IsDefaultTupleElement)
             Debug.Assert(correspondingDefaultFieldOpt Is Nothing OrElse correspondingDefaultFieldOpt.IsDefaultTupleElement)
 
             _correspondingDefaultField = If(correspondingDefaultFieldOpt, Me)

src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenTuples.vb

@@ -6496,10 +6496,11 @@ End Class
 
             Dim intType As NamedTypeSymbol = comp.GetSpecialType(SpecialType.System_Int32)
             Dim vt2 = comp.GetWellKnownType(WellKnownType.System_ValueTuple_T2).Construct(intType, intType)
+            Dim vt3 = comp.GetWellKnownType(WellKnownType.System_ValueTuple_T3).Construct(intType, intType, intType)
 
-            ' Illegal VB identifier and blank
-            Dim tuple2 = comp.CreateTupleTypeSymbol(vt2, ImmutableArray.Create("123", " "))
-            Assert.Equal({"123", " "}, GetTupleElementNames(tuple2))
+            ' Illegal VB identifier, space and null
+            Dim tuple2 = comp.CreateTupleTypeSymbol(vt3, ImmutableArray.Create("123", " ", Nothing))
+            Assert.Equal({"123", " ", Nothing}, GetTupleElementNames(tuple2))
 
             ' Reserved keywords
             Dim tuple3 = comp.CreateTupleTypeSymbol(vt2, ImmutableArray.Create("return", "class"))
@@ -6514,6 +6515,25 @@ End Class
 
         End Sub
 
+        <Fact>
+        Public Sub CreateTupleTypeSymbol_EmptyNames()
+
+            Dim comp = VisualBasicCompilation.Create("test", references:={MscorlibRef})
+
+            Dim intType As NamedTypeSymbol = comp.GetSpecialType(SpecialType.System_Int32)
+            Dim vt2 = comp.GetWellKnownType(WellKnownType.System_ValueTuple_T2).Construct(intType, intType)
+
+            Try
+                ' Illegal VB identifier and empty
+                Dim tuple2 = comp.CreateTupleTypeSymbol(vt2, ImmutableArray.Create("123", ""))
+                Assert.True(False)
+            Catch ex As ArgumentException
+                Assert.Contains(CodeAnalysisResources.TupleElementNameEmpty, ex.Message)
+                Assert.Contains("1", ex.Message)
+            End Try
+
+        End Sub
+
         <Fact>
         Public Sub CreateTupleTypeSymbol_CSharpElements()
 
@@ -6787,7 +6807,7 @@ End Class
                 Return Nothing
             End If
 
-            Return elements.SelectAsArray(Function(e) If(e.IsImplicitlyDeclared, Nothing, e.Name))
+            Return elements.SelectAsArray(Function(e) e.ProvidedTupleElementNameOrNull)
         End Function
 
         <Fact>

src/Workspaces/Core/Portable/Shared/Utilities/SymbolEquivalenceComparer.GetHashCodeVisitor.cs

@@ -189,12 +189,7 @@ private int CombineNamedTypeHashCode(INamedTypeSymbol x, int currentHash)
             {
                 if (x.IsTupleType)
                 {
-                    foreach(var element in x.TupleElements)
-                    {
-                        currentHash = Hash.Combine(element.Type, currentHash);
-                    }
-
-                    return currentHash;
+                    return Hash.Combine(currentHash, Hash.CombineValues(x.TupleElements));
                 }
 
                 // If we want object and dynamic to be the same, and this is 'object', then return

src/Workspaces/Core/Portable/SymbolKey/SymbolKey.TupleTypeSymbolKey.cs

@@ -15,16 +15,16 @@ public static void Create(INamedTypeSymbol symbol, SymbolKeyWriter visitor)
                 Debug.Assert(symbol.IsTupleType);
                 visitor.WriteSymbolKey(symbol.TupleUnderlyingType);
 
-                var frieldlyNames = ArrayBuilder<String>.GetInstance();
+                var friendlyNames = ArrayBuilder<String>.GetInstance();
                 var locations = ArrayBuilder<Location>.GetInstance();
 
-                foreach(var element in symbol.TupleElements)
+                foreach (var element in symbol.TupleElements)
                 {
-                    frieldlyNames.Add(element.IsImplicitlyDeclared? null: element.Name);
+                    friendlyNames.Add(element.IsImplicitlyDeclared ? null : element.Name);
                     locations.Add(element.Locations.FirstOrDefault());
                 }
 
-                visitor.WriteStringArray(frieldlyNames.ToImmutableAndFree());
+                visitor.WriteStringArray(friendlyNames.ToImmutableAndFree());
                 visitor.WriteLocationArray(locations.ToImmutableAndFree());
             }