Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support DbCommand command-text via constructor string sql query parse #135

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 46 additions & 8 deletions src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Dapper.Internal.Roslyn;
using Dapper.SqlAnalysis;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
DeagleGross marked this conversation as resolved.
Show resolved Hide resolved
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using System;
Expand Down Expand Up @@ -35,8 +36,11 @@
// per-run state (in particular, so we can have "first time only" diagnostics)
var state = new AnalyzerState(context);

// respond to method usages
context.RegisterOperationAction(state.OnOperation, OperationKind.Invocation, OperationKind.SimpleAssignment);
context.RegisterOperationAction(state.OnOperation,
OperationKind.Invocation, // for Dapper method invocations
OperationKind.SimpleAssignment, // for assignments of query
OperationKind.VariableDeclaration // for instantiating Command objects
);

// final actions
context.RegisterCompilationEndAction(state.OnCompilationEndAction);
Expand Down Expand Up @@ -111,8 +115,9 @@
try
{
// we'll look for:
// method calls with a parameter called "sql" or marked [Sql]
// property assignments to "CommandText"
// - method calls with a parameter called "sql" or marked [Sql]
// - property assignments to "CommandText"
// - allocation of SqlCommand (i.e. `new SqlCommand(queryString, ...)` )
switch (ctx.Operation.Kind)
{
case OperationKind.Invocation when ctx.Operation is IInvocationOperation invoke:
Expand Down Expand Up @@ -155,6 +160,39 @@
ValidatePropertyUsage(ctx, assignment.Value, false);
}
}
break;
case OperationKind.VariableDeclaration when ctx.Operation is IVariableDeclarationOperation variableDeclarationOperation
&& variableDeclarationOperation.Declarators.FirstOrDefault() is { } declarator
DeagleGross marked this conversation as resolved.
Show resolved Hide resolved
&& declarator.Initializer?.Value is IObjectCreationOperation objectCreationOperation:

var ctor = objectCreationOperation.Constructor;
var receiverType = ctor?.ReceiverType;

if (ctor is not null && receiverType is
{
Name: "SqlCommand",
ContainingNamespace:
{
Name: "SqlClient",
ContainingNamespace:
{
Name: "Data",
ContainingNamespace:
{
Name: "Microsoft",
ContainingNamespace.IsGlobalNamespace: true
DeagleGross marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
})
{
var sqlParam = ctor.Parameters.FirstOrDefault();
if (sqlParam is not null && sqlParam.Type.SpecialType == SpecialType.System_String && sqlParam.Name == "cmdText")
{
ValidateParameterUsage(ctx, objectCreationOperation.Arguments.First(), sqlUsage: objectCreationOperation);
}
}

break;
}
}
Expand Down Expand Up @@ -327,11 +365,11 @@
}
}

private void ValidateParameterUsage(in OperationAnalysisContext ctx, IOperation sqlSource)
private void ValidateParameterUsage(in OperationAnalysisContext ctx, IOperation sqlSource, IOperation sqlUsage = null)

Check warning on line 368 in src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Cannot convert null literal to non-nullable reference type.

Check warning on line 368 in src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Cannot convert null literal to non-nullable reference type.

Check warning on line 368 in src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Cannot convert null literal to non-nullable reference type.

Check warning on line 368 in src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Cannot convert null literal to non-nullable reference type.
{
// TODO: check other parameters for special markers like command type?
var flags = SqlParseInputFlags.None;
ValidateSql(ctx, sqlSource, flags, SqlParameters.None);
ValidateSql(ctx, sqlSource, flags, SqlParameters.None, sqlUsageOperation: sqlUsage);
}

private void ValidatePropertyUsage(in OperationAnalysisContext ctx, IOperation sqlSource, bool isCommand)
Expand Down Expand Up @@ -372,12 +410,12 @@
}

private void ValidateSql(in OperationAnalysisContext ctx, IOperation sqlSource, SqlParseInputFlags flags,
ImmutableArray<SqlParameter> parameters, Location? location = null)
ImmutableArray<SqlParameter> parameters, Location? location = null, IOperation sqlUsageOperation = null)

Check warning on line 413 in src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Cannot convert null literal to non-nullable reference type.

Check warning on line 413 in src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Cannot convert null literal to non-nullable reference type.

Check warning on line 413 in src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Cannot convert null literal to non-nullable reference type.

Check warning on line 413 in src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs

View workflow job for this annotation

GitHub Actions / build

Cannot convert null literal to non-nullable reference type.
{
var parseState = new ParseState(ctx);

// should we consider this as a syntax we can handle?
var syntax = IdentifySqlSyntax(parseState, ctx.Operation, out var caseSensitive) ?? DefaultSqlSyntax ?? SqlSyntax.General;
var syntax = IdentifySqlSyntax(parseState, sqlUsageOperation ?? ctx.Operation, out var caseSensitive) ?? DefaultSqlSyntax ?? SqlSyntax.General;
switch (syntax)
{
case SqlSyntax.SqlServer:
Expand Down
20 changes: 20 additions & 0 deletions src/Dapper.AOT.Analyzers/Internal/Inspection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1432,6 +1432,26 @@ internal static bool IsCommand(INamedTypeSymbol type)
}
}
}
else if (op is IObjectCreationOperation objectCreationOp)
{
var ctorTypeNamespace = objectCreationOp.Type?.ContainingNamespace;
var ctorTypeName = objectCreationOp.Type?.Name;

if (ctorTypeNamespace is not null && ctorTypeName is not null)
{
foreach (var candidate in KnownConnectionTypes)
{
var current = ctorTypeNamespace;
if (ctorTypeName == candidate.Command
&& AssertAndAscend(ref current, candidate.Namespace0)
&& AssertAndAscend(ref current, candidate.Namespace1)
&& AssertAndAscend(ref current, candidate.Namespace2))
{
return candidate.Syntax;
}
}
}
}

return null;

Expand Down
2 changes: 1 addition & 1 deletion src/Dapper.AOT.Analyzers/Internal/Roslyn/LanguageHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ internal virtual bool IsGlobalStatement(SyntaxNode syntax, out SyntaxNode? entry

internal virtual StringSyntaxKind? TryDetectOperationStringSyntaxKind(IOperation operation)
{
if (operation is null) return null;
if (operation is null || operation is ILiteralOperation) return null;
if (operation is IBinaryOperation)
{
return StringSyntaxKind.ConcatenatedString;
Expand Down
5 changes: 3 additions & 2 deletions test/Dapper.AOT.Test/Verifiers/SqlDetection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ static class Program
{
static void Main()
{
using var conn = new SqlConnection("my connection string here");
using var conn = new SqlConnection("my connection string here");
string name = "abc";
conn.{|#0:Execute|}("""
select Id, Name, Age
Expand All @@ -200,7 +200,7 @@ from Users
age = 24,
});

using var cmd = new SqlCommand("should ' verify this too", conn);
using var cmd = new SqlCommand("should {|#3:|}' verify this too", conn);
cmd.CommandText = """
select Id, Name, Age
from Users
Expand All @@ -217,6 +217,7 @@ from Users
Diagnostic(DapperAnalyzer.Diagnostics.ExecuteCommandWithQuery).WithLocation(0),
Diagnostic(DapperAnalyzer.Diagnostics.NullLiteralComparison).WithLocation(1),
Diagnostic(DapperAnalyzer.Diagnostics.NullLiteralComparison).WithLocation(2),
Diagnostic(DapperAnalyzer.Diagnostics.ParseError).WithLocation(3).WithArguments(46030, "Expected but did not find a closing quotation mark after the character string ' verify this too.")
], SqlSyntax.General, refDapperAot: false);

[Fact]
Expand Down