Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEMO: Localization source generators and analyzers #2723

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions Flow.Launcher.Analyzers/AnalyzerDiagnostics.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
using Microsoft.CodeAnalysis;

namespace Flow.Launcher.Analyzers
{
public static class AnalyzerDiagnostics
{
public static readonly DiagnosticDescriptor OldLocalizationApiUsed = new DiagnosticDescriptor(
"FLAN0001",
"Old localization API used",
"Use `Localize.{0}({1})` instead",
"Localization",
DiagnosticSeverity.Warning,
isEnabledByDefault: true
);

public static readonly DiagnosticDescriptor ContextIsAField = new DiagnosticDescriptor(
"FLAN0002",
"Plugin context is a field",
"Plugin context must be at least internal static property",
"Localization",
DiagnosticSeverity.Error,
isEnabledByDefault: true
);

public static readonly DiagnosticDescriptor ContextIsNotStatic = new DiagnosticDescriptor(
"FLAN0003",
"Plugin context is not static",
"Plugin context must be at least internal static property",
"Localization",
DiagnosticSeverity.Error,
isEnabledByDefault: true
);

public static readonly DiagnosticDescriptor ContextAccessIsTooRestrictive = new DiagnosticDescriptor(
"FLAN0004",
"Plugin context property access modifier is too restrictive",
"Plugin context property must be at least internal static property",
"Localization",
DiagnosticSeverity.Error,
isEnabledByDefault: true
);

public static readonly DiagnosticDescriptor ContextIsNotDeclared = new DiagnosticDescriptor(
"FLAN0005",
"Plugin context is not declared",
"Plugin context must be at least internal static property of type `PluginInitContext`",
"Localization",
DiagnosticSeverity.Error,
isEnabledByDefault: true
);
}
}
2 changes: 2 additions & 0 deletions Flow.Launcher.Analyzers/AnalyzerReleases.Shipped.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
; Shipped analyzer releases
; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md
11 changes: 11 additions & 0 deletions Flow.Launcher.Analyzers/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
; Unshipped analyzer release
; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md
### New Rules

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
FLAN0001 | Localization | Warning | FLAN0001_OldLocalizationApiUsed
FLAN0002 | Localization | Error | FLAN0002_ContextIsAField
FLAN0003 | Localization | Error | FLAN0003_ContextIsNotStatic
FLAN0004 | Localization | Error | FLAN0004_ContextAccessIsTooRestrictive
FLAN0005 | Localization | Error | FLAN0005_ContextIsNotDeclared
18 changes: 18 additions & 0 deletions Flow.Launcher.Analyzers/Flow.Launcher.Analyzers.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<Version>1.0.0</Version>
<TargetFramework>netstandard2.0</TargetFramework>
<EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.9.2"/>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.9.2"/>
</ItemGroup>

</Project>
94 changes: 94 additions & 0 deletions Flow.Launcher.Analyzers/Localize/ContextAvailabilityAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Flow.Launcher.Analyzers.Localize
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class ContextAvailabilityAnalyzer : DiagnosticAnalyzer
{
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(
AnalyzerDiagnostics.ContextIsAField,
AnalyzerDiagnostics.ContextIsNotStatic,
AnalyzerDiagnostics.ContextAccessIsTooRestrictive,
AnalyzerDiagnostics.ContextIsNotDeclared
);

private const string PluginContextTypeName = "PluginInitContext";

private const string PluginInterfaceName = "IPluginI18n";

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exclude generated code from analysis

Currently, the analyzer is configured to analyze generated code, which may produce unnecessary diagnostics and impact performance. Typically, analyzers exclude generated code to focus on user-written code.

Update the configuration to exclude generated code:

context.ConfigureGeneratedCodeAnalysis(
-    GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics
+    GeneratedCodeAnalysisFlags.None
);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);

context.EnableConcurrentExecution();
context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.ClassDeclaration);
}

private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
{
var classDeclaration = (ClassDeclarationSyntax)context.Node;
var semanticModel = context.SemanticModel;
var classSymbol = semanticModel.GetDeclaredSymbol(classDeclaration);

if (!IsPluginEntryClass(classSymbol)) return;

var contextProperty = classDeclaration.Members.OfType<PropertyDeclarationSyntax>()
.Select(p => semanticModel.GetDeclaredSymbol(p))
.FirstOrDefault(p => p?.Type.Name is PluginContextTypeName);
Comment on lines +39 to +41
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix incorrect string comparison using 'is' operator

The code uses the is operator for string comparison, which is incorrect. The is operator checks for type compatibility, not value equality. Use == for string comparison to ensure correct functionality.

Apply this diff to fix the comparison:

- .FirstOrDefault(p => p?.Type.Name is PluginContextTypeName);
+ .FirstOrDefault(p => p?.Type.Name == PluginContextTypeName);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var contextProperty = classDeclaration.Members.OfType<PropertyDeclarationSyntax>()
.Select(p => semanticModel.GetDeclaredSymbol(p))
.FirstOrDefault(p => p?.Type.Name is PluginContextTypeName);
var contextProperty = classDeclaration.Members.OfType<PropertyDeclarationSyntax>()
.Select(p => semanticModel.GetDeclaredSymbol(p))
.FirstOrDefault(p => p?.Type.Name == PluginContextTypeName);


if (contextProperty != null)
{
if (!contextProperty.IsStatic)
{
context.ReportDiagnostic(Diagnostic.Create(
AnalyzerDiagnostics.ContextIsNotStatic,
contextProperty.DeclaringSyntaxReferences[0].GetSyntax().GetLocation()
));
Comment on lines +47 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prevent potential IndexOutOfRangeException when accessing DeclaringSyntaxReferences

Accessing DeclaringSyntaxReferences[0] without checking if the array has elements can lead to a runtime exception if the array is empty. Ensure that the array has elements before accessing the first item.

Apply this diff to add a null check:

if (!contextProperty.IsStatic)
{
-    context.ReportDiagnostic(Diagnostic.Create(
-        AnalyzerDiagnostics.ContextIsNotStatic,
-        contextProperty.DeclaringSyntaxReferences[0].GetSyntax().GetLocation()
-    ));
+    var syntaxReference = contextProperty.DeclaringSyntaxReferences.FirstOrDefault();
+    if (syntaxReference != null)
+    {
+        context.ReportDiagnostic(Diagnostic.Create(
+            AnalyzerDiagnostics.ContextIsNotStatic,
+            syntaxReference.GetSyntax().GetLocation()
+        ));
+    }
}

Repeat similar checks for other instances where DeclaringSyntaxReferences[0] is accessed.

Also applies to: 57-59, 85-88

return;
}

if (contextProperty.DeclaredAccessibility is Accessibility.Private || contextProperty.DeclaredAccessibility is Accessibility.Protected)
{
context.ReportDiagnostic(Diagnostic.Create(
AnalyzerDiagnostics.ContextAccessIsTooRestrictive,
contextProperty.DeclaringSyntaxReferences[0].GetSyntax().GetLocation()
));
return;
}

return;
}

var fieldDeclaration = classDeclaration.Members
.OfType<FieldDeclarationSyntax>()
.SelectMany(f => f.Declaration.Variables)
.Select(f => semanticModel.GetDeclaredSymbol(f))
.FirstOrDefault(f => f is IFieldSymbol fs && fs.Type.Name is PluginContextTypeName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix incorrect string comparison using 'is' operator

Similar to the previous issue, the is operator is incorrectly used for comparing strings. Replace is with == to compare string values correctly.

Apply this diff to fix the comparison:

- .FirstOrDefault(f => f is IFieldSymbol fs && fs.Type.Name is PluginContextTypeName);
+ .FirstOrDefault(f => f is IFieldSymbol fs && fs.Type.Name == PluginContextTypeName);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.FirstOrDefault(f => f is IFieldSymbol fs && fs.Type.Name is PluginContextTypeName);
.FirstOrDefault(f => f is IFieldSymbol fs && fs.Type.Name == PluginContextTypeName);

var parentSyntax = fieldDeclaration
?.DeclaringSyntaxReferences[0]
.GetSyntax()
.FirstAncestorOrSelf<FieldDeclarationSyntax>();

if (parentSyntax != null)
{
context.ReportDiagnostic(Diagnostic.Create(
AnalyzerDiagnostics.ContextIsAField,
parentSyntax.GetLocation()
));
return;
}

context.ReportDiagnostic(Diagnostic.Create(
AnalyzerDiagnostics.ContextIsNotDeclared,
classDeclaration.Identifier.GetLocation()
));
}

private static bool IsPluginEntryClass(INamedTypeSymbol namedTypeSymbol) =>
namedTypeSymbol?.Interfaces.Any(i => i.Name == PluginInterfaceName) ?? false;
Comment on lines +91 to +92
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compare interface symbols using SymbolEqualityComparer

When checking if a class implements an interface, comparing symbols directly is more reliable than comparing names. This approach accounts for namespace differences and ensures accurate type comparisons.

Refactor the IsPluginEntryClass method:

-private static bool IsPluginEntryClass(INamedTypeSymbol namedTypeSymbol) =>
-    namedTypeSymbol?.Interfaces.Any(i => i.Name == PluginInterfaceName) ?? false;
+private static bool IsPluginEntryClass(INamedTypeSymbol namedTypeSymbol)
+{
+    var pluginInterface = context.Compilation.GetTypeByMetadataName("YourNamespace.IPluginI18n");
+    return namedTypeSymbol.Interfaces.Contains(pluginInterface, SymbolEqualityComparer.Default);
+}

Replace "YourNamespace.IPluginI18n" with the fully qualified name of the IPluginI18n interface.

Committable suggestion was skipped due to low confidence.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
using System.Collections.Immutable;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the Byte Order Mark (BOM) at the beginning of the file

The file appears to start with a Byte Order Mark (BOM), represented by an unexpected character before the using statement. This can cause issues with some compilers and tools. It's recommended to remove the BOM for consistent encoding.

Apply this diff to remove the BOM:

-using System.Collections.Immutable;
+using System.Collections.Immutable;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
using System.Collections.Immutable;
using System.Collections.Immutable;

using System.Composition;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Simplification;
using Microsoft.CodeAnalysis.Text;

namespace Flow.Launcher.Analyzers.Localize
{
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ContextAvailabilityAnalyzerCodeFixProvider)), Shared]
public class ContextAvailabilityAnalyzerCodeFixProvider : CodeFixProvider
{
public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(
AnalyzerDiagnostics.ContextIsAField.Id,
AnalyzerDiagnostics.ContextIsNotStatic.Id,
AnalyzerDiagnostics.ContextAccessIsTooRestrictive.Id,
AnalyzerDiagnostics.ContextIsNotDeclared.Id
);

public sealed override FixAllProvider GetFixAllProvider()
{
return WellKnownFixAllProviders.BatchFixer;
}

public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);

var diagnostic = context.Diagnostics.First();
var diagnosticSpan = diagnostic.Location.SourceSpan;

if (diagnostic.Id == AnalyzerDiagnostics.ContextIsAField.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Replace with static property",
createChangedDocument: _ => Task.FromResult(FixContextIsAFieldError(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextIsAField.Id
),
diagnostic
);
}
else if (diagnostic.Id == AnalyzerDiagnostics.ContextIsNotStatic.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Make static",
createChangedDocument: _ => Task.FromResult(FixContextIsNotStaticError(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextIsNotStatic.Id
),
diagnostic
);
}
else if (diagnostic.Id == AnalyzerDiagnostics.ContextAccessIsTooRestrictive.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Make internal",
createChangedDocument: _ => Task.FromResult(FixContextIsTooRestricted(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextAccessIsTooRestrictive.Id
),
diagnostic
);
}
else if (diagnostic.Id == AnalyzerDiagnostics.ContextIsNotDeclared.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Declare context property",
createChangedDocument: _ => Task.FromResult(FixContextNotDeclared(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextIsNotDeclared.Id
),
diagnostic
);
}
}
Comment on lines +31 to +82
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Process all diagnostics in RegisterCodeFixesAsync

Currently, the method RegisterCodeFixesAsync processes only the first diagnostic from context.Diagnostics. To ensure that all diagnostics are addressed, consider iterating over all diagnostics.

Apply this refactor to process each diagnostic:

 public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 {
     var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
 
-    var diagnostic = context.Diagnostics.First();
-    var diagnosticSpan = diagnostic.Location.SourceSpan;
-
-    if (diagnostic.Id == AnalyzerDiagnostics.ContextIsAField.Id)
-    {
-        // Register code fix for this diagnostic
-    }
-    // Other diagnostic handling...
+    foreach (var diagnostic in context.Diagnostics)
+    {
+        var diagnosticSpan = diagnostic.Location.SourceSpan;
+
+        if (diagnostic.Id == AnalyzerDiagnostics.ContextIsAField.Id)
+        {
+            // Register code fix for this diagnostic
+        }
+        // Other diagnostic handling...
+    }
 }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
var diagnostic = context.Diagnostics.First();
var diagnosticSpan = diagnostic.Location.SourceSpan;
if (diagnostic.Id == AnalyzerDiagnostics.ContextIsAField.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Replace with static property",
createChangedDocument: _ => Task.FromResult(FixContextIsAFieldError(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextIsAField.Id
),
diagnostic
);
}
else if (diagnostic.Id == AnalyzerDiagnostics.ContextIsNotStatic.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Make static",
createChangedDocument: _ => Task.FromResult(FixContextIsNotStaticError(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextIsNotStatic.Id
),
diagnostic
);
}
else if (diagnostic.Id == AnalyzerDiagnostics.ContextAccessIsTooRestrictive.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Make internal",
createChangedDocument: _ => Task.FromResult(FixContextIsTooRestricted(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextAccessIsTooRestrictive.Id
),
diagnostic
);
}
else if (diagnostic.Id == AnalyzerDiagnostics.ContextIsNotDeclared.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Declare context property",
createChangedDocument: _ => Task.FromResult(FixContextNotDeclared(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextIsNotDeclared.Id
),
diagnostic
);
}
}
public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
foreach (var diagnostic in context.Diagnostics)
{
var diagnosticSpan = diagnostic.Location.SourceSpan;
if (diagnostic.Id == AnalyzerDiagnostics.ContextIsAField.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Replace with static property",
createChangedDocument: _ => Task.FromResult(FixContextIsAFieldError(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextIsAField.Id
),
diagnostic
);
}
else if (diagnostic.Id == AnalyzerDiagnostics.ContextIsNotStatic.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Make static",
createChangedDocument: _ => Task.FromResult(FixContextIsNotStaticError(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextIsNotStatic.Id
),
diagnostic
);
}
else if (diagnostic.Id == AnalyzerDiagnostics.ContextAccessIsTooRestrictive.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Make internal",
createChangedDocument: _ => Task.FromResult(FixContextIsTooRestricted(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextAccessIsTooRestrictive.Id
),
diagnostic
);
}
else if (diagnostic.Id == AnalyzerDiagnostics.ContextIsNotDeclared.Id)
{
context.RegisterCodeFix(
CodeAction.Create(
title: "Declare context property",
createChangedDocument: _ => Task.FromResult(FixContextNotDeclared(context, root, diagnosticSpan)),
equivalenceKey: AnalyzerDiagnostics.ContextIsNotDeclared.Id
),
diagnostic
);
}
}
}


private static MemberDeclarationSyntax GetStaticContextPropertyDeclaration(string propertyName = "Context") =>
SyntaxFactory.ParseMemberDeclaration(
$"internal static PluginInitContext {propertyName} {{ get; private set; }} = null!;"
);

private static Document GetFormattedDocument(CodeFixContext context, SyntaxNode root)
{
var formattedRoot = Formatter.Format(
root,
Formatter.Annotation,
context.Document.Project.Solution.Workspace
);

return context.Document.WithSyntaxRoot(formattedRoot);
}

private static Document FixContextNotDeclared(CodeFixContext context, SyntaxNode root, TextSpan diagnosticSpan)
{
var classDeclaration = GetDeclarationSyntax<ClassDeclarationSyntax>(root, diagnosticSpan);
if (classDeclaration?.BaseList is null) return context.Document;

var newPropertyDeclaration = GetStaticContextPropertyDeclaration();
if (newPropertyDeclaration is null) return context.Document;

var annotatedNewPropertyDeclaration = newPropertyDeclaration
.WithLeadingTrivia(SyntaxFactory.ElasticLineFeed)
.WithTrailingTrivia(SyntaxFactory.ElasticLineFeed)
.WithAdditionalAnnotations(Formatter.Annotation, Simplifier.Annotation);

var newMembers = classDeclaration.Members.Insert(0, annotatedNewPropertyDeclaration);
var newClassDeclaration = classDeclaration.WithMembers(newMembers);

var newRoot = root.ReplaceNode(classDeclaration, newClassDeclaration);

return GetFormattedDocument(context, newRoot);
}

private static Document FixContextIsNotStaticError(CodeFixContext context, SyntaxNode root, TextSpan diagnosticSpan)
{
var propertyDeclaration = GetDeclarationSyntax<PropertyDeclarationSyntax>(root, diagnosticSpan);
if (propertyDeclaration is null) return context.Document;

var newPropertyDeclaration = FixRestrictivePropertyModifiers(propertyDeclaration).AddModifiers(SyntaxFactory.Token(SyntaxKind.StaticKeyword));

var newRoot = root.ReplaceNode(propertyDeclaration, newPropertyDeclaration);
return context.Document.WithSyntaxRoot(newRoot);
}

private static Document FixContextIsTooRestricted(CodeFixContext context, SyntaxNode root, TextSpan diagnosticSpan)
{
var propertyDeclaration = GetDeclarationSyntax<PropertyDeclarationSyntax>(root, diagnosticSpan);
if (propertyDeclaration is null) return context.Document;

var newPropertyDeclaration = FixRestrictivePropertyModifiers(propertyDeclaration);

var newRoot = root.ReplaceNode(propertyDeclaration, newPropertyDeclaration);
return context.Document.WithSyntaxRoot(newRoot);
}

private static Document FixContextIsAFieldError(CodeFixContext context, SyntaxNode root, TextSpan diagnosticSpan) {
var fieldDeclaration = GetDeclarationSyntax<FieldDeclarationSyntax>(root, diagnosticSpan);
if (fieldDeclaration is null) return context.Document;

var field = fieldDeclaration.Declaration.Variables.First();
var fieldIdentifier = field.Identifier.ToString();

var propertyDeclaration = GetStaticContextPropertyDeclaration(fieldIdentifier);
if (propertyDeclaration is null) return context.Document;

var annotatedNewPropertyDeclaration = propertyDeclaration
.WithTriviaFrom(fieldDeclaration)
.WithAdditionalAnnotations(Formatter.Annotation, Simplifier.Annotation);

var newRoot = root.ReplaceNode(fieldDeclaration, annotatedNewPropertyDeclaration);

return GetFormattedDocument(context, newRoot);
}

private static PropertyDeclarationSyntax FixRestrictivePropertyModifiers(PropertyDeclarationSyntax propertyDeclaration)
{
var newModifiers = SyntaxFactory.TokenList();
foreach (var modifier in propertyDeclaration.Modifiers)
{
if (modifier.IsKind(SyntaxKind.PrivateKeyword) || modifier.IsKind(SyntaxKind.ProtectedKeyword))
{
newModifiers = newModifiers.Add(SyntaxFactory.Token(SyntaxKind.InternalKeyword));
}
else
{
newModifiers = newModifiers.Add(modifier);
}
}

return propertyDeclaration.WithModifiers(newModifiers);
}
Comment on lines +162 to +178
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor modifier handling in FixRestrictivePropertyModifiers

The current implementation may inadvertently retain or duplicate access modifiers. To ensure modifiers are updated correctly without duplication, consider explicitly setting the desired access modifiers.

Refactor the method to replace existing access modifiers with internal:

 private static PropertyDeclarationSyntax FixRestrictivePropertyModifiers(PropertyDeclarationSyntax propertyDeclaration)
 {
-    var newModifiers = SyntaxFactory.TokenList();
-    foreach (var modifier in propertyDeclaration.Modifiers)
-    {
-        if (modifier.IsKind(SyntaxKind.PrivateKeyword) || modifier.IsKind(SyntaxKind.ProtectedKeyword))
-        {
-            newModifiers = newModifiers.Add(SyntaxFactory.Token(SyntaxKind.InternalKeyword));
-        }
-        else
-        {
-            newModifiers = newModifiers.Add(modifier);
-        }
-    }
+    var nonAccessModifiers = propertyDeclaration.Modifiers.Where(m =>
+        !m.IsKind(SyntaxKind.PrivateKeyword) &&
+        !m.IsKind(SyntaxKind.ProtectedKeyword) &&
+        !m.IsKind(SyntaxKind.PublicKeyword) &&
+        !m.IsKind(SyntaxKind.InternalKeyword)
+    );
+
+    var newModifiers = SyntaxFactory.TokenList()
+        .Add(SyntaxFactory.Token(SyntaxKind.InternalKeyword))
+        .AddRange(nonAccessModifiers);
 
     return propertyDeclaration.WithModifiers(newModifiers);
 }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static PropertyDeclarationSyntax FixRestrictivePropertyModifiers(PropertyDeclarationSyntax propertyDeclaration)
{
var newModifiers = SyntaxFactory.TokenList();
foreach (var modifier in propertyDeclaration.Modifiers)
{
if (modifier.IsKind(SyntaxKind.PrivateKeyword) || modifier.IsKind(SyntaxKind.ProtectedKeyword))
{
newModifiers = newModifiers.Add(SyntaxFactory.Token(SyntaxKind.InternalKeyword));
}
else
{
newModifiers = newModifiers.Add(modifier);
}
}
return propertyDeclaration.WithModifiers(newModifiers);
}
private static PropertyDeclarationSyntax FixRestrictivePropertyModifiers(PropertyDeclarationSyntax propertyDeclaration)
{
var nonAccessModifiers = propertyDeclaration.Modifiers.Where(m =>
!m.IsKind(SyntaxKind.PrivateKeyword) &&
!m.IsKind(SyntaxKind.ProtectedKeyword) &&
!m.IsKind(SyntaxKind.PublicKeyword) &&
!m.IsKind(SyntaxKind.InternalKeyword)
);
var newModifiers = SyntaxFactory.TokenList()
.Add(SyntaxFactory.Token(SyntaxKind.InternalKeyword))
.AddRange(nonAccessModifiers);
return propertyDeclaration.WithModifiers(newModifiers);
}


private static T GetDeclarationSyntax<T>(SyntaxNode root, TextSpan diagnosticSpan) where T : SyntaxNode =>
root
.FindToken(diagnosticSpan.Start)
.Parent
?.AncestorsAndSelf()
.OfType<T>()
.First();
Comment on lines +180 to +186
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle potential exceptions when using First() in GetDeclarationSyntax<T>

The method GetDeclarationSyntax<T> uses First() without checking if any elements exist, which can throw an exception if no matching syntax node is found. Consider using FirstOrDefault() and handling the case when the result is null.

Apply this refactor to prevent possible exceptions:

-private static T GetDeclarationSyntax<T>(SyntaxNode root, TextSpan diagnosticSpan) where T : SyntaxNode =>
+private static T? GetDeclarationSyntax<T>(SyntaxNode root, TextSpan diagnosticSpan) where T : SyntaxNode =>
     root
         .FindToken(diagnosticSpan.Start)
         .Parent
         ?.AncestorsAndSelf()
         .OfType<T>()
-        .First();
+        .FirstOrDefault();

And update the calling methods to handle a possible null value from GetDeclarationSyntax<T>.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static T GetDeclarationSyntax<T>(SyntaxNode root, TextSpan diagnosticSpan) where T : SyntaxNode =>
root
.FindToken(diagnosticSpan.Start)
.Parent
?.AncestorsAndSelf()
.OfType<T>()
.First();
private static T? GetDeclarationSyntax<T>(SyntaxNode root, TextSpan diagnosticSpan) where T : SyntaxNode =>
root
.FindToken(diagnosticSpan.Start)
.Parent
?.AncestorsAndSelf()
.OfType<T>()
.FirstOrDefault();

}
}
Loading
Loading