From 3ca02e6adfb1df9909c26598e4e2c09369ff5e96 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 20 Jul 2026 13:05:44 +0200 Subject: [PATCH 1/8] Add unsafe-v2 migration analyzers and code fixes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9b5b020c-fb1b-459b-a49e-36b77cb62d6c --- .../AddUnsafeToExternCodeFixProvider.cs | 47 ++++ .../AddUnsafeToFieldCodeFixProvider.cs | 44 +++ ...UnsafeToPointerSignatureCodeFixProvider.cs | 47 ++++ .../RemoveInvalidUnsafeCodeFixProvider.cs | 74 +++++ ...RemoveUndocumentedUnsafeCodeFixProvider.cs | 87 ++++++ .../illink/src/ILLink.CodeFix/Resources.resx | 18 ++ .../UnsafeModifierCodeFixHelpers.cs | 259 +++++++++++++++++ .../PointerSignatureRequiresUnsafeAnalyzer.cs | 65 +++++ ...emberMissingSafetyDocumentationAnalyzer.cs | 76 +++++ .../UnsafeMigrationAnalyzerHelpers.cs | 264 ++++++++++++++++++ .../src/ILLink.Shared/DiagnosticCategory.cs | 1 + .../illink/src/ILLink.Shared/DiagnosticId.cs | 5 + .../src/ILLink.Shared/SharedStrings.resx | 12 + .../AddUnsafeToExternCodeFixTests.cs | 169 +++++++++++ .../AddUnsafeToFieldCodeFixTests.cs | 192 +++++++++++++ ...AddUnsafeToPointerSignatureCodeFixTests.cs | 207 ++++++++++++++ ...terSignatureRequiresUnsafeAnalyzerTests.cs | 102 +++++++ .../RemoveInvalidUnsafeCodeFixTests.cs | 61 ++++ .../RemoveUndocumentedUnsafeCodeFixTests.cs | 222 +++++++++++++++ ...MissingSafetyDocumentationAnalyzerTests.cs | 120 ++++++++ .../UnsafeMigrationTestHelpers.cs | 78 ++++++ 21 files changed, 2150 insertions(+) create mode 100644 src/tools/illink/src/ILLink.CodeFix/AddUnsafeToExternCodeFixProvider.cs create mode 100644 src/tools/illink/src/ILLink.CodeFix/AddUnsafeToFieldCodeFixProvider.cs create mode 100644 src/tools/illink/src/ILLink.CodeFix/AddUnsafeToPointerSignatureCodeFixProvider.cs create mode 100644 src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs create mode 100644 src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs create mode 100644 src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs create mode 100644 src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs create mode 100644 src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs create mode 100644 src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs create mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToExternCodeFixTests.cs create mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToFieldCodeFixTests.cs create mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToPointerSignatureCodeFixTests.cs create mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs create mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveInvalidUnsafeCodeFixTests.cs create mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveUndocumentedUnsafeCodeFixTests.cs create mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs create mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs diff --git a/src/tools/illink/src/ILLink.CodeFix/AddUnsafeToExternCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/AddUnsafeToExternCodeFixProvider.cs new file mode 100644 index 00000000000000..d3a5479b631db9 --- /dev/null +++ b/src/tools/illink/src/ILLink.CodeFix/AddUnsafeToExternCodeFixProvider.cs @@ -0,0 +1,47 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Collections.Immutable; +using System.Composition; +using System.Threading.Tasks; +using ILLink.CodeFixProvider; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace ILLink.CodeFix +{ + /// + /// Fixes compiler diagnostic CS9389 by marking an unclassified extern member unsafe. + /// The generated contract is intentionally conservative so developers can replace it with safe after audit. + /// + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AddUnsafeToExternCodeFixProvider)), Shared] + public sealed class AddUnsafeToExternCodeFixProvider : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider + { + public const string ExternMemberRequiresUnsafeOrSafeDiagnosticId = "CS9389"; + + private static LocalizableString CodeFixTitle => + new LocalizableResourceString( + nameof(Resources.AddUnsafeToExternCodeFixTitle), + Resources.ResourceManager, + typeof(Resources)); + + public override ImmutableArray FixableDiagnosticIds => + [ExternMemberRequiresUnsafeOrSafeDiagnosticId]; + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override Task RegisterCodeFixesAsync(CodeFixContext context) => + UnsafeModifierCodeFixHelpers.RegisterAddUnsafeCodeFixAsync( + context, + CodeFixTitle, + static declaration => declaration is BaseMethodDeclarationSyntax + or PropertyDeclarationSyntax + or IndexerDeclarationSyntax + or EventDeclarationSyntax + or EventFieldDeclarationSyntax + or LocalFunctionStatementSyntax); + } +} +#endif diff --git a/src/tools/illink/src/ILLink.CodeFix/AddUnsafeToFieldCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/AddUnsafeToFieldCodeFixProvider.cs new file mode 100644 index 00000000000000..75cad48a0cc2e3 --- /dev/null +++ b/src/tools/illink/src/ILLink.CodeFix/AddUnsafeToFieldCodeFixProvider.cs @@ -0,0 +1,44 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Collections.Immutable; +using System.Composition; +using System.Threading.Tasks; +using ILLink.CodeFixProvider; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace ILLink.CodeFix +{ + /// + /// Fixes compiler diagnostic CS9392 by marking field-like members in explicit or extended layouts unsafe. + /// Primary-constructor parameters are intentionally unsupported because C# cannot place the modifier on them. + /// + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AddUnsafeToFieldCodeFixProvider)), Shared] + public sealed class AddUnsafeToFieldCodeFixProvider : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider + { + public const string FieldRequiresUnsafeOrSafeDiagnosticId = "CS9392"; + + private static LocalizableString CodeFixTitle => + new LocalizableResourceString( + nameof(Resources.AddUnsafeToFieldCodeFixTitle), + Resources.ResourceManager, + typeof(Resources)); + + public override ImmutableArray FixableDiagnosticIds => + [FieldRequiresUnsafeOrSafeDiagnosticId]; + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override Task RegisterCodeFixesAsync(CodeFixContext context) => + UnsafeModifierCodeFixHelpers.RegisterAddUnsafeCodeFixAsync( + context, + CodeFixTitle, + static declaration => declaration is FieldDeclarationSyntax + or PropertyDeclarationSyntax + or EventFieldDeclarationSyntax); + } +} +#endif diff --git a/src/tools/illink/src/ILLink.CodeFix/AddUnsafeToPointerSignatureCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/AddUnsafeToPointerSignatureCodeFixProvider.cs new file mode 100644 index 00000000000000..a51647a5fae0ea --- /dev/null +++ b/src/tools/illink/src/ILLink.CodeFix/AddUnsafeToPointerSignatureCodeFixProvider.cs @@ -0,0 +1,47 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Collections.Immutable; +using System.Composition; +using System.Threading.Tasks; +using ILLink.CodeFixProvider; +using ILLink.Shared; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace ILLink.CodeFix +{ + /// + /// Fixes analyzer diagnostic IL5006 by adding unsafe to a pointer or function-pointer signature. + /// This preserves the caller-unsafe behavior that legacy memory-safety rules inferred from those signatures. + /// + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AddUnsafeToPointerSignatureCodeFixProvider)), Shared] + public sealed class AddUnsafeToPointerSignatureCodeFixProvider : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider + { + private static LocalizableString CodeFixTitle => + new LocalizableResourceString( + nameof(Resources.AddUnsafeToPointerSignatureCodeFixTitle), + Resources.ResourceManager, + typeof(Resources)); + + public override ImmutableArray FixableDiagnosticIds => + [DiagnosticId.PointerSignatureRequiresUnsafe.AsString()]; + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override Task RegisterCodeFixesAsync(CodeFixContext context) => + UnsafeModifierCodeFixHelpers.RegisterAddUnsafeCodeFixAsync( + context, + CodeFixTitle, + static declaration => declaration is BaseMethodDeclarationSyntax + or PropertyDeclarationSyntax + or IndexerDeclarationSyntax + or EventDeclarationSyntax + or EventFieldDeclarationSyntax + or FieldDeclarationSyntax + or LocalFunctionStatementSyntax); + } +} +#endif diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs new file mode 100644 index 00000000000000..2d7320ff740e23 --- /dev/null +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs @@ -0,0 +1,74 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Collections.Immutable; +using System.Composition; +using System.Globalization; +using System.Threading.Tasks; +using ILLink.CodeFixProvider; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; + +namespace ILLink.CodeFix +{ + /// + /// Fixes CS9377 and unsafe-specific CS0106 diagnostics by removing the invalid unsafe modifier. + /// The shared CS0106 ID is filtered so modifiers unrelated to unsafe are left to their own fixes. + /// + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(RemoveInvalidUnsafeCodeFixProvider)), Shared] + public sealed class RemoveInvalidUnsafeCodeFixProvider : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider + { + public const string UnsafeModifierHasNoEffectDiagnosticId = "CS9377"; + public const string InvalidModifierDiagnosticId = "CS0106"; + + private static LocalizableString CodeFixTitle => + new LocalizableResourceString( + nameof(Resources.RemoveInvalidUnsafeCodeFixTitle), + Resources.ResourceManager, + typeof(Resources)); + + public override ImmutableArray FixableDiagnosticIds => + [UnsafeModifierHasNoEffectDiagnosticId, InvalidModifierDiagnosticId]; + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + Diagnostic diagnostic = context.Diagnostics[0]; + // CS0106 is shared by every invalid modifier, so only handle the unsafe-specific form. + if (diagnostic.Id == InvalidModifierDiagnosticId + && diagnostic.GetMessage(CultureInfo.InvariantCulture) + .IndexOf("'unsafe'", System.StringComparison.Ordinal) < 0) + { + return; + } + + if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root) + return; + + SyntaxNode targetNode = root.FindNode( + diagnostic.Location.SourceSpan, + getInnermostNodeForTie: true); + if (UnsafeModifierCodeFixHelpers.FindDeclaration(targetNode) is not { } declaration + || !UnsafeModifierCodeFixHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) + { + return; + } + + string title = CodeFixTitle.ToString(); + context.RegisterCodeFix( + CodeAction.Create( + title, + cancellationToken => UnsafeModifierCodeFixHelpers.RemoveUnsafeModifierAsync( + context.Document, + declaration, + cancellationToken), + title), + diagnostic); + } + } +} +#endif diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs new file mode 100644 index 00000000000000..c33eed5d7a6593 --- /dev/null +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs @@ -0,0 +1,87 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Collections.Immutable; +using System.Composition; +using System.Threading.Tasks; +using ILLink.CodeFixProvider; +using ILLink.RoslynAnalyzer; +using ILLink.Shared; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; + +namespace ILLink.CodeFix +{ + /// + /// Fixes IL5005 by removing undocumented legacy unsafe scopes that became caller contracts under unsafe-v2. + /// Pointer signatures retain unsafe, while field-like CS9392 declarations are changed to safe. + /// + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(RemoveUndocumentedUnsafeCodeFixProvider)), Shared] + public sealed class RemoveUndocumentedUnsafeCodeFixProvider : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider + { + private static LocalizableString RemoveCodeFixTitle => + new LocalizableResourceString( + nameof(Resources.RemoveUndocumentedUnsafeCodeFixTitle), + Resources.ResourceManager, + typeof(Resources)); + + private static LocalizableString ReplaceCodeFixTitle => + new LocalizableResourceString( + nameof(Resources.ReplaceUndocumentedUnsafeWithSafeCodeFixTitle), + Resources.ResourceManager, + typeof(Resources)); + + public override ImmutableArray FixableDiagnosticIds => + [DiagnosticId.UnsafeMemberMissingSafetyDocumentation.AsString()]; + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + Diagnostic diagnostic = context.Diagnostics[0]; + // These members were already caller-unsafe under the legacy pointer compatibility rules. + if (diagnostic.Properties.ContainsKey( + UnsafeMemberMissingSafetyDocumentationAnalyzer.PointerSignatureProperty)) + { + return; + } + + if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root) + return; + + SyntaxNode targetNode = root.FindNode( + diagnostic.Location.SourceSpan, + getInnermostNodeForTie: true); + if (UnsafeModifierCodeFixHelpers.FindDeclaration(targetNode) is not { } declaration + || !UnsafeModifierCodeFixHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) + { + return; + } + + bool replaceWithSafe = diagnostic.Properties.ContainsKey( + UnsafeMemberMissingSafetyDocumentationAnalyzer.RequiresExplicitSafetyModifierProperty) + && !UnsafeModifierCodeFixHelpers.HasSafeModifier(declaration); + // Bare removal would recreate CS9392 for field-backed explicit or extended layout members. + string title = (replaceWithSafe ? ReplaceCodeFixTitle : RemoveCodeFixTitle).ToString(); + + context.RegisterCodeFix( + CodeAction.Create( + title, + cancellationToken => replaceWithSafe + ? UnsafeModifierCodeFixHelpers.ReplaceUnsafeWithSafeModifierAsync( + context.Document, + declaration, + cancellationToken) + : UnsafeModifierCodeFixHelpers.RemoveUnsafeModifierAsync( + context.Document, + declaration, + cancellationToken), + title), + diagnostic); + } + } +} +#endif diff --git a/src/tools/illink/src/ILLink.CodeFix/Resources.resx b/src/tools/illink/src/ILLink.CodeFix/Resources.resx index d552d7228b3e62..70bbe4a82b7e7e 100644 --- a/src/tools/illink/src/ILLink.CodeFix/Resources.resx +++ b/src/tools/illink/src/ILLink.CodeFix/Resources.resx @@ -129,6 +129,24 @@ Add 'unsafe' to parent method + + Mark extern member 'unsafe' + + + Remove invalid 'unsafe' modifier + + + Remove undocumented 'unsafe' modifier + + + Mark field-like member 'safe' + + + Add 'unsafe' to pointer signature + + + Mark field-like member 'unsafe' + Add UnconditionalSuppressMessage attribute to parent method diff --git a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs new file mode 100644 index 00000000000000..f75532e04f38a6 --- /dev/null +++ b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs @@ -0,0 +1,259 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; + +namespace ILLink.CodeFix +{ + /// + /// Centralizes modifier discovery and trivia-preserving edits for the unsafe-v2 code-fix providers. + /// It is shared by fixes for CS9389, CS9377/CS0106, IL5005, IL5006, and CS9392. + /// + internal static class UnsafeModifierCodeFixHelpers + { + // The referenced Workspaces package parses safe but does not expose SyntaxKind.SafeKeyword yet. + private static readonly SyntaxToken s_safeModifier = + ((FieldDeclarationSyntax)SyntaxFactory.ParseMemberDeclaration("safe int field;")!).Modifiers[0]; + + internal static async Task RegisterAddUnsafeCodeFixAsync( + CodeFixContext context, + LocalizableString codeFixTitle, + Func isSupportedDeclaration) + { + if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root) + return; + + SyntaxNode targetNode = root.FindNode( + context.Diagnostics[0].Location.SourceSpan, + getInnermostNodeForTie: true); + if (FindDeclaration(targetNode) is not { } declaration + || !isSupportedDeclaration(declaration) + || HasModifier(declaration, SyntaxKind.UnsafeKeyword) + || HasSafeModifier(declaration)) + { + return; + } + + string title = codeFixTitle.ToString(); + context.RegisterCodeFix( + CodeAction.Create( + title, + cancellationToken => AddUnsafeModifierAsync(context.Document, declaration, cancellationToken), + title), + context.Diagnostics[0]); + } + + internal static SyntaxNode? FindDeclaration(SyntaxNode node) => + node.AncestorsAndSelf().FirstOrDefault(static ancestor => + ancestor is MemberDeclarationSyntax + or LocalFunctionStatementSyntax + or AccessorDeclarationSyntax); + + internal static bool HasModifier(SyntaxNode declaration, SyntaxKind modifier) => + GetModifiers(declaration).Any(modifier); + + internal static bool HasSafeModifier(SyntaxNode declaration) + { + foreach (SyntaxToken modifier in GetModifiers(declaration)) + { + if (modifier.ValueText == "safe") + return true; + } + + return false; + } + + internal static async Task AddUnsafeModifierAsync( + Document document, + SyntaxNode declaration, + CancellationToken cancellationToken) + { + var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + if (declaration is AccessorDeclarationSyntax accessor) + { + editor.ReplaceNode(accessor, AddUnsafeModifier(accessor)); + } + else if (declaration is DestructorDeclarationSyntax destructor) + { + // SyntaxGenerator does not preserve extern on destructors in the current Workspaces dependency. + editor.ReplaceNode( + destructor, + destructor.WithModifiers(AddUnsafeModifier(destructor.Modifiers))); + } + else + { + var modifiers = editor.Generator.GetModifiers(declaration); + editor.SetModifiers(declaration, modifiers.WithIsUnsafe(true)); + } + + return editor.GetChangedDocument(); + } + + internal static async Task RemoveUnsafeModifierAsync( + Document document, + SyntaxNode declaration, + CancellationToken cancellationToken) + { + var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + if (declaration is AccessorDeclarationSyntax accessor) + { + editor.ReplaceNode(accessor, RemoveUnsafeModifier(accessor)); + } + else if (declaration is DestructorDeclarationSyntax destructor + && destructor.Modifiers.Any(SyntaxKind.ExternKeyword)) + { + // Remove only unsafe; routing this through SyntaxGenerator would also remove extern. + editor.ReplaceNode( + destructor, + destructor.WithModifiers(RemoveUnsafeModifier(destructor.Modifiers))); + } + else + { + var modifiers = editor.Generator.GetModifiers(declaration); + editor.SetModifiers(declaration, modifiers.WithIsUnsafe(false)); + } + + return editor.GetChangedDocument(); + } + + internal static async Task ReplaceUnsafeWithSafeModifierAsync( + Document document, + SyntaxNode declaration, + CancellationToken cancellationToken) + { + SyntaxToken unsafeModifier = GetUnsafeModifier(declaration); + SyntaxToken safeModifier = s_safeModifier + .WithLeadingTrivia(unsafeModifier.LeadingTrivia) + .WithTrailingTrivia(unsafeModifier.TrailingTrivia); + + var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + editor.ReplaceNode(declaration, declaration.ReplaceToken(unsafeModifier, safeModifier)); + return editor.GetChangedDocument(); + } + + private static SyntaxTokenList GetModifiers(SyntaxNode declaration) => + declaration switch + { + MemberDeclarationSyntax member => member.Modifiers, + LocalFunctionStatementSyntax localFunction => localFunction.Modifiers, + AccessorDeclarationSyntax accessor => accessor.Modifiers, + _ => default, + }; + + private static AccessorDeclarationSyntax AddUnsafeModifier(AccessorDeclarationSyntax accessor) + { + // SyntaxGenerator does not yet model unsafe property accessors, so edit their tokens directly. + if (accessor.Modifiers.Count > 0) + return accessor.WithModifiers(AddUnsafeModifier(accessor.Modifiers)); + + SyntaxToken unsafeModifier = SyntaxFactory.Token(SyntaxKind.UnsafeKeyword) + .WithLeadingTrivia(accessor.Keyword.LeadingTrivia) + .WithTrailingTrivia(SyntaxFactory.ElasticSpace); + return accessor + .WithModifiers([unsafeModifier]) + .WithKeyword(accessor.Keyword.WithLeadingTrivia(default(SyntaxTriviaList))); + } + + private static AccessorDeclarationSyntax RemoveUnsafeModifier(AccessorDeclarationSyntax accessor) + { + // Keep declaration-leading trivia attached to the first remaining token. + SyntaxTokenList modifiers = accessor.Modifiers; + int unsafeIndex = GetUnsafeModifierIndex(modifiers); + SyntaxTriviaList leadingTrivia = modifiers[unsafeIndex].LeadingTrivia; + modifiers = modifiers.RemoveAt(unsafeIndex); + + if (unsafeIndex == 0) + { + if (modifiers.Count > 0) + { + modifiers = modifiers.Replace( + modifiers[0], + modifiers[0].WithLeadingTrivia(leadingTrivia.AddRange(modifiers[0].LeadingTrivia))); + } + else + { + return accessor + .WithModifiers(modifiers) + .WithKeyword(accessor.Keyword.WithLeadingTrivia(leadingTrivia.AddRange(accessor.Keyword.LeadingTrivia))); + } + } + + return accessor.WithModifiers(modifiers); + } + + private static SyntaxTokenList AddUnsafeModifier(SyntaxTokenList modifiers) + { + // Match SyntaxGenerator's canonical order by placing unsafe before extern. + int insertionIndex = modifiers.Count; + for (int i = 0; i < modifiers.Count; i++) + { + if (modifiers[i].IsKind(SyntaxKind.ExternKeyword)) + { + insertionIndex = i; + break; + } + } + + SyntaxToken unsafeModifier = SyntaxFactory.Token(SyntaxKind.UnsafeKeyword) + .WithTrailingTrivia(SyntaxFactory.ElasticSpace); + if (insertionIndex == 0) + { + unsafeModifier = unsafeModifier.WithLeadingTrivia(modifiers[0].LeadingTrivia); + modifiers = modifiers.Replace( + modifiers[0], + modifiers[0].WithLeadingTrivia(default(SyntaxTriviaList))); + } + + return modifiers.Insert(insertionIndex, unsafeModifier); + } + + private static SyntaxTokenList RemoveUnsafeModifier(SyntaxTokenList modifiers) + { + int unsafeIndex = GetUnsafeModifierIndex(modifiers); + SyntaxTriviaList leadingTrivia = modifiers[unsafeIndex].LeadingTrivia; + modifiers = modifiers.RemoveAt(unsafeIndex); + + if (unsafeIndex == 0 && modifiers.Count > 0) + { + modifiers = modifiers.Replace( + modifiers[0], + modifiers[0].WithLeadingTrivia(leadingTrivia.AddRange(modifiers[0].LeadingTrivia))); + } + + return modifiers; + } + + private static SyntaxToken GetUnsafeModifier(SyntaxNode declaration) + { + foreach (SyntaxToken modifier in GetModifiers(declaration)) + { + if (modifier.IsKind(SyntaxKind.UnsafeKeyword)) + return modifier; + } + + return default; + } + + private static int GetUnsafeModifierIndex(SyntaxTokenList modifiers) + { + for (int i = 0; i < modifiers.Count; i++) + { + if (modifiers[i].IsKind(SyntaxKind.UnsafeKeyword)) + return i; + } + + return -1; + } + } +} +#endif diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs new file mode 100644 index 00000000000000..87e59c8c3f071c --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs @@ -0,0 +1,65 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Collections.Immutable; +using ILLink.Shared; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace ILLink.RoslynAnalyzer +{ + /// + /// Reports IL5006 for pointer or function-pointer signatures that lost their legacy caller-unsafe contract. + /// An existing unsafe/safe modifier or a <safety> XML comment suppresses the diagnostic. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class PointerSignatureRequiresUnsafeAnalyzer : DiagnosticAnalyzer + { + private static readonly DiagnosticDescriptor s_rule = + DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.PointerSignatureRequiresUnsafe); + + public override ImmutableArray SupportedDiagnostics => [s_rule]; + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + if (!System.Diagnostics.Debugger.IsAttached) + context.EnableConcurrentExecution(); + + context.RegisterSyntaxNodeAction( + AnalyzeDeclaration, + UnsafeMigrationAnalyzerHelpers.PointerSignatureDeclarationKinds); + } + + private static void AnalyzeDeclaration(SyntaxNodeAnalysisContext context) + { + SyntaxNode declaration = context.Node; + // safe is an explicit audited contract on the limited declaration kinds where Roslyn permits it. + if (UnsafeMigrationAnalyzerHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword) + || UnsafeMigrationAnalyzerHelpers.HasSafeModifier(declaration)) + { + return; + } + + ISymbol? symbol = UnsafeMigrationAnalyzerHelpers.GetDeclaredSymbol( + declaration, + context.SemanticModel, + context.CancellationToken); + + if (symbol is null + || !UnsafeMigrationAnalyzerHelpers.HasPointerOrFunctionPointerSignature(symbol) + || UnsafeMigrationAnalyzerHelpers.HasSafetyDocumentation(declaration, symbol, context.CancellationToken)) + { + return; + } + + context.ReportDiagnostic(Diagnostic.Create( + s_rule, + UnsafeMigrationAnalyzerHelpers.GetIdentifierLocation(declaration))); + } + } +} +#endif diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs new file mode 100644 index 00000000000000..111a857e3afa34 --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs @@ -0,0 +1,76 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Collections.Immutable; +using ILLink.Shared; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace ILLink.RoslynAnalyzer +{ + /// + /// Reports IL5005 when an explicit unsafe member contract has no <safety> XML documentation. + /// Diagnostic properties guide the fixer when pointer compatibility or CS9392 requires preserving a modifier. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class UnsafeMemberMissingSafetyDocumentationAnalyzer : DiagnosticAnalyzer + { + public const string PointerSignatureProperty = nameof(PointerSignatureProperty); + public const string RequiresExplicitSafetyModifierProperty = nameof(RequiresExplicitSafetyModifierProperty); + + private static readonly DiagnosticDescriptor s_rule = + DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.UnsafeMemberMissingSafetyDocumentation); + + public override ImmutableArray SupportedDiagnostics => [s_rule]; + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + if (!System.Diagnostics.Debugger.IsAttached) + context.EnableConcurrentExecution(); + + context.RegisterSyntaxNodeAction( + AnalyzeDeclaration, + UnsafeMigrationAnalyzerHelpers.UnsafeContractDeclarationKinds); + } + + private static void AnalyzeDeclaration(SyntaxNodeAnalysisContext context) + { + SyntaxNode declaration = context.Node; + SyntaxToken unsafeModifier = UnsafeMigrationAnalyzerHelpers.GetModifier( + declaration, + SyntaxKind.UnsafeKeyword); + + if (unsafeModifier == default) + return; + + ISymbol? symbol = UnsafeMigrationAnalyzerHelpers.GetDeclaredSymbol( + declaration, + context.SemanticModel, + context.CancellationToken); + + if (!UnsafeMigrationAnalyzerHelpers.IsUnsafeContractMember(declaration, symbol) + || UnsafeMigrationAnalyzerHelpers.HasSafetyDocumentation(declaration, symbol, context.CancellationToken)) + { + return; + } + + var properties = ImmutableDictionary.Empty; + // Pointer signatures were caller-unsafe under the legacy rules, so the fixer must retain unsafe. + if (symbol is not null && UnsafeMigrationAnalyzerHelpers.HasPointerOrFunctionPointerSignature(symbol)) + properties = properties.Add(PointerSignatureProperty, bool.TrueString); + // Explicit and extended layouts still require an explicit safe/unsafe marker after migration. + if (UnsafeMigrationAnalyzerHelpers.RequiresExplicitSafetyModifier(declaration, symbol)) + properties = properties.Add(RequiresExplicitSafetyModifierProperty, bool.TrueString); + + context.ReportDiagnostic(Diagnostic.Create( + s_rule, + unsafeModifier.GetLocation(), + properties: properties)); + } + } +} +#endif diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs new file mode 100644 index 00000000000000..285fdd6b5cb528 --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs @@ -0,0 +1,264 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace ILLink.RoslynAnalyzer +{ + /// + /// Provides shared declaration, signature, documentation, and layout analysis for the unsafe-v2 migration tooling. + /// The helpers encode the compatibility rules used by the IL5005 and IL5006 analyzers. + /// + internal static class UnsafeMigrationAnalyzerHelpers + { + internal static readonly SyntaxKind[] UnsafeContractDeclarationKinds = + [ + SyntaxKind.MethodDeclaration, + SyntaxKind.ConstructorDeclaration, + SyntaxKind.OperatorDeclaration, + SyntaxKind.ConversionOperatorDeclaration, + SyntaxKind.LocalFunctionStatement, + SyntaxKind.PropertyDeclaration, + SyntaxKind.IndexerDeclaration, + SyntaxKind.GetAccessorDeclaration, + SyntaxKind.SetAccessorDeclaration, + SyntaxKind.InitAccessorDeclaration, + SyntaxKind.EventDeclaration, + SyntaxKind.EventFieldDeclaration, + SyntaxKind.FieldDeclaration, + ]; + + internal static readonly SyntaxKind[] PointerSignatureDeclarationKinds = + [ + SyntaxKind.MethodDeclaration, + SyntaxKind.ConstructorDeclaration, + SyntaxKind.OperatorDeclaration, + SyntaxKind.ConversionOperatorDeclaration, + SyntaxKind.LocalFunctionStatement, + SyntaxKind.PropertyDeclaration, + SyntaxKind.IndexerDeclaration, + SyntaxKind.EventDeclaration, + SyntaxKind.EventFieldDeclaration, + SyntaxKind.FieldDeclaration, + ]; + + internal static SyntaxTokenList GetModifiers(SyntaxNode declaration) => + declaration switch + { + MemberDeclarationSyntax member => member.Modifiers, + LocalFunctionStatementSyntax localFunction => localFunction.Modifiers, + AccessorDeclarationSyntax accessor => accessor.Modifiers, + _ => default, + }; + + internal static bool HasModifier(SyntaxNode declaration, SyntaxKind modifier) => + GetModifiers(declaration).Any(modifier); + + internal static bool HasSafeModifier(SyntaxNode declaration) + { + foreach (SyntaxToken modifier in GetModifiers(declaration)) + { + if (modifier.ValueText == "safe") + return true; + } + + return false; + } + + internal static SyntaxToken GetModifier(SyntaxNode declaration, SyntaxKind modifier) + { + foreach (SyntaxToken token in GetModifiers(declaration)) + { + if (token.IsKind(modifier)) + return token; + } + + return default; + } + + internal static ISymbol? GetDeclaredSymbol( + SyntaxNode declaration, + SemanticModel semanticModel, + CancellationToken cancellationToken) => + declaration switch + { + BaseFieldDeclarationSyntax field => field.Declaration.Variables + .Select(variable => semanticModel.GetDeclaredSymbol(variable, cancellationToken)) + .FirstOrDefault(symbol => symbol is not null), + _ => semanticModel.GetDeclaredSymbol(declaration, cancellationToken), + }; + + internal static bool IsUnsafeContractMember(SyntaxNode declaration, ISymbol? symbol) => + symbol switch + { + IMethodSymbol { MethodKind: MethodKind.StaticConstructor or MethodKind.Destructor } => false, + IMethodSymbol => true, + IFieldSymbol { IsConst: false } => true, + IPropertySymbol or IEventSymbol => true, + null => declaration switch + { + ConstructorDeclarationSyntax constructor when constructor.Modifiers.Any(SyntaxKind.StaticKeyword) => false, + DestructorDeclarationSyntax => false, + FieldDeclarationSyntax field when field.Modifiers.Any(SyntaxKind.ConstKeyword) => false, + _ => true, + }, + _ => false, + }; + + internal static bool HasPointerOrFunctionPointerSignature(ISymbol symbol) => + // Match Roslyn's compatibility rule: only member signature types participate, not constraints. + symbol switch + { + IMethodSymbol method => ContainsPointerOrFunctionPointer(method.ReturnType) + || method.Parameters.Any(static parameter => ContainsPointerOrFunctionPointer(parameter.Type)), + IPropertySymbol property => ContainsPointerOrFunctionPointer(property.Type) + || property.Parameters.Any(static parameter => ContainsPointerOrFunctionPointer(parameter.Type)), + IFieldSymbol { IsFixedSizeBuffer: false } field => ContainsPointerOrFunctionPointer(field.Type), + IEventSymbol @event => ContainsPointerOrFunctionPointer(@event.Type), + _ => false, + }; + + internal static bool HasSafetyDocumentation( + SyntaxNode declaration, + ISymbol? symbol, + CancellationToken cancellationToken) + { + if (HasSafetyDocumentation(declaration)) + return true; + + // Documentation on another partial declaration or on an associated property also describes this contract. + return symbol is not null && GetDocumentationSymbols(symbol) + .SelectMany(static relatedSymbol => relatedSymbol.DeclaringSyntaxReferences) + .Select(reference => reference.GetSyntax(cancellationToken)) + .Any(HasSafetyDocumentation); + } + + internal static bool RequiresExplicitSafetyModifier(SyntaxNode declaration, ISymbol? symbol) + { + if (declaration is AccessorDeclarationSyntax || symbol?.ContainingType is not { } containingType) + return false; + + // Removing unsafe from a field-backed declaration in these layouts would immediately reintroduce CS9392. + bool hasInstanceField = symbol switch + { + IFieldSymbol { IsStatic: false, IsConst: false } => true, + IPropertySymbol property => HasInstanceBackingField(property), + IEventSymbol { IsStatic: false } when declaration is EventFieldDeclarationSyntax => true, + IEventSymbol @event => HasInstanceBackingField(@event), + _ => false, + }; + + return hasInstanceField && HasExplicitOrExtendedLayout(containingType); + } + + internal static Location GetIdentifierLocation(SyntaxNode declaration) => + declaration switch + { + MethodDeclarationSyntax method => method.Identifier.GetLocation(), + ConstructorDeclarationSyntax constructor => constructor.Identifier.GetLocation(), + OperatorDeclarationSyntax @operator => @operator.OperatorToken.GetLocation(), + ConversionOperatorDeclarationSyntax conversion => conversion.Type.GetLocation(), + LocalFunctionStatementSyntax localFunction => localFunction.Identifier.GetLocation(), + PropertyDeclarationSyntax property => property.Identifier.GetLocation(), + IndexerDeclarationSyntax indexer => indexer.ThisKeyword.GetLocation(), + AccessorDeclarationSyntax accessor => accessor.Keyword.GetLocation(), + EventDeclarationSyntax @event => @event.Identifier.GetLocation(), + BaseFieldDeclarationSyntax field when field.Declaration.Variables.FirstOrDefault() is { } variable => variable.Identifier.GetLocation(), + _ => declaration.GetFirstToken().GetLocation(), + }; + + private static bool ContainsPointerOrFunctionPointer(ITypeSymbol type) + { + var types = new Stack(); + types.Push(type); + + while (types.Count > 0) + { + switch (types.Pop()) + { + case IPointerTypeSymbol or IFunctionPointerTypeSymbol: + return true; + + case IArrayTypeSymbol array: + types.Push(array.ElementType); + break; + + case INamedTypeSymbol namedType: + // Nested types can carry pointer-containing arguments on an enclosing type. + if (namedType.ContainingType is { } containingType) + types.Push(containingType); + + foreach (ITypeSymbol typeArgument in namedType.TypeArguments) + types.Push(typeArgument); + break; + } + } + + return false; + } + + private static IEnumerable GetDocumentationSymbols(ISymbol symbol) + { + yield return symbol; + + // Partial declarations share one contract, and accessor contracts may be documented on the property. + switch (symbol) + { + case IMethodSymbol method: + if (method.PartialDefinitionPart is { } methodDefinition) + yield return methodDefinition; + if (method.PartialImplementationPart is { } methodImplementation) + yield return methodImplementation; + if (method.AssociatedSymbol is { } associatedSymbol) + { + foreach (ISymbol relatedSymbol in GetDocumentationSymbols(associatedSymbol)) + yield return relatedSymbol; + } + break; + + case IPropertySymbol property: + if (property.PartialDefinitionPart is { } propertyDefinition) + yield return propertyDefinition; + if (property.PartialImplementationPart is { } propertyImplementation) + yield return propertyImplementation; + break; + + case IEventSymbol @event: + if (@event.PartialDefinitionPart is { } eventDefinition) + yield return eventDefinition; + if (@event.PartialImplementationPart is { } eventImplementation) + yield return eventImplementation; + break; + } + } + + private static bool HasSafetyDocumentation(SyntaxNode declaration) => + declaration.GetLeadingTrivia().Any(static trivia => + trivia.GetStructure() is DocumentationCommentTriviaSyntax documentationComment + && documentationComment.DescendantNodes().Any(static node => + node is XmlElementSyntax { StartTag.Name.LocalName.ValueText: "safety" } + or XmlEmptyElementSyntax { Name.LocalName.ValueText: "safety" })); + + private static bool HasInstanceBackingField(ISymbol associatedSymbol) => + associatedSymbol.ContainingType.GetMembers() + .OfType() + .Any(field => !field.IsStatic + && SymbolEqualityComparer.Default.Equals(field.AssociatedSymbol, associatedSymbol)); + + private static bool HasExplicitOrExtendedLayout(INamedTypeSymbol type) => + type.GetAttributes().Any(static attribute => + attribute.AttributeClass is { } attributeType + && (attributeType.IsTypeOf("System.Runtime.InteropServices", "ExtendedLayoutAttribute") + || (attributeType.IsTypeOf("System.Runtime.InteropServices", nameof(StructLayoutAttribute)) + && attribute.ConstructorArguments is [{ Value: int layoutKind }] + && layoutKind == (int)LayoutKind.Explicit))); + } +} +#endif diff --git a/src/tools/illink/src/ILLink.Shared/DiagnosticCategory.cs b/src/tools/illink/src/ILLink.Shared/DiagnosticCategory.cs index 39626951acaaef..0ff3549bfd6977 100644 --- a/src/tools/illink/src/ILLink.Shared/DiagnosticCategory.cs +++ b/src/tools/illink/src/ILLink.Shared/DiagnosticCategory.cs @@ -11,5 +11,6 @@ internal static class DiagnosticCategory public const string SingleFile = nameof(SingleFile); public const string Trimming = nameof(Trimming); public const string AOT = nameof(AOT); + public const string Safety = nameof(Safety); } } diff --git a/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs b/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs index 997f3f232ae018..116954140fcd65 100644 --- a/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs +++ b/src/tools/illink/src/ILLink.Shared/DiagnosticId.cs @@ -219,6 +219,10 @@ public enum DiagnosticId // Feature guard diagnostic ids. ReturnValueDoesNotMatchFeatureGuards = 4000, InvalidFeatureGuard = 4001, + + // Memory safety migration diagnostic ids. + UnsafeMemberMissingSafetyDocumentation = 5005, + PointerSignatureRequiresUnsafe = 5006, } public static class DiagnosticIdExtensions @@ -251,6 +255,7 @@ public static string GetDiagnosticCategory(this DiagnosticId diagnosticId) => { > 2000 and < 3000 => DiagnosticCategory.Trimming, >= 3000 and < 3050 => DiagnosticCategory.SingleFile, + 5005 or 5006 => DiagnosticCategory.Safety, >= 3050 and <= 6000 => DiagnosticCategory.AOT, _ => throw new ArgumentException($"The provided diagnostic id '{diagnosticId}' does not fall into the range of supported warning codes 2001 to 6000 (inclusive).") }; diff --git a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx index 77eb755610ecb3..dcb7b1bdc6cbab 100644 --- a/src/tools/illink/src/ILLink.Shared/SharedStrings.resx +++ b/src/tools/illink/src/ILLink.Shared/SharedStrings.resx @@ -687,6 +687,18 @@ P/invoke method '{0}' declares a parameter with COM marshalling. Correctness of COM interop cannot be guaranteed after trimming. Interfaces and interface members might be removed. + + Unsafe members should document their safety requirements + + + An unsafe member must include a '<safety>' XML documentation comment. + + + Pointer signatures should preserve their unsafe contract + + + A member with a pointer or function-pointer signature must be marked 'unsafe' or include a '<safety>' XML documentation comment explaining why it is safe. + An attribute element has property but this could not be found. diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToExternCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToExternCodeFixTests.cs new file mode 100644 index 00000000000000..967b0f28a85a11 --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToExternCodeFixTests.cs @@ -0,0 +1,169 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Threading.Tasks; +using ILLink.CodeFix; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + /// + /// Verifies the CS9389 code fix for every extern declaration shape supported by the compiler. + /// The expected output also checks modifier ordering and preservation on destructors and local functions. + /// + public class AddUnsafeToExternCodeFixTests + { + public static TheoryData MemberKinds => new() + { + { + """ + class C + { + public {|CS9389:extern|} void M(); + } + """, + """ + class C + { + public unsafe extern void M(); + } + """ + }, + { + """ + class C + { + public {|CS9389:extern|} int P { get; set; } + } + """, + """ + class C + { + public unsafe extern int P { get; set; } + } + """ + }, + { + """ + class C + { + public {|CS9389:extern|} int this[int index] { get; set; } + } + """, + """ + class C + { + public unsafe extern int this[int index] { get; set; } + } + """ + }, + { + """ + using System; + + class C + { + public static {|CS9389:extern|} event Action E; + } + """, + """ + using System; + + class C + { + public static unsafe extern event Action E; + } + """ + }, + { + """ + class C + { + public {|CS9389:extern|} C(int value); + } + """, + """ + class C + { + public unsafe extern C(int value); + } + """ + }, + { + """ + class C + { + public static {|CS9389:extern|} C operator +(C left, C right); + } + """, + """ + class C + { + public static unsafe extern C operator +(C left, C right); + } + """ + }, + { + """ + class C + { + public static {|CS9389:extern|} explicit operator int(C value); + } + """, + """ + class C + { + public static unsafe extern explicit operator int(C value); + } + """ + }, + { + """ + class C + { + {|CS9389:extern|} ~C(); + } + """, + """ + class C + { + unsafe extern ~C(); + } + """ + }, + { + """ + class C + { + void M() + { + static {|CS9389:extern|} void Local(); + } + } + """, + """ + class C + { + void M() + { + static unsafe extern void Local(); + } + } + """ + }, + }; + + [Theory] + [MemberData(nameof(MemberKinds))] + public async Task AddsUnsafeToExternMember(string source, string fixedSource) + { + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + await test.RunAsync(); + } + } +} +#endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToFieldCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToFieldCodeFixTests.cs new file mode 100644 index 00000000000000..11371258256f85 --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToFieldCodeFixTests.cs @@ -0,0 +1,192 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Threading.Tasks; +using ILLink.CodeFix; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + /// + /// Verifies the CS9392 fix for explicit and extended-layout fields, properties, and field-like events. + /// It also covers Fix All behavior and the unfixable primary-constructor backing-field case. + /// + public class AddUnsafeToFieldCodeFixTests + { + public static TheoryData FieldLikeMembers => new() + { + { + """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [FieldOffset(0)] + public int {|CS9392:F|}; + } + """, + """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [FieldOffset(0)] + public unsafe int F; + } + """ + }, + { + """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + public int {|CS9392:P|} { get; set; } + } + """, + """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + public unsafe int P { get; set; } + } + """ + }, + { + """ + using System; + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + public event Action {|CS9392:E|}; + } + """, + """ + using System; + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + public unsafe event Action E; + } + """ + }, + { + """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + public int {|CS9392:P|} => field; + } + """, + """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + public unsafe int P => field; + } + """ + }, + { + """ + using System.Runtime.InteropServices; + + [ExtendedLayout(ExtendedLayoutKind.CUnion)] + struct S + { + public int {|CS9392:F|}; + } + """, + """ + using System.Runtime.InteropServices; + + [ExtendedLayout(ExtendedLayoutKind.CUnion)] + struct S + { + public unsafe int F; + } + """ + }, + }; + + [Theory] + [MemberData(nameof(FieldLikeMembers))] + public async Task AddsUnsafeToFieldLikeMember(string source, string fixedSource) + { + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + await test.RunAsync(); + } + + [Fact] + public async Task DoesNotOfferFixForPrimaryConstructorBackingField() + { + var source = """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + record struct R([field: FieldOffset(0)] int {|CS9392:X|}); + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest(source); + await test.RunAsync(); + } + + [Fact] + public async Task FixAllAddsUnsafeToEveryField() + { + var source = """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + struct S + { + [FieldOffset(0)] public int {|CS9392:F1|}; + [FieldOffset(4)] public int {|CS9392:F2|}; + } + """; + var fixedSource = """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + struct S + { + [FieldOffset(0)] public unsafe int F1; + [FieldOffset(4)] public unsafe int F2; + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + test.BatchFixedCode = fixedSource; + test.NumberOfIncrementalIterations = 2; + test.NumberOfFixAllIterations = 1; + await test.RunAsync(); + } + } +} +#endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToPointerSignatureCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToPointerSignatureCodeFixTests.cs new file mode 100644 index 00000000000000..262972e702f329 --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToPointerSignatureCodeFixTests.cs @@ -0,0 +1,207 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Threading.Tasks; +using ILLink.CodeFix; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + /// + /// Verifies the IL5006 code fix across pointer-bearing methods, constructors, properties, events, and fields. + /// The expected output checks that unsafe is added at the correct declaration level and modifier position. + /// + public class AddUnsafeToPointerSignatureCodeFixTests + { + public static TheoryData PointerMembers => new() + { + { + """ + class C + { + public int* {|IL5006:M|}(int* value) => value; + } + """, + """ + class C + { + public unsafe int* M(int* value) => value; + } + """ + }, + { + """ + class C + { + public {|IL5006:C|}(delegate* callback) { } + } + """, + """ + class C + { + public unsafe C(delegate* callback) { } + } + """ + }, + { + """ + class C + { + public int* {|IL5006:P|} { get; set; } + } + """, + """ + class C + { + public unsafe int* P { get; set; } + } + """ + }, + { + """ + class C + { + public int* {|IL5006:this|}[int* index] => index; + } + """, + """ + class C + { + public unsafe int* this[int* index] => index; + } + """ + }, + { + """ + class C + { + public delegate* {|IL5006:F|}; + } + """, + """ + class C + { + public unsafe delegate* F; + } + """ + }, + { + """ + class Outer + { + public delegate void D(); + } + + class C + { + public event Outer.D {|IL5006:E|}; + } + """, + """ + class Outer + { + public delegate void D(); + } + + class C + { + public unsafe event Outer.D E; + } + """ + }, + { + """ + class Outer + { + public delegate void D(); + } + + class C + { + public event Outer.D {|IL5006:E|} + { + add { } + remove { } + } + } + """, + """ + class Outer + { + public delegate void D(); + } + + class C + { + public unsafe event Outer.D E + { + add { } + remove { } + } + } + """ + }, + { + """ + class C + { + public static int* operator {|IL5006:+|}(C value, int offset) => null; + } + """, + """ + class C + { + public static unsafe int* operator +(C value, int offset) => null; + } + """ + }, + { + """ + class C + { + public static explicit operator {|IL5006:int*|}(C value) => null; + } + """, + """ + class C + { + public static unsafe explicit operator int*(C value) => null; + } + """ + }, + { + """ + class C + { + void M() + { + static int* {|IL5006:Local|}(int* value) => value; + } + } + """, + """ + class C + { + void M() + { + static unsafe int* Local(int* value) => value; + } + } + """ + }, + }; + + [Theory] + [MemberData(nameof(PointerMembers))] + public async Task AddsUnsafeToPointerSignature(string source, string fixedSource) + { + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + await test.RunAsync(); + } + } +} +#endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs new file mode 100644 index 00000000000000..6daa8a3857138c --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs @@ -0,0 +1,102 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Threading.Tasks; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + /// + /// Verifies IL5006 for pointer and function-pointer signatures across supported member kinds. + /// The suppression cases cover explicit modifiers, safety documentation, partials, constraints, and fixed buffers. + /// + public class PointerSignatureRequiresUnsafeAnalyzerTests + { + [Fact] + public async Task ReportsPointerSignaturesOnAllSupportedMemberKinds() + { + var source = """ + class Outer + { + public delegate void D(); + } + + class C + { + public int* {|IL5006:Method|}(int*[] values) => values[0]; + public {|IL5006:C|}(delegate* callback) { } + public int* {|IL5006:Property|} { get; set; } + public int* {|IL5006:this|}[int* index] => index; + public delegate* {|IL5006:Field|}; + public event Outer.D? {|IL5006:Event|}; + public event Outer.D {|IL5006:ManualEvent|} + { + add { } + remove { } + } + public static int* operator {|IL5006:+|}(C value, int offset) => null; + public static explicit operator {|IL5006:int*|}(C value) => null; + + public void OuterMethod() + { + int* {|IL5006:Local|}(int* value) => value; + _ = Local(null); + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateAnalyzerTest(source); + await test.RunAsync(); + } + + [Fact] + public async Task SafetyDocumentationAndExplicitModifiersSuppressDiagnostics() + { + var source = """ + interface I { } + + class C + { + /// The pointer is never dereferenced. + public void Documented(void* value) { } + + public unsafe void Unsafe(void* value) { } + public safe extern void Extern(void* value); + + public void ConstraintOnly() where T : I { } + public T TypeParameterOnly(T value) => value; + } + + struct S + { + public fixed int Buffer[4]; + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateAnalyzerTest(source); + await test.RunAsync(); + } + + [Fact] + public async Task DocumentationOnOnePartialDeclarationSuppressesBothParts() + { + var source = """ + partial class C + { + /// The returned pointer remains valid for the documented lifetime. + public partial int* M(); + + public partial int* M() => null; + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateAnalyzerTest(source); + await test.RunAsync(); + } + } +} +#endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveInvalidUnsafeCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveInvalidUnsafeCodeFixTests.cs new file mode 100644 index 00000000000000..cf6f19a23ab791 --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveInvalidUnsafeCodeFixTests.cs @@ -0,0 +1,61 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Threading.Tasks; +using ILLink.CodeFix; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + /// + /// Verifies removal of invalid unsafe modifiers reported by CS9377 and CS0106. + /// The cases cover meaningless type-level scopes and declaration kinds where unsafe is syntactically invalid. + /// + public class RemoveInvalidUnsafeCodeFixTests + { + public static TheoryData InvalidDeclarations => new() + { + { + "unsafe class {|CS9377:C|} { }", + "class C { }" + }, + { + "unsafe struct {|CS9377:S|} { }", + "struct S { }" + }, + { + "unsafe interface {|CS9377:I|} { }", + "interface I { }" + }, + { + "unsafe record {|CS9377:R|};", + "record R;" + }, + { + "unsafe delegate void {|CS9377:D|}();", + "delegate void D();" + }, + { + "unsafe enum {|CS0106:E|} { A }", + "enum E { A }" + }, + { + "class C { unsafe const int {|CS0106:F|} = 0; }", + "class C { const int F = 0; }" + }, + }; + + [Theory] + [MemberData(nameof(InvalidDeclarations))] + public async Task RemovesInvalidUnsafeModifier(string source, string fixedSource) + { + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + await test.RunAsync(); + } + } +} +#endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveUndocumentedUnsafeCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveUndocumentedUnsafeCodeFixTests.cs new file mode 100644 index 00000000000000..467aea88a08746 --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveUndocumentedUnsafeCodeFixTests.cs @@ -0,0 +1,222 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Threading.Tasks; +using ILLink.CodeFix; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + /// + /// Verifies the IL5005 fixer removes legacy unsafe scopes without weakening pointer compatibility. + /// Explicit and extended-layout cases confirm that required CS9392 markers become safe instead. + /// + public class RemoveUndocumentedUnsafeCodeFixTests + { + [Fact] + public async Task RemovesUnsafeFromOrdinaryMember() + { + var source = """ + class C + { + public {|IL5005:unsafe|} void M() { } + } + """; + var fixedSource = """ + class C + { + public void M() { } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + await test.RunAsync(); + } + + [Fact] + public async Task RemovesUnsafeFromAccessor() + { + var source = """ + class C + { + public int P + { + {|IL5005:unsafe|} get => 0; + } + } + """; + var fixedSource = """ + class C + { + public int P + { + get => 0; + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + await test.RunAsync(); + } + + [Fact] + public async Task ReplacesUnsafeWithSafeForExplicitLayoutField() + { + var source = """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [FieldOffset(0)] + public {|IL5005:unsafe|} int F; + } + """; + var fixedSource = """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [FieldOffset(0)] + public safe int F; + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + await test.RunAsync(); + } + + [Fact] + public async Task ReplacesUnsafeWithSafeForExplicitLayoutProperty() + { + var source = """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + public {|IL5005:unsafe|} int P { get; set; } + } + """; + var fixedSource = """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + public safe int P { get; set; } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + await test.RunAsync(); + } + + [Fact] + public async Task ReplacesUnsafeWithSafeForExplicitLayoutEvent() + { + var source = """ + using System; + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + public {|IL5005:unsafe|} event Action E; + } + """; + var fixedSource = """ + using System; + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + public safe event Action E; + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + await test.RunAsync(); + } + + [Fact] + public async Task ReplacesUnsafeWithSafeForExtendedLayoutField() + { + var source = """ + using System.Runtime.InteropServices; + + [ExtendedLayout(ExtendedLayoutKind.CUnion)] + struct S + { + public {|IL5005:unsafe|} int F; + } + """; + var fixedSource = """ + using System.Runtime.InteropServices; + + [ExtendedLayout(ExtendedLayoutKind.CUnion)] + struct S + { + public safe int F; + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + await test.RunAsync(); + } + + [Fact] + public async Task FixAllKeepsUnsafeOnPointerSignatures() + { + var source = """ + class C + { + public {|IL5005:unsafe|} void M() { } + public {|IL5005:unsafe|} int* Pointer(int* value) => value; + } + """; + var fixedSource = """ + class C + { + public void M() { } + public {|IL5005:unsafe|} int* Pointer(int* value) => value; + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + test.BatchFixedCode = fixedSource; + test.FixedState.MarkupHandling = Microsoft.CodeAnalysis.Testing.MarkupMode.Allow; + test.BatchFixedState.MarkupHandling = Microsoft.CodeAnalysis.Testing.MarkupMode.Allow; + await test.RunAsync(); + } + } +} +#endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs new file mode 100644 index 00000000000000..454c4be9838f8c --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs @@ -0,0 +1,120 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Threading.Tasks; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + /// + /// Verifies IL5005 coverage across caller-unsafe member kinds and related documentation locations. + /// The cases also confirm that declarations which cannot expose an unsafe contract are ignored. + /// + public class UnsafeMemberMissingSafetyDocumentationAnalyzerTests + { + [Fact] + public async Task ReportsAllUnsafeContractMemberKinds() + { + var source = """ + using System; + + class C + { + public {|IL5005:unsafe|} C(int value) { } + public {|IL5005:unsafe|} void Method() { } + public static {|IL5005:unsafe|} C operator +(C left, C right) => left; + public static {|IL5005:unsafe|} explicit operator int(C value) => 0; + public {|IL5005:unsafe|} int Property { get; set; } + public {|IL5005:unsafe|} int this[int index] => index; + + public int Getter + { + {|IL5005:unsafe|} get => 0; + set { } + } + + public int Initializer + { + get => 0; + {|IL5005:unsafe|} init { } + } + + public int Setter + { + get => 0; + {|IL5005:unsafe|} set { } + } + + public {|IL5005:unsafe|} event Action Event + { + add { } + remove { } + } + + public {|IL5005:unsafe|} event Action? EventField; + public {|IL5005:unsafe|} int Field; + + public void Outer() + { + {|IL5005:unsafe|} void Local() { } + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateAnalyzerTest(source); + await test.RunAsync(); + } + + [Fact] + public async Task SafetyDocumentationSuppressesDiagnosticsAcrossRelatedDeclarations() + { + var source = """ + partial class C + { + /// Callers must satisfy the documented preconditions. + public unsafe partial void Partial(); + + public unsafe partial void Partial() { } + + /// + public unsafe int Property { get; set; } + + /// The getter validates its state. + public int Accessor + { + unsafe get => 0; + } + + public void Outer() + { + /// The local function is only called after validation. + unsafe void Local() { } + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateAnalyzerTest(source); + await test.RunAsync(); + } + + [Fact] + public async Task IgnoresDeclarationsThatCannotExposeUnsafeContracts() + { + var source = """ + class C + { + static unsafe C() { } + unsafe ~C() { } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateAnalyzerTest(source); + await test.RunAsync(); + } + } +} +#endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs new file mode 100644 index 00000000000000..5f9f405f1f941e --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs @@ -0,0 +1,78 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; + +namespace ILLink.RoslynAnalyzer.Tests +{ + /// + /// Creates analyzer and code-fix test projects configured for the updated memory-safety rules. + /// The shared setup enables compiler diagnostics such as CS9377, CS9389, and CS9392. + /// + internal static class UnsafeMigrationTestHelpers + { + internal static CSharpAnalyzerTest CreateAnalyzerTest(string source) + where TAnalyzer : DiagnosticAnalyzer, new() + { + var test = new CSharpAnalyzerTest + { + TestCode = source, + ReferenceAssemblies = new ReferenceAssemblies(string.Empty), + }; + test.TestState.AdditionalReferences.AddRange(SourceGenerators.Tests.LiveReferencePack.GetMetadataReferences()); + test.SolutionTransforms.Add(SetOptions); + return test; + } + + internal static CSharpCodeFixVerifier.Test CreateCodeFixTest( + string source, + string fixedSource) + where TAnalyzer : DiagnosticAnalyzer, new() + where TCodeFix : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider, new() + { + var test = new CSharpCodeFixVerifier.Test + { + TestCode = source, + FixedCode = fixedSource, + }; + test.SolutionTransforms.Add(SetOptions); + return test; + } + + internal static CSharpCodeFixVerifier.Test CreateCodeFixTest( + string source) + where TAnalyzer : DiagnosticAnalyzer, new() + where TCodeFix : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider, new() + { + var test = new CSharpCodeFixVerifier.Test + { + TestCode = source, + }; + test.SolutionTransforms.Add(SetOptions); + return test; + } + + internal static Solution SetOptions(Solution solution, ProjectId projectId) + { + var project = solution.GetProject(projectId)!; + var parseOptions = (CSharpParseOptions)project.ParseOptions!; + parseOptions = parseOptions.WithLanguageVersion(LanguageVersion.Preview) + .WithFeatures([.. parseOptions.Features, new("updated-memory-safety-rules", "")]); + + var compilationOptions = (CSharpCompilationOptions)project.CompilationOptions!; + // CS9377 is emitted at a warning level above the test framework's default. + compilationOptions = compilationOptions + .WithAllowUnsafe(true) + .WithWarningLevel(999); + + return solution.WithProjectParseOptions(projectId, parseOptions) + .WithProjectCompilationOptions(projectId, compilationOptions); + } + } +} +#endif From 9da5c61b7a27993eb32a34a7bc3b3a335a558b62 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 20 Jul 2026 13:18:12 +0200 Subject: [PATCH 2/8] Address unsafe migrator review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9b5b020c-fb1b-459b-a49e-36b77cb62d6c --- .../RemoveInvalidUnsafeCodeFixProvider.cs | 29 ++++++---- ...RemoveUndocumentedUnsafeCodeFixProvider.cs | 3 +- .../UnsafeModifierCodeFixHelpers.cs | 56 ++++++++++--------- .../UnsafeMigrationAnalyzerHelpers.cs | 15 +++++ .../RemoveInvalidUnsafeCodeFixTests.cs | 10 ++++ 5 files changed, 75 insertions(+), 38 deletions(-) diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs index 2d7320ff740e23..6adc44183f1298 100644 --- a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs @@ -4,13 +4,13 @@ #if DEBUG using System.Collections.Immutable; using System.Composition; -using System.Globalization; using System.Threading.Tasks; using ILLink.CodeFixProvider; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; namespace ILLink.CodeFix { @@ -38,15 +38,8 @@ public sealed class RemoveInvalidUnsafeCodeFixProvider : Microsoft.CodeAnalysis. public override async Task RegisterCodeFixesAsync(CodeFixContext context) { Diagnostic diagnostic = context.Diagnostics[0]; - // CS0106 is shared by every invalid modifier, so only handle the unsafe-specific form. - if (diagnostic.Id == InvalidModifierDiagnosticId - && diagnostic.GetMessage(CultureInfo.InvariantCulture) - .IndexOf("'unsafe'", System.StringComparison.Ordinal) < 0) - { - return; - } - - if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root) + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) return; SyntaxNode targetNode = root.FindNode( @@ -58,6 +51,11 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) return; } + // CS0106 is shared by every invalid modifier. Restrict this fixer to declaration shapes where + // Roslyn specifically reports unsafe as invalid, rather than inspecting localized message text. + if (diagnostic.Id == InvalidModifierDiagnosticId && !IsInvalidUnsafeDeclaration(declaration)) + return; + string title = CodeFixTitle.ToString(); context.RegisterCodeFix( CodeAction.Create( @@ -69,6 +67,17 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) title), diagnostic); } + + /// + /// Identifies the well-formed declarations for which Roslyn reports unsafe-specific CS0106. + /// + private static bool IsInvalidUnsafeDeclaration(SyntaxNode declaration) => + declaration switch + { + EnumDeclarationSyntax => true, + FieldDeclarationSyntax { Modifiers: var modifiers } => modifiers.Any(SyntaxKind.ConstKeyword), + _ => false, + }; } } #endif diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs index c33eed5d7a6593..b6311910676510 100644 --- a/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs @@ -49,7 +49,8 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) return; } - if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root) + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) return; SyntaxNode targetNode = root.FindNode( diff --git a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs index f75532e04f38a6..faffaddbf3ecbc 100644 --- a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs +++ b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs @@ -25,12 +25,16 @@ internal static class UnsafeModifierCodeFixHelpers private static readonly SyntaxToken s_safeModifier = ((FieldDeclarationSyntax)SyntaxFactory.ParseMemberDeclaration("safe int field;")!).Modifiers[0]; + /// + /// Registers an add-unsafe action for a supported declaration that has no existing safety modifier. + /// internal static async Task RegisterAddUnsafeCodeFixAsync( CodeFixContext context, LocalizableString codeFixTitle, Func isSupportedDeclaration) { - if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root) + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) return; SyntaxNode targetNode = root.FindNode( @@ -53,6 +57,9 @@ internal static async Task RegisterAddUnsafeCodeFixAsync( context.Diagnostics[0]); } + /// + /// Finds the nearest declaration whose modifier list can contain unsafe-v2 contract markers. + /// internal static SyntaxNode? FindDeclaration(SyntaxNode node) => node.AncestorsAndSelf().FirstOrDefault(static ancestor => ancestor is MemberDeclarationSyntax @@ -62,6 +69,9 @@ or LocalFunctionStatementSyntax internal static bool HasModifier(SyntaxNode declaration, SyntaxKind modifier) => GetModifiers(declaration).Any(modifier); + /// + /// Checks for the contextual safe modifier without depending on a newer SyntaxKind API. + /// internal static bool HasSafeModifier(SyntaxNode declaration) { foreach (SyntaxToken modifier in GetModifiers(declaration)) @@ -73,6 +83,9 @@ internal static bool HasSafeModifier(SyntaxNode declaration) return false; } + /// + /// Adds unsafe while preserving declaration-specific modifiers and trivia. + /// internal static async Task AddUnsafeModifierAsync( Document document, SyntaxNode declaration, @@ -99,6 +112,9 @@ internal static async Task AddUnsafeModifierAsync( return editor.GetChangedDocument(); } + /// + /// Removes unsafe without disturbing modifiers that the current SyntaxGenerator does not model. + /// internal static async Task RemoveUnsafeModifierAsync( Document document, SyntaxNode declaration, @@ -126,6 +142,9 @@ internal static async Task RemoveUnsafeModifierAsync( return editor.GetChangedDocument(); } + /// + /// Replaces unsafe with the contextual safe token required by field-like explicit-layout contracts. + /// internal static async Task ReplaceUnsafeWithSafeModifierAsync( Document document, SyntaxNode declaration, @@ -194,15 +213,9 @@ private static AccessorDeclarationSyntax RemoveUnsafeModifier(AccessorDeclaratio private static SyntaxTokenList AddUnsafeModifier(SyntaxTokenList modifiers) { // Match SyntaxGenerator's canonical order by placing unsafe before extern. - int insertionIndex = modifiers.Count; - for (int i = 0; i < modifiers.Count; i++) - { - if (modifiers[i].IsKind(SyntaxKind.ExternKeyword)) - { - insertionIndex = i; - break; - } - } + int insertionIndex = modifiers.IndexOf(SyntaxKind.ExternKeyword); + if (insertionIndex < 0) + insertionIndex = modifiers.Count; SyntaxToken unsafeModifier = SyntaxFactory.Token(SyntaxKind.UnsafeKeyword) .WithTrailingTrivia(SyntaxFactory.ElasticSpace); @@ -223,6 +236,7 @@ private static SyntaxTokenList RemoveUnsafeModifier(SyntaxTokenList modifiers) SyntaxTriviaList leadingTrivia = modifiers[unsafeIndex].LeadingTrivia; modifiers = modifiers.RemoveAt(unsafeIndex); + // If unsafe owned the declaration's leading trivia, move it to the next modifier. if (unsafeIndex == 0 && modifiers.Count > 0) { modifiers = modifiers.Replace( @@ -235,25 +249,13 @@ private static SyntaxTokenList RemoveUnsafeModifier(SyntaxTokenList modifiers) private static SyntaxToken GetUnsafeModifier(SyntaxNode declaration) { - foreach (SyntaxToken modifier in GetModifiers(declaration)) - { - if (modifier.IsKind(SyntaxKind.UnsafeKeyword)) - return modifier; - } - - return default; + SyntaxTokenList modifiers = GetModifiers(declaration); + int unsafeIndex = GetUnsafeModifierIndex(modifiers); + return unsafeIndex >= 0 ? modifiers[unsafeIndex] : default; } - private static int GetUnsafeModifierIndex(SyntaxTokenList modifiers) - { - for (int i = 0; i < modifiers.Count; i++) - { - if (modifiers[i].IsKind(SyntaxKind.UnsafeKeyword)) - return i; - } - - return -1; - } + private static int GetUnsafeModifierIndex(SyntaxTokenList modifiers) => + modifiers.IndexOf(SyntaxKind.UnsafeKeyword); } } #endif diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs index 285fdd6b5cb528..be82e970b056ab 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs @@ -83,6 +83,9 @@ internal static SyntaxToken GetModifier(SyntaxNode declaration, SyntaxKind modif return default; } + /// + /// Resolves the declaration symbol, using the first declared variable for field-like syntax nodes. + /// internal static ISymbol? GetDeclaredSymbol( SyntaxNode declaration, SemanticModel semanticModel, @@ -95,6 +98,9 @@ internal static SyntaxToken GetModifier(SyntaxNode declaration, SyntaxKind modif _ => semanticModel.GetDeclaredSymbol(declaration, cancellationToken), }; + /// + /// Filters out declarations such as static constructors and destructors that cannot expose caller-unsafe contracts. + /// internal static bool IsUnsafeContractMember(SyntaxNode declaration, ISymbol? symbol) => symbol switch { @@ -112,6 +118,9 @@ FieldDeclarationSyntax field when field.Modifiers.Any(SyntaxKind.ConstKeyword) = _ => false, }; + /// + /// Implements the legacy pointer-signature compatibility test used by both migration analyzers. + /// internal static bool HasPointerOrFunctionPointerSignature(ISymbol symbol) => // Match Roslyn's compatibility rule: only member signature types participate, not constraints. symbol switch @@ -125,6 +134,9 @@ internal static bool HasPointerOrFunctionPointerSignature(ISymbol symbol) => _ => false, }; + /// + /// Finds safety documentation on this declaration, another partial part, or an associated property. + /// internal static bool HasSafetyDocumentation( SyntaxNode declaration, ISymbol? symbol, @@ -140,6 +152,9 @@ internal static bool HasSafetyDocumentation( .Any(HasSafetyDocumentation); } + /// + /// Determines whether removing unsafe must be paired with safe to avoid recreating CS9392. + /// internal static bool RequiresExplicitSafetyModifier(SyntaxNode declaration, ISymbol? symbol) { if (declaration is AccessorDeclarationSyntax || symbol?.ContainingType is not { } containingType) diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveInvalidUnsafeCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveInvalidUnsafeCodeFixTests.cs index cf6f19a23ab791..f38ac36a1b18d2 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveInvalidUnsafeCodeFixTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveInvalidUnsafeCodeFixTests.cs @@ -56,6 +56,16 @@ public async Task RemovesInvalidUnsafeModifier(string source, string fixedSource fixedSource); await test.RunAsync(); } + + [Fact] + public async Task DoesNotRemoveUnsafeForAnotherInvalidModifier() + { + var source = "class C { public unsafe virtual int {|CS0106:F|}; }"; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest(source); + await test.RunAsync(); + } } } #endif From e4876f90c627c7ca4f6b054a2d102ba4e8391278 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 20 Jul 2026 13:31:13 +0200 Subject: [PATCH 3/8] Keep explicit layout fields unsafe by default Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9b5b020c-fb1b-459b-a49e-36b77cb62d6c --- ...RemoveUndocumentedUnsafeCodeFixProvider.cs | 34 +++------- .../illink/src/ILLink.CodeFix/Resources.resx | 3 - .../UnsafeModifierCodeFixHelpers.cs | 29 -------- .../UnsafeMigrationAnalyzerHelpers.cs | 2 +- .../RemoveUndocumentedUnsafeCodeFixTests.cs | 66 +++---------------- 5 files changed, 20 insertions(+), 114 deletions(-) diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs index b6311910676510..f3c63e085397e4 100644 --- a/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs @@ -17,7 +17,7 @@ namespace ILLink.CodeFix { /// /// Fixes IL5005 by removing undocumented legacy unsafe scopes that became caller contracts under unsafe-v2. - /// Pointer signatures retain unsafe, while field-like CS9392 declarations are changed to safe. + /// Pointer signatures and field-like CS9392 declarations retain unsafe for compatibility and safety. /// [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(RemoveUndocumentedUnsafeCodeFixProvider)), Shared] public sealed class RemoveUndocumentedUnsafeCodeFixProvider : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider @@ -28,12 +28,6 @@ public sealed class RemoveUndocumentedUnsafeCodeFixProvider : Microsoft.CodeAnal Resources.ResourceManager, typeof(Resources)); - private static LocalizableString ReplaceCodeFixTitle => - new LocalizableResourceString( - nameof(Resources.ReplaceUndocumentedUnsafeWithSafeCodeFixTitle), - Resources.ResourceManager, - typeof(Resources)); - public override ImmutableArray FixableDiagnosticIds => [DiagnosticId.UnsafeMemberMissingSafetyDocumentation.AsString()]; @@ -42,9 +36,10 @@ public sealed class RemoveUndocumentedUnsafeCodeFixProvider : Microsoft.CodeAnal public override async Task RegisterCodeFixesAsync(CodeFixContext context) { Diagnostic diagnostic = context.Diagnostics[0]; - // These members were already caller-unsafe under the legacy pointer compatibility rules. - if (diagnostic.Properties.ContainsKey( - UnsafeMemberMissingSafetyDocumentationAnalyzer.PointerSignatureProperty)) + // Pointer signatures were already caller-unsafe under legacy rules. Field-like explicit and extended + // layout declarations must also keep a marker for CS9392, and default to unsafe until manually audited. + if (diagnostic.Properties.ContainsKey(UnsafeMemberMissingSafetyDocumentationAnalyzer.PointerSignatureProperty) + || diagnostic.Properties.ContainsKey(UnsafeMemberMissingSafetyDocumentationAnalyzer.RequiresExplicitSafetyModifierProperty)) { return; } @@ -62,24 +57,15 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) return; } - bool replaceWithSafe = diagnostic.Properties.ContainsKey( - UnsafeMemberMissingSafetyDocumentationAnalyzer.RequiresExplicitSafetyModifierProperty) - && !UnsafeModifierCodeFixHelpers.HasSafeModifier(declaration); - // Bare removal would recreate CS9392 for field-backed explicit or extended layout members. - string title = (replaceWithSafe ? ReplaceCodeFixTitle : RemoveCodeFixTitle).ToString(); + string title = RemoveCodeFixTitle.ToString(); context.RegisterCodeFix( CodeAction.Create( title, - cancellationToken => replaceWithSafe - ? UnsafeModifierCodeFixHelpers.ReplaceUnsafeWithSafeModifierAsync( - context.Document, - declaration, - cancellationToken) - : UnsafeModifierCodeFixHelpers.RemoveUnsafeModifierAsync( - context.Document, - declaration, - cancellationToken), + cancellationToken => UnsafeModifierCodeFixHelpers.RemoveUnsafeModifierAsync( + context.Document, + declaration, + cancellationToken), title), diagnostic); } diff --git a/src/tools/illink/src/ILLink.CodeFix/Resources.resx b/src/tools/illink/src/ILLink.CodeFix/Resources.resx index 70bbe4a82b7e7e..99b54822b57a9f 100644 --- a/src/tools/illink/src/ILLink.CodeFix/Resources.resx +++ b/src/tools/illink/src/ILLink.CodeFix/Resources.resx @@ -138,9 +138,6 @@ Remove undocumented 'unsafe' modifier - - Mark field-like member 'safe' - Add 'unsafe' to pointer signature diff --git a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs index faffaddbf3ecbc..eea566aa4bdb05 100644 --- a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs +++ b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs @@ -21,10 +21,6 @@ namespace ILLink.CodeFix /// internal static class UnsafeModifierCodeFixHelpers { - // The referenced Workspaces package parses safe but does not expose SyntaxKind.SafeKeyword yet. - private static readonly SyntaxToken s_safeModifier = - ((FieldDeclarationSyntax)SyntaxFactory.ParseMemberDeclaration("safe int field;")!).Modifiers[0]; - /// /// Registers an add-unsafe action for a supported declaration that has no existing safety modifier. /// @@ -142,24 +138,6 @@ internal static async Task RemoveUnsafeModifierAsync( return editor.GetChangedDocument(); } - /// - /// Replaces unsafe with the contextual safe token required by field-like explicit-layout contracts. - /// - internal static async Task ReplaceUnsafeWithSafeModifierAsync( - Document document, - SyntaxNode declaration, - CancellationToken cancellationToken) - { - SyntaxToken unsafeModifier = GetUnsafeModifier(declaration); - SyntaxToken safeModifier = s_safeModifier - .WithLeadingTrivia(unsafeModifier.LeadingTrivia) - .WithTrailingTrivia(unsafeModifier.TrailingTrivia); - - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - editor.ReplaceNode(declaration, declaration.ReplaceToken(unsafeModifier, safeModifier)); - return editor.GetChangedDocument(); - } - private static SyntaxTokenList GetModifiers(SyntaxNode declaration) => declaration switch { @@ -247,13 +225,6 @@ private static SyntaxTokenList RemoveUnsafeModifier(SyntaxTokenList modifiers) return modifiers; } - private static SyntaxToken GetUnsafeModifier(SyntaxNode declaration) - { - SyntaxTokenList modifiers = GetModifiers(declaration); - int unsafeIndex = GetUnsafeModifierIndex(modifiers); - return unsafeIndex >= 0 ? modifiers[unsafeIndex] : default; - } - private static int GetUnsafeModifierIndex(SyntaxTokenList modifiers) => modifiers.IndexOf(SyntaxKind.UnsafeKeyword); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs index be82e970b056ab..285183ef51a7cf 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs @@ -153,7 +153,7 @@ internal static bool HasSafetyDocumentation( } /// - /// Determines whether removing unsafe must be paired with safe to avoid recreating CS9392. + /// Determines whether removing unsafe must be suppressed to avoid recreating CS9392. /// internal static bool RequiresExplicitSafetyModifier(SyntaxNode declaration, ISymbol? symbol) { diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveUndocumentedUnsafeCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveUndocumentedUnsafeCodeFixTests.cs index 467aea88a08746..da6523059cc265 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveUndocumentedUnsafeCodeFixTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RemoveUndocumentedUnsafeCodeFixTests.cs @@ -10,7 +10,7 @@ namespace ILLink.RoslynAnalyzer.Tests { /// /// Verifies the IL5005 fixer removes legacy unsafe scopes without weakening pointer compatibility. - /// Explicit and extended-layout cases confirm that required CS9392 markers become safe instead. + /// Explicit and extended-layout cases confirm that required CS9392 markers remain unsafe. /// public class RemoveUndocumentedUnsafeCodeFixTests { @@ -67,7 +67,7 @@ public int P } [Fact] - public async Task ReplacesUnsafeWithSafeForExplicitLayoutField() + public async Task KeepsUnsafeForExplicitLayoutField() { var source = """ using System.Runtime.InteropServices; @@ -79,26 +79,14 @@ class C public {|IL5005:unsafe|} int F; } """; - var fixedSource = """ - using System.Runtime.InteropServices; - - [StructLayout(LayoutKind.Explicit)] - class C - { - [FieldOffset(0)] - public safe int F; - } - """; var test = UnsafeMigrationTestHelpers - .CreateCodeFixTest( - source, - fixedSource); + .CreateCodeFixTest(source); await test.RunAsync(); } [Fact] - public async Task ReplacesUnsafeWithSafeForExplicitLayoutProperty() + public async Task KeepsUnsafeForExplicitLayoutProperty() { var source = """ using System.Runtime.InteropServices; @@ -110,26 +98,14 @@ class C public {|IL5005:unsafe|} int P { get; set; } } """; - var fixedSource = """ - using System.Runtime.InteropServices; - - [StructLayout(LayoutKind.Explicit)] - class C - { - [field: FieldOffset(0)] - public safe int P { get; set; } - } - """; var test = UnsafeMigrationTestHelpers - .CreateCodeFixTest( - source, - fixedSource); + .CreateCodeFixTest(source); await test.RunAsync(); } [Fact] - public async Task ReplacesUnsafeWithSafeForExplicitLayoutEvent() + public async Task KeepsUnsafeForExplicitLayoutEvent() { var source = """ using System; @@ -142,27 +118,14 @@ class C public {|IL5005:unsafe|} event Action E; } """; - var fixedSource = """ - using System; - using System.Runtime.InteropServices; - - [StructLayout(LayoutKind.Explicit)] - class C - { - [field: FieldOffset(0)] - public safe event Action E; - } - """; var test = UnsafeMigrationTestHelpers - .CreateCodeFixTest( - source, - fixedSource); + .CreateCodeFixTest(source); await test.RunAsync(); } [Fact] - public async Task ReplacesUnsafeWithSafeForExtendedLayoutField() + public async Task KeepsUnsafeForExtendedLayoutField() { var source = """ using System.Runtime.InteropServices; @@ -173,20 +136,9 @@ struct S public {|IL5005:unsafe|} int F; } """; - var fixedSource = """ - using System.Runtime.InteropServices; - - [ExtendedLayout(ExtendedLayoutKind.CUnion)] - struct S - { - public safe int F; - } - """; var test = UnsafeMigrationTestHelpers - .CreateCodeFixTest( - source, - fixedSource); + .CreateCodeFixTest(source); await test.RunAsync(); } From 70612f7cc1f45c97ec7d54da7bdc1d7ed3e2efcb Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 20 Jul 2026 13:54:34 +0200 Subject: [PATCH 4/8] Disable unsafe migration analyzers by default Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9b5b020c-fb1b-459b-a49e-36b77cb62d6c --- .../PointerSignatureRequiresUnsafeAnalyzer.cs | 6 ++++-- .../UnsafeMemberMissingSafetyDocumentationAnalyzer.cs | 6 ++++-- .../PointerSignatureRequiresUnsafeAnalyzerTests.cs | 10 +++++++++- ...afeMemberMissingSafetyDocumentationAnalyzerTests.cs | 10 +++++++++- .../UnsafeMigrationTestHelpers.cs | 8 +++++++- 5 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs index 87e59c8c3f071c..6dd4b15a10eb9e 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs @@ -12,13 +12,15 @@ namespace ILLink.RoslynAnalyzer { /// /// Reports IL5006 for pointer or function-pointer signatures that lost their legacy caller-unsafe contract. - /// An existing unsafe/safe modifier or a <safety> XML comment suppresses the diagnostic. + /// The diagnostic is disabled by default while this migration tooling remains experimental. /// [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class PointerSignatureRequiresUnsafeAnalyzer : DiagnosticAnalyzer { private static readonly DiagnosticDescriptor s_rule = - DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.PointerSignatureRequiresUnsafe); + DiagnosticDescriptors.GetDiagnosticDescriptor( + DiagnosticId.PointerSignatureRequiresUnsafe, + isEnabledByDefault: false); public override ImmutableArray SupportedDiagnostics => [s_rule]; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs index 111a857e3afa34..9ce5a80411d5f2 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs @@ -12,7 +12,7 @@ namespace ILLink.RoslynAnalyzer { /// /// Reports IL5005 when an explicit unsafe member contract has no <safety> XML documentation. - /// Diagnostic properties guide the fixer when pointer compatibility or CS9392 requires preserving a modifier. + /// The diagnostic is disabled by default while this migration tooling remains experimental. /// [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class UnsafeMemberMissingSafetyDocumentationAnalyzer : DiagnosticAnalyzer @@ -21,7 +21,9 @@ public sealed class UnsafeMemberMissingSafetyDocumentationAnalyzer : DiagnosticA public const string RequiresExplicitSafetyModifierProperty = nameof(RequiresExplicitSafetyModifierProperty); private static readonly DiagnosticDescriptor s_rule = - DiagnosticDescriptors.GetDiagnosticDescriptor(DiagnosticId.UnsafeMemberMissingSafetyDocumentation); + DiagnosticDescriptors.GetDiagnosticDescriptor( + DiagnosticId.UnsafeMemberMissingSafetyDocumentation, + isEnabledByDefault: false); public override ImmutableArray SupportedDiagnostics => [s_rule]; diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs index 6daa8a3857138c..65cf1d320a8f47 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs @@ -13,6 +13,14 @@ namespace ILLink.RoslynAnalyzer.Tests /// public class PointerSignatureRequiresUnsafeAnalyzerTests { + [Fact] + public void IsDisabledByDefault() + { + var analyzer = new PointerSignatureRequiresUnsafeAnalyzer(); + + Assert.False(analyzer.SupportedDiagnostics[0].IsEnabledByDefault); + } + [Fact] public async Task ReportsPointerSignaturesOnAllSupportedMemberKinds() { @@ -29,7 +37,7 @@ class C public int* {|IL5006:Property|} { get; set; } public int* {|IL5006:this|}[int* index] => index; public delegate* {|IL5006:Field|}; - public event Outer.D? {|IL5006:Event|}; + public event Outer.D {|IL5006:Event|}; public event Outer.D {|IL5006:ManualEvent|} { add { } diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs index 454c4be9838f8c..6627ae972341f7 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs @@ -13,6 +13,14 @@ namespace ILLink.RoslynAnalyzer.Tests /// public class UnsafeMemberMissingSafetyDocumentationAnalyzerTests { + [Fact] + public void IsDisabledByDefault() + { + var analyzer = new UnsafeMemberMissingSafetyDocumentationAnalyzer(); + + Assert.False(analyzer.SupportedDiagnostics[0].IsEnabledByDefault); + } + [Fact] public async Task ReportsAllUnsafeContractMemberKinds() { @@ -52,7 +60,7 @@ public int Setter remove { } } - public {|IL5005:unsafe|} event Action? EventField; + public {|IL5005:unsafe|} event Action EventField; public {|IL5005:unsafe|} int Field; public void Outer() diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs index 5f9f405f1f941e..63ac6c37f9fca7 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. #if DEBUG +using ILLink.Shared; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Testing; @@ -66,9 +67,14 @@ internal static Solution SetOptions(Solution solution, ProjectId projectId) var compilationOptions = (CSharpCompilationOptions)project.CompilationOptions!; // CS9377 is emitted at a warning level above the test framework's default. + var diagnosticOptions = compilationOptions.SpecificDiagnosticOptions + .SetItems(CSharpVerifierHelper.NullableWarnings) + .SetItem(DiagnosticId.UnsafeMemberMissingSafetyDocumentation.AsString(), ReportDiagnostic.Warn) + .SetItem(DiagnosticId.PointerSignatureRequiresUnsafe.AsString(), ReportDiagnostic.Warn); compilationOptions = compilationOptions .WithAllowUnsafe(true) - .WithWarningLevel(999); + .WithWarningLevel(999) + .WithSpecificDiagnosticOptions(diagnosticOptions); return solution.WithProjectParseOptions(projectId, parseOptions) .WithProjectCompilationOptions(projectId, compilationOptions); From 7ab234869465bfb78aac01522f20eea0161cbae8 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Tue, 21 Jul 2026 22:47:36 +0200 Subject: [PATCH 5/8] Address unsafe migration review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e39a2bb4-8a72-40d6-8a1c-d98b6c346afe --- .../RemoveInvalidUnsafeCodeFixProvider.cs | 3 +- ...RemoveUndocumentedUnsafeCodeFixProvider.cs | 2 +- .../UnsafeModifierCodeFixHelpers.cs | 79 ++++++++-------- .../ILLink.RoslynAnalyzer.csproj | 4 + .../PointerSignatureRequiresUnsafeAnalyzer.cs | 64 ++++++++----- ...emberMissingSafetyDocumentationAnalyzer.cs | 79 ++++++++++------ .../UnsafeMigrationAnalyzerHelpers.cs | 89 ++++++------------- .../AddUnsafeToExternCodeFixTests.cs | 30 +++++++ .../AddUnsafeToFieldCodeFixTests.cs | 46 ++++++++++ ...AddUnsafeToPointerSignatureCodeFixTests.cs | 34 ++++++- ...terSignatureRequiresUnsafeAnalyzerTests.cs | 4 +- ...MissingSafetyDocumentationAnalyzerTests.cs | 2 +- 12 files changed, 273 insertions(+), 163 deletions(-) diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs index 6adc44183f1298..70ffd7b625512b 100644 --- a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs @@ -6,6 +6,7 @@ using System.Composition; using System.Threading.Tasks; using ILLink.CodeFixProvider; +using ILLink.RoslynAnalyzer; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; @@ -46,7 +47,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); if (UnsafeModifierCodeFixHelpers.FindDeclaration(targetNode) is not { } declaration - || !UnsafeModifierCodeFixHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) + || !UnsafeMigrationAnalyzerHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) { return; } diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs index f3c63e085397e4..4465b4ddeb1f54 100644 --- a/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs @@ -52,7 +52,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); if (UnsafeModifierCodeFixHelpers.FindDeclaration(targetNode) is not { } declaration - || !UnsafeModifierCodeFixHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) + || !UnsafeMigrationAnalyzerHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) { return; } diff --git a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs index eea566aa4bdb05..a9b78d2c6be60c 100644 --- a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs +++ b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using ILLink.RoslynAnalyzer; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; @@ -16,7 +17,7 @@ namespace ILLink.CodeFix { /// - /// Centralizes modifier discovery and trivia-preserving edits for the unsafe-v2 code-fix providers. + /// Centralizes declaration discovery and trivia-preserving modifier edits for the unsafe-v2 code-fix providers. /// It is shared by fixes for CS9389, CS9377/CS0106, IL5005, IL5006, and CS9392. /// internal static class UnsafeModifierCodeFixHelpers @@ -38,8 +39,8 @@ internal static async Task RegisterAddUnsafeCodeFixAsync( getInnermostNodeForTie: true); if (FindDeclaration(targetNode) is not { } declaration || !isSupportedDeclaration(declaration) - || HasModifier(declaration, SyntaxKind.UnsafeKeyword) - || HasSafeModifier(declaration)) + || UnsafeMigrationAnalyzerHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword) + || UnsafeMigrationAnalyzerHelpers.HasSafeModifier(declaration)) { return; } @@ -58,27 +59,14 @@ internal static async Task RegisterAddUnsafeCodeFixAsync( /// internal static SyntaxNode? FindDeclaration(SyntaxNode node) => node.AncestorsAndSelf().FirstOrDefault(static ancestor => - ancestor is MemberDeclarationSyntax + ancestor is BaseTypeDeclarationSyntax + or DelegateDeclarationSyntax + or BaseMethodDeclarationSyntax + or BasePropertyDeclarationSyntax + or BaseFieldDeclarationSyntax or LocalFunctionStatementSyntax or AccessorDeclarationSyntax); - internal static bool HasModifier(SyntaxNode declaration, SyntaxKind modifier) => - GetModifiers(declaration).Any(modifier); - - /// - /// Checks for the contextual safe modifier without depending on a newer SyntaxKind API. - /// - internal static bool HasSafeModifier(SyntaxNode declaration) - { - foreach (SyntaxToken modifier in GetModifiers(declaration)) - { - if (modifier.ValueText == "safe") - return true; - } - - return false; - } - /// /// Adds unsafe while preserving declaration-specific modifiers and trivia. /// @@ -88,21 +76,21 @@ internal static async Task AddUnsafeModifierAsync( CancellationToken cancellationToken) { var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + SyntaxTokenList modifiers = UnsafeMigrationAnalyzerHelpers.GetModifiers(declaration); if (declaration is AccessorDeclarationSyntax accessor) { editor.ReplaceNode(accessor, AddUnsafeModifier(accessor)); } - else if (declaration is DestructorDeclarationSyntax destructor) + else if (modifiers.Count > 0) { - // SyntaxGenerator does not preserve extern on destructors in the current Workspaces dependency. editor.ReplaceNode( - destructor, - destructor.WithModifiers(AddUnsafeModifier(destructor.Modifiers))); + declaration, + WithModifiers(declaration, AddUnsafeModifier(modifiers))); } else { - var modifiers = editor.Generator.GetModifiers(declaration); - editor.SetModifiers(declaration, modifiers.WithIsUnsafe(true)); + DeclarationModifiers declarationModifiers = editor.Generator.GetModifiers(declaration); + editor.SetModifiers(declaration, declarationModifiers.WithIsUnsafe(true)); } return editor.GetChangedDocument(); @@ -117,36 +105,26 @@ internal static async Task RemoveUnsafeModifierAsync( CancellationToken cancellationToken) { var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + SyntaxTokenList modifiers = UnsafeMigrationAnalyzerHelpers.GetModifiers(declaration); if (declaration is AccessorDeclarationSyntax accessor) { editor.ReplaceNode(accessor, RemoveUnsafeModifier(accessor)); } - else if (declaration is DestructorDeclarationSyntax destructor - && destructor.Modifiers.Any(SyntaxKind.ExternKeyword)) + else if (modifiers.Count > 1) { - // Remove only unsafe; routing this through SyntaxGenerator would also remove extern. editor.ReplaceNode( - destructor, - destructor.WithModifiers(RemoveUnsafeModifier(destructor.Modifiers))); + declaration, + WithModifiers(declaration, RemoveUnsafeModifier(modifiers))); } else { - var modifiers = editor.Generator.GetModifiers(declaration); - editor.SetModifiers(declaration, modifiers.WithIsUnsafe(false)); + DeclarationModifiers declarationModifiers = editor.Generator.GetModifiers(declaration); + editor.SetModifiers(declaration, declarationModifiers.WithIsUnsafe(false)); } return editor.GetChangedDocument(); } - private static SyntaxTokenList GetModifiers(SyntaxNode declaration) => - declaration switch - { - MemberDeclarationSyntax member => member.Modifiers, - LocalFunctionStatementSyntax localFunction => localFunction.Modifiers, - AccessorDeclarationSyntax accessor => accessor.Modifiers, - _ => default, - }; - private static AccessorDeclarationSyntax AddUnsafeModifier(AccessorDeclarationSyntax accessor) { // SyntaxGenerator does not yet model unsafe property accessors, so edit their tokens directly. @@ -190,7 +168,7 @@ private static AccessorDeclarationSyntax RemoveUnsafeModifier(AccessorDeclaratio private static SyntaxTokenList AddUnsafeModifier(SyntaxTokenList modifiers) { - // Match SyntaxGenerator's canonical order by placing unsafe before extern. + // Place unsafe before extern while preserving the existing modifier order. int insertionIndex = modifiers.IndexOf(SyntaxKind.ExternKeyword); if (insertionIndex < 0) insertionIndex = modifiers.Count; @@ -225,6 +203,19 @@ private static SyntaxTokenList RemoveUnsafeModifier(SyntaxTokenList modifiers) return modifiers; } + private static SyntaxNode WithModifiers(SyntaxNode declaration, SyntaxTokenList modifiers) => + declaration switch + { + BaseTypeDeclarationSyntax type => type.WithModifiers(modifiers), + DelegateDeclarationSyntax @delegate => @delegate.WithModifiers(modifiers), + BaseMethodDeclarationSyntax method => method.WithModifiers(modifiers), + BasePropertyDeclarationSyntax property => property.WithModifiers(modifiers), + BaseFieldDeclarationSyntax field => field.WithModifiers(modifiers), + LocalFunctionStatementSyntax localFunction => localFunction.WithModifiers(modifiers), + AccessorDeclarationSyntax accessor => accessor.WithModifiers(modifiers), + _ => declaration, + }; + private static int GetUnsafeModifierIndex(SyntaxTokenList modifiers) => modifiers.IndexOf(SyntaxKind.UnsafeKeyword); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj b/src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj index fa69abc3faa189..302f64270b43ad 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj @@ -50,6 +50,10 @@ + + + + diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs index 6dd4b15a10eb9e..c1536a581c4c3a 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs @@ -2,11 +2,14 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. #if DEBUG +using System; using System.Collections.Immutable; +using System.Threading; using ILLink.Shared; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; namespace ILLink.RoslynAnalyzer { @@ -31,36 +34,57 @@ public override void Initialize(AnalysisContext context) if (!System.Diagnostics.Debugger.IsAttached) context.EnableConcurrentExecution(); - context.RegisterSyntaxNodeAction( - AnalyzeDeclaration, - UnsafeMigrationAnalyzerHelpers.PointerSignatureDeclarationKinds); + context.RegisterSymbolAction( + AnalyzeSymbol, + SymbolKind.Method, + SymbolKind.Property, + SymbolKind.Field, + SymbolKind.Event); + context.RegisterOperationAction(AnalyzeLocalFunction, OperationKind.LocalFunction); } - private static void AnalyzeDeclaration(SyntaxNodeAnalysisContext context) + private static void AnalyzeSymbol(SymbolAnalysisContext context) { - SyntaxNode declaration = context.Node; - // safe is an explicit audited contract on the limited declaration kinds where Roslyn permits it. - if (UnsafeMigrationAnalyzerHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword) - || UnsafeMigrationAnalyzerHelpers.HasSafeModifier(declaration)) - { + // Roslyn does not invoke symbol actions for local functions. + if (context.Symbol is IMethodSymbol { MethodKind: MethodKind.LocalFunction }) return; - } - ISymbol? symbol = UnsafeMigrationAnalyzerHelpers.GetDeclaredSymbol( - declaration, - context.SemanticModel, - context.CancellationToken); + AnalyzeSymbol(context.Symbol, context.CancellationToken, context.ReportDiagnostic); + } + + private static void AnalyzeLocalFunction(OperationAnalysisContext context) => + AnalyzeSymbol( + ((ILocalFunctionOperation)context.Operation).Symbol, + context.CancellationToken, + context.ReportDiagnostic); - if (symbol is null - || !UnsafeMigrationAnalyzerHelpers.HasPointerOrFunctionPointerSignature(symbol) - || UnsafeMigrationAnalyzerHelpers.HasSafetyDocumentation(declaration, symbol, context.CancellationToken)) + private static void AnalyzeSymbol( + ISymbol symbol, + CancellationToken cancellationToken, + Action reportDiagnostic) + { + // Property and event accessors are represented by method symbols, but their containing declaration + // already carries the pointer-signature contract. + if (symbol is IMethodSymbol { AssociatedSymbol: not null } + || !UnsafeMigrationAnalyzerHelpers.HasPointerOrFunctionPointerSignature(symbol)) { return; } - context.ReportDiagnostic(Diagnostic.Create( - s_rule, - UnsafeMigrationAnalyzerHelpers.GetIdentifierLocation(declaration))); + foreach (SyntaxNode declaration in UnsafeMigrationAnalyzerHelpers.GetDeclarations(symbol, cancellationToken)) + { + // safe is an explicit audited contract on the limited declaration kinds where Roslyn permits it. + if (UnsafeMigrationAnalyzerHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword) + || UnsafeMigrationAnalyzerHelpers.HasSafeModifier(declaration) + || UnsafeMigrationAnalyzerHelpers.HasSafetyDocumentation(declaration, symbol, cancellationToken)) + { + continue; + } + + reportDiagnostic(Diagnostic.Create( + s_rule, + UnsafeMigrationAnalyzerHelpers.GetIdentifierLocation(declaration))); + } } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs index 9ce5a80411d5f2..4026b0892ede0d 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs @@ -2,11 +2,14 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. #if DEBUG +using System; using System.Collections.Immutable; +using System.Threading; using ILLink.Shared; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; namespace ILLink.RoslynAnalyzer { @@ -34,44 +37,62 @@ public override void Initialize(AnalysisContext context) if (!System.Diagnostics.Debugger.IsAttached) context.EnableConcurrentExecution(); - context.RegisterSyntaxNodeAction( - AnalyzeDeclaration, - UnsafeMigrationAnalyzerHelpers.UnsafeContractDeclarationKinds); + context.RegisterSymbolAction( + AnalyzeSymbol, + SymbolKind.Method, + SymbolKind.Property, + SymbolKind.Field, + SymbolKind.Event); + context.RegisterOperationAction(AnalyzeLocalFunction, OperationKind.LocalFunction); } - private static void AnalyzeDeclaration(SyntaxNodeAnalysisContext context) + private static void AnalyzeSymbol(SymbolAnalysisContext context) { - SyntaxNode declaration = context.Node; - SyntaxToken unsafeModifier = UnsafeMigrationAnalyzerHelpers.GetModifier( - declaration, - SyntaxKind.UnsafeKeyword); - - if (unsafeModifier == default) + // Roslyn does not invoke symbol actions for local functions. + if (context.Symbol is IMethodSymbol { MethodKind: MethodKind.LocalFunction }) return; - ISymbol? symbol = UnsafeMigrationAnalyzerHelpers.GetDeclaredSymbol( - declaration, - context.SemanticModel, - context.CancellationToken); + AnalyzeSymbol(context.Symbol, context.CancellationToken, context.ReportDiagnostic); + } - if (!UnsafeMigrationAnalyzerHelpers.IsUnsafeContractMember(declaration, symbol) - || UnsafeMigrationAnalyzerHelpers.HasSafetyDocumentation(declaration, symbol, context.CancellationToken)) - { + private static void AnalyzeLocalFunction(OperationAnalysisContext context) => + AnalyzeSymbol( + ((ILocalFunctionOperation)context.Operation).Symbol, + context.CancellationToken, + context.ReportDiagnostic); + + private static void AnalyzeSymbol( + ISymbol symbol, + CancellationToken cancellationToken, + Action reportDiagnostic) + { + if (!UnsafeMigrationAnalyzerHelpers.IsUnsafeContractMember(symbol)) return; - } - var properties = ImmutableDictionary.Empty; - // Pointer signatures were caller-unsafe under the legacy rules, so the fixer must retain unsafe. - if (symbol is not null && UnsafeMigrationAnalyzerHelpers.HasPointerOrFunctionPointerSignature(symbol)) - properties = properties.Add(PointerSignatureProperty, bool.TrueString); - // Explicit and extended layouts still require an explicit safe/unsafe marker after migration. - if (UnsafeMigrationAnalyzerHelpers.RequiresExplicitSafetyModifier(declaration, symbol)) - properties = properties.Add(RequiresExplicitSafetyModifierProperty, bool.TrueString); + foreach (SyntaxNode declaration in UnsafeMigrationAnalyzerHelpers.GetDeclarations(symbol, cancellationToken)) + { + SyntaxToken unsafeModifier = UnsafeMigrationAnalyzerHelpers.GetModifier( + declaration, + SyntaxKind.UnsafeKeyword); + if (unsafeModifier == default + || UnsafeMigrationAnalyzerHelpers.HasSafetyDocumentation(declaration, symbol, cancellationToken)) + { + continue; + } + + var properties = ImmutableDictionary.Empty; + // Pointer signatures were caller-unsafe under the legacy rules, so the fixer must retain unsafe. + if (UnsafeMigrationAnalyzerHelpers.HasPointerOrFunctionPointerSignature(symbol)) + properties = properties.Add(PointerSignatureProperty, bool.TrueString); + // Explicit and extended layouts still require an explicit safe/unsafe marker after migration. + if (UnsafeMigrationAnalyzerHelpers.RequiresExplicitSafetyModifier(declaration, symbol)) + properties = properties.Add(RequiresExplicitSafetyModifierProperty, bool.TrueString); - context.ReportDiagnostic(Diagnostic.Create( - s_rule, - unsafeModifier.GetLocation(), - properties: properties)); + reportDiagnostic(Diagnostic.Create( + s_rule, + unsafeModifier.GetLocation(), + properties: properties)); + } } } } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs index 285183ef51a7cf..699ebbaa9b1c93 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs @@ -18,36 +18,8 @@ namespace ILLink.RoslynAnalyzer /// internal static class UnsafeMigrationAnalyzerHelpers { - internal static readonly SyntaxKind[] UnsafeContractDeclarationKinds = - [ - SyntaxKind.MethodDeclaration, - SyntaxKind.ConstructorDeclaration, - SyntaxKind.OperatorDeclaration, - SyntaxKind.ConversionOperatorDeclaration, - SyntaxKind.LocalFunctionStatement, - SyntaxKind.PropertyDeclaration, - SyntaxKind.IndexerDeclaration, - SyntaxKind.GetAccessorDeclaration, - SyntaxKind.SetAccessorDeclaration, - SyntaxKind.InitAccessorDeclaration, - SyntaxKind.EventDeclaration, - SyntaxKind.EventFieldDeclaration, - SyntaxKind.FieldDeclaration, - ]; - - internal static readonly SyntaxKind[] PointerSignatureDeclarationKinds = - [ - SyntaxKind.MethodDeclaration, - SyntaxKind.ConstructorDeclaration, - SyntaxKind.OperatorDeclaration, - SyntaxKind.ConversionOperatorDeclaration, - SyntaxKind.LocalFunctionStatement, - SyntaxKind.PropertyDeclaration, - SyntaxKind.IndexerDeclaration, - SyntaxKind.EventDeclaration, - SyntaxKind.EventFieldDeclaration, - SyntaxKind.FieldDeclaration, - ]; + // The analyzer builds against a Roslyn version that predates SyntaxKind.SafeKeyword. + private static readonly SyntaxKind s_safeKeyword = SyntaxFacts.GetContextualKeywordKind("safe"); internal static SyntaxTokenList GetModifiers(SyntaxNode declaration) => declaration switch @@ -61,16 +33,8 @@ internal static SyntaxTokenList GetModifiers(SyntaxNode declaration) => internal static bool HasModifier(SyntaxNode declaration, SyntaxKind modifier) => GetModifiers(declaration).Any(modifier); - internal static bool HasSafeModifier(SyntaxNode declaration) - { - foreach (SyntaxToken modifier in GetModifiers(declaration)) - { - if (modifier.ValueText == "safe") - return true; - } - - return false; - } + internal static bool HasSafeModifier(SyntaxNode declaration) => + s_safeKeyword != SyntaxKind.None && GetModifiers(declaration).Any(s_safeKeyword); internal static SyntaxToken GetModifier(SyntaxNode declaration, SyntaxKind modifier) { @@ -84,37 +48,36 @@ internal static SyntaxToken GetModifier(SyntaxNode declaration, SyntaxKind modif } /// - /// Resolves the declaration symbol, using the first declared variable for field-like syntax nodes. + /// Gets source declarations for a symbol, normalizing field-like variable declarators to their shared declaration. /// - internal static ISymbol? GetDeclaredSymbol( - SyntaxNode declaration, - SemanticModel semanticModel, - CancellationToken cancellationToken) => - declaration switch + internal static IEnumerable GetDeclarations(ISymbol symbol, CancellationToken cancellationToken) + { + foreach (SyntaxReference reference in symbol.DeclaringSyntaxReferences) { - BaseFieldDeclarationSyntax field => field.Declaration.Variables - .Select(variable => semanticModel.GetDeclaredSymbol(variable, cancellationToken)) - .FirstOrDefault(symbol => symbol is not null), - _ => semanticModel.GetDeclaredSymbol(declaration, cancellationToken), - }; + SyntaxNode declaration = reference.GetSyntax(cancellationToken); + if (declaration is VariableDeclaratorSyntax variable + && variable.Parent?.Parent is BaseFieldDeclarationSyntax field) + { + if (field.Declaration.Variables[0] != variable) + continue; + + declaration = field; + } + + yield return declaration; + } + } /// /// Filters out declarations such as static constructors and destructors that cannot expose caller-unsafe contracts. /// - internal static bool IsUnsafeContractMember(SyntaxNode declaration, ISymbol? symbol) => + internal static bool IsUnsafeContractMember(ISymbol symbol) => symbol switch { IMethodSymbol { MethodKind: MethodKind.StaticConstructor or MethodKind.Destructor } => false, IMethodSymbol => true, IFieldSymbol { IsConst: false } => true, IPropertySymbol or IEventSymbol => true, - null => declaration switch - { - ConstructorDeclarationSyntax constructor when constructor.Modifiers.Any(SyntaxKind.StaticKeyword) => false, - DestructorDeclarationSyntax => false, - FieldDeclarationSyntax field when field.Modifiers.Any(SyntaxKind.ConstKeyword) => false, - _ => true, - }, _ => false, }; @@ -139,14 +102,14 @@ internal static bool HasPointerOrFunctionPointerSignature(ISymbol symbol) => /// internal static bool HasSafetyDocumentation( SyntaxNode declaration, - ISymbol? symbol, + ISymbol symbol, CancellationToken cancellationToken) { if (HasSafetyDocumentation(declaration)) return true; // Documentation on another partial declaration or on an associated property also describes this contract. - return symbol is not null && GetDocumentationSymbols(symbol) + return GetDocumentationSymbols(symbol) .SelectMany(static relatedSymbol => relatedSymbol.DeclaringSyntaxReferences) .Select(reference => reference.GetSyntax(cancellationToken)) .Any(HasSafetyDocumentation); @@ -155,9 +118,9 @@ internal static bool HasSafetyDocumentation( /// /// Determines whether removing unsafe must be suppressed to avoid recreating CS9392. /// - internal static bool RequiresExplicitSafetyModifier(SyntaxNode declaration, ISymbol? symbol) + internal static bool RequiresExplicitSafetyModifier(SyntaxNode declaration, ISymbol symbol) { - if (declaration is AccessorDeclarationSyntax || symbol?.ContainingType is not { } containingType) + if (declaration is AccessorDeclarationSyntax || symbol.ContainingType is not { } containingType) return false; // Removing unsafe from a field-backed declaration in these layouts would immediately reintroduce CS9392. diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToExternCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToExternCodeFixTests.cs index 967b0f28a85a11..a2986560933fc8 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToExternCodeFixTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToExternCodeFixTests.cs @@ -152,6 +152,36 @@ void M() } """ }, + { + """ + class C + { + required public {|CS9389:extern|} int P { get; set; } + } + """, + """ + class C + { + required public unsafe extern int P { get; set; } + } + """ + }, + { + """ + class C + { + /// Invokes native code. + {|CS9389:extern|} void M(); + } + """, + """ + class C + { + /// Invokes native code. + unsafe extern void M(); + } + """ + }, }; [Theory] diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToFieldCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToFieldCodeFixTests.cs index 11371258256f85..f6a6d73fec3335 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToFieldCodeFixTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToFieldCodeFixTests.cs @@ -126,6 +126,52 @@ struct S } """ }, + { + """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + required public int {|CS9392:P|} { get; set; } + } + """, + """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + [field: FieldOffset(0)] + required public unsafe int P { get; set; } + } + """ + }, + { + """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + /// Stores the native value. + [FieldOffset(0)] + int {|CS9392:F|}; + } + """, + """ + using System.Runtime.InteropServices; + + [StructLayout(LayoutKind.Explicit)] + class C + { + /// Stores the native value. + [FieldOffset(0)] + unsafe int F; + } + """ + }, }; [Theory] diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToPointerSignatureCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToPointerSignatureCodeFixTests.cs index 262972e702f329..e9eafe62a6b572 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToPointerSignatureCodeFixTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeToPointerSignatureCodeFixTests.cs @@ -76,13 +76,13 @@ class C """ class C { - public delegate* {|IL5006:F|}; + public delegate* {|IL5006:F1|}, F2; } """, """ class C { - public unsafe delegate* F; + public unsafe delegate* F1, F2; } """ }, @@ -190,6 +190,36 @@ void M() } """ }, + { + """ + class C + { + required public int* {|IL5006:P|} { get; set; } + } + """, + """ + class C + { + required public unsafe int* P { get; set; } + } + """ + }, + { + """ + class C + { + /// Returns the supplied pointer. + int* {|IL5006:M|}(int* value) => value; + } + """, + """ + class C + { + /// Returns the supplied pointer. + unsafe int* M(int* value) => value; + } + """ + }, }; [Theory] diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs index 65cf1d320a8f47..02b82ce77a9c0f 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/PointerSignatureRequiresUnsafeAnalyzerTests.cs @@ -36,8 +36,8 @@ class C public {|IL5006:C|}(delegate* callback) { } public int* {|IL5006:Property|} { get; set; } public int* {|IL5006:this|}[int* index] => index; - public delegate* {|IL5006:Field|}; - public event Outer.D {|IL5006:Event|}; + public delegate* {|IL5006:Field1|}, Field2; + public event Outer.D {|IL5006:Event1|}, Event2; public event Outer.D {|IL5006:ManualEvent|} { add { } diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs index 6627ae972341f7..35559d26bab9e7 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs @@ -61,7 +61,7 @@ public int Setter } public {|IL5005:unsafe|} event Action EventField; - public {|IL5005:unsafe|} int Field; + public {|IL5005:unsafe|} int Field1, Field2; public void Outer() { From d9b33a59787123c48a795a2ba1a4c9d8dd565f03 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Tue, 21 Jul 2026 23:13:40 +0200 Subject: [PATCH 6/8] Share unsafe migration syntax helpers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e39a2bb4-8a72-40d6-8a1c-d98b6c346afe --- .../ILLink.CodeFixProvider.csproj | 4 ++ .../RemoveInvalidUnsafeCodeFixProvider.cs | 2 +- ...RemoveUndocumentedUnsafeCodeFixProvider.cs | 2 +- .../UnsafeModifierCodeFixHelpers.cs | 8 ++-- .../ILLink.RoslynAnalyzer.csproj | 4 -- .../PointerSignatureRequiresUnsafeAnalyzer.cs | 4 +- ...emberMissingSafetyDocumentationAnalyzer.cs | 2 +- .../UnsafeMigrationAnalyzerHelpers.cs | 30 ------------ .../UnsafeMigrationSyntaxHelpers.cs | 47 +++++++++++++++++++ 9 files changed, 60 insertions(+), 43 deletions(-) create mode 100644 src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs diff --git a/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj b/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj index 5dd13cb7772158..441acde0ff83a8 100644 --- a/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj +++ b/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj @@ -13,6 +13,10 @@ + + + + diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs index 70ffd7b625512b..c68fa9b60dd4d1 100644 --- a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs @@ -47,7 +47,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); if (UnsafeModifierCodeFixHelpers.FindDeclaration(targetNode) is not { } declaration - || !UnsafeMigrationAnalyzerHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) + || !UnsafeMigrationSyntaxHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) { return; } diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs index 4465b4ddeb1f54..9d48cd16842362 100644 --- a/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveUndocumentedUnsafeCodeFixProvider.cs @@ -52,7 +52,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); if (UnsafeModifierCodeFixHelpers.FindDeclaration(targetNode) is not { } declaration - || !UnsafeMigrationAnalyzerHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) + || !UnsafeMigrationSyntaxHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) { return; } diff --git a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs index a9b78d2c6be60c..2813ff19d1402d 100644 --- a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs +++ b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs @@ -39,8 +39,8 @@ internal static async Task RegisterAddUnsafeCodeFixAsync( getInnermostNodeForTie: true); if (FindDeclaration(targetNode) is not { } declaration || !isSupportedDeclaration(declaration) - || UnsafeMigrationAnalyzerHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword) - || UnsafeMigrationAnalyzerHelpers.HasSafeModifier(declaration)) + || UnsafeMigrationSyntaxHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword) + || UnsafeMigrationSyntaxHelpers.HasSafeModifier(declaration)) { return; } @@ -76,7 +76,7 @@ internal static async Task AddUnsafeModifierAsync( CancellationToken cancellationToken) { var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - SyntaxTokenList modifiers = UnsafeMigrationAnalyzerHelpers.GetModifiers(declaration); + SyntaxTokenList modifiers = UnsafeMigrationSyntaxHelpers.GetModifiers(declaration); if (declaration is AccessorDeclarationSyntax accessor) { editor.ReplaceNode(accessor, AddUnsafeModifier(accessor)); @@ -105,7 +105,7 @@ internal static async Task RemoveUnsafeModifierAsync( CancellationToken cancellationToken) { var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - SyntaxTokenList modifiers = UnsafeMigrationAnalyzerHelpers.GetModifiers(declaration); + SyntaxTokenList modifiers = UnsafeMigrationSyntaxHelpers.GetModifiers(declaration); if (declaration is AccessorDeclarationSyntax accessor) { editor.ReplaceNode(accessor, RemoveUnsafeModifier(accessor)); diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj b/src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj index 302f64270b43ad..fa69abc3faa189 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/ILLink.RoslynAnalyzer.csproj @@ -50,10 +50,6 @@ - - - - diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs index c1536a581c4c3a..ff8e260a5aae6c 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs @@ -74,8 +74,8 @@ private static void AnalyzeSymbol( foreach (SyntaxNode declaration in UnsafeMigrationAnalyzerHelpers.GetDeclarations(symbol, cancellationToken)) { // safe is an explicit audited contract on the limited declaration kinds where Roslyn permits it. - if (UnsafeMigrationAnalyzerHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword) - || UnsafeMigrationAnalyzerHelpers.HasSafeModifier(declaration) + if (UnsafeMigrationSyntaxHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword) + || UnsafeMigrationSyntaxHelpers.HasSafeModifier(declaration) || UnsafeMigrationAnalyzerHelpers.HasSafetyDocumentation(declaration, symbol, cancellationToken)) { continue; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs index 4026b0892ede0d..f1ea717de1fffb 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs @@ -71,7 +71,7 @@ private static void AnalyzeSymbol( foreach (SyntaxNode declaration in UnsafeMigrationAnalyzerHelpers.GetDeclarations(symbol, cancellationToken)) { - SyntaxToken unsafeModifier = UnsafeMigrationAnalyzerHelpers.GetModifier( + SyntaxToken unsafeModifier = UnsafeMigrationSyntaxHelpers.GetModifier( declaration, SyntaxKind.UnsafeKeyword); if (unsafeModifier == default diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs index 699ebbaa9b1c93..144e9fd98942f6 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs @@ -7,7 +7,6 @@ using System.Runtime.InteropServices; using System.Threading; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; namespace ILLink.RoslynAnalyzer @@ -18,35 +17,6 @@ namespace ILLink.RoslynAnalyzer /// internal static class UnsafeMigrationAnalyzerHelpers { - // The analyzer builds against a Roslyn version that predates SyntaxKind.SafeKeyword. - private static readonly SyntaxKind s_safeKeyword = SyntaxFacts.GetContextualKeywordKind("safe"); - - internal static SyntaxTokenList GetModifiers(SyntaxNode declaration) => - declaration switch - { - MemberDeclarationSyntax member => member.Modifiers, - LocalFunctionStatementSyntax localFunction => localFunction.Modifiers, - AccessorDeclarationSyntax accessor => accessor.Modifiers, - _ => default, - }; - - internal static bool HasModifier(SyntaxNode declaration, SyntaxKind modifier) => - GetModifiers(declaration).Any(modifier); - - internal static bool HasSafeModifier(SyntaxNode declaration) => - s_safeKeyword != SyntaxKind.None && GetModifiers(declaration).Any(s_safeKeyword); - - internal static SyntaxToken GetModifier(SyntaxNode declaration, SyntaxKind modifier) - { - foreach (SyntaxToken token in GetModifiers(declaration)) - { - if (token.IsKind(modifier)) - return token; - } - - return default; - } - /// /// Gets source declarations for a symbol, normalizing field-like variable declarators to their shared declaration. /// diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs new file mode 100644 index 00000000000000..66fbe621f1628f --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs @@ -0,0 +1,47 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if DEBUG +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace ILLink.RoslynAnalyzer +{ + /// + /// Provides source-shared modifier inspection for the unsafe-v2 analyzers and code fixes. + /// + internal static class UnsafeMigrationSyntaxHelpers + { + // The analyzer builds against a Roslyn version that predates SyntaxKind.SafeKeyword. + private static readonly SyntaxKind s_safeKeyword = SyntaxFacts.GetContextualKeywordKind("safe"); + + internal static SyntaxTokenList GetModifiers(SyntaxNode declaration) => + declaration switch + { + MemberDeclarationSyntax member => member.Modifiers, + LocalFunctionStatementSyntax localFunction => localFunction.Modifiers, + AccessorDeclarationSyntax accessor => accessor.Modifiers, + _ => default, + }; + + internal static bool HasModifier(SyntaxNode declaration, SyntaxKind modifier) => + GetModifiers(declaration).Any(modifier); + + internal static bool HasSafeModifier(SyntaxNode declaration) => + s_safeKeyword != SyntaxKind.None && GetModifiers(declaration).Any(s_safeKeyword); + + internal static SyntaxToken GetModifier(SyntaxNode declaration, SyntaxKind modifier) + { + foreach (SyntaxToken token in GetModifiers(declaration)) + { + if (token.IsKind(modifier)) + return token; + } + + return default; + } + } +} +#endif From 02aeb519d0f18b3f0b284a6776fd2494a838d21d Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 23 Jul 2026 22:26:10 +0200 Subject: [PATCH 7/8] Address remaining unsafe migration review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68ec5b5d-ac1d-42bc-b2fb-9b96ea2a0456 --- .../src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj | 2 +- .../ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs | 2 +- .../src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs | 2 +- .../PointerSignatureRequiresUnsafeAnalyzer.cs | 8 +------- .../UnsafeMemberMissingSafetyDocumentationAnalyzer.cs | 8 +------- .../illink/src/ILLink.Shared/ILLink.Shared.projitems | 5 ++++- .../UnsafeMigrationSyntaxHelpers.cs | 2 +- 7 files changed, 10 insertions(+), 19 deletions(-) rename src/tools/illink/src/{ILLink.RoslynAnalyzer => ILLink.Shared}/UnsafeMigrationSyntaxHelpers.cs (98%) diff --git a/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj b/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj index 441acde0ff83a8..a2fb42a72a1fc6 100644 --- a/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj +++ b/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj @@ -14,7 +14,7 @@ - + diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs index c68fa9b60dd4d1..80cdbc5f7db644 100644 --- a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs @@ -6,7 +6,7 @@ using System.Composition; using System.Threading.Tasks; using ILLink.CodeFixProvider; -using ILLink.RoslynAnalyzer; +using ILLink.Shared; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; diff --git a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs index 2813ff19d1402d..74a531089be3c5 100644 --- a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs +++ b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs @@ -6,7 +6,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; -using ILLink.RoslynAnalyzer; +using ILLink.Shared; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs index ff8e260a5aae6c..b46197a7ca0674 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/PointerSignatureRequiresUnsafeAnalyzer.cs @@ -43,14 +43,8 @@ public override void Initialize(AnalysisContext context) context.RegisterOperationAction(AnalyzeLocalFunction, OperationKind.LocalFunction); } - private static void AnalyzeSymbol(SymbolAnalysisContext context) - { - // Roslyn does not invoke symbol actions for local functions. - if (context.Symbol is IMethodSymbol { MethodKind: MethodKind.LocalFunction }) - return; - + private static void AnalyzeSymbol(SymbolAnalysisContext context) => AnalyzeSymbol(context.Symbol, context.CancellationToken, context.ReportDiagnostic); - } private static void AnalyzeLocalFunction(OperationAnalysisContext context) => AnalyzeSymbol( diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs index f1ea717de1fffb..e5548f984bf12e 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMemberMissingSafetyDocumentationAnalyzer.cs @@ -46,14 +46,8 @@ public override void Initialize(AnalysisContext context) context.RegisterOperationAction(AnalyzeLocalFunction, OperationKind.LocalFunction); } - private static void AnalyzeSymbol(SymbolAnalysisContext context) - { - // Roslyn does not invoke symbol actions for local functions. - if (context.Symbol is IMethodSymbol { MethodKind: MethodKind.LocalFunction }) - return; - + private static void AnalyzeSymbol(SymbolAnalysisContext context) => AnalyzeSymbol(context.Symbol, context.CancellationToken, context.ReportDiagnostic); - } private static void AnalyzeLocalFunction(OperationAnalysisContext context) => AnalyzeSymbol( diff --git a/src/tools/illink/src/ILLink.Shared/ILLink.Shared.projitems b/src/tools/illink/src/ILLink.Shared/ILLink.Shared.projitems index 39c8dce4f0a800..242d14c8ce81c4 100644 --- a/src/tools/illink/src/ILLink.Shared/ILLink.Shared.projitems +++ b/src/tools/illink/src/ILLink.Shared/ILLink.Shared.projitems @@ -8,7 +8,10 @@ ILLink.Shared - + + diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs b/src/tools/illink/src/ILLink.Shared/UnsafeMigrationSyntaxHelpers.cs similarity index 98% rename from src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs rename to src/tools/illink/src/ILLink.Shared/UnsafeMigrationSyntaxHelpers.cs index 66fbe621f1628f..d099218d1b261e 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs +++ b/src/tools/illink/src/ILLink.Shared/UnsafeMigrationSyntaxHelpers.cs @@ -7,7 +7,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; -namespace ILLink.RoslynAnalyzer +namespace ILLink.Shared { /// /// Provides source-shared modifier inspection for the unsafe-v2 analyzers and code fixes. From 0522222c2575289f9060caabf544d1f0424af860 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 23 Jul 2026 22:57:26 +0200 Subject: [PATCH 8/8] Keep Roslyn helpers out of ILLink.Shared Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68ec5b5d-ac1d-42bc-b2fb-9b96ea2a0456 --- .../illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj | 2 +- .../src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs | 2 +- .../src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs | 2 +- .../UnsafeMigrationSyntaxHelpers.cs | 2 +- src/tools/illink/src/ILLink.Shared/ILLink.Shared.projitems | 5 +---- 5 files changed, 5 insertions(+), 8 deletions(-) rename src/tools/illink/src/{ILLink.Shared => ILLink.RoslynAnalyzer}/UnsafeMigrationSyntaxHelpers.cs (98%) diff --git a/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj b/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj index a2fb42a72a1fc6..441acde0ff83a8 100644 --- a/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj +++ b/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj @@ -14,7 +14,7 @@ - + diff --git a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs index 80cdbc5f7db644..c68fa9b60dd4d1 100644 --- a/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs +++ b/src/tools/illink/src/ILLink.CodeFix/RemoveInvalidUnsafeCodeFixProvider.cs @@ -6,7 +6,7 @@ using System.Composition; using System.Threading.Tasks; using ILLink.CodeFixProvider; -using ILLink.Shared; +using ILLink.RoslynAnalyzer; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; diff --git a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs index 74a531089be3c5..2813ff19d1402d 100644 --- a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs +++ b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs @@ -6,7 +6,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; -using ILLink.Shared; +using ILLink.RoslynAnalyzer; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; diff --git a/src/tools/illink/src/ILLink.Shared/UnsafeMigrationSyntaxHelpers.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs similarity index 98% rename from src/tools/illink/src/ILLink.Shared/UnsafeMigrationSyntaxHelpers.cs rename to src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs index d099218d1b261e..66fbe621f1628f 100644 --- a/src/tools/illink/src/ILLink.Shared/UnsafeMigrationSyntaxHelpers.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs @@ -7,7 +7,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; -namespace ILLink.Shared +namespace ILLink.RoslynAnalyzer { /// /// Provides source-shared modifier inspection for the unsafe-v2 analyzers and code fixes. diff --git a/src/tools/illink/src/ILLink.Shared/ILLink.Shared.projitems b/src/tools/illink/src/ILLink.Shared/ILLink.Shared.projitems index 242d14c8ce81c4..39c8dce4f0a800 100644 --- a/src/tools/illink/src/ILLink.Shared/ILLink.Shared.projitems +++ b/src/tools/illink/src/ILLink.Shared/ILLink.Shared.projitems @@ -8,10 +8,7 @@ ILLink.Shared - - +