Add a setting to turn off Ole Db "smell check" (#1974)

* Add a setting to turn off Ole Db "smell check" * Rename setting, fix logic * Test for SupportLegacyParameterTokens * Fix exception type * Add release note
DapperLib · Oct 11, 2023 · 19193b5 · 19193b5
1 parent 2837480
commit 19193b5
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 2 deletions.
diff --git a/Dapper/PublicAPI.Shipped.txt b/Dapper/PublicAPI.Shipped.txt
@@ -303,6 +303,8 @@ static Dapper.SqlMapper.SanitizeParameterValue(object? value) -> object!
 static Dapper.SqlMapper.SetDbType(System.Data.IDataParameter! parameter, object! value) -> void
 static Dapper.SqlMapper.Settings.ApplyNullValues.get -> bool
 static Dapper.SqlMapper.Settings.ApplyNullValues.set -> void
+static Dapper.SqlMapper.Settings.SupportLegacyParameterTokens.get -> bool
+static Dapper.SqlMapper.Settings.SupportLegacyParameterTokens.set -> void
 static Dapper.SqlMapper.Settings.CommandTimeout.get -> int?
 static Dapper.SqlMapper.Settings.CommandTimeout.set -> void
 static Dapper.SqlMapper.Settings.FetchSize.get -> long

diff --git a/Dapper/SqlMapper.Settings.cs b/Dapper/SqlMapper.Settings.cs
@@ -121,6 +121,13 @@ public static long FetchSize
                 }
             }
 
+            /// <summary>
+            /// Indicates whether single-character parameter tokens (<c>?</c> etc) will be detected and used where possible;
+            /// this feature is not recommended and will be disabled by default in future versions;
+            /// where possible, prefer named parameters (<c>@yourParam</c> etc) or Dapper's "pseudo-positional" parameters (<c>?yourParam? etc</c>).
+            /// </summary>
+            public static bool SupportLegacyParameterTokens { get; set; } = true;
+
             private static long s_FetchSize = -1;
         }
     }

diff --git a/Dapper/SqlMapper.cs b/Dapper/SqlMapper.cs
@@ -2534,11 +2534,13 @@ private static bool IsValueTuple(Type? type) => (type?.IsValueType == true
                 throw new NotSupportedException("ValueTuple should not be used for parameters - the language-level names are not available to use as parameter names, and it adds unnecessary boxing");
             }
 
-            bool filterParams = false;
-            if (removeUnused && identity.CommandType.GetValueOrDefault(CommandType.Text) == CommandType.Text)
+            bool filterParams = removeUnused && identity.CommandType.GetValueOrDefault(CommandType.Text) == CommandType.Text;
+
+            if (filterParams && Settings.SupportLegacyParameterTokens)
             {
                 filterParams = !smellsLikeOleDb.IsMatch(identity.Sql);
             }
+
             var dm = new DynamicMethod("ParamInfo" + Guid.NewGuid().ToString(), null, new[] { typeof(IDbCommand), typeof(object) }, type, true);
 
             var il = dm.GetILGenerator();

diff --git a/docs/index.md b/docs/index.md
@@ -25,6 +25,7 @@ Note: to get the latest pre-release build, add ` -Pre` to the end of the command
 (note: new PRs will not be merged until they add release note wording here)
 
 - infer command text without any whitespace as stored-procedure (#1975 via @mgravell)
+- add global `SupportLegacyParameterTokens` setting to enable or disable single-character parameter tokens (#1974 via @Giorgi)
 
 ### 2.1.4
 

diff --git a/tests/Dapper.Tests/ParameterTests.cs b/tests/Dapper.Tests/ParameterTests.cs
@@ -1511,6 +1511,36 @@ public void Issue601_InternationalParameterNamesWork()
         [FactLongRunning]
         public void TestListExpansionPadding_Disabled() => TestListExpansionPadding(false);
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void OleDbParamFilterFails(bool legacyParameterToken)
+        {
+            SqlMapper.PurgeQueryCache();
+            var oldValue = SqlMapper.Settings.SupportLegacyParameterTokens;
+            try
+            {
+                SqlMapper.Settings.SupportLegacyParameterTokens = legacyParameterToken;
+
+                if (legacyParameterToken) // OLE DB parameter support enabled; can false-positive
+                {
+                    Assert.Throws<NotSupportedException>(() => GetValue(connection));
+                }
+                else // OLE DB parameter support disabled; more reliable
+                {
+                    Assert.Equal("this ? could be awkward", GetValue(connection));
+                }
+            }
+            finally
+            {
+                SqlMapper.Settings.SupportLegacyParameterTokens = oldValue;
+            }
+
+            static string GetValue(DbConnection connection)
+                => connection.QuerySingle<string>("select 'this ? could be awkward'",
+                    new TypeWithDodgyProperties());
+        }
+
         private void TestListExpansionPadding(bool enabled)
         {
             bool oldVal = SqlMapper.Settings.PadListExpansions;
@@ -1706,5 +1736,10 @@ class HazNullableSqlDecimal
             public int Id { get; set; }
             public SqlDecimal? Value { get; set; }
         }
+
+        class TypeWithDodgyProperties
+        {
+            public string Name => throw new NotSupportedException();
+        }
     }
 }