Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion Documentation/Diagnostics/PH2066.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
| Diagnostic ID | PH2066 |
| Category | [Maintainability](../Maintainability.md) |
| Analyzer | [LockObjectsMustBeReadonlyAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MaintainabilityAnalyzers/Maintainability/LockObjectsMustBeReadonlyAnalyzer.cs)
| CodeFix | No |
| CodeFix | Yes |
| Severity | Error |
| Enabled By Default | Yes |

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// © 2025 Koninklijke Philips N.V. See License.md in the project root for license information.

using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Philips.CodeAnalysis.Common;

namespace Philips.CodeAnalysis.MaintainabilityAnalyzers.Maintainability
{
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(LockObjectsMustBeReadonlyCodeFixProvider)), Shared]
public class LockObjectsMustBeReadonlyCodeFixProvider : SingleDiagnosticCodeFixProvider<IdentifierNameSyntax>
{
protected override string Title => "Add readonly modifier to field";

protected override DiagnosticId DiagnosticId => DiagnosticId.LocksShouldBeReadonly;

protected override async Task<Document> ApplyFix(Document document, IdentifierNameSyntax node, ImmutableDictionary<string, string> properties, CancellationToken cancellationToken)
{
SyntaxNode rootNode = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
SemanticModel semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);

// Get the symbol referenced by the identifier
SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(node, cancellationToken);
if (symbolInfo.Symbol is not IFieldSymbol fieldSymbol)
{
return document;
}

// Use the field symbol's declaration reference to find the actual syntax node
SyntaxReference declaringSyntaxReference = fieldSymbol.DeclaringSyntaxReferences.FirstOrDefault();
if (declaringSyntaxReference == null)
{
return document;
}

SyntaxNode declaringSyntax = await declaringSyntaxReference.GetSyntaxAsync(cancellationToken).ConfigureAwait(false);
FieldDeclarationSyntax fieldDeclaration = declaringSyntax.FirstAncestorOrSelf<FieldDeclarationSyntax>();

if (fieldDeclaration == null)
{
return document;
}

// Add readonly modifier if not already present
SyntaxTokenList modifiers = fieldDeclaration.Modifiers;
if (!modifiers.Any(m => m.IsKind(SyntaxKind.ReadOnlyKeyword)))
{
SyntaxToken readonlyModifier = SyntaxFactory.Token(SyntaxKind.ReadOnlyKeyword).WithTrailingTrivia(SyntaxFactory.Space);

// Insert readonly after access modifiers but before static
SyntaxTokenList newModifiers = modifiers;
var insertIndex = 0;

// Find appropriate position for readonly modifier
for (var i = 0; i < modifiers.Count; i++)
{
if (modifiers[i].IsKind(SyntaxKind.PublicKeyword) ||

Choose a reason for hiding this comment

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

The position of the modifier, while generally standard practice, is governed by settings. Is it straightforward for the analyzer to respect the configured modifier order preference? (e.g., by having roslyn Format the modified line of code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the implementation to use Roslyn's formatting capabilities instead of hardcoded modifier positioning. The CodeFix now adds the readonly modifier and uses Formatter.Annotation to let Roslyn determine the correct position based on the configured csharp_preferred_modifier_order preference in .editorconfig. This ensures the CodeFix respects project-specific formatting rules. Commit: b49488c

modifiers[i].IsKind(SyntaxKind.PrivateKeyword) ||
modifiers[i].IsKind(SyntaxKind.ProtectedKeyword) ||
modifiers[i].IsKind(SyntaxKind.InternalKeyword))
{
insertIndex = i + 1;
}
else if (modifiers[i].IsKind(SyntaxKind.StaticKeyword))
{
insertIndex = i + 1;
break;
}
}

newModifiers = newModifiers.Insert(insertIndex, readonlyModifier);
FieldDeclarationSyntax newFieldDeclaration = fieldDeclaration.WithModifiers(newModifiers);

SyntaxNode newRoot = rootNode.ReplaceNode(fieldDeclaration, newFieldDeclaration);
return document.WithSyntaxRoot(newRoot);
}

return document;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// © 2019 Koninklijke Philips N.V. See License.md in the project root for license information.

using System.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Philips.CodeAnalysis.Common;
Expand All @@ -11,13 +12,18 @@
namespace Philips.CodeAnalysis.Test.Maintainability.Maintainability
{
[TestClass]
public class LockObjectsMustBeReadonlyAnalyzerTest : DiagnosticVerifier
public class LockObjectsMustBeReadonlyAnalyzerTest : CodeFixVerifier
{
protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
{
return new LockObjectsMustBeReadonlyAnalyzer();
}

protected override CodeFixProvider GetCodeFixProvider()
{
return new LockObjectsMustBeReadonlyCodeFixProvider();
}

[DataRow("static object _foo", true)]
[DataRow("object _foo", true)]
[DataRow("static readonly object _foo", false)]
Expand Down Expand Up @@ -155,5 +161,110 @@ public void Test()
";
await VerifyDiagnostic(template, DiagnosticId.LocksShouldBeReadonly, regex: "'_foo'").ConfigureAwait(false);
}

[TestMethod]
[TestCategory(TestDefinitions.UnitTests)]
public async Task LockObjectsMustBeReadonlyCodeFixPrivateFieldAsync()
{
const string original = @"using System;
class Foo
{
private object _foo;

public void Test()
{
lock(_foo) { }
}
}
";
const string expected = @"using System;
class Foo
{
private readonly object _foo;

public void Test()
{
lock(_foo) { }
}
}
";
await VerifyFix(original, expected).ConfigureAwait(false);
}

[TestMethod]
[TestCategory(TestDefinitions.UnitTests)]
public async Task LockObjectsMustBeReadonlyCodeFixStaticFieldAsync()
{
const string original = @"using System;
class Foo
{
static object _foo;

public void Test()
{
lock(_foo) { }
}
}
";
const string expected = @"using System;
class Foo
{
static readonly object _foo;

public void Test()
{
lock(_foo) { }
}
}
";
await VerifyFix(original, expected).ConfigureAwait(false);
}

[TestMethod]
[TestCategory(TestDefinitions.UnitTests)]
public async Task LockObjectsMustBeReadonlyCodeFixPrivateStaticFieldAsync()
{
const string original = @"using System;
class Foo
{
private static object _foo;

public void Test()
{
lock(_foo) { }
}
}
";
const string expected = @"using System;
class Foo
{
private static readonly object _foo;

public void Test()
{
lock(_foo) { }
}
}
";
await VerifyFix(original, expected).ConfigureAwait(false);
}

[TestMethod]
[TestCategory(TestDefinitions.UnitTests)]
public async Task LockObjectsMustBeReadonlyCodeFixAlreadyReadonlyAsync()
{
const string code = @"using System;
class Foo
{
private readonly object _foo;

public void Test()
{
lock(_foo) { }
}
}
";
await VerifySuccessfulCompilation(code).ConfigureAwait(false);
}
}
}
Loading