From 3f43775945e77354fcb2fa1987431b99265c9163 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Thu, 19 Oct 2023 01:05:04 +0200 Subject: [PATCH 1/9] analyze interpolated strings --- .../DapperAnalyzer.Diagnostics.cs | 3 +- .../CodeAnalysis/DapperAnalyzer.cs | 19 ++++++++++-- .../Internal/Inspection.cs | 19 ++++++++++-- .../Internal/Roslyn/SyntaxNodeExtensions.cs | 14 +++++++++ test/Dapper.AOT.Test/Verifiers/DAP241.cs | 29 +++++++++++++++++++ 5 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 src/Dapper.AOT.Analyzers/Internal/Roslyn/SyntaxNodeExtensions.cs create mode 100644 test/Dapper.AOT.Test/Verifiers/DAP241.cs diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs index 06b8735b..5394e8f1 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs @@ -89,6 +89,7 @@ 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 concatenated into SQL - use parameters instead"); } } diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs index 477c42b2..f57582f4 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs +++ b/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,21 @@ 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 syntaxKind)) + { + DiagnosticDescriptor? descriptor = syntaxKind switch + { + SyntaxKind.InterpolatedStringExpression or SyntaxKind.InterpolatedStringText + => Diagnostics.InterpolatedStringSqlExpression, + _ => 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 +476,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; } diff --git a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs index 1417f354..b858a65a 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs @@ -2,6 +2,7 @@ using Dapper.Internal.Roslyn; using Dapper.SqlAnalysis; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Operations; using System; using System.Collections.Generic; @@ -888,7 +889,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(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,7 +908,7 @@ internal static bool CouldBeNullable(ITypeSymbol symbol) => symbol.IsValueType ? symbol.NullableAnnotation == NullableAnnotation.Annotated : symbol.NullableAnnotation != NullableAnnotation.NotAnnotated; - public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value, out SyntaxNode? syntax) + public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value, out SyntaxNode? syntax, out SyntaxKind syntaxKind) { try { @@ -915,6 +916,7 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value { value = (T?)val.ConstantValue.Value; syntax = val.Syntax; + syntaxKind = syntax.GetKind(); return true; } if (val is IArgumentOperation arg) @@ -927,11 +929,20 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value val = conv.Operand; } + if (val is IInterpolatedStringOperation) + { + value = default!; + syntax = null; + syntaxKind = SyntaxKind.InterpolatedStringExpression; + 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 = syntax!.GetKind(); return true; } @@ -940,6 +951,7 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value { value = (T?)local.Local.ConstantValue; syntax = local.Local.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); + syntaxKind = syntax!.GetKind(); return true; } @@ -948,6 +960,7 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value { value = (T?)val.ConstantValue.Value; syntax = val.Syntax; + syntaxKind = syntax!.GetKind(); return true; } @@ -957,12 +970,14 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value // we already ruled out explicit constant above, so: must be default value = default; syntax = val.Syntax; + syntaxKind = syntax!.GetKind(); return true; } } catch { } value = default!; syntax = null; + syntaxKind = SyntaxKind.None; return false; } diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/SyntaxNodeExtensions.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/SyntaxNodeExtensions.cs new file mode 100644 index 00000000..a62ed7c9 --- /dev/null +++ b/src/Dapper.AOT.Analyzers/Internal/Roslyn/SyntaxNodeExtensions.cs @@ -0,0 +1,14 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; + +namespace Dapper.Internal.Roslyn +{ + internal static class SyntaxNodeExtensions + { + public static SyntaxKind GetKind(this SyntaxNode syntaxNode) + { + if (syntaxNode is null) return SyntaxKind.None; + return syntaxNode.Kind(); + } + } +} diff --git a/test/Dapper.AOT.Test/Verifiers/DAP241.cs b/test/Dapper.AOT.Test/Verifiers/DAP241.cs new file mode 100644 index 00000000..c26c2690 --- /dev/null +++ b/test/Dapper.AOT.Test/Verifiers/DAP241.cs @@ -0,0 +1,29 @@ +using Dapper.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; +using Diagnostics = Dapper.CodeAnalysis.DapperAnalyzer.Diagnostics; +namespace Dapper.AOT.Test.Verifiers; + +public class DAP241 : Verifier +{ + [Fact] + public Task InterpolatedStringDetection1() => CSVerifyAsync("""" + using Dapper; + using System.Data; + using System.Data.Common; + + [DapperAot] + class HasUnusedParameter + { + void SomeMethod(DbConnection connection) + { + int id = 1; + _ = connection.Query({|#0:$"select Id from Customers where Id = {id}"|}); + } + } + """", + DefaultConfig, + [ Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0) ] + ); + +} \ No newline at end of file From 15d8ecb605e70916d9b325f5708023510c25d6b2 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Thu, 19 Oct 2023 01:10:43 +0200 Subject: [PATCH 2/9] add test for local variable usage --- .../DapperAnalyzer.Diagnostics.cs | 2 +- test/Dapper.AOT.Test/Verifiers/DAP241.cs | 28 +++++++++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs index 5394e8f1..19852f23 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs @@ -90,6 +90,6 @@ public static readonly DiagnosticDescriptor 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"), - InterpolatedStringSqlExpression = SqlWarning("DAP241", "Interpolated string usage", "Data values should not be concatenated into SQL - use parameters instead"); + InterpolatedStringSqlExpression = SqlWarning("DAP241", "Interpolated string usage", "Data values should not be interpolated into SQL string - use parameters instead"); } } diff --git a/test/Dapper.AOT.Test/Verifiers/DAP241.cs b/test/Dapper.AOT.Test/Verifiers/DAP241.cs index c26c2690..45d81e34 100644 --- a/test/Dapper.AOT.Test/Verifiers/DAP241.cs +++ b/test/Dapper.AOT.Test/Verifiers/DAP241.cs @@ -7,7 +7,7 @@ namespace Dapper.AOT.Test.Verifiers; public class DAP241 : Verifier { [Fact] - public Task InterpolatedStringDetection1() => CSVerifyAsync("""" + public Task InterpolatedStringDetection_DirectUsage() => CSVerifyAsync("""" using Dapper; using System.Data; using System.Data.Common; @@ -23,7 +23,31 @@ void SomeMethod(DbConnection connection) } """", DefaultConfig, - [ Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0) ] + [ + Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0), + ] ); + [Fact] + public Task InterpolatedStringDetection_LocalVariableUsage() => CSVerifyAsync("""" + using Dapper; + using System.Data; + using System.Data.Common; + + [DapperAot] + class HasUnusedParameter + { + void SomeMethod(DbConnection connection) + { + int id = 1; + var sqlQuery = $"select Id from Customers where Id = {id}"; + _ = connection.Query({|#0:sqlQuery|}); + } + } + """", + DefaultConfig, + [ + Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0), + ] + ); } \ No newline at end of file From 35af2d129667cc432836eff717d7df83d2e493b6 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Sun, 22 Oct 2023 01:28:38 +0200 Subject: [PATCH 3/9] supported local var declaration --- .../Internal/Inspection.cs | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs index b858a65a..d12eb0c8 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs @@ -3,6 +3,7 @@ using Dapper.SqlAnalysis; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Operations; using System; using System.Collections.Generic; @@ -929,14 +930,6 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value val = conv.Operand; } - if (val is IInterpolatedStringOperation) - { - value = default!; - syntax = null; - syntaxKind = SyntaxKind.InterpolatedStringExpression; - return false; - } - // type-level constants if (val is IFieldReferenceOperation field && field.Field.HasConstantValue) { @@ -947,12 +940,35 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value } // local constants - if (val is ILocalReferenceOperation local && local.Local.HasConstantValue) + if (val is ILocalReferenceOperation local) { - value = (T?)local.Local.ConstantValue; - syntax = local.Local.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); - syntaxKind = syntax!.GetKind(); - return true; + if (local.Local.HasConstantValue) + { + value = (T?)local.Local.ConstantValue; + syntax = local.Local.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); + syntaxKind = syntax!.GetKind(); + return true; + } + + var referenceSyntax = local.Local?.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); + if (referenceSyntax is VariableDeclaratorSyntax varDeclSyntax) + { + if (varDeclSyntax.Initializer?.Value is InterpolatedStringExpressionSyntax) + { + value = default!; + syntax = null; + syntaxKind = SyntaxKind.InterpolatedStringExpression; + return false; + } + } + } + + if (val is IInterpolatedStringOperation) + { + value = default!; + syntax = null; + syntaxKind = SyntaxKind.InterpolatedStringExpression; + return false; } // other non-trivial default constant values From 1280a1a0ab0facc25de42aba22e72d745de57f33 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Sun, 22 Oct 2023 02:08:11 +0200 Subject: [PATCH 4/9] support concat strings --- .../DapperAnalyzer.Diagnostics.cs | 3 +- .../CodeAnalysis/DapperAnalyzer.cs | 2 + .../Internal/Inspection.cs | 31 ++++++++++-- test/Dapper.AOT.Test/Verifiers/DAP242.cs | 50 +++++++++++++++++++ 4 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 test/Dapper.AOT.Test/Verifiers/DAP242.cs diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs index 19852f23..3044453c 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs @@ -90,6 +90,7 @@ public static readonly DiagnosticDescriptor 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"), - InterpolatedStringSqlExpression = SqlWarning("DAP241", "Interpolated string usage", "Data values should not be interpolated into SQL string - use parameters instead"); + 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"); } } diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs index f57582f4..5fc16ecb 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs @@ -378,6 +378,8 @@ private void ValidateSql(in OperationAnalysisContext ctx, IOperation sqlSource, { SyntaxKind.InterpolatedStringExpression or SyntaxKind.InterpolatedStringText => Diagnostics.InterpolatedStringSqlExpression, + SyntaxKind.AddExpression + => Diagnostics.ConcatenatedStringSqlExpression, _ => null }; diff --git a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs index d12eb0c8..1985d168 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs @@ -953,12 +953,24 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value var referenceSyntax = local.Local?.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); if (referenceSyntax is VariableDeclaratorSyntax varDeclSyntax) { - if (varDeclSyntax.Initializer?.Value is InterpolatedStringExpressionSyntax) + var initializer = varDeclSyntax.Initializer?.Value; + if (initializer is not null) { - value = default!; - syntax = null; - syntaxKind = SyntaxKind.InterpolatedStringExpression; - return false; + if (initializer is InterpolatedStringExpressionSyntax) + { + value = default!; + syntax = null; + syntaxKind = SyntaxKind.InterpolatedStringExpression; + return false; + } + + if (initializer is BinaryExpressionSyntax) + { + value = default!; + syntax = null; + syntaxKind = SyntaxKind.AddExpression; + return false; + } } } } @@ -971,6 +983,15 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value return false; } + if (val is IBinaryOperation op) + { + // since we are parsing `string` here, `BinaryOperation` stands for string concatenation + value = default!; + syntax = null; + syntaxKind = SyntaxKind.AddExpression; + return false; + } + // other non-trivial default constant values if (val.ConstantValue.HasValue) { diff --git a/test/Dapper.AOT.Test/Verifiers/DAP242.cs b/test/Dapper.AOT.Test/Verifiers/DAP242.cs new file mode 100644 index 00000000..dca03f2f --- /dev/null +++ b/test/Dapper.AOT.Test/Verifiers/DAP242.cs @@ -0,0 +1,50 @@ +using Dapper.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; +using System.Threading.Tasks; +using Xunit; +using Diagnostics = Dapper.CodeAnalysis.DapperAnalyzer.Diagnostics; +namespace Dapper.AOT.Test.Verifiers; + +public class DAP242 : Verifier +{ + [Fact] + public Task ConcatenatedStringDetection_DirectUsage_PlusInt() => Test(""" + int id = 1; + _ = connection.Query({|#0:"select Id from Customers where Id = " + id|}); + """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + + [Fact] + public Task ConcatenatedStringDetection_DirectUsage_PlusString() => Test(""" + string id = "1"; + _ = connection.Query({|#0:"select Id from Customers where Id = " + id|}); + """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + + [Fact] + public Task ConcatenatedStringDetection_DirectUsage_MultiVariablesConcat() => Test(""" + int id = 1; + string name = "dima"; + _ = connection.Query({|#0:"select Id from Customers where Id = " + id + " and Name = " + name|}); + """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + + [Fact] + public Task ConcatenatedStringDetection_LocalVariableUsage() => Test(""" + int id = 1; + var sqlQuery = "select Id from Customers where Id = " + id; + _ = connection.Query({|#0:sqlQuery|}); + """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + + private Task Test(string methodCode, params DiagnosticResult[] expected) => CSVerifyAsync($$""" + using Dapper; + using System.Data; + using System.Data.Common; + + [DapperAot] + class HasUnusedParameter + { + void SomeMethod(DbConnection connection) + { + {{methodCode}} + } + } + """, DefaultConfig, expected); +} \ No newline at end of file From c61b69c41b7e665695297072ac553fb438cc3808 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Sun, 22 Oct 2023 02:33:19 +0200 Subject: [PATCH 5/9] support string.Format in direct usage --- src/Dapper.AOT.Analyzers/Internal/Inspection.cs | 16 ++++++++++++++++ test/Dapper.AOT.Test/Verifiers/DAP242.cs | 6 ++++++ 2 files changed, 22 insertions(+) diff --git a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs index 1985d168..26247e6e 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs @@ -966,6 +966,7 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value if (initializer is BinaryExpressionSyntax) { + // since we are parsing `string` here, `BinaryExpression` stands for string concatenation value = default!; syntax = null; syntaxKind = SyntaxKind.AddExpression; @@ -992,6 +993,21 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value return false; } + if (val is IInvocationOperation invocation) + { + if (invocation.TargetMethod is { + Name: "Format", + ContainingType: { SpecialType: SpecialType.System_String }, + ContainingNamespace: { Name: "System" } + }) + { + value = default!; + syntax = null; + syntaxKind = SyntaxKind.AddExpression; + return false; + } + } + // other non-trivial default constant values if (val.ConstantValue.HasValue) { diff --git a/test/Dapper.AOT.Test/Verifiers/DAP242.cs b/test/Dapper.AOT.Test/Verifiers/DAP242.cs index dca03f2f..4ed88d46 100644 --- a/test/Dapper.AOT.Test/Verifiers/DAP242.cs +++ b/test/Dapper.AOT.Test/Verifiers/DAP242.cs @@ -33,6 +33,12 @@ public Task ConcatenatedStringDetection_LocalVariableUsage() => Test(""" _ = connection.Query({|#0:sqlQuery|}); """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + [Fact] + public Task ConcatenatedStringDetection_StringFormat() => Test(""" + int id = 1; + _ = connection.Query({|#0:string.Format("select Id from Customers where Id = {0}", id)|}); + """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + private Task Test(string methodCode, params DiagnosticResult[] expected) => CSVerifyAsync($$""" using Dapper; using System.Data; From 5c59ca79aca4a3f220006e66844f1d2d03056cac Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Sun, 22 Oct 2023 02:41:52 +0200 Subject: [PATCH 6/9] explain special case --- src/Dapper.AOT.Analyzers/Internal/Inspection.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs index 26247e6e..8f3d0851 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs @@ -995,6 +995,7 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value if (val is IInvocationOperation invocation) { + // `string.Format()` special case if (invocation.TargetMethod is { Name: "Format", ContainingType: { SpecialType: SpecialType.System_String }, From 30aaab7c7b17cad4073fe3ca305f2bbf7713ab6d Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Sun, 22 Oct 2023 18:23:57 +0200 Subject: [PATCH 7/9] add docs + interpolated raw string literal syntax test --- docs/rules/DAP241.md | 28 ++++++++++++++++++++++++ docs/rules/DAP242.md | 28 ++++++++++++++++++++++++ test/Dapper.AOT.Test/Verifiers/DAP242.cs | 17 ++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 docs/rules/DAP241.md create mode 100644 docs/rules/DAP242.md diff --git a/docs/rules/DAP241.md b/docs/rules/DAP241.md new file mode 100644 index 00000000..67521c20 --- /dev/null +++ b/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 } +); +``` \ No newline at end of file diff --git a/docs/rules/DAP242.md b/docs/rules/DAP242.md new file mode 100644 index 00000000..19eeb3cd --- /dev/null +++ b/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 } +); +``` \ No newline at end of file diff --git a/test/Dapper.AOT.Test/Verifiers/DAP242.cs b/test/Dapper.AOT.Test/Verifiers/DAP242.cs index 4ed88d46..a25c0042 100644 --- a/test/Dapper.AOT.Test/Verifiers/DAP242.cs +++ b/test/Dapper.AOT.Test/Verifiers/DAP242.cs @@ -33,6 +33,23 @@ public Task ConcatenatedStringDetection_LocalVariableUsage() => Test(""" _ = connection.Query({|#0:sqlQuery|}); """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + [Fact] + public Task InterpolatedRawStringLiteralsDetection_DirectUsage() => Test("""" + int id = 1; + _ = connection.Query({|#0:$""" + select Id from Customers where Id = {id} + """|}); + """", Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0)); + + [Fact] + public Task InterpolatedRawStringLiteralsDetection_LocalVariableUsage() => Test("""" + int id = 1; + var sqlQuery = $""" + select Id from Customers where Id = {id} + """; + _ = connection.Query({|#0:sqlQuery|}); + """", Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0)); + [Fact] public Task ConcatenatedStringDetection_StringFormat() => Test(""" int id = 1; From 690cd6ab0a365d8fbc14fa18e261cbe64d54b5d0 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Sun, 22 Oct 2023 20:35:45 +0200 Subject: [PATCH 8/9] refactor to not use specific language in `Inspection` --- .../CodeAnalysis/DapperAnalyzer.cs | 8 +- .../Internal/Inspection.cs | 100 +++++------------- .../Internal/Roslyn/LanguageHelper.CSharp.cs | 50 +++++++++ .../Internal/Roslyn/LanguageHelper.Null.cs | 3 + .../Roslyn/LanguageHelper.VisualBasic.cs | 5 + .../Internal/Roslyn/LanguageHelper.cs | 4 + .../Internal/Roslyn/StringSyntaxKind.cs | 18 ++++ .../Internal/Roslyn/SyntaxNodeExtensions.cs | 14 --- 8 files changed, 111 insertions(+), 91 deletions(-) create mode 100644 src/Dapper.AOT.Analyzers/Internal/Roslyn/StringSyntaxKind.cs delete mode 100644 src/Dapper.AOT.Analyzers/Internal/Roslyn/SyntaxNodeExtensions.cs diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs index 5fc16ecb..7891900c 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs @@ -372,13 +372,13 @@ 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, out var syntaxKind)) + if (!TryGetConstantValueWithSyntax(sqlSource, out string? sql, out var sqlSyntax, out var stringSyntaxKind)) { - DiagnosticDescriptor? descriptor = syntaxKind switch + DiagnosticDescriptor? descriptor = stringSyntaxKind switch { - SyntaxKind.InterpolatedStringExpression or SyntaxKind.InterpolatedStringText + StringSyntaxKind.InterpolatedString => Diagnostics.InterpolatedStringSqlExpression, - SyntaxKind.AddExpression + StringSyntaxKind.ConcatenatedString or StringSyntaxKind.FormatString => Diagnostics.ConcatenatedStringSqlExpression, _ => null }; diff --git a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs index 8f3d0851..cb39eb98 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Inspection.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Inspection.cs @@ -2,8 +2,6 @@ using Dapper.Internal.Roslyn; using Dapper.SqlAnalysis; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Operations; using System; using System.Collections.Generic; @@ -909,7 +907,7 @@ internal static bool CouldBeNullable(ITypeSymbol symbol) => symbol.IsValueType ? symbol.NullableAnnotation == NullableAnnotation.Annotated : symbol.NullableAnnotation != NullableAnnotation.NotAnnotated; - public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value, out SyntaxNode? syntax, out SyntaxKind syntaxKind) + public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value, out SyntaxNode? syntax, out StringSyntaxKind? syntaxKind) { try { @@ -917,7 +915,7 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value { value = (T?)val.ConstantValue.Value; syntax = val.Syntax; - syntaxKind = syntax.GetKind(); + syntaxKind = val.TryDetectOperationStringSyntaxKind(); return true; } if (val is IArgumentOperation arg) @@ -930,83 +928,39 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value val = conv.Operand; } - // type-level constants - if (val is IFieldReferenceOperation field && field.Field.HasConstantValue) - { - value = (T?)field.Field.ConstantValue; - syntax = field.Field.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); - syntaxKind = syntax!.GetKind(); - return true; - } - - // local constants - if (val is ILocalReferenceOperation local) + if (!val.ConstantValue.HasValue) { - if (local.Local.HasConstantValue) + var stringSyntaxKind = val.TryDetectOperationStringSyntaxKind(); + switch (stringSyntaxKind) { - value = (T?)local.Local.ConstantValue; - syntax = local.Local.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); - syntaxKind = syntax!.GetKind(); - return true; - } - - var referenceSyntax = local.Local?.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); - if (referenceSyntax is VariableDeclaratorSyntax varDeclSyntax) - { - var initializer = varDeclSyntax.Initializer?.Value; - if (initializer is not null) + case StringSyntaxKind.ConcatenatedString: + case StringSyntaxKind.InterpolatedString: + case StringSyntaxKind.FormatString: { - if (initializer is InterpolatedStringExpressionSyntax) - { - value = default!; - syntax = null; - syntaxKind = SyntaxKind.InterpolatedStringExpression; - return false; - } - - if (initializer is BinaryExpressionSyntax) - { - // since we are parsing `string` here, `BinaryExpression` stands for string concatenation - value = default!; - syntax = null; - syntaxKind = SyntaxKind.AddExpression; - return false; - } + value = default!; + syntax = null; + syntaxKind = stringSyntaxKind; + return false; } } } - if (val is IInterpolatedStringOperation) - { - value = default!; - syntax = null; - syntaxKind = SyntaxKind.InterpolatedStringExpression; - return false; - } - - if (val is IBinaryOperation op) + // type-level constants + if (val is IFieldReferenceOperation field && field.Field.HasConstantValue) { - // since we are parsing `string` here, `BinaryOperation` stands for string concatenation - value = default!; - syntax = null; - syntaxKind = SyntaxKind.AddExpression; - return false; + value = (T?)field.Field.ConstantValue; + syntax = field.Field.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); + syntaxKind = val.TryDetectOperationStringSyntaxKind(); + return true; } - if (val is IInvocationOperation invocation) + // local constants + if (val is ILocalReferenceOperation local && local.Local.HasConstantValue) { - // `string.Format()` special case - if (invocation.TargetMethod is { - Name: "Format", - ContainingType: { SpecialType: SpecialType.System_String }, - ContainingNamespace: { Name: "System" } - }) - { - value = default!; - syntax = null; - syntaxKind = SyntaxKind.AddExpression; - return false; - } + value = (T?)local.Local.ConstantValue; + syntax = local.Local.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); + syntaxKind = val.TryDetectOperationStringSyntaxKind(); + return true; } // other non-trivial default constant values @@ -1014,7 +968,7 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value { value = (T?)val.ConstantValue.Value; syntax = val.Syntax; - syntaxKind = syntax!.GetKind(); + syntaxKind = val.TryDetectOperationStringSyntaxKind(); return true; } @@ -1024,14 +978,14 @@ public static bool TryGetConstantValueWithSyntax(IOperation val, out T? value // we already ruled out explicit constant above, so: must be default value = default; syntax = val.Syntax; - syntaxKind = syntax!.GetKind(); + syntaxKind = val.TryDetectOperationStringSyntaxKind(); return true; } } catch { } value = default!; syntax = null; - syntaxKind = SyntaxKind.None; + syntaxKind = StringSyntaxKind.NotRecognized; return false; } diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.CSharp.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.CSharp.cs index 5e74b3f2..705aa708 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.CSharp.cs +++ b/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,54 @@ internal override bool TryGetStringSpan(SyntaxToken token, string text, scoped i skip = take = 0; return false; } + + internal override StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation) + { + if (operation is null) return null; + if (operation is IBinaryOperation && operation.Type?.SpecialType == SpecialType.System_String) + { + return StringSyntaxKind.ConcatenatedString; + } + if (operation is IInterpolatedStringOperation) + { + return StringSyntaxKind.InterpolatedString; + } + if (operation is IInvocationOperation invocation) + { + // `string.Format()` usage + if (invocation.TargetMethod is + { + Name: "Format", + ContainingType: { SpecialType: SpecialType.System_String }, + ContainingNamespace: { Name: "System" } + }) + { + return StringSyntaxKind.FormatString; + } + } + if (operation is ILocalReferenceOperation localOperation) + { + 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) diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.Null.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.Null.cs index 796b059e..7375f8a3 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.Null.cs +++ b/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; } } diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.VisualBasic.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.VisualBasic.cs index f7301eb1..e9a6e1c5 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.VisualBasic.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.VisualBasic.cs @@ -53,5 +53,10 @@ internal override bool TryGetStringSpan(SyntaxToken token, string text, scoped i skip = take = 0; return false; } + + internal override StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation) + { + throw new NotImplementedException(); + } } } \ No newline at end of file diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.cs index 923ae5d9..ad1ad8c0 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.cs +++ b/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(); @@ -95,5 +98,6 @@ internal abstract partial class LanguageHelper internal abstract bool TryGetLiteralToken(SyntaxNode syntax, out SyntaxToken token); 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 StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation); internal abstract string GetDisplayString(ISymbol method); } \ No newline at end of file diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/StringSyntaxKind.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/StringSyntaxKind.cs new file mode 100644 index 00000000..e21dd4ce --- /dev/null +++ b/src/Dapper.AOT.Analyzers/Internal/Roslyn/StringSyntaxKind.cs @@ -0,0 +1,18 @@ +namespace Dapper.Internal.Roslyn +{ + /// + /// Represents a syntaxKind for the string usage + /// + internal enum StringSyntaxKind + { + NotRecognized, + + ConcatenatedString, + InterpolatedString, + + /// + /// Represents a syntax for `string.Format()` API + /// + FormatString + } +} diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/SyntaxNodeExtensions.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/SyntaxNodeExtensions.cs deleted file mode 100644 index a62ed7c9..00000000 --- a/src/Dapper.AOT.Analyzers/Internal/Roslyn/SyntaxNodeExtensions.cs +++ /dev/null @@ -1,14 +0,0 @@ -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; - -namespace Dapper.Internal.Roslyn -{ - internal static class SyntaxNodeExtensions - { - public static SyntaxKind GetKind(this SyntaxNode syntaxNode) - { - if (syntaxNode is null) return SyntaxKind.None; - return syntaxNode.Kind(); - } - } -} From 7a0b845618318154a38c869b139ad225d2c6f60e Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Sun, 22 Oct 2023 23:51:39 +0200 Subject: [PATCH 9/9] support VB --- .../Internal/Roslyn/LanguageHelper.CSharp.cs | 50 +++++---------- .../Roslyn/LanguageHelper.VisualBasic.cs | 42 ++++++++++++- .../Internal/Roslyn/LanguageHelper.cs | 29 ++++++++- test/Dapper.AOT.Test/Dapper.AOT.Test.csproj | 6 +- test/Dapper.AOT.Test/Verifiers/DAP241.VB.cs | 34 ++++++++++ test/Dapper.AOT.Test/Verifiers/DAP241.cs | 63 ++++++++++--------- test/Dapper.AOT.Test/Verifiers/DAP242.VB.cs | 47 ++++++++++++++ test/Dapper.AOT.Test/Verifiers/DAP242.cs | 45 +++++-------- 8 files changed, 216 insertions(+), 100 deletions(-) create mode 100644 test/Dapper.AOT.Test/Verifiers/DAP241.VB.cs create mode 100644 test/Dapper.AOT.Test/Verifiers/DAP242.VB.cs diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.CSharp.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.CSharp.cs index 705aa708..c055d10f 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.CSharp.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.CSharp.cs @@ -83,47 +83,27 @@ internal override bool TryGetStringSpan(SyntaxToken token, string text, scoped i internal override StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation) { - if (operation is null) return null; - if (operation is IBinaryOperation && operation.Type?.SpecialType == SpecialType.System_String) + if (operation is not ILocalReferenceOperation localOperation) { - return StringSyntaxKind.ConcatenatedString; + return base.TryDetectOperationStringSyntaxKind(operation); } - if (operation is IInterpolatedStringOperation) - { - return StringSyntaxKind.InterpolatedString; - } - if (operation is IInvocationOperation invocation) + + var referenceSyntax = localOperation.Local?.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); + if (referenceSyntax is VariableDeclaratorSyntax variableDeclaratorSyntax) { - // `string.Format()` usage - if (invocation.TargetMethod is - { - Name: "Format", - ContainingType: { SpecialType: SpecialType.System_String }, - ContainingNamespace: { Name: "System" } - }) + var initializer = variableDeclaratorSyntax.Initializer?.Value; + if (initializer is null) { - return StringSyntaxKind.FormatString; + return StringSyntaxKind.NotRecognized; } - } - if (operation is ILocalReferenceOperation localOperation) - { - var referenceSyntax = localOperation.Local?.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax(); - if (referenceSyntax is VariableDeclaratorSyntax variableDeclaratorSyntax) + if (initializer is InterpolatedStringExpressionSyntax) + { + return StringSyntaxKind.InterpolatedString; + } + if (initializer is BinaryExpressionSyntax) { - 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.ConcatenatedString; + } } return StringSyntaxKind.NotRecognized; diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.VisualBasic.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.VisualBasic.cs index e9a6e1c5..c511ca89 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.VisualBasic.cs +++ b/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; @@ -56,7 +58,45 @@ internal override bool TryGetStringSpan(SyntaxToken token, string text, scoped i internal override StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation) { - throw new NotImplementedException(); + 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; + } } } } \ No newline at end of file diff --git a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.cs b/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.cs index ad1ad8c0..cf131453 100644 --- a/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.cs +++ b/src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.cs @@ -98,6 +98,33 @@ internal abstract partial class LanguageHelper internal abstract bool TryGetLiteralToken(SyntaxNode syntax, out SyntaxToken token); 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 StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation); 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; + } } \ No newline at end of file diff --git a/test/Dapper.AOT.Test/Dapper.AOT.Test.csproj b/test/Dapper.AOT.Test/Dapper.AOT.Test.csproj index edf11ced..841ac5cf 100644 --- a/test/Dapper.AOT.Test/Dapper.AOT.Test.csproj +++ b/test/Dapper.AOT.Test/Dapper.AOT.Test.csproj @@ -21,7 +21,11 @@ $([System.String]::Copy(%(Filename)).Replace('.output.netfx', '.input.cs')) - + + $([System.String]::Copy(%(Filename)).Replace('.VB.cs', '.cs')) + + + diff --git a/test/Dapper.AOT.Test/Verifiers/DAP241.VB.cs b/test/Dapper.AOT.Test/Verifiers/DAP241.VB.cs new file mode 100644 index 00000000..5f784456 --- /dev/null +++ b/test/Dapper.AOT.Test/Verifiers/DAP241.VB.cs @@ -0,0 +1,34 @@ +using Dapper.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; +using Diagnostics = Dapper.CodeAnalysis.DapperAnalyzer.Diagnostics; + +namespace Dapper.AOT.Test.Verifiers; + +public partial class DAP241 +{ + [Fact] + public Task VBInterpolatedStringDetection_DirectUsage() => VerifyVBInterpolatedTest(""" + Dim id As Integer = 42 + conn.Execute({|#0:$"select Id from Customers where Id = {id}"|}) + """); + + [Fact] + public Task VBInterpolatedStringDetection_LocalVariableUsage() => VerifyVBInterpolatedTest(""" + Dim id As Integer = 42 + Dim sqlQuery As String = $"select Id from Customers where Id = {id}" + conn.Execute({|#0:sqlQuery|}) + """); + + private Task VerifyVBInterpolatedTest(string methodCode) => VBVerifyAsync($$""" + Imports Dapper + Imports System.Data.Common + + + Class SomeCode + Public Sub Foo(conn As DbConnection) + {{methodCode}} + End Sub + End Class + """, DefaultConfig, [Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0)]); +} \ No newline at end of file diff --git a/test/Dapper.AOT.Test/Verifiers/DAP241.cs b/test/Dapper.AOT.Test/Verifiers/DAP241.cs index 45d81e34..2e2ac20b 100644 --- a/test/Dapper.AOT.Test/Verifiers/DAP241.cs +++ b/test/Dapper.AOT.Test/Verifiers/DAP241.cs @@ -2,34 +2,42 @@ using System.Threading.Tasks; using Xunit; using Diagnostics = Dapper.CodeAnalysis.DapperAnalyzer.Diagnostics; + namespace Dapper.AOT.Test.Verifiers; -public class DAP241 : Verifier +public partial class DAP241 : Verifier { [Fact] - public Task InterpolatedStringDetection_DirectUsage() => CSVerifyAsync("""" - using Dapper; - using System.Data; - using System.Data.Common; - - [DapperAot] - class HasUnusedParameter - { - void SomeMethod(DbConnection connection) - { - int id = 1; - _ = connection.Query({|#0:$"select Id from Customers where Id = {id}"|}); - } - } - """", - DefaultConfig, - [ - Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0), - ] - ); + public Task CSharpInterpolatedStringDetection_DirectUsage() => VerifyCSharpInterpolatedTest(""" + int id = 1; + _ = connection.Query({|#0:$"select Id from Customers where Id = {id}"|}); + """); + + [Fact] + public Task InterpolatedRawStringLiteralsDetection_DirectUsage() => VerifyCSharpInterpolatedTest("""" + int id = 1; + _ = connection.Query({|#0:$""" + select Id from Customers where Id = {id} + """|}); + """"); + + [Fact] + public Task InterpolatedRawStringLiteralsDetection_LocalVariableUsage() => VerifyCSharpInterpolatedTest("""" + int id = 1; + var sqlQuery = $""" + select Id from Customers where Id = {id} + """; + _ = connection.Query({|#0:sqlQuery|}); + """"); [Fact] - public Task InterpolatedStringDetection_LocalVariableUsage() => CSVerifyAsync("""" + public Task InterpolatedStringDetection_LocalVariableUsage() => VerifyCSharpInterpolatedTest(""" + int id = 1; + var sqlQuery = $"select Id from Customers where Id = {id}"; + _ = connection.Query({|#0:sqlQuery|}); + """); + + private Task VerifyCSharpInterpolatedTest(string methodCode) => CSVerifyAsync($$""" using Dapper; using System.Data; using System.Data.Common; @@ -39,15 +47,8 @@ class HasUnusedParameter { void SomeMethod(DbConnection connection) { - int id = 1; - var sqlQuery = $"select Id from Customers where Id = {id}"; - _ = connection.Query({|#0:sqlQuery|}); + {{methodCode}} } } - """", - DefaultConfig, - [ - Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0), - ] - ); + """, DefaultConfig, [ Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0) ]); } \ No newline at end of file diff --git a/test/Dapper.AOT.Test/Verifiers/DAP242.VB.cs b/test/Dapper.AOT.Test/Verifiers/DAP242.VB.cs new file mode 100644 index 00000000..e92626d3 --- /dev/null +++ b/test/Dapper.AOT.Test/Verifiers/DAP242.VB.cs @@ -0,0 +1,47 @@ +using Dapper.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; +using Diagnostics = Dapper.CodeAnalysis.DapperAnalyzer.Diagnostics; + +namespace Dapper.AOT.Test.Verifiers; + +public partial class DAP242 : Verifier +{ + [Fact] + public Task VBConcatenatedStringDetection_DirectUsage_PlusInt() => VerifyVBInterpolatedTest(""" + Dim id As Integer = 42 + conn.Execute({|#0:"select Id from Customers where Id = " + id|}) + """); + + [Fact] + public Task VBConcatenatedStringDetection_DirectUsage_PlusString() => VerifyVBInterpolatedTest(""" + Dim id = "42" + conn.Execute({|#0:"select Id from Customers where Id = " + id|}) + """); + + [Fact] + public Task VBConcatenatedStringDetection_DirectUsage_MultiVariablesConcat() => VerifyVBInterpolatedTest(""" + Dim id = 42 + Dim name = "dima" + conn.Execute({|#0:"select Id from Customers where Id = " + id + " and name = " + name|}) + """); + + [Fact] + public Task VBConcatenatedStringDetection_LocalVariableUsage() => VerifyVBInterpolatedTest(""" + Dim id As Integer = 42 + Dim sqlQuery As String = "select Id from Customers where Id = " + id + conn.Execute({|#0:sqlQuery|}) + """); + + private Task VerifyVBInterpolatedTest(string methodCode) => VBVerifyAsync($$""" + Imports Dapper + Imports System.Data.Common + + + Class SomeCode + Public Sub Foo(conn As DbConnection) + {{methodCode}} + End Sub + End Class + """, DefaultConfig, [Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)]); +} \ No newline at end of file diff --git a/test/Dapper.AOT.Test/Verifiers/DAP242.cs b/test/Dapper.AOT.Test/Verifiers/DAP242.cs index a25c0042..fc4de346 100644 --- a/test/Dapper.AOT.Test/Verifiers/DAP242.cs +++ b/test/Dapper.AOT.Test/Verifiers/DAP242.cs @@ -1,62 +1,45 @@ using Dapper.CodeAnalysis; -using Microsoft.CodeAnalysis.Testing; using System.Threading.Tasks; using Xunit; using Diagnostics = Dapper.CodeAnalysis.DapperAnalyzer.Diagnostics; + namespace Dapper.AOT.Test.Verifiers; -public class DAP242 : Verifier +public partial class DAP242 : Verifier { [Fact] - public Task ConcatenatedStringDetection_DirectUsage_PlusInt() => Test(""" + public Task ConcatenatedStringDetection_DirectUsage_PlusInt() => VerifyCSharpConcatenatedTest(""" int id = 1; _ = connection.Query({|#0:"select Id from Customers where Id = " + id|}); - """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + """); [Fact] - public Task ConcatenatedStringDetection_DirectUsage_PlusString() => Test(""" + public Task ConcatenatedStringDetection_DirectUsage_PlusString() => VerifyCSharpConcatenatedTest(""" string id = "1"; _ = connection.Query({|#0:"select Id from Customers where Id = " + id|}); - """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + """); [Fact] - public Task ConcatenatedStringDetection_DirectUsage_MultiVariablesConcat() => Test(""" + public Task ConcatenatedStringDetection_DirectUsage_MultiVariablesConcat() => VerifyCSharpConcatenatedTest(""" int id = 1; string name = "dima"; _ = connection.Query({|#0:"select Id from Customers where Id = " + id + " and Name = " + name|}); - """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + """); [Fact] - public Task ConcatenatedStringDetection_LocalVariableUsage() => Test(""" + public Task ConcatenatedStringDetection_LocalVariableUsage() => VerifyCSharpConcatenatedTest(""" int id = 1; var sqlQuery = "select Id from Customers where Id = " + id; _ = connection.Query({|#0:sqlQuery|}); - """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); - - [Fact] - public Task InterpolatedRawStringLiteralsDetection_DirectUsage() => Test("""" - int id = 1; - _ = connection.Query({|#0:$""" - select Id from Customers where Id = {id} - """|}); - """", Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0)); - - [Fact] - public Task InterpolatedRawStringLiteralsDetection_LocalVariableUsage() => Test("""" - int id = 1; - var sqlQuery = $""" - select Id from Customers where Id = {id} - """; - _ = connection.Query({|#0:sqlQuery|}); - """", Diagnostic(Diagnostics.InterpolatedStringSqlExpression).WithLocation(0)); + """); [Fact] - public Task ConcatenatedStringDetection_StringFormat() => Test(""" + public Task ConcatenatedStringDetection_StringFormat() => VerifyCSharpConcatenatedTest(""" int id = 1; _ = connection.Query({|#0:string.Format("select Id from Customers where Id = {0}", id)|}); - """, Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0)); + """); - private Task Test(string methodCode, params DiagnosticResult[] expected) => CSVerifyAsync($$""" + private Task VerifyCSharpConcatenatedTest(string methodCode) => CSVerifyAsync($$""" using Dapper; using System.Data; using System.Data.Common; @@ -69,5 +52,5 @@ void SomeMethod(DbConnection connection) {{methodCode}} } } - """, DefaultConfig, expected); + """, DefaultConfig, [ Diagnostic(Diagnostics.ConcatenatedStringSqlExpression).WithLocation(0) ]); } \ No newline at end of file