diff --git a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs index b78be58d..059f0c2a 100644 --- a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs +++ b/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 RefactorAsync(Document document, LocalDeclarationStatementSyntax localDecl, CancellationToken token) + private static async Task 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 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 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 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 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 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)); } } diff --git a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs index 30c63d18..82372c96 100644 --- a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs +++ b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs @@ -1,10 +1,11 @@ -namespace Creedengo.Core.Analyzers; + +namespace Creedengo.Core.Analyzers; /// GCI82: Variable can be made constant. [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class VariableCanBeMadeConstant : DiagnosticAnalyzer { - private static readonly ImmutableArray SyntaxKinds = [SyntaxKind.LocalDeclarationStatement]; + private static readonly ImmutableArray SyntaxKinds = [SyntaxKind.LocalDeclarationStatement, SyntaxKind.FieldDeclaration]; /// The diagnostic descriptor. 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; + } + } } diff --git a/src/Creedengo.Tests/Tests/GCI82.VariableCanBeMadeConstant.Tests.cs b/src/Creedengo.Tests/Tests/GCI82.VariableCanBeMadeConstant.Tests.cs index 2951ee97..626452d5 100644 --- a/src/Creedengo.Tests/Tests/GCI82.VariableCanBeMadeConstant.Tests.cs +++ b/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;