diff --git a/src/EFCore.Analyzers/AnalyzerReleases.Shipped.md b/src/EFCore.Analyzers/AnalyzerReleases.Shipped.md index ed10f609a61..3a3164ea024 100644 --- a/src/EFCore.Analyzers/AnalyzerReleases.Shipped.md +++ b/src/EFCore.Analyzers/AnalyzerReleases.Shipped.md @@ -22,4 +22,11 @@ EF1000 | Security | Disabled | RawSqlStringInjectionDiagnosticAnalyzer, [Docume ### New Rules Rule ID | Category | Severity | Notes --------|----------|----------|------- -EF1002 | Security | Warning | InterpolatedStringUsageInRawQueriesDiagnosticAnalyzer, [Documentation](https://learn.microsoft.com/ef/core/querying/sql-queries#passing-parameters) \ No newline at end of file +EF1002 | Security | Warning | StringsUsageInRawQueriesDiagnosticAnalyzer, [Documentation](https://learn.microsoft.com/ef/core/querying/sql-queries#passing-parameters) + +## Release 10.0.0 + +### New Rules +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +EF1003 | Security | Warning | StringsUsageInRawQueriesDiagnosticAnalyzer, [Documentation](https://learn.microsoft.com/ef/core/querying/sql-queries#passing-parameters) \ No newline at end of file diff --git a/src/EFCore.Analyzers/InterpolatedStringUsageInRawQueriesCodeFixProvider.cs b/src/EFCore.Analyzers/InterpolatedStringUsageInRawQueriesCodeFixProvider.cs index e5b7da68834..b701bdcfe91 100644 --- a/src/EFCore.Analyzers/InterpolatedStringUsageInRawQueriesCodeFixProvider.cs +++ b/src/EFCore.Analyzers/InterpolatedStringUsageInRawQueriesCodeFixProvider.cs @@ -77,7 +77,7 @@ private static SimpleNameSyntax GetReplacementName(SimpleNameSyntax oldName) var oldNameToken = oldName.Identifier; var oldMethodName = oldNameToken.ValueText; - var replacementMethodName = InterpolatedStringUsageInRawQueriesDiagnosticAnalyzer.GetReplacementMethodName(oldMethodName); + var replacementMethodName = StringsUsageInRawQueriesDiagnosticAnalyzer.GetReplacementMethodName(oldMethodName); Debug.Assert(replacementMethodName != oldMethodName, "At this point we must find correct replacement name"); var replacementToken = SyntaxFactory.Identifier(replacementMethodName).WithTriviaFrom(oldNameToken); diff --git a/src/EFCore.Analyzers/NullableAttributes.cs b/src/EFCore.Analyzers/NullableAttributes.cs new file mode 100644 index 00000000000..65ae3ef18b8 --- /dev/null +++ b/src/EFCore.Analyzers/NullableAttributes.cs @@ -0,0 +1,201 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis +{ +#if !NETSTANDARD2_1 + /// Specifies that null is allowed as an input even if the corresponding type disallows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property, Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class AllowNullAttribute : Attribute + { } + + /// Specifies that null is disallowed as an input even if the corresponding type allows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property, Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class DisallowNullAttribute : Attribute + { } + + /// Specifies that an output may be null even if the corresponding type disallows it. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class MaybeNullAttribute : Attribute + { } + + /// Specifies that an output will not be null even if the corresponding type allows it. Specifies that an input argument was not null when the call returns. + [AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class NotNullAttribute : Attribute + { } + + /// Specifies that when a method returns , the parameter may be null even if the corresponding type disallows it. + [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class MaybeNullWhenAttribute : Attribute + { + /// Initializes the attribute with the specified return value condition. + /// + /// The return value condition. If the method returns this value, the associated parameter may be null. + /// + public MaybeNullWhenAttribute(bool returnValue) => ReturnValue = returnValue; + + /// Gets the return value condition. + public bool ReturnValue { get; } + } + + /// Specifies that when a method returns , the parameter will not be null even if the corresponding type allows it. + [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class NotNullWhenAttribute : Attribute + { + /// Initializes the attribute with the specified return value condition. + /// + /// The return value condition. If the method returns this value, the associated parameter will not be null. + /// + public NotNullWhenAttribute(bool returnValue) => ReturnValue = returnValue; + + /// Gets the return value condition. + public bool ReturnValue { get; } + } + + /// Specifies that the output will be non-null if the named parameter is non-null. + [AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, AllowMultiple = true, Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class NotNullIfNotNullAttribute : Attribute + { + /// Initializes the attribute with the associated parameter name. + /// + /// The associated parameter name. The output will be non-null if the argument to the parameter specified is non-null. + /// + public NotNullIfNotNullAttribute(string parameterName) => ParameterName = parameterName; + + /// Gets the associated parameter name. + public string ParameterName { get; } + } + + /// Applied to a method that will never return under any circumstance. + [AttributeUsage(AttributeTargets.Method, Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class DoesNotReturnAttribute : Attribute + { } + + /// Specifies that the method will not return if the associated Boolean parameter is passed the specified value. + [AttributeUsage(AttributeTargets.Parameter, Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class DoesNotReturnIfAttribute : Attribute + { + /// Initializes the attribute with the specified parameter value. + /// + /// The condition parameter value. Code after the method will be considered unreachable by diagnostics if the argument to + /// the associated parameter matches this value. + /// + public DoesNotReturnIfAttribute(bool parameterValue) => ParameterValue = parameterValue; + + /// Gets the condition parameter value. + public bool ParameterValue { get; } + } +#endif + + /// Specifies that the method or property will ensure that the listed field and property members have not-null values. + [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class MemberNotNullAttribute : Attribute + { + /// Initializes the attribute with a field or property member. + /// + /// The field or property member that is promised to be not-null. + /// + public MemberNotNullAttribute(string member) => Members = [member]; + + /// Initializes the attribute with the list of field and property members. + /// + /// The list of field and property members that are promised to be not-null. + /// + public MemberNotNullAttribute(params string[] members) => Members = members; + + /// Gets field or property member names. + public string[] Members { get; } + } + + /// Specifies that the method or property will ensure that the listed field and property members have not-null values when returning with the specified return value condition. + [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class MemberNotNullWhenAttribute : Attribute + { + /// Initializes the attribute with the specified return value condition and a field or property member. + /// + /// The return value condition. If the method returns this value, the associated field or property member will not be null. + /// + /// + /// The field or property member that is promised to be not-null. + /// + public MemberNotNullWhenAttribute(bool returnValue, string member) + { + ReturnValue = returnValue; + Members = [member]; + } + + /// Initializes the attribute with the specified return value condition and list of field and property members. + /// + /// The return value condition. If the method returns this value, the associated field and property members will not be null. + /// + /// + /// The list of field and property members that are promised to be not-null. + /// + public MemberNotNullWhenAttribute(bool returnValue, params string[] members) + { + ReturnValue = returnValue; + Members = members; + } + + /// Gets the return value condition. + public bool ReturnValue { get; } + + /// Gets field or property member names. + public string[] Members { get; } + } +} diff --git a/src/EFCore.Analyzers/Properties/AnalyzerStrings.Designer.cs b/src/EFCore.Analyzers/Properties/AnalyzerStrings.Designer.cs index bcd1664c2ab..803786208ee 100644 --- a/src/EFCore.Analyzers/Properties/AnalyzerStrings.Designer.cs +++ b/src/EFCore.Analyzers/Properties/AnalyzerStrings.Designer.cs @@ -105,6 +105,24 @@ public static string InterpolatedStringUsageInRawQueriesMessageFormat { } } + /// + /// Looks up a localized string similar to Risk of vulnerability to SQL injection.. + /// + public static string StringConcatenationUsageInRawQueriesAnalyzerTitle { + get { + return ResourceManager.GetString("StringConcatenationUsageInRawQueriesAnalyzerTitle", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Method '{0}' inserts concatenated strings directly into the SQL, without any protection against SQL injection. Consider using '{1}' instead, which protects against SQL injection, or make sure that the value is sanitized and suppress the warning.. + /// + public static string StringConcatenationUsageInRawQueriesMessageFormat { + get { + return ResourceManager.GetString("StringConcatenationUsageInRawQueriesMessageFormat", resourceCulture); + } + } + /// /// Looks up a localized string similar to DbSet properties on DbContext subclasses are automatically populated by the DbContext constructor.. /// diff --git a/src/EFCore.Analyzers/Properties/AnalyzerStrings.resx b/src/EFCore.Analyzers/Properties/AnalyzerStrings.resx index 2282189be54..731c6ef635e 100644 --- a/src/EFCore.Analyzers/Properties/AnalyzerStrings.resx +++ b/src/EFCore.Analyzers/Properties/AnalyzerStrings.resx @@ -36,4 +36,10 @@ Method '{0}' inserts interpolated strings directly into the SQL, without any protection against SQL injection. Consider using '{1}' instead, which protects against SQL injection, or make sure that the value is sanitized and suppress the warning. + + Risk of vulnerability to SQL injection. + + + Method '{0}' inserts concatenated strings directly into the SQL, without any protection against SQL injection. Consider using '{1}' instead, which protects against SQL injection, or make sure that the value is sanitized and suppress the warning. + \ No newline at end of file diff --git a/src/EFCore.Analyzers/InterpolatedStringUsageInRawQueriesDiagnosticAnalyzer.cs b/src/EFCore.Analyzers/StringsUsageInRawQueriesDiagnosticAnalyzer.cs similarity index 67% rename from src/EFCore.Analyzers/InterpolatedStringUsageInRawQueriesDiagnosticAnalyzer.cs rename to src/EFCore.Analyzers/StringsUsageInRawQueriesDiagnosticAnalyzer.cs index e4a0ea8f28d..3461b63aacd 100644 --- a/src/EFCore.Analyzers/InterpolatedStringUsageInRawQueriesDiagnosticAnalyzer.cs +++ b/src/EFCore.Analyzers/StringsUsageInRawQueriesDiagnosticAnalyzer.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; @@ -10,9 +11,9 @@ namespace Microsoft.EntityFrameworkCore; [DiagnosticAnalyzer(LanguageNames.CSharp)] -public sealed class InterpolatedStringUsageInRawQueriesDiagnosticAnalyzer : DiagnosticAnalyzer +public sealed class StringsUsageInRawQueriesDiagnosticAnalyzer : DiagnosticAnalyzer { - private static readonly DiagnosticDescriptor Descriptor + private static readonly DiagnosticDescriptor InterpolatedStringDescriptor = new( EFDiagnostics.InterpolatedStringUsageInRawQueries, title: AnalyzerStrings.InterpolatedStringUsageInRawQueriesAnalyzerTitle, @@ -21,8 +22,17 @@ private static readonly DiagnosticDescriptor Descriptor defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true); + private static readonly DiagnosticDescriptor StringConcatenationDescriptor + = new( + EFDiagnostics.StringConcatenationUsageInRawQueries, + title: AnalyzerStrings.StringConcatenationUsageInRawQueriesAnalyzerTitle, + messageFormat: AnalyzerStrings.StringConcatenationUsageInRawQueriesMessageFormat, + category: "Security", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true); + public override ImmutableArray SupportedDiagnostics - => [Descriptor]; + => [InterpolatedStringDescriptor, StringConcatenationDescriptor]; public override void Initialize(AnalysisContext context) { @@ -37,19 +47,19 @@ private void AnalyzerInvocation(OperationAnalysisContext context) var invocation = (IInvocationOperation)context.Operation; var targetMethod = invocation.TargetMethod; - var report = targetMethod.Name switch + var diagnosticDescriptor = targetMethod.Name switch { "FromSqlRaw" => AnalyzeFromSqlRawInvocation(invocation), "ExecuteSqlRaw" or "ExecuteSqlRawAsync" => AnalyzeExecuteSqlRawInvocation(invocation), "SqlQueryRaw" => AnalyzeSqlQueryRawInvocation(invocation), - _ => false + _ => null }; - if (report) + if (diagnosticDescriptor is not null) { context.ReportDiagnostic( Diagnostic.Create( - Descriptor, + diagnosticDescriptor, GetTargetLocation(invocation.Syntax), targetMethod.Name, GetReplacementMethodName(targetMethod.Name))); @@ -93,7 +103,7 @@ internal static string GetReplacementMethodName(string oldName) _ => oldName }; - private static bool AnalyzeFromSqlRawInvocation(IInvocationOperation invocation) + private static DiagnosticDescriptor? AnalyzeFromSqlRawInvocation(IInvocationOperation invocation) { var targetMethod = invocation.TargetMethod; Debug.Assert(targetMethod.Name == "FromSqlRaw"); @@ -103,19 +113,13 @@ private static bool AnalyzeFromSqlRawInvocation(IInvocationOperation invocation) Debug.Assert(correctFromSqlRaw is not null, "Unable to find original `FromSqlRaw` method"); - // Verify that the method is the one we analyze and its second argument, which corresponds to `string sql`, is an interpolated string - if (correctFromSqlRaw is null - || !targetMethod.ConstructedFrom.Equals(correctFromSqlRaw, SymbolEqualityComparer.Default) - || invocation.Arguments[1].Value is not IInterpolatedStringOperation interpolatedString) - { - return false; - } - - // Report warning if interpolated string is not a constant and all its interpolations are not constants - return AnalyzeInterpolatedString(interpolatedString); + // Verify that the method is the one we analyze and its second argument, which corresponds to `string sql`, is... + return correctFromSqlRaw is not null && targetMethod.ConstructedFrom.Equals(correctFromSqlRaw, SymbolEqualityComparer.Default) + ? AnalyzeInvocation(invocation) + : null; } - private static bool AnalyzeExecuteSqlRawInvocation(IInvocationOperation invocation) + private static DiagnosticDescriptor? AnalyzeExecuteSqlRawInvocation(IInvocationOperation invocation) { var targetMethod = invocation.TargetMethod; Debug.Assert(targetMethod.Name is "ExecuteSqlRaw" or "ExecuteSqlRawAsync"); @@ -130,7 +134,7 @@ private static bool AnalyzeExecuteSqlRawInvocation(IInvocationOperation invocati if (!correctMethods.Contains(targetMethod.ConstructedFrom, SymbolEqualityComparer.Default)) { - return false; + return null; } } else @@ -141,23 +145,17 @@ private static bool AnalyzeExecuteSqlRawInvocation(IInvocationOperation invocati if (!correctMethods.Contains(targetMethod.ConstructedFrom, SymbolEqualityComparer.Default)) { - return false; + return null; } } // At this point assume that the method is correct since both `ExecuteSqlRaw` and `ExecuteSqlRawAsync` have multiple overloads. // Checking for every possible one is too much work for almost no gain. // So check whether the second argument, that corresponds to `string sql` parameter, is an interpolated string... - if (invocation.Arguments[1].Value is not IInterpolatedStringOperation interpolatedString) - { - return false; - } - - // ...and report warning if interpolated string is not a constant and all its interpolations are not constants - return AnalyzeInterpolatedString(interpolatedString); + return AnalyzeInvocation(invocation); } - private static bool AnalyzeSqlQueryRawInvocation(IInvocationOperation invocation) + private static DiagnosticDescriptor? AnalyzeSqlQueryRawInvocation(IInvocationOperation invocation) { var targetMethod = invocation.TargetMethod; Debug.Assert(targetMethod.Name == "SqlQueryRaw"); @@ -168,16 +166,29 @@ private static bool AnalyzeSqlQueryRawInvocation(IInvocationOperation invocation Debug.Assert(correctSqlQueryRaw is not null, "Unable to find original `SqlQueryRaw` method"); - // Verify that the method is the one we analyze and its second argument, which corresponds to `string sql`, is an interpolated string - if (correctSqlQueryRaw is null - || !targetMethod.ConstructedFrom.Equals(correctSqlQueryRaw, SymbolEqualityComparer.Default) - || invocation.Arguments[1].Value is not IInterpolatedStringOperation interpolatedString) + // Verify that the method is the one we analyze and its second argument, which corresponds to `string sql`, is... + return correctSqlQueryRaw is not null && targetMethod.ConstructedFrom.Equals(correctSqlQueryRaw, SymbolEqualityComparer.Default) + ? AnalyzeInvocation(invocation) + : null; + } + + private static DiagnosticDescriptor? AnalyzeInvocation(IInvocationOperation invocation) + { + // ...an interpolated string + if (invocation.Arguments[1].Value is IInterpolatedStringOperation interpolatedString + && AnalyzeInterpolatedString(interpolatedString)) { - return false; + return InterpolatedStringDescriptor; } - // Report warning if interpolated string is not a constant and all its interpolations are not constants - return AnalyzeInterpolatedString(interpolatedString); + // ...a string concatenation + if (TryGetStringConcatenation(invocation.Arguments[1].Value, out var concatenation) + && AnalyzeConcatenation(concatenation)) + { + return StringConcatenationDescriptor; + } + + return null; } private static bool AnalyzeInterpolatedString(IInterpolatedStringOperation interpolatedString) @@ -204,6 +215,36 @@ private static bool AnalyzeInterpolatedString(IInterpolatedStringOperation inter return false; } + private static bool TryGetStringConcatenation(IOperation operation, [NotNullWhen(true)] out IBinaryOperation? concatenation) + { + if (operation is IBinaryOperation + { + OperatorKind: BinaryOperatorKind.Add, + Type.SpecialType: SpecialType.System_String, + } binaryOperation) + { + concatenation = binaryOperation; + return true; + } + + concatenation = default; + return false; + } + + private static bool AnalyzeConcatenation(IBinaryOperation operation) + { + var left = operation.LeftOperand; + var right = operation.RightOperand; + + if ((left is IBinaryOperation leftBinary && AnalyzeConcatenation(leftBinary)) + || (right is IBinaryOperation rightBinary && AnalyzeConcatenation(rightBinary))) + { + return true; + } + + return !left.ConstantValue.HasValue || !right.ConstantValue.HasValue; + } + private static IMethodSymbol? FromSqlRawMethod(Compilation compilation) { var type = compilation.RelationalQueryableExtensionsType(); diff --git a/src/Shared/EFDiagnostics.cs b/src/Shared/EFDiagnostics.cs index 8cb61890d9e..b8437c7980e 100644 --- a/src/Shared/EFDiagnostics.cs +++ b/src/Shared/EFDiagnostics.cs @@ -10,6 +10,7 @@ internal static class EFDiagnostics { public const string InternalUsage = "EF1001"; public const string InterpolatedStringUsageInRawQueries = "EF1002"; + public const string StringConcatenationUsageInRawQueries = "EF1003"; public const string SuppressUninitializedDbSetRule = "EFSPR1001"; // Diagnostics for [Experimental] diff --git a/test/EFCore.Analyzers.Tests/InterpolatedStringUsageInRawQueriesTests.cs b/test/EFCore.Analyzers.Tests/InterpolatedStringUsageInRawQueriesTests.cs deleted file mode 100644 index c7c25890144..00000000000 --- a/test/EFCore.Analyzers.Tests/InterpolatedStringUsageInRawQueriesTests.cs +++ /dev/null @@ -1,873 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace Microsoft.EntityFrameworkCore; - -using VerifyCS = - CSharpCodeFixVerifier; - -public class InterpolatedStringUsageInRawQueriesTests -{ - private const string MyDbContext = - """ -using Microsoft.EntityFrameworkCore; -using System.Collections.Generic; - -class User -{ - public int Id { get; set; } - public string FirstName { get; set; } - public string LastName { get; set; } -} - -class MyDbContext : DbContext -{ - public DbSet Users { get; init; } -} -"""; - - [Fact] - public Task FromSql_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Users.FromSql($"SELECT * FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task FromSqlInterpolated_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Users.FromSqlInterpolated($"SELECT * FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task ExecuteSql_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.ExecuteSql($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlInterpolated_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.ExecuteSqlInterpolated($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlAsync_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.ExecuteSqlAsync($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlInterpolatedAsync_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.ExecuteSqlInterpolatedAsync($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task SqlQuery_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.SqlQuery($"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - #region FromSqlRaw - - [Fact] - public Task FromSqlRaw_constant_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - db.Users.FromSqlRaw("SELECT * FROM [Users] WHERE [Id] = 1;"); - } -} -"""); - - [Fact] - public Task FromSqlRaw_constant_string_with_parameters_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Users.FromSqlRaw("SELECT * FROM [Users] WHERE [Id] = {0};", id); - } -} -"""); - - [Fact] - public Task FromSqlRaw_interpolated_string_report() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Users.[|FromSqlRaw|]($"SELECT * FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Users.FromSql($"SELECT * FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public async Task FromSqlRaw_interpolated_string_with_other_parameters_report() - { - var source = MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Users.[|FromSqlRaw|]($"SELECT * FROM [Users] WHERE [Id] = {id};", id); - } -} -"""; - - await VerifyCS.VerifyCodeFixAsync(source, source); - } - - [Fact] - public Task FromSqlRaw_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - const string id = "1"; - db.Users.FromSqlRaw($"SELECT * FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task FromSqlRaw_pseudo_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - db.Users.FromSqlRaw($"SELECT [{nameof(MyDbContext)}] FROM [Users] WHERE [Id] = {1};"); - } -} -"""); - - [Fact] - public Task FromSqlRaw_direct_extension_class_usage_interpolated_string_report() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - RelationalQueryableExtensions.[|FromSqlRaw|](db.Users, $"SELECT * FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - RelationalQueryableExtensions.FromSql(db.Users, $"SELECT * FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task FromSqlRaw_direct_extension_class_usage_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - const string id = "1"; - RelationalQueryableExtensions.FromSqlRaw(db.Users, $"SELECT * FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task FromSqlRaw_direct_extension_class_usage_pseudo_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - RelationalQueryableExtensions.FromSqlRaw(db.Users, $"SELECT [{nameof(MyDbContext)}] FROM [Users] WHERE [Id] = {1};"); - } -} -"""); - - [Fact] - public Task FromSqlRaw_FixAll() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Users.[|FromSqlRaw|]($"SELECT * FROM [Users] WHERE [Id] = {id};"); - db.Users.[|FromSqlRaw|]($"SELECT * FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Users.FromSql($"SELECT * FROM [Users] WHERE [Id] = {id};"); - db.Users.FromSql($"SELECT * FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - #endregion - - #region ExecuteSqlRaw - - [Fact] - public Task ExecuteSqlRaw_constant_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - db.Database.ExecuteSqlRaw("DELETE FROM [Users] WHERE [Id] = 1;"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRaw_constant_string_with_parameters_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.ExecuteSqlRaw("DELETE FROM FROM [Users] WHERE [Id] = {0};", id); - } -} -"""); - - [Fact] - public Task ExecuteSqlRaw_interpolated_string_report() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|ExecuteSqlRaw|]($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.ExecuteSql($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public async Task ExecuteSqlRaw_interpolated_string_with_other_parameters_report() - { - var source = MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|ExecuteSqlRaw|]($"DELETE FROM FROM [Users] WHERE [Id] = {id};", id); - } -} -"""; - - await VerifyCS.VerifyCodeFixAsync(source, source); - } - - [Fact] - public async Task ExecuteSqlRaw_interpolated_string_with_other_parameters_IEnumerable_report() - { - var source = MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|ExecuteSqlRaw|]($"DELETE FROM FROM [Users] WHERE [Id] = {id};", (IEnumerable)null); - } -} -"""; - - await VerifyCS.VerifyCodeFixAsync(source, source); - } - - [Fact] - public Task ExecuteSqlRaw_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - const string id = "1"; - db.Database.ExecuteSqlRaw($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRaw_pseudo_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - db.Database.ExecuteSqlRaw($"DELETE FROM FROM [Users] WHERE [{nameof(MyDbContext)}] = {1};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRaw_direct_extension_class_usage_interpolated_string_report() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - RelationalDatabaseFacadeExtensions.[|ExecuteSqlRaw|](db.Database, $"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - RelationalDatabaseFacadeExtensions.ExecuteSql(db.Database, $"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRaw_direct_extension_class_usage_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - const string id = "1"; - RelationalDatabaseFacadeExtensions.ExecuteSqlRaw(db.Database, $"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRaw_direct_extension_class_usage_pseudo_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - RelationalDatabaseFacadeExtensions.ExecuteSqlRaw(db.Database, $"DELETE FROM FROM [{nameof(MyDbContext)}] WHERE [Id] = {1};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRaw_FixAll() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|ExecuteSqlRaw|]($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - db.Database.[|ExecuteSqlRaw|]($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.ExecuteSql($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - db.Database.ExecuteSql($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - #endregion - - #region ExecuteSqlRawAsync - - [Fact] - public Task ExecuteSqlRawAsync_constant_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - db.Database.ExecuteSqlRawAsync("DELETE FROM [Users] WHERE [Id] = 1;"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRawAsync_constant_string_with_parameters_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.ExecuteSqlRaw("DELETE FROM FROM [Users] WHERE [Id] = {0};", id); - } -} -"""); - - [Fact] - public Task ExecuteSqlRawAsync_interpolated_string_report() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|ExecuteSqlRawAsync|]($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.ExecuteSqlAsync($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public async Task ExecuteSqlRawAsync_interpolated_string_with_other_parameters_report() - { - var source = MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|ExecuteSqlRawAsync|]($"DELETE FROM FROM [Users] WHERE [Id] = {id};", id); - } -} -"""; - - await VerifyCS.VerifyCodeFixAsync(source, source); - } - - [Fact] - public async Task ExecuteSqlRawAsync_interpolated_string_with_other_parameters_IEnumerable_report() - { - var source = MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|ExecuteSqlRawAsync|]($"DELETE FROM FROM [Users] WHERE [Id] = {id};", (IEnumerable)null); - } -} -"""; - - await VerifyCS.VerifyCodeFixAsync(source, source); - } - - [Fact] - public Task ExecuteSqlRawAsync_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - const string id = "1"; - db.Database.ExecuteSqlRawAsync($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRawAsync_pseudo_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - const string id = "1"; - db.Database.ExecuteSqlRawAsync($"DELETE FROM FROM [{nameof(MyDbContext)}] WHERE [Id] = {1};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRawAsync_direct_extension_class_usage_interpolated_string_report() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - RelationalDatabaseFacadeExtensions.[|ExecuteSqlRawAsync|](db.Database, $"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - RelationalDatabaseFacadeExtensions.ExecuteSqlAsync(db.Database, $"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRawAsync_direct_extension_class_usage_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - const string id = "1"; - RelationalDatabaseFacadeExtensions.ExecuteSqlRawAsync(db.Database, $"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRawAsync_direct_extension_class_usage_pseudo_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - RelationalDatabaseFacadeExtensions.ExecuteSqlRawAsync(db.Database, $"DELETE FROM FROM [{nameof(MyDbContext)}] WHERE [Id] = {1};"); - } -} -"""); - - [Fact] - public Task ExecuteSqlRawAsync_FixAll() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|ExecuteSqlRawAsync|]($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - db.Database.[|ExecuteSqlRawAsync|]($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.ExecuteSqlAsync($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - db.Database.ExecuteSqlAsync($"DELETE FROM FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - #endregion - - #region SqlQueryRaw - - [Fact] - public Task SqlQueryRaw_constant_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - db.Database.SqlQueryRaw("SELECT [Age] FROM [Users] WHERE [Id] = 1;"); - } -} -"""); - - [Fact] - public Task SqlQueryRaw_constant_string_with_parameters_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.SqlQueryRaw("SELECT [Age] FROM [Users] WHERE [Id] = {0};", id); - } -} -"""); - - [Fact] - public Task SqlQueryRaw_interpolated_string_report() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|SqlQueryRaw|]($"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.SqlQuery($"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public async Task SqlQueryRaw_interpolated_string_with_other_parameters_report() - { - var source = MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|SqlQueryRaw|]($"SELECT [Age] FROM [Users] WHERE [Id] = {id};", id); - } -} -"""; - - await VerifyCS.VerifyCodeFixAsync(source, source); - } - - [Fact] - public Task SqlQueryRaw_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - const string id = "1"; - db.Database.SqlQueryRaw($"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task SqlQueryRaw_pseudo_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - db.Database.SqlQueryRaw($"SELECT [{nameof(MyDbContext)}] FROM [Users] WHERE [Id] = {1};"); - } -} -"""); - - [Fact] - public Task SqlQueryRaw_direct_extension_class_usage_interpolated_string_report() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - RelationalDatabaseFacadeExtensions.[|SqlQueryRaw|](db.Database, $"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - RelationalDatabaseFacadeExtensions.SqlQuery(db.Database, $"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task SqlQueryRaw_direct_extension_class_usage_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - const string id = "1"; - RelationalDatabaseFacadeExtensions.SqlQueryRaw(db.Database, $"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - [Fact] - public Task SqlQueryRaw_direct_extension_class_usage_pseudo_constant_interpolated_string_do_not_report() - => VerifyCS.VerifyAnalyzerAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db) - { - RelationalDatabaseFacadeExtensions.SqlQueryRaw(db.Database, $"SELECT [{nameof(MyDbContext)}] FROM [Users] WHERE [Id] = {1};"); - } -} -"""); - - [Fact] - public Task SqlQueryRaw_FixAll() - => VerifyCS.VerifyCodeFixAsync( - MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.[|SqlQueryRaw|]($"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - db.Database.[|SqlQueryRaw|]($"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - } -} -""", MyDbContext - + """ -class C -{ - void M(MyDbContext db, int id) - { - db.Database.SqlQuery($"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - db.Database.SqlQuery($"SELECT [Age] FROM [Users] WHERE [Id] = {id};"); - } -} -"""); - - #endregion -} diff --git a/test/EFCore.Analyzers.Tests/StringConcatenationInRawQueriesAnalyzerTests.cs b/test/EFCore.Analyzers.Tests/StringConcatenationInRawQueriesAnalyzerTests.cs new file mode 100644 index 00000000000..80a8ed665e5 --- /dev/null +++ b/test/EFCore.Analyzers.Tests/StringConcatenationInRawQueriesAnalyzerTests.cs @@ -0,0 +1,167 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore; + +using Microsoft.CodeAnalysis.Testing; + +using Verify = CSharpAnalyzerVerifier; + +public class StringConcatenationInRawQueriesAnalyzerTests +{ + public static readonly TheoryData DoNotReportData = + [ + "Users.FromSqlRaw", + "Database.ExecuteSqlRaw", + "Database.ExecuteSqlRawAsync", + "Database.SqlQueryRaw", + ]; + + public static readonly TheoryData ShouldReportData = + [ + "Users.{|#0:FromSqlRaw|}", + "Database.{|#0:ExecuteSqlRaw|}", + "Database.{|#0:ExecuteSqlRawAsync|}", + "Database.{|#0:SqlQueryRaw|}", + ]; + + private const string MyDbContext = + """ +using Microsoft.EntityFrameworkCore; +using System.Collections.Generic; + +class User +{ + public int Id { get; set; } + public string FirstName { get; set; } + public string LastName { get; set; } +} + +class MyDbContext : DbContext +{ + public DbSet Users { get; init; } +} +"""; + + [Theory] + [MemberData(nameof(DoNotReportData))] + public Task Constant_string_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db) + { + db.{{call}}("FooBar WHERE Id = 1"); + } +} +"""); + + [Theory] + [MemberData(nameof(DoNotReportData))] + public Task Constant_strings_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db) + { + db.{{call}}("FooBar" + + " WHERE Id = 1"); + } +} +"""); + + [Theory] + [MemberData(nameof(DoNotReportData))] + public Task Constant_string_with_parameters_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, string id) + { + db.{{call}}("FooBar WHERE Id = {0}", id); + } +} +"""); + + [Theory] + [MemberData(nameof(ShouldReportData))] + public Task Argument_should_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, string id) + { + db.{{call}}("FooBar WHERE Id = " + id); + } +} +""", + DiagnosticResult.CompilerWarning(EFDiagnostics.StringConcatenationUsageInRawQueries).WithLocation(0)); + + [Theory] + [MemberData(nameof(ShouldReportData))] + public Task Method_call_should_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db) + { + db.{{call}}("FooBar WHERE Id = " + GetId()); + } + + string GetId() => "1"; +} +""", + DiagnosticResult.CompilerWarning(EFDiagnostics.StringConcatenationUsageInRawQueries).WithLocation(0)); + + [Theory] + [MemberData(nameof(DoNotReportData))] + public Task Constant_string_member_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + const string Id = "1"; + + void M(MyDbContext db) + { + db.{{call}}("FooBar WHERE Id = " + Id); + } +} +"""); + + [Theory] + [MemberData(nameof(ShouldReportData))] + public Task Readonly_static_string_variable_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + readonly static string Id = "1"; + + void M(MyDbContext db) + { + db.{{call}}("FooBar WHERE Id = " + Id); + } +} +""", + DiagnosticResult.CompilerWarning(EFDiagnostics.StringConcatenationUsageInRawQueries).WithLocation(0)); +} diff --git a/test/EFCore.Analyzers.Tests/StringInterpolationnRawQueriesAnalyzerTests.cs b/test/EFCore.Analyzers.Tests/StringInterpolationnRawQueriesAnalyzerTests.cs new file mode 100644 index 00000000000..7b18727dd2f --- /dev/null +++ b/test/EFCore.Analyzers.Tests/StringInterpolationnRawQueriesAnalyzerTests.cs @@ -0,0 +1,225 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore; + +using Microsoft.CodeAnalysis.Testing; + +using Verify = CSharpCodeFixVerifier; + +public class StringInterpolationnRawQueriesAnalyzerTests +{ + public static readonly TheoryData DoNotReportData = + [ + "db.Users.FromSqlRaw(", + "db.Database.ExecuteSqlRaw(", + "db.Database.ExecuteSqlRawAsync(", + "db.Database.SqlQueryRaw(", + "RelationalQueryableExtensions.FromSqlRaw(db.Users, ", + ]; + + public static readonly TheoryData ShouldReportData = + [ + "db.Users.{|#0:FromSqlRaw|}(", + "db.Database.{|#0:ExecuteSqlRaw|}(", + "db.Database.{|#0:ExecuteSqlRawAsync|}(", + "db.Database.{|#0:SqlQueryRaw|}(", + "RelationalQueryableExtensions.{|#0:FromSqlRaw|}(db.Users, ", + ]; + + public static readonly TheoryData ShouldReportDataWithFix = new() + { + { "db.Users.{|#0:FromSqlRaw|}(", "db.Users.FromSql(" }, + { "db.Database.{|#0:ExecuteSqlRaw|}(", "db.Database.ExecuteSql(" }, + { "db.Database.{|#0:ExecuteSqlRawAsync|}(", "db.Database.ExecuteSqlAsync(" }, + { "db.Database.{|#0:SqlQueryRaw|}(", "db.Database.SqlQuery(" }, + { "RelationalQueryableExtensions.{|#0:FromSqlRaw|}(db.Users, ", "RelationalQueryableExtensions.FromSql(db.Users, " }, + }; + + public static readonly TheoryData CorrectCallsData = + [ + "db.Users.FromSql(", + "db.Users.FromSqlInterpolated(", + "db.Database.ExecuteSql(", + "db.Database.ExecuteSqlInterpolated(", + "db.Database.ExecuteSqlAsync(", + "db.Database.ExecuteSqlInterpolatedAsync(", + "db.Database.SqlQuery(", + ]; + + private const string MyDbContext = + """ +using Microsoft.EntityFrameworkCore; +using System.Collections.Generic; + +class User +{ + public int Id { get; set; } + public string FirstName { get; set; } + public string LastName { get; set; } +} + +class MyDbContext : DbContext +{ + public DbSet Users { get; init; } +} +"""; + + [Theory] + [MemberData(nameof(CorrectCallsData))] + public Task Correct_interpolation_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, int id) + { + {{call}}$"FooBar WHERE Id = {id}"); + } +} +"""); + + [Theory] + [MemberData(nameof(DoNotReportData))] + public Task Constant_string_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db) + { + {{call}}"FooBar WHERE Id = 1"); + } +} +"""); + + [Theory] + [MemberData(nameof(DoNotReportData))] + public Task Constant_string_with_parameters_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, int id) + { + {{call}}"FooBar WHERE Id = {0}", id); + } +} +"""); + + [Theory] + [MemberData(nameof(ShouldReportDataWithFix))] + public Task Interpolated_string_should_report(string call, string fixedCall) + => Verify.VerifyCodeFixAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, int id) + { + {{call}}$"FooBar WHERE Id = {id}"); + } +} +""", + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, int id) + { + {{fixedCall}}$"FooBar WHERE Id = {id}"); + } +} +""", + DiagnosticResult.CompilerWarning(EFDiagnostics.InterpolatedStringUsageInRawQueries).WithLocation(0)); + + [Theory] + [MemberData(nameof(ShouldReportData))] + public async Task Interpolated_string_with_other_parameters_should_report(string call) + { + var source = $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, int id) + { + {{call}}$"FooBar WHERE Id = {id}", id); + } +} +"""; + await Verify.VerifyCodeFixAsync(source, source, + DiagnosticResult.CompilerWarning(EFDiagnostics.InterpolatedStringUsageInRawQueries).WithLocation(0)); + } + + [Theory] + [MemberData(nameof(DoNotReportData))] + public Task Constant_interpolated_string_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db) + { + const string id = "1"; + {{call}}$"FooBar WHERE Id = {id}"); + } +} +"""); + + [Theory] + [MemberData(nameof(DoNotReportData))] + public Task Pseudo_constant_interpolated_string_do_not_report(string call) + => Verify.VerifyAnalyzerAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db) + { + {{call}}$"FooBar WHERE Id = {1}"); + } +} +"""); + + [Fact] + public Task Fix_all() + => Verify.VerifyCodeFixAsync( + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, int id) + { + db.Users.{|#0:FromSqlRaw|}($"FooBar WHERE Id = {id}"); + db.Users.{|#1:FromSqlRaw|}($"FooBar WHERE Id = {id}"); + } +} +""", + $$""" +{{MyDbContext}} + +class C +{ + void M(MyDbContext db, int id) + { + db.Users.FromSql($"FooBar WHERE Id = {id}"); + db.Users.FromSql($"FooBar WHERE Id = {id}"); + } +} +""", + DiagnosticResult.CompilerWarning(EFDiagnostics.InterpolatedStringUsageInRawQueries).WithLocation(0), + DiagnosticResult.CompilerWarning(EFDiagnostics.InterpolatedStringUsageInRawQueries).WithLocation(1)); +} diff --git a/test/EFCore.Analyzers.Tests/TestUtilities/CSharpCodeFixVerifier.cs b/test/EFCore.Analyzers.Tests/TestUtilities/CSharpCodeFixVerifier.cs index 5354b86e3d3..6fda4f4c1f9 100644 --- a/test/EFCore.Analyzers.Tests/TestUtilities/CSharpCodeFixVerifier.cs +++ b/test/EFCore.Analyzers.Tests/TestUtilities/CSharpCodeFixVerifier.cs @@ -29,10 +29,10 @@ public static Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] return test.RunAsync(); } - public static async Task VerifyCodeFixAsync(string source, string fixedSource) + public static async Task VerifyCodeFixAsync(string source, string fixedSource, params DiagnosticResult[] expected) { var test = new Test { TestCode = source, FixedCode = fixedSource }; - + test.ExpectedDiagnostics.AddRange(expected); await test.RunAsync(); }