From ad9fc039a73c034fe7827155b2080aff45396ada Mon Sep 17 00:00:00 2001 From: m_axwel_l Date: Fri, 1 May 2026 10:50:58 +0500 Subject: [PATCH] Detect explicit string.Format/string.Concat in raw SQL APIs (EF1003) The EF1003 analyzer warns when a raw SQL API receives a dynamically built string. It already covered interpolated strings (EF1002) and the binary `+` concatenation operator (EF1003), but explicit calls such as db.Database.ExecuteSqlRaw(string.Format("UPDATE T SET X={0}", id)); db.Users.FromSqlRaw(string.Concat("SELECT * FROM T WHERE Id=", id)); were silently allowed. Extend the analyzer to also report these when at least one argument is non-constant. Implicit conversions (object boxing in the `string.Format(string, object)` overloads) are unwrapped so that fully constant calls do not trigger the warning. Fixes #37915 --- ...ingsUsageInRawQueriesDiagnosticAnalyzer.cs | 56 ++++++++++ ...gConcatenationInRawQueriesAnalyzerTests.cs | 104 ++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/src/EFCore.Analyzers/StringsUsageInRawQueriesDiagnosticAnalyzer.cs b/src/EFCore.Analyzers/StringsUsageInRawQueriesDiagnosticAnalyzer.cs index faa7e1f7990..4fed2d0adfb 100644 --- a/src/EFCore.Analyzers/StringsUsageInRawQueriesDiagnosticAnalyzer.cs +++ b/src/EFCore.Analyzers/StringsUsageInRawQueriesDiagnosticAnalyzer.cs @@ -186,9 +186,65 @@ IInterpolatedStringOperation interpolatedString when AnalyzeInterpolatedString(i } concatenation when AnalyzeConcatenation(concatenation) => StringConcatenationDescriptor, + // ...an explicit call to string.Format(...) or string.Concat(...) + IInvocationOperation argInvocation + => AnalyzeStringMethodInvocation(argInvocation), + _ => null, }; + private static DiagnosticDescriptor? AnalyzeStringMethodInvocation(IInvocationOperation invocation) + { + if (invocation.TargetMethod.ContainingType.SpecialType != SpecialType.System_String + || invocation.TargetMethod.Name is not (nameof(string.Format) or nameof(string.Concat))) + { + return null; + } + + return HasNonConstantArgument(invocation) ? StringConcatenationDescriptor : null; + } + + private static bool HasNonConstantArgument(IInvocationOperation invocation) + { + foreach (var argument in invocation.Arguments) + { + var value = Unwrap(argument.Value); + + // Implicit params arrays — inspect each element rather than the array itself. + if (argument.ArgumentKind == ArgumentKind.ParamArray + && value is IArrayCreationOperation { Initializer.ElementValues: var elements }) + { + foreach (var element in elements) + { + if (!Unwrap(element).ConstantValue.HasValue) + { + return true; + } + } + + continue; + } + + if (!value.ConstantValue.HasValue) + { + return true; + } + } + + return false; + + // Strip implicit conversions (e.g. boxing to object) so we see the original constant. + static IOperation Unwrap(IOperation operation) + { + while (operation is IConversionOperation { IsImplicit: true } conversion) + { + operation = conversion.Operand; + } + + return operation; + } + } + private static bool AnalyzeInterpolatedString(IInterpolatedStringOperation interpolatedString) { if (interpolatedString.ConstantValue.HasValue) diff --git a/test/EFCore.Analyzers.Tests/StringConcatenationInRawQueriesAnalyzerTests.cs b/test/EFCore.Analyzers.Tests/StringConcatenationInRawQueriesAnalyzerTests.cs index 7f603b10de5..36891201878 100644 --- a/test/EFCore.Analyzers.Tests/StringConcatenationInRawQueriesAnalyzerTests.cs +++ b/test/EFCore.Analyzers.Tests/StringConcatenationInRawQueriesAnalyzerTests.cs @@ -162,6 +162,110 @@ void M(MyDbContext db) db.{{call}}("FooBar WHERE Id = " + Id); } } +""", + DiagnosticResult.CompilerWarning(EFDiagnostics.StringConcatenationUsageInRawQueries).WithLocation(0)); + + [Theory] + [MemberData(nameof(DoNotReportData))] + public Task Constant_string_format_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db) + { + db.{{call}}(string.Format("FooBar WHERE Id = {0}", "1")); + } +} +"""); + + [Theory] + [MemberData(nameof(ShouldReportData))] + public Task String_format_with_argument_should_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, string id) + { + db.{{call}}(string.Format("FooBar WHERE Id = {0}", id)); + } +} +""", + DiagnosticResult.CompilerWarning(EFDiagnostics.StringConcatenationUsageInRawQueries).WithLocation(0)); + + [Theory] + [MemberData(nameof(ShouldReportData))] + public Task String_format_with_method_call_should_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db) + { + db.{{call}}(string.Format("FooBar WHERE Id = {0}", GetId())); + } + + string GetId() => "1"; +} +""", + DiagnosticResult.CompilerWarning(EFDiagnostics.StringConcatenationUsageInRawQueries).WithLocation(0)); + + [Theory] + [MemberData(nameof(DoNotReportData))] + public Task Constant_string_concat_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db) + { + db.{{call}}(string.Concat("FooBar", " WHERE Id = ", "1")); + } +} +"""); + + [Theory] + [MemberData(nameof(ShouldReportData))] + public Task String_concat_with_argument_should_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, string id) + { + db.{{call}}(string.Concat("FooBar WHERE Id = ", id)); + } +} +""", + DiagnosticResult.CompilerWarning(EFDiagnostics.StringConcatenationUsageInRawQueries).WithLocation(0)); + + [Theory] + [MemberData(nameof(ShouldReportData))] + public Task String_concat_with_method_call_should_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db) + { + db.{{call}}(string.Concat("FooBar WHERE Id = ", GetId())); + } + + string GetId() => "1"; +} """, DiagnosticResult.CompilerWarning(EFDiagnostics.StringConcatenationUsageInRawQueries).WithLocation(0)); }