Skip to content

Commit db91927

Browse files
Copilotbcollamore
andcommitted
Implement CodeFixer for LockObjectsMustBeReadonlyAnalyzer
Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>
1 parent 2405aaf commit db91927

3 files changed

Lines changed: 200 additions & 2 deletions

File tree

Documentation/Diagnostics/PH2066.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
| Diagnostic ID | PH2066 |
77
| Category | [Maintainability](../Maintainability.md) |
88
| Analyzer | [LockObjectsMustBeReadonlyAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MaintainabilityAnalyzers/Maintainability/LockObjectsMustBeReadonlyAnalyzer.cs)
9-
| CodeFix | No |
9+
| CodeFix | Yes |
1010
| Severity | Error |
1111
| Enabled By Default | Yes |
1212

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// © 2025 Koninklijke Philips N.V. See License.md in the project root for license information.
2+
3+
using System.Collections.Immutable;
4+
using System.Composition;
5+
using System.Linq;
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Microsoft.CodeAnalysis;
9+
using Microsoft.CodeAnalysis.CodeFixes;
10+
using Microsoft.CodeAnalysis.CSharp;
11+
using Microsoft.CodeAnalysis.CSharp.Syntax;
12+
using Philips.CodeAnalysis.Common;
13+
14+
namespace Philips.CodeAnalysis.MaintainabilityAnalyzers.Maintainability
15+
{
16+
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(LockObjectsMustBeReadonlyCodeFixProvider)), Shared]
17+
public class LockObjectsMustBeReadonlyCodeFixProvider : SingleDiagnosticCodeFixProvider<IdentifierNameSyntax>
18+
{
19+
protected override string Title => "Add readonly modifier to field";
20+
21+
protected override DiagnosticId DiagnosticId => DiagnosticId.LocksShouldBeReadonly;
22+
23+
protected override async Task<Document> ApplyFix(Document document, IdentifierNameSyntax node, ImmutableDictionary<string, string> properties, CancellationToken cancellationToken)
24+
{
25+
SyntaxNode rootNode = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
26+
SemanticModel semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
27+
28+
// Get the symbol referenced by the identifier
29+
SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(node, cancellationToken);
30+
if (symbolInfo.Symbol is not IFieldSymbol fieldSymbol)
31+
{
32+
return document;
33+
}
34+
35+
// Use the field symbol's declaration reference to find the actual syntax node
36+
SyntaxReference declaringSyntaxReference = fieldSymbol.DeclaringSyntaxReferences.FirstOrDefault();
37+
if (declaringSyntaxReference == null)
38+
{
39+
return document;
40+
}
41+
42+
SyntaxNode declaringSyntax = await declaringSyntaxReference.GetSyntaxAsync(cancellationToken).ConfigureAwait(false);
43+
FieldDeclarationSyntax fieldDeclaration = declaringSyntax.FirstAncestorOrSelf<FieldDeclarationSyntax>();
44+
45+
if (fieldDeclaration == null)
46+
{
47+
return document;
48+
}
49+
50+
// Add readonly modifier if not already present
51+
SyntaxTokenList modifiers = fieldDeclaration.Modifiers;
52+
if (!modifiers.Any(m => m.IsKind(SyntaxKind.ReadOnlyKeyword)))
53+
{
54+
SyntaxToken readonlyModifier = SyntaxFactory.Token(SyntaxKind.ReadOnlyKeyword).WithTrailingTrivia(SyntaxFactory.Space);
55+
56+
// Insert readonly after access modifiers but before static
57+
SyntaxTokenList newModifiers = modifiers;
58+
var insertIndex = 0;
59+
60+
// Find appropriate position for readonly modifier
61+
for (var i = 0; i < modifiers.Count; i++)
62+
{
63+
if (modifiers[i].IsKind(SyntaxKind.PublicKeyword) ||
64+
modifiers[i].IsKind(SyntaxKind.PrivateKeyword) ||
65+
modifiers[i].IsKind(SyntaxKind.ProtectedKeyword) ||
66+
modifiers[i].IsKind(SyntaxKind.InternalKeyword))
67+
{
68+
insertIndex = i + 1;
69+
}
70+
else if (modifiers[i].IsKind(SyntaxKind.StaticKeyword))
71+
{
72+
insertIndex = i + 1;
73+
break;
74+
}
75+
}
76+
77+
newModifiers = newModifiers.Insert(insertIndex, readonlyModifier);
78+
FieldDeclarationSyntax newFieldDeclaration = fieldDeclaration.WithModifiers(newModifiers);
79+
80+
SyntaxNode newRoot = rootNode.ReplaceNode(fieldDeclaration, newFieldDeclaration);
81+
return document.WithSyntaxRoot(newRoot);
82+
}
83+
84+
return document;
85+
}
86+
}
87+
}

Philips.CodeAnalysis.Test/Maintainability/Maintainability/LockObjectsMustBeReadonlyAnalyzerTest.cs

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// © 2019 Koninklijke Philips N.V. See License.md in the project root for license information.
22

33
using System.Threading.Tasks;
4+
using Microsoft.CodeAnalysis.CodeFixes;
45
using Microsoft.CodeAnalysis.Diagnostics;
56
using Microsoft.VisualStudio.TestTools.UnitTesting;
67
using Philips.CodeAnalysis.Common;
@@ -11,13 +12,18 @@
1112
namespace Philips.CodeAnalysis.Test.Maintainability.Maintainability
1213
{
1314
[TestClass]
14-
public class LockObjectsMustBeReadonlyAnalyzerTest : DiagnosticVerifier
15+
public class LockObjectsMustBeReadonlyAnalyzerTest : CodeFixVerifier
1516
{
1617
protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
1718
{
1819
return new LockObjectsMustBeReadonlyAnalyzer();
1920
}
2021

22+
protected override CodeFixProvider GetCodeFixProvider()
23+
{
24+
return new LockObjectsMustBeReadonlyCodeFixProvider();
25+
}
26+
2127
[DataRow("static object _foo", true)]
2228
[DataRow("object _foo", true)]
2329
[DataRow("static readonly object _foo", false)]
@@ -155,5 +161,110 @@ public void Test()
155161
";
156162
await VerifyDiagnostic(template, DiagnosticId.LocksShouldBeReadonly, regex: "'_foo'").ConfigureAwait(false);
157163
}
164+
165+
[TestMethod]
166+
[TestCategory(TestDefinitions.UnitTests)]
167+
public async Task LockObjectsMustBeReadonlyCodeFixPrivateFieldAsync()
168+
{
169+
const string original = @"using System;
170+
class Foo
171+
{
172+
private object _foo;
173+
174+
public void Test()
175+
{
176+
lock(_foo) { }
177+
}
178+
}
179+
";
180+
const string expected = @"using System;
181+
class Foo
182+
{
183+
private readonly object _foo;
184+
185+
public void Test()
186+
{
187+
lock(_foo) { }
188+
}
189+
}
190+
";
191+
await VerifyFix(original, expected).ConfigureAwait(false);
192+
}
193+
194+
[TestMethod]
195+
[TestCategory(TestDefinitions.UnitTests)]
196+
public async Task LockObjectsMustBeReadonlyCodeFixStaticFieldAsync()
197+
{
198+
const string original = @"using System;
199+
class Foo
200+
{
201+
static object _foo;
202+
203+
public void Test()
204+
{
205+
lock(_foo) { }
206+
}
207+
}
208+
";
209+
const string expected = @"using System;
210+
class Foo
211+
{
212+
static readonly object _foo;
213+
214+
public void Test()
215+
{
216+
lock(_foo) { }
217+
}
218+
}
219+
";
220+
await VerifyFix(original, expected).ConfigureAwait(false);
221+
}
222+
223+
[TestMethod]
224+
[TestCategory(TestDefinitions.UnitTests)]
225+
public async Task LockObjectsMustBeReadonlyCodeFixPrivateStaticFieldAsync()
226+
{
227+
const string original = @"using System;
228+
class Foo
229+
{
230+
private static object _foo;
231+
232+
public void Test()
233+
{
234+
lock(_foo) { }
235+
}
236+
}
237+
";
238+
const string expected = @"using System;
239+
class Foo
240+
{
241+
private static readonly object _foo;
242+
243+
public void Test()
244+
{
245+
lock(_foo) { }
246+
}
247+
}
248+
";
249+
await VerifyFix(original, expected).ConfigureAwait(false);
250+
}
251+
252+
[TestMethod]
253+
[TestCategory(TestDefinitions.UnitTests)]
254+
public async Task LockObjectsMustBeReadonlyCodeFixAlreadyReadonlyAsync()
255+
{
256+
const string code = @"using System;
257+
class Foo
258+
{
259+
private readonly object _foo;
260+
261+
public void Test()
262+
{
263+
lock(_foo) { }
264+
}
265+
}
266+
";
267+
await VerifySuccessfulCompilation(code).ConfigureAwait(false);
268+
}
158269
}
159270
}

0 commit comments

Comments
 (0)