File: src\Analyzers\CSharp\Analyzers\MakeStructMemberReadOnly\CSharpMakeStructMemberReadOnlyAnalyzer.cs
Web Access
Project: src\src\CodeStyle\CSharp\Analyzers\Microsoft.CodeAnalysis.CSharp.CodeStyle.csproj (Microsoft.CodeAnalysis.CSharp.CodeStyle)
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
 
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;
 
namespace Microsoft.CodeAnalysis.CSharp.MakeStructMemberReadOnly;
 
[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal sealed class CSharpMakeStructMemberReadOnlyDiagnosticAnalyzer()
    : AbstractBuiltInCodeStyleDiagnosticAnalyzer(
        IDEDiagnosticIds.MakeStructMemberReadOnlyDiagnosticId,
        EnforceOnBuildValues.MakeStructMemberReadOnly,
        CSharpCodeStyleOptions.PreferReadOnlyStructMember,
        new LocalizableResourceString(nameof(CSharpAnalyzersResources.Make_member_readonly), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)),
        new LocalizableResourceString(nameof(CSharpAnalyzersResources.Member_can_be_made_readonly), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)))
{
    public override DiagnosticAnalyzerCategory GetAnalyzerCategory()
        => DiagnosticAnalyzerCategory.SemanticSpanAnalysis;
 
    protected override void InitializeWorker(AnalysisContext context)
        => context.RegisterCompilationStartAction(context =>
        {
            var compilation = context.Compilation;
            if (compilation.LanguageVersion() < LanguageVersion.CSharp8)
                return;
 
            context.RegisterSymbolStartAction(context =>
            {
                if (!ShouldAnalyze(context, out var option))
                    return;
 
                var methodToDiagnostic = PooledDictionary<IMethodSymbol, Diagnostic>.GetInstance();
 
                context.RegisterOperationBlockAction(
                    context => AnalyzeBlock(context, option.Notification, methodToDiagnostic));
 
                context.RegisterSymbolEndAction(
                    context => ProcessResults(context, option.Notification.Severity, methodToDiagnostic));
            }, SymbolKind.NamedType);
 
            bool ShouldAnalyze(SymbolStartAnalysisContext context, [NotNullWhen(true)] out CodeStyleOption2<bool>? option)
            {
                // Only run on non-readonly structs.  If the struct is already readonly, no need to make the members readonly.
                if (context.Symbol is not INamedTypeSymbol
                    {
                        TypeKind: TypeKind.Struct,
                        IsReadOnly: false,
                        DeclaringSyntaxReferences: [var reference, ..],
                    } structType)
                {
                    option = null;
                    return false;
                }
 
                var cancellationToken = context.CancellationToken;
                var declaration = reference.GetSyntax(cancellationToken);
                var options = context.GetCSharpAnalyzerOptions(declaration.SyntaxTree);
                option = options.PreferReadOnlyStructMember;
                if (!option.Value || ShouldSkipAnalysis(declaration.SyntaxTree, context.Options, context.Compilation.Options, option.Notification, cancellationToken))
                    return false;
 
                // Skip analysis if the analysis filter span does not contain the primary location where we would report a diagnostic.
                if (context.FilterSpan is not null)
                {
                    Contract.ThrowIfNull(context.FilterTree);
                    var shouldAnalyze = false;
                    foreach (var member in structType.GetMembers())
                    {
                        if (member is not IMethodSymbol method)
                            continue;
 
                        var (location, _) = GetDiagnosticLocation(method, cancellationToken);
                        if (location != null && context.ShouldAnalyzeLocation(location))
                        {
                            shouldAnalyze = true;
                            break;
                        }
                    }
 
                    if (!shouldAnalyze)
                        return false;
                }
 
                return true;
            }
 
            void ProcessResults(
                SymbolAnalysisContext context, ReportDiagnostic severity, PooledDictionary<IMethodSymbol, Diagnostic> methodToDiagnostic)
            {
                var cancellationToken = context.CancellationToken;
 
                // No need to lock the dictionary here.  Processing only is called once, after all mutation work is done.
                foreach (var (method, diagnostic) in methodToDiagnostic)
                {
                    if (method.IsInitOnly && method.AssociatedSymbol is IPropertySymbol owningProperty)
                    {
                        // Iff we have an init method that we want to mark as readonly, we can only do so if there is no
                        // `get` accessor, or if the `get` method is already `readonly` or would determined we want to
                        // mark as `readonly`.
                        var getMethodIsReadOnly =
                            owningProperty.GetMethod is null ||
                            owningProperty.GetMethod.IsReadOnly ||
                            methodToDiagnostic.ContainsKey(owningProperty.GetMethod);
 
                        // Skip marking this property as readonly for this init method if it would conflict with the get method.
                        if (!getMethodIsReadOnly)
                            continue;
                    }
 
                    // normal case
                    context.ReportDiagnostic(diagnostic);
                }
 
                methodToDiagnostic.Free();
            }
        });
 
    private void AnalyzeBlock(
        OperationBlockAnalysisContext context,
        NotificationOption2 notificationOption,
        Dictionary<IMethodSymbol, Diagnostic> methodToDiagnostic)
    {
        var cancellationToken = context.CancellationToken;
 
        if (context.OwningSymbol is not IMethodSymbol owningMethod)
            return;
 
        var (location, additionalLocation) = GetDiagnosticLocation(owningMethod, cancellationToken);
        if (location == null || additionalLocation == null || !context.ShouldAnalyzeSpan(location.SourceSpan))
            return;
 
        foreach (var blockOperation in context.OperationBlocks)
        {
            // If we have a trivial method that is just `{ throw ... }` or `=> throw ...`, then do not bother
            // analyzing/reporting that it could be made 'readonly'.  These members are likely just being written (or
            // have been generated) and spamming the user with notifications to go change these all is unhelpful.
            if (blockOperation is IBlockOperation { Operations: [IThrowOperation or IExpressionStatementOperation { Operation: IThrowOperation }] })
                return;
 
            if (BlockOperationPotentiallyMutatesThis(owningMethod, blockOperation, cancellationToken))
                return;
        }
 
        // Called concurrently.  Make sure we write to this dictionary safely.
        lock (methodToDiagnostic)
        {
            methodToDiagnostic[owningMethod] = DiagnosticHelper.Create(
                Descriptor,
                location,
                notificationOption,
                context.Options,
                additionalLocations: [additionalLocation],
                properties: null);
        }
    }
 
    private static (Location? location, Location? additionalLocation) GetDiagnosticLocation(
        IMethodSymbol owningMethod,
        CancellationToken cancellationToken)
    {
        // if it's not a method, or it's already readonly, nothing to do.
        if (owningMethod.MethodKind is not (MethodKind.Ordinary or MethodKind.ExplicitInterfaceImplementation or MethodKind.PropertyGet or MethodKind.PropertySet)
            || owningMethod.IsReadOnly
            || owningMethod.IsStatic
            || owningMethod.IsImplicitlyDeclared)
        {
            return default;
        }
 
        // An init accessor in a readonly property is already readonly.  No need to analyze it.  Note: there is no way
        // to tell this symbolically.  We have to check to the syntax here.
        if (owningMethod.IsInitOnly &&
            owningMethod.AssociatedSymbol is IPropertySymbol { DeclaringSyntaxReferences: [var reference, ..] } &&
            reference.GetSyntax(cancellationToken) is PropertyDeclarationSyntax property &&
            property.Modifiers.Any(SyntaxKind.ReadOnlyKeyword))
        {
            return default;
        }
 
        var methodReference = owningMethod.DeclaringSyntaxReferences[0];
        var declaration = methodReference.GetSyntax(cancellationToken);
 
        var nameToken = declaration switch
        {
            MethodDeclarationSyntax methodDeclaration => methodDeclaration.Identifier,
            AccessorDeclarationSyntax accessorDeclaration => accessorDeclaration.Keyword,
            PropertyDeclarationSyntax propertyDeclaration => propertyDeclaration.Identifier,
            IndexerDeclarationSyntax indexerDeclaration => indexerDeclaration.ThisKeyword,
            ArrowExpressionClauseSyntax arrowExpression => arrowExpression.ArrowToken,
            _ => (SyntaxToken?)null
        };
 
        if (declaration is ArrowExpressionClauseSyntax)
            declaration = declaration.GetRequiredParent();
 
        if (nameToken is null)
            return default;
 
        return (nameToken.Value.GetLocation(), declaration.GetLocation());
    }
 
    private static bool BlockOperationPotentiallyMutatesThis(
        IMethodSymbol owningMethod,
        IOperation blockOperation,
        CancellationToken cancellationToken)
    {
        var semanticModel = blockOperation.SemanticModel;
        Contract.ThrowIfNull(semanticModel);
        foreach (var operation in blockOperation.DescendantsAndSelf())
        {
            // Do not suggest making containing method readonly until we have full understanding of it.
            if (operation is IInvalidOperation)
                return true;
 
            if (ReferencesThisInstance(operation, cancellationToken) &&
                OperationPotentiallyMutatesThis(semanticModel, owningMethod, operation, cancellationToken))
            {
                return true;
            }
        }
 
        return false;
 
        static bool ReferencesThisInstance(IOperation operation, CancellationToken cancellationToken)
        {
            // An actual usage of `this` or `base` in the code.
            if (operation is IInstanceReferenceOperation)
                return true;
 
            // A primary constructor parameter implicitly references 'this' instance.
            if (operation is IParameterReferenceOperation { Parameter: var parameter } &&
                parameter.IsPrimaryConstructor(cancellationToken))
            {
                return true;
            }
 
            return false;
        }
    }
 
    private static bool IsPotentiallyValueType(IOperation? instance)
    {
        // 1. A struct is a value type.
        // 2. A type parameter that does not have the explicit 'class' constraint is potentially a value type.
        return instance is { Type.TypeKind: TypeKind.Struct } ||
               instance is { Type: ITypeParameterSymbol { HasReferenceTypeConstraint: false } };
    }
 
    private static bool OperationPotentiallyMutatesThis(
        SemanticModel semanticModel,
        IMethodSymbol owningMethod,
        IOperation instanceOperation,
        CancellationToken cancellationToken)
    {
        // if we have an explicit 'this' in code, and we're overwriting it directly (e.g. `ref this` or `this = ...`
        // then can't make this `readonly`.
        if (!instanceOperation.IsImplicit &&
            CSharpSemanticFacts.Instance.IsWrittenTo(semanticModel, instanceOperation.Syntax, cancellationToken))
        {
            return true;
        }
 
        // We only care if operation is a value type when looking at if it is somehow mutated with the operations that
        // are performed on it.  In other words.  `valueType.X = 0` is not allowed while `referenceType.X = 0` is fine
        // (since the former actually mutates storage in 'this' which would prevent this method from becoming readonly.
        if (!IsPotentiallyValueType(instanceOperation))
            return false;
 
        // Now walk up the instance-operation and see if any operation actually or potentially mutates this value.
        for (var operation = instanceOperation.Parent; operation != null; operation = operation.Parent)
        {
            // Had a parent we didn't understand.  Assume that 'this' could be mutated.
            if (operation.Kind == OperationKind.None)
                return true;
 
            if (operation is IFieldReferenceOperation { Field.IsReadOnly: false } fieldReference &&
                IsPotentiallyValueType(fieldReference.Instance))
            {
                // If we're writing to a field off of 'this'.  Can't make this `readonly`.
                if (CSharpSemanticFacts.Instance.IsWrittenTo(semanticModel, fieldReference.Syntax, cancellationToken))
                    return true;
 
                // otherwise, keeping walking upwards to make sure subsequent accesses off this field are ok.
                continue;
            }
 
            if (operation is IPropertyReferenceOperation propertyReference &&
                IsPotentiallyValueType(propertyReference.Instance))
            {
                // If we're writing to a prop off of 'this'.  Can't make this `readonly`.
                if (CSharpSemanticFacts.Instance.IsWrittenTo(semanticModel, propertyReference.Syntax, cancellationToken))
                    return true;
 
                // If we're reading, that's only ok if we know the get-accessor exists and it is itself readonly.
                // Otherwise it could mutate the value.
                if (propertyReference.Property.GetMethod is null ||
                    !propertyReference.Property.GetMethod.IsReadOnly)
                {
                    return true;
                }
 
                // a safe property reference.  Can stop looking upwards as this cannot return a value that could
                // mutate this value.
                return false;
            }
 
            if (operation is IEventReferenceOperation eventReference &&
                IsPotentiallyValueType(eventReference.Instance))
            {
                // += or -= on an event off of a struct will cause a copy if we become readonly.
                if (operation.Parent is IEventAssignmentOperation)
                    return true;
 
                // a safe event reference.  Can stop looking upwards as this cannot return a value that could
                // mutate this value.
                return false;
            }
 
            if (operation is IInlineArrayAccessOperation)
            {
                // If we're writing into an inline-array off of 'this'.  Then we can't make this `readonly`.
                if (CSharpSemanticFacts.Instance.IsWrittenTo(semanticModel, operation.Syntax, cancellationToken))
                    return true;
 
                // We're reading a value from inside the inline-array.  Have to keep looking upwards to see how the
                // value is treated.
                continue;
            }
 
            // See if we're accessing or invoking a method.
            if (operation is IMethodReferenceOperation methodRefOperation)
            {
                // Either a mutating or not mutating method reference.  Regardless, once we examine it, we're done
                // looking up as the method itself cannot return anything that could mutate this.
                return IsPotentiallyMutatingMethod(owningMethod, methodRefOperation.Instance, methodRefOperation.Method);
            }
 
            if (operation is IInvocationOperation invocationOperation)
            {
                // Either a mutating or not mutating method reference.  Regardless, once we examine it, we're done
                // looking up as the method itself cannot return anything that could mutate this.
                return IsPotentiallyMutatingMethod(owningMethod, invocationOperation.Instance, invocationOperation.TargetMethod);
            }
 
            // Converting an inline-array into a Span<T> allows the array to be written into.  As such, we have to
            // consider this a potential future mutation of 'this'.
            if (operation is IConversionOperation conversionOperation)
            {
                var conversion = conversionOperation.GetConversion();
                if (conversion.IsInlineArray && conversionOperation.Type.IsSpan())
                    return true;
            }
 
            // Wasn't something that mutates this instance.  Go onto the next instance expression.
            break;
        }
 
        return false;
    }
 
    private static bool IsPotentiallyMutatingMethod(
        IMethodSymbol owningMethod,
        IOperation? instance,
        IMethodSymbol methodReference)
    {
        if (!IsPotentiallyValueType(instance))
            return false;
 
        // Calling a readonly method off of a struct is fine since we know it can't mutate.
        if (methodReference.IsReadOnly)
            return false;
 
        // If we're referencing the method we're in (Which isn't readonly yet) we don't want to mark us as
        // not-readonly. a recursive call shouldn't impact the final result.
        if (methodReference.Equals(owningMethod))
            return false;
 
        // Any methods from System.Object or System.ValueType called on this `this` don't stop this from being readonly.
        if (methodReference.ContainingType.SpecialType is SpecialType.System_Object or SpecialType.System_ValueType)
            return false;
 
        // Any other non-readonly method usage on this means we can't be readonly.
        return true;
    }
}