Skip to content

C#: Models for Microsoft.Data.SqlClient. #19877

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

Merged
merged 7 commits into from
Jun 27, 2025

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Jun 25, 2025

In this PR we

  • Add explicit Models as Data models for Microsoft.Data.SqlClient.[SqlCommand|SqlDataAdapter].
  • Add stubs for Microsoft.Data.SqlClient using the stub generator.
  • Convert tests for cs/sql-injection to inline expectations tests.

Note that prior to this change Microsoft.Data.SqlClient.SqlCommand was identified as a sink since it is declared in type that implements the System.Data.IDbCommand interface. Furthermore, Microsoft.Data.SqlClient.SqlDataAdapter was not modelled as a sink.

@github-actions github-actions bot added the C# label Jun 25, 2025
@michaelnebel michaelnebel force-pushed the csharp/microsoftdatasqlclient branch from eef6daa to 23ee048 Compare June 25, 2025 12:53
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

csharp

Generated file changes for csharp

  • Changes to framework-coverage-csharp.rst:
-    Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.AspNetCore.Components``, ``Microsoft.AspNetCore.Http``, ``Microsoft.AspNetCore.Mvc``, ``Microsoft.AspNetCore.WebUtilities``, ``Microsoft.CSharp``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.DotNet.PlatformAbstractions``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.JSInterop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.VisualBasic``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",60,2253,152,4
+    Others,"``Amazon.Lambda.APIGatewayEvents``, ``Amazon.Lambda.Core``, ``Dapper``, ``ILCompiler``, ``ILLink.RoslynAnalyzer``, ``ILLink.Shared``, ``ILLink.Tasks``, ``Internal.IL``, ``Internal.Pgo``, ``Internal.TypeSystem``, ``Microsoft.ApplicationBlocks.Data``, ``Microsoft.AspNetCore.Components``, ``Microsoft.AspNetCore.Http``, ``Microsoft.AspNetCore.Mvc``, ``Microsoft.AspNetCore.WebUtilities``, ``Microsoft.CSharp``, ``Microsoft.Data.SqlClient``, ``Microsoft.Diagnostics.Tools.Pgo``, ``Microsoft.DotNet.Build.Tasks``, ``Microsoft.DotNet.PlatformAbstractions``, ``Microsoft.EntityFrameworkCore``, ``Microsoft.Extensions.Caching.Distributed``, ``Microsoft.Extensions.Caching.Memory``, ``Microsoft.Extensions.Configuration``, ``Microsoft.Extensions.DependencyInjection``, ``Microsoft.Extensions.DependencyModel``, ``Microsoft.Extensions.Diagnostics.Metrics``, ``Microsoft.Extensions.FileProviders``, ``Microsoft.Extensions.FileSystemGlobbing``, ``Microsoft.Extensions.Hosting``, ``Microsoft.Extensions.Http``, ``Microsoft.Extensions.Logging``, ``Microsoft.Extensions.Options``, ``Microsoft.Extensions.Primitives``, ``Microsoft.Interop``, ``Microsoft.JSInterop``, ``Microsoft.NET.Build.Tasks``, ``Microsoft.VisualBasic``, ``Microsoft.Win32``, ``Mono.Linker``, ``MySql.Data.MySqlClient``, ``Newtonsoft.Json``, ``SourceGenerators``, ``Windows.Security.Cryptography.Core``",60,2257,159,4
-    Totals,,107,14399,400,9
+    Totals,,107,14403,407,9
  • Changes to framework-coverage-csharp.csv:
+ Microsoft.Data.SqlClient,7,,4,,,,,,,,,,7,,,,,,,,,4,

@michaelnebel michaelnebel marked this pull request as ready for review June 25, 2025 14:02
@Copilot Copilot AI review requested due to automatic review settings June 25, 2025 14:02
@michaelnebel michaelnebel requested a review from a team as a code owner June 25, 2025 14:02
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds support for modeling Microsoft.Data.SqlClient sinks and establishes stubs for related assemblies to enable inline expectation tests.

  • Include Microsoft.Data.SqlClient in the stub-generation script.
  • Add stub project and code files for various .NET libraries under csharp/ql/test/resources/stubs.
  • (Also converted cs/sql-injection tests to inline expectations, not shown here.)

Reviewed Changes

Copilot reviewed 64 out of 66 changed files in this pull request and generated no comments.

Show a summary per file
File Description
csharp/scripts/stubs/make_stubs_all.py Include "Microsoft.Data.SqlClient" in the list of libraries for stub generation.
csharp/ql/test/resources/stubs/System.Threading.Tasks.Extensions/4.5.4/System.Threading.Tasks.Extensions.csproj Add stub project for System.Threading.Tasks.Extensions 4.5.4.
csharp/ql/test/resources/stubs/System.Text.Json/4.7.2/System.Text.Json.csproj Add stub project for System.Text.Json 4.7.2.
csharp/ql/test/resources/stubs/System.Text.Encodings.Web/4.7.2/System.Text.Encodings.Web.csproj Add stub project for System.Text.Encodings.Web 4.7.2.
csharp/ql/test/resources/stubs/System.Security.Cryptography.ProtectedData/9.0.4/System.Security.Cryptography.ProtectedData.csproj Add stub project for System.Security.Cryptography.ProtectedData 9.0.4.

@michaelnebel michaelnebel force-pushed the csharp/microsoftdatasqlclient branch from fcc247e to cfadd30 Compare June 26, 2025 06:53
@hvitved
Copy link
Contributor

hvitved commented Jun 26, 2025

Note that prior to this change Microsoft.Data.SqlClient.SqlCommand was identified as a sink since it is declared in type that implements the System.Data.IDbCommand interface.

Why do we then need to (re)model it?

@michaelnebel
Copy link
Contributor Author

Note that prior to this change Microsoft.Data.SqlClient.SqlCommand was identified as a sink since it is declared in type that implements the System.Data.IDbCommand interface.

Why do we then need to (re)model it?

  • For consistency with other sealed classes and model them using MaD.
  • It might be what is causing the result from missing in the reported issue; If the build-mode: none for some reason doesn't correctly extract the type information (the inheritance hierarchy) then the method will not be identified as a sink.

@michaelnebel
Copy link
Contributor Author

DCA looks uneventful.

@michaelnebel michaelnebel merged commit 2f208bd into github:main Jun 27, 2025
22 checks passed
@michaelnebel michaelnebel deleted the csharp/microsoftdatasqlclient branch June 27, 2025 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants