From 5cfa0660e2fdbd2d11ebbbd94c1cf2858109d2fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9opaul?= Date: Wed, 21 May 2025 13:41:13 +0200 Subject: [PATCH 1/5] Implement rule S3962 --- ...3962.ReplaceStaticReadonlyByConst.Fixer.cs | 72 ++++++++++++++++ .../GCIS3962.ReplaceStaticReadonlyByConst.cs | 86 +++++++++++++++++++ src/Creedengo.Core/Models/Rule.cs | 1 + ...3962.ReplaceStaticReadonlyByConst.Tests.cs | 84 ++++++++++++++++++ 4 files changed, 243 insertions(+) create mode 100644 src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.Fixer.cs create mode 100644 src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.cs create mode 100644 src/Creedengo.Tests/Tests/GCIS3962.ReplaceStaticReadonlyByConst.Tests.cs diff --git a/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.Fixer.cs b/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.Fixer.cs new file mode 100644 index 00000000..4dc2a410 --- /dev/null +++ b/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.Fixer.cs @@ -0,0 +1,72 @@ +namespace Creedengo.Core.Analyzers; + +/// GCIS3962 fixer: Replace static readonly by const when possible. +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ReplaceStaticReadonlyByConstFixer)), Shared] +public sealed class ReplaceStaticReadonlyByConstFixer : CodeFixProvider +{ + /// + public override ImmutableArray FixableDiagnosticIds => _fixableDiagnosticIds; + private static readonly ImmutableArray _fixableDiagnosticIds = [ReplaceStaticReadonlyByConst.Descriptor.Id]; + + /// + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + /// + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + if (context.Diagnostics.Length == 0) return; + + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + if (root is null) return; + + foreach (var diagnostic in context.Diagnostics) + { + var token = root.FindToken(diagnostic.Location.SourceSpan.Start); + var node = token.Parent?.AncestorsAndSelf().OfType().FirstOrDefault(); + if (node is null) continue; + + context.RegisterCodeFix( + CodeAction.Create( + title: "Replace static readonly by const", + createChangedDocument: ct => RefactorAsync(context.Document, node, ct), + equivalenceKey: "Replace static readonly by const"), + diagnostic); + } + } + + private static async Task RefactorAsync(Document document, FieldDeclarationSyntax fieldDecl, CancellationToken token) + { + // Remove static and readonly, add const + var modifiers = fieldDecl.Modifiers; + modifiers = new SyntaxTokenList(modifiers.Where(m => !m.IsKind(SyntaxKind.StaticKeyword) && !m.IsKind(SyntaxKind.ReadOnlyKeyword))); + var constToken = SyntaxFactory.Token(fieldDecl.GetLeadingTrivia(), SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker)); + modifiers = modifiers.Insert(0, constToken); + + // 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(Microsoft.CodeAnalysis.Simplification.Simplifier.Annotation)); + } + } + + var newField = fieldDecl + .WithModifiers(modifiers) + .WithDeclaration(declaration) + .WithAdditionalAnnotations(Formatter.Annotation); + + var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false); + if (root is null) return document; + var newRoot = root.ReplaceNode(fieldDecl, newField); + return document.WithSyntaxRoot(newRoot); + } +} diff --git a/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.cs b/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.cs new file mode 100644 index 00000000..3bbfde70 --- /dev/null +++ b/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.cs @@ -0,0 +1,86 @@ +namespace Creedengo.Core.Analyzers; + +/// GCIS3962: Replace static readonly by const when possible. +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ReplaceStaticReadonlyByConst : DiagnosticAnalyzer +{ + /// The diagnostic descriptor. + public static DiagnosticDescriptor Descriptor { get; } = Rule.CreateDescriptor( + id: Rule.Ids.GCI3962_ReplaceStaticReadonlyByConst, + title: "Replace static readonly by const", + message: "Field '{0}' can be made const", + category: Rule.Categories.Performance, + severity: DiagnosticSeverity.Warning, + description: "Static readonly fields initialized with compile-time constants should be declared as const for better performance."); + + /// + public override ImmutableArray SupportedDiagnostics => _supportedDiagnostics; + private static readonly ImmutableArray _supportedDiagnostics = [Descriptor]; + + /// + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterSyntaxNodeAction(static c => AnalyzeField(c), SyntaxKind.FieldDeclaration); + } + + private static void AnalyzeField(SyntaxNodeAnalysisContext context) + { + var fieldDecl = (FieldDeclarationSyntax)context.Node; + + // Must be static readonly, not already const + if (!fieldDecl.Modifiers.Any(SyntaxKind.StaticKeyword) || + !fieldDecl.Modifiers.Any(SyntaxKind.ReadOnlyKeyword) || + fieldDecl.Modifiers.Any(SyntaxKind.ConstKeyword)) + return; + + // Only one variable per declaration for simplicity + foreach (var variable in fieldDecl.Declaration.Variables) + { + // Must have initializer + if (variable.Initializer is null) + continue; + + var variableSymbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken) as IFieldSymbol; + if (variableSymbol is null) + continue; + + // Only allow types that can be const + if (!IsAllowedConstType(variableSymbol.Type)) + continue; + + // Must be assigned a compile-time constant + var constantValue = context.SemanticModel.GetConstantValue(variable.Initializer.Value, context.CancellationToken); + if (!constantValue.HasValue) + continue; + + context.ReportDiagnostic(Diagnostic.Create(Descriptor, fieldDecl.GetLocation(), variable.Identifier.Text)); + } + } + + 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.Core/Models/Rule.cs b/src/Creedengo.Core/Models/Rule.cs index 0015b636..d4a5cd41 100644 --- a/src/Creedengo.Core/Models/Rule.cs +++ b/src/Creedengo.Core/Models/Rule.cs @@ -27,6 +27,7 @@ public static class Ids public const string GCI91_UseWhereBeforeOrderBy = "GCI91"; public const string GCI92_UseStringEmptyLength = "GCI92"; public const string GCI93_ReturnTaskDirectly = "GCI93"; + public const string GCI3962_ReplaceStaticReadonlyByConst = "GCI3962"; } /// Creates a diagnostic descriptor. diff --git a/src/Creedengo.Tests/Tests/GCIS3962.ReplaceStaticReadonlyByConst.Tests.cs b/src/Creedengo.Tests/Tests/GCIS3962.ReplaceStaticReadonlyByConst.Tests.cs new file mode 100644 index 00000000..4c54712e --- /dev/null +++ b/src/Creedengo.Tests/Tests/GCIS3962.ReplaceStaticReadonlyByConst.Tests.cs @@ -0,0 +1,84 @@ +namespace Creedengo.Tests.Tests; + +[TestClass] +public sealed class ReplaceStaticReadonlyByConstTests +{ + private static readonly CodeFixerDlg VerifyAsync = TestRunner.VerifyAsync; + + [TestMethod] + public Task EmptyCodeAsync() => VerifyAsync(""); + + [TestMethod] + public Task DontWarnOnConstAsync() => VerifyAsync(""" + public class TestClass + { + const int x = 1; + const string s = "Bar"; + } + """); + + [TestMethod] + public Task WarnOnStaticReadonlyIntAsync() => VerifyAsync(""" + public class TestClass + { + [|static readonly int x = 1;|] + } + """, """ + public class TestClass + { + const int x = 1; + } + """); + + [TestMethod] + public Task WarnOnStaticReadonlyStringAsync() => VerifyAsync(""" + public class TestClass + { + [|static readonly string s = "Bar";|] + } + """, """ + public class TestClass + { + const string s = "Bar"; + } + """); + + [TestMethod] + public Task DontWarnOnStaticReadonlyNonConstTypeAsync() => VerifyAsync(""" + public class TestClass + { + static readonly object o = new object(); + } + """); + + [TestMethod] + public Task DontWarnOnStaticReadonlyNonConstantValueAsync() => VerifyAsync(""" + public class TestClass + { + static readonly int x = System.Environment.TickCount; + } + """); + + [TestMethod] + public Task WarnOnMultipleStaticReadonlyFieldsAsync() => VerifyAsync(""" + public class TestClass + { + [|static readonly int a = 1;|] + [|static readonly string b = "foo";|] + } + """, """ + public class TestClass + { + const int a = 1; + const string b = "foo"; + } + """); + + [TestMethod] + public Task DontWarnOnStaticReadonlyWithoutInitializerAsync() => VerifyAsync(""" + public class TestClass + { + static readonly int x; + } + """); +} From 85b4c2cb0e10ad4a5f6e55a23a0cecd1226c8b36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9opaul?= Date: Wed, 21 May 2025 14:55:18 +0200 Subject: [PATCH 2/5] Fusion rules GCI82 and ReplaceStaticReadonlyByConstant --- .../GCI82.VariableCanBeMadeConstant.Fixer.cs | 146 +++++++++++------- .../GCI82.VariableCanBeMadeConstant.cs | 144 ++++++++++++----- ...3962.ReplaceStaticReadonlyByConst.Fixer.cs | 72 --------- .../GCIS3962.ReplaceStaticReadonlyByConst.cs | 86 ----------- src/Creedengo.Core/Models/Rule.cs | 1 - .../GCI82.VariableCanBeMadeConstant.Tests.cs | 74 +++++++++ ...3962.ReplaceStaticReadonlyByConst.Tests.cs | 84 ---------- 7 files changed, 270 insertions(+), 337 deletions(-) delete mode 100644 src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.Fixer.cs delete mode 100644 src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.cs delete mode 100644 src/Creedengo.Tests/Tests/GCIS3962.ReplaceStaticReadonlyByConst.Tests.cs diff --git a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs index b78be58d..25993a6d 100644 --- a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs +++ b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs @@ -1,6 +1,6 @@ namespace Creedengo.Core.Analyzers; -/// GCI82 fixer: Variable can be made constant. +/// GCI82 fixer: Variable can be made constant (local or static readonly field). [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(VariableCanBeMadeConstantFixer)), Shared] public sealed class VariableCanBeMadeConstantFixer : CodeFixProvider { @@ -9,7 +9,7 @@ public sealed class VariableCanBeMadeConstantFixer : CodeFixProvider private static readonly ImmutableArray _fixableDiagnosticIds = [VariableCanBeMadeConstant.Descriptor.Id]; /// - [ExcludeFromCodeCoverage] + [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; /// @@ -22,65 +22,107 @@ 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: "Rendre la variable constante", + 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) { - // Remove the leading trivia from the local declaration. - var firstToken = localDecl.GetFirstToken(); - var leadingTrivia = firstToken.LeadingTrivia; - var trimmedLocal = leadingTrivia.Any() - ? localDecl.ReplaceToken(firstToken, firstToken.WithLeadingTrivia(SyntaxTriviaList.Empty)) - : localDecl; - - // Create a const token with the leading trivia. - var constToken = SyntaxFactory.Token(leadingTrivia, SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker)); - - // If the type of the declaration is 'var', create a new type name for the inferred type. - var varDecl = localDecl.Declaration; - var varTypeName = varDecl.Type; - if (varTypeName.IsVar) - varDecl = await GetDeclarationForVarAsync(document, varDecl, varTypeName, token).ConfigureAwait(false); - - // Produce the new local declaration with an annotation - var formattedLocal = trimmedLocal - .WithModifiers(trimmedLocal.Modifiers.Insert(0, constToken)) // Insert the const token into the modifiers list - .WithDeclaration(varDecl) - .WithAdditionalAnnotations(Formatter.Annotation); - - return await document.WithUpdatedRoot(localDecl, formattedLocal).ConfigureAwait(false); - - static async Task GetDeclarationForVarAsync(Document document, VariableDeclarationSyntax varDecl, TypeSyntax varTypeName, CancellationToken token) + switch (node) { - var semanticModel = await document.GetSemanticModelAsync(token).ConfigureAwait(false); + case LocalDeclarationStatementSyntax localDecl: + { + // Remove the leading trivia from the local declaration. + var firstToken = localDecl.GetFirstToken(); + var leadingTrivia = firstToken.LeadingTrivia; + var trimmedLocal = leadingTrivia.Any() + ? localDecl.ReplaceToken(firstToken, firstToken.WithLeadingTrivia(SyntaxTriviaList.Empty)) + : localDecl; + + // Create a const token with the leading trivia. + var constToken = SyntaxFactory.Token(leadingTrivia, SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker)); + + // If the type of the declaration is 'var', create a new type name for the inferred type. + var varDecl = localDecl.Declaration; + var varTypeName = varDecl.Type; + if (varTypeName.IsVar) + varDecl = await GetDeclarationForVarAsync(document, varDecl, varTypeName, token).ConfigureAwait(false); + + // Produce the new local declaration with an annotation + var formattedLocal = trimmedLocal + .WithModifiers(trimmedLocal.Modifiers.Insert(0, constToken)) // Insert the const token into the modifiers list + .WithDeclaration(varDecl) + .WithAdditionalAnnotations(Formatter.Annotation); + + var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false); + if (root is null) return document; + var newRoot = root.ReplaceNode(localDecl, formattedLocal); + return document.WithSyntaxRoot(newRoot); + } + case FieldDeclarationSyntax fieldDecl: + { + // Remove static and readonly, add const + var modifiers = fieldDecl.Modifiers; + modifiers = new SyntaxTokenList(modifiers.Where(m => !m.IsKind(SyntaxKind.StaticKeyword) && !m.IsKind(SyntaxKind.ReadOnlyKeyword))); + var constToken = SyntaxFactory.Token(fieldDecl.GetLeadingTrivia(), SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker)); + modifiers = modifiers.Insert(0, constToken); - 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) + // 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(Microsoft.CodeAnalysis.Simplification.Simplifier.Annotation)); + } + } - 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' + var newField = fieldDecl + .WithModifiers(modifiers) + .WithDeclaration(declaration) + .WithAdditionalAnnotations(Formatter.Annotation); - // 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)); + var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false); + if (root is null) return document; + var newRoot = root.ReplaceNode(fieldDecl, newField); + return document.WithSyntaxRoot(newRoot); + } + default: + return document; } } + + private static async Task GetDeclarationForVarAsync(Document document, VariableDeclarationSyntax varDecl, TypeSyntax varTypeName, CancellationToken token) + { + var semanticModel = await document.GetSemanticModelAsync(token).ConfigureAwait(false); + + 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..0c7430c5 100644 --- a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs +++ b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs @@ -4,7 +4,7 @@ [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,52 +29,112 @@ public override void Initialize(AnalysisContext context) private static void AnalyzeNode(SyntaxNodeAnalysisContext context) { - var localDeclaration = (LocalDeclarationStatementSyntax)context.Node; + switch (context.Node) + { + case LocalDeclarationStatementSyntax localDeclaration: + { + // Make sure the declaration isn't already const + if (localDeclaration.Modifiers.Any(SyntaxKind.ConstKeyword)) + return; - // Make sure the declaration isn't already const - if (localDeclaration.Modifiers.Any(SyntaxKind.ConstKeyword)) - return; + // Ensure that all variables in the local declaration have initializers that are assigned with constant values + var variableType = context.SemanticModel.GetTypeInfo(localDeclaration.Declaration.Type, context.CancellationToken).ConvertedType; + if (variableType is null) return; + foreach (var variable in localDeclaration.Declaration.Variables) + { + var initializer = variable.Initializer; + if (initializer is null) return; - // Ensure that all variables in the local declaration have initializers that are assigned with constant values - var variableType = context.SemanticModel.GetTypeInfo(localDeclaration.Declaration.Type, context.CancellationToken).ConvertedType; - if (variableType is null) return; - foreach (var variable in localDeclaration.Declaration.Variables) - { - var initializer = variable.Initializer; - if (initializer is null) return; - - var constantValue = context.SemanticModel.GetConstantValue(initializer.Value, context.CancellationToken); - if (!constantValue.HasValue) return; - - // Ensure that the initializer value can be converted to the type of the local declaration without a user-defined conversion. - var conversion = context.SemanticModel.ClassifyConversion(initializer.Value, variableType); - if (!conversion.Exists || conversion.IsUserDefined) return; - - // Special cases: - // * If the constant value is a string, the type of the local declaration must be string - // * If the constant value is null, the type of the local declaration must be a reference type - if (constantValue.Value is string) - { - if (variableType.SpecialType is not SpecialType.System_String) return; - } - else if (variableType.IsReferenceType && constantValue.Value is not null) - { - return; - } - } + var constantValue = context.SemanticModel.GetConstantValue(initializer.Value, context.CancellationToken); + if (!constantValue.HasValue) return; - // Perform data flow analysis on the local declaration - var dataFlowAnalysis = context.SemanticModel.AnalyzeDataFlow(localDeclaration); - if (dataFlowAnalysis is null) return; + // Ensure that the initializer value can be converted to the type of the local declaration without a user-defined conversion. + var conversion = context.SemanticModel.ClassifyConversion(initializer.Value, variableType); + if (!conversion.Exists || conversion.IsUserDefined) return; - foreach (var variable in localDeclaration.Declaration.Variables) - { - // Retrieve the local symbol for each variable in the local declaration and ensure that it is not written outside of the data flow analysis region - var variableSymbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken); - if (variableSymbol is null || dataFlowAnalysis.WrittenOutside.Contains(variableSymbol)) - return; + // Special cases: + // * If the constant value is a string, the type of the local declaration must be string + // * If the constant value is null, the type of the local declaration must be a reference type + if (constantValue.Value is string) + { + if (variableType.SpecialType is not SpecialType.System_String) return; + } + else if (variableType.IsReferenceType && constantValue.Value is not null) + { + return; + } + } + + // Perform data flow analysis on the local declaration + var dataFlowAnalysis = context.SemanticModel.AnalyzeDataFlow(localDeclaration); + if (dataFlowAnalysis is null) return; + + foreach (var variable in localDeclaration.Declaration.Variables) + { + // Retrieve the local symbol for each variable in the local declaration and ensure that it is not written outside of the data flow analysis region + var variableSymbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken); + if (variableSymbol is null || dataFlowAnalysis.WrittenOutside.Contains(variableSymbol)) + return; + } + + context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.GetLocation())); + break; + } + case 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())); + break; + } } + } - context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.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.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.Fixer.cs b/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.Fixer.cs deleted file mode 100644 index 4dc2a410..00000000 --- a/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.Fixer.cs +++ /dev/null @@ -1,72 +0,0 @@ -namespace Creedengo.Core.Analyzers; - -/// GCIS3962 fixer: Replace static readonly by const when possible. -[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ReplaceStaticReadonlyByConstFixer)), Shared] -public sealed class ReplaceStaticReadonlyByConstFixer : CodeFixProvider -{ - /// - public override ImmutableArray FixableDiagnosticIds => _fixableDiagnosticIds; - private static readonly ImmutableArray _fixableDiagnosticIds = [ReplaceStaticReadonlyByConst.Descriptor.Id]; - - /// - public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; - - /// - public override async Task RegisterCodeFixesAsync(CodeFixContext context) - { - if (context.Diagnostics.Length == 0) return; - - var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - if (root is null) return; - - foreach (var diagnostic in context.Diagnostics) - { - var token = root.FindToken(diagnostic.Location.SourceSpan.Start); - var node = token.Parent?.AncestorsAndSelf().OfType().FirstOrDefault(); - if (node is null) continue; - - context.RegisterCodeFix( - CodeAction.Create( - title: "Replace static readonly by const", - createChangedDocument: ct => RefactorAsync(context.Document, node, ct), - equivalenceKey: "Replace static readonly by const"), - diagnostic); - } - } - - private static async Task RefactorAsync(Document document, FieldDeclarationSyntax fieldDecl, CancellationToken token) - { - // Remove static and readonly, add const - var modifiers = fieldDecl.Modifiers; - modifiers = new SyntaxTokenList(modifiers.Where(m => !m.IsKind(SyntaxKind.StaticKeyword) && !m.IsKind(SyntaxKind.ReadOnlyKeyword))); - var constToken = SyntaxFactory.Token(fieldDecl.GetLeadingTrivia(), SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker)); - modifiers = modifiers.Insert(0, constToken); - - // 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(Microsoft.CodeAnalysis.Simplification.Simplifier.Annotation)); - } - } - - var newField = fieldDecl - .WithModifiers(modifiers) - .WithDeclaration(declaration) - .WithAdditionalAnnotations(Formatter.Annotation); - - var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false); - if (root is null) return document; - var newRoot = root.ReplaceNode(fieldDecl, newField); - return document.WithSyntaxRoot(newRoot); - } -} diff --git a/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.cs b/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.cs deleted file mode 100644 index 3bbfde70..00000000 --- a/src/Creedengo.Core/Analyzers/GCIS3962.ReplaceStaticReadonlyByConst.cs +++ /dev/null @@ -1,86 +0,0 @@ -namespace Creedengo.Core.Analyzers; - -/// GCIS3962: Replace static readonly by const when possible. -[DiagnosticAnalyzer(LanguageNames.CSharp)] -public sealed class ReplaceStaticReadonlyByConst : DiagnosticAnalyzer -{ - /// The diagnostic descriptor. - public static DiagnosticDescriptor Descriptor { get; } = Rule.CreateDescriptor( - id: Rule.Ids.GCI3962_ReplaceStaticReadonlyByConst, - title: "Replace static readonly by const", - message: "Field '{0}' can be made const", - category: Rule.Categories.Performance, - severity: DiagnosticSeverity.Warning, - description: "Static readonly fields initialized with compile-time constants should be declared as const for better performance."); - - /// - public override ImmutableArray SupportedDiagnostics => _supportedDiagnostics; - private static readonly ImmutableArray _supportedDiagnostics = [Descriptor]; - - /// - public override void Initialize(AnalysisContext context) - { - context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); - context.EnableConcurrentExecution(); - context.RegisterSyntaxNodeAction(static c => AnalyzeField(c), SyntaxKind.FieldDeclaration); - } - - private static void AnalyzeField(SyntaxNodeAnalysisContext context) - { - var fieldDecl = (FieldDeclarationSyntax)context.Node; - - // Must be static readonly, not already const - if (!fieldDecl.Modifiers.Any(SyntaxKind.StaticKeyword) || - !fieldDecl.Modifiers.Any(SyntaxKind.ReadOnlyKeyword) || - fieldDecl.Modifiers.Any(SyntaxKind.ConstKeyword)) - return; - - // Only one variable per declaration for simplicity - foreach (var variable in fieldDecl.Declaration.Variables) - { - // Must have initializer - if (variable.Initializer is null) - continue; - - var variableSymbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken) as IFieldSymbol; - if (variableSymbol is null) - continue; - - // Only allow types that can be const - if (!IsAllowedConstType(variableSymbol.Type)) - continue; - - // Must be assigned a compile-time constant - var constantValue = context.SemanticModel.GetConstantValue(variable.Initializer.Value, context.CancellationToken); - if (!constantValue.HasValue) - continue; - - context.ReportDiagnostic(Diagnostic.Create(Descriptor, fieldDecl.GetLocation(), variable.Identifier.Text)); - } - } - - 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.Core/Models/Rule.cs b/src/Creedengo.Core/Models/Rule.cs index d4a5cd41..0015b636 100644 --- a/src/Creedengo.Core/Models/Rule.cs +++ b/src/Creedengo.Core/Models/Rule.cs @@ -27,7 +27,6 @@ public static class Ids public const string GCI91_UseWhereBeforeOrderBy = "GCI91"; public const string GCI92_UseStringEmptyLength = "GCI92"; public const string GCI93_ReturnTaskDirectly = "GCI93"; - public const string GCI3962_ReplaceStaticReadonlyByConst = "GCI3962"; } /// Creates a diagnostic descriptor. diff --git a/src/Creedengo.Tests/Tests/GCI82.VariableCanBeMadeConstant.Tests.cs b/src/Creedengo.Tests/Tests/GCI82.VariableCanBeMadeConstant.Tests.cs index 2951ee97..67195a00 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 + { + const int x = 1; + const string s = "Bar"; + } + """); + + [TestMethod] + public Task WarnOnStaticReadonlyIntAsync() => VerifyAsync(""" + public class TestClass + { + [|static readonly int x = 1;|] + } + """, """ + public class TestClass + { + const int x = 1; + } + """); + + [TestMethod] + public Task WarnOnStaticReadonlyStringAsync() => VerifyAsync(""" + public class TestClass + { + [|static readonly string s = "Bar";|] + } + """, """ + public class TestClass + { + const string s = "Bar"; + } + """); + + [TestMethod] + public Task DontWarnOnStaticReadonlyNonConstTypeAsync() => VerifyAsync(""" + public class TestClass + { + static readonly object o = new object(); + } + """); + + [TestMethod] + public Task DontWarnOnStaticReadonlyNonConstantValueAsync() => VerifyAsync(""" + public class TestClass + { + static readonly int x = System.Environment.TickCount; + } + """); + + [TestMethod] + public Task WarnOnMultipleStaticReadonlyFieldsAsync() => VerifyAsync(""" + public class TestClass + { + [|static readonly int a = 1;|] + [|static readonly string b = "foo";|] + } + """, """ + public class TestClass + { + const int a = 1; + const string b = "foo"; + } + """); + + [TestMethod] + public Task DontWarnOnStaticReadonlyWithoutInitializerAsync() => VerifyAsync(""" + public class TestClass + { + static readonly int x; + } + """); + [TestMethod] public Task VariableCanBeConstAsync() => VerifyAsync(""" using System; diff --git a/src/Creedengo.Tests/Tests/GCIS3962.ReplaceStaticReadonlyByConst.Tests.cs b/src/Creedengo.Tests/Tests/GCIS3962.ReplaceStaticReadonlyByConst.Tests.cs deleted file mode 100644 index 4c54712e..00000000 --- a/src/Creedengo.Tests/Tests/GCIS3962.ReplaceStaticReadonlyByConst.Tests.cs +++ /dev/null @@ -1,84 +0,0 @@ -namespace Creedengo.Tests.Tests; - -[TestClass] -public sealed class ReplaceStaticReadonlyByConstTests -{ - private static readonly CodeFixerDlg VerifyAsync = TestRunner.VerifyAsync; - - [TestMethod] - public Task EmptyCodeAsync() => VerifyAsync(""); - - [TestMethod] - public Task DontWarnOnConstAsync() => VerifyAsync(""" - public class TestClass - { - const int x = 1; - const string s = "Bar"; - } - """); - - [TestMethod] - public Task WarnOnStaticReadonlyIntAsync() => VerifyAsync(""" - public class TestClass - { - [|static readonly int x = 1;|] - } - """, """ - public class TestClass - { - const int x = 1; - } - """); - - [TestMethod] - public Task WarnOnStaticReadonlyStringAsync() => VerifyAsync(""" - public class TestClass - { - [|static readonly string s = "Bar";|] - } - """, """ - public class TestClass - { - const string s = "Bar"; - } - """); - - [TestMethod] - public Task DontWarnOnStaticReadonlyNonConstTypeAsync() => VerifyAsync(""" - public class TestClass - { - static readonly object o = new object(); - } - """); - - [TestMethod] - public Task DontWarnOnStaticReadonlyNonConstantValueAsync() => VerifyAsync(""" - public class TestClass - { - static readonly int x = System.Environment.TickCount; - } - """); - - [TestMethod] - public Task WarnOnMultipleStaticReadonlyFieldsAsync() => VerifyAsync(""" - public class TestClass - { - [|static readonly int a = 1;|] - [|static readonly string b = "foo";|] - } - """, """ - public class TestClass - { - const int a = 1; - const string b = "foo"; - } - """); - - [TestMethod] - public Task DontWarnOnStaticReadonlyWithoutInitializerAsync() => VerifyAsync(""" - public class TestClass - { - static readonly int x; - } - """); -} From 14a8743d7b5ba3a95814713e7a758b9d26869f8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9opaul?= Date: Wed, 21 May 2025 15:00:11 +0200 Subject: [PATCH 3/5] fix rule --- src/Creedengo.Core/Models/Rule.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Creedengo.Core/Models/Rule.cs b/src/Creedengo.Core/Models/Rule.cs index 063ba450..f0b6a60c 100644 --- a/src/Creedengo.Core/Models/Rule.cs +++ b/src/Creedengo.Core/Models/Rule.cs @@ -27,7 +27,6 @@ public static class Ids public const string GCI91_UseWhereBeforeOrderBy = "GCI91"; public const string GCI92_UseStringEmptyLength = "GCI92"; public const string GCI93_ReturnTaskDirectly = "GCI93"; - public const string GCI3962_ReplaceStaticReadonlyByConst = "GCI3962"; public const string GCIACV_NonReadOnlyStruct = "GCIACV"; public const string GCI95_UseIsOperatorInsteadOfAsOperator = "GCI95"; public const string GCIXX_UnecessaryAssignment = "GCIXX"; From 6a4a33ec9ee860dd42f44557afbe851d74353d55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9opaul?= Date: Wed, 21 May 2025 15:19:30 +0200 Subject: [PATCH 4/5] refacto --- .../GCI82.VariableCanBeMadeConstant.Fixer.cs | 136 +++++++------- .../GCI82.VariableCanBeMadeConstant.cs | 167 +++++++++--------- 2 files changed, 158 insertions(+), 145 deletions(-) diff --git a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs index 25993a6d..37534a7b 100644 --- a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs +++ b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs @@ -1,6 +1,6 @@ namespace Creedengo.Core.Analyzers; -/// GCI82 fixer: Variable can be made constant (local or static readonly field). +/// GCI82 fixer: Variable can be made constant. [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(VariableCanBeMadeConstantFixer)), Shared] public sealed class VariableCanBeMadeConstantFixer : CodeFixProvider { @@ -9,7 +9,7 @@ public sealed class VariableCanBeMadeConstantFixer : CodeFixProvider private static readonly ImmutableArray _fixableDiagnosticIds = [VariableCanBeMadeConstant.Descriptor.Id]; /// - [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] + [ExcludeFromCodeCoverage] public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; /// @@ -28,7 +28,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) context.RegisterCodeFix( CodeAction.Create( - title: "Rendre la variable constante", + title: "Make variable constant", createChangedDocument: ct => RefactorAsync(context.Document, node, ct), equivalenceKey: "Make variable constant"), diagnostic); @@ -40,74 +40,80 @@ private static async Task RefactorAsync(Document document, SyntaxNode switch (node) { case LocalDeclarationStatementSyntax localDecl: - { - // Remove the leading trivia from the local declaration. - var firstToken = localDecl.GetFirstToken(); - var leadingTrivia = firstToken.LeadingTrivia; - var trimmedLocal = leadingTrivia.Any() - ? localDecl.ReplaceToken(firstToken, firstToken.WithLeadingTrivia(SyntaxTriviaList.Empty)) - : localDecl; - - // Create a const token with the leading trivia. - var constToken = SyntaxFactory.Token(leadingTrivia, SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker)); - - // If the type of the declaration is 'var', create a new type name for the inferred type. - var varDecl = localDecl.Declaration; - var varTypeName = varDecl.Type; - if (varTypeName.IsVar) - varDecl = await GetDeclarationForVarAsync(document, varDecl, varTypeName, token).ConfigureAwait(false); - - // Produce the new local declaration with an annotation - var formattedLocal = trimmedLocal - .WithModifiers(trimmedLocal.Modifiers.Insert(0, constToken)) // Insert the const token into the modifiers list - .WithDeclaration(varDecl) - .WithAdditionalAnnotations(Formatter.Annotation); - - var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false); - if (root is null) return document; - var newRoot = root.ReplaceNode(localDecl, formattedLocal); - return document.WithSyntaxRoot(newRoot); - } + return await RefactorLocalAsync(document, localDecl, token).ConfigureAwait(false); case FieldDeclarationSyntax fieldDecl: - { - // Remove static and readonly, add const - var modifiers = fieldDecl.Modifiers; - modifiers = new SyntaxTokenList(modifiers.Where(m => !m.IsKind(SyntaxKind.StaticKeyword) && !m.IsKind(SyntaxKind.ReadOnlyKeyword))); - var constToken = SyntaxFactory.Token(fieldDecl.GetLeadingTrivia(), SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker)); - modifiers = modifiers.Insert(0, constToken); - - // 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(Microsoft.CodeAnalysis.Simplification.Simplifier.Annotation)); - } - } - - var newField = fieldDecl - .WithModifiers(modifiers) - .WithDeclaration(declaration) - .WithAdditionalAnnotations(Formatter.Annotation); - - var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false); - if (root is null) return document; - var newRoot = root.ReplaceNode(fieldDecl, newField); - return document.WithSyntaxRoot(newRoot); - } + 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(); + var leadingTrivia = firstToken.LeadingTrivia; + var trimmedLocal = leadingTrivia.Any() + ? localDecl.ReplaceToken(firstToken, firstToken.WithLeadingTrivia(SyntaxTriviaList.Empty)) + : localDecl; + + // Create a const token with the leading trivia. + var constToken = SyntaxFactory.Token(leadingTrivia, SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker)); + + // If the type of the declaration is 'var', create a new type name for the inferred type. + var varDecl = localDecl.Declaration; + var varTypeName = varDecl.Type; + if (varTypeName.IsVar) + varDecl = await GetDeclarationForVarAsync(document, varDecl, varTypeName, token).ConfigureAwait(false); + + // Produce the new local declaration with an annotation + var formattedLocal = trimmedLocal + .WithModifiers(trimmedLocal.Modifiers.Insert(0, constToken)) // Insert the const token into the modifiers list + .WithDeclaration(varDecl) + .WithAdditionalAnnotations(Formatter.Annotation); + + 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 + var modifiers = fieldDecl.Modifiers; + modifiers = [.. modifiers.Where(m => !m.IsKind(SyntaxKind.StaticKeyword) && !m.IsKind(SyntaxKind.ReadOnlyKeyword))]; + var constToken = SyntaxFactory.Token(fieldDecl.GetLeadingTrivia(), SyntaxKind.ConstKeyword, SyntaxFactory.TriviaList(SyntaxFactory.ElasticMarker)); + modifiers = modifiers.Insert(0, constToken); + + // 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(modifiers) + .WithDeclaration(declaration) + .WithAdditionalAnnotations(Formatter.Annotation); + + var root = await document.GetSyntaxRootAsync(token).ConfigureAwait(false); + if (root is null) return document; + var newRoot = root.ReplaceNode(fieldDecl, newField); + return document.WithSyntaxRoot(newRoot); + } + private static async Task GetDeclarationForVarAsync(Document document, VariableDeclarationSyntax varDecl, TypeSyntax varTypeName, CancellationToken token) { var semanticModel = await document.GetSemanticModelAsync(token).ConfigureAwait(false); diff --git a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs index 0c7430c5..82372c96 100644 --- a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs +++ b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.cs @@ -1,4 +1,5 @@ -namespace Creedengo.Core.Analyzers; + +namespace Creedengo.Core.Analyzers; /// GCI82: Variable can be made constant. [DiagnosticAnalyzer(LanguageNames.CSharp)] @@ -32,88 +33,94 @@ private static void AnalyzeNode(SyntaxNodeAnalysisContext context) switch (context.Node) { case LocalDeclarationStatementSyntax localDeclaration: - { - // Make sure the declaration isn't already const - if (localDeclaration.Modifiers.Any(SyntaxKind.ConstKeyword)) - return; - - // Ensure that all variables in the local declaration have initializers that are assigned with constant values - var variableType = context.SemanticModel.GetTypeInfo(localDeclaration.Declaration.Type, context.CancellationToken).ConvertedType; - if (variableType is null) return; - foreach (var variable in localDeclaration.Declaration.Variables) - { - var initializer = variable.Initializer; - if (initializer is null) return; - - var constantValue = context.SemanticModel.GetConstantValue(initializer.Value, context.CancellationToken); - if (!constantValue.HasValue) return; - - // Ensure that the initializer value can be converted to the type of the local declaration without a user-defined conversion. - var conversion = context.SemanticModel.ClassifyConversion(initializer.Value, variableType); - if (!conversion.Exists || conversion.IsUserDefined) return; - - // Special cases: - // * If the constant value is a string, the type of the local declaration must be string - // * If the constant value is null, the type of the local declaration must be a reference type - if (constantValue.Value is string) - { - if (variableType.SpecialType is not SpecialType.System_String) return; - } - else if (variableType.IsReferenceType && constantValue.Value is not null) - { - return; - } - } - - // Perform data flow analysis on the local declaration - var dataFlowAnalysis = context.SemanticModel.AnalyzeDataFlow(localDeclaration); - if (dataFlowAnalysis is null) return; - - foreach (var variable in localDeclaration.Declaration.Variables) - { - // Retrieve the local symbol for each variable in the local declaration and ensure that it is not written outside of the data flow analysis region - var variableSymbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken); - if (variableSymbol is null || dataFlowAnalysis.WrittenOutside.Contains(variableSymbol)) - return; - } - - context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.GetLocation())); - break; - } - case 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())); - break; - } + 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; + + // Ensure that all variables in the local declaration have initializers that are assigned with constant values + var variableType = context.SemanticModel.GetTypeInfo(localDeclaration.Declaration.Type, context.CancellationToken).ConvertedType; + if (variableType is null) return; + foreach (var variable in localDeclaration.Declaration.Variables) + { + var initializer = variable.Initializer; + if (initializer is null) return; + + var constantValue = context.SemanticModel.GetConstantValue(initializer.Value, context.CancellationToken); + if (!constantValue.HasValue) return; + + // Ensure that the initializer value can be converted to the type of the local declaration without a user-defined conversion. + var conversion = context.SemanticModel.ClassifyConversion(initializer.Value, variableType); + if (!conversion.Exists || conversion.IsUserDefined) return; + + // Special cases: + // * If the constant value is a string, the type of the local declaration must be string + // * If the constant value is null, the type of the local declaration must be a reference type + if (constantValue.Value is string) + { + if (variableType.SpecialType is not SpecialType.System_String) return; + } + else if (variableType.IsReferenceType && constantValue.Value is not null) + { + return; + } + } + + // Perform data flow analysis on the local declaration + var dataFlowAnalysis = context.SemanticModel.AnalyzeDataFlow(localDeclaration); + if (dataFlowAnalysis is null) return; + + foreach (var variable in localDeclaration.Declaration.Variables) + { + // Retrieve the local symbol for each variable in the local declaration and ensure that it is not written outside of the data flow analysis region + var variableSymbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken); + if (variableSymbol is null || dataFlowAnalysis.WrittenOutside.Contains(variableSymbol)) + return; + } + + 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) From 52faf8e1c372b60489dbf614b8f45966cd215510 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9opaul?= Date: Wed, 21 May 2025 15:31:17 +0200 Subject: [PATCH 5/5] correctif test --- .../GCI82.VariableCanBeMadeConstant.Fixer.cs | 23 +++++++++++++--- .../GCI82.VariableCanBeMadeConstant.Tests.cs | 26 +++++++++---------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs index 37534a7b..059f0c2a 100644 --- a/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs +++ b/src/Creedengo.Core/Analyzers/GCI82.VariableCanBeMadeConstant.Fixer.cs @@ -80,11 +80,26 @@ private static async Task RefactorLocalAsync(Document document, LocalD private static async Task RefactorFieldAsync(Document document, FieldDeclarationSyntax fieldDecl, CancellationToken token) { - // Remove static and readonly, add const + // Remove static and readonly, add const after access modifiers var modifiers = fieldDecl.Modifiers; - modifiers = [.. modifiers.Where(m => !m.IsKind(SyntaxKind.StaticKeyword) && !m.IsKind(SyntaxKind.ReadOnlyKeyword))]; + 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)); - modifiers = modifiers.Insert(0, constToken); + var newModifiers = new SyntaxTokenList(); + newModifiers = newModifiers.AddRange(accessModifiers); + newModifiers = newModifiers.Add(constToken); + newModifiers = newModifiers.AddRange(otherModifiers); // If the type is 'var', replace with the actual type var declaration = fieldDecl.Declaration; @@ -104,7 +119,7 @@ private static async Task RefactorFieldAsync(Document document, FieldD } var newField = fieldDecl - .WithModifiers(modifiers) + .WithModifiers(newModifiers) .WithDeclaration(declaration) .WithAdditionalAnnotations(Formatter.Annotation); diff --git a/src/Creedengo.Tests/Tests/GCI82.VariableCanBeMadeConstant.Tests.cs b/src/Creedengo.Tests/Tests/GCI82.VariableCanBeMadeConstant.Tests.cs index 67195a00..626452d5 100644 --- a/src/Creedengo.Tests/Tests/GCI82.VariableCanBeMadeConstant.Tests.cs +++ b/src/Creedengo.Tests/Tests/GCI82.VariableCanBeMadeConstant.Tests.cs @@ -12,8 +12,8 @@ public sealed class VariableCanBeMadeConstantTests public Task DontWarnOnConstAsync() => VerifyAsync(""" public class TestClass { - const int x = 1; - const string s = "Bar"; + private const int x = 1; + private const string s = "Bar"; } """); @@ -21,12 +21,12 @@ public class TestClass public Task WarnOnStaticReadonlyIntAsync() => VerifyAsync(""" public class TestClass { - [|static readonly int x = 1;|] + [|private static readonly int x = 1;|] } """, """ public class TestClass { - const int x = 1; + private const int x = 1; } """); @@ -34,12 +34,12 @@ public class TestClass public Task WarnOnStaticReadonlyStringAsync() => VerifyAsync(""" public class TestClass { - [|static readonly string s = "Bar";|] + [|private static readonly string s = "Bar";|] } """, """ public class TestClass { - const string s = "Bar"; + private const string s = "Bar"; } """); @@ -47,7 +47,7 @@ public class TestClass public Task DontWarnOnStaticReadonlyNonConstTypeAsync() => VerifyAsync(""" public class TestClass { - static readonly object o = new object(); + private static readonly object o = new object(); } """); @@ -55,7 +55,7 @@ public class TestClass public Task DontWarnOnStaticReadonlyNonConstantValueAsync() => VerifyAsync(""" public class TestClass { - static readonly int x = System.Environment.TickCount; + private static readonly int x = System.Environment.TickCount; } """); @@ -63,14 +63,14 @@ public class TestClass public Task WarnOnMultipleStaticReadonlyFieldsAsync() => VerifyAsync(""" public class TestClass { - [|static readonly int a = 1;|] - [|static readonly string b = "foo";|] + [|private static readonly int a = 1;|] + [|private static readonly string b = "foo";|] } """, """ public class TestClass { - const int a = 1; - const string b = "foo"; + private const int a = 1; + private const string b = "foo"; } """); @@ -78,7 +78,7 @@ public class TestClass public Task DontWarnOnStaticReadonlyWithoutInitializerAsync() => VerifyAsync(""" public class TestClass { - static readonly int x; + private static readonly int x; } """);