-
Notifications
You must be signed in to change notification settings - Fork 1.9k
C#: Update Roslyn, MsBuild and Binlog extractor dependencies. #20996
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
Conversation
51fc015 to
cd8b6ad
Compare
…version operator.
…arams overload for string.Format.
cd8b6ad to
fa76d35
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the Roslyn, MsBuild, and Binlog extractor dependencies to support .NET 10 and C# 14, with necessary adaptations for breaking changes in Roslyn's handling of implicit Span conversions and the new field keyword.
Key changes include:
- Updated Roslyn from 4.14.0 to 5.0.0 and MsBuild from 17.14.28 to 18.0.2
- Fixed
ImplicitToStringExprimplementation to correctly handle params collections - Adapted test code to avoid C# 14
fieldkeyword conflicts by using escaped identifiers
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| csharp/paket.dependencies | Updated dependency versions for Basic.CompilerLog.Util (0.9.21→0.9.25), Microsoft.CodeAnalysis (4.14.0→5.0.0), and Microsoft.Build (17.14.28→18.0.2) |
| csharp/paket.lock | Updated lock file reflecting new dependency versions and transitive dependencies |
| csharp/paket.main.bzl | Updated Bazel package definitions with new versions and SHA512 hashes |
| csharp/ql/lib/semmle/code/csharp/commons/Strings.qll | Fixed bug: changed ArrayType to ParamsCollectionType in ImplicitToStringExpr to properly handle params collections in string.Format |
| csharp/ql/test/library-tests/dataflow/local/LocalDataFlow.cs | Replaced Span conversion test with DateTime conversion test due to Roslyn no longer extracting implicit Span conversion operators |
| csharp/ql/test/library-tests/dataflow/local/*.expected | Updated test expectations to reflect changes in data flow from test code modification |
| csharp/ql/test/library-tests/csharp7/CSharp7.cs | Escaped field identifier as @field to avoid conflict with C# 14 reserved keyword |
| csharp/ql/test/library-tests/csharp7/*.expected | Updated test expectations reflecting column shifts from escaped identifier change |
| csharp/ql/test/library-tests/csharp7.3/PrintAst.expected | Updated AST output: Span conversion now represented as CastExpr instead of OperatorCall |
| csharp/ql/lib/change-notes/2025-12-11-net10-basic-support.md | Added change note documenting basic .NET 10 support |
| csharp/.vscode/launch.json | Updated debug configurations from net9.0 to net10.0 target framework |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hvitved
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR updates the Roslyn, Build, and Binlog dependencies. A few notes:
Spanas discussed here, Roslyn no longer extracts implicit conversion methods. The rules governing implicit conversions are outlined here. As a result, we had to modify a test example and update some output formatting.string.Formatexposed a bug in theImplicitToStringExprimplementation, which has now been resolved.Notes on DCA execution.
cs/call-to-object-tostringwhich can be explained by the change inImplicitToStringExpr(results appear to all be related tostring.Format).cs/useless-assignment-to-local. A couple of false positives, but also several alerts related to fields namedfieldin properties in BMN which probably can be explained by compilation errors (sincefieldis a reserved word in these contexts from C# 14).Overall I think DCA looks good.