diff --git a/src/tools/illink/src/ILLink.CodeFix/AddUnsafeContextCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/AddUnsafeContextCodeFixProvider.cs new file mode 100644 index 00000000000000..5b277f7902950e --- /dev/null +++ b/src/tools/illink/src/ILLink.CodeFix/AddUnsafeContextCodeFixProvider.cs @@ -0,0 +1,1588 @@ +// 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.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +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; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Text; + +namespace ILLink.CodeFix +{ + /// + /// Fixes unsafe-context diagnostics by introducing the narrowest context that preserves source semantics. + /// Unsafe statements are preferred so the generated audit comment can be expanded during review. + /// + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AddUnsafeContextCodeFixProvider)), Shared] + public sealed class AddUnsafeContextCodeFixProvider : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider + { + public const string UnsafeOperationDiagnosticId = "CS9360"; + public const string UnsafeUninitializedStackAllocDiagnosticId = "CS9361"; + public const string UnsafeMemberOperationDiagnosticId = "CS9362"; + public const string UnsafeMemberOperationCompatDiagnosticId = "CS9363"; + public const string UnsafeConstructorConstraintDiagnosticId = "CS9376"; + + internal const string SafetyAuditComment = "// SAFETY: Audit"; + + private const string UnsafeExpressionPlaceholder = "__unsafeExpression"; + + /// + /// How many audited-but-safe statements may be absorbed when merging a new unsafe region with an adjacent + /// generated one. Merging trades a slightly wider audit scope for far fewer unsafe blocks per member; keep + /// this small so that migration does not silently blanket unrelated code. + /// + private const int MaxSafeStatementsBetweenMergedRegions = 1; + + private static readonly SymbolDisplayFormat s_localTypeDisplayFormat = + SymbolDisplayFormat.MinimallyQualifiedFormat.WithMiscellaneousOptions( + SymbolDisplayFormat.MinimallyQualifiedFormat.MiscellaneousOptions + | SymbolDisplayMiscellaneousOptions.EscapeKeywordIdentifiers + | SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier + | SymbolDisplayMiscellaneousOptions.UseSpecialTypes); + + private static LocalizableString CodeFixTitle => + new LocalizableResourceString( + nameof(Resources.AddUnsafeContextCodeFixTitle), + Resources.ResourceManager, + typeof(Resources)); + + public override ImmutableArray FixableDiagnosticIds => + [ + UnsafeOperationDiagnosticId, + UnsafeUninitializedStackAllocDiagnosticId, + UnsafeMemberOperationDiagnosticId, + UnsafeMemberOperationCompatDiagnosticId, + UnsafeConstructorConstraintDiagnosticId, + ]; + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + Diagnostic diagnostic = context.Diagnostics[0]; + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) + return; + + TextSpan diagnosticSpan = diagnostic.Location.SourceSpan; + SyntaxNode targetNode = root.FindNode(diagnosticSpan, getInnermostNodeForTie: true); + string title = CodeFixTitle.ToString(); + + // Attribute applications are deliberately not suppressible under the updated language rules. + if (targetNode.AncestorsAndSelf().OfType().Any()) + return; + + if (TryGetConstructorRequiringUnsafe(diagnostic, targetNode) is { } constructor) + { + RegisterFix( + context, + title, + cancellationToken => MarkConstructorUnsafeAsync( + context.Document, + constructor, + cancellationToken)); + return; + } + + if (diagnostic.Id == UnsafeConstructorConstraintDiagnosticId + && targetNode.AncestorsAndSelf().OfType().FirstOrDefault() is { } usingDirective) + { + RegisterFix( + context, + title, + _ => Task.FromResult(AddUnsafeToUsingDirective(context.Document, root, usingDirective))); + return; + } + + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (semanticModel is null) + return; + + if (IsExpressionOnlyContext(targetNode, diagnosticSpan)) + { + TryRegisterExpressionFix(context, root, semanticModel, targetNode, diagnosticSpan, title); + return; + } + + ArrowExpressionClauseSyntax? arrowExpression = targetNode.AncestorsAndSelf() + .OfType() + .FirstOrDefault(arrow => arrow.Expression.FullSpan.Contains(diagnosticSpan)); + if (arrowExpression is not null) + { + if (arrowExpression.Parent is AnonymousFunctionExpressionSyntax + || arrowExpression.Expression.DescendantNodesAndSelf().OfType().Any() + || HasDirectiveWithinSpan(arrowExpression)) + { + TryRegisterExpressionFix(context, root, semanticModel, targetNode, diagnosticSpan, title); + } + else + { + RegisterFix( + context, + title, + cancellationToken => ConvertExpressionBodyAsync( + context.Document, + root, + semanticModel, + arrowExpression, + cancellationToken)); + } + return; + } + + StatementSyntax? containingStatement = targetNode.AncestorsAndSelf() + .OfType() + .FirstOrDefault(static statement => statement is not BlockSyntax); + while (containingStatement?.Parent is LabeledStatementSyntax labeledStatement) + containingStatement = labeledStatement; + + if (containingStatement is null) + { + TryRegisterExpressionFix(context, root, semanticModel, targetNode, diagnosticSpan, title); + return; + } + + if (containingStatement is LocalDeclarationStatementSyntax localDeclaration) + { + if (TryCreateSplitLocalDeclaration( + diagnostic, + localDeclaration, + semanticModel, + context.CancellationToken, + out LocalDeclarationStatementSyntax forwardDeclaration, + out StatementSyntax assignmentStatement)) + { + RegisterFix( + context, + title, + _ => Task.FromResult(ReplaceStatementWithStatements( + context.Document, + root, + localDeclaration, + [forwardDeclaration, CreateUnsafeStatement(assignmentStatement)]))); + return; + } + + if (TryRegisterExpressionFix(context, root, semanticModel, targetNode, diagnosticSpan, title)) + return; + + if (!localDeclaration.AwaitKeyword.IsKind(SyntaxKind.None)) + return; + + if (TryGetEnclosingSwitch(localDeclaration) is { } localSwitch) + { + RegisterEnclosingSwitchFix(context, root, localSwitch, title); + return; + } + + if (StatementRange.TryCreateForStatement(localDeclaration, out StatementRange localRange, out int localIndex)) + { + TryRegisterStatementRangeFix( + context, + root, + semanticModel, + localRange, + localIndex, + forceThroughContainerEnd: IsUsingDeclaration(localDeclaration), + title); + } + return; + } + + bool cannotContainUnsafeStatement = containingStatement.DescendantNodesAndSelf() + .OfType() + .Any(); + bool topLevelScopeSensitive = containingStatement.Parent is GlobalStatementSyntax + && GetDeclaredNames(containingStatement).Count > 0; + bool containsAwaitStatement = containingStatement switch + { + CommonForEachStatementSyntax forEach => + !forEach.AwaitKeyword.IsKind(SyntaxKind.None), + UsingStatementSyntax usingStatement => + !usingStatement.AwaitKeyword.IsKind(SyntaxKind.None), + _ => false, + }; + bool preferExpression = containingStatement is YieldStatementSyntax + || containingStatement.DescendantNodesAndSelf().OfType().Any() + || containsAwaitStatement + || topLevelScopeSensitive + || HasDirectiveWithinSpan(containingStatement) + || WouldNarrowDeclaredVariableScope( + containingStatement, + semanticModel, + context.CancellationToken); + + if ((cannotContainUnsafeStatement || preferExpression) + && TryRegisterExpressionFix(context, root, semanticModel, targetNode, diagnosticSpan, title)) + { + return; + } + + bool hasStatementList = StatementRange.TryCreateForStatement( + containingStatement, + out StatementRange statements, + out int statementIndex); + + if (topLevelScopeSensitive && hasStatementList) + { + TryRegisterStatementRangeFix( + context, + root, + semanticModel, + statements, + statementIndex, + forceThroughContainerEnd: true, + title); + return; + } + + if (cannotContainUnsafeStatement || containsAwaitStatement) + return; + + if (preferExpression && TryGetEnclosingSwitch(containingStatement) is { } containingSwitch) + { + RegisterEnclosingSwitchFix(context, root, containingSwitch, title); + return; + } + + if (hasStatementList) + { + TryRegisterStatementRangeFix( + context, + root, + semanticModel, + statements, + statementIndex, + forceThroughContainerEnd: false, + title); + return; + } + + if (preferExpression) + return; + + // Embedded statements (such as the body of an 'if' without braces) have no statement list to extend. + RegisterFix( + context, + title, + _ => Task.FromResult(ReplaceStatementWithStatements( + context.Document, + root, + containingStatement, + [CreateUnsafeStatement(containingStatement)]))); + } + + private static void RegisterFix( + CodeFixContext context, + string title, + System.Func> createChangedDocument) => + context.RegisterCodeFix( + CodeAction.Create(title, createChangedDocument, title), + context.Diagnostics); + + private static void RegisterEnclosingSwitchFix( + CodeFixContext context, + SyntaxNode root, + SwitchStatementSyntax switchStatement, + string title) + { + if (ContainsAwaitOrYield(switchStatement)) + return; + + RegisterFix( + context, + title, + _ => Task.FromResult(ReplaceStatementWithStatements( + context.Document, + root, + switchStatement, + [CreateUnsafeStatement(switchStatement)]))); + } + + private static bool TryRegisterExpressionFix( + CodeFixContext context, + SyntaxNode root, + SemanticModel semanticModel, + SyntaxNode targetNode, + TextSpan diagnosticSpan, + string title) + { + if (TryGetUnsafeExpressionFix( + targetNode, + diagnosticSpan, + semanticModel, + context.CancellationToken) is not { } fix) + { + return false; + } + + (ExpressionSyntax expression, ExpressionSyntax operand, ExpressionSyntax template) = fix; + RegisterFix( + context, + title, + _ => Task.FromResult(ReplaceWithUnsafeExpression( + context.Document, + root, + expression, + operand, + template))); + return true; + } + + private static bool TryRegisterStatementRangeFix( + CodeFixContext context, + SyntaxNode root, + SemanticModel semanticModel, + in StatementRange statements, + int statementIndex, + bool forceThroughContainerEnd, + string title) + { + if (!TryGetStatementRangeToWrap( + statements, + statementIndex, + semanticModel, + context.CancellationToken, + forceThroughContainerEnd, + out int start, + out int end)) + { + return false; + } + + (int mergedStart, int mergedEnd) = ExtendRangeOverGeneratedUnsafeStatements(statements, start, end); + if (mergedStart != start || mergedEnd != end) + { + if (CanWrapStatementRange(statements, mergedStart, mergedEnd, semanticModel, context.CancellationToken)) + { + (start, end) = (mergedStart, mergedEnd); + } + } + + if (!CanWrapStatementRange(statements, start, end, semanticModel, context.CancellationToken)) + return false; + + StatementRange range = statements; + RegisterFix( + context, + title, + _ => Task.FromResult(WrapStatementRange(context.Document, root, range, start, end))); + return true; + } + + private static ConstructorDeclarationSyntax? TryGetConstructorRequiringUnsafe( + Diagnostic diagnostic, + SyntaxNode targetNode) + { + if (diagnostic.Id is not (UnsafeMemberOperationDiagnosticId or UnsafeMemberOperationCompatDiagnosticId)) + return null; + + if (targetNode.AncestorsAndSelf().OfType().FirstOrDefault() is { } initializer + && diagnostic.Location.SourceSpan.Contains(initializer.Span)) + { + return initializer.Parent as ConstructorDeclarationSyntax; + } + + if (targetNode.AncestorsAndSelf().OfType().FirstOrDefault() is { } constructor + && constructor.Initializer is null + && diagnostic.Location.SourceSpan.Contains(constructor.Span)) + { + return constructor; + } + + return null; + } + + /// + /// An unsafe constructor is the only way to establish an unsafe context for its initializer, so this widens + /// the constructor's own contract and is documented like any other caller-unsafe declaration. + /// + private static async Task MarkConstructorUnsafeAsync( + Document document, + ConstructorDeclarationSyntax constructor, + CancellationToken cancellationToken) + { + document = await UnsafeModifierCodeFixHelpers.SetUnsafeModifierAsync( + document, + constructor, + cancellationToken).ConfigureAwait(false); + return await UnsafeModifierCodeFixHelpers.AddPendingSafetyDocumentationAsync( + document, + cancellationToken).ConfigureAwait(false); + } + + private static bool IsExpressionOnlyContext(SyntaxNode targetNode, TextSpan diagnosticSpan) + { + if (targetNode.AncestorsAndSelf().OfType() + .Any(filter => filter.FilterExpression.FullSpan.Contains(diagnosticSpan))) + { + return true; + } + + if (targetNode.AncestorsAndSelf().OfType() + .Any(initializer => initializer.ArgumentList.FullSpan.Contains(diagnosticSpan))) + { + return true; + } + + return false; + } + + /// + /// Builds the unsafe expression template with the parse options of the document being fixed, so the generated + /// syntax matches the language mode the project compiles with. Returns when that mode + /// cannot express an unsafe expression, or when the loaded compiler predates the syntax. + /// + private static ExpressionSyntax? TryParseUnsafeExpressionTemplate(SyntaxNode node) + { + ExpressionSyntax template = SyntaxFactory.ParseExpression( + $"unsafe(/* SAFETY: Audit */ {UnsafeExpressionPlaceholder})", + options: node.SyntaxTree.Options as CSharpParseOptions); + return !template.ContainsDiagnostics && TryFindPlaceholder(template) is not null + ? template + : null; + } + + private static IdentifierNameSyntax? TryFindPlaceholder(ExpressionSyntax expression) => + expression.DescendantNodesAndSelf() + .OfType() + .FirstOrDefault(static identifier => identifier.Identifier.ValueText == UnsafeExpressionPlaceholder); + + /// + /// Builds the expression-level fix, or returns when no semantics-preserving unsafe + /// expression can be written for the reported operation. + /// + private static (ExpressionSyntax Expression, ExpressionSyntax Operand, ExpressionSyntax Template)? TryGetUnsafeExpressionFix( + SyntaxNode targetNode, + TextSpan diagnosticSpan, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + if (TryGetUnsafeExpression(targetNode, diagnosticSpan) is not { } expression + || !CanUseUnsafeExpression(expression) + || TryParseUnsafeExpressionTemplate(expression) is not { } template) + { + return null; + } + + // Reaching here from inside an unsafe expression means the compiler does not treat that expression as an + // unsafe context for this operation, so wrapping it again would only nest without fixing anything. + if (expression.AncestorsAndSelf().Any(node => node.RawKind == template.RawKind)) + return null; + + if (TryGetUnsafeExpressionOperand(semanticModel, expression, cancellationToken) is not { } operand) + return null; + + return (expression, operand, template); + } + + private static ExpressionSyntax? TryGetUnsafeExpression(SyntaxNode targetNode, TextSpan diagnosticSpan) => + PromoteUnsafeExpression( + targetNode.AncestorsAndSelf() + .OfType() + .FirstOrDefault(expression => expression.FullSpan.Contains(diagnosticSpan)), + diagnosticSpan); + + private static ExpressionSyntax? PromoteUnsafeExpression( + ExpressionSyntax? expression, + TextSpan diagnosticSpan) + { + if (expression is null) + return null; + + foreach (SyntaxNode ancestor in expression.AncestorsAndSelf()) + { + switch (ancestor) + { + case ConditionalAccessExpressionSyntax conditionalAccess + when conditionalAccess.WhenNotNull.FullSpan.Contains(diagnosticSpan): + expression = conditionalAccess; + break; + + case BaseObjectCreationExpressionSyntax objectCreation + when objectCreation.Initializer?.FullSpan.Contains(diagnosticSpan) == true: + expression = objectCreation; + break; + + case WithExpressionSyntax withExpression + when withExpression.Initializer.FullSpan.Contains(diagnosticSpan): + expression = withExpression; + break; + + case CollectionExpressionSyntax collectionExpression + when collectionExpression.FullSpan.Contains(diagnosticSpan): + expression = collectionExpression; + break; + } + } + + return expression; + } + + private static bool CanUseUnsafeExpression(ExpressionSyntax expression) => + !expression.DescendantNodesAndSelf().OfType().Any() + && (expression.Parent switch + { + ExpressionStatementSyntax { Expression: var statementExpression } + when statementExpression == expression => false, + ForStatementSyntax forStatement + when forStatement.Initializers.Contains(expression) + || forStatement.Incrementors.Contains(expression) => false, + AttributeArgumentSyntax => false, + // An unsafe expression cannot be the target of an assignment or an increment, and wrapping the right + // side of a deconstruction does not put the generated Deconstruct call in an unsafe context. + AssignmentExpressionSyntax assignment => + assignment.Left != expression + && assignment.Left is not (DeclarationExpressionSyntax or TupleExpressionSyntax), + PrefixUnaryExpressionSyntax prefix => + !prefix.IsKind(SyntaxKind.PreIncrementExpression) + && !prefix.IsKind(SyntaxKind.PreDecrementExpression), + PostfixUnaryExpressionSyntax => false, + _ => true, + }); + + /// + /// Produces the expression to place inside unsafe(...). Conversions, property and indexer accessors, + /// and method group conversions are bound by the enclosing binder, so they only enter the unsafe context when + /// an explicit cast forces them inside it. + /// + private static ExpressionSyntax? TryGetUnsafeExpressionOperand( + SemanticModel semanticModel, + ExpressionSyntax expression, + CancellationToken cancellationToken) + { + ExpressionSyntax operand = expression.WithoutLeadingTrivia().WithoutTrailingTrivia(); + if (TryGetRequiredCastType(semanticModel, expression, cancellationToken, out bool requiresCast) is { } castType) + { + if (TryParseTypeName(semanticModel, castType, expression.SpanStart) is not { } castTypeSyntax) + return null; + + if (NeedsParenthesesForCast(operand)) + operand = SyntaxFactory.ParenthesizedExpression(operand); + + return SyntaxFactory.CastExpression(castTypeSyntax, operand); + } + + return requiresCast ? null : operand; + } + + private static ITypeSymbol? TryGetRequiredCastType( + SemanticModel semanticModel, + ExpressionSyntax expression, + CancellationToken cancellationToken, + out bool requiresCast) + { + requiresCast = true; + TypeInfo typeInfo = semanticModel.GetTypeInfo(expression, cancellationToken); + + // A user-defined implicit conversion is applied to the result of the unsafe expression; casting to its + // result type also pulls any receiver evaluation inside, so it is checked first. + Conversion conversion = semanticModel.GetConversion(expression, cancellationToken); + if (conversion.IsUserDefined + && conversion.IsImplicit + && conversion.MethodSymbol is { } conversionOperator) + { + return GetUserDefinedConversionResultType( + semanticModel, + expression, + conversionOperator, + cancellationToken); + } + + ISymbol? symbol = semanticModel.GetSymbolInfo(expression, cancellationToken).Symbol; + if (typeInfo.Type is null && symbol is IMethodSymbol) + { + return typeInfo.ConvertedType is INamedTypeSymbol { TypeKind: TypeKind.Delegate } delegateType + ? delegateType + : null; + } + + if (symbol is IPropertySymbol property) + { + // A cast would drop the reference, so a ref-returning accessor cannot be fixed this way. + return property.RefKind == RefKind.None + ? WithFlowNullability(typeInfo, property.Type) + : null; + } + + requiresCast = false; + return null; + } + + private static ITypeSymbol WithFlowNullability(TypeInfo typeInfo, ITypeSymbol type) => + typeInfo.Nullability.FlowState == NullableFlowState.NotNull && type.IsReferenceType + ? type.WithNullableAnnotation(NullableAnnotation.NotAnnotated) + : type; + + private static bool NeedsParenthesesForCast(ExpressionSyntax expression) => + expression is BinaryExpressionSyntax + or AssignmentExpressionSyntax + or ConditionalExpressionSyntax + or IsPatternExpressionSyntax + or SwitchExpressionSyntax + or AnonymousFunctionExpressionSyntax + or QueryExpressionSyntax + or RangeExpressionSyntax + or WithExpressionSyntax + or ThrowExpressionSyntax + // A cast followed by '&', '*', '+' or '-' is parsed as a binary operator when the cast type is also a + // valid expression, so unary operands always get parentheses. + or PrefixUnaryExpressionSyntax; + + /// + /// Renders a type as source, returning for types that cannot be named at that position. + /// + private static TypeSyntax? TryParseTypeName( + SemanticModel semanticModel, + ITypeSymbol type, + int position) + { + if (type is IErrorTypeSymbol || type.SpecialType == SpecialType.System_Void || ContainsAnonymousType(type)) + return null; + + string displayString = type.ToMinimalDisplayString(semanticModel, position, s_localTypeDisplayFormat); + TypeSyntax parsedType = SyntaxFactory.ParseTypeName(displayString); + return parsedType.ContainsDiagnostics || parsedType.ToFullString() != displayString + ? null + : parsedType; + } + + private static ITypeSymbol GetUserDefinedConversionResultType( + SemanticModel semanticModel, + ExpressionSyntax expression, + IMethodSymbol conversionOperator, + CancellationToken cancellationToken) + { + ITypeSymbol resultType = conversionOperator.ReturnType; + ITypeSymbol? sourceType = semanticModel.GetTypeInfo(expression, cancellationToken).Type; + if (sourceType is INamedTypeSymbol sourceNamedType + && sourceNamedType.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T + && sourceNamedType.TypeArguments.Length == 1 + && conversionOperator.Parameters.Length == 1 + && SymbolEqualityComparer.Default.Equals( + sourceNamedType.TypeArguments[0], + conversionOperator.Parameters[0].Type) + && resultType.IsValueType + && resultType.OriginalDefinition.SpecialType != SpecialType.System_Nullable_T) + { + INamedTypeSymbol nullableType = semanticModel.Compilation.GetSpecialType( + SpecialType.System_Nullable_T); + if (nullableType.TypeKind != TypeKind.Error) + resultType = nullableType.Construct(resultType); + } + + return resultType; + } + + private static bool TryCreateSplitLocalDeclaration( + Diagnostic diagnostic, + LocalDeclarationStatementSyntax localDeclaration, + SemanticModel semanticModel, + CancellationToken cancellationToken, + out LocalDeclarationStatementSyntax forwardDeclaration, + out StatementSyntax assignmentStatement) + { + forwardDeclaration = null!; + assignmentStatement = null!; + + if (diagnostic.Id == UnsafeConstructorConstraintDiagnosticId + || localDeclaration.IsConst + || IsUsingDeclaration(localDeclaration) + || localDeclaration.Parent is not BlockSyntax and not SwitchSectionSyntax + || HasDirectiveWithinSpan(localDeclaration) + || localDeclaration.Declaration.Variables.Count != 1 + || IsRefType(localDeclaration.Declaration.Type)) + { + return false; + } + + VariableDeclaratorSyntax variable = localDeclaration.Declaration.Variables[0]; + if (variable.Initializer is not { } initializer + || !initializer.Value.FullSpan.Contains(diagnostic.Location.SourceSpan) + || initializer.Value.DescendantNodesAndSelf().OfType().Any()) + { + return false; + } + + var localSymbol = semanticModel.GetDeclaredSymbol(variable, cancellationToken) as ILocalSymbol; + if (localSymbol is null || localSymbol.Type is IErrorTypeSymbol || ContainsAnonymousType(localSymbol.Type)) + return false; + + if (InitializerVariablesEscape( + localDeclaration, + initializer.Value, + localSymbol, + semanticModel, + cancellationToken)) + { + return false; + } + + TypeSyntax type = localDeclaration.Declaration.Type; + if (type.IsVar) + { + if (TryParseTypeName(semanticModel, localSymbol.Type, localDeclaration.SpanStart) is not { } inferredType) + return false; + + type = inferredType; + } + + if (localSymbol.Type.IsRefLikeType) + { + if (!ContainsStackAllocThatFlowsToLocal(initializer.Value)) + return false; + + if (type is not ScopedTypeSyntax) + { + type = SyntaxFactory.ScopedType( + SyntaxFactory.Token(SyntaxKind.ScopedKeyword) + .WithTrailingTrivia(SyntaxFactory.Space), + type.WithoutLeadingTrivia()); + } + } + + VariableDeclaratorSyntax declarationVariable = variable.WithInitializer(null); + VariableDeclarationSyntax declaration = localDeclaration.Declaration + .WithType(type.WithTriviaFrom(localDeclaration.Declaration.Type)) + .WithVariables([declarationVariable]); + + forwardDeclaration = localDeclaration + .WithDeclaration(declaration) + .WithLeadingTrivia(localDeclaration.GetLeadingTrivia()) + .WithTrailingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed) + .WithAdditionalAnnotations(Formatter.Annotation); + + AssignmentExpressionSyntax assignment = SyntaxFactory.AssignmentExpression( + SyntaxKind.SimpleAssignmentExpression, + SyntaxFactory.IdentifierName(variable.Identifier.WithoutTrivia()), + initializer.EqualsToken, + initializer.Value.WithoutLeadingTrivia()); + assignmentStatement = SyntaxFactory.ExpressionStatement(assignment) + .WithTrailingTrivia(localDeclaration.GetTrailingTrivia()) + .WithAdditionalAnnotations(Formatter.Annotation); + return true; + } + + private static bool InitializerVariablesEscape( + LocalDeclarationStatementSyntax localDeclaration, + ExpressionSyntax initializer, + ILocalSymbol declaredLocal, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + DataFlowAnalysis? analysis = semanticModel.AnalyzeDataFlow(initializer); + if (analysis is null || !analysis.Succeeded) + return true; + + var initializerVariables = new HashSet( + analysis.VariablesDeclared.Where(symbol => !SymbolEqualityComparer.Default.Equals(symbol, declaredLocal)), + SymbolEqualityComparer.Default); + if (initializerVariables.Count == 0 + || !StatementRange.TryCreateForStatement( + localDeclaration, + out StatementRange statements, + out int statementIndex)) + { + return false; + } + + IEnumerable laterStatements = GetLaterStatements( + statements, + statementIndex, + localDeclaration); + return laterStatements.Any(statement => ReferencesAnySymbol( + statement, + initializerVariables, + semanticModel, + cancellationToken)); + } + + /// + /// Enumerates the statements that could observe declarations made by , including + /// the remaining sections of an enclosing switch statement. + /// + private static IEnumerable GetLaterStatements( + StatementRange statements, + int statementIndex, + StatementSyntax statement) + { + for (int index = statementIndex + 1; index < statements.Count; index++) + yield return statements[index]; + + if (statement.Parent is SwitchSectionSyntax switchSection + && switchSection.Parent is SwitchStatementSyntax switchStatement) + { + int sectionIndex = switchStatement.Sections.IndexOf(switchSection); + foreach (SwitchSectionSyntax laterSection in switchStatement.Sections.Skip(sectionIndex + 1)) + { + foreach (StatementSyntax laterStatement in laterSection.Statements) + yield return laterStatement; + } + } + } + + private static bool IsRefType(TypeSyntax type) => + type is RefTypeSyntax + or ScopedTypeSyntax { Type: RefTypeSyntax }; + + private static bool ContainsAnonymousType(ITypeSymbol type) => + type switch + { + INamedTypeSymbol { IsAnonymousType: true } => true, + IArrayTypeSymbol array => ContainsAnonymousType(array.ElementType), + INamedTypeSymbol namedType => namedType.TypeArguments.Any(ContainsAnonymousType), + _ => false, + }; + + private static bool ContainsStackAllocThatFlowsToLocal(ExpressionSyntax expression) => + expression switch + { + StackAllocArrayCreationExpressionSyntax or ImplicitStackAllocArrayCreationExpressionSyntax => true, + ParenthesizedExpressionSyntax parenthesized => + ContainsStackAllocThatFlowsToLocal(parenthesized.Expression), + CastExpressionSyntax cast => + ContainsStackAllocThatFlowsToLocal(cast.Expression), + ConditionalExpressionSyntax conditional => + ContainsStackAllocThatFlowsToLocal(conditional.WhenTrue) + || ContainsStackAllocThatFlowsToLocal(conditional.WhenFalse), + SwitchExpressionSyntax switchExpression => + switchExpression.Arms.Any(static arm => ContainsStackAllocThatFlowsToLocal(arm.Expression)), + _ => false, + }; + + private static bool IsUsingDeclaration(LocalDeclarationStatementSyntax declaration) => + !declaration.UsingKeyword.IsKind(SyntaxKind.None); + + private static bool WouldNarrowDeclaredVariableScope( + StatementSyntax statement, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + if (!StatementRange.TryCreateForStatement(statement, out StatementRange statements, out int statementIndex)) + return false; + + DataFlowAnalysis? analysis = semanticModel.AnalyzeDataFlow(statement); + if (analysis is null || !analysis.Succeeded) + return false; + + var declaredSymbols = new HashSet( + analysis.VariablesDeclared, + SymbolEqualityComparer.Default); + AddSyntacticallyDeclaredSymbols( + statement, + declaredSymbols, + semanticModel, + cancellationToken); + HashSet declaredNames = GetDeclaredNames(statement); + if (declaredSymbols.Count == 0 && declaredNames.Count == 0) + return false; + + return GetLaterStatements(statements, statementIndex, statement).Any(laterStatement => + ReferencesAnySymbol( + laterStatement, + declaredSymbols, + semanticModel, + cancellationToken) + || ReferencesAnyUnresolvedName(laterStatement, declaredNames, semanticModel, cancellationToken)); + } + + private static bool TryGetStatementRangeToWrap( + in StatementRange statements, + int triggerIndex, + SemanticModel semanticModel, + CancellationToken cancellationToken, + bool forceThroughContainerEnd, + out int start, + out int end) + { + start = triggerIndex; + end = forceThroughContainerEnd ? statements.Count - 1 : triggerIndex; + if (forceThroughContainerEnd) + return true; + + while (true) + { + HashSet? declaredSymbols = GetDeclaredSymbols( + statements, + start, + end, + semanticModel, + cancellationToken); + if (declaredSymbols is null) + { + end = statements.Count - 1; + return true; + } + + // A using declaration disposes at the end of its container, so its unsafe region must reach there. + for (int i = start; i <= end; i++) + { + if (statements[i] is LocalDeclarationStatementSyntax declaration && IsUsingDeclaration(declaration)) + { + end = statements.Count - 1; + return true; + } + } + + var selectedStatements = new List(end - start + 1); + for (int i = start; i <= end; i++) + selectedStatements.Add(statements[i]); + HashSet declaredNames = GetDeclaredNames(selectedStatements); + + int expandedEnd = end; + for (int i = end + 1; i < statements.Count; i++) + { + if (ReferencesAnySymbol(statements[i], declaredSymbols, semanticModel, cancellationToken) + || ReferencesAnyUnresolvedName(statements[i], declaredNames, semanticModel, cancellationToken)) + { + expandedEnd = i; + } + } + + if (expandedEnd == end) + return true; + + end = expandedEnd; + } + } + + /// + /// Grows a range so that it absorbs neighbouring unsafe regions that this fixer generated and has not yet been + /// audited, which keeps a migrated member from accumulating a long run of tiny unsafe blocks. + /// Statements between those regions are only absorbed when they declare nothing, because a declaration moved + /// into the merged block would no longer be visible to the statements that follow it. + /// + private static (int Start, int End) ExtendRangeOverGeneratedUnsafeStatements( + in StatementRange statements, + int start, + int end) + { + int gap = 0; + for (int index = start - 1; index >= 0; index--) + { + if (IsGeneratedUnsafeStatement(statements[index])) + { + start = index; + gap = 0; + continue; + } + + if (gap == MaxSafeStatementsBetweenMergedRegions || GetDeclaredNames(statements[index]).Count > 0) + break; + + gap++; + } + + gap = 0; + for (int index = end + 1; index < statements.Count; index++) + { + if (IsGeneratedUnsafeStatement(statements[index])) + { + end = index; + gap = 0; + continue; + } + + if (gap == MaxSafeStatementsBetweenMergedRegions || GetDeclaredNames(statements[index]).Count > 0) + break; + + gap++; + } + + return (start, end); + } + + /// + /// Recognizes an unsafe statement that this fixer produced and that still carries the unaudited marker. + /// Blocks whose marker was replaced during review are left alone. + /// + private static bool IsGeneratedUnsafeStatement(StatementSyntax statement) => + statement is UnsafeStatementSyntax unsafeStatement + && unsafeStatement.Block.Statements.Count > 0 + && HasSafetyAuditMarker(unsafeStatement.Block.Statements[0]); + + private static bool HasSafetyAuditMarker(SyntaxNode node) => + node.GetLeadingTrivia().Any(static trivia => + trivia.IsKind(SyntaxKind.SingleLineCommentTrivia) + && trivia.ToString() == SafetyAuditComment); + + private static StatementSyntax RemoveSafetyAuditMarker(StatementSyntax statement) + { + SyntaxTriviaList leadingTrivia = statement.GetLeadingTrivia(); + for (int index = 0; index < leadingTrivia.Count; index++) + { + if (!leadingTrivia[index].IsKind(SyntaxKind.SingleLineCommentTrivia) + || leadingTrivia[index].ToString() != SafetyAuditComment) + { + continue; + } + + int count = index + 1 < leadingTrivia.Count && leadingTrivia[index + 1].IsKind(SyntaxKind.EndOfLineTrivia) + ? 2 + : 1; + return statement.WithLeadingTrivia( + leadingTrivia.Take(index).Concat(leadingTrivia.Skip(index + count))); + } + + return statement; + } + + /// + /// Replaces the line break that separated a flattened statement from its former closing brace with an elastic + /// one, so the formatter lays the merged block out instead of inheriting the nested indentation. + /// + private static StatementSyntax NormalizeTrailingEndOfLine(StatementSyntax statement) + { + SyntaxTriviaList trailingTrivia = statement.GetTrailingTrivia(); + int count = trailingTrivia.Count; + while (count > 0 && trailingTrivia[count - 1].IsKind(SyntaxKind.EndOfLineTrivia)) + count--; + + return statement.WithTrailingTrivia( + trailingTrivia.Take(count).Concat([SyntaxFactory.ElasticCarriageReturnLineFeed])); + } + + private static HashSet? GetDeclaredSymbols( + in StatementRange statements, + int start, + int end, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + var symbols = new HashSet(SymbolEqualityComparer.Default); + + // Top-level statements are separate members, so they can only be analyzed one at a time. + if (statements.IsTopLevel) + { + for (int i = start; i <= end; i++) + { + DataFlowAnalysis? statementAnalysis = semanticModel.AnalyzeDataFlow(statements[i]); + if (statementAnalysis is null || !statementAnalysis.Succeeded) + return null; + + symbols.UnionWith(statementAnalysis.VariablesDeclared); + AddSyntacticallyDeclaredSymbols(statements[i], symbols, semanticModel, cancellationToken); + } + + return symbols; + } + + DataFlowAnalysis? analysis = semanticModel.AnalyzeDataFlow(statements[start], statements[end]); + if (analysis is null || !analysis.Succeeded) + return null; + + symbols.UnionWith(analysis.VariablesDeclared); + for (int i = start; i <= end; i++) + AddSyntacticallyDeclaredSymbols(statements[i], symbols, semanticModel, cancellationToken); + + return symbols; + } + + private static void AddSyntacticallyDeclaredSymbols( + SyntaxNode node, + HashSet symbols, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + foreach (VariableDeclaratorSyntax variable in + node.DescendantNodesAndSelf().OfType()) + { + if (semanticModel.GetDeclaredSymbol(variable, cancellationToken) is { } symbol) + symbols.Add(symbol); + } + + foreach (SingleVariableDesignationSyntax designation in + node.DescendantNodesAndSelf().OfType()) + { + if (semanticModel.GetDeclaredSymbol(designation, cancellationToken) is { } symbol) + symbols.Add(symbol); + } + } + + private static bool ReferencesAnySymbol( + SyntaxNode node, + HashSet symbols, + SemanticModel semanticModel, + CancellationToken cancellationToken) => + symbols.Count > 0 + && node.DescendantNodesAndSelf() + .OfType() + .Any(name => + semanticModel.GetSymbolInfo(name, cancellationToken).Symbol is { } symbol + && symbols.Contains(symbol)); + + private static HashSet GetDeclaredNames(SyntaxNode node) => + GetDeclaredNames([node]); + + private static HashSet GetDeclaredNames(IEnumerable nodes) + { + var names = new HashSet(); + foreach (SyntaxNode node in nodes) + { + foreach (VariableDeclaratorSyntax variable in + node.DescendantNodesAndSelf().OfType()) + { + names.Add(variable.Identifier.ValueText); + } + + foreach (SingleVariableDesignationSyntax designation in + node.DescendantNodesAndSelf().OfType()) + { + names.Add(designation.Identifier.ValueText); + } + } + + return names; + } + + /// + /// Falls back to matching identifier text for names the semantic model cannot resolve. The document contains + /// compiler errors by definition here, so this keeps ranges conservative without widening them whenever an + /// unrelated member happens to share a local's name. + /// + private static bool ReferencesAnyUnresolvedName( + SyntaxNode node, + HashSet names, + SemanticModel semanticModel, + CancellationToken cancellationToken) => + names.Count > 0 + && node.DescendantNodesAndSelf() + .OfType() + .Any(name => + names.Contains(name.Identifier.ValueText) + && semanticModel.GetSymbolInfo(name, cancellationToken).Symbol is null); + + private static bool HasDirectiveWithinSpan(SyntaxNode node) => + node.DescendantTrivia(descendIntoTrivia: true) + .Any(trivia => trivia.IsDirective + && node.Span.Contains(trivia.SpanStart)); + + private static Document AddUnsafeToUsingDirective( + Document document, + SyntaxNode root, + UsingDirectiveSyntax usingDirective) + { + SyntaxToken unsafeKeyword = SyntaxFactory.Token(SyntaxKind.UnsafeKeyword) + .WithTrailingTrivia(SyntaxFactory.Space); + UsingDirectiveSyntax replacement = usingDirective + .WithUnsafeKeyword(unsafeKeyword) + .WithAdditionalAnnotations(Formatter.Annotation); + return document.WithSyntaxRoot(root.ReplaceNode(usingDirective, replacement)); + } + + private static Document ReplaceWithUnsafeExpression( + Document document, + SyntaxNode root, + ExpressionSyntax expression, + ExpressionSyntax operand, + ExpressionSyntax template) + { + IdentifierNameSyntax placeholder = TryFindPlaceholder(template)!; + ExpressionSyntax replacement = template + .ReplaceNode(placeholder, operand) + .WithLeadingTrivia(expression.GetLeadingTrivia()) + .WithTrailingTrivia(expression.GetTrailingTrivia()); + return document.WithSyntaxRoot(root.ReplaceNode(expression, replacement)); + } + + private static bool CanWrapStatementRange( + in StatementRange statements, + int start, + int end, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + var selectedStatements = new List(end - start + 1); + for (int i = start; i <= end; i++) + selectedStatements.Add(statements[i]); + + if (selectedStatements.Any(ContainsAwaitOrYield)) + return false; + + if (end > start && selectedStatements.Any(static statement => statement.ContainsDirectives)) + return false; + + var localFunctions = new HashSet( + selectedStatements + .SelectMany(static statement => statement.DescendantNodesAndSelf().OfType()) + .Select(localFunction => semanticModel.GetDeclaredSymbol(localFunction, cancellationToken)) + .OfType(), + SymbolEqualityComparer.Default); + string[] labels = selectedStatements + .SelectMany(static statement => statement.DescendantNodesAndSelf().OfType()) + .Select(static label => label.Identifier.ValueText) + .Distinct() + .ToArray(); + if (localFunctions.Count == 0 && labels.Length == 0) + return true; + + for (int i = 0; i < statements.Count; i++) + { + if (i >= start && i <= end) + continue; + + StatementSyntax statement = statements[i]; + + // Moving a local function into a nested block hides it from the rest of the container. + if (ReferencesAnySymbol(statement, localFunctions, semanticModel, cancellationToken)) + return false; + + // A goto cannot jump into an unsafe block. + if (labels.Length > 0 + && statement.DescendantNodesAndSelf().OfType().Any(gotoStatement => + gotoStatement.Expression is IdentifierNameSyntax identifier + && labels.Contains(identifier.Identifier.ValueText))) + { + return false; + } + } + + return true; + } + + private static SwitchStatementSyntax? TryGetEnclosingSwitch(StatementSyntax statement) => + statement.AncestorsAndSelf() + .OfType() + .Select(static section => section.Parent) + .OfType() + .FirstOrDefault(); + + private static bool ContainsAwaitOrYield(SyntaxNode node) => + node.DescendantNodesAndSelf().Any(static descendant => + descendant is AwaitExpressionSyntax + or YieldStatementSyntax + or CommonForEachStatementSyntax { AwaitKeyword.RawKind: not 0 } + or UsingStatementSyntax { AwaitKeyword.RawKind: not 0 } + or LocalDeclarationStatementSyntax { AwaitKeyword.RawKind: not 0 }); + + private static Task ConvertExpressionBodyAsync( + Document document, + SyntaxNode root, + SemanticModel semanticModel, + ArrowExpressionClauseSyntax arrowExpression, + CancellationToken cancellationToken) + { + SyntaxNode declaration = arrowExpression.Parent!; + ExpressionSyntax expression = arrowExpression.Expression; + StatementSyntax statement; + + if (expression is ThrowExpressionSyntax throwExpression) + { + statement = SyntaxFactory.ThrowStatement(throwExpression.Expression); + } + else + { + ISymbol? symbol = declaration switch + { + MemberDeclarationSyntax member => semanticModel.GetDeclaredSymbol(member, cancellationToken), + LocalFunctionStatementSyntax localFunction => semanticModel.GetDeclaredSymbol(localFunction, cancellationToken), + AccessorDeclarationSyntax accessor => semanticModel.GetDeclaredSymbol(accessor, cancellationToken), + _ => null, + }; + ITypeSymbol? expressionType = semanticModel.GetTypeInfo(expression, cancellationToken).Type; + bool isVoidExpression = symbol is IMethodSymbol { ReturnsVoid: true } + || expressionType?.SpecialType == SpecialType.System_Void + || symbol is IMethodSymbol asyncMethod + && IsResultlessAsyncMethod(asyncMethod); + statement = isVoidExpression + ? SyntaxFactory.ExpressionStatement(expression) + : SyntaxFactory.ReturnStatement(expression); + } + + UnsafeStatementSyntax unsafeStatement = CreateUnsafeStatement(statement); + BlockSyntax body = SyntaxFactory.Block(unsafeStatement) + .WithAdditionalAnnotations(Formatter.Annotation); + + SyntaxNode replacement = declaration switch + { + MethodDeclarationSyntax method => method + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithBody(body), + ConstructorDeclarationSyntax constructor => constructor + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithBody(body), + DestructorDeclarationSyntax destructor => destructor + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithBody(body), + OperatorDeclarationSyntax @operator => @operator + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithBody(body), + ConversionOperatorDeclarationSyntax conversion => conversion + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithBody(body), + LocalFunctionStatementSyntax localFunction => localFunction + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithBody(body), + PropertyDeclarationSyntax property => property + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithAccessorList(CreateGetterAccessorList(unsafeStatement)), + IndexerDeclarationSyntax indexer => indexer + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithAccessorList(CreateGetterAccessorList(unsafeStatement)), + AccessorDeclarationSyntax accessor => accessor + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithBody(body), + _ => declaration, + }; + + return Task.FromResult(document.WithSyntaxRoot(root.ReplaceNode( + declaration, + replacement.WithAdditionalAnnotations(Formatter.Annotation)))); + } + + private static bool IsResultlessAsyncMethod(IMethodSymbol method) + { + if (!method.IsAsync) + return false; + + if (method.ReturnsVoid) + return true; + + if (method.ReturnType is INamedTypeSymbol + { + Arity: 0, + Name: "Task" or "ValueTask", + } taskType + && taskType.ContainingNamespace.ToDisplayString() == "System.Threading.Tasks") + { + return true; + } + + foreach (AttributeData attribute in method.ReturnType.GetAttributes()) + { + if (attribute.AttributeClass?.ToDisplayString() + != "System.Runtime.CompilerServices.AsyncMethodBuilderAttribute" + || attribute.ConstructorArguments.Length != 1 + || attribute.ConstructorArguments[0].Value is not INamedTypeSymbol builderType) + { + continue; + } + + return builderType.GetMembers("SetResult") + .OfType() + .Any(static setResult => setResult.Parameters.IsEmpty); + } + + return false; + } + + private static AccessorListSyntax CreateGetterAccessorList(UnsafeStatementSyntax unsafeStatement) => + SyntaxFactory.AccessorList( + SyntaxFactory.SingletonList( + SyntaxFactory.AccessorDeclaration(SyntaxKind.GetAccessorDeclaration) + .WithBody(SyntaxFactory.Block(unsafeStatement)))); + + private static UnsafeStatementSyntax CreateUnsafeStatement(StatementSyntax statement) + { + StatementSyntax innerStatement = statement + .WithoutLeadingTrivia() + .WithoutTrailingTrivia() + .WithLeadingTrivia( + SyntaxFactory.Comment(SafetyAuditComment), + SyntaxFactory.ElasticCarriageReturnLineFeed); + return SyntaxFactory.UnsafeStatement(SyntaxFactory.Block(innerStatement)) + .WithLeadingTrivia(statement.GetLeadingTrivia()) + .WithTrailingTrivia(statement.GetTrailingTrivia()) + .WithAdditionalAnnotations(Formatter.Annotation); + } + + private static Document ReplaceStatementWithStatements( + Document document, + SyntaxNode root, + StatementSyntax oldStatement, + IReadOnlyList replacements) + { + switch (oldStatement.Parent) + { + case BlockSyntax block: + { + int index = block.Statements.IndexOf(oldStatement); + SyntaxList statements = block.Statements + .RemoveAt(index) + .InsertRange(index, replacements); + return document.WithSyntaxRoot(root.ReplaceNode( + block, + block.WithStatements(statements))); + } + + case SwitchSectionSyntax switchSection: + { + int index = switchSection.Statements.IndexOf(oldStatement); + SyntaxList statements = switchSection.Statements + .RemoveAt(index) + .InsertRange(index, replacements); + return document.WithSyntaxRoot(root.ReplaceNode( + switchSection, + switchSection.WithStatements(statements))); + } + + default: + return document.WithSyntaxRoot(root.ReplaceNode( + oldStatement, + replacements.Count == 1 + ? replacements[0] + : SyntaxFactory.Block(replacements) + .WithAdditionalAnnotations(Formatter.Annotation))); + } + } + + private static Document WrapStatementRange( + Document document, + SyntaxNode root, + in StatementRange statements, + int start, + int end) + { + var statementsToWrap = new List(end - start + 1); + for (int i = start; i <= end; i++) + { + StatementSyntax statement = statements[i]; + + // Merged regions are flattened so the result is one audit scope instead of nested unsafe blocks. + if (IsGeneratedUnsafeStatement(statement)) + { + UnsafeStatementSyntax generated = (UnsafeStatementSyntax)statement; + StatementSyntax firstInnerStatement = RemoveSafetyAuditMarker(generated.Block.Statements[0]); + statementsToWrap.Add( + NormalizeTrailingEndOfLine(firstInnerStatement) + .WithLeadingTrivia(generated.GetLeadingTrivia() + .AddRange(firstInnerStatement.GetLeadingTrivia()))); + statementsToWrap.AddRange(generated.Block.Statements.Skip(1).Select(NormalizeTrailingEndOfLine)); + continue; + } + + statementsToWrap.Add(statement); + } + + StatementSyntax firstStatement = statementsToWrap[0]; + statementsToWrap[0] = firstStatement + .WithoutLeadingTrivia() + .WithLeadingTrivia( + SyntaxFactory.Comment(SafetyAuditComment), + SyntaxFactory.ElasticCarriageReturnLineFeed); + + UnsafeStatementSyntax unsafeStatement = SyntaxFactory.UnsafeStatement( + SyntaxFactory.Block(statementsToWrap)) + .WithLeadingTrivia(firstStatement.GetLeadingTrivia()) + .WithAdditionalAnnotations(Formatter.Annotation); + + SyntaxNode replacementContainer = statements.ReplaceRange(start, end, unsafeStatement); + return document.WithSyntaxRoot(root.ReplaceNode(statements.Container, replacementContainer)); + } + + /// + /// Provides uniform access to the statement list that contains a statement, including the top-level statement + /// list of a compilation unit, whose statements are wrapped in members. + /// + private readonly struct StatementRange + { + private readonly ImmutableArray _statements; + + private StatementRange(SyntaxNode container, ImmutableArray statements) + { + Container = container; + _statements = statements; + } + + internal SyntaxNode Container { get; } + + internal int Count => _statements.Length; + + internal StatementSyntax this[int index] => _statements[index]; + + internal bool IsTopLevel => Container is CompilationUnitSyntax; + + internal static bool TryCreateForStatement( + StatementSyntax statement, + out StatementRange statements, + out int statementIndex) + { + switch (statement.Parent) + { + case BlockSyntax block: + statements = new StatementRange(block, [.. block.Statements]); + break; + + case SwitchSectionSyntax switchSection: + statements = new StatementRange(switchSection, [.. switchSection.Statements]); + break; + + case GlobalStatementSyntax { Parent: CompilationUnitSyntax compilationUnit }: + statements = new StatementRange( + compilationUnit, + [.. compilationUnit.Members.OfType().Select(static global => global.Statement)]); + break; + + default: + statements = default; + statementIndex = -1; + return false; + } + + statementIndex = statements._statements.IndexOf(statement); + return statementIndex >= 0; + } + + /// + /// Replaces the statements in [start, end] with a single statement. + /// + internal SyntaxNode ReplaceRange(int start, int end, StatementSyntax replacement) + { + if (Container is CompilationUnitSyntax compilationUnit) + { + SyntaxList members = compilationUnit.Members; + GlobalStatementSyntax[] globalStatements = members.OfType().ToArray(); + + // Resolve every member index before editing: removing a member rebuilds the list, and top-level + // statements are not guaranteed to be adjacent members. + int[] memberIndices = new int[end - start + 1]; + for (int i = start; i <= end; i++) + memberIndices[i - start] = members.IndexOf(globalStatements[i]); + + for (int i = memberIndices.Length - 1; i >= 0; i--) + members = members.RemoveAt(memberIndices[i]); + + return compilationUnit.WithMembers( + members.Insert(memberIndices[0], SyntaxFactory.GlobalStatement(replacement))); + } + + var replacementStatements = new List(Count - (end - start)); + for (int i = 0; i < Count; i++) + { + if (i == start) + replacementStatements.Add(replacement); + if (i < start || i > end) + replacementStatements.Add(_statements[i]); + } + + SyntaxList statements = SyntaxFactory.List(replacementStatements); + return Container switch + { + BlockSyntax block => block.WithStatements(statements), + SwitchSectionSyntax switchSection => switchSection.WithStatements(statements), + _ => Container, + }; + } + } + } +} +#endif diff --git a/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj b/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj index 441acde0ff83a8..1556cc90ef1c8d 100644 --- a/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj +++ b/src/tools/illink/src/ILLink.CodeFix/ILLink.CodeFixProvider.csproj @@ -14,6 +14,7 @@ + diff --git a/src/tools/illink/src/ILLink.CodeFix/RequiresUnsafeCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/RequiresUnsafeCodeFixProvider.cs deleted file mode 100644 index fd9c7d22ad55cd..00000000000000 --- a/src/tools/illink/src/ILLink.CodeFix/RequiresUnsafeCodeFixProvider.cs +++ /dev/null @@ -1,574 +0,0 @@ -// 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.Collections.Immutable; -using System.Composition; -using System.Linq; -using System.Threading; -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; -using Microsoft.CodeAnalysis.Editing; - -namespace ILLink.CodeFix -{ - [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(RequiresUnsafeCodeFixProvider)), Shared] - public sealed class RequiresUnsafeCodeFixProvider : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider - { - private const string WrapInUnsafeBlockTitle = "Wrap in unsafe block"; - - public const string UnsafeMemberOperationDiagnosticId = "CS9362"; - - public sealed override ImmutableArray FixableDiagnosticIds => [UnsafeMemberOperationDiagnosticId]; - - private static LocalizableString CodeFixTitle => new LocalizableResourceString(nameof(Resources.RequiresUnsafeCodeFixTitle), Resources.ResourceManager, typeof(Resources)); - - public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) - { - var document = context.Document; - var diagnostic = context.Diagnostics.First(); - var codeFixTitle = CodeFixTitle.ToString(); - - if (await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root) - return; - - SyntaxNode targetNode = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); - - // Register "add unsafe modifier" code fix - var unsafeModifierTarget = GetUnsafeModifierTarget(targetNode); - if (unsafeModifierTarget is not null && !HasUnsafeModifier(unsafeModifierTarget)) - { - context.RegisterCodeFix(CodeAction.Create( - title: codeFixTitle, - createChangedDocument: ct => AddUnsafeModifierAsync(document, unsafeModifierTarget, ct), - equivalenceKey: codeFixTitle), diagnostic); - } - - // Register the "wrap in unsafe block" code fix - - // Find the statement containing the unsafe call - var containingStatement = targetNode.AncestorsAndSelf().OfType().FirstOrDefault(); - - // Check if this is a local function with expression body - treat it like expression-bodied member - if (containingStatement is LocalFunctionStatementSyntax localFunc && localFunc.ExpressionBody != null) - { - if (!HasDirectiveTrivia(localFunc.ExpressionBody)) - { - context.RegisterCodeFix(CodeAction.Create( - title: WrapInUnsafeBlockTitle, - createChangedDocument: ct => ConvertExpressionBodyToUnsafeBlockAsync(document, localFunc.ExpressionBody, ct), - equivalenceKey: WrapInUnsafeBlockTitle), diagnostic); - } - return; - } - - if (containingStatement is null || containingStatement is BlockSyntax) - { - // Try expression-bodied member - var arrowExpr = targetNode.AncestorsAndSelf().OfType().FirstOrDefault(); - if (arrowExpr != null && !HasDirectiveTrivia(arrowExpr)) - { - context.RegisterCodeFix(CodeAction.Create( - title: WrapInUnsafeBlockTitle, - createChangedDocument: ct => ConvertExpressionBodyToUnsafeBlockAsync(document, arrowExpr, ct), - equivalenceKey: WrapInUnsafeBlockTitle), diagnostic); - } - return; - } - - // Find the parent block containing this statement - var parentBlock = containingStatement.Parent as BlockSyntax; - if (parentBlock != null) - { - context.RegisterCodeFix(CodeAction.Create( - title: WrapInUnsafeBlockTitle, - createChangedDocument: ct => WrapStatementsInUnsafeBlockAsync(document, parentBlock, containingStatement, ct), - equivalenceKey: WrapInUnsafeBlockTitle), diagnostic); - return; - } - - // Handle switch case sections - var switchSection = containingStatement.Parent as SwitchSectionSyntax; - if (switchSection != null) - { - context.RegisterCodeFix(CodeAction.Create( - title: WrapInUnsafeBlockTitle, - createChangedDocument: ct => WrapSwitchSectionStatementInUnsafeBlockAsync(document, switchSection, containingStatement, ct), - equivalenceKey: WrapInUnsafeBlockTitle), diagnostic); - return; - } - - // Handle embedded statements (if/else/while/for without braces) - if (IsEmbeddedStatement(containingStatement)) - { - context.RegisterCodeFix(CodeAction.Create( - title: WrapInUnsafeBlockTitle, - createChangedDocument: ct => WrapEmbeddedStatementInUnsafeBlockAsync(document, containingStatement, ct), - equivalenceKey: WrapInUnsafeBlockTitle), diagnostic); - return; - } - } - - private static bool IsEmbeddedStatement(StatementSyntax statement) - { - // An embedded statement is a statement that is the direct child of a control flow statement - // without being wrapped in a block (e.g., "if (x) return;" instead of "if (x) { return; }") - return statement.Parent is IfStatementSyntax - || statement.Parent is ElseClauseSyntax - || statement.Parent is WhileStatementSyntax - || statement.Parent is ForStatementSyntax - || statement.Parent is ForEachStatementSyntax - || statement.Parent is DoStatementSyntax - || statement.Parent is UsingStatementSyntax - || statement.Parent is LockStatementSyntax - || statement.Parent is FixedStatementSyntax; - } - - private static async Task AddUnsafeModifierAsync( - Document document, - SyntaxNode declaration, - CancellationToken cancellationToken) - { - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - var modifiers = editor.Generator.GetModifiers(declaration); - editor.SetModifiers(declaration, modifiers.WithIsUnsafe(true)); - return editor.GetChangedDocument(); - } - - private static SyntaxNode? GetUnsafeModifierTarget(SyntaxNode targetNode) - => targetNode.AncestorsAndSelf().FirstOrDefault(static node => node is MethodDeclarationSyntax - or ConstructorDeclarationSyntax - or DestructorDeclarationSyntax - or LocalFunctionStatementSyntax - or PropertyDeclarationSyntax - or IndexerDeclarationSyntax - or AccessorDeclarationSyntax); - - private static bool HasUnsafeModifier(SyntaxNode declaration) - => declaration switch - { - MethodDeclarationSyntax method => method.Modifiers.Any(SyntaxKind.UnsafeKeyword), - ConstructorDeclarationSyntax constructor => constructor.Modifiers.Any(SyntaxKind.UnsafeKeyword), - DestructorDeclarationSyntax destructor => destructor.Modifiers.Any(SyntaxKind.UnsafeKeyword), - LocalFunctionStatementSyntax localFunction => localFunction.Modifiers.Any(SyntaxKind.UnsafeKeyword), - PropertyDeclarationSyntax property => property.Modifiers.Any(SyntaxKind.UnsafeKeyword), - IndexerDeclarationSyntax indexer => indexer.Modifiers.Any(SyntaxKind.UnsafeKeyword), - AccessorDeclarationSyntax accessor => accessor.Modifiers.Any(SyntaxKind.UnsafeKeyword), - _ => false - }; - - private static async Task WrapStatementsInUnsafeBlockAsync( - Document document, - BlockSyntax parentBlock, - StatementSyntax triggerStatement, - CancellationToken cancellationToken) - { - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - if (semanticModel == null) - return document; - - // Find the trigger statement in the block - var statements = parentBlock.Statements; - int triggerIndex = statements.IndexOf(triggerStatement); - if (triggerIndex < 0) - return document; - - // Check if any statement has directive trivia that would be lost or mangled - var leadingTrivia = triggerStatement.GetLeadingTrivia(); - if (leadingTrivia.Any(t => t.IsDirective)) - { - // Skip the fix - directives in statement trivia would be lost or malformed - return document; - } - - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - - // Create the TODO comment - var todoComment = SyntaxFactory.Comment("// TODO(unsafe): Baselining unsafe usage"); - var newLine = SyntaxFactory.ElasticCarriageReturnLineFeed; - - // Check if we can use forward declaration strategy - // This applies when the trigger is a local declaration with a single variable - // Ref locals (including ref readonly and scoped ref) cannot be forward-declared - LocalDeclarationStatementSyntax? forwardDecl = null; - StatementSyntax statementToWrap = triggerStatement; - int endIndex = triggerIndex; // For block expansion when forward decl not possible - - bool isRefOrScopedLocal = triggerStatement is LocalDeclarationStatementSyntax localDeclCheck && - (localDeclCheck.Declaration.Type is RefTypeSyntax || localDeclCheck.Declaration.Type is ScopedTypeSyntax); - - if (triggerStatement is LocalDeclarationStatementSyntax localDecl && - !localDecl.IsConst && - !isRefOrScopedLocal && - localDecl.Declaration.Variables.Count == 1 && - localDecl.Declaration.Variables[0].Initializer != null) - { - var variable = localDecl.Declaration.Variables[0]; - TypeSyntax? typeSyntax = localDecl.Declaration.Type; - - // If using 'var', resolve to explicit type - if (typeSyntax.IsVar) - { - var typeInfo = semanticModel.GetTypeInfo(typeSyntax, cancellationToken); - if (typeInfo.Type is not null and not IErrorTypeSymbol) - { - typeSyntax = SyntaxFactory.ParseTypeName(typeInfo.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat)) - .WithTrailingTrivia(SyntaxFactory.Space); - } - else - { - // Can't resolve type, fall back to wrapping the whole declaration - typeSyntax = null; - } - } - - if (typeSyntax != null) - { - // Create forward declaration: Type varName; - var forwardDeclVariable = SyntaxFactory.VariableDeclarator(variable.Identifier); - var forwardDeclDeclaration = SyntaxFactory.VariableDeclaration(typeSyntax) - .AddVariables(forwardDeclVariable); - forwardDecl = SyntaxFactory.LocalDeclarationStatement(forwardDeclDeclaration) - .WithLeadingTrivia(triggerStatement.GetLeadingTrivia()) - .WithTrailingTrivia(SyntaxFactory.TriviaList(SyntaxFactory.ElasticCarriageReturnLineFeed)); - - // Create assignment: varName = initializer; - var assignment = SyntaxFactory.AssignmentExpression( - SyntaxKind.SimpleAssignmentExpression, - SyntaxFactory.IdentifierName(variable.Identifier), - variable.Initializer!.Value); - statementToWrap = SyntaxFactory.ExpressionStatement(assignment) - .WithTrailingTrivia(SyntaxFactory.TriviaList(SyntaxFactory.ElasticCarriageReturnLineFeed)); - } - } - else if (isRefOrScopedLocal) - { - // For ref/scoped locals, we must expand the block until no variables - // declared inside are used outside. This handles chains of dependencies - // where ref locals lead to regular locals that are also used later. - var localDeclStmt = (LocalDeclarationStatementSyntax)triggerStatement; - if (localDeclStmt.Declaration.Variables.Count == 1) - { - // Iteratively expand until no variables declared inside escape outside - bool expanded = true; - while (expanded && endIndex < statements.Count - 1) - { - expanded = false; - - // Collect all variables declared in the current range (using symbols for correct scoping) - var declaredVariableSymbols = new HashSet(SymbolEqualityComparer.Default); - for (int i = triggerIndex; i <= endIndex; i++) - { - if (statements[i] is LocalDeclarationStatementSyntax rangeLocalDecl) - { - foreach (var variable in rangeLocalDecl.Declaration.Variables) - { - var symbol = semanticModel.GetDeclaredSymbol(variable, cancellationToken); - if (symbol is not null) - declaredVariableSymbols.Add(symbol); - } - } - } - - // Check ALL remaining statements - does any use a declared variable? - for (int nextIndex = endIndex + 1; nextIndex < statements.Count; nextIndex++) - { - var stmt = statements[nextIndex]; - - bool usesAnyDeclaredVariable = stmt.DescendantNodes() - .OfType() - .Any(id => - { - var symbolInfo = semanticModel.GetSymbolInfo(id, cancellationToken); - return symbolInfo.Symbol is not null && declaredVariableSymbols.Contains(symbolInfo.Symbol); - }); - - if (usesAnyDeclaredVariable) - { - // Check for directive trivia that would be mangled - if (stmt.GetLeadingTrivia().Any(t => t.IsDirective)) - { - // Stop expansion here - don't include statements with directives - break; - } - - // Expand to include this statement - endIndex = nextIndex; - expanded = true; - break; // Restart the outer loop to recollect variables - } - } - } - } - } - - // Build the statements to wrap - List statementsToWrap; - if (forwardDecl != null) - { - statementsToWrap = new List { statementToWrap }; - } - else - { - statementsToWrap = statements.Skip(triggerIndex).Take(endIndex - triggerIndex + 1).ToList(); - } - - // Create the unsafe block - var wrappedStatements = statementsToWrap.Select(s => - s.WithoutTrivia() - .WithTrailingTrivia(SyntaxFactory.TriviaList(SyntaxFactory.ElasticCarriageReturnLineFeed))); - var unsafeBlock = SyntaxFactory.UnsafeStatement( - SyntaxFactory.Block(wrappedStatements)) - .WithLeadingTrivia(forwardDecl != null - ? SyntaxFactory.TriviaList(todoComment, newLine) - : triggerStatement.GetLeadingTrivia().InsertRange(0, new[] { todoComment, newLine })) - .WithTrailingTrivia(forwardDecl != null - ? triggerStatement.GetTrailingTrivia() - : statementsToWrap.Last().GetTrailingTrivia()); - - // Build the new list of statements - var newStatements = new List(); - for (int i = 0; i < statements.Count; i++) - { - if (i == triggerIndex) - { - if (forwardDecl != null) - { - newStatements.Add(forwardDecl); - } - newStatements.Add(unsafeBlock); - } - else if (i > triggerIndex && i <= endIndex) - { - // Skip - already included in unsafe block - } - else - { - newStatements.Add(statements[i]); - } - } - - var newBlock = parentBlock.WithStatements(SyntaxFactory.List(newStatements)); - editor.ReplaceNode(parentBlock, newBlock); - - return editor.GetChangedDocument(); - } - - private static async Task WrapSwitchSectionStatementInUnsafeBlockAsync( - Document document, - SwitchSectionSyntax _, - StatementSyntax triggerStatement, - CancellationToken cancellationToken) - { - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - - // Create the TODO comment - var todoComment = SyntaxFactory.Comment("// TODO(unsafe): Baselining unsafe usage"); - var newLine = SyntaxFactory.CarriageReturnLineFeed; - - // Create the unsafe block wrapping just the trigger statement - var unsafeBlock = SyntaxFactory.UnsafeStatement( - SyntaxFactory.Block(triggerStatement.WithoutTrivia())) - .WithLeadingTrivia(triggerStatement.GetLeadingTrivia().InsertRange(0, new[] { todoComment, newLine })) - .WithTrailingTrivia(triggerStatement.GetTrailingTrivia()); - - // Replace the trigger statement with the unsafe block - editor.ReplaceNode(triggerStatement, unsafeBlock); - - return editor.GetChangedDocument(); - } - - private static async Task WrapEmbeddedStatementInUnsafeBlockAsync( - Document document, - StatementSyntax embeddedStatement, - CancellationToken cancellationToken) - { - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - - // Create the TODO comment - var todoComment = SyntaxFactory.Comment("// TODO(unsafe): Baselining unsafe usage"); - var newLine = SyntaxFactory.CarriageReturnLineFeed; - - // Create the unsafe block wrapping the statement - var unsafeBlock = SyntaxFactory.UnsafeStatement( - SyntaxFactory.Block(embeddedStatement.WithoutTrivia())) - .WithLeadingTrivia(todoComment, newLine); - - // Wrap in a block to replace the embedded statement - var wrappingBlock = SyntaxFactory.Block(unsafeBlock) - .WithLeadingTrivia(embeddedStatement.GetLeadingTrivia()) - .WithTrailingTrivia(embeddedStatement.GetTrailingTrivia()); - - editor.ReplaceNode(embeddedStatement, wrappingBlock); - - return editor.GetChangedDocument(); - } - - private static async Task ConvertExpressionBodyToUnsafeBlockAsync( - Document document, - ArrowExpressionClauseSyntax arrowExpr, - CancellationToken cancellationToken) - { - var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - - // Create the TODO comment - var todoComment = SyntaxFactory.Comment("// TODO(unsafe): Baselining unsafe usage"); - var newLine = SyntaxFactory.CarriageReturnLineFeed; - - // Get the parent member to determine the return type - var parent = arrowExpr.Parent; - bool isVoid = false; - - if (parent is MethodDeclarationSyntax method) - { - isVoid = method.ReturnType is PredefinedTypeSyntax pts && pts.Keyword.IsKind(SyntaxKind.VoidKeyword); - } - else if (parent is LocalFunctionStatementSyntax localFunc) - { - isVoid = localFunc.ReturnType is PredefinedTypeSyntax pts && pts.Keyword.IsKind(SyntaxKind.VoidKeyword); - } - else if (parent is DestructorDeclarationSyntax) - { - isVoid = true; - } - else if (parent is AccessorDeclarationSyntax or PropertyDeclarationSyntax or IndexerDeclarationSyntax) - { - isVoid = false; - } - - // Create the statement that goes inside the unsafe block - StatementSyntax innerStatement = isVoid - ? SyntaxFactory.ExpressionStatement(arrowExpr.Expression.WithoutTrivia()) - : SyntaxFactory.ReturnStatement(arrowExpr.Expression.WithoutTrivia()); - - // Create the unsafe block - var unsafeBlock = SyntaxFactory.UnsafeStatement( - SyntaxFactory.Block(innerStatement)) - .WithLeadingTrivia(todoComment, newLine); - - // Create the block body - var blockBody = SyntaxFactory.Block(unsafeBlock); - - // Replace based on parent type - switch (parent) - { - case MethodDeclarationSyntax methodDecl: - var newMethod = methodDecl - .WithExpressionBody(null) - .WithSemicolonToken(default) - .WithBody(blockBody); - editor.ReplaceNode(methodDecl, newMethod); - break; - - case LocalFunctionStatementSyntax localFunc: - var newLocalFunc = localFunc - .WithExpressionBody(null) - .WithSemicolonToken(default) - .WithBody(blockBody); - editor.ReplaceNode(localFunc, newLocalFunc); - break; - - case DestructorDeclarationSyntax destructorDecl: - var newDestructor = destructorDecl - .WithExpressionBody(null) - .WithSemicolonToken(default) - .WithBody(blockBody); - editor.ReplaceNode(destructorDecl, newDestructor); - break; - - case PropertyDeclarationSyntax propDecl: - var getter = SyntaxFactory.AccessorDeclaration(SyntaxKind.GetAccessorDeclaration) - .WithBody(SyntaxFactory.Block(unsafeBlock)); - var accessorList = SyntaxFactory.AccessorList(SyntaxFactory.SingletonList(getter)); - var newProp = propDecl - .WithExpressionBody(null) - .WithSemicolonToken(default) - .WithAccessorList(accessorList); - editor.ReplaceNode(propDecl, newProp); - break; - - case AccessorDeclarationSyntax accessor: - var newAccessor = accessor - .WithExpressionBody(null) - .WithSemicolonToken(default) - .WithBody(SyntaxFactory.Block(unsafeBlock)); - editor.ReplaceNode(accessor, newAccessor); - break; - - case IndexerDeclarationSyntax indexerDecl: - var indexerGetter = SyntaxFactory.AccessorDeclaration(SyntaxKind.GetAccessorDeclaration) - .WithBody(SyntaxFactory.Block(unsafeBlock)); - var indexerAccessorList = SyntaxFactory.AccessorList(SyntaxFactory.SingletonList(indexerGetter)); - var newIndexer = indexerDecl - .WithExpressionBody(null) - .WithSemicolonToken(default) - .WithAccessorList(indexerAccessorList); - editor.ReplaceNode(indexerDecl, newIndexer); - break; - } - - return editor.GetChangedDocument(); - } - - /// - /// Checks if the arrow expression clause or its expression has preprocessor directive trivia. - /// Converting expression bodies with directives to block bodies is error-prone, so we skip the fix. - /// - private static bool HasDirectiveTrivia(ArrowExpressionClauseSyntax arrowExpr) - { - // Check the arrow expression clause's leading trivia (e.g., #if or #else before =>) - if (arrowExpr.GetLeadingTrivia().Any(t => t.IsDirective)) - return true; - - // Check the arrow token's trailing trivia (e.g., #if right after =>) - if (arrowExpr.ArrowToken.TrailingTrivia.Any(t => t.IsDirective)) - return true; - - // Check the expression's leading trivia (e.g., #if before the expression) - if (arrowExpr.Expression.GetLeadingTrivia().Any(t => t.IsDirective)) - return true; - - // Check the expression's trailing trivia (e.g., #endif after the expression) - if (arrowExpr.Expression.GetTrailingTrivia().Any(t => t.IsDirective)) - return true; - - // Check the arrow expression clause's trailing trivia - if (arrowExpr.GetTrailingTrivia().Any(t => t.IsDirective)) - return true; - - // Check the parent member for directives between the signature and the arrow - // e.g., method() #if FOO => expr1; #else => expr2; #endif - if (arrowExpr.Parent is MethodDeclarationSyntax method) - { - // Check trivia after the parameter list (where #if might appear) - if (method.ParameterList.GetTrailingTrivia().Any(t => t.IsDirective)) - return true; - // Check constraint clauses if present - foreach (var constraint in method.ConstraintClauses) - { - if (constraint.GetTrailingTrivia().Any(t => t.IsDirective)) - return true; - } - } - else if (arrowExpr.Parent is LocalFunctionStatementSyntax localFunc) - { - if (localFunc.ParameterList.GetTrailingTrivia().Any(t => t.IsDirective)) - return true; - } - else if (arrowExpr.Parent is PropertyDeclarationSyntax prop) - { - if (prop.Identifier.TrailingTrivia.Any(t => t.IsDirective)) - return true; - } - - return false; - } - } -} -#endif diff --git a/src/tools/illink/src/ILLink.CodeFix/Resources.resx b/src/tools/illink/src/ILLink.CodeFix/Resources.resx index 99b54822b57a9f..70bf39f119c41d 100644 --- a/src/tools/illink/src/ILLink.CodeFix/Resources.resx +++ b/src/tools/illink/src/ILLink.CodeFix/Resources.resx @@ -126,8 +126,8 @@ Add RequiresDynamicCode attribute to parent method - - Add 'unsafe' to parent method + + Add unsafe context Mark extern member 'unsafe' @@ -144,6 +144,12 @@ Mark field-like member 'unsafe' + + Propagate unsafe contract + + + Propagate unsafe contract (discards explicit 'safe') + Add UnconditionalSuppressMessage attribute to parent method diff --git a/src/tools/illink/src/ILLink.CodeFix/SynchronizeUnsafeContractCodeFixProvider.cs b/src/tools/illink/src/ILLink.CodeFix/SynchronizeUnsafeContractCodeFixProvider.cs new file mode 100644 index 00000000000000..a14303fd69b921 --- /dev/null +++ b/src/tools/illink/src/ILLink.CodeFix/SynchronizeUnsafeContractCodeFixProvider.cs @@ -0,0 +1,739 @@ +// 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.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using ILLink.CodeFixProvider; +using ILLink.RoslynAnalyzer; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Text; + +namespace ILLink.CodeFix +{ + /// + /// Propagates intentional caller-unsafe contracts through partials, overrides, and interface implementations. + /// + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(SynchronizeUnsafeContractCodeFixProvider)), Shared] + public sealed class SynchronizeUnsafeContractCodeFixProvider : Microsoft.CodeAnalysis.CodeFixes.CodeFixProvider + { + public const string UnsafeOverrideDiagnosticId = "CS9364"; + public const string UnsafeImplicitImplementationDiagnosticId = "CS9365"; + public const string UnsafeExplicitImplementationDiagnosticId = "CS9366"; + public const string PartialUnsafeMismatchDiagnosticId = "CS0764"; + public const string PartialSafeMismatchDiagnosticId = "CS9390"; + + private static LocalizableString UnsafeCodeFixTitle => + new LocalizableResourceString( + nameof(Resources.SynchronizeUnsafeContractCodeFixTitle), + Resources.ResourceManager, + typeof(Resources)); + + private static LocalizableString DiscardSafeCodeFixTitle => + new LocalizableResourceString( + nameof(Resources.SynchronizeUnsafeContractDiscardingSafeCodeFixTitle), + Resources.ResourceManager, + typeof(Resources)); + + public override ImmutableArray FixableDiagnosticIds => + [ + UnsafeOverrideDiagnosticId, + UnsafeImplicitImplementationDiagnosticId, + UnsafeExplicitImplementationDiagnosticId, + PartialUnsafeMismatchDiagnosticId, + PartialSafeMismatchDiagnosticId, + ]; + + public override FixAllProvider GetFixAllProvider() => ContractFixAllProvider.Instance; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + Diagnostic diagnostic = context.Diagnostics[0]; + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null || semanticModel is null) + return; + + SyntaxNode targetNode = root.FindNode( + diagnostic.Location.SourceSpan, + getInnermostNodeForTie: true); + if (targetNode.AncestorsAndSelf().OfType() + .FirstOrDefault(static variable => + variable.Parent?.Parent is EventFieldDeclarationSyntax) is { } eventVariable + && eventVariable.Parent?.Parent is EventFieldDeclarationSyntax + { + Declaration.Variables.Count: > 1, + } eventField + && eventField.ContainsDirectives) + { + return; + } + + if (UnsafeModifierCodeFixHelpers.FindDeclaration(targetNode) is not { } declaration + || GetDeclaredContractSymbol( + semanticModel, + targetNode, + declaration, + context.CancellationToken) is not { } declaredSymbol) + { + return; + } + + ISymbol symbol = GetContractSymbolForDiagnostic( + diagnostic, + declaredSymbol, + context.CancellationToken); + if (!IsSupportedContractSymbol(symbol)) + return; + + HashSet closure = GetContractClosure(symbol, context.CancellationToken); + Solution solution = context.Document.Project.Solution; + if (!CanPropagateUnsafeContract( + solution, + closure, + context.CancellationToken)) + { + return; + } + + // Replacing an explicit safe contract discards a deliberate audit, so it is offered under its own title. + string title = DiscardsExplicitSafeContract(solution, closure, context.CancellationToken) + ? DiscardSafeCodeFixTitle.ToString() + : UnsafeCodeFixTitle.ToString(); + context.RegisterCodeFix( + CodeAction.Create( + title, + cancellationToken => ApplyModifierAsync( + solution, + closure, + cancellationToken), + title), + context.Diagnostics); + } + + private sealed class ContractFixAllProvider : FixAllProvider + { + internal static ContractFixAllProvider Instance { get; } = new(); + + public override async Task GetFixAsync(FixAllContext fixAllContext) + { + ImmutableArray diagnostics = + await GetDiagnosticsInScopeAsync(fixAllContext).ConfigureAwait(false); + if (diagnostics.IsEmpty) + return null; + + string title = UnsafeCodeFixTitle.ToString(); + return CodeAction.Create( + title, + cancellationToken => PropagateDiagnosticsAsync( + fixAllContext.Solution, + diagnostics, + cancellationToken), + fixAllContext.CodeActionEquivalenceKey ?? title); + } + + private static async Task> GetDiagnosticsInScopeAsync( + FixAllContext context) => + context.Scope switch + { + FixAllScope.Document => await context + .GetDocumentDiagnosticsAsync(context.Document!) + .ConfigureAwait(false), + FixAllScope.Project => await context + .GetAllDiagnosticsAsync(context.Project!) + .ConfigureAwait(false), + FixAllScope.Solution => ImmutableArray.CreateRange( + (await Task.WhenAll( + context.Solution.Projects.Select(context.GetAllDiagnosticsAsync)) + .ConfigureAwait(false)) + .SelectMany(static diagnostics => diagnostics)), + _ => [], + }; + } + + private static async Task PropagateDiagnosticsAsync( + Solution solution, + ImmutableArray diagnostics, + CancellationToken cancellationToken) + { + var closure = new HashSet(SymbolEqualityComparer.Default); + foreach (Diagnostic diagnostic in diagnostics) + { + if (await GetDiagnosticSymbolAsync( + solution, + diagnostic, + cancellationToken).ConfigureAwait(false) is not { } symbol) + { + continue; + } + + HashSet diagnosticClosure = GetContractClosure(symbol, cancellationToken); + if (CanPropagateUnsafeContract( + solution, + diagnosticClosure, + cancellationToken)) + { + closure.UnionWith(diagnosticClosure); + } + } + + return await ApplyModifierAsync( + solution, + closure, + cancellationToken).ConfigureAwait(false); + } + + private static async Task GetDiagnosticSymbolAsync( + Solution solution, + Diagnostic diagnostic, + CancellationToken cancellationToken) + { + Document? document = solution.GetDocument(diagnostic.Location.SourceTree); + if (document is null) + return null; + + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + if (root is null || semanticModel is null) + return null; + + SyntaxNode targetNode = root.FindNode( + diagnostic.Location.SourceSpan, + getInnermostNodeForTie: true); + if (targetNode.AncestorsAndSelf().OfType() + .FirstOrDefault(static variable => + variable.Parent?.Parent is EventFieldDeclarationSyntax) is { } eventVariable + && eventVariable.Parent?.Parent is EventFieldDeclarationSyntax + { + Declaration.Variables.Count: > 1, + } eventField + && eventField.ContainsDirectives) + { + return null; + } + + if (UnsafeModifierCodeFixHelpers.FindDeclaration(targetNode) is not { } declaration + || GetDeclaredContractSymbol( + semanticModel, + targetNode, + declaration, + cancellationToken) is not { } declaredSymbol) + { + return null; + } + + ISymbol symbol = GetContractSymbolForDiagnostic( + diagnostic, + declaredSymbol, + cancellationToken); + return IsSupportedContractSymbol(symbol) ? symbol : null; + } + + private static ISymbol GetContractSymbolForDiagnostic( + Diagnostic diagnostic, + ISymbol declaredSymbol, + CancellationToken cancellationToken) + { + ISymbol symbol = UnsafeContractHelpers.NormalizeContractSymbol(declaredSymbol); + if (diagnostic.Id is not (PartialUnsafeMismatchDiagnosticId or PartialSafeMismatchDiagnosticId) + || symbol is not IPropertySymbol property) + { + return symbol; + } + + IPropertySymbol[] propertyParts = UnsafeContractHelpers.GetPartialParts(property) + .OfType() + .ToArray(); + if (propertyParts.Length < 2) + return symbol; + + if (HasMismatch(propertyParts, HasPropertyUnsafe)) + return symbol; + + if (property.GetMethod is not null + && HasMismatch(propertyParts, propertyPart => + HasAccessorUnsafe(propertyPart, MethodKind.PropertyGet))) + { + return property.GetMethod; + } + + if (property.SetMethod is not null + && HasMismatch(propertyParts, propertyPart => + HasAccessorUnsafe(propertyPart, MethodKind.PropertySet))) + { + return property.SetMethod; + } + + return symbol; + + bool HasPropertyUnsafe(IPropertySymbol propertyPart) => + UnsafeContractHelpers.GetDirectDeclarations(propertyPart, cancellationToken) + .Any(declaration => + UnsafeMigrationSyntaxHelpers.HasModifier( + declaration, + SyntaxKind.UnsafeKeyword)); + + bool HasAccessorUnsafe(IPropertySymbol propertyPart, MethodKind accessorKind) + { + IMethodSymbol? accessor = accessorKind == MethodKind.PropertyGet + ? propertyPart.GetMethod + : propertyPart.SetMethod; + return accessor is not null + && UnsafeContractHelpers.GetDirectDeclarations(accessor, cancellationToken) + .Any(declaration => + declaration is AccessorDeclarationSyntax + && UnsafeMigrationSyntaxHelpers.HasModifier( + declaration, + SyntaxKind.UnsafeKeyword)); + } + + static bool HasMismatch( + IEnumerable parts, + System.Func hasModifier) + { + bool? first = null; + foreach (IPropertySymbol part in parts) + { + bool current = hasModifier(part); + if (first is null) + { + first = current; + } + else if (first.Value != current) + { + return true; + } + } + + return false; + } + } + + private static ISymbol? GetDeclaredContractSymbol( + SemanticModel semanticModel, + SyntaxNode targetNode, + SyntaxNode declaration, + CancellationToken cancellationToken) + { + VariableDeclaratorSyntax? eventVariable = targetNode.AncestorsAndSelf() + .OfType() + .FirstOrDefault(static variable => + variable.Parent?.Parent is EventFieldDeclarationSyntax); + return eventVariable is not null + ? semanticModel.GetDeclaredSymbol(eventVariable, cancellationToken) + : semanticModel.GetDeclaredSymbol(declaration, cancellationToken); + } + + private static bool IsSupportedContractSymbol(ISymbol symbol) => + symbol is IMethodSymbol or IPropertySymbol or IEventSymbol; + + private static bool CanPropagateUnsafeContract( + Solution solution, + IEnumerable symbols, + CancellationToken cancellationToken) + { + foreach (ISymbol symbol in symbols) + { + bool hasEditableDeclaration = false; + foreach (SyntaxNode declaration in UnsafeContractHelpers.GetDeclarations( + symbol, + cancellationToken)) + { + if (solution.GetDocument(declaration.SyntaxTree) is null) + continue; + + if (declaration.AncestorsAndSelf().OfType() + .Any(static parameter => + parameter.Parent?.Parent is RecordDeclarationSyntax)) + { + return false; + } + + if (declaration is VariableDeclaratorSyntax + { + Parent.Parent: EventFieldDeclarationSyntax eventField, + }) + { + if (eventField.Declaration.Variables.Count > 1 + && eventField.ContainsDirectives) + { + return false; + } + + hasEditableDeclaration = true; + continue; + } + + if (declaration is ArrowExpressionClauseSyntax) + { + hasEditableDeclaration = true; + continue; + } + + if (UnsafeModifierCodeFixHelpers.FindDeclaration(declaration) + is { } editableDeclaration + && editableDeclaration is not BaseTypeDeclarationSyntax) + { + hasEditableDeclaration = true; + } + } + + if (!hasEditableDeclaration) + return false; + } + + return true; + } + + /// + /// Collects the declarations that must become caller-unsafe: the partial parts of the reported member and the + /// base and interface slots it overrides or implements, transitively. + /// Overrides and implementations that are not themselves marked unsafe are deliberately left alone; the + /// language permits a safe member to override or implement a caller-unsafe one, so widening them would grow + /// the audit surface and cascade an unsafe contract onto their callers for no compiler-mandated reason. + /// + private static HashSet GetContractClosure(ISymbol seed, CancellationToken cancellationToken) + { + var closure = new HashSet(SymbolEqualityComparer.Default); + var queue = new Queue(); + + Enqueue(seed); + while (queue.Count > 0) + { + cancellationToken.ThrowIfCancellationRequested(); + ISymbol symbol = queue.Dequeue(); + + foreach (ISymbol part in UnsafeContractHelpers.GetPartialParts(symbol)) + Enqueue(part); + + if (UnsafeContractHelpers.GetOverriddenMember(symbol) is { } overriddenMember) + Enqueue(overriddenMember); + + foreach (ISymbol interfaceMember in UnsafeContractHelpers.GetImplementedInterfaceMembers(symbol)) + Enqueue(interfaceMember); + } + + return closure; + + void Enqueue(ISymbol symbol) + { + symbol = UnsafeContractHelpers.NormalizeContractSymbol(symbol); + if (closure.Add(symbol)) + queue.Enqueue(symbol); + } + } + + /// + /// Determines whether propagation would replace a deliberate safe audit with an unsafe contract. + /// + private static bool DiscardsExplicitSafeContract( + Solution solution, + IEnumerable symbols, + CancellationToken cancellationToken) + { + foreach (ISymbol symbol in symbols) + { + foreach (SyntaxNode declaration in UnsafeContractHelpers.GetDeclarations(symbol, cancellationToken)) + { + if (solution.GetDocument(declaration.SyntaxTree) is not null + && UnsafeMigrationSyntaxHelpers.HasSafeModifier(declaration)) + { + return true; + } + } + } + + return false; + } + + private static async Task ApplyModifierAsync( + Solution solution, + IEnumerable symbols, + CancellationToken cancellationToken) + { + var targetsByDocument = new Dictionary>(); + var eventTargetsByDocument = + new Dictionary>>(); + foreach (ISymbol symbol in symbols) + { + foreach (SyntaxNode declaration in UnsafeContractHelpers.GetDeclarations(symbol, cancellationToken)) + { + Document? document = solution.GetDocument(declaration.SyntaxTree); + if (document is null) + continue; + + if (declaration is VariableDeclaratorSyntax + { + Parent.Parent: EventFieldDeclarationSyntax eventField, + } eventVariable) + { + if (!eventTargetsByDocument.TryGetValue( + document.Id, + out Dictionary>? eventFields)) + { + eventFields = new Dictionary>(); + eventTargetsByDocument.Add(document.Id, eventFields); + } + + if (!eventFields.TryGetValue( + eventField.Span, + out HashSet? eventNames)) + { + eventNames = new HashSet(); + eventFields.Add(eventField.Span, eventNames); + } + + eventNames.Add(eventVariable.Identifier.ValueText); + continue; + } + + if (!targetsByDocument.TryGetValue(document.Id, out HashSet? spans)) + { + spans = new HashSet(); + targetsByDocument.Add(document.Id, spans); + } + + spans.Add(declaration.Span); + } + } + + var documentIds = new HashSet(targetsByDocument.Keys); + documentIds.UnionWith(eventTargetsByDocument.Keys); + foreach (DocumentId documentId in documentIds) + { + Document document = solution.GetDocument(documentId)!; + var targets = new List(); + if (targetsByDocument.TryGetValue(documentId, out HashSet? spans)) + { + targets.AddRange(spans.Select(static span => + new ModifierTarget(span))); + } + if (eventTargetsByDocument.TryGetValue( + documentId, + out Dictionary>? eventFields)) + { + targets.AddRange(eventFields.Select(static eventField => + new ModifierTarget(eventField.Key) + { + EventNames = eventField.Value, + })); + } + foreach (ModifierTarget target in + targets.OrderByDescending(static target => target.Span.Start)) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is null) + continue; + + SyntaxNode node = root.FindNode(target.Span, getInnermostNodeForTie: true); + if (target.EventNames is not null) + { + EventFieldDeclarationSyntax? eventField = node.AncestorsAndSelf() + .OfType() + .FirstOrDefault(); + if (eventField is not null) + { + document = await SetEventVariablesUnsafeAsync( + document, + root, + eventField, + target.EventNames, + cancellationToken).ConfigureAwait(false); + } + continue; + } + + ArrowExpressionClauseSyntax? expressionBody = node.AncestorsAndSelf() + .OfType() + .FirstOrDefault(); + if (expressionBody is not null) + { + document = SetExpressionBodiedGetterUnsafe( + document, + root, + expressionBody); + continue; + } + + if (UnsafeModifierCodeFixHelpers.FindDeclaration(node) is not { } declaration) + continue; + + document = await UnsafeModifierCodeFixHelpers.SetUnsafeModifierAsync( + document, + declaration, + cancellationToken).ConfigureAwait(false); + } + + document = await UnsafeModifierCodeFixHelpers.AddPendingSafetyDocumentationAsync( + document, + cancellationToken).ConfigureAwait(false); + solution = document.Project.Solution; + } + + return solution; + } + + private static Document SetExpressionBodiedGetterUnsafe( + Document document, + SyntaxNode root, + ArrowExpressionClauseSyntax expressionBody) + { + SyntaxToken unsafeModifier = SyntaxFactory.Token(SyntaxKind.UnsafeKeyword) + .WithTrailingTrivia(SyntaxFactory.ElasticSpace); + AccessorDeclarationSyntax getter = SyntaxFactory.AccessorDeclaration( + SyntaxKind.GetAccessorDeclaration) + .WithModifiers([unsafeModifier]) + .WithExpressionBody(expressionBody) + .WithSemicolonToken(expressionBody.Parent switch + { + PropertyDeclarationSyntax property => property.SemicolonToken, + IndexerDeclarationSyntax indexer => indexer.SemicolonToken, + _ => default, + }); + getter = (AccessorDeclarationSyntax)UnsafeModifierCodeFixHelpers.MarkForSafetyDocumentation(getter); + + SyntaxNode replacement = expressionBody.Parent switch + { + PropertyDeclarationSyntax property => property + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithAccessorList(SyntaxFactory.AccessorList( + SyntaxFactory.SingletonList(getter))), + IndexerDeclarationSyntax indexer => indexer + .WithExpressionBody(null) + .WithSemicolonToken(default) + .WithAccessorList(SyntaxFactory.AccessorList( + SyntaxFactory.SingletonList(getter))), + _ => expressionBody.Parent!, + }; + return document.WithSyntaxRoot(root.ReplaceNode( + expressionBody.Parent!, + replacement.WithAdditionalAnnotations(Formatter.Annotation))); + } + + private static async Task SetEventVariablesUnsafeAsync( + Document document, + SyntaxNode root, + EventFieldDeclarationSyntax eventField, + HashSet eventNames, + CancellationToken cancellationToken) + { + SeparatedSyntaxList variables = eventField.Declaration.Variables; + if (variables.Count > 1 && eventField.ContainsDirectives) + return document; + + if (variables.Count == 1 + || variables.All(variable => eventNames.Contains(variable.Identifier.ValueText))) + { + return await UnsafeModifierCodeFixHelpers.SetUnsafeModifierAsync( + document, + eventField, + cancellationToken).ConfigureAwait(false); + } + + if (eventField.Parent is not TypeDeclarationSyntax containingType) + return document; + + var splitEvents = new List(variables.Count); + for (int i = 0; i < variables.Count; i++) + { + VariableDeclaratorSyntax variable = variables[i]; + EventFieldDeclarationSyntax splitEvent = eventField + .WithDeclaration(eventField.Declaration.WithVariables( + SyntaxFactory.SingletonSeparatedList(variable))) + // Later declarations rely on the line break that the preceding one already carries, so that + // generated documentation does not introduce a blank line between them. + .WithLeadingTrivia(i == 0 + ? eventField.GetLeadingTrivia() + : default(SyntaxTriviaList)) + .WithTrailingTrivia(i == variables.Count - 1 + ? eventField.GetTrailingTrivia() + : variables.GetSeparator(i).LeadingTrivia + .AddRange(variables.GetSeparator(i).TrailingTrivia) + .Add(SyntaxFactory.ElasticCarriageReturnLineFeed)); + + if (eventNames.Contains(variable.Identifier.ValueText)) + { + bool wasUnsafe = UnsafeMigrationSyntaxHelpers.HasModifier(splitEvent, SyntaxKind.UnsafeKeyword); + splitEvent = SetEventFieldUnsafe(splitEvent); + if (!wasUnsafe) + { + splitEvent = (EventFieldDeclarationSyntax)UnsafeModifierCodeFixHelpers + .MarkForSafetyDocumentation(splitEvent); + } + } + + splitEvents.Add(splitEvent.WithAdditionalAnnotations(Formatter.Annotation)); + } + + int memberIndex = containingType.Members.IndexOf(eventField); + SyntaxList members = containingType.Members + .RemoveAt(memberIndex) + .InsertRange(memberIndex, splitEvents); + TypeDeclarationSyntax replacement = containingType.WithMembers(members); + return document.WithSyntaxRoot(root.ReplaceNode(containingType, replacement)); + } + + private static EventFieldDeclarationSyntax SetEventFieldUnsafe( + EventFieldDeclarationSyntax eventField) + { + SyntaxToken safeModifier = UnsafeMigrationSyntaxHelpers.GetSafeModifier(eventField); + if (safeModifier != default) + { + return eventField.ReplaceToken( + safeModifier, + SyntaxFactory.Token(SyntaxKind.UnsafeKeyword).WithTriviaFrom(safeModifier)); + } + + if (UnsafeMigrationSyntaxHelpers.HasModifier(eventField, SyntaxKind.UnsafeKeyword)) + return eventField; + + SyntaxTokenList modifiers = eventField.Modifiers; + int insertionIndex = modifiers.IndexOf(SyntaxKind.ExternKeyword); + int partialIndex = modifiers.IndexOf(SyntaxKind.PartialKeyword); + if (partialIndex >= 0 && (insertionIndex < 0 || partialIndex < insertionIndex)) + insertionIndex = partialIndex; + if (insertionIndex < 0) + insertionIndex = modifiers.Count; + + SyntaxToken unsafeModifier = SyntaxFactory.Token(SyntaxKind.UnsafeKeyword) + .WithTrailingTrivia(SyntaxFactory.ElasticSpace); + if (modifiers.Count == 0) + { + unsafeModifier = unsafeModifier.WithLeadingTrivia(eventField.EventKeyword.LeadingTrivia); + eventField = eventField.WithEventKeyword( + eventField.EventKeyword.WithLeadingTrivia(default(SyntaxTriviaList))); + } + else if (insertionIndex == 0) + { + unsafeModifier = unsafeModifier.WithLeadingTrivia(modifiers[0].LeadingTrivia); + modifiers = modifiers.Replace( + modifiers[0], + modifiers[0].WithLeadingTrivia(default(SyntaxTriviaList))); + } + + return eventField.WithModifiers(modifiers.Insert(insertionIndex, unsafeModifier)); + } + + private sealed class ModifierTarget + { + internal ModifierTarget(TextSpan span) + { + Span = span; + } + + internal TextSpan Span { get; } + internal HashSet? EventNames { get; init; } + } + } +} +#endif diff --git a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs index 2813ff19d1402d..1c29de6ab7d17f 100644 --- a/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs +++ b/src/tools/illink/src/ILLink.CodeFix/UnsafeModifierCodeFixHelpers.cs @@ -13,15 +13,21 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Text; namespace ILLink.CodeFix { /// /// 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. + /// It is shared by unsafe migration fixes for compiler diagnostics, IL5005, and IL5006. /// internal static class UnsafeModifierCodeFixHelpers { + internal const string SafetyDocumentationText = "TODO: Audit"; + + private static readonly SyntaxAnnotation s_safetyDocumentationAnnotation = + new(nameof(s_safetyDocumentationAnnotation)); + /// /// Registers an add-unsafe action for a supported declaration that has no existing safety modifier. /// @@ -96,6 +102,147 @@ internal static async Task AddUnsafeModifierAsync( return editor.GetChangedDocument(); } + /// + /// Replaces an explicit safe contract with unsafe, or adds unsafe when no safety modifier is present. + /// The declaration is annotated so that can document the + /// new caller-unsafe contract, which also keeps IL5005 from immediately removing the modifier again. + /// + internal static async Task SetUnsafeModifierAsync( + Document document, + SyntaxNode declaration, + CancellationToken cancellationToken) + { + // Only contracts that this fix introduces are documented; a modifier the developer already wrote stays + // subject to the IL5005 rules. + if (UnsafeMigrationSyntaxHelpers.GetSafeModifier(declaration) != default) + { + (document, declaration) = await AnnotateForSafetyDocumentationAsync( + document, + declaration, + cancellationToken).ConfigureAwait(false); + + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + SyntaxToken safeModifier = UnsafeMigrationSyntaxHelpers.GetSafeModifier(declaration); + if (root is null || safeModifier == default) + return document; + + SyntaxToken unsafeModifier = SyntaxFactory.Token(SyntaxKind.UnsafeKeyword) + .WithTriviaFrom(safeModifier); + return document.WithSyntaxRoot(root.ReplaceToken(safeModifier, unsafeModifier)); + } + + if (UnsafeMigrationSyntaxHelpers.HasModifier(declaration, SyntaxKind.UnsafeKeyword)) + return document; + + (document, declaration) = await AnnotateForSafetyDocumentationAsync( + document, + declaration, + cancellationToken).ConfigureAwait(false); + return await AddUnsafeModifierAsync(document, declaration, cancellationToken).ConfigureAwait(false); + } + + /// + /// Marks a declaration so that documents its new contract. + /// + internal static SyntaxNode MarkForSafetyDocumentation(SyntaxNode declaration) => + declaration.WithAdditionalAnnotations(s_safetyDocumentationAnnotation); + + /// + /// Documents every declaration that marked as caller-unsafe. + /// + internal static async Task AddPendingSafetyDocumentationAsync( + Document document, + CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is null) + return document; + + SyntaxNode[] documentationTargets = root.GetAnnotatedNodes(s_safetyDocumentationAnnotation) + .Select(GetSafetyDocumentationTarget) + .Distinct() + .Where(static target => !UnsafeMigrationSyntaxHelpers.HasSafetyDocumentation(target)) + .ToArray(); + if (documentationTargets.Length > 0) + { + root = root.ReplaceNodes( + documentationTargets, + (_, target) => AddSafetyDocumentation(target, cancellationToken)); + } + + SyntaxNode[] annotated = root.GetAnnotatedNodes(s_safetyDocumentationAnnotation).ToArray(); + if (annotated.Length > 0) + { + root = root.ReplaceNodes( + annotated, + static (_, node) => node.WithoutAnnotations(s_safetyDocumentationAnnotation)); + } + + return document.WithSyntaxRoot(root); + } + + private static async Task<(Document Document, SyntaxNode Declaration)> AnnotateForSafetyDocumentationAsync( + Document document, + SyntaxNode declaration, + CancellationToken cancellationToken) + { + if (declaration.HasAnnotation(s_safetyDocumentationAnnotation)) + return (document, declaration); + + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + if (root is null) + return (document, declaration); + + var annotation = new SyntaxAnnotation(); + SyntaxNode annotatedDeclaration = declaration + .WithAdditionalAnnotations(s_safetyDocumentationAnnotation, annotation); + document = document.WithSyntaxRoot(root.ReplaceNode(declaration, annotatedDeclaration)); + + root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + return (document, root?.GetAnnotatedNodes(annotation).FirstOrDefault() ?? annotatedDeclaration); + } + + /// + /// Finds the declaration that owns the documentation for a caller-unsafe contract. + /// Accessors cannot carry XML documentation, so their contract is documented on the containing member. + /// + private static SyntaxNode GetSafetyDocumentationTarget(SyntaxNode declaration) => + declaration is AccessorDeclarationSyntax + ? declaration.Ancestors().OfType().FirstOrDefault() ?? declaration + : declaration; + + private static SyntaxNode AddSafetyDocumentation(SyntaxNode declaration, CancellationToken cancellationToken) + { + SyntaxTriviaList leadingTrivia = declaration.GetLeadingTrivia(); + string indentation = leadingTrivia.LastOrDefault(static trivia => + trivia.IsKind(SyntaxKind.WhitespaceTrivia)).ToFullString(); + SyntaxTriviaList documentation = SyntaxFactory.ParseLeadingTrivia( + $"/// <{UnsafeMigrationSyntaxHelpers.SafetyDocumentationElement}>{SafetyDocumentationText}" + + $"" + + $"{GetEndOfLine(declaration, cancellationToken)}{indentation}"); + return declaration.WithLeadingTrivia(leadingTrivia.AddRange(documentation)); + } + + /// + /// Reuses the line ending the file already uses so generated documentation does not mix line endings. + /// + private static string GetEndOfLine(SyntaxNode declaration, CancellationToken cancellationToken) + { + SyntaxTrivia endOfLine = declaration.GetLeadingTrivia() + .FirstOrDefault(static trivia => trivia.IsKind(SyntaxKind.EndOfLineTrivia)); + if (endOfLine != default) + return endOfLine.ToFullString(); + + SourceText text = declaration.SyntaxTree.GetText(cancellationToken); + foreach (TextLine line in text.Lines) + { + if (line.EndIncludingLineBreak > line.End) + return text.ToString(TextSpan.FromBounds(line.End, line.EndIncludingLineBreak)); + } + + return SyntaxFactory.CarriageReturnLineFeed.ToFullString(); + } + /// /// Removes unsafe without disturbing modifiers that the current SyntaxGenerator does not model. /// @@ -168,8 +315,11 @@ private static AccessorDeclarationSyntax RemoveUnsafeModifier(AccessorDeclaratio private static SyntaxTokenList AddUnsafeModifier(SyntaxTokenList modifiers) { - // Place unsafe before extern while preserving the existing modifier order. + // Place unsafe before syntax-sensitive extern/partial modifiers while preserving other modifier order. int insertionIndex = modifiers.IndexOf(SyntaxKind.ExternKeyword); + int partialIndex = modifiers.IndexOf(SyntaxKind.PartialKeyword); + if (partialIndex >= 0 && (insertionIndex < 0 || partialIndex < insertionIndex)) + insertionIndex = partialIndex; if (insertionIndex < 0) insertionIndex = modifiers.Count; @@ -203,7 +353,7 @@ private static SyntaxTokenList RemoveUnsafeModifier(SyntaxTokenList modifiers) return modifiers; } - private static SyntaxNode WithModifiers(SyntaxNode declaration, SyntaxTokenList modifiers) => + internal static SyntaxNode WithModifiers(SyntaxNode declaration, SyntaxTokenList modifiers) => declaration switch { BaseTypeDeclarationSyntax type => type.WithModifiers(modifiers), @@ -221,3 +371,4 @@ private static int GetUnsafeModifierIndex(SyntaxTokenList modifiers) => } } #endif + diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeContractHelpers.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeContractHelpers.cs new file mode 100644 index 00000000000000..2a0cca88c7cf59 --- /dev/null +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeContractHelpers.cs @@ -0,0 +1,220 @@ +// 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.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; + +namespace ILLink.RoslynAnalyzer +{ + /// + /// Provides source and symbol helpers shared by unsafe-contract analyzers and code fixes. + /// + internal static class UnsafeContractHelpers + { + internal static ISymbol NormalizeContractSymbol(ISymbol symbol) => + symbol is IMethodSymbol + { + MethodKind: MethodKind.EventAdd or MethodKind.EventRemove, + AssociatedSymbol: IEventSymbol @event, + } + ? @event + : symbol; + + internal static IEnumerable GetPartialParts(ISymbol symbol) + { + symbol = NormalizeContractSymbol(symbol); + var seen = new HashSet(SymbolEqualityComparer.Default); + + foreach (ISymbol part in GetPartialPartsCore(symbol)) + { + if (seen.Add(part)) + yield return part; + } + } + + internal static IEnumerable GetDeclarations( + ISymbol symbol, + CancellationToken cancellationToken) + { + var seen = new HashSet<(SyntaxTree Tree, TextSpan Span)>(); + foreach (ISymbol part in GetPartialParts(symbol)) + { + foreach (SyntaxReference reference in part.DeclaringSyntaxReferences) + { + SyntaxNode declaration = GetDeclarationForSymbol( + part, + reference.GetSyntax(cancellationToken)); + if (seen.Add((declaration.SyntaxTree, declaration.Span))) + yield return declaration; + } + } + } + + internal static IEnumerable GetDirectDeclarations( + ISymbol symbol, + CancellationToken cancellationToken) + { + symbol = NormalizeContractSymbol(symbol); + foreach (SyntaxReference reference in symbol.DeclaringSyntaxReferences) + { + yield return GetDeclarationForSymbol( + symbol, + reference.GetSyntax(cancellationToken)); + } + } + + internal static ISymbol? GetOverriddenMember(ISymbol symbol) => + NormalizeContractSymbol(symbol) switch + { + IMethodSymbol method => method.OverriddenMethod, + IPropertySymbol property => property.OverriddenProperty, + IEventSymbol @event => @event.OverriddenEvent, + _ => null, + }; + + internal static IEnumerable GetImplementedInterfaceMembers(ISymbol symbol) + { + symbol = NormalizeContractSymbol(symbol); + INamedTypeSymbol? containingType = symbol.ContainingType; + if (containingType is null) + yield break; + + var seen = new HashSet(SymbolEqualityComparer.Default); + foreach (INamedTypeSymbol @interface in containingType.AllInterfaces) + { + foreach (ISymbol interfaceMember in GetInterfaceContractSlots(@interface)) + { + ISymbol? implementation = containingType.FindImplementationForInterfaceMember(interfaceMember); + if (implementation is not null + && ContractSymbolsMatch(implementation, symbol) + && seen.Add(interfaceMember)) + { + yield return interfaceMember; + } + } + } + } + + internal static IEnumerable GetInterfaceContractSlots(INamedTypeSymbol @interface) + { + foreach (ISymbol member in @interface.GetMembers()) + { + if (member is IMethodSymbol { AssociatedSymbol: not null }) + continue; + + if (member is IPropertySymbol property) + { + yield return property; + if (property.GetMethod is not null) + yield return property.GetMethod; + if (property.SetMethod is not null) + yield return property.SetMethod; + continue; + } + + yield return member; + } + } + + private static IEnumerable GetPartialPartsCore(ISymbol symbol) + { + yield return symbol; + + switch (symbol) + { + case IMethodSymbol { AssociatedSymbol: IPropertySymbol property } accessor: + foreach (IPropertySymbol propertyPart in GetPropertyParts(property)) + { + IMethodSymbol? accessorPart = accessor.MethodKind == MethodKind.PropertyGet + ? propertyPart.GetMethod + : propertyPart.SetMethod; + if (accessorPart is not null) + yield return accessorPart; + } + break; + + case IMethodSymbol method: + if (method.PartialDefinitionPart is { } methodDefinition) + yield return methodDefinition; + if (method.PartialImplementationPart is { } methodImplementation) + yield return methodImplementation; + break; + + case IPropertySymbol property: + foreach (IPropertySymbol propertyPart in GetPropertyParts(property)) + yield return propertyPart; + break; + + case IEventSymbol @event: + if (@event.PartialDefinitionPart is { } eventDefinition) + yield return eventDefinition; + if (@event.PartialImplementationPart is { } eventImplementation) + yield return eventImplementation; + break; + } + } + + private static IEnumerable GetPropertyParts(IPropertySymbol property) + { + yield return property; + if (property.PartialDefinitionPart is { } propertyDefinition) + yield return propertyDefinition; + if (property.PartialImplementationPart is { } propertyImplementation) + yield return propertyImplementation; + } + + private static bool ContractSymbolsMatch(ISymbol left, ISymbol right) + { + foreach (ISymbol leftPart in GetPartialParts(NormalizeContractSymbol(left))) + { + foreach (ISymbol rightPart in GetPartialParts(NormalizeContractSymbol(right))) + { + if (SymbolEqualityComparer.Default.Equals(leftPart, rightPart) + || SymbolEqualityComparer.Default.Equals( + leftPart.OriginalDefinition, + rightPart.OriginalDefinition)) + { + return true; + } + } + } + + return false; + } + + private static SyntaxNode FindDeclaration(SyntaxNode node) => + node.AncestorsAndSelf().FirstOrDefault(static ancestor => + ancestor is VariableDeclaratorSyntax + { + Parent.Parent: EventFieldDeclarationSyntax, + } + or BaseMethodDeclarationSyntax + or BasePropertyDeclarationSyntax + or EventDeclarationSyntax + or EventFieldDeclarationSyntax + or AccessorDeclarationSyntax) + ?? node; + + private static SyntaxNode GetDeclarationForSymbol(ISymbol symbol, SyntaxNode syntax) + { + SyntaxNode declaration = FindDeclaration(syntax); + if (symbol is IMethodSymbol { MethodKind: MethodKind.PropertyGet }) + { + return declaration switch + { + PropertyDeclarationSyntax { ExpressionBody: { } expressionBody } => expressionBody, + IndexerDeclarationSyntax { ExpressionBody: { } expressionBody } => expressionBody, + _ => declaration, + }; + } + + return declaration; + } + } +} +#endif diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs index 144e9fd98942f6..d81219354cfc5f 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationAnalyzerHelpers.cs @@ -188,11 +188,7 @@ private static IEnumerable GetDocumentationSymbols(ISymbol symbol) } 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" })); + UnsafeMigrationSyntaxHelpers.HasSafetyDocumentation(declaration); private static bool HasInstanceBackingField(ISymbol associatedSymbol) => associatedSymbol.ContainingType.GetMembers() diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs index 66fbe621f1628f..27c06d03c98c03 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMigrationSyntaxHelpers.cs @@ -14,7 +14,11 @@ namespace ILLink.RoslynAnalyzer /// internal static class UnsafeMigrationSyntaxHelpers { - // The analyzer builds against a Roslyn version that predates SyntaxKind.SafeKeyword. + internal const string SafetyDocumentationElement = "safety"; + + // This project compiles against MicrosoftCodeAnalysisVersion_LatestVS, which predates SyntaxKind.SafeKeyword, + // but it runs inside whichever compiler the host loaded, which may be newer. Resolve the kind at run time so + // the migration tooling works on both, and treats 'safe' as unsupported when the host cannot parse it. private static readonly SyntaxKind s_safeKeyword = SyntaxFacts.GetContextualKeywordKind("safe"); internal static SyntaxTokenList GetModifiers(SyntaxNode declaration) => @@ -32,6 +36,9 @@ internal static bool HasModifier(SyntaxNode declaration, SyntaxKind modifier) => internal static bool HasSafeModifier(SyntaxNode declaration) => s_safeKeyword != SyntaxKind.None && GetModifiers(declaration).Any(s_safeKeyword); + internal static SyntaxToken GetSafeModifier(SyntaxNode declaration) => + s_safeKeyword == SyntaxKind.None ? default : GetModifier(declaration, s_safeKeyword); + internal static SyntaxToken GetModifier(SyntaxNode declaration, SyntaxKind modifier) { foreach (SyntaxToken token in GetModifiers(declaration)) @@ -42,6 +49,16 @@ internal static SyntaxToken GetModifier(SyntaxNode declaration, SyntaxKind modif return default; } + + /// + /// Determines whether a declaration carries a <safety> documentation element. + /// + internal 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: SafetyDocumentationElement } + or XmlEmptyElementSyntax { Name.LocalName.ValueText: SafetyDocumentationElement })); } } #endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeContextCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeContextCodeFixTests.cs new file mode 100644 index 00000000000000..9add823f6b5698 --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/AddUnsafeContextCodeFixTests.cs @@ -0,0 +1,2011 @@ +// 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 Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + /// + /// Verifies unsafe-context migration across compiler diagnostics and syntax positions. + /// + public class AddUnsafeContextCodeFixTests + { + [Fact] + public async Task SplitsLocalDeclarationAndAddsSafetyComment() + { + var source = """ + class C + { + static unsafe int Read() => 0; + + static int M() + { + int value = {|CS9362:Read()|}; + return value; + } + } + """; + var fixedSource = """ + class C + { + static unsafe int Read() => 0; + + static int M() + { + int value; + unsafe + { + // SAFETY: Audit + value = Read(); + } + return value; + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task InitializerOutVariableKeepsItsScope() + { + var source = """ + class C + { + static unsafe int Get(out int other) + { + other = 0; + return 0; + } + + static void M() + { + int value = {|CS9362:Get(out int other)|}; + System.Console.WriteLine(value + other); + } + } + """; + var fixedSource = """ + class C + { + static unsafe int Get(out int other) + { + other = 0; + return 0; + } + + static void M() + { + int value = unsafe(/* SAFETY: Audit */ Get(out int other)); + System.Console.WriteLine(value + other); + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task UnsafeImplicitConversionPreservesSelectedOperator() + { + var source = """ + struct Convertible + { + public static unsafe implicit operator short(Convertible value) => 1; + public static explicit operator int(Convertible value) => 2; + } + + class C + { + static int Value = {|CS9362:new Convertible()|}; + } + """; + var fixedSource = """ + struct Convertible + { + public static unsafe implicit operator short(Convertible value) => 1; + public static explicit operator int(Convertible value) => 2; + } + + class C + { + static int Value = unsafe(/* SAFETY: Audit */ (short)new Convertible()); + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task StackAllocForwardDeclarationIsScoped() + { + var source = """ + [module: System.Runtime.CompilerServices.SkipLocalsInit] + + class C + { + static void M() + { + System.Span bytes = {|CS9361:stackalloc byte[10]|}; + bytes.Clear(); + } + } + """; + var fixedSource = """ + [module: System.Runtime.CompilerServices.SkipLocalsInit] + + class C + { + static void M() + { + scoped System.Span bytes; + unsafe + { + // SAFETY: Audit + bytes = stackalloc byte[10]; + } + bytes.Clear(); + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ExtendsUnsafeStatementAroundLabelForOutVariable() + { + var source = """ + class C + { + static unsafe void Get(out int value) => value = 0; + static void Use(int value) { } + + static void M() + { + Label: + {|CS9362:Get(out int value)|}; + Use(value); + } + } + """; + var fixedSource = """ + class C + { + static unsafe void Get(out int value) => value = 0; + static void Use(int value) { } + + static void M() + { + unsafe + { + // SAFETY: Audit + Label: + Get(out int value); + Use(value); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task NoFixWhenGotoWouldEnterUnsafeStatement() + { + var source = """ + class C + { + static unsafe void Work() { } + + static void M() + { + goto Label; + Label: + {|CS9362:Work()|}; + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task WrapsPointerDereferenceStatement() + { + var source = """ + class C + { + static void M(int* pointer) + { + System.Console.WriteLine({|CS9360:*|}pointer); + } + } + """; + var fixedSource = """ + class C + { + static void M(int* pointer) + { + unsafe + { + // SAFETY: Audit + System.Console.WriteLine(*pointer); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task UsesUnsafeExpressionForCatchFilter() + { + var source = """ + class C + { + static unsafe bool Filter(System.Exception exception) => true; + + static void M() + { + try + { + } + catch (System.Exception exception) when ({|CS9362:Filter(exception)|}) + { + } + } + } + """; + var fixedSource = """ + class C + { + static unsafe bool Filter(System.Exception exception) => true; + + static void M() + { + try + { + } + catch (System.Exception exception) when (unsafe(/* SAFETY: Audit */ Filter(exception))) + { + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task UsesUnsafeExpressionForFieldInitializer() + { + var source = """ + class C + { + static unsafe int Read() => 0; + static int Value = {|CS9362:Read()|}; + } + """; + var fixedSource = """ + class C + { + static unsafe int Read() => 0; + static int Value = unsafe(/* SAFETY: Audit */ Read()); + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task UnsafeImplicitConversionStaysInsideExpression() + { + var source = """ + class Convertible + { + public static unsafe implicit operator int(Convertible value) => 0; + } + + class C + { + static int Value = {|CS9362:new Convertible()|}; + } + """; + var fixedSource = """ + class Convertible + { + public static unsafe implicit operator int(Convertible value) => 0; + } + + class C + { + static int Value = unsafe(/* SAFETY: Audit */ (int)new Convertible()); + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ConditionalAccessWrapsTheCompleteExpression() + { + var source = """ + class Data + { + public unsafe int Value { get; } + } + + class C + { + static Data s_data = new(); + static int? Value = s_data?{|CS9362:.Value|}; + } + """; + var fixedSource = """ + class Data + { + public unsafe int Value { get; } + } + + class C + { + static Data s_data = new(); + static int? Value = unsafe(/* SAFETY: Audit */ s_data?.Value); + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ObjectInitializerWrapsTheCompleteCreation() + { + var source = """ + class Data + { + public unsafe int Value { get; set; } + } + + class C + { + static Data Value = new() { {|CS9362:Value|} = 1 }; + } + """; + var fixedSource = """ + class Data + { + public unsafe int Value { get; set; } + } + + class C + { + static Data Value = unsafe(/* SAFETY: Audit */ new() { Value = 1 }); + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task CollectionInitializerWrapsTheCompleteCreation() + { + var source = """ + class Collection : System.Collections.Generic.IEnumerable + { + public unsafe void Add(int value) { } + public System.Collections.Generic.IEnumerator GetEnumerator() => null; + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); + } + + class C + { + static Collection Value = new() { {|CS9362:1|} }; + } + """; + var fixedSource = """ + class Collection : System.Collections.Generic.IEnumerable + { + public unsafe void Add(int value) { } + public System.Collections.Generic.IEnumerator GetEnumerator() => null; + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator(); + } + + class C + { + static Collection Value = unsafe(/* SAFETY: Audit */ new() { 1 }); + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task UsesUnsafeExpressionForUsingDeclarationInitializer() + { + var source = """ + class Resource : System.IDisposable + { + public void Dispose() { } + } + + class C + { + static unsafe Resource Create() => new(); + + static void M() + { + using Resource resource = {|CS9362:Create()|}; + System.Console.WriteLine(resource); + } + } + """; + var fixedSource = """ + class Resource : System.IDisposable + { + public void Dispose() { } + } + + class C + { + static unsafe Resource Create() => new(); + + static void M() + { + using Resource resource = unsafe(/* SAFETY: Audit */ Create()); + System.Console.WriteLine(resource); + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task UsesUnsafeExpressionForScopedRefInitializer() + { + var source = """ + class C + { + static int s_value; + static unsafe ref int GetReference() => ref s_value; + + static void M() + { + scoped ref int value = ref {|CS9362:GetReference()|}; + value++; + } + } + """; + var fixedSource = """ + class C + { + static int s_value; + static unsafe ref int GetReference() => ref s_value; + + static void M() + { + scoped ref int value = ref unsafe(/* SAFETY: Audit */ GetReference()); + value++; + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task AwaitUsesUnsafeExpression() + { + var source = """ + class C + { + static unsafe System.Threading.Tasks.Task WorkAsync() => System.Threading.Tasks.Task.CompletedTask; + + static async System.Threading.Tasks.Task M() + { + await {|CS9362:WorkAsync()|}; + } + } + """; + var fixedSource = """ + class C + { + static unsafe System.Threading.Tasks.Task WorkAsync() => System.Threading.Tasks.Task.CompletedTask; + + static async System.Threading.Tasks.Task M() + { + await unsafe(/* SAFETY: Audit */ WorkAsync()); + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ExpressionBodiedAwaitUsesUnsafeExpression() + { + var source = """ + class C + { + static unsafe System.Threading.Tasks.Task WorkAsync() => System.Threading.Tasks.Task.CompletedTask; + static async System.Threading.Tasks.Task M() => await {|CS9362:WorkAsync()|}; + } + """; + var fixedSource = """ + class C + { + static unsafe System.Threading.Tasks.Task WorkAsync() => System.Threading.Tasks.Task.CompletedTask; + static async System.Threading.Tasks.Task M() => await unsafe(/* SAFETY: Audit */ WorkAsync()); + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task AwaitUsingInitializerUsesUnsafeExpression() + { + var source = """ + class Resource : System.IAsyncDisposable + { + public System.Threading.Tasks.ValueTask DisposeAsync() => default; + } + + class C + { + static unsafe Resource Create() => new(); + + static async System.Threading.Tasks.Task M() + { + await using Resource resource = {|CS9362:Create()|}; + await System.Threading.Tasks.Task.Yield(); + } + } + """; + var fixedSource = """ + class Resource : System.IAsyncDisposable + { + public System.Threading.Tasks.ValueTask DisposeAsync() => default; + } + + class C + { + static unsafe Resource Create() => new(); + + static async System.Threading.Tasks.Task M() + { + await using Resource resource = unsafe(/* SAFETY: Audit */ Create()); + await System.Threading.Tasks.Task.Yield(); + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task AwaitedLocalInitializerUsesUnsafeExpression() + { + var source = """ + class C + { + static unsafe System.Threading.Tasks.Task GetAsync() => System.Threading.Tasks.Task.FromResult(0); + + static async System.Threading.Tasks.Task M() + { + int value = await {|CS9362:GetAsync()|}; + return value; + } + } + """; + var fixedSource = """ + class C + { + static unsafe System.Threading.Tasks.Task GetAsync() => System.Threading.Tasks.Task.FromResult(0); + + static async System.Threading.Tasks.Task M() + { + int value = await unsafe(/* SAFETY: Audit */ GetAsync()); + return value; + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task NoFixWhenUnsafeAwaitPatternSpansAwaitExpression() + { + var source = """ + class Awaitable + { + public unsafe Awaiter GetAwaiter() => default; + } + + struct Awaiter : System.Runtime.CompilerServices.INotifyCompletion + { + public bool IsCompleted => true; + public void OnCompleted(System.Action continuation) { } + public void GetResult() { } + } + + class C + { + static async System.Threading.Tasks.Task M() + { + {|CS9362:await new Awaitable()|}; + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task NoFixForExpressionBodiedUnsafeAwaitPattern() + { + var source = """ + class Awaitable + { + public unsafe Awaiter GetAwaiter() => default; + } + + struct Awaiter : System.Runtime.CompilerServices.INotifyCompletion + { + public bool IsCompleted => true; + public void OnCompleted(System.Action continuation) { } + public void GetResult() { } + } + + class C + { + static async System.Threading.Tasks.Task M() => + {|CS9362:await new Awaitable()|}; + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task NoFixForAwaitUsingWithUnsafeDispose() + { + var source = """ + class Resource + { + public unsafe System.Threading.Tasks.ValueTask DisposeAsync() => default; + } + + class C + { + static async System.Threading.Tasks.Task M() + { + {|CS9362:await using Resource resource = new();|} + await System.Threading.Tasks.Task.Yield(); + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task NoFixWhenUsingRangeWouldHideGenericLocalFunction() + { + var source = """ + ref struct Resource + { + public unsafe void Dispose() { } + } + + class C + { + static void M() + { + Local(); + {|CS9362:using Resource resource = new();|} + void Local() { } + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task NoFixWhenRangeCrossesConditionalDirective() + { + var source = """ + #define A + + class C + { + static unsafe void Get(out int value) => value = 0; + + static void M() + { + #if A + {|CS9362:Get(out int value)|}; + #endif + System.Console.WriteLine(value); + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task NoFixWhenUsingRangeWouldHideLocalFunction() + { + var source = """ + ref struct Resource + { + public unsafe void Dispose() { } + } + + class C + { + static void M() + { + Local(); + {|CS9362:using Resource resource = new();|} + void Local() { } + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task NoFixForUnsafeForeachContainingYield() + { + var source = """ + class Enumerable + { + public unsafe Enumerator GetEnumerator() => default; + + public struct Enumerator + { + public int Current => 0; + public bool MoveNext() => false; + } + } + + class C + { + static System.Collections.Generic.IEnumerable M() + { + {|CS9362:foreach|} (int value in new Enumerable()) + { + yield return value; + } + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task YieldUsesUnsafeExpression() + { + var source = """ + class C + { + static unsafe int Read() => 0; + + static System.Collections.Generic.IEnumerable M() + { + yield return {|CS9362:Read()|}; + } + } + """; + var fixedSource = """ + class C + { + static unsafe int Read() => 0; + + static System.Collections.Generic.IEnumerable M() + { + yield return unsafe(/* SAFETY: Audit */ Read()); + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ConvertsExpressionBodiedMemberToUnsafeStatement() + { + var source = """ + class C + { + static unsafe int Read() => 0; + static int M() => {|CS9362:Read()|}; + } + """; + var fixedSource = """ + class C + { + static unsafe int Read() => 0; + static int M() + { + unsafe + { + // SAFETY: Audit + return Read(); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ConvertsAsyncTaskExpressionBodyToExpressionStatement() + { + var source = """ + class C + { + static unsafe System.Threading.Tasks.Task WorkAsync() => + System.Threading.Tasks.Task.CompletedTask; + + static async System.Threading.Tasks.Task M() => {|CS9362:WorkAsync()|}; + } + """; + var fixedSource = """ + class C + { + static unsafe System.Threading.Tasks.Task WorkAsync() => + System.Threading.Tasks.Task.CompletedTask; + + static async System.Threading.Tasks.Task M() + { + unsafe + { + // SAFETY: Audit + WorkAsync(); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ConvertsAsyncTaskValueExpressionBodyToExpressionStatement() + { + var source = """ + class C + { + static unsafe int Work() => 0; + static async System.Threading.Tasks.Task M() => {|CS9362:Work()|}; + } + """; + var fixedSource = """ + class C + { + static unsafe int Work() => 0; + static async System.Threading.Tasks.Task M() + { + unsafe + { + // SAFETY: Audit + Work(); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task PreservesOutVariableScope() + { + var source = """ + class C + { + static unsafe bool TryGet(out int value) + { + value = 0; + return true; + } + + static void M() + { + if ({|CS9362:TryGet(out int value)|}) + { + } + + System.Console.WriteLine(value); + } + } + """; + var fixedSource = """ + class C + { + static unsafe bool TryGet(out int value) + { + value = 0; + return true; + } + + static void M() + { + if (unsafe(/* SAFETY: Audit */ TryGet(out int value))) + { + } + + System.Console.WriteLine(value); + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ExtendsUnsafeStatementForOutVariableInExpressionStatement() + { + var source = """ + class C + { + static unsafe void Get(out int value) => value = 0; + + static void M() + { + {|CS9362:Get(out int value)|}; + System.Console.WriteLine(value); + } + } + """; + var fixedSource = """ + class C + { + static unsafe void Get(out int value) => value = 0; + + static void M() + { + unsafe + { + // SAFETY: Audit + Get(out int value); + System.Console.WriteLine(value); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ExtendsUnsafeStatementForUsingDeclarationDispose() + { + var source = """ + ref struct Resource + { + public unsafe void Dispose() { } + public void Use() { } + } + + class C + { + static void M() + { + {|CS9362:using Resource resource = new();|} + resource.Use(); + } + } + """; + var fixedSource = """ + ref struct Resource + { + public unsafe void Dispose() { } + public void Use() { } + } + + class C + { + static void M() + { + unsafe + { + // SAFETY: Audit + using Resource resource = new(); + resource.Use(); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task UsesUnsafeExpressionInsideDirective() + { + var source = """ + class C + { + static unsafe int Read() => 0; + + static int M() + { + int value = + #if true + {|CS9362:Read()|} + #else + 0 + #endif + ; + return value; + } + } + """; + var fixedSource = """ + class C + { + static unsafe int Read() => 0; + + static int M() + { + int value = + #if true + unsafe(/* SAFETY: Audit */ Read()) + #else + 0 + #endif + ; + return value; + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task UnsafeStatementStaysInsideConditionalDirective() + { + var source = """ + class C + { + static unsafe void Work() { } + + static void M() + { + #if true + {|CS9362:Work()|}; + #endif + } + } + """; + var fixedSource = """ + class C + { + static unsafe void Work() { } + + static void M() + { + #if true + unsafe + { + // SAFETY: Audit + Work(); + } + #endif + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task TopLevelLocalKeepsItsScope() + { + var source = """ + int value = {|CS9362:Read()|}; + System.Console.WriteLine(value); + + static unsafe int Read() => 0; + """; + var fixedSource = """ + int value = unsafe(/* SAFETY: Audit */ Read()); + System.Console.WriteLine(value); + + static unsafe int Read() => 0; + """; + + var test = CreateTest(source, fixedSource); + test.TestState.OutputKind = OutputKind.ConsoleApplication; + await test.RunAsync(); + } + + [Fact] + public async Task TopLevelOutVariableKeepsItsScope() + { + var source = """ + {|CS9362:Get(out int value)|}; + System.Console.WriteLine(value); + + static unsafe void Get(out int value) => value = 0; + """; + var fixedSource = """ + unsafe + { + // SAFETY: Audit + Get(out int value); + System.Console.WriteLine(value); + + static unsafe void Get(out int value) => value = 0; + } + """; + + var test = CreateTest(source, fixedSource); + test.TestState.OutputKind = OutputKind.ConsoleApplication; + await test.RunAsync(); + } + + [Fact] + public async Task NoFixWhenTopLevelGotoWouldEnterUnsafeStatement() + { + var source = """ + goto Label; + Label: + {|CS9362:Work()|}; + + static unsafe void Work() { } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + test.TestState.OutputKind = OutputKind.ConsoleApplication; + await test.RunAsync(); + } + + [Fact] + public async Task UsesUnsafeExpressionInConstructorArgument() + { + var source = """ + class B + { + protected B(int value) { } + } + + class C : B + { + static unsafe int Read() => 0; + C() : base({|CS9362:Read()|}) { } + } + """; + var fixedSource = """ + class B + { + protected B(int value) { } + } + + class C : B + { + static unsafe int Read() => 0; + C() : base(unsafe(/* SAFETY: Audit */ Read())) { } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task MarksConstructorUnsafeForUnsafeBaseCall() + { + var source = """ + class B + { + protected unsafe B() { } + } + + class C : B + { + {|CS9362:public C() { }|} + } + """; + var fixedSource = """ + class B + { + protected unsafe B() { } + } + + class C : B + { + /// TODO: Audit + public unsafe C() { } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task MarksConstructorUnsafeForLegacyPointerBaseCall() + { + var source = """ + class C : LegacyBase + { + public C(int* pointer) {|CS9363:: base(pointer)|} { } + } + """; + var fixedSource = """ + class C : LegacyBase + { + /// TODO: Audit + public unsafe C(int* pointer) : base(pointer) { } + } + """; + + var test = CreateTest(source, fixedSource); + test.TestState.AdditionalReferences.Add(CreateLegacyReference()); + await test.RunAsync(); + } + + [Fact] + public async Task AddsUnsafeToUsingAliasForConstructorConstraint() + { + var source = """ + using {|CS9376:Alias|} = Generic; + + class UnsafeConstructor + { + public unsafe UnsafeConstructor() { } + } + + class Generic where T : new() { } + """; + var fixedSource = """ + using unsafe Alias = Generic; + + class UnsafeConstructor + { + public unsafe UnsafeConstructor() { } + } + + class Generic where T : new() { } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task WrapsConstructorConstraintInvocation() + { + var source = """ + class UnsafeConstructor + { + public unsafe UnsafeConstructor() { } + } + + class C + { + static void Create() where T : new() { } + + static void M() + { + {|CS9376:Create()|}; + } + } + """; + var fixedSource = """ + class UnsafeConstructor + { + public unsafe UnsafeConstructor() { } + } + + class C + { + static void Create() where T : new() { } + + static void M() + { + unsafe + { + // SAFETY: Audit + Create(); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task FixesLegacyPointerSignatureDiagnostic() + { + var source = """ + class C + { + static void M() + { + {|CS9363:Legacy.GetPointer()|}; + } + } + """; + var fixedSource = """ + class C + { + static void M() + { + unsafe + { + // SAFETY: Audit + Legacy.GetPointer(); + } + } + } + """; + + var test = CreateTest(source, fixedSource); + test.TestState.AdditionalReferences.Add(CreateLegacyReference()); + await test.RunAsync(); + } + + [Fact] + public async Task OneUnsafeStatementFixesOverlappingDiagnostics() + { + var source = """ + class C + { + static void M() + { + Legacy.Callback(); + } + } + """; + var fixedSource = """ + class C + { + static void M() + { + unsafe + { + // SAFETY: Audit + Legacy.Callback(); + } + } + } + """; + + var test = CreateTest(source, fixedSource); + test.TestState.AdditionalReferences.Add(CreateLegacyReference()); + test.TestState.ExpectedDiagnostics.Add( + Microsoft.CodeAnalysis.Testing.DiagnosticResult + .CompilerError(AddUnsafeContextCodeFixProvider.UnsafeMemberOperationCompatDiagnosticId) + .WithSpan(5, 9, 5, 24) + .WithArguments("Legacy.Callback")); + test.TestState.ExpectedDiagnostics.Add( + Microsoft.CodeAnalysis.Testing.DiagnosticResult + .CompilerError(AddUnsafeContextCodeFixProvider.UnsafeOperationDiagnosticId) + .WithSpan(5, 9, 5, 26)); + await test.RunAsync(); + } + + [Fact] + public async Task UnsafePropertyReadUsesCastInsideUnsafeExpression() + { + var source = """ + class Data + { + public unsafe int Value { get; } + } + + class C + { + static Data s_data = new(); + static int Value = {|CS9362:s_data.Value|}; + } + """; + var fixedSource = """ + class Data + { + public unsafe int Value { get; } + } + + class C + { + static Data s_data = new(); + static int Value = unsafe(/* SAFETY: Audit */ (int)s_data.Value); + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task UnsafeIndexerReadUsesCastInsideUnsafeExpression() + { + var source = """ + class Data + { + public unsafe bool this[int index] => true; + } + + class C + { + static Data s_data = new(); + + static void M() + { + try + { + } + catch (System.Exception) when ({|CS9362:s_data[0]|}) + { + } + } + } + """; + var fixedSource = """ + class Data + { + public unsafe bool this[int index] => true; + } + + class C + { + static Data s_data = new(); + + static void M() + { + try + { + } + catch (System.Exception) when (unsafe(/* SAFETY: Audit */ (bool)s_data[0])) + { + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task MethodGroupConversionUsesCastInsideUnsafeExpression() + { + var source = """ + class C + { + static unsafe void Work() { } + static System.Action s_action = {|CS9362:Work|}; + } + """; + var fixedSource = """ + class C + { + static unsafe void Work() { } + static System.Action s_action = unsafe(/* SAFETY: Audit */ (System.Action)Work); + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task MethodGroupConversionInYieldReturnUsesCast() + { + var source = """ + class C + { + static unsafe void Work() { } + + static System.Collections.Generic.IEnumerable M() + { + yield return {|CS9362:Work|}; + } + } + """; + var fixedSource = """ + class C + { + static unsafe void Work() { } + + static System.Collections.Generic.IEnumerable M() + { + yield return unsafe(/* SAFETY: Audit */ (System.Action)Work); + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task DeconstructionUsesUnsafeStatement() + { + var source = """ + class D + { + public unsafe void Deconstruct(out int first, out int second) + { + first = 0; + second = 0; + } + } + + class C + { + static void M() + { + var (first, second) = {|CS9362:new D()|}; + System.Console.WriteLine(first + second); + } + } + """; + var fixedSource = """ + class D + { + public unsafe void Deconstruct(out int first, out int second) + { + first = 0; + second = 0; + } + } + + class C + { + static void M() + { + unsafe + { + // SAFETY: Audit + var (first, second) = new D(); + System.Console.WriteLine(first + second); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task NoFixWhenUnsafeExpressionIsAlreadyPresent() + { + var source = """ + class Data + { + public unsafe int Value { get; } + } + + class C + { + static Data s_data = new(); + static int Value = unsafe(/* SAFETY: Audit */ {|CS9362:s_data.Value|}); + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task NoFixForUnsafePropertyAssignmentInAwaitStatement() + { + var source = """ + class Data + { + public unsafe int Value { get; set; } + } + + class C + { + static Data s_data = new(); + static async System.Threading.Tasks.Task GetAsync() => 1; + + static async System.Threading.Tasks.Task M() + { + {|CS9362:{|CS9362:s_data.Value|}|} += await GetAsync(); + } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task NoFixForRefReturningUnsafeProperty() + { + var source = """ + class Data + { + static int s_value; + public unsafe ref int Value => ref s_value; + } + + class C + { + static Data s_data = new(); + static int Value = {|CS9362:s_data.Value|}; + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source); + await test.RunAsync(); + } + + [Fact] + public async Task MergesAdjacentGeneratedUnsafeStatements() + { + var source = """ + class C + { + static unsafe void Work() { } + + static void M() + { + {|CS9362:Work()|}; + {|CS9362:Work()|}; + } + } + """; + var fixedSource = """ + class C + { + static unsafe void Work() { } + + static void M() + { + unsafe + { + // SAFETY: Audit + Work(); + Work(); + } + } + } + """; + // Fix all evaluates every diagnostic against the original document, so it cannot merge the regions that + // the individual fixes produce. + var batchFixedSource = """ + class C + { + static unsafe void Work() { } + + static void M() + { + unsafe + { + // SAFETY: Audit + Work(); + } + + unsafe + { + // SAFETY: Audit + Work(); + } + } + } + """; + + var test = CreateTest(source, fixedSource); + test.BatchFixedCode = batchFixedSource; + await test.RunAsync(); + } + + [Fact] + public async Task MergesGeneratedUnsafeStatementsSeparatedBySafeStatement() + { + var source = """ + class C + { + static unsafe void Work() { } + + static void M() + { + {|CS9362:Work()|}; + System.Console.WriteLine(); + {|CS9362:Work()|}; + } + } + """; + var fixedSource = """ + class C + { + static unsafe void Work() { } + + static void M() + { + unsafe + { + // SAFETY: Audit + Work(); + System.Console.WriteLine(); + Work(); + } + } + } + """; + var batchFixedSource = """ + class C + { + static unsafe void Work() { } + + static void M() + { + unsafe + { + // SAFETY: Audit + Work(); + } + + System.Console.WriteLine(); + unsafe + { + // SAFETY: Audit + Work(); + } + } + } + """; + + var test = CreateTest(source, fixedSource); + test.BatchFixedCode = batchFixedSource; + await test.RunAsync(); + } + + [Fact] + public async Task DoesNotMergeIntoAuditedUnsafeStatement() + { + var source = """ + class C + { + static unsafe void Work() { } + + static void M() + { + unsafe + { + // SAFETY: Reviewed by the runtime team. + Work(); + } + {|CS9362:Work()|}; + } + } + """; + var fixedSource = """ + class C + { + static unsafe void Work() { } + + static void M() + { + unsafe + { + // SAFETY: Reviewed by the runtime team. + Work(); + } + unsafe + { + // SAFETY: Audit + Work(); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task DoesNotMergeAcrossADeclaringStatement() + { + var source = """ + class C + { + static unsafe void Work() { } + + static void M() + { + {|CS9362:Work()|}; + int value = 5; + {|CS9362:Work()|}; + System.Console.WriteLine(value); + } + } + """; + var fixedSource = """ + class C + { + static unsafe void Work() { } + + static void M() + { + unsafe + { + // SAFETY: Audit + Work(); + } + + int value = 5; + unsafe + { + // SAFETY: Audit + Work(); + } + + System.Console.WriteLine(value); + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task MergingKeepsCommentsFromTheAbsorbedRegion() + { + var source = """ + class C + { + static unsafe void Work() { } + + static void M() + { + unsafe + { + // SAFETY: Audit + // Only the first call needs the pointer. + Work(); + } + {|CS9362:Work()|}; + } + } + """; + var fixedSource = """ + class C + { + static unsafe void Work() { } + + static void M() + { + // Only the first call needs the pointer. + unsafe + { + // SAFETY: Audit + Work(); + Work(); + } + } + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ParenthesizesUnaryOperandOfInsertedCast() + { + var source = """ + struct Ptr + { + public static implicit operator Ptr(int value) => default; + } + + class C + { + static int* GetPointer() => null; + static Ptr s_ptr = {|CS9360:*|}GetPointer(); + } + """; + var fixedSource = """ + struct Ptr + { + public static implicit operator Ptr(int value) => default; + } + + class C + { + static int* GetPointer() => null; + static Ptr s_ptr = unsafe(/* SAFETY: Audit */ (Ptr)(*GetPointer())); + } + """; + + await CreateTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task GeneratedDocumentationMatchesFileLineEndings() + { + // Written with explicit line feeds: the generated documentation must not introduce carriage returns. + var source = "class B\n{\n protected unsafe B() { }\n}\n\nclass C : B\n{\n {|CS9362:public C() { }|}\n}\n"; + var fixedSource = "class B\n{\n protected unsafe B() { }\n}\n\nclass C : B\n{\n" + + " /// TODO: Audit\n public unsafe C() { }\n}\n"; + + await CreateTest(source, fixedSource).RunAsync(); + } + + private static CSharpCodeFixVerifier.Test CreateTest( + string source, + string fixedSource) => + UnsafeMigrationTestHelpers + .CreateCodeFixTest( + source, + fixedSource); + + private static MetadataReference CreateLegacyReference() + { + var syntaxTree = SyntaxFactory.ParseSyntaxTree( + """ + public static class Legacy + { + public static unsafe int* GetPointer() => null; + public static unsafe delegate* Callback; + } + + public class LegacyBase + { + public unsafe LegacyBase(int* pointer) { } + } + """, + new CSharpParseOptions(LanguageVersion.Preview)); + CSharpCompilation compilation = CSharpCompilation.Create( + "Legacy", + [syntaxTree], + SourceGenerators.Tests.LiveReferencePack.GetMetadataReferences(), + new CSharpCompilationOptions( + OutputKind.DynamicallyLinkedLibrary, + allowUnsafe: true)); + return compilation.EmitToImageReference(); + } + } +} +#endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresUnsafeCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresUnsafeCodeFixTests.cs deleted file mode 100644 index aab096802e3cf5..00000000000000 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresUnsafeCodeFixTests.cs +++ /dev/null @@ -1,1675 +0,0 @@ -// 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.Threading.Tasks; -using ILLink.CodeFix; -using ILLink.Shared; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.Testing; -using Microsoft.CodeAnalysis.Text; -using Xunit; -using VerifyCS = ILLink.RoslynAnalyzer.Tests.CSharpCodeFixVerifier< - ILLink.RoslynAnalyzer.DynamicallyAccessedMembersAnalyzer, - ILLink.CodeFix.RequiresUnsafeCodeFixProvider>; - -namespace ILLink.RoslynAnalyzer.Tests -{ - public class RequiresUnsafeCodeFixTests - { - 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!; - compilationOptions = compilationOptions.WithAllowUnsafe(true); - return solution.WithProjectParseOptions(projectId, parseOptions) - .WithProjectCompilationOptions(projectId, compilationOptions); - } - - static Task VerifyRequiresUnsafeCodeFix( - string source, - string fixedSource, - DiagnosticResult[] baselineExpected, - DiagnosticResult[] fixedExpected, - int? numberOfIterations = null, - int codeActionIndex = 1) - { - var test = new VerifyCS.Test - { - TestCode = source, - FixedCode = fixedSource, - CodeActionIndex = codeActionIndex - }; - test.ExpectedDiagnostics.AddRange(baselineExpected); - test.SolutionTransforms.Add(SetOptions); - if (numberOfIterations != null) - { - test.NumberOfIncrementalIterations = numberOfIterations; - test.NumberOfFixAllIterations = numberOfIterations; - } - test.FixedState.ExpectedDiagnostics.AddRange(fixedExpected); - return test.RunAsync(); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_SimpleStatement() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int x = M1(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int x; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - x = M1(); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 17, 9, 21) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ExpressionStatement() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe void M1() { } - - public void M2() - { - M1(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe void M1() { } - - public void M2() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - M1(); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 9, 9, 13) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ReturnStatement() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2() - { - return M1(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 16, 9, 20) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_IfStatement() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe bool M1() => true; - - public void M2() - { - if (M1()) - { - System.Console.WriteLine("yes"); - } - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe bool M1() => true; - - public void M2() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - if (M1()) - { - System.Console.WriteLine("yes"); - } - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 13, 9, 17) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task NoWarning_InsideUnsafeBlock() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - unsafe - { - int x = M1(); - } - } - } - """; - - // No diagnostics expected - already in unsafe block - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: test, // No change expected - baselineExpected: Array.Empty(), - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_InsideUnsafeMethod() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public unsafe void M2() - { - int x = M1(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public unsafe void M2() - { - int x; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - x = M1(); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 17, 9, 21) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty(), - codeActionIndex: 0); - } - - [Fact] - public async Task CodeFix_InsideUnsafeClass() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public unsafe class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int x = M1(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public unsafe class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int x; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - x = M1(); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 17, 9, 21) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_InsideUnsafeProperty() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public unsafe int P - { - get => M1(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public unsafe int P - { - get - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 16, 9, 20) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty(), - codeActionIndex: 1); - } - - [Fact] - public async Task CodeFix_InsideUnsafeLocalFunction() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - unsafe int Local() => M1(); - - _ = Local(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public unsafe void M2() - { - unsafe int Local() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - // TODO(unsafe): Baselining unsafe usage - - unsafe - { - _ = Local(); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 31, 9, 35) - .WithArguments("C.M1()"), - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(11, 13, 11, 20) - .WithArguments("Local()") - }, - fixedExpected: Array.Empty(), - numberOfIterations: 3, - codeActionIndex: 0); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ExpressionBodiedMethod() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2() => M1(); - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(7, 24, 7, 28) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ExpressionBodiedVoidMethod() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe void M1() { } - - public void M2() => M1(); - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe void M1() { } - - public void M2() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - M1(); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(7, 25, 7, 29) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ExpressionBodiedDestructor() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe void M1() { } - - ~C() => M1(); - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe void M1() { } - - ~C() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - M1(); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(7, 13, 7, 17) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ExpressionBodiedProperty() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int P => M1(); - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int P - { - get - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(7, 21, 7, 25) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty(), - codeActionIndex: 1); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ExpressionBodiedAccessor() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int P - { - get => M1(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int P - { - get - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 16, 9, 20) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty(), - codeActionIndex: 1); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_LocalFunction_ConvertsExpressionBody() - { - // Local functions with expression bodies are converted to block bodies - // with the unsafe block inside, preserving their scope. - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int Local() => M1(); - _ = Local(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int Local() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - - _ = Local(); - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 24, 9, 28) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ForwardDeclaration() - { - // When a variable is declared with unsafe code and used later, - // use forward declaration instead of expanding the unsafe block. - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int x = M1(); - int y = x + 1; - System.Console.WriteLine(y); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int x; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - x = M1(); - } - int y = x + 1; - System.Console.WriteLine(y); - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 17, 9, 21) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_UnusedVariable() - { - // When a variable declared with unsafe code is not used after, - // forward declaration still applies consistently. - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int x = M1(); - int y = 42; - System.Console.WriteLine(y); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int x; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - x = M1(); - } - int y = 42; - System.Console.WriteLine(y); - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 17, 9, 21) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ForwardDeclaration_VarType() - { - // When using 'var', the forward declaration should use the explicit type. - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - var x = M1(); - System.Console.WriteLine(x); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int x; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - x = M1(); - } - System.Console.WriteLine(x); - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 17, 9, 21) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_RefLocal_NoForwardDeclaration() - { - // Ref locals cannot use forward declaration (can't declare without initializer), - // so wrap the entire declaration in the unsafe block. - // Note: If the ref local is used after the declaration, those uses will be broken - // and need manual fixing. This test has no usage after the declaration. - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - private int _field; - - public static unsafe ref int M1(ref int x) => ref x; - - public void M2() - { - ref int x = ref M1(ref _field); - } - } - """; - - // ref locals can't be forward-declared, so wrap the whole declaration - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - private int _field; - - public static unsafe ref int M1(ref int x) => ref x; - - public void M2() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - ref int x = ref M1(ref _field); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(11, 25, 11, 39) - .WithArguments("C.M1(ref int)") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_RefLocalFromUnsafeAs_NoForwardDeclaration() - { - // Pattern matching real-world usage: ref byte x = ref Unsafe.As(ref source) - // The Unsafe.As call is unsafe, and the result is assigned to a ref local. - // When the ref local is used after the declaration, those statements must be included - // in the unsafe block since ref locals can't be forward-declared. - var test = """ - using System.Diagnostics.CodeAnalysis; - using System.Runtime.CompilerServices; - - public class C - { - public void M2() - { - char c = 'x'; - ref byte x = ref Unsafe.As(ref c); - int y = x + 1; - } - } - - namespace System.Runtime.CompilerServices - { - public static class Unsafe - { - public static unsafe ref TTo As(ref TFrom source) => throw null!; - } - } - """; - - // ref locals can't be forward-declared, so must expand block to include usages - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - using System.Runtime.CompilerServices; - - public class C - { - public void M2() - { - char c = 'x'; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - ref byte x = ref Unsafe.As(ref c); - int y = x + 1; - } - } - } - - namespace System.Runtime.CompilerServices - { - public static class Unsafe - { - public static unsafe ref TTo As(ref TFrom source) => throw null!; - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 26, 9, 54) - .WithArguments("System.Runtime.CompilerServices.Unsafe.As(ref char)") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ChainedRefLocals_ExpandsToIncludeAll() - { - // When a ref local is used to create another ref local, and that second ref local - // is used later, the unsafe block must expand to include ALL usages. - // This matches the pattern in CharUnicodeInfo.cs where rsStart is used to create rsDelta. - var test = """ - using System.Diagnostics.CodeAnalysis; - using System.Runtime.CompilerServices; - - public class C - { - public void M2() - { - byte b = 0; - ref ushort rsStart = ref Unsafe.As(ref b); - ref ushort rsDelta = ref Unsafe.Add(ref rsStart, 1); - int delta = rsDelta; - } - } - - namespace System.Runtime.CompilerServices - { - public static class Unsafe - { - public static unsafe ref TTo As(ref TFrom source) => throw null!; - public static ref T Add(ref T source, int elementOffset) => throw null!; - } - } - """; - - // Both ref locals and the usage of rsDelta must be inside the unsafe block - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - using System.Runtime.CompilerServices; - - public class C - { - public void M2() - { - byte b = 0; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - ref ushort rsStart = ref Unsafe.As(ref b); - ref ushort rsDelta = ref Unsafe.Add(ref rsStart, 1); - int delta = rsDelta; - } - } - } - - namespace System.Runtime.CompilerServices - { - public static class Unsafe - { - public static unsafe ref TTo As(ref TFrom source) => throw null!; - public static ref T Add(ref T source, int elementOffset) => throw null!; - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 34, 9, 64) - .WithArguments("System.Runtime.CompilerServices.Unsafe.As(ref byte)") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_RefLocal_RegularVarEscapes_ExpandsToIncludeUsage() - { - // When a ref local block expansion pulls in a regular variable declaration, - // and that regular variable is used after the block, we must expand further. - // This matches the CharUnicodeInfo.cs pattern. - var test = """ - using System.Diagnostics.CodeAnalysis; - using System.Runtime.CompilerServices; - - public class C - { - public int M2() - { - byte b = 0; - ref ushort rsStart = ref Unsafe.As(ref b); - ref ushort rsDelta = ref Unsafe.Add(ref rsStart, 1); - int delta = rsDelta; - return delta + 1; - } - } - - namespace System.Runtime.CompilerServices - { - public static class Unsafe - { - public static unsafe ref TTo As(ref TFrom source) => throw null!; - public static ref T Add(ref T source, int elementOffset) => throw null!; - } - } - """; - - // Block must expand to include return statement since delta is used there - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - using System.Runtime.CompilerServices; - - public class C - { - public int M2() - { - byte b = 0; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - ref ushort rsStart = ref Unsafe.As(ref b); - ref ushort rsDelta = ref Unsafe.Add(ref rsStart, 1); - int delta = rsDelta; - return delta + 1; - } - } - } - - namespace System.Runtime.CompilerServices - { - public static class Unsafe - { - public static unsafe ref TTo As(ref TFrom source) => throw null!; - public static ref T Add(ref T source, int elementOffset) => throw null!; - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 34, 9, 64) - .WithArguments("System.Runtime.CompilerServices.Unsafe.As(ref byte)") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_RefReadonlyLocal_NoForwardDeclaration() - { - // Ref readonly locals cannot use forward declaration, so wrap the declaration. - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe ref readonly int M1(in int x) => ref x; - - public void M2() - { - int value = 42; - ref readonly int x = ref M1(in value); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe ref readonly int M1(in int x) => ref x; - - public void M2() - { - int value = 42; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - ref readonly int x = ref M1(in value); - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(10, 34, 10, 46) - .WithArguments("C.M1(in int)") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_NotOfferedForStatementsWithPragmaDirectives() - { - // When statements to wrap have #pragma directives after the diagnostic statement, - // the unsafe block should not include the directive trivia. - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public void M2() - { - int x = M1(); - #pragma warning disable CS0168 - if (x > 0) - #pragma warning restore CS0168 - { - } - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public unsafe void M2() - { - int x; - // TODO(unsafe): Baselining unsafe usage - unsafe - { - x = M1(); - } - #pragma warning disable CS0168 - if (x > 0) - #pragma warning restore CS0168 - { - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 17, 9, 21) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty(), - numberOfIterations: 2, - codeActionIndex: 0); - } - - [Fact] - public async Task CodeFix_AddUnsafeModifier_Method() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2() - { - return M1(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public unsafe int M2() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - } - """; - - var addUnsafeTest = new VerifyCS.Test - { - TestCode = test, - FixedCode = fixedSource, - CodeActionIndex = 0, - NumberOfIncrementalIterations = 2, - NumberOfFixAllIterations = 2 - }; - addUnsafeTest.ExpectedDiagnostics.Add( - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 16, 9, 20) - .WithArguments("C.M1()")); - addUnsafeTest.SolutionTransforms.Add(SetOptions); - await addUnsafeTest.RunAsync(); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_Method() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2() - { - return M1(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - } - """; - - var addAttributeTest = new VerifyCS.Test - { - TestCode = test, - FixedCode = fixedSource, - CodeActionIndex = 1 - }; - addAttributeTest.ExpectedDiagnostics.Add( - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 16, 9, 20) - .WithArguments("C.M1()")); - addAttributeTest.SolutionTransforms.Add(SetOptions); - await addAttributeTest.RunAsync(); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_Constructor() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe void M1() { } - - public C() - { - M1(); - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe void M1() { } - - public C() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - M1(); - } - } - } - """; - - var addAttributeTest = new VerifyCS.Test - { - TestCode = test, - FixedCode = fixedSource, - CodeActionIndex = 1 - }; - addAttributeTest.ExpectedDiagnostics.Add( - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(9, 9, 9, 13) - .WithArguments("C.M1()")); - addAttributeTest.SolutionTransforms.Add(SetOptions); - await addAttributeTest.RunAsync(); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_NotOfferedForExpressionBodyWithPreprocessorDirectives() - { - // When an expression-bodied member has preprocessor directives (#if/#else/#endif), - // the "Wrap in unsafe block" fix should NOT be offered because it would destroy - // the conditional compilation structure. - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2() - #if SOME_DEFINE - => 42; - #else - => M1(); - #endif - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public unsafe int M2() - #if SOME_DEFINE - => 42; - #else - => M1(); - #endif - } - """; - - // The "Wrap in unsafe block" fix should NOT be available. - - var addAttributeTest = new VerifyCS.Test - { - TestCode = test, - FixedCode = fixedSource, - CodeActionIndex = 0, - NumberOfIncrementalIterations = 1, - NumberOfFixAllIterations = 1 - }; - addAttributeTest.ExpectedDiagnostics.Add( - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(11, 12, 11, 16) - .WithArguments("C.M1()")); - addAttributeTest.FixedState.ExpectedDiagnostics.Add( - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(11, 12, 11, 16) - .WithArguments("C.M1()")); - addAttributeTest.SolutionTransforms.Add(SetOptions); - await addAttributeTest.RunAsync(); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_SwitchCaseSection() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2(int x) - { - switch (x) - { - case 1: - return M1(); - default: - return 0; - } - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2(int x) - { - switch (x) - { - case 1: - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - default: - return 0; - } - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(12, 24, 12, 28) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_IfStatementWithoutBraces() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2(bool condition) - { - if (condition) - return M1(); - return 0; - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2(bool condition) - { - if (condition) - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - return 0; - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(10, 20, 10, 24) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - - [Fact] - public async Task CodeFix_WrapInUnsafeBlock_ExpressionBodiedLocalFunction() - { - var test = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2() - { - int x = 1; - static int LocalFunc() => M1(); - return LocalFunc() + x; - } - } - """; - - var fixedSource = """ - using System.Diagnostics.CodeAnalysis; - - public class C - { - public static unsafe int M1() => 0; - - public int M2() - { - int x = 1; - static int LocalFunc() - { - // TODO(unsafe): Baselining unsafe usage - unsafe - { - return M1(); - } - } - - return LocalFunc() + x; - } - } - """; - - await VerifyRequiresUnsafeCodeFix( - source: test, - fixedSource: fixedSource, - baselineExpected: new[] { - DiagnosticResult.CompilerError(RequiresUnsafeCodeFixProvider.UnsafeMemberOperationDiagnosticId) - .WithSpan(10, 35, 10, 39) - .WithArguments("C.M1()") - }, - fixedExpected: Array.Empty()); - } - } -} -#endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/SynchronizeUnsafeContractCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/SynchronizeUnsafeContractCodeFixTests.cs new file mode 100644 index 00000000000000..ba7b4c725213d4 --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/SynchronizeUnsafeContractCodeFixTests.cs @@ -0,0 +1,628 @@ +// 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 Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + /// + /// Verifies one-way unsafe propagation for compiler-reported contract mismatches. + /// + public class SynchronizeUnsafeContractCodeFixTests + { + [Fact] + public async Task CompilerOverrideFixPropagatesUpAndDown() + { + var source = """ + class Base + { + public virtual void Method() { } + } + + class First : Base + { + public override unsafe void {|CS9364:Method|}() { } + } + + class Second : Base + { + public override void Method() { } + } + """; + var fixedSource = """ + class Base + { + /// TODO: Audit + public virtual unsafe void Method() { } + } + + class First : Base + { + public override unsafe void Method() { } + } + + class Second : Base + { + public override void Method() { } + } + """; + + await CreateCompilerTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task CompilerInterfaceFixPropagatesToEveryImplementation() + { + var source = """ + interface I + { + void Method(); + } + + class Implicit : I + { + public unsafe void {|CS9365:Method|}() { } + } + + class Explicit : I + { + unsafe void I.{|CS9366:Method|}() { } + } + + class Other : I + { + public void Method() { } + } + """; + var fixedSource = """ + interface I + { + /// TODO: Audit + unsafe void Method(); + } + + class Implicit : I + { + public unsafe void Method() { } + } + + class Explicit : I + { + unsafe void I.Method() { } + } + + class Other : I + { + public void Method() { } + } + """; + + await CreateCompilerTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task CompilerPropertyFixPropagatesPropertyContract() + { + var source = """ + interface I + { + int Property { get; } + } + + class UnsafeImplementation : I + { + public unsafe int Property => {|CS9365:0|}; + } + + class OtherImplementation : I + { + public int Property => 0; + } + """; + var fixedSource = """ + interface I + { + /// TODO: Audit + unsafe int Property { get; } + } + + class UnsafeImplementation : I + { + public unsafe int Property => 0; + } + + class OtherImplementation : I + { + public int Property => 0; + } + """; + + await CreateCompilerTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task PropagatesToInterfaceEvenWhenOtherImplementationIsSynthesized() + { + var source = """ + interface I + { + int Property { get; } + } + + class UnsafeImplementation : I + { + public unsafe int Property => {|CS9365:0|}; + } + + record OtherImplementation(int Property) : I; + """; + var fixedSource = """ + interface I + { + /// TODO: Audit + unsafe int Property { get; } + } + + class UnsafeImplementation : I + { + public unsafe int Property => 0; + } + + record OtherImplementation(int Property) : I; + """; + + await CreateCompilerTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task CompilerDefaultInterfaceFixPropagatesToBaseContract() + { + var source = """ + interface IBase + { + void Method(); + } + + interface IDerived : IBase + { + unsafe void IBase.{|CS9366:Method|}() { } + } + """; + var fixedSource = """ + interface IBase + { + /// TODO: Audit + unsafe void Method(); + } + + interface IDerived : IBase + { + unsafe void IBase.Method() { } + } + """; + + await CreateCompilerTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task NoFixForInheritedImplementationDiagnosticOnType() + { + var source = """ + interface I + { + void Method(); + } + + class Base + { + public unsafe void Method() { } + } + + class Derived : Base, {|CS9365:I|} + { + } + """; + + await CreateCompilerTest(source).RunAsync(); + } + + [Fact] + public async Task NoFixForUneditableMetadataContract() + { + var source = """ + class UnsafeImplementation : I + { + public unsafe void {|CS9365:Method|}() { } + } + + class SafeImplementation : I + { + public void Method() { } + } + """; + + var test = CreateCompilerTest(source); + test.TestState.AdditionalReferences.Add(CreateReference( + """ + public interface I + { + void Method(); + } + """)); + await test.RunAsync(); + } + + [Fact] + public async Task PartialUnsafeMismatchAddsUnsafeToEveryPart() + { + var source = """ + partial class C + { + public partial void Method(); + public unsafe partial void {|CS0764:Method|}() { } + } + """; + var fixedSource = """ + partial class C + { + /// TODO: Audit + public unsafe partial void Method(); + public unsafe partial void Method() { } + } + """; + + var test = CreateCompilerTest(source, fixedSource); + test.CodeActionEquivalenceKey = "Propagate unsafe contract"; + await test.RunAsync(); + } + + [Fact] + public async Task UnsafeWinsOverSafePartialContract() + { + var source = """ + partial class C + { + public unsafe partial void Method(); + public safe extern partial void {|CS0764:{|CS9390:Method|}|}(); + } + """; + var fixedSource = """ + partial class C + { + public unsafe partial void Method(); + /// TODO: Audit + public unsafe extern partial void Method(); + } + """; + + await CreateCompilerTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task PureSafePartialMismatchDefaultsToUnsafe() + { + var source = """ + partial class C + { + public partial void {|CS9389:Method|}(); + public safe extern partial void {|CS9390:Method|}(); + } + """; + var fixedSource = """ + partial class C + { + /// TODO: Audit + public unsafe partial void Method(); + /// TODO: Audit + public unsafe extern partial void Method(); + } + """; + + var test = CreateCompilerTest(source, fixedSource); + // Replacing a deliberate audit is offered under a title that says so. + test.CodeActionEquivalenceKey = "Propagate unsafe contract (discards explicit 'safe')"; + await test.RunAsync(); + } + + [Fact] + public async Task AccessorContractIsPropagatedIndependently() + { + var source = """ + class Base + { + public virtual int Property { get; set; } + } + + class Derived : Base + { + public override int Property + { + unsafe {|CS9364:get|} => 0; + set { } + } + } + """; + var fixedSource = """ + class Base + { + /// TODO: Audit + public virtual int Property { unsafe get; set; } + } + + class Derived : Base + { + public override int Property + { + unsafe get => 0; + set { } + } + } + """; + + await CreateCompilerTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task ExpressionBodiedBaseGetterBecomesUnsafeAccessor() + { + var source = """ + class Base + { + public virtual int Property => 0; + } + + class Derived : Base + { + public override int Property + { + unsafe {|CS9364:get|} => 1; + } + } + """; + var fixedSource = """ + class Base + { + /// TODO: Audit + public virtual int Property { unsafe get => 0; } + } + + class Derived : Base + { + public override int Property + { + unsafe get => 1; + } + } + """; + + await CreateCompilerTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task PropagatesAcrossDocuments() + { + var test = new CSharpCodeFixVerifier< + DynamicallyAccessedMembersAnalyzer, + SynchronizeUnsafeContractCodeFixProvider>.Test(); + test.TestState.Sources.Add(( + "Base.cs", + """ + class Base + { + public virtual void Method() { } + } + """)); + test.TestState.Sources.Add(( + "First.cs", + """ + class First : Base + { + public override unsafe void {|CS9364:Method|}() { } + } + """)); + test.TestState.Sources.Add(( + "Second.cs", + """ + class Second : Base + { + public override void Method() { } + } + """)); + test.FixedState.Sources.Add(( + "Base.cs", + """ + class Base + { + /// TODO: Audit + public virtual unsafe void Method() { } + } + """)); + test.FixedState.Sources.Add(( + "First.cs", + """ + class First : Base + { + public override unsafe void Method() { } + } + """)); + test.FixedState.Sources.Add(( + "Second.cs", + """ + class Second : Base + { + public override void Method() { } + } + """)); + test.SolutionTransforms.Add(UnsafeMigrationTestHelpers.SetOptions); + + await test.RunAsync(); + } + + [Fact] + public async Task MixedPartialPropertyFormsUseAccessorModifiers() + { + var source = """ + partial class C + { + public partial int Property { unsafe get; } + public partial int Property => {|CS0764:0|}; + } + """; + var fixedSource = """ + partial class C + { + public partial int Property { unsafe get; } + /// TODO: Audit + public partial int Property { unsafe get => 0; } + } + """; + + await CreateCompilerTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task CompilerEventFixSplitsSharedInterfaceDeclaration() + { + var source = """ + #pragma warning disable CS0067 + + interface I + { + event System.Action First, Safe; + } + + class C : I + { + public unsafe event System.Action {|CS9365:First|}; + public event System.Action Safe; + } + """; + var fixedSource = """ + #pragma warning disable CS0067 + + interface I + { + /// TODO: Audit + unsafe event System.Action First; + event System.Action Safe; + } + + class C : I + { + public unsafe event System.Action First; + public event System.Action Safe; + } + """; + + await CreateCompilerTest(source, fixedSource).RunAsync(); + } + + [Fact] + public async Task FixAllAggregatesMultipleEventsFromOneInterfaceDeclaration() + { + var source = """ + #pragma warning disable CS0067 + + interface I + { + event System.Action First, Second, Safe; + } + + class C : I + { + public unsafe event System.Action {|CS9365:First|}; + public unsafe event System.Action {|CS9365:Second|}; + public event System.Action Safe; + } + """; + var fixedSource = """ + #pragma warning disable CS0067 + + interface I + { + /// TODO: Audit + unsafe event System.Action First; + /// TODO: Audit + unsafe event System.Action Second; + event System.Action Safe; + } + + class C : I + { + public unsafe event System.Action First; + public unsafe event System.Action Second; + public event System.Action Safe; + } + """; + // Fix all computes both fixes against the original shared declaration, so the merged text keeps a blank + // line between the two declarations it split apart. + var batchFixedSource = """ + #pragma warning disable CS0067 + + interface I + { + /// TODO: Audit + unsafe event System.Action First; + + /// TODO: Audit + unsafe event System.Action Second; + event System.Action Safe; + } + + class C : I + { + public unsafe event System.Action First; + public unsafe event System.Action Second; + public event System.Action Safe; + } + """; + + var test = CreateCompilerTest(source, fixedSource); + test.BatchFixedCode = batchFixedSource; + await test.RunAsync(); + } + + private static CSharpCodeFixVerifier< + DynamicallyAccessedMembersAnalyzer, + SynchronizeUnsafeContractCodeFixProvider>.Test CreateCompilerTest( + string source, + string fixedSource) => + UnsafeMigrationTestHelpers + .CreateCodeFixTest< + DynamicallyAccessedMembersAnalyzer, + SynchronizeUnsafeContractCodeFixProvider>( + source, + fixedSource); + + private static CSharpCodeFixVerifier< + DynamicallyAccessedMembersAnalyzer, + SynchronizeUnsafeContractCodeFixProvider>.Test CreateCompilerTest( + string source) => + UnsafeMigrationTestHelpers + .CreateCodeFixTest< + DynamicallyAccessedMembersAnalyzer, + SynchronizeUnsafeContractCodeFixProvider>( + source); + + private static MetadataReference CreateReference(string source) + { + SyntaxTree syntaxTree = SyntaxFactory.ParseSyntaxTree( + source, + new CSharpParseOptions(LanguageVersion.Preview)); + CSharpCompilation compilation = CSharpCompilation.Create( + "ContractReference", + [syntaxTree], + SourceGenerators.Tests.LiveReferencePack.GetMetadataReferences(), + new CSharpCompilationOptions( + OutputKind.DynamicallyLinkedLibrary, + allowUnsafe: true)); + return compilation.EmitToImageReference(); + } + } +} +#endif diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs index 35559d26bab9e7..16e84af47675a4 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMemberMissingSafetyDocumentationAnalyzerTests.cs @@ -108,6 +108,35 @@ unsafe void Local() { } await test.RunAsync(); } + [Fact] + public async Task GeneratedSafetyDocumentationSuppressesTheDiagnostic() + { + // Contracts introduced by the migration fixers carry this stub, so a later analysis pass does not remove + // the modifier they just added. + var source = """ + class Base + { + /// TODO: Audit + public virtual unsafe void Method() { } + } + + class B + { + protected {|IL5005:unsafe|} B() { } + } + + class C : B + { + /// TODO: Audit + public unsafe C() { } + } + """; + + var test = UnsafeMigrationTestHelpers + .CreateAnalyzerTest(source); + await test.RunAsync(); + } + [Fact] public async Task IgnoresDeclarationsThatCannotExposeUnsafeContracts() { diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs index 63ac6c37f9fca7..32d3cb0c5f3ad6 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMigrationTestHelpers.cs @@ -79,6 +79,7 @@ internal static Solution SetOptions(Solution solution, ProjectId projectId) return solution.WithProjectParseOptions(projectId, parseOptions) .WithProjectCompilationOptions(projectId, compilationOptions); } + } } #endif