From 2f07f47adbe232722eada254d36a02454ca7705c Mon Sep 17 00:00:00 2001
From: Arthur Vickers <ajcvickers@hotmail.com>
Date: Tue, 18 Dec 2018 16:00:27 -0800
Subject: [PATCH] Fix scaffolding type mapper FixedLength bug

Fixes #14160 Superceeds #14162

ScafoldingTypeMapper should have been passing `false` when looking for the default mapping, since, non-fixed length is the default. (Compare this to Unicode, where we pass `true` because Unicode is the default.)

Also included is a fix for test, which wasn't Asserting for fixed length, as found by Smit.

Also fixed the code in RelationalTypeMappingInfo that was for some reason using parsed-length to drive the fixed-length flag, although this was a secondary issue.
---
 .../Internal/ScaffoldingTypeMapper.cs            |  4 ++--
 .../Storage/RelationalTypeMappingInfo.cs         | 16 ++++++----------
 .../ScaffoldingTypeMapperSqlServerTest.cs        |  5 +++--
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs b/src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs
index 593fd6e5b02..3b33bedd19d 100644
--- a/src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs
+++ b/src/EFCore.Design/Scaffolding/Internal/ScaffoldingTypeMapper.cs
@@ -72,7 +72,7 @@ public virtual TypeScaffoldingInfo FindMapping(
                         keyOrIndex,
                         rowVersion: rowVersion,
                         size: mapping.Size,
-                        fixedLength: true);
+                        fixedLength: false);
 
                     scaffoldFixedLength = fixedLengthMapping.IsFixedLength != byteArrayMapping.IsFixedLength ? (bool?)byteArrayMapping.IsFixedLength : null;
 
@@ -120,7 +120,7 @@ public virtual TypeScaffoldingInfo FindMapping(
                         keyOrIndex,
                         unicode: mapping.IsUnicode,
                         size: mapping.Size,
-                        fixedLength: true);
+                        fixedLength: false);
 
                     scaffoldFixedLength = fixedLengthMapping.IsFixedLength != stringMapping.IsFixedLength ? (bool?)stringMapping.IsFixedLength : null;
 
diff --git a/src/EFCore.Relational/Storage/RelationalTypeMappingInfo.cs b/src/EFCore.Relational/Storage/RelationalTypeMappingInfo.cs
index d95f0959b9a..ed9a5c39504 100644
--- a/src/EFCore.Relational/Storage/RelationalTypeMappingInfo.cs
+++ b/src/EFCore.Relational/Storage/RelationalTypeMappingInfo.cs
@@ -44,7 +44,7 @@ public RelationalTypeMappingInfo([NotNull] IReadOnlyList<IProperty> principals)
             _coreTypeMappingInfo = new TypeMappingInfo(principals);
 
             string storeTypeName = null;
-            var fixedLength = false;
+            bool? fixedLength = null;
             for (var i = 0; i < principals.Count; i++)
             {
                 var principal = principals[i];
@@ -57,13 +57,9 @@ public RelationalTypeMappingInfo([NotNull] IReadOnlyList<IProperty> principals)
                     }
                 }
 
-                if (!fixedLength)
+                if (fixedLength == null)
                 {
-                    var isFixedLength = principal.Relational().IsFixedLength;
-                    if (isFixedLength)
-                    {
-                        fixedLength = true;
-                    }
+                    fixedLength = principal.Relational().IsFixedLength;
                 }
             }
 
@@ -81,7 +77,7 @@ public RelationalTypeMappingInfo([NotNull] Type type)
             _coreTypeMappingInfo = new TypeMappingInfo(type);
             StoreTypeName = null;
             StoreTypeNameBase = null;
-            IsFixedLength = false;
+            IsFixedLength = null;
             _parsedSize = null;
             _parsedPrecision = null;
             _parsedScale = null;
@@ -100,7 +96,7 @@ public RelationalTypeMappingInfo([NotNull] string storeTypeName)
             _coreTypeMappingInfo = new TypeMappingInfo();
             StoreTypeName = storeTypeName;
             StoreTypeNameBase = ParseStoreTypeName(storeTypeName, out _parsedSize, out _parsedPrecision, out _parsedScale, out _isMax);
-            IsFixedLength = _parsedSize != null;
+            IsFixedLength = null;
         }
 
         /// <summary>
@@ -130,7 +126,7 @@ public RelationalTypeMappingInfo([NotNull] MemberInfo member)
                 _isMax = false;
             }
 
-            IsFixedLength = _parsedSize != null;
+            IsFixedLength = null;
         }
 
         /// <summary>
diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqlServerTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqlServerTest.cs
index ed1b662b094..8687318a5f6 100644
--- a/test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqlServerTest.cs
+++ b/test/EFCore.Design.Tests/Scaffolding/Internal/ScaffoldingTypeMapperSqlServerTest.cs
@@ -295,7 +295,7 @@ public void Maps_key_nchar_max_column()
         {
             var mapping = CreateMapper().FindMapping("nchar(max)", keyOrIndex: true, rowVersion: false);
 
-            AssertMapping<string>(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: true);
+            AssertMapping<string>(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: null);
         }
 
         [Fact]
@@ -311,7 +311,7 @@ public void Maps_key_char_max_column()
         {
             var mapping = CreateMapper().FindMapping("char(max)", keyOrIndex: true, rowVersion: false);
 
-            AssertMapping<string>(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: true);
+            AssertMapping<string>(mapping, inferred: false, maxLength: null, unicode: null, fixedLength: null);
         }
 
         [Fact]
@@ -352,6 +352,7 @@ private static void AssertMapping<T>(TypeScaffoldingInfo mapping, bool inferred,
             Assert.Equal(inferred, mapping.IsInferred);
             Assert.Equal(maxLength, mapping.ScaffoldMaxLength);
             Assert.Equal(unicode, mapping.ScaffoldUnicode);
+            Assert.Equal(fixedLength, mapping.ScaffoldFixedLength);
         }
 
         private static ScaffoldingTypeMapper CreateMapper()