ILLink: codefix for C# unsafe evolution#128304
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-text-json |
There was a problem hiding this comment.
Pull request overview
This PR adds a new ILLink Roslyn analyzer + code fix (IL5005) intended to move the unsafe member modifier into a method-/accessor-scoped unsafe { ... } block, and wires an MSBuild property to enable the analyzer. It also applies the fixer output broadly across System.Text.Json and some shared library code.
Changes:
- Add IL5005 (
UnsafeModifierOnMethod) analyzer and code fix provider, plus associated resource strings and MSBuild property plumbed through ILLink analyzer infrastructure. - Update build logic (
eng/liveILLink.targets) and System.Text.Json project settings to enable the analyzer. - Mechanical rewrites across many BCL files replacing
unsafemodifiers with method-bodyunsafe { ... }blocks and inserting// SAFETY-TODOcomments.
Reviewed changes
Copilot reviewed 58 out of 58 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/illink/src/ILLink.Shared/SharedStrings.resx | Adds IL5005 title/message strings. |
| src/tools/illink/src/ILLink.Shared/DiagnosticId.cs | Introduces DiagnosticId.UnsafeModifierOnMethod = 5005 (DEBUG-only). |
| src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeModifierOnMethodAnalyzer.cs | New analyzer that reports IL5005 on members using the unsafe modifier (when enabled). |
| src/tools/illink/src/ILLink.RoslynAnalyzer/MSBuildPropertyOptionNames.cs | Adds MSBuild property name constant to enable the new analyzer (DEBUG-only). |
| src/tools/illink/src/ILLink.RoslynAnalyzer/build/Microsoft.NET.ILLink.Analyzers.props | Makes the new MSBuild property compiler-visible. |
| src/tools/illink/src/ILLink.CodeFix/UnsafeModifierOnMethodCodeFixProvider.cs | New code fix to move unsafe into the body and insert audit comments. |
| src/tools/illink/src/ILLink.CodeFix/Resources.resx | Adds the code fix title resource. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.UnsignedNumber.cs | Applies fixer output: wraps stackalloc usage in unsafe {} and adds SAFETY-TODO. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.StringSegment.cs | Applies fixer output to multiple writer helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.String.cs | Applies fixer output to string escaping helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.SignedNumber.cs | Applies fixer output for numeric formatting helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Raw.cs | Applies fixer output around transcoding + pooled buffer logic. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Float.cs | Applies fixer output for float formatting paths. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Double.cs | Applies fixer output for double formatting paths. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Decimal.cs | Applies fixer output for decimal formatting paths. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.UnsignedNumber.cs | Applies fixer output in property-name numeric writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.SignedNumber.cs | Applies fixer output in property-name numeric writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Literal.cs | Applies fixer output in literal property-name writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Guid.cs | Applies fixer output in Guid property-name writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.FormattedNumber.cs | Applies fixer output in formatted-number property writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Float.cs | Applies fixer output in float property writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Double.cs | Applies fixer output in double property writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Decimal.cs | Applies fixer output in decimal property writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.DateTimeOffset.cs | Applies fixer output in DateTimeOffset property writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.DateTime.cs | Applies fixer output in DateTime property writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteProperties.Bytes.cs | Applies fixer output in base64 property writers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs | Applies fixer output in writer start-property escaping helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.Date.cs | Applies fixer output in Date/DateTimeOffset trim formatting helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Writer/JsonWriterHelper.cs | Applies fixer output in string quoting/escaping helper. |
| src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs | Applies fixer output in stackalloc-based Truncate helper. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.String.cs | Applies fixer output in UTF-16->UTF-8 transcoding read helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/UInt128Converter.cs | Applies fixer output in converter read/write helpers using stackalloc / pools. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TimeSpanConverter.cs | Applies fixer output in TimeSpan converter stackalloc paths. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/TimeOnlyConverter.cs | Applies fixer output in TimeOnly converter stackalloc paths. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/Int128Converter.cs | Applies fixer output in converter read/write helpers using stackalloc / pools. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/HalfConverter.cs | Applies fixer output in converter read/write helpers and constant parsing. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs | Applies fixer output around ValueStringBuilder(stackalloc) usage. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/DateOnlyConverter.cs | Applies fixer output in DateOnly converter stackalloc formatting. |
| src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/CharConverter.cs | Applies fixer output around stackalloc + CopyString. |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.TryGet.cs | Applies fixer output around escape handling / parsing helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.MultiSegment.cs | Applies fixer output in multi-segment literal validation helper. |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs | Applies fixer output in ValueTextEquals transcoding helper. |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.Unescaping.cs | Applies fixer output across unescaping and base64 helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.netstandard.cs | Applies fixer output in netstandard span scanning helper (vectorized path). |
| src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs | Applies fixer output in escaped DateTime/Guid parsing helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.cs | Applies fixer output to GetPath, introducing unsafe {} + SAFETY-TODO. |
| src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.Escaping.cs | Applies fixer output in escaping helpers using stackalloc/ArrayPool. |
| src/libraries/System.Text.Json/src/System/Text/Json/JsonEncodedText.cs | Applies fixer output in TranscodeAndEncode stackalloc/ArrayPool path. |
| src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.TryGetProperty.cs | Applies fixer output in property lookup helpers. |
| src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs | Applies fixer output in TextEquals transcoding helper. |
| src/libraries/System.Text.Json/src/System.Text.Json.csproj | Enables the new analyzer via project property. |
| src/libraries/System.Private.CoreLib/src/System/Text/Rune.cs | Applies fixer output to remove unsafe modifier and wrap bodies in unsafe {}. |
| src/libraries/Common/src/System/Text/AsciiPolyfills.cs | Applies fixer output to move unsafe from signature into body. |
| src/libraries/Common/src/Polyfills/StringBuilderPolyfills.cs | Applies fixer output to move unsafe from signature into body. |
| src/libraries/Common/src/Polyfills/SinglePolyfills.cs | Applies fixer output to move unsafe from signature into body. |
| src/libraries/Common/src/Polyfills/EncodingPolyfills.cs | Applies fixer output to move unsafe from signatures into bodies and wrap pointer helpers. |
| eng/liveILLink.targets | Treats the new MSBuild property as requiring live ILLink wiring. |
|
PTAL @agocke @jkotas @jjonescz @tannergooding @333fred I tested it on STJ and it worked as I expected, it gave up on a few cases (see desc.) but I was able to easily fix those by hand.
We might need unsafe-as-expression for it then. |
Analyzer + code fixers driven by `dotnet format` (and IDE lightbulb) to migrate assemblies to the proposed C# unsafe-evolution rules (https://github.com/dotnet/csharplang/blob/main/proposals/unsafe-evolution.md). - IL5005 / IL5006 + RemoveUnsafeModifierCodeFixProvider: strip `unsafe` from declarations where it is meaningless (types, static ctors, destructors, delegates, members whose signature has no pointers). - IntroduceUnsafeBlockCodeFixProvider: wraps CS0214 / CS9360 / CS9361 / CS9362 / CS9363 sites in `unsafe { /* SAFETY-TODO: Audit */ ... }`, with smart forward-decl + `scoped` insertion for stackalloc-into-Span, density-based whole-body wrap, lambda-boundary handling, and bail-outs for unsafe rewrites (`using`, `ref`/`scoped` locals, `out var` patterns, anonymous types, internal preprocessor directives). - All gated by `#if DEBUG`, following the existing RequiresUnsafeCodeFixProvider pattern. - 77 xUnit tests in ILLink.RoslynAnalyzer.Tests.
…ceUnsafeBlockCodeFix Address PR dotnet#128304 Copilot review: * AnyReferenceAfter now scans SwitchSectionSyntax statement lists too. Without this, a local declared in a case whose subsequent statements use it was mis-routed to WrapAsIs and produced code where the local went out of scope (uncompilable). * WrapSingleStatementAsync now only attaches the original statement's outer trivia to the inner pieces when splicing into a Block/SwitchSection list. For embedded statements ReplaceStatementWithStatements wraps the result in a fresh block and applies the trivia there; doing it on both layers duplicated comments, blank lines and indentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…builds IL5005/IL5006 are declared in AnalyzerReleases.Unshipped.md but the UnsafeEvolutionAnalyzer class that supports them is gated by #if DEBUG. In Release builds of the analyzer DLL the rule ids have no matching SupportedDiagnostics entry, so the release-tracking analyzer (RS2002) fires on every consumer that loads it. Microsoft.CodeAnalysis.Analyzers' targets auto-include AnalyzerReleases.*.md when the file exists, so a plain conditional <AdditionalFiles Include/Remove> in an <ItemGroup> is undone by the package import that runs afterwards. The Remove has to live in a <Target> that fires BeforeTargets=CoreCompile so it runs after all .targets have been imported. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ests, polish
Comments addressed:
* IntroduceUnsafeBlockCodeFixProvider now handles expression-bodied members. Diagnostics in 'int M() => UnsafeCall();', 'int P => UnsafeCall();', etc. previously offered no fix because FindContainingStatement returned null. The fixer now also walks for an enclosing ArrowExpressionClauseSyntax and, when found, rewrites the member to a block body with 'unsafe { /* SAFETY-TODO */ return expr; }' (or 'expr;' for void/Task/set/init/add/remove/ctor/dtor). Properties/indexers with arrow bodies are converted to explicit 'get { ... }' accessors.
* Added EventFieldDeclaration / EventDeclaration tests for IL5006 -- the analyzer has registered for those syntax kinds but no test covered them.
* Removed misleading 'fall back to wrap-as-is' comment on the ForwardDeclare defensive path -- the implementation actually bails out unchanged (wrap-as-is would not be safe because the local escapes past the wrap point, which is why we picked ForwardDeclare). Comment updated to match behavior.
* Removed unused 'using System;' from RemoveUnsafeModifierCodeFixTests.cs.
All 51 UnsafeEvolution tests pass (was 46; +2 event tests, +3 arrow-body tests).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Analyzer + code fixers driven by
dotnet format(and the IDE lightbulb) to migrate assemblies to the proposed C# unsafe-evolution rules.Diagnostics
IL5005- meaninglessunsafeon a type / static ctor / destructor / delegate.IL5006- probably-unnecessaryunsafeon a member whose signature contains no pointers (skipsextern,partial, and members nested inside anunsafetype).Code fixers
RemoveUnsafeModifierCodeFixProvider- stripsunsafefor IL5005 / IL5006 / CS9377.IntroduceUnsafeBlockCodeFixProvider- wraps CS0214 / CS9360 / CS9361 / CS9362 / CS9363 sites inunsafe { /* SAFETY-TODO: Audit */ ... }. Features:T x;+unsafe { x = ...; }(addsscopedfor ref structs sostackalloctoSpan<T>compiles).int M() => UnsafeCall();) are rewritten into block bodies with the unsafe wrap.using/ref/scopedlocals,out varpatterns referenced after, anonymous-type locals that escape, internal preprocessor directives, and lambda-body diagnostics.Run it against System.Text.Json
#if DEBUG-gated):<Features>updated-memory-safety-rules</Features>line so the project compiles -dotnet formatskips projects whose references don't load:unsafe. Scope tosrc\so the heuristic doesn't touch shared polyfills:<Features>flag so CS9360/CS9361/CS9362 start firing:Result on STJ today: ~48 files auto-fixed cleanly. A handful of cases need manual finishing (CS8352 from
stackalloc-into-outer-scoped-Span splices, property-initializerCS9362).Tests: 82 xUnit tests in
ILLink.RoslynAnalyzer.Tests.All gated by
#if DEBUG, following the existingRequiresUnsafeCodeFixProviderpattern.