Add safety documentation analyzers

src/tools/illink/src/ILLink.CodeFix/AddSafetyCommentCodeFixProvider.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.Collections.Immutable;
+using System.Composition;
+using System.Linq;
+using System.Threading;
+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;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Editing;
+
+namespace ILLink.CodeFix
+{
+    /// <summary>
+    /// Fixes analyzer diagnostic <c>IL5009</c> by inserting a <c>// SAFETY: TODO</c> stub above an undocumented
+    /// <c>unsafe</c> region.
+    /// </summary>
+    /// <remarks>
+    /// The stub only marks the region for review. It deliberately does not attempt to describe the reasoning,
+    /// which is what the developer has to supply.
+    /// </remarks>
+    [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AddSafetyCommentCodeFixProvider)), Shared]
+    public sealed class AddSafetyCommentCodeFixProvider : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider
+    {
+        private const string SafetyComment = "// SAFETY: TODO";
+
+        private static LocalizableString CodeFixTitle =>
+            new LocalizableResourceString(
+                nameof(Resources.AddSafetyCommentCodeFixTitle),
+                Resources.ResourceManager,
+                typeof(Resources));
+
+        public override ImmutableArray<string> FixableDiagnosticIds =>
+            [DiagnosticId.UnsafeBlockMissingSafetyComment.AsString()];
+
+        public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+        public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+        {
+            Diagnostic diagnostic = context.Diagnostics[0];
+            if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root)
+                return;
+
+            SyntaxToken unsafeKeyword = root.FindToken(diagnostic.Location.SourceSpan.Start);
+            if (!unsafeKeyword.IsKind(SyntaxKind.UnsafeKeyword)
+                || GetCommentTarget(unsafeKeyword) is not { } target)
+            {
+                return;
+            }
+
+            string title = CodeFixTitle.ToString();
+            context.RegisterCodeFix(
+                CodeAction.Create(
+                    title,
+                    cancellationToken => AddSafetyCommentAsync(context.Document, target, cancellationToken),
+                    title),
+                diagnostic);
+        }
+
+        private static async Task<Document> AddSafetyCommentAsync(
+            Document document,
+            SyntaxNode target,
+            CancellationToken cancellationToken)
+        {
+            var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
+
+            // The comment goes on its own line immediately above the target, below any existing leading trivia,
+            // so that it stays adjacent to the code it describes and XML documentation keeps its place on top.
+            SyntaxTriviaList leadingTrivia = target.GetLeadingTrivia();
+            SyntaxTriviaList indentation = GetIndentation(leadingTrivia);
+            SyntaxTriviaList updated = leadingTrivia
+                .Add(SyntaxFactory.Comment(SafetyComment))
+                .Add(SyntaxFactory.ElasticCarriageReturnLineFeed)
+                .AddRange(indentation);
+
+            editor.ReplaceNode(target, target.WithLeadingTrivia(updated));
+            return editor.GetChangedDocument();
+        }
+
+        /// <summary>
+        /// Returns the whitespace that indents the target, so the inserted comment and the target line up.
+        /// </summary>
+        private static SyntaxTriviaList GetIndentation(SyntaxTriviaList leadingTrivia)
+        {
+            int start = leadingTrivia.Count;
+            while (start > 0 && leadingTrivia[start - 1].IsKind(SyntaxKind.WhitespaceTrivia))
+                start--;
+
+            return SyntaxFactory.TriviaList(leadingTrivia.Skip(start));
+        }
+
+        /// <summary>
+        /// Finds the node the comment should be attached to.
+        /// </summary>
+        /// <remarks>
+        /// An <c>unsafe</c> block is its own statement, but an <c>unsafe</c> expression sits inside a larger
+        /// statement, and a comment reads better above that statement than inline before the keyword.
+        /// </remarks>
+        private static SyntaxNode? GetCommentTarget(SyntaxToken unsafeKeyword)
+        {
+            if (unsafeKeyword.Parent is null)
+                return null;
+
+            if (unsafeKeyword.Parent.IsKind(SyntaxKind.UnsafeStatement))
+                return unsafeKeyword.Parent;
+
+            return unsafeKeyword.Parent.AncestorsAndSelf()
+                .FirstOrDefault(static ancestor => ancestor is StatementSyntax or MemberDeclarationSyntax);
+        }
+    }
+}
+#endif

src/tools/illink/src/ILLink.CodeFix/Resources.resx

@@ -144,6 +144,9 @@
   <data name="AddUnsafeToFieldCodeFixTitle" xml:space="preserve">
     <value>Mark field-like member 'unsafe'</value>
   </data>
+  <data name="AddSafetyCommentCodeFixTitle" xml:space="preserve">
+    <value>Add '// SAFETY:' comment</value>
+  </data>
   <data name="UconditionalSuppressMessageCodeFixTitle" xml:space="preserve">
     <value>Add UnconditionalSuppressMessage attribute to parent method</value>
   </data>

src/tools/illink/src/ILLink.RoslynAnalyzer/SafeModifierMissingJustificationAnalyzer.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 ILLink.Shared;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace ILLink.RoslynAnalyzer
+{
+    /// <summary>
+    /// Reports <c>IL5010</c> for an explicit <c>safe</c> modifier that has no <c>&lt;safety&gt;</c> XML
+    /// documentation.
+    /// </summary>
+    /// <remarks>
+    /// This is the symmetric hole to <c>IL5005</c>. <c>safe</c> is a hand-written assertion the compiler cannot
+    /// verify: on an <c>extern</c> member or a <c>LibraryImport</c> it claims the native boundary upholds its
+    /// contract, and on a field in an explicit or extended layout it claims the overlap cannot be used to
+    /// type-pun. Both deserve a recorded audit. No code fix is offered because only a developer can write the
+    /// justification. The diagnostic is disabled by default while this migration tooling remains experimental.
+    /// </remarks>
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public sealed class SafeModifierMissingJustificationAnalyzer : DiagnosticAnalyzer
+    {
+        private static readonly DiagnosticDescriptor s_rule =
+            DiagnosticDescriptors.GetDiagnosticDescriptor(
+                DiagnosticId.SafeModifierMissingJustification,
+                isEnabledByDefault: false);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [s_rule];
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+            if (!System.Diagnostics.Debugger.IsAttached)
+                context.EnableConcurrentExecution();
+
+            context.RegisterSymbolAction(
+                AnalyzeSymbol,
+                SymbolKind.Method,
+                SymbolKind.Property,
+                SymbolKind.Field,
+                SymbolKind.Event);
+            context.RegisterOperationAction(AnalyzeLocalFunction, OperationKind.LocalFunction);
+        }
+
+        private static void AnalyzeSymbol(SymbolAnalysisContext context) =>
+            AnalyzeSymbol(context.Symbol, context.CancellationToken, context.ReportDiagnostic);
+
+        private static void AnalyzeLocalFunction(OperationAnalysisContext context) =>
+            AnalyzeSymbol(
+                ((ILocalFunctionOperation)context.Operation).Symbol,
+                context.CancellationToken,
+                context.ReportDiagnostic);
+
+        private static void AnalyzeSymbol(
+            ISymbol symbol,
+            System.Threading.CancellationToken cancellationToken,
+            System.Action<Diagnostic> reportDiagnostic)
+        {
+            // Accessors take their contract from the containing property or event, which is analyzed separately.
+            if (symbol is IMethodSymbol { AssociatedSymbol: not null })
+                return;
+
+            foreach (SyntaxNode declaration in UnsafeMigrationAnalyzerHelpers.GetDeclarations(symbol, cancellationToken))
+            {
+                if (!UnsafeMigrationSyntaxHelpers.HasSafeModifier(declaration)
+                    || UnsafeMigrationAnalyzerHelpers.HasSafetyDocumentation(declaration, symbol, cancellationToken))
+                {
+                    continue;
+                }
+
+                SyntaxToken safeModifier = UnsafeMigrationSyntaxHelpers.GetModifier(
+                    declaration,
+                    UnsafeMigrationSyntaxHelpers.SafeKeywordKind);
+                if (safeModifier == default)
+                    continue;
+
+                reportDiagnostic(Diagnostic.Create(s_rule, safeModifier.GetLocation()));
+            }
+        }
+    }
+}
+#endif

src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeBlockMissingSafetyCommentAnalyzer.cs

@@ -0,0 +1,131 @@
+// 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.Linq;
+using System.Text.RegularExpressions;
+using ILLink.Shared;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace ILLink.RoslynAnalyzer
+{
+    /// <summary>
+    /// Reports <c>IL5009</c> for an <c>unsafe</c> region that has no <c>// SAFETY:</c> comment explaining how its
+    /// obligations are discharged.
+    /// </summary>
+    /// <remarks>
+    /// <c>IL5005</c> covers the signature side of the contract, where an <c>unsafe</c> member documents what it
+    /// asks of its callers. This covers the other side: an <c>unsafe</c> region is where those obligations are
+    /// actually discharged, and the reasoning is invisible unless it is written down. The convention follows
+    /// <see href="https://std-dev-guide.rust-lang.org/policy/safety-comments.html">Rust's safety comments</see>,
+    /// which the speclet recommends. The diagnostic is disabled by default while this migration tooling remains
+    /// experimental.
+    /// </remarks>
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public sealed class UnsafeBlockMissingSafetyCommentAnalyzer : DiagnosticAnalyzer
+    {
+        /// <summary>
+        /// Matches the <c>SAFETY:</c> marker anywhere inside a comment.
+        /// </summary>
+        /// <remarks>
+        /// The marker is matched as a whole word so that <c>// UNSAFETY:</c> or <c>// SAFETYNET</c> do not count,
+        /// and it is case-sensitive to keep the convention greppable across a code base. Matching anywhere in the
+        /// comment allows the marker to sit on an inner line of a block comment.
+        /// </remarks>
+        private static readonly Regex s_safetyComment = new(@"\bSAFETY\s*:", RegexOptions.CultureInvariant);
+
+        private static readonly DiagnosticDescriptor s_rule =
+            DiagnosticDescriptors.GetDiagnosticDescriptor(
+                DiagnosticId.UnsafeBlockMissingSafetyComment,
+                isEnabledByDefault: false);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [s_rule];
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+            if (!System.Diagnostics.Debugger.IsAttached)
+                context.EnableConcurrentExecution();
+
+            // 'unsafe' expressions are newer than the Roslyn this analyzer compiles against, so both forms are
+            // matched on the keyword token rather than on a node type.
+            context.RegisterSyntaxNodeAction(AnalyzeUnsafeStatement, SyntaxKind.UnsafeStatement);
+            context.RegisterSyntaxTreeAction(AnalyzeUnsafeExpressions);
+        }
+
+        private static void AnalyzeUnsafeStatement(SyntaxNodeAnalysisContext context)
+        {
+            SyntaxNode unsafeStatement = context.Node;
+
+            // A nested region inherits the reasoning of the one that already documents it.
+            if (IsNestedInUnsafeRegion(unsafeStatement)
+                || HasSafetyComment(unsafeStatement.GetFirstToken()))
+            {
+                return;
+            }
+
+            context.ReportDiagnostic(Diagnostic.Create(s_rule, unsafeStatement.GetFirstToken().GetLocation()));
+        }
+
+        private static void AnalyzeUnsafeExpressions(SyntaxTreeAnalysisContext context)
+        {
+            SyntaxNode root = context.Tree.GetRoot(context.CancellationToken);
+            foreach (SyntaxToken token in root.DescendantTokens())
+            {
+                if (!UnsafeMigrationSyntaxHelpers.IsUnsafeExpressionKeyword(token)
+                    || token.Parent is null
+                    || IsNestedInUnsafeRegion(token.Parent)
+                    || HasSafetyComment(token))
+                {
+                    continue;
+                }
+
+                context.ReportDiagnostic(Diagnostic.Create(s_rule, token.GetLocation()));
+            }
+        }
+
+        /// <summary>
+        /// Determines whether a region sits inside another <c>unsafe</c> region, whose comment already covers it.
+        /// </summary>
+        private static bool IsNestedInUnsafeRegion(SyntaxNode region) =>
+            region.Ancestors().Any(static ancestor =>
+                ancestor.IsKind(SyntaxKind.UnsafeStatement)
+                || UnsafeMigrationSyntaxHelpers.IsUnsafeExpressionKeyword(ancestor.GetFirstToken()));
+
+        /// <summary>
+        /// Looks for a <c>// SAFETY:</c> comment attached to the region or to the statement that contains it.
+        /// </summary>
+        /// <remarks>
+        /// An <c>unsafe</c> expression sits inside a larger statement, so its comment is normally written above
+        /// that statement rather than immediately before the keyword.
+        /// </remarks>
+        private static bool HasSafetyComment(SyntaxToken unsafeKeyword)
+        {
+            if (ContainsSafetyComment(unsafeKeyword.LeadingTrivia))
+                return true;
+
+            for (SyntaxNode? node = unsafeKeyword.Parent; node is not null; node = node.Parent)
+            {
+                if (ContainsSafetyComment(node.GetLeadingTrivia()))
+                    return true;
+
+                // Stop at the statement or member that owns the region; trivia further out documents something else.
+                if (node is StatementSyntax or MemberDeclarationSyntax)
+                    break;
+            }
+
+            return false;
+        }
+
+        private static bool ContainsSafetyComment(SyntaxTriviaList trivia) =>
+            trivia.Any(static t =>
+                (t.IsKind(SyntaxKind.SingleLineCommentTrivia) || t.IsKind(SyntaxKind.MultiLineCommentTrivia))
+                && s_safetyComment.IsMatch(t.ToString()));
+    }
+}
+#endif

src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs

@@ -17,6 +17,12 @@ internal static class UnsafeMigrationSyntaxHelpers
         // The analyzer builds against a Roslyn version that predates SyntaxKind.SafeKeyword.
         private static readonly SyntaxKind s_safeKeyword = SyntaxFacts.GetContextualKeywordKind("safe");
 
+        /// <summary>
+        /// The kind of the <c>safe</c> contextual keyword, or <see cref="SyntaxKind.None"/> when the hosting
+        /// compiler does not know it.
+        /// </summary>
+        internal static SyntaxKind SafeKeywordKind => s_safeKeyword;
+
         internal static SyntaxTokenList GetModifiers(SyntaxNode declaration) =>
             declaration switch
             {
@@ -42,6 +48,19 @@ internal static SyntaxToken GetModifier(SyntaxNode declaration, SyntaxKind modif
 
             return default;
         }
+
+        /// <summary>
+        /// Determines whether an <c>unsafe</c> keyword token introduces an <c>unsafe(...)</c> expression rather
+        /// than a modifier or a block.
+        /// </summary>
+        /// <remarks>
+        /// <c>UnsafeExpressionSyntax</c> is newer than the Roslyn these analyzers compile against, but it derives
+        /// from <see cref="ExpressionSyntax"/>, which is not, so the expression form is recognized by its parent's
+        /// base type. Matching on a following open parenthesis instead would misclassify the modifier on any
+        /// member whose type is a tuple, such as <c>static unsafe (int, int) M()</c>.
+        /// </remarks>
+        internal static bool IsUnsafeExpressionKeyword(SyntaxToken token) =>
+            token.IsKind(SyntaxKind.UnsafeKeyword) && token.Parent is ExpressionSyntax;
     }
 }
 #endif