Team RAISON - Implement rule S3962 CSharp

src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs

@@ -22,24 +22,33 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
 
         foreach (var diagnostic in context.Diagnostics)
         {
-            var parent = root.FindToken(diagnostic.Location.SourceSpan.Start).Parent;
-            if (parent is null) return;
+            var token = root.FindToken(diagnostic.Location.SourceSpan.Start);
+            var node = token.Parent?.AncestorsAndSelf().FirstOrDefault(n => n is LocalDeclarationStatementSyntax || n is FieldDeclarationSyntax);
+            if (node is null) continue;
 
-            foreach (var node in parent.AncestorsAndSelf())
-            {
-                if (node is not LocalDeclarationStatementSyntax declaration) continue;
-                context.RegisterCodeFix(
-                    CodeAction.Create(
-                        title: "Make variable constant",
-                        createChangedDocument: token => RefactorAsync(context.Document, declaration, token),
-                        equivalenceKey: "Make variable constant"),
-                    diagnostic);
-                break;
-            }
+            context.RegisterCodeFix(
+                CodeAction.Create(
+                    title: "Make variable constant",
+                    createChangedDocument: ct => RefactorAsync(context.Document, node, ct),
+                    equivalenceKey: "Make variable constant"),
+                diagnostic);
         }
     }
 
-    private static async Task<Document> RefactorAsync(Document document, LocalDeclarationStatementSyntax localDecl, CancellationToken token)
+    private static async Task<Document> RefactorAsync(Document document, SyntaxNode node, CancellationToken token)
+    {
+        switch (node)
+        {
+            case LocalDeclarationStatementSyntax localDecl:
+                return await RefactorLocalAsync(document, localDecl, token).ConfigureAwait(false);
+            case FieldDeclarationSyntax fieldDecl:
+                return await RefactorFieldAsync(document, fieldDecl, token).ConfigureAwait(false);
+            default:
+                return document;
+        }
+    }
+
+    private static async Task<Document> RefactorLocalAsync(Document document, LocalDeclarationStatementSyntax localDecl, CancellationToken token)
     {
         // Remove the leading trivia from the local declaration.
         var firstToken = localDecl.GetFirstToken();
@@ -63,24 +72,78 @@ private static async Task<Document> RefactorAsync(Document document, LocalDeclar
             .WithDeclaration(varDecl)
             .WithAdditionalAnnotations(Formatter.Annotation);
 
-        return await document.WithUpdatedRoot(localDecl, formattedLocal).ConfigureAwait(false);
+        var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false);
+        if (root is null) return document;
+        var newRoot = root.ReplaceNode(localDecl, formattedLocal);
+        return document.WithSyntaxRoot(newRoot);
+    }
+
+    private static async Task<Document> RefactorFieldAsync(Document document, FieldDeclarationSyntax fieldDecl, CancellationToken token)
+    {
+        // Remove static and readonly, add const after access modifiers
+        var modifiers = fieldDecl.Modifiers;
+        var accessModifiers = modifiers.Where(m =>
+            m.IsKind(SyntaxKind.PublicKeyword) ||
+            m.IsKind(SyntaxKind.PrivateKeyword) ||
+            m.IsKind(SyntaxKind.ProtectedKeyword) ||
+            m.IsKind(SyntaxKind.InternalKeyword)).ToList();
+        var otherModifiers = modifiers.Where(m =>
+            !m.IsKind(SyntaxKind.PublicKeyword) &&
+            !m.IsKind(SyntaxKind.PrivateKeyword) &&
+            !m.IsKind(SyntaxKind.ProtectedKeyword) &&
+            !m.IsKind(SyntaxKind.InternalKeyword) &&
+            !m.IsKind(SyntaxKind.StaticKeyword) &&
+            !m.IsKind(SyntaxKind.ReadOnlyKeyword)).ToList();
+
+        var constToken = SyntaxFactory.Token(fieldDecl.GetLeadingTrivia(), SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker));
+        var newModifiers = new SyntaxTokenList();
+        newModifiers = newModifiers.AddRange(accessModifiers);
+        newModifiers = newModifiers.Add(constToken);
+        newModifiers = newModifiers.AddRange(otherModifiers);
 
-        static async Task<VariableDeclarationSyntax> GetDeclarationForVarAsync(Document document, VariableDeclarationSyntax varDecl, TypeSyntax varTypeName, CancellationToken token)
+        // If the type is 'var', replace with the actual type
+        var declaration = fieldDecl.Declaration;
+        var typeSyntax = declaration.Type;
+        if (typeSyntax.IsVar)
         {
             var semanticModel = await document.GetSemanticModelAsync(token).ConfigureAwait(false);
+            var type = semanticModel?.GetTypeInfo(typeSyntax, token).ConvertedType;
+            if (type != null && type.Name != "var")
+            {
+                declaration = declaration.WithType(
+                    SyntaxFactory.ParseTypeName(type.ToDisplayString())
+                        .WithLeadingTrivia(typeSyntax.GetLeadingTrivia())
+                        .WithTrailingTrivia(typeSyntax.GetTrailingTrivia())
+                        .WithAdditionalAnnotations(Simplifier.Annotation));
+            }
+        }
+
+        var newField = fieldDecl
+            .WithModifiers(newModifiers)
+            .WithDeclaration(declaration)
+            .WithAdditionalAnnotations(Formatter.Annotation);
 
-            if (semanticModel is null || semanticModel.GetAliasInfo(varTypeName, token) is not null)
-                return varDecl; // Special case: Ensure that 'var' isn't actually an alias to another type (e.g. using var = System.String)
+        var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false);
+        if (root is null) return document;
+        var newRoot = root.ReplaceNode(fieldDecl, newField);
+        return document.WithSyntaxRoot(newRoot);
+    }
 
-            var type = semanticModel.GetTypeInfo(varTypeName, token).ConvertedType;
-            if (type is null || type.Name == "var") return varDecl; // Special case: Ensure that 'var' isn't actually a type named 'var'
+    private static async Task<VariableDeclarationSyntax> GetDeclarationForVarAsync(Document document, VariableDeclarationSyntax varDecl, TypeSyntax varTypeName, CancellationToken token)
+    {
+        var semanticModel = await document.GetSemanticModelAsync(token).ConfigureAwait(false);
 
-            // Create a new TypeSyntax for the inferred type. Be careful to keep any leading and trailing trivia from the var keyword.
-            return varDecl.WithType(SyntaxFactory
-                .ParseTypeName(type.ToDisplayString())
-                .WithLeadingTrivia(varTypeName.GetLeadingTrivia())
-                .WithTrailingTrivia(varTypeName.GetTrailingTrivia())
-                .WithAdditionalAnnotations(Simplifier.Annotation));
-        }
+        if (semanticModel is null || semanticModel.GetAliasInfo(varTypeName, token) is not null)
+            return varDecl; // Special case: Ensure that 'var' isn't actually an alias to another type (e.g. using var = System.String)
+
+        var type = semanticModel.GetTypeInfo(varTypeName, token).ConvertedType;
+        if (type is null || type.Name == "var") return varDecl; // Special case: Ensure that 'var' isn't actually a type named 'var'
+
+        // Create a new TypeSyntax for the inferred type. Be careful to keep any leading and trailing trivia from the var keyword.
+        return varDecl.WithType(SyntaxFactory
+            .ParseTypeName(type.ToDisplayString())
+            .WithLeadingTrivia(varTypeName.GetLeadingTrivia())
+            .WithTrailingTrivia(varTypeName.GetTrailingTrivia())
+            .WithAdditionalAnnotations(Simplifier.Annotation));
     }
 }

src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs

@@ -1,10 +1,11 @@
-namespace Creedengo.Core.Analyzers;
+
+namespace Creedengo.Core.Analyzers;
 
 /// <summary>GCI82: Variable can be made constant.</summary>
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
 public sealed class VariableCanBeMadeConstant : DiagnosticAnalyzer
 {
-    private static readonly ImmutableArray<SyntaxKind> SyntaxKinds = [SyntaxKind.LocalDeclarationStatement];
+    private static readonly ImmutableArray<SyntaxKind> SyntaxKinds = [SyntaxKind.LocalDeclarationStatement, SyntaxKind.FieldDeclaration];
 
     /// <summary>The diagnostic descriptor.</summary>
     public static DiagnosticDescriptor Descriptor { get; } = Rule.CreateDescriptor(
@@ -29,8 +30,19 @@ public override void Initialize(AnalysisContext context)
 
     private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
     {
-        var localDeclaration = (LocalDeclarationStatementSyntax)context.Node;
+        switch (context.Node)
+        {
+            case LocalDeclarationStatementSyntax localDeclaration:
+                AnalyzeLocalDeclaration(context, localDeclaration);
+                break;
+            case FieldDeclarationSyntax fieldDeclaration:
+                AnalyzeFieldDeclaration(context, fieldDeclaration);
+                break;
+        }
+    }
 
+    private static void AnalyzeLocalDeclaration(SyntaxNodeAnalysisContext context, LocalDeclarationStatementSyntax localDeclaration)
+    {
         // Make sure the declaration isn't already const
         if (localDeclaration.Modifiers.Any(SyntaxKind.ConstKeyword))
             return;
@@ -77,4 +89,59 @@ private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
 
         context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.GetLocation()));
     }
+
+    private static void AnalyzeFieldDeclaration(SyntaxNodeAnalysisContext context, FieldDeclarationSyntax fieldDecl)
+    {
+        // Ignore const fields
+        if (fieldDecl.Modifiers.Any(SyntaxKind.ConstKeyword))
+            return;
+
+        // Only static readonly fields
+        if (!fieldDecl.Modifiers.Any(SyntaxKind.StaticKeyword) ||
+            !fieldDecl.Modifiers.Any(SyntaxKind.ReadOnlyKeyword))
+            return;
+
+        var variableType = context.SemanticModel.GetTypeInfo(fieldDecl.Declaration.Type, context.CancellationToken).ConvertedType;
+        if (variableType is null) return;
+
+        foreach (var variable in fieldDecl.Declaration.Variables)
+        {
+            if (variable.Initializer is null) return;
+
+            var variableSymbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken) as IFieldSymbol;
+            if (variableSymbol is null) return;
+
+            // Only allow types that can be const
+            if (!IsAllowedConstType(variableSymbol.Type)) return;
+
+            var constantValue = context.SemanticModel.GetConstantValue(variable.Initializer.Value, context.CancellationToken);
+            if (!constantValue.HasValue) return;
+        }
+
+        context.ReportDiagnostic(Diagnostic.Create(Descriptor, fieldDecl.GetLocation()));
+    }
+
+    private static bool IsAllowedConstType(ITypeSymbol type)
+    {
+        switch (type.SpecialType)
+        {
+            case SpecialType.System_Boolean:
+            case SpecialType.System_Byte:
+            case SpecialType.System_SByte:
+            case SpecialType.System_Char:
+            case SpecialType.System_Decimal:
+            case SpecialType.System_Double:
+            case SpecialType.System_Single:
+            case SpecialType.System_Int32:
+            case SpecialType.System_UInt32:
+            case SpecialType.System_Int64:
+            case SpecialType.System_UInt64:
+            case SpecialType.System_Int16:
+            case SpecialType.System_UInt16:
+            case SpecialType.System_String:
+                return true;
+            default:
+                return false;
+        }
+    }
 }

src/Creedengo.Tests/Tests/GCI82.VariableCanBeMadeConstant.Tests.cs

@@ -8,6 +8,80 @@ public sealed class VariableCanBeMadeConstantTests
     [TestMethod]
     public Task EmptyCodeAsync() => VerifyAsync("");
 
+    [TestMethod]
+    public Task DontWarnOnConstAsync() => VerifyAsync("""
+        public class TestClass
+        {
+            private const int x = 1;
+            private const string s = "Bar";
+        }
+        """);
+
+    [TestMethod]
+    public Task WarnOnStaticReadonlyIntAsync() => VerifyAsync("""
+        public class TestClass
+        {
+            [|private static readonly int x = 1;|]
+        }
+        """, """
+        public class TestClass
+        {
+            private const int x = 1;
+        }
+        """);
+
+    [TestMethod]
+    public Task WarnOnStaticReadonlyStringAsync() => VerifyAsync("""
+        public class TestClass
+        {
+            [|private static readonly string s = "Bar";|]
+        }
+        """, """
+        public class TestClass
+        {
+            private const string s = "Bar";
+        }
+        """);
+
+    [TestMethod]
+    public Task DontWarnOnStaticReadonlyNonConstTypeAsync() => VerifyAsync("""
+        public class TestClass
+        {
+            private static readonly object o = new object();
+        }
+        """);
+
+    [TestMethod]
+    public Task DontWarnOnStaticReadonlyNonConstantValueAsync() => VerifyAsync("""
+        public class TestClass
+        {
+            private static readonly int x = System.Environment.TickCount;
+        }
+        """);
+
+    [TestMethod]
+    public Task WarnOnMultipleStaticReadonlyFieldsAsync() => VerifyAsync("""
+        public class TestClass
+        {
+            [|private static readonly int a = 1;|]
+            [|private static readonly string b = "foo";|]
+        }
+        """, """
+        public class TestClass
+        {
+            private const int a = 1;
+            private const string b = "foo";
+        }
+        """);
+
+    [TestMethod]
+    public Task DontWarnOnStaticReadonlyWithoutInitializerAsync() => VerifyAsync("""
+        public class TestClass
+        {
+            private static readonly int x;
+        }
+        """);
+
     [TestMethod]
     public Task VariableCanBeConstAsync() => VerifyAsync("""
         using System;