feat: trivial parameterization anti-patterns analysis support (Dapper…

…Lib#61)

* analyze interpolated strings

* add test for local variable usage

* supported local var declaration

* support concat strings

* support string.Format in direct usage

* explain special case

* add docs + interpolated raw string literal syntax test

* refactor to not use specific language in `Inspection`

* support VB

docs/rules/DAP241.md

@@ -0,0 +1,28 @@
+# DAP241
+
+Dapper allows writing [parameterized queries](https://github.com/DapperLib/Dapper/blob/main/Readme.md#parameterized-queries),
+but developer should **never interpolate** values into the query string (`sql` parameter for any Dapper-method).
+
+Bad:
+
+``` csharp
+var id = 42;
+conn.Execute($"select Id from Customers where Id = {id}");
+```
+
+Instead the intended way is to pass the anynomous object, 
+members of which will be mapped into the query parameter using the same name.
+
+In example for the object `new { A = 42, B = "hello world" }`:
+- member `A` will be mapped into the parameter `@A`
+- member `B` will be mapped into the parameter `@B`
+
+Good:
+
+``` csharp
+var id = 42;
+conn.Execute(
+	$"select Id from Customers where Id = @queryId",
+	new { queryId = id }
+);
+```

docs/rules/DAP242.md

@@ -0,0 +1,28 @@
+# DAP242
+
+Dapper allows writing [parameterized queries](https://github.com/DapperLib/Dapper/blob/main/Readme.md#parameterized-queries),
+but developer should **never concatenate** values into the query string (`sql` parameter for any Dapper-method).
+
+Bad:
+
+``` csharp
+var id = 42;
+conn.Execute("select Id from Customers where Id = " + id);
+```
+
+Instead the intended way is to pass the anynomous object, 
+members of which will be mapped into the query parameter using the same name.
+
+In example for the object `new { A = 42, B = "hello world" }`:
+- member `A` will be mapped into the parameter `@A`
+- member `B` will be mapped into the parameter `@B`
+
+Good:
+
+``` csharp
+var id = 42;
+conn.Execute(
+	$"select Id from Customers where Id = @queryId",
+	new { queryId = id }
+);
+```

src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs

@@ -89,6 +89,8 @@ public static readonly DiagnosticDescriptor
         DivideByZero = SqlWarning("DAP237", "Divide by zero", "Division by zero detected"),
         TrivialOperand = SqlWarning("DAP238", "Trivial operand", "Operand makes this calculation trivial; it can be simplified"),
         InvalidNullExpression = SqlWarning("DAP239", "Invalid null expression", "Operation requires a non-null operand"),
-        VariableParameterConflict = SqlError("DAP240", "Parameter/variable conflict", "The declaration of variable '{0}' conflicts with a parameter");
+        VariableParameterConflict = SqlError("DAP240", "Parameter/variable conflict", "The declaration of variable '{0}' conflicts with a parameter"),
+        InterpolatedStringSqlExpression = SqlWarning("DAP241", "Interpolated string usage", "Data values should not be interpolated into SQL string - use parameters instead"),
+        ConcatenatedStringSqlExpression = SqlWarning("DAP242", "Concatenated string usage", "Data values should not be concatenated into SQL string - use parameters instead");
     }
 }

src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs

@@ -2,6 +2,7 @@
 using Dapper.Internal.Roslyn;
 using Dapper.SqlAnalysis;
 using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Operations;
 using System;
@@ -371,7 +372,23 @@ private void ValidateSql(in OperationAnalysisContext ctx, IOperation sqlSource,
             if (caseSensitive) flags |= SqlParseInputFlags.CaseSensitive;
 
             // can we get the SQL itself?
-            if (!TryGetConstantValueWithSyntax(sqlSource, out string? sql, out var sqlSyntax)) return;
+            if (!TryGetConstantValueWithSyntax(sqlSource, out string? sql, out var sqlSyntax, out var stringSyntaxKind))
+            {
+                DiagnosticDescriptor? descriptor = stringSyntaxKind switch
+                {
+                    StringSyntaxKind.InterpolatedString 
+                        => Diagnostics.InterpolatedStringSqlExpression,
+                    StringSyntaxKind.ConcatenatedString or StringSyntaxKind.FormatString 
+                        => Diagnostics.ConcatenatedStringSqlExpression,
+                    _ => null
+                };
+
+                if (descriptor is not null)
+                {
+                    ctx.ReportDiagnostic(Diagnostic.Create(descriptor, sqlSource.Syntax.GetLocation()));
+                }
+                return;
+            }
             if (string.IsNullOrWhiteSpace(sql) || !HasWhitespace.IsMatch(sql)) return; // need non-trivial content to validate
 
             location ??= ctx.Operation.Syntax.GetLocation();
@@ -461,7 +478,7 @@ internal static Location SharedParseArgsAndFlags(in ParseState ctx, IInvocationO
             switch (arg.Parameter?.Name)
             {
                 case "sql":
-                    if (TryGetConstantValueWithSyntax(arg, out string? s, out _))
+                    if (TryGetConstantValueWithSyntax(arg, out string? s, out _, out _))
                     {
                         sql = s;
                     }

src/Dapper.AOT.Analyzers/Internal/Inspection.cs

@@ -888,7 +888,7 @@ public static bool IsDapperMethod(this IInvocationOperation operation, out Opera
     public static bool HasAll(this OperationFlags value, OperationFlags testFor) => (value & testFor) == testFor;
 
     public static bool TryGetConstantValue<T>(IOperation op, out T? value)
-            => TryGetConstantValueWithSyntax(op, out value, out _);
+            => TryGetConstantValueWithSyntax(op, out value, out _, out _);
 
     public static ITypeSymbol? GetResultType(this IInvocationOperation invocation, OperationFlags flags)
     {
@@ -907,14 +907,15 @@ internal static bool CouldBeNullable(ITypeSymbol symbol) => symbol.IsValueType
         ? symbol.NullableAnnotation == NullableAnnotation.Annotated
         : symbol.NullableAnnotation != NullableAnnotation.NotAnnotated;
 
-    public static bool TryGetConstantValueWithSyntax<T>(IOperation val, out T? value, out SyntaxNode? syntax)
+    public static bool TryGetConstantValueWithSyntax<T>(IOperation val, out T? value, out SyntaxNode? syntax, out StringSyntaxKind? syntaxKind)
     {
         try
         {
             if (val.ConstantValue.HasValue)
             {
                 value = (T?)val.ConstantValue.Value;
                 syntax = val.Syntax;
+                syntaxKind = val.TryDetectOperationStringSyntaxKind();
                 return true;
             }
             if (val is IArgumentOperation arg)
@@ -927,11 +928,29 @@ public static bool TryGetConstantValueWithSyntax<T>(IOperation val, out T? value
                 val = conv.Operand;
             }
 
+            if (!val.ConstantValue.HasValue)
+            {
+                var stringSyntaxKind = val.TryDetectOperationStringSyntaxKind();
+                switch (stringSyntaxKind)
+                {
+                    case StringSyntaxKind.ConcatenatedString:
+                    case StringSyntaxKind.InterpolatedString:
+                    case StringSyntaxKind.FormatString:
+                    {
+                        value = default!;
+                        syntax = null;
+                        syntaxKind = stringSyntaxKind;
+                        return false;
+                    }
+                }
+            }
+
             // type-level constants
             if (val is IFieldReferenceOperation field && field.Field.HasConstantValue)
             {
                 value = (T?)field.Field.ConstantValue;
                 syntax = field.Field.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax();
+                syntaxKind = val.TryDetectOperationStringSyntaxKind();
                 return true;
             }
 
@@ -940,6 +959,7 @@ public static bool TryGetConstantValueWithSyntax<T>(IOperation val, out T? value
             {
                 value = (T?)local.Local.ConstantValue;
                 syntax = local.Local.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax();
+                syntaxKind = val.TryDetectOperationStringSyntaxKind();
                 return true;
             }
 
@@ -948,6 +968,7 @@ public static bool TryGetConstantValueWithSyntax<T>(IOperation val, out T? value
             {
                 value = (T?)val.ConstantValue.Value;
                 syntax = val.Syntax;
+                syntaxKind = val.TryDetectOperationStringSyntaxKind();
                 return true;
             }
 
@@ -957,12 +978,14 @@ public static bool TryGetConstantValueWithSyntax<T>(IOperation val, out T? value
                 // we already ruled out explicit constant above, so: must be default
                 value = default;
                 syntax = val.Syntax;
+                syntaxKind = val.TryDetectOperationStringSyntaxKind();
                 return true;
             }
         }
         catch { }
         value = default!;
         syntax = null;
+        syntaxKind = StringSyntaxKind.NotRecognized;
         return false;
     }
 

src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.CSharp.cs

@@ -2,7 +2,9 @@
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Operations;
 using System;
+using System.Linq;
 
 namespace Dapper.Internal.Roslyn;
 partial class LanguageHelper
@@ -78,6 +80,34 @@ internal override bool TryGetStringSpan(SyntaxToken token, string text, scoped i
             skip = take = 0;
             return false;
         }
+
+        internal override StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation)
+        {
+            if (operation is not ILocalReferenceOperation localOperation)
+            {
+                return base.TryDetectOperationStringSyntaxKind(operation);
+            }
+
+            var referenceSyntax = localOperation.Local?.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax();
+            if (referenceSyntax is VariableDeclaratorSyntax variableDeclaratorSyntax)
+            {
+                var initializer = variableDeclaratorSyntax.Initializer?.Value;
+                if (initializer is null)
+                {
+                    return StringSyntaxKind.NotRecognized;
+                }
+                if (initializer is InterpolatedStringExpressionSyntax)
+                {
+                    return StringSyntaxKind.InterpolatedString;
+                }
+                if (initializer is BinaryExpressionSyntax)
+                {
+                    return StringSyntaxKind.ConcatenatedString;
+                }
+            }
+
+            return StringSyntaxKind.NotRecognized;
+        }
     }
 
     static int CountQuotes(string text)

src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.Null.cs

@@ -26,5 +26,8 @@ internal override bool TryGetStringSpan(SyntaxToken syntax, string text, scoped
 
         internal override string GetDisplayString(ISymbol symbol)
             => symbol?.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)!;
+
+        internal override StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation)
+            => null;
     }
 }

src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.VisualBasic.cs

@@ -1,8 +1,10 @@
 using Dapper.SqlAnalysis;
 using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Operations;
 using Microsoft.CodeAnalysis.VisualBasic;
 using Microsoft.CodeAnalysis.VisualBasic.Syntax;
 using System;
+using System.Linq;
 
 namespace Dapper.Internal.Roslyn;
 
@@ -53,5 +55,48 @@ internal override bool TryGetStringSpan(SyntaxToken token, string text, scoped i
             skip = take = 0;
             return false;
         }
+
+        internal override StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation)
+        {
+            if (operation is not ILocalReferenceOperation localOperation)
+            {
+                return base.TryDetectOperationStringSyntaxKind(operation);
+            }
+
+            var referenceSyntax = localOperation.Local?.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax();
+            var variableDeclaratorSyntax = FindVariableDeclarator();
+            if (variableDeclaratorSyntax is not null)
+            {
+                var initializer = variableDeclaratorSyntax.Initializer?.Value;
+                if (initializer is null)
+                {
+                    return StringSyntaxKind.NotRecognized;
+                }
+                if (initializer is InterpolatedStringExpressionSyntax)
+                {
+                    return StringSyntaxKind.InterpolatedString;
+                }
+                if (initializer is BinaryExpressionSyntax)
+                {
+                    return StringSyntaxKind.ConcatenatedString;
+                }
+            }
+
+            return StringSyntaxKind.NotRecognized;
+
+            VariableDeclaratorSyntax? FindVariableDeclarator()
+            {
+                if (referenceSyntax is ModifiedIdentifierSyntax modifiedIdentifierSyntax)
+                {
+                    if (modifiedIdentifierSyntax.Parent is VariableDeclaratorSyntax)
+                    {
+                        return (VariableDeclaratorSyntax)modifiedIdentifierSyntax.Parent;
+                    }
+                }
+
+                if (referenceSyntax is VariableDeclaratorSyntax varDeclar) return varDeclar;
+                return null;
+            }
+        }
     }
 }

src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.cs

@@ -26,6 +26,9 @@ public static bool IsMemberAccess(this SyntaxNode syntax)
     public static bool IsMethodDeclaration(this SyntaxNode syntax)
         => GetHelper(syntax.Language).IsMethodDeclaration(syntax);
 
+    public static StringSyntaxKind? TryDetectOperationStringSyntaxKind(this IOperation operation)
+        => GetHelper(operation.Syntax?.Language).TryDetectOperationStringSyntaxKind(operation);
+
     public static Location ComputeLocation(this SyntaxToken token, scoped in TSqlProcessor.Location location)
     {
         var origin = token.GetLocation();
@@ -96,4 +99,32 @@ internal abstract partial class LanguageHelper
     internal abstract bool TryGetStringSpan(SyntaxToken token, string text, scoped in TSqlProcessor.Location location, out int skip, out int take);
     internal abstract bool IsName(SyntaxNode syntax);
     internal abstract string GetDisplayString(ISymbol method);
+
+    internal virtual StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation)
+    {
+        if (operation is null) return null;
+        if (operation is IBinaryOperation)
+        {
+            return StringSyntaxKind.ConcatenatedString;
+        }
+        if (operation is IInterpolatedStringOperation)
+        {
+            return StringSyntaxKind.InterpolatedString;
+        }
+        if (operation is IInvocationOperation invocation)
+        {
+            // `string.Format()`
+            if (invocation.TargetMethod is
+                {
+                    Name: "Format",
+                    ContainingType: { SpecialType: SpecialType.System_String },
+                    ContainingNamespace: { Name: "System" }
+                })
+            {
+                return StringSyntaxKind.FormatString;
+            }
+        }
+
+        return StringSyntaxKind.NotRecognized;
+    }
 }

src/Dapper.AOT.Analyzers/Internal/Roslyn/StringSyntaxKind.cs

@@ -0,0 +1,18 @@
+namespace Dapper.Internal.Roslyn
+{
+    /// <summary>
+    /// Represents a syntaxKind for the string usage
+    /// </summary>
+    internal enum StringSyntaxKind
+    {
+        NotRecognized,
+
+        ConcatenatedString,
+        InterpolatedString,
+
+        /// <summary>
+        /// Represents a syntax for `string.Format()` API
+        /// </summary>
+        FormatString
+    }
+}

test/Dapper.AOT.Test/Dapper.AOT.Test.csproj

@@ -21,7 +21,11 @@
         <None Update="**/*.output.netfx.*">
             <DependentUpon>$([System.String]::Copy(%(Filename)).Replace('.output.netfx', '.input.cs'))</DependentUpon>
         </None>
-    </ItemGroup>
+		<None Update="**/*DAP*.*.cs*">
+			<DependentUpon>$([System.String]::Copy(%(Filename)).Replace('.VB.cs', '.cs'))</DependentUpon>
+		</None>
+
+	</ItemGroup>
     <ItemGroup>
         <PackageReference Include="Dapper" Condition="'$(TargetFramework)'!='net48'" />
         <PackageReference Include="Dapper.StrongName" Condition="'$(TargetFramework)'=='net48'" />