From add392791aa7efc75c376f94a4c852d4ddc7c251 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 8 Feb 2023 14:48:51 +0100
Subject: [PATCH 01/24] Scaffolding

---
 analyzers/rspec/cs/S2445_c#.html              | 39 +++++++++++++++++
 analyzers/rspec/cs/S2445_c#.json              | 24 +++++++++++
 analyzers/rspec/cs/Sonar_way_profile.json     |  1 +
 .../Rules/SynchronizedFieldAssignment.cs      | 43 +++++++++++++++++++
 .../PackagingTests/RuleTypeMappingCS.cs       |  2 +-
 .../Rules/SynchronizedFieldAssignmentTest.cs  | 33 ++++++++++++++
 .../TestCases/SynchronizedFieldAssignment.cs  |  5 +++
 7 files changed, 146 insertions(+), 1 deletion(-)
 create mode 100644 analyzers/rspec/cs/S2445_c#.html
 create mode 100644 analyzers/rspec/cs/S2445_c#.json
 create mode 100644 analyzers/src/SonarAnalyzer.CSharp/Rules/SynchronizedFieldAssignment.cs
 create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/Rules/SynchronizedFieldAssignmentTest.cs
 create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SynchronizedFieldAssignment.cs

diff --git a/analyzers/rspec/cs/S2445_c#.html b/analyzers/rspec/cs/S2445_c#.html
new file mode 100644
index 00000000000..a5b4b409e90
--- /dev/null
+++ b/analyzers/rspec/cs/S2445_c#.html
@@ -0,0 +1,39 @@
+<p>Synchronizing on a class field synchronizes not on the field itself, but on the object assigned to it. So synchronizing on a non-<code>final</code>
+field makes it possible for the field’s value to change while a thread is in a block synchronized on the old value. That would allow a second thread,
+synchronized on the new value, to enter the block at the same time.</p>
+<p>The story is very similar for synchronizing on parameters; two different threads running the method in parallel could pass two different object
+instances in to the method as parameters, completely undermining the synchronization.</p>
+<h2>Noncompliant Code Example</h2>
+<pre>
+private String color = "red";
+
+private void doSomething(){
+  synchronized(color) {  // Noncompliant; lock is actually on object instance "red" referred to by the color variable
+    //...
+    color = "green"; // other threads now allowed into this block
+    // ...
+  }
+  synchronized(new Object()) { // Noncompliant this is a no-op.
+     // ...
+  }
+}
+</pre>
+<h2>Compliant Solution</h2>
+<pre>
+private String color = "red";
+private final Object lockObj = new Object();
+
+private void doSomething(){
+  synchronized(lockObj) {
+    //...
+    color = "green";
+    // ...
+  }
+}
+</pre>
+<h2>See</h2>
+<ul>
+  <li> <a href="https://cwe.mitre.org/data/definitions/412">MITRE, CWE-412</a> - Unrestricted Externally Accessible Lock </li>
+  <li> <a href="https://cwe.mitre.org/data/definitions/413">MITRE, CWE-413</a> - Improper Resource Locking </li>
+</ul>
+
diff --git a/analyzers/rspec/cs/S2445_c#.json b/analyzers/rspec/cs/S2445_c#.json
new file mode 100644
index 00000000000..c3e5b80de4c
--- /dev/null
+++ b/analyzers/rspec/cs/S2445_c#.json
@@ -0,0 +1,24 @@
+{
+  "title": "Blocks should be synchronized on \"private final\" fields",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "15min"
+  },
+  "tags": [
+    "cwe",
+    "multi-threading"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-2445",
+  "sqKey": "S2445",
+  "scope": "All",
+  "securityStandards": {
+    "CWE": [
+      412,
+      413
+    ]
+  },
+  "quickfix": "unknown"
+}
diff --git a/analyzers/rspec/cs/Sonar_way_profile.json b/analyzers/rspec/cs/Sonar_way_profile.json
index d84e4e17b49..8bee12a45f8 100644
--- a/analyzers/rspec/cs/Sonar_way_profile.json
+++ b/analyzers/rspec/cs/Sonar_way_profile.json
@@ -99,6 +99,7 @@
     "S2386",
     "S2436",
     "S2437",
+    "S2445",
     "S2479",
     "S2486",
     "S2551",
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SynchronizedFieldAssignment.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SynchronizedFieldAssignment.cs
new file mode 100644
index 00000000000..dd150f9be38
--- /dev/null
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SynchronizedFieldAssignment.cs
@@ -0,0 +1,43 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2023 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+namespace SonarAnalyzer.Rules.CSharp;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class SynchronizedFieldAssignment : SonarDiagnosticAnalyzer
+{
+    private const string DiagnosticId = "S2445";
+    private const string MessageFormat = "FIXME";
+
+    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    protected override void Initialize(SonarAnalysisContext context) =>
+        context.RegisterNodeAction(c =>
+            {
+                var node = c.Node;
+                if (true)
+                {
+                    c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation()));
+                }
+            },
+            SyntaxKind.InvocationExpression);
+}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs
index 052bc77b511..7d53275d567 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs
@@ -2369,7 +2369,7 @@ internal static class RuleTypeMappingCS
         // ["S2442"],
         // ["S2443"],
         // ["S2444"],
-        // ["S2445"],
+        ["S2445"] = "BUG",
         // ["S2446"],
         // ["S2447"],
         // ["S2448"],
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SynchronizedFieldAssignmentTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SynchronizedFieldAssignmentTest.cs
new file mode 100644
index 00000000000..a58f1221e33
--- /dev/null
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SynchronizedFieldAssignmentTest.cs
@@ -0,0 +1,33 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2023 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+using SonarAnalyzer.Rules.CSharp;
+
+namespace SonarAnalyzer.UnitTest.Rules;
+
+[TestClass]
+public class SynchronizedFieldAssignmentTest
+{
+    private readonly VerifierBuilder builder = new VerifierBuilder<SynchronizedFieldAssignment>();
+
+    [TestMethod]
+    public void SynchronizedFieldAssignment_CS() =>
+        builder.AddPaths("SynchronizedFieldAssignment.cs").Verify();
+}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SynchronizedFieldAssignment.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SynchronizedFieldAssignment.cs
new file mode 100644
index 00000000000..a790d6d1f92
--- /dev/null
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SynchronizedFieldAssignment.cs
@@ -0,0 +1,5 @@
+using System;
+
+public class Program
+{
+}

From 645491e387812d0d441e5563f8a72eee32c4e244 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 8 Feb 2023 16:09:39 +0100
Subject: [PATCH 02/24] Implementation

---
 analyzers/rspec/cs/S2445_c#.html              |  31 +--
 analyzers/rspec/cs/S2445_c#.json              |   2 +-
 .../Rules/LockedFieldPrivateReadonly.cs       |  61 ++++++
 .../Rules/SynchronizedFieldAssignment.cs      |  43 -----
 ...t.cs => LockedFieldPrivateReadonlyTest.cs} |  22 ++-
 .../LockedFieldPrivateReadonly.CSharp11.cs    |  21 ++
 .../LockedFieldPrivateReadonly.CSharp8.cs     |  56 ++++++
 .../TestCases/LockedFieldPrivateReadonly.cs   | 180 ++++++++++++++++++
 .../TestCases/SynchronizedFieldAssignment.cs  |   5 -
 9 files changed, 355 insertions(+), 66 deletions(-)
 create mode 100644 analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldPrivateReadonly.cs
 delete mode 100644 analyzers/src/SonarAnalyzer.CSharp/Rules/SynchronizedFieldAssignment.cs
 rename analyzers/tests/SonarAnalyzer.UnitTest/Rules/{SynchronizedFieldAssignmentTest.cs => LockedFieldPrivateReadonlyTest.cs} (60%)
 create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp11.cs
 create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp8.cs
 create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.cs
 delete mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SynchronizedFieldAssignment.cs

diff --git a/analyzers/rspec/cs/S2445_c#.html b/analyzers/rspec/cs/S2445_c#.html
index a5b4b409e90..a4cc24ea031 100644
--- a/analyzers/rspec/cs/S2445_c#.html
+++ b/analyzers/rspec/cs/S2445_c#.html
@@ -1,30 +1,35 @@
-<p>Synchronizing on a class field synchronizes not on the field itself, but on the object assigned to it. So synchronizing on a non-<code>final</code>
-field makes it possible for the field’s value to change while a thread is in a block synchronized on the old value. That would allow a second thread,
-synchronized on the new value, to enter the block at the same time.</p>
-<p>The story is very similar for synchronizing on parameters; two different threads running the method in parallel could pass two different object
-instances in to the method as parameters, completely undermining the synchronization.</p>
+<p>Locking on a class field synchronizes not on the field itself, but on the object assigned to it. So locking on a non-<code>readonly</code> field
+makes it possible for the field’s value to change while a thread is in a block locked on the old value. That would allow a second thread, locked on
+the new value, to enter the block at the same time.</p>
+<p>The story is very similar for locking on a local variable or on a new instance; two different threads running the method in parallel would lock on
+two different object instances, completely undermining the synchronization.</p>
 <h2>Noncompliant Code Example</h2>
 <pre>
-private String color = "red";
+private string color = "red";
 
-private void doSomething(){
-  synchronized(color) {  // Noncompliant; lock is actually on object instance "red" referred to by the color variable
+private void DoSomething()
+{
+  lock (color) // Noncompliant; lock is actually on object instance "red" referred to by the color variable
+  {
     //...
     color = "green"; // other threads now allowed into this block
     // ...
   }
-  synchronized(new Object()) { // Noncompliant this is a no-op.
+  lock (new object()) // Noncompliant this is a no-op.
+  {
      // ...
   }
 }
 </pre>
 <h2>Compliant Solution</h2>
 <pre>
-private String color = "red";
-private final Object lockObj = new Object();
+private string color = "red";
+private readonly object lockObj = new object();
 
-private void doSomething(){
-  synchronized(lockObj) {
+private void DoSomething()
+{
+  lock (lockObj)
+  {
     //...
     color = "green";
     // ...
diff --git a/analyzers/rspec/cs/S2445_c#.json b/analyzers/rspec/cs/S2445_c#.json
index c3e5b80de4c..3bad3ab9135 100644
--- a/analyzers/rspec/cs/S2445_c#.json
+++ b/analyzers/rspec/cs/S2445_c#.json
@@ -1,5 +1,5 @@
 {
-  "title": "Blocks should be synchronized on \"private final\" fields",
+  "title": "Blocks should be synchronized on \"private readonly\" fields",
   "type": "BUG",
   "status": "ready",
   "remediation": {
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldPrivateReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldPrivateReadonly.cs
new file mode 100644
index 00000000000..5929b808723
--- /dev/null
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldPrivateReadonly.cs
@@ -0,0 +1,61 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2023 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+namespace SonarAnalyzer.Rules.CSharp;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class LockedFieldPrivateReadonly : SonarDiagnosticAnalyzer
+{
+    private const string DiagnosticId = "S2445";
+
+    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, "{0}");
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    protected override void Initialize(SonarAnalysisContext context) =>
+        context.RegisterNodeAction(c =>
+            {
+                var expression = ((LockStatementSyntax)c.Node).Expression;
+                if (expression is ObjectCreationExpressionSyntax
+                    or AnonymousObjectCreationExpressionSyntax
+                    or ArrayCreationExpressionSyntax
+                    or ImplicitArrayCreationExpressionSyntax
+                    or QueryExpressionSyntax)
+                {
+                    c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), "Locking on a new instance is a no-op."));
+                }
+                else if (expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression))
+                {
+                    c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), "Strings can be interned, and should not be used for locking."));
+                }
+                else if (expression is IdentifierNameSyntax
+                    && c.SemanticModel.GetSymbolInfo(expression).Symbol is ILocalSymbol lockedSymbol)
+                {
+                    c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), $"'{lockedSymbol.Name}' is a local variable, and should not be used for locking."));
+                }
+                else if (expression is (IdentifierNameSyntax or MemberAccessExpressionSyntax)
+                    && c.SemanticModel.GetSymbolInfo(expression).Symbol is IFieldSymbol lockedField
+                    && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private))
+                {
+                    c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), $"'{lockedField.Name}' is not 'private readonly', and should not be used for locking."));
+                }
+            },
+            SyntaxKind.LockStatement);
+}
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SynchronizedFieldAssignment.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SynchronizedFieldAssignment.cs
deleted file mode 100644
index dd150f9be38..00000000000
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SynchronizedFieldAssignment.cs
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * SonarAnalyzer for .NET
- * Copyright (C) 2015-2023 SonarSource SA
- * mailto: contact AT sonarsource DOT com
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 3 of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
- */
-
-namespace SonarAnalyzer.Rules.CSharp;
-
-[DiagnosticAnalyzer(LanguageNames.CSharp)]
-public sealed class SynchronizedFieldAssignment : SonarDiagnosticAnalyzer
-{
-    private const string DiagnosticId = "S2445";
-    private const string MessageFormat = "FIXME";
-
-    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
-
-    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
-
-    protected override void Initialize(SonarAnalysisContext context) =>
-        context.RegisterNodeAction(c =>
-            {
-                var node = c.Node;
-                if (true)
-                {
-                    c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation()));
-                }
-            },
-            SyntaxKind.InvocationExpression);
-}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SynchronizedFieldAssignmentTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldPrivateReadonlyTest.cs
similarity index 60%
rename from analyzers/tests/SonarAnalyzer.UnitTest/Rules/SynchronizedFieldAssignmentTest.cs
rename to analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldPrivateReadonlyTest.cs
index a58f1221e33..61cf3ea09f0 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SynchronizedFieldAssignmentTest.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldPrivateReadonlyTest.cs
@@ -23,11 +23,25 @@
 namespace SonarAnalyzer.UnitTest.Rules;
 
 [TestClass]
-public class SynchronizedFieldAssignmentTest
+public class LockedFieldPrivateReadonlyTest
 {
-    private readonly VerifierBuilder builder = new VerifierBuilder<SynchronizedFieldAssignment>();
+    private readonly VerifierBuilder builder = new VerifierBuilder<LockedFieldPrivateReadonly>();
 
     [TestMethod]
-    public void SynchronizedFieldAssignment_CS() =>
-        builder.AddPaths("SynchronizedFieldAssignment.cs").Verify();
+    public void LockedFieldPrivateReadonly_CS() =>
+        builder.AddPaths("LockedFieldPrivateReadonly.cs").Verify();
+
+    [TestMethod]
+    public void LockedFieldPrivateReadonly_CSharp8() =>
+        builder
+            .AddPaths("LockedFieldPrivateReadonly.CSharp8.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp8)
+            .Verify();
+
+    [TestMethod]
+    public void LockedFieldPrivateReadonly_CSharp11() =>
+        builder
+            .AddPaths("LockedFieldPrivateReadonly.CSharp11.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp11)
+            .Verify();
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp11.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp11.cs
new file mode 100644
index 00000000000..395faf70089
--- /dev/null
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp11.cs
@@ -0,0 +1,21 @@
+using System;
+
+class Test
+{
+    static readonly object staticReadonlyField = null;
+    static object staticReadWriteField = null;
+
+    readonly object readonlyField = null;
+    object readWriteField = null;
+
+    void OnANewInstance()
+    {
+        lock ("""a raw string literal""") { }                    // Noncompliant
+        lock ($"""an interpolated {"raw string literal"}""") { } // Noncompliant
+    }
+
+    void TargetTypedObjectCreation()
+    {
+        lock (new() as Tuple<int>) { }                           // Error [CS8754]
+    }
+}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp8.cs
new file mode 100644
index 00000000000..2bad6133323
--- /dev/null
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp8.cs
@@ -0,0 +1,56 @@
+using System;
+
+class Test
+{
+    static readonly object staticReadonlyField = null;
+    static object staticReadWriteField = null;
+
+    readonly object readonlyField = null;
+    object readWriteField = null;
+
+    Test()
+    {
+        ref object refToReadonlyField = ref readonlyField;
+        lock (refToReadonlyField) { }  // Noncompliant, while the reference is to a readonly field, the reference itself is a local variable and as of C# 7.3 can be ref reassigned
+
+        ref object refToReadWriteField = ref readWriteField;
+        lock (refToReadWriteField) { } // Noncompliant
+    }
+
+    void ReadonlyReferences()
+    {
+        lock (RefReturnReadonlyField(this)) { }
+        lock (RefReturnStaticReadonlyField()) { }
+        lock (StaticRefReturnReadonlyField(this)) { }
+        lock (StaticRefReturnStaticReadonlyField()) { }
+
+        ref readonly object RefReturnReadonlyField(Test instance) => ref instance.readonlyField;
+        ref readonly object RefReturnStaticReadonlyField() => ref Test.staticReadonlyField;
+        static ref readonly object StaticRefReturnReadonlyField(Test instance) => ref instance.readonlyField;
+        static ref readonly object StaticRefReturnStaticReadonlyField() => ref Test.staticReadonlyField;
+    }
+
+    void OnANewInstanceOnStack()
+    {
+        lock (stackalloc int[] { }) { }   // Error [CS0185]
+        lock (stackalloc [] { 1 }) { }    // Error [CS0185]
+    }
+
+    void CoalescingAssignment(object oPar)
+    {
+        lock (oPar ??= readonlyField) { } // FN, null conditional assignment not supported
+    }
+}
+
+class NonPrivateAccessibily
+{
+    private protected readonly object privateProtectedField = null;
+
+    private object PrivateProtectedProperty => null;
+
+    void Test()
+    {
+        lock (privateProtectedField) { }    // Noncompliant
+        lock (PrivateProtectedProperty) { } // Compliant, not a field
+    }
+}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.cs
new file mode 100644
index 00000000000..4fb8a46f7d4
--- /dev/null
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.cs
@@ -0,0 +1,180 @@
+using System;
+using System.Linq;
+
+class Test
+{
+    private static readonly object staticReadonlyField = null;
+    private static object staticReadWriteField = null;
+
+    private readonly object readonlyField = null;
+    private readonly string readonlyStringField = "a string";
+    private object readWriteField = null;
+
+    private static object StaticReadonlyProperty => null;
+    private object ReadonlyProperty => null;
+
+    private static object StaticReadWriteProperty { get; set; }
+    private object ReadWriteProperty { get; set; }
+
+    void OnAStaticField()
+    {
+        lock (staticReadonlyField) { }
+        lock (staticReadWriteField) { }                  // Noncompliant {{'staticReadWriteField' is not 'private readonly', and should not be used for locking.}}
+        //    ^^^^^^^^^^^^^^^^^^^^
+        lock (Test.staticReadonlyField) { }
+        lock (Test.staticReadWriteField) { }             // Noncompliant {{'staticReadWriteField' is not 'private readonly', and should not be used for locking.}}
+        //    ^^^^^^^^^^^^^^^^^^^^^^^^^
+        lock (AnotherClass.staticReadonlyField) { }      // Noncompliant {{'staticReadonlyField' is not 'private readonly', and should not be used for locking.}}
+        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{'staticReadWriteField' is not 'private readonly', and should not be used for locking.}}
+        //    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    }
+
+    void OnAFieldOfSameInstance()
+    {
+        lock (readonlyField) { }
+        lock (readonlyStringField) { }
+        lock (readWriteField) { }                        // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
+        lock (this.readonlyField) { }
+        lock (this.readWriteField) { }                   // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
+    }
+
+    void OnAFieldOfDifferentInstance()
+    {
+        var anotherInstance = new Test();
+        lock (anotherInstance.readonlyField) { }
+        lock (anotherInstance.readWriteField) { }        // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
+        lock (anotherInstance.readonlyField) { }
+    }
+
+    void OnALocalVariable()
+    {
+        object localVarNull = null;
+        lock (localVarNull) { }                          // Noncompliant {{'localVarNull' is a local variable, and should not be used for locking.}}
+        object localVarReadonlyField = readonlyField;
+        lock (localVarReadonlyField) { }                 // Noncompliant, while the local variable references a readonly field, the local variable itself can mutate
+        object localVarReadWriteField = readWriteField;
+        lock (localVarReadWriteField) { }                // Noncompliant
+    }
+
+    void OnANewInstance()
+    {
+        lock (new object()) { }                          // Noncompliant {{Locking on a new instance is a no-op.}}
+        lock (new ANamespace.AClass()) { }               // Noncompliant
+        lock (new Test[] { })                            // Noncompliant
+        lock (new[] { readonlyField}) { }                // Noncompliant
+        lock (new Tuple<object>(readonlyField)) { }      // Noncompliant
+        lock (new { })                                   // Noncompliant
+
+        lock ("a string") { }                            // Noncompliant
+        lock ($"an interpolated {"string"}") { }         // Noncompliant
+        lock (1) { }                                     // Error [CS0185]
+        lock ((a: readonlyField, b: readonlyField)) { }  // Error [CS0185]
+
+        lock (new ADelegate(x => x)) { }                 // Noncompliant
+        lock (new Func<int, int>(x => x)) { }            // Noncompliant
+        lock (x => x) { }                                // Error [CS0185]
+        lock ((int?)1) { }                               // Error [CS0185]
+
+        lock (from x in new object[2] select x) { }      // Noncompliant
+    }
+
+    void OnAssignment()
+    {
+        object x;
+        lock (x = readonlyField) { }
+        lock (x = readWriteField) { }                    // FN, assignment not supported
+    }
+
+    void OtherCases(object oPar, bool bPar)
+    {
+        lock (null) { }
+
+        lock (oPar) { }
+
+        lock (this) { }
+
+        lock (SomeMethod()) { }
+        lock (oPar.GetType()) { }
+        lock (typeof(Test)) { }
+        lock (default(Test)) { }
+
+        object SomeMethod() => null;
+
+        lock (StaticReadonlyProperty) { }
+        lock (ReadonlyProperty) { }
+        lock (StaticReadWriteProperty) { }
+        lock (ReadWriteProperty) { }
+
+        lock (bPar ? readWriteField : readonlyField) { }
+
+        lock (oPar ?? readonlyField) { }
+        lock (oPar = readonlyField) { }
+    }
+
+    void ReadWriteReferences()
+    {
+        lock (RefReturnReadWriteField(this)) { }         // FN, the method returns a readwrite ref to a member
+        lock (RefReturnStaticReadonlyField(this)) { }    // FN, the method returns a readwrite ref to a member
+
+        ref object RefReturnReadWriteField(Test instance) => ref instance.readWriteField;
+        ref object RefReturnStaticReadonlyField(Test instance) => ref Test.staticReadWriteField;
+    }
+
+    delegate object ADelegate(object oPar);
+}
+
+class TestExplicitCast
+{
+    private readonly object readonlyField = null;
+
+    void Test()
+    {
+        lock ((AnotherClass)readonlyField) { } // Compliant, the cast operator can build
+    }
+}
+
+class AnotherClass
+{
+    public static readonly object staticReadonlyField = null;
+    public static object staticReadWriteField = null;
+
+    public readonly object readonlyField = null;
+    public object readWriteField = null;
+
+    public static explicit operator AnotherClass(Test o) => new AnotherClass();
+}
+
+class NonPrivateAccessibily
+{
+    private readonly object privateField = null;
+    protected readonly object protectedField = null;
+    protected internal readonly object protectedInternalField = null;
+    internal readonly object internalField = null;
+    public readonly object publicField = null;
+
+    private object PrivateProperty => null;
+    protected object ProtectedProperty => null;
+    protected internal object ProtectedInternalProperty => null;
+    internal object InternalProperty => null;
+    public object PublicProperty => null;
+
+    void Test()
+    {
+        lock (privateField) { }              // Compliant
+        lock (protectedField) { }            // Noncompliant
+        lock (protectedInternalField) { }    // Noncompliant
+        lock (internalField) { }             // Noncompliant
+        lock (publicField) { }               // Noncompliant
+
+        lock (PrivateProperty) { }           // Compliant, not a field
+        lock (ProtectedProperty) { }         // Compliant, not a field
+        lock (ProtectedInternalProperty) { } // Compliant, not a field
+        lock (InternalProperty) { }          // Compliant, not a field
+        lock (PublicProperty) { }            // Compliant, not a field
+    }
+}
+
+namespace ANamespace
+{
+    class AClass { }
+}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SynchronizedFieldAssignment.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SynchronizedFieldAssignment.cs
deleted file mode 100644
index a790d6d1f92..00000000000
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SynchronizedFieldAssignment.cs
+++ /dev/null
@@ -1,5 +0,0 @@
-using System;
-
-public class Program
-{
-}

From 2344ae363eee761a114e1ea2bf191970b73e05b7 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 15 Feb 2023 11:07:25 +0100
Subject: [PATCH 03/24] IT

---
 .../Automapper/AutoMapper--net461-S2445.json    | 17 +++++++++++++++++
 .../AutoMapper--netstandard2.0-S2445.json       | 17 +++++++++++++++++
 .../its/expected/Nancy/Nancy--net452-S2445.json | 17 +++++++++++++++++
 .../Nancy/Nancy--netstandard2.0-S2445.json      | 17 +++++++++++++++++
 4 files changed, 68 insertions(+)
 create mode 100644 analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json
 create mode 100644 analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json
 create mode 100644 analyzers/its/expected/Nancy/Nancy--net452-S2445.json
 create mode 100644 analyzers/its/expected/Nancy/Nancy--netstandard2.0-S2445.json

diff --git a/analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json b/analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json
new file mode 100644
index 00000000000..1dea7b12535
--- /dev/null
+++ b/analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json
@@ -0,0 +1,17 @@
+{
+"issues":  [
+{
+"id":  "S2445",
+"message":  "'typeMap' is a local variable, and should not be used for locking.",
+"location":  {
+"uri":  "sources\Automapper\src\AutoMapper\Configuration\MapperConfiguration.cs",
+"region":  {
+"startLine":  256,
+"startColumn":  27,
+"endLine":  256,
+"endColumn":  34
+}
+}
+}
+]
+}
diff --git a/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json b/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json
new file mode 100644
index 00000000000..1dea7b12535
--- /dev/null
+++ b/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json
@@ -0,0 +1,17 @@
+{
+"issues":  [
+{
+"id":  "S2445",
+"message":  "'typeMap' is a local variable, and should not be used for locking.",
+"location":  {
+"uri":  "sources\Automapper\src\AutoMapper\Configuration\MapperConfiguration.cs",
+"region":  {
+"startLine":  256,
+"startColumn":  27,
+"endLine":  256,
+"endColumn":  34
+}
+}
+}
+]
+}
diff --git a/analyzers/its/expected/Nancy/Nancy--net452-S2445.json b/analyzers/its/expected/Nancy/Nancy--net452-S2445.json
new file mode 100644
index 00000000000..5f3cf072756
--- /dev/null
+++ b/analyzers/its/expected/Nancy/Nancy--net452-S2445.json
@@ -0,0 +1,17 @@
+{
+"issues":  [
+{
+"id":  "S2445",
+"message":  "'entitiesLock' is not 'private readonly', and should not be used for locking.",
+"location":  {
+"uri":  "sources\Nancy\src\Nancy\Helpers\HttpEncoder.cs",
+"region":  {
+"startLine":  62,
+"startColumn":  23,
+"endLine":  62,
+"endColumn":  35
+}
+}
+}
+]
+}
diff --git a/analyzers/its/expected/Nancy/Nancy--netstandard2.0-S2445.json b/analyzers/its/expected/Nancy/Nancy--netstandard2.0-S2445.json
new file mode 100644
index 00000000000..5f3cf072756
--- /dev/null
+++ b/analyzers/its/expected/Nancy/Nancy--netstandard2.0-S2445.json
@@ -0,0 +1,17 @@
+{
+"issues":  [
+{
+"id":  "S2445",
+"message":  "'entitiesLock' is not 'private readonly', and should not be used for locking.",
+"location":  {
+"uri":  "sources\Nancy\src\Nancy\Helpers\HttpEncoder.cs",
+"region":  {
+"startLine":  62,
+"startColumn":  23,
+"endLine":  62,
+"endColumn":  35
+}
+}
+}
+]
+}

From 9b8482420db341443ce5f08b21f7ef62ba087695 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 22 Feb 2023 10:58:23 +0100
Subject: [PATCH 04/24] Review

---
 analyzers/rspec/cs/S2445_c#.html              |  8 +-
 .../Rules/LockedFieldPrivateReadonly.cs       | 61 ----------------
 .../LockedFieldShouldBePrivateAndReadonly.cs  | 73 +++++++++++++++++++
 ...kedFieldShouldBePrivateAndReadonlyTest.cs} | 23 ++++--
 ...eldShouldBePrivateAndReadonly.CSharp11.cs} |  0
 ...ieldShouldBePrivateAndReadonly.CSharp8.cs} | 15 ++--
 ...FieldShouldBePrivateAndReadonly.CSharp9.cs | 23 ++++++
 ... LockedFieldShouldBePrivateAndReadonly.cs} | 38 +++++++---
 8 files changed, 155 insertions(+), 86 deletions(-)
 delete mode 100644 analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldPrivateReadonly.cs
 create mode 100644 analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
 rename analyzers/tests/SonarAnalyzer.UnitTest/Rules/{LockedFieldPrivateReadonlyTest.cs => LockedFieldShouldBePrivateAndReadonlyTest.cs} (61%)
 rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/{LockedFieldPrivateReadonly.CSharp11.cs => LockedFieldShouldBePrivateAndReadonly.CSharp11.cs} (100%)
 rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/{LockedFieldPrivateReadonly.CSharp8.cs => LockedFieldShouldBePrivateAndReadonly.CSharp8.cs} (80%)
 create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.cs
 rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/{LockedFieldPrivateReadonly.cs => LockedFieldShouldBePrivateAndReadonly.cs} (80%)

diff --git a/analyzers/rspec/cs/S2445_c#.html b/analyzers/rspec/cs/S2445_c#.html
index a4cc24ea031..0119f6dfac7 100644
--- a/analyzers/rspec/cs/S2445_c#.html
+++ b/analyzers/rspec/cs/S2445_c#.html
@@ -1,8 +1,12 @@
 <p>Locking on a class field synchronizes not on the field itself, but on the object assigned to it. So locking on a non-<code>readonly</code> field
 makes it possible for the field’s value to change while a thread is in a block locked on the old value. That would allow a second thread, locked on
 the new value, to enter the block at the same time.</p>
-<p>The story is very similar for locking on a local variable or on a new instance; two different threads running the method in parallel would lock on
-two different object instances, completely undermining the synchronization.</p>
+<p>Locking on a <code>readonly</code> field of a class which is not <code>private</code> allows external code to lock the field, potentially
+interfering with synchronization by methods of that class.</p>
+<p>Locking on a local variable or on a new instance undermines the synchronization: two different threads running the method in parallel would lock on
+two different object instances.</p>
+<p>Locking on a string literal is even more dangerous: depending on whether the string is interned or not, different threads may or may not
+synchronize on the same object instance.</p>
 <h2>Noncompliant Code Example</h2>
 <pre>
 private string color = "red";
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldPrivateReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldPrivateReadonly.cs
deleted file mode 100644
index 5929b808723..00000000000
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldPrivateReadonly.cs
+++ /dev/null
@@ -1,61 +0,0 @@
-/*
- * SonarAnalyzer for .NET
- * Copyright (C) 2015-2023 SonarSource SA
- * mailto: contact AT sonarsource DOT com
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 3 of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
- */
-
-namespace SonarAnalyzer.Rules.CSharp;
-
-[DiagnosticAnalyzer(LanguageNames.CSharp)]
-public sealed class LockedFieldPrivateReadonly : SonarDiagnosticAnalyzer
-{
-    private const string DiagnosticId = "S2445";
-
-    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, "{0}");
-
-    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
-
-    protected override void Initialize(SonarAnalysisContext context) =>
-        context.RegisterNodeAction(c =>
-            {
-                var expression = ((LockStatementSyntax)c.Node).Expression;
-                if (expression is ObjectCreationExpressionSyntax
-                    or AnonymousObjectCreationExpressionSyntax
-                    or ArrayCreationExpressionSyntax
-                    or ImplicitArrayCreationExpressionSyntax
-                    or QueryExpressionSyntax)
-                {
-                    c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), "Locking on a new instance is a no-op."));
-                }
-                else if (expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression))
-                {
-                    c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), "Strings can be interned, and should not be used for locking."));
-                }
-                else if (expression is IdentifierNameSyntax
-                    && c.SemanticModel.GetSymbolInfo(expression).Symbol is ILocalSymbol lockedSymbol)
-                {
-                    c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), $"'{lockedSymbol.Name}' is a local variable, and should not be used for locking."));
-                }
-                else if (expression is (IdentifierNameSyntax or MemberAccessExpressionSyntax)
-                    && c.SemanticModel.GetSymbolInfo(expression).Symbol is IFieldSymbol lockedField
-                    && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private))
-                {
-                    c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), $"'{lockedField.Name}' is not 'private readonly', and should not be used for locking."));
-                }
-            },
-            SyntaxKind.LockStatement);
-}
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
new file mode 100644
index 00000000000..324e02c2a2c
--- /dev/null
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
@@ -0,0 +1,73 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2023 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+namespace SonarAnalyzer.Rules.CSharp;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class LockedFieldShouldBePrivateAndReadonly : SonarDiagnosticAnalyzer
+{
+    private const string DiagnosticId = "S2445";
+
+    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, "{0}");
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    protected override void Initialize(SonarAnalysisContext context) =>
+        context.RegisterNodeAction(c =>
+            {
+                var expression = ((LockStatementSyntax)c.Node).Expression?.RemoveParentheses();
+                if (expression is ObjectCreationExpressionSyntax
+                    or AnonymousObjectCreationExpressionSyntax
+                    or ArrayCreationExpressionSyntax
+                    or ImplicitArrayCreationExpressionSyntax
+                    or QueryExpressionSyntax)
+                {
+                    ReportIssue("Locking on a new instance is a no-op.");
+                }
+                else
+                {
+                    var type = c.SemanticModel.GetTypeInfo(expression).Type;
+
+                    if (expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression)
+                        || (type is not null && type.Is(KnownType.System_String)))
+                    {
+                        ReportIssue("Strings can be interned, and should not be used for locking.");
+                    }
+                    else if (expression is IdentifierNameSyntax
+                        && c.SemanticModel.GetSymbolInfo(expression).Symbol is ILocalSymbol lockedSymbol)
+                    {
+                        ReportIssue($"'{lockedSymbol.Name}' is a local variable, and should not be used for locking.");
+                    }
+                    else if (expression is (IdentifierNameSyntax or MemberAccessExpressionSyntax)
+                        && c.SemanticModel.GetSymbolInfo(expression).Symbol is IFieldSymbol lockedField
+                        && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private))
+                    {
+                        ReportIssue(lockedField.ContainingType is { } lockedFieldType
+                            && c.ContainingSymbol?.ContainingType is { } containingType
+                            && !lockedFieldType.Equals(containingType)
+                                ? $"Use field from '{containingType.ToMinimalDisplayString(c.SemanticModel, expression.SpanStart)}' for locking."
+                                : $"'{lockedField.Name}' is not 'private readonly', and should not be used for locking.");
+                    }
+                }
+
+                void ReportIssue(string message) => c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), message));
+            },
+            SyntaxKind.LockStatement);
+}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldPrivateReadonlyTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBePrivateAndReadonlyTest.cs
similarity index 61%
rename from analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldPrivateReadonlyTest.cs
rename to analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBePrivateAndReadonlyTest.cs
index 61cf3ea09f0..ed6a414f847 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldPrivateReadonlyTest.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBePrivateAndReadonlyTest.cs
@@ -23,25 +23,32 @@
 namespace SonarAnalyzer.UnitTest.Rules;
 
 [TestClass]
-public class LockedFieldPrivateReadonlyTest
+public class LockedFieldShouldBePrivateAndReadonlyTest
 {
-    private readonly VerifierBuilder builder = new VerifierBuilder<LockedFieldPrivateReadonly>();
+    private readonly VerifierBuilder builder = new VerifierBuilder<LockedFieldShouldBePrivateAndReadonly>();
 
     [TestMethod]
-    public void LockedFieldPrivateReadonly_CS() =>
-        builder.AddPaths("LockedFieldPrivateReadonly.cs").Verify();
+    public void LockedFieldShouldBePrivateAndReadonly_CS() =>
+        builder.AddPaths("LockedFieldShouldBePrivateAndReadonly.cs").Verify();
 
     [TestMethod]
-    public void LockedFieldPrivateReadonly_CSharp8() =>
+    public void LockedFieldShouldBePrivateAndReadonly_CSharp8() =>
         builder
-            .AddPaths("LockedFieldPrivateReadonly.CSharp8.cs")
+            .AddPaths("LockedFieldShouldBePrivateAndReadonly.CSharp8.cs")
             .WithOptions(ParseOptionsHelper.FromCSharp8)
             .Verify();
 
     [TestMethod]
-    public void LockedFieldPrivateReadonly_CSharp11() =>
+    public void LockedFieldShouldBePrivateAndReadonly_CSharp9() =>
         builder
-            .AddPaths("LockedFieldPrivateReadonly.CSharp11.cs")
+            .AddPaths("LockedFieldShouldBePrivateAndReadonly.CSharp9.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp9)
+            .Verify();
+
+    [TestMethod]
+    public void LockedFieldShouldBePrivateAndReadonly_CSharp11() =>
+        builder
+            .AddPaths("LockedFieldShouldBePrivateAndReadonly.CSharp11.cs")
             .WithOptions(ParseOptionsHelper.FromCSharp11)
             .Verify();
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp11.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp11.cs
similarity index 100%
rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp11.cs
rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp11.cs
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp8.cs
similarity index 80%
rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp8.cs
rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp8.cs
index 2bad6133323..ffb6bbd01fe 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.CSharp8.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp8.cs
@@ -1,6 +1,4 @@
-using System;
-
-class Test
+class Test
 {
     static readonly object staticReadonlyField = null;
     static object staticReadWriteField = null;
@@ -32,13 +30,18 @@ void ReadonlyReferences()
 
     void OnANewInstanceOnStack()
     {
-        lock (stackalloc int[] { }) { }   // Error [CS0185]
-        lock (stackalloc [] { 1 }) { }    // Error [CS0185]
+        lock (stackalloc int[] { }) { }              // Error [CS0185]
+        lock (stackalloc [] { 1 }) { }               // Error [CS0185]
     }
 
     void CoalescingAssignment(object oPar)
     {
-        lock (oPar ??= readonlyField) { } // FN, null conditional assignment not supported
+        lock (oPar ??= readonlyField) { }            // FN, null conditional assignment not supported
+    }
+
+    void SwitchExpression(object oPar)
+    {
+        lock (oPar switch { _ => new object() }) { } // FN, switch expression not supported
     }
 }
 
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.cs
new file mode 100644
index 00000000000..f90b56d9fb0
--- /dev/null
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.cs
@@ -0,0 +1,23 @@
+class Test
+{
+    readonly ARecord readonlyField = new();
+    ARecord readWriteField = new();
+
+    static readonly ARecord staticReadonlyField = new();
+    static ARecord staticReadWriteField = new();
+
+    void OnAFieldOfTypeRecord()
+    {
+        lock (readonlyField) { }
+        lock (readWriteField) { }       // Noncompliant
+        lock (staticReadonlyField) { }
+        lock (staticReadWriteField) { } // Noncompliant
+    }
+
+    void OnANewRecordInstance()
+    {
+        lock (new ARecord()) { }  // Noncompliant
+    }
+
+    record ARecord();
+}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
similarity index 80%
rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.cs
rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
index 4fb8a46f7d4..8675c562d26 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldPrivateReadonly.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
@@ -24,18 +24,21 @@ void OnAStaticField()
         lock (Test.staticReadonlyField) { }
         lock (Test.staticReadWriteField) { }             // Noncompliant {{'staticReadWriteField' is not 'private readonly', and should not be used for locking.}}
         //    ^^^^^^^^^^^^^^^^^^^^^^^^^
-        lock (AnotherClass.staticReadonlyField) { }      // Noncompliant {{'staticReadonlyField' is not 'private readonly', and should not be used for locking.}}
-        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{'staticReadWriteField' is not 'private readonly', and should not be used for locking.}}
+        lock (AnotherClass.staticReadonlyField) { }      // Noncompliant {{Use field from 'Test' for locking.}}
+        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{Use field from 'Test' for locking.}}
         //    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     }
 
     void OnAFieldOfSameInstance()
     {
         lock (readonlyField) { }
-        lock (readonlyStringField) { }
+        lock ((readonlyField)) { }
+        lock (readonlyStringField) { }                   // Noncompliant {{Strings can be interned, and should not be used for locking.}}
         lock (readWriteField) { }                        // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
+        lock ((readWriteField)) { }                      // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
         lock (this.readonlyField) { }
         lock (this.readWriteField) { }                   // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
+        lock ((this.readWriteField)) { }                 // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
     }
 
     void OnAFieldOfDifferentInstance()
@@ -44,6 +47,7 @@ void OnAFieldOfDifferentInstance()
         lock (anotherInstance.readonlyField) { }
         lock (anotherInstance.readWriteField) { }        // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
         lock (anotherInstance.readonlyField) { }
+        lock (anotherInstance?.readWriteField) { }       // FN: ?. not supported
     }
 
     void OnALocalVariable()
@@ -60,13 +64,11 @@ void OnANewInstance()
     {
         lock (new object()) { }                          // Noncompliant {{Locking on a new instance is a no-op.}}
         lock (new ANamespace.AClass()) { }               // Noncompliant
-        lock (new Test[] { })                            // Noncompliant
-        lock (new[] { readonlyField}) { }                // Noncompliant
+        lock (new Test[] { }) { }                        // Noncompliant
+        lock (new[] { readonlyField }) { }               // Noncompliant
         lock (new Tuple<object>(readonlyField)) { }      // Noncompliant
-        lock (new { })                                   // Noncompliant
+        lock (new { }) { }                               // Noncompliant
 
-        lock ("a string") { }                            // Noncompliant
-        lock ($"an interpolated {"string"}") { }         // Noncompliant
         lock (1) { }                                     // Error [CS0185]
         lock ((a: readonlyField, b: readonlyField)) { }  // Error [CS0185]
 
@@ -78,6 +80,16 @@ void OnANewInstance()
         lock (from x in new object[2] select x) { }      // Noncompliant
     }
 
+    void OnAStringInstance()
+    {
+        lock ("a string") { }                            // Noncompliant {{Strings can be interned, and should not be used for locking.}}
+        lock ($"an interpolated {"string"}") { }         // Noncompliant {{Strings can be interned, and should not be used for locking.}}
+        lock ("a" + "string") { }                        // Noncompliant {{Strings can be interned, and should not be used for locking.}}
+        lock (MethodReturningString()) { }               // Noncompliant {{Strings can be interned, and should not be used for locking.}}
+
+        string MethodReturningString() => "a string";
+    }
+
     void OnAssignment()
     {
         object x;
@@ -85,7 +97,7 @@ void OnAssignment()
         lock (x = readWriteField) { }                    // FN, assignment not supported
     }
 
-    void OtherCases(object oPar, bool bPar)
+    void OtherCases(object oPar, bool bPar, object[] arrayPar)
     {
         lock (null) { }
 
@@ -109,6 +121,8 @@ void OtherCases(object oPar, bool bPar)
 
         lock (oPar ?? readonlyField) { }
         lock (oPar = readonlyField) { }
+
+        lock (arrayPar[0]) { }
     }
 
     void ReadWriteReferences()
@@ -120,6 +134,12 @@ void ReadWriteReferences()
         ref object RefReturnStaticReadonlyField(Test instance) => ref Test.staticReadWriteField;
     }
 
+    void NoIdentifier()
+    {
+        lock () { }   // Error
+        lock (()) { } // Error
+    }
+
     delegate object ADelegate(object oPar);
 }
 

From ef84ace7ff0056a99eeca5bf38b922fce1f3aee3 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 22 Feb 2023 11:04:17 +0100
Subject: [PATCH 05/24] Update from rspec

---
 analyzers/rspec/cs/S2445_c#.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/analyzers/rspec/cs/S2445_c#.html b/analyzers/rspec/cs/S2445_c#.html
index 0119f6dfac7..b7bbc9cb9d8 100644
--- a/analyzers/rspec/cs/S2445_c#.html
+++ b/analyzers/rspec/cs/S2445_c#.html
@@ -2,7 +2,7 @@
 makes it possible for the field’s value to change while a thread is in a block locked on the old value. That would allow a second thread, locked on
 the new value, to enter the block at the same time.</p>
 <p>Locking on a <code>readonly</code> field of a class which is not <code>private</code> allows external code to lock the field, potentially
-interfering with synchronization by methods of that class.</p>
+interfering with synchronization of methods in that class.</p>
 <p>Locking on a local variable or on a new instance undermines the synchronization: two different threads running the method in parallel would lock on
 two different object instances.</p>
 <p>Locking on a string literal is even more dangerous: depending on whether the string is interned or not, different threads may or may not

From 02a3dc58e8b936521f38c70fbefbbdb6616602bf Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 22 Feb 2023 13:12:18 +0100
Subject: [PATCH 06/24] Fix Code Smell about cognitive complexity

---
 .../LockedFieldShouldBePrivateAndReadonly.cs  | 55 ++++++++++---------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
index 324e02c2a2c..2e07cfef0d3 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
@@ -33,41 +33,46 @@ public sealed class LockedFieldShouldBePrivateAndReadonly : SonarDiagnosticAnaly
         context.RegisterNodeAction(c =>
             {
                 var expression = ((LockStatementSyntax)c.Node).Expression?.RemoveParentheses();
-                if (expression is ObjectCreationExpressionSyntax
-                    or AnonymousObjectCreationExpressionSyntax
-                    or ArrayCreationExpressionSyntax
-                    or ImplicitArrayCreationExpressionSyntax
-                    or QueryExpressionSyntax)
+                if (IsCreation(expression))
                 {
                     ReportIssue("Locking on a new instance is a no-op.");
                 }
-                else
+                else if (IsOfTypeString(c.SemanticModel, expression))
                 {
-                    var type = c.SemanticModel.GetTypeInfo(expression).Type;
-
-                    if (expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression)
-                        || (type is not null && type.Is(KnownType.System_String)))
-                    {
-                        ReportIssue("Strings can be interned, and should not be used for locking.");
-                    }
-                    else if (expression is IdentifierNameSyntax
-                        && c.SemanticModel.GetSymbolInfo(expression).Symbol is ILocalSymbol lockedSymbol)
+                    ReportIssue("Strings can be interned, and should not be used for locking.");
+                }
+                else if (expression is IdentifierNameSyntax
+                    && c.SemanticModel.GetSymbolInfo(expression).Symbol is ILocalSymbol lockedSymbol)
+                {
+                    ReportIssue($"'{lockedSymbol.Name}' is a local variable, and should not be used for locking.");
+                }
+                else if (expression is (IdentifierNameSyntax or MemberAccessExpressionSyntax)
+                    && c.SemanticModel.GetSymbolInfo(expression).Symbol is IFieldSymbol lockedField
+                    && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private))
+                {
+                    if (lockedField.ContainingType is { } lockedFieldType && c.ContainingSymbol?.ContainingType is { } containingType && !lockedFieldType.Equals(containingType))
                     {
-                        ReportIssue($"'{lockedSymbol.Name}' is a local variable, and should not be used for locking.");
+                        ReportIssue($"Use field from '{containingType.ToMinimalDisplayString(c.SemanticModel, expression.SpanStart)}' for locking.");
                     }
-                    else if (expression is (IdentifierNameSyntax or MemberAccessExpressionSyntax)
-                        && c.SemanticModel.GetSymbolInfo(expression).Symbol is IFieldSymbol lockedField
-                        && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private))
+                    else
                     {
-                        ReportIssue(lockedField.ContainingType is { } lockedFieldType
-                            && c.ContainingSymbol?.ContainingType is { } containingType
-                            && !lockedFieldType.Equals(containingType)
-                                ? $"Use field from '{containingType.ToMinimalDisplayString(c.SemanticModel, expression.SpanStart)}' for locking."
-                                : $"'{lockedField.Name}' is not 'private readonly', and should not be used for locking.");
+                        ReportIssue($"'{lockedField.Name}' is not 'private readonly', and should not be used for locking.");
                     }
                 }
 
-                void ReportIssue(string message) => c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), message));
+                void ReportIssue(string message) =>
+                    c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), message));
             },
             SyntaxKind.LockStatement);
+
+    private static bool IsCreation(ExpressionSyntax expression) =>
+        expression is ObjectCreationExpressionSyntax
+            or AnonymousObjectCreationExpressionSyntax
+            or ArrayCreationExpressionSyntax
+            or ImplicitArrayCreationExpressionSyntax
+            or QueryExpressionSyntax;
+
+    private static bool IsOfTypeString(SemanticModel model, ExpressionSyntax expression) =>
+        expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression)
+            || (model.GetTypeInfo(expression).Type is { } type && type.Is(KnownType.System_String));
 }

From b1216ef776ee86694d85afa4d158914ac626d63f Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 22 Feb 2023 13:17:37 +0100
Subject: [PATCH 07/24] Fix spelling

---
 .../Rules/LockedFieldShouldBePrivateAndReadonly.cs              | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
index 2e07cfef0d3..b5d73f011e8 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
@@ -52,7 +52,7 @@ public sealed class LockedFieldShouldBePrivateAndReadonly : SonarDiagnosticAnaly
                 {
                     if (lockedField.ContainingType is { } lockedFieldType && c.ContainingSymbol?.ContainingType is { } containingType && !lockedFieldType.Equals(containingType))
                     {
-                        ReportIssue($"Use field from '{containingType.ToMinimalDisplayString(c.SemanticModel, expression.SpanStart)}' for locking.");
+                        ReportIssue($"Use fields from '{containingType.ToMinimalDisplayString(c.SemanticModel, expression.SpanStart)}' for locking.");
                     }
                     else
                     {

From 5dadc9efe49dc03ede65b4a5db3b749d8547c401 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 22 Feb 2023 13:18:22 +0100
Subject: [PATCH 08/24] Fix spelling 2

---
 .../TestCases/LockedFieldShouldBePrivateAndReadonly.cs        | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
index 8675c562d26..92419aabc29 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
@@ -24,8 +24,8 @@ void OnAStaticField()
         lock (Test.staticReadonlyField) { }
         lock (Test.staticReadWriteField) { }             // Noncompliant {{'staticReadWriteField' is not 'private readonly', and should not be used for locking.}}
         //    ^^^^^^^^^^^^^^^^^^^^^^^^^
-        lock (AnotherClass.staticReadonlyField) { }      // Noncompliant {{Use field from 'Test' for locking.}}
-        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{Use field from 'Test' for locking.}}
+        lock (AnotherClass.staticReadonlyField) { }      // Noncompliant {{Use fields from 'Test' for locking.}}
+        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{Use fields from 'Test' for locking.}}
         //    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     }
 

From 6d636e84b9ae9361708f4865b7e5ce74915f5ca9 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 22 Feb 2023 13:46:41 +0100
Subject: [PATCH 09/24] Update rspec

---
 analyzers/rspec/cs/S2445_c#.html | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/analyzers/rspec/cs/S2445_c#.html b/analyzers/rspec/cs/S2445_c#.html
index b7bbc9cb9d8..fc6dc281284 100644
--- a/analyzers/rspec/cs/S2445_c#.html
+++ b/analyzers/rspec/cs/S2445_c#.html
@@ -3,31 +3,37 @@
 the new value, to enter the block at the same time.</p>
 <p>Locking on a <code>readonly</code> field of a class which is not <code>private</code> allows external code to lock the field, potentially
 interfering with synchronization of methods in that class.</p>
-<p>Locking on a local variable or on a new instance undermines the synchronization: two different threads running the method in parallel would lock on
-two different object instances.</p>
+<p>Locking on a local variable or on a new instance undermines the synchronization: two different threads running the same method in parallel would
+lock on two different object instances. That would allow a second thread, locked on that new value, to enter the same block concurrently.</p>
 <p>Locking on a string literal is even more dangerous: depending on whether the string is interned or not, different threads may or may not
 synchronize on the same object instance.</p>
 <h2>Noncompliant Code Example</h2>
 <pre>
-private string color = "red";
+private Color colorObject = new Color("red");
+private readonly colorString = "red";
 
 private void DoSomething()
 {
-  lock (color) // Noncompliant; lock is actually on object instance "red" referred to by the color variable
+  // Synchronizing access via "colorObject"
+  lock (colorObject) // Noncompliant; lock is actually on object instance "red" referred to by the color field
   {
     //...
-    color = "green"; // other threads now allowed into this block
+    colorObject = new Color("green"); // other threads now allowed into this block
     // ...
   }
-  lock (new object()) // Noncompliant this is a no-op.
+  lock (new object()) // Noncompliant; this is a no-op
   {
-     // ...
+    // ...
+  }
+  lock (colorString)  // Noncompliant; strings can be interned
+  {
+    // ...
   }
 }
 </pre>
 <h2>Compliant Solution</h2>
 <pre>
-private string color = "red";
+private Color colorObject = new Color("red");
 private readonly object lockObj = new object();
 
 private void DoSomething()
@@ -35,7 +41,7 @@ <h2>Compliant Solution</h2>
   lock (lockObj)
   {
     //...
-    color = "green";
+    color = new Color("green");
     // ...
   }
 }

From b81bc28c9dfffaefa9997b295342abc868223312 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 22 Feb 2023 16:42:50 +0100
Subject: [PATCH 10/24] Update rspec 2

---
 analyzers/rspec/cs/S2445_c#.html | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/analyzers/rspec/cs/S2445_c#.html b/analyzers/rspec/cs/S2445_c#.html
index fc6dc281284..aa8f8ab4612 100644
--- a/analyzers/rspec/cs/S2445_c#.html
+++ b/analyzers/rspec/cs/S2445_c#.html
@@ -1,12 +1,15 @@
-<p>Locking on a class field synchronizes not on the field itself, but on the object assigned to it. So locking on a non-<code>readonly</code> field
-makes it possible for the field’s value to change while a thread is in a block locked on the old value. That would allow a second thread, locked on
-the new value, to enter the block at the same time.</p>
-<p>Locking on a <code>readonly</code> field of a class which is not <code>private</code> allows external code to lock the field, potentially
-interfering with synchronization of methods in that class.</p>
-<p>Locking on a local variable or on a new instance undermines the synchronization: two different threads running the same method in parallel would
-lock on two different object instances. That would allow a second thread, locked on that new value, to enter the same block concurrently.</p>
-<p>Locking on a string literal is even more dangerous: depending on whether the string is interned or not, different threads may or may not
-synchronize on the same object instance.</p>
+<p>Locking on a class field synchronizes not on the field itself, but on the object assigned to it. Thus, there are some good practices to follow to
+avoid bugs related to thread synchronization.</p>
+<ol>
+  <li> Locking on a non-<code>readonly</code> field makes it possible for the field’s value to change while a thread is in the code block locked on
+  the old value. This allows another thread to lock on the new value and access the same block concurrently. </li>
+  <li> Locking on a <code>readonly</code> non-<code>private</code> field allows, for example, a method from another class to lock on the same field,
+  potentially interfering with the synchronization. </li>
+  <li> Locking on a local variable or a new instance of an object undermines the synchronization because two different threads running the same method
+  in parallel will lock on two different instances of the same object, allowing them to access the synchronized block at the same time. </li>
+  <li> Locking on a string literal is also dangerous since, depending on whether the string is interned or not, different threads may or may not
+  synchronize on the same object instance. </li>
+</ol>
 <h2>Noncompliant Code Example</h2>
 <pre>
 private Color colorObject = new Color("red");
@@ -48,6 +51,9 @@ <h2>Compliant Solution</h2>
 </pre>
 <h2>See</h2>
 <ul>
+  <li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/lock">Lock Statement</a> - lock statement - ensure
+  exclusive access to a shared resource </li>
+  <li> <a href="https://learn.microsoft.com/en-us/dotnet/api/system.string.intern">String.Intern</a> - <code>String.Intern(String)</code> Method </li>
   <li> <a href="https://cwe.mitre.org/data/definitions/412">MITRE, CWE-412</a> - Unrestricted Externally Accessible Lock </li>
   <li> <a href="https://cwe.mitre.org/data/definitions/413">MITRE, CWE-413</a> - Improper Resource Locking </li>
 </ul>

From 3645def0cf314d73ec460824f11a85986b157c0a Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Thu, 23 Feb 2023 13:48:54 +0100
Subject: [PATCH 11/24] Code review

---
 .../LockedFieldShouldBePrivateAndReadonly.cs  | 64 ++++++++++---------
 .../LockedFieldShouldBePrivateAndReadonly.cs  |  6 +-
 2 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
index b5d73f011e8..7137fc7773a 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
@@ -30,40 +30,44 @@ public sealed class LockedFieldShouldBePrivateAndReadonly : SonarDiagnosticAnaly
     public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
 
     protected override void Initialize(SonarAnalysisContext context) =>
-        context.RegisterNodeAction(c =>
+        context.RegisterNodeAction(CheckLockStatement, SyntaxKind.LockStatement);
+
+    private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
+    {
+        var expression = ((LockStatementSyntax)context.Node).Expression?.RemoveParentheses();
+        if (IsCreation(expression))
+        {
+            ReportIssue("Locking on a new instance is a no-op.");
+        }
+        else
+        {
+            var symbol = context.SemanticModel.GetSymbolInfo(expression).Symbol;
+            if (IsOfTypeString(expression, symbol))
             {
-                var expression = ((LockStatementSyntax)c.Node).Expression?.RemoveParentheses();
-                if (IsCreation(expression))
-                {
-                    ReportIssue("Locking on a new instance is a no-op.");
-                }
-                else if (IsOfTypeString(c.SemanticModel, expression))
-                {
-                    ReportIssue("Strings can be interned, and should not be used for locking.");
-                }
-                else if (expression is IdentifierNameSyntax
-                    && c.SemanticModel.GetSymbolInfo(expression).Symbol is ILocalSymbol lockedSymbol)
+                ReportIssue("Strings can be interned, and should not be used for locking.");
+            }
+            else if (expression is IdentifierNameSyntax && symbol is ILocalSymbol lockedSymbol)
+            {
+                ReportIssue($"'{lockedSymbol.Name}' is a local variable, and should not be used for locking.");
+            }
+            else if (expression is (IdentifierNameSyntax or MemberAccessExpressionSyntax)
+                && symbol is IFieldSymbol lockedField
+                && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private))
+            {
+                if (lockedField.ContainingType is { } lockedFieldType && context.ContainingSymbol?.ContainingType is { } containingType && !lockedFieldType.Equals(containingType))
                 {
-                    ReportIssue($"'{lockedSymbol.Name}' is a local variable, and should not be used for locking.");
+                    ReportIssue($"Use members from '{containingType.ToMinimalDisplayString(context.SemanticModel, expression.SpanStart)}' for locking.");
                 }
-                else if (expression is (IdentifierNameSyntax or MemberAccessExpressionSyntax)
-                    && c.SemanticModel.GetSymbolInfo(expression).Symbol is IFieldSymbol lockedField
-                    && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private))
+                else
                 {
-                    if (lockedField.ContainingType is { } lockedFieldType && c.ContainingSymbol?.ContainingType is { } containingType && !lockedFieldType.Equals(containingType))
-                    {
-                        ReportIssue($"Use fields from '{containingType.ToMinimalDisplayString(c.SemanticModel, expression.SpanStart)}' for locking.");
-                    }
-                    else
-                    {
-                        ReportIssue($"'{lockedField.Name}' is not 'private readonly', and should not be used for locking.");
-                    }
+                    ReportIssue($"'{lockedField.Name}' is not 'private readonly', and should not be used for locking.");
                 }
+            }
+        }
 
-                void ReportIssue(string message) =>
-                    c.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), message));
-            },
-            SyntaxKind.LockStatement);
+        void ReportIssue(string message) =>
+            context.ReportIssue(Diagnostic.Create(Rule, expression.GetLocation(), message));
+    }
 
     private static bool IsCreation(ExpressionSyntax expression) =>
         expression is ObjectCreationExpressionSyntax
@@ -72,7 +76,7 @@ or ArrayCreationExpressionSyntax
             or ImplicitArrayCreationExpressionSyntax
             or QueryExpressionSyntax;
 
-    private static bool IsOfTypeString(SemanticModel model, ExpressionSyntax expression) =>
+    private static bool IsOfTypeString(ExpressionSyntax expression, ISymbol symbol) =>
         expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression)
-            || (model.GetTypeInfo(expression).Type is { } type && type.Is(KnownType.System_String));
+            || (symbol.GetSymbolType() is { } type && type.Is(KnownType.System_String));
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
index 92419aabc29..f8d713321c5 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
@@ -83,9 +83,9 @@ void OnANewInstance()
     void OnAStringInstance()
     {
         lock ("a string") { }                            // Noncompliant {{Strings can be interned, and should not be used for locking.}}
-        lock ($"an interpolated {"string"}") { }         // Noncompliant {{Strings can be interned, and should not be used for locking.}}
-        lock ("a" + "string") { }                        // Noncompliant {{Strings can be interned, and should not be used for locking.}}
-        lock (MethodReturningString()) { }               // Noncompliant {{Strings can be interned, and should not be used for locking.}}
+        lock ($"an interpolated {"string"}") { }         // Noncompliant
+        lock ("a" + "string") { }                        // Noncompliant
+        lock (MethodReturningString()) { }               // Noncompliant
 
         string MethodReturningString() => "a string";
     }

From cb85465ec5c9c40a7d28a4f8695500a1b6f8c560 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Fri, 24 Feb 2023 10:44:30 +0100
Subject: [PATCH 12/24] Code review 2

---
 .../LockedFieldShouldBePrivateAndReadonly.cs  | 44 +++++++++++--------
 ...ckedFieldShouldBePrivateAndReadonlyTest.cs |  8 ++++
 ...eAndReadonly.CSharp9.TopLevelStatements.cs | 40 +++++++++++++++++
 .../LockedFieldShouldBePrivateAndReadonly.cs  |  6 +--
 4 files changed, 76 insertions(+), 22 deletions(-)
 create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
index 7137fc7773a..029ba72e8ed 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
@@ -42,7 +42,7 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
         else
         {
             var symbol = context.SemanticModel.GetSymbolInfo(expression).Symbol;
-            if (IsOfTypeString(expression, symbol))
+            if (IsOfTypeString(context.SemanticModel, expression))
             {
                 ReportIssue("Strings can be interned, and should not be used for locking.");
             }
@@ -50,18 +50,11 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
             {
                 ReportIssue($"'{lockedSymbol.Name}' is a local variable, and should not be used for locking.");
             }
-            else if (expression is (IdentifierNameSyntax or MemberAccessExpressionSyntax)
-                && symbol is IFieldSymbol lockedField
-                && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private))
+            else if (FieldNotReadonlyOrNotPrivate(expression, symbol) is { } lockedField)
             {
-                if (lockedField.ContainingType is { } lockedFieldType && context.ContainingSymbol?.ContainingType is { } containingType && !lockedFieldType.Equals(containingType))
-                {
-                    ReportIssue($"Use members from '{containingType.ToMinimalDisplayString(context.SemanticModel, expression.SpanStart)}' for locking.");
-                }
-                else
-                {
-                    ReportIssue($"'{lockedField.Name}' is not 'private readonly', and should not be used for locking.");
-                }
+                ReportIssue(FieldInSameTypeAs(lockedField, context.ContainingSymbol?.ContainingType) is { } containingType
+                    ? $"Use members from '{containingType.ToMinimalDisplayString(context.SemanticModel, expression.SpanStart)}' for locking."
+                    : $"'{lockedField.Name}' is not 'private readonly', and should not be used for locking.");
             }
         }
 
@@ -70,13 +63,26 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
     }
 
     private static bool IsCreation(ExpressionSyntax expression) =>
-        expression is ObjectCreationExpressionSyntax
-            or AnonymousObjectCreationExpressionSyntax
-            or ArrayCreationExpressionSyntax
-            or ImplicitArrayCreationExpressionSyntax
-            or QueryExpressionSyntax;
+        expression.IsAnyKind(
+            SyntaxKind.ObjectCreationExpression,
+            SyntaxKind.AnonymousObjectCreationExpression,
+            SyntaxKind.ArrayCreationExpression,
+            SyntaxKind.ImplicitArrayCreationExpression,
+            SyntaxKind.QueryExpression);
 
-    private static bool IsOfTypeString(ExpressionSyntax expression, ISymbol symbol) =>
+    private static bool IsOfTypeString(SemanticModel model, ExpressionSyntax expression) =>
         expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression)
-            || (symbol.GetSymbolType() is { } type && type.Is(KnownType.System_String));
+            || expression.IsKnownType(KnownType.System_String, model);
+
+    private static IFieldSymbol FieldNotReadonlyOrNotPrivate(ExpressionSyntax expression, ISymbol symbol) =>
+        expression.IsAnyKind(SyntaxKind.IdentifierName, SyntaxKind.SimpleMemberAccessExpression)
+            && symbol is IFieldSymbol lockedField
+            && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private)
+                ? lockedField
+                : null;
+
+    private static ITypeSymbol FieldInSameTypeAs(IFieldSymbol field, INamedTypeSymbol type) =>
+        field.ContainingType is { } fieldType && type is { } && !fieldType.Equals(type)
+            ? type
+            : null;
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBePrivateAndReadonlyTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBePrivateAndReadonlyTest.cs
index ed6a414f847..f59f72770b5 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBePrivateAndReadonlyTest.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBePrivateAndReadonlyTest.cs
@@ -45,6 +45,14 @@ public class LockedFieldShouldBePrivateAndReadonlyTest
             .WithOptions(ParseOptionsHelper.FromCSharp9)
             .Verify();
 
+    [TestMethod]
+    public void LockedFieldShouldBePrivateAndReadonly_CSharp9_TopLevelStatements() =>
+        builder
+            .AddPaths("LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp9)
+            .WithTopLevelStatements()
+            .Verify();
+
     [TestMethod]
     public void LockedFieldShouldBePrivateAndReadonly_CSharp11() =>
         builder
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs
new file mode 100644
index 00000000000..3fcd5a91737
--- /dev/null
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs
@@ -0,0 +1,40 @@
+lock ("a string") { }              // Noncompliant
+//    ^^^^^^^^^^
+lock (MethodReturningString()) { } // Noncompliant
+lock (new object()) { }            // Noncompliant
+
+string MethodReturningString() => "a string";
+
+class ClassNestedAtTopLevel
+{
+    readonly object readonlyField = new();
+    object readWriteField = new();
+
+    static readonly object staticReadonlyField = new();
+    static object staticReadWriteField = new();
+
+    void Test()
+    {
+        lock (readonlyField) { }
+        lock (readWriteField) { }           // Noncompliant
+        lock (staticReadonlyField) { }
+        lock (staticReadWriteField) { }     // Noncompliant
+    }
+
+    class SecondLevelNesting
+    {
+        readonly object readonlyField = new();
+        object readWriteField = new();
+
+        static readonly object staticReadonlyField = new();
+        static object staticReadWriteField = new();
+
+        void Test()
+        {
+            lock (readonlyField) { }
+            lock (readWriteField) { }       // Noncompliant
+            lock (staticReadonlyField) { }
+            lock (staticReadWriteField) { } // Noncompliant
+        }
+    }
+}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
index f8d713321c5..06796fbbd96 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
@@ -24,8 +24,8 @@ void OnAStaticField()
         lock (Test.staticReadonlyField) { }
         lock (Test.staticReadWriteField) { }             // Noncompliant {{'staticReadWriteField' is not 'private readonly', and should not be used for locking.}}
         //    ^^^^^^^^^^^^^^^^^^^^^^^^^
-        lock (AnotherClass.staticReadonlyField) { }      // Noncompliant {{Use fields from 'Test' for locking.}}
-        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{Use fields from 'Test' for locking.}}
+        lock (AnotherClass.staticReadonlyField) { }      // Noncompliant {{Use members from 'Test' for locking.}}
+        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{Use members from 'Test' for locking.}}
         //    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     }
 
@@ -149,7 +149,7 @@ class TestExplicitCast
 
     void Test()
     {
-        lock ((AnotherClass)readonlyField) { } // Compliant, the cast operator can build
+        lock ((AnotherClass)readonlyField) { } // Compliant, the cast operator can run arbitrary code
     }
 }
 

From 72f83a26ed646ff7ce6d37024e6ad769959087eb Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Fri, 24 Feb 2023 18:00:29 +0100
Subject: [PATCH 13/24] Code review 3

---
 ...ouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs
index 3fcd5a91737..a31ca9aa2f6 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs
@@ -2,9 +2,17 @@
 //    ^^^^^^^^^^
 lock (MethodReturningString()) { } // Noncompliant
 lock (new object()) { }            // Noncompliant
+lock (staticReadonlyField) { }
+lock (staticReadWriteField) {}     // Noncompliant
 
 string MethodReturningString() => "a string";
 
+partial class Program
+{
+    static readonly object staticReadonlyField = new();
+    static object staticReadWriteField = new();
+}
+
 class ClassNestedAtTopLevel
 {
     readonly object readonlyField = new();

From a999919cdb7bc804915bcb0a27fba0042f572799 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Fri, 24 Feb 2023 18:16:04 +0100
Subject: [PATCH 14/24] Code review 4

---
 .../Rules/LockedFieldShouldBePrivateAndReadonly.cs     | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
index 029ba72e8ed..aa058389533 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
@@ -72,14 +72,14 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
 
     private static bool IsOfTypeString(SemanticModel model, ExpressionSyntax expression) =>
         expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression)
-            || expression.IsKnownType(KnownType.System_String, model);
+        || expression.IsKnownType(KnownType.System_String, model);
 
     private static IFieldSymbol FieldNotReadonlyOrNotPrivate(ExpressionSyntax expression, ISymbol symbol) =>
         expression.IsAnyKind(SyntaxKind.IdentifierName, SyntaxKind.SimpleMemberAccessExpression)
-            && symbol is IFieldSymbol lockedField
-            && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private)
-                ? lockedField
-                : null;
+        && symbol is IFieldSymbol lockedField
+        && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private)
+            ? lockedField
+            : null;
 
     private static ITypeSymbol FieldInSameTypeAs(IFieldSymbol field, INamedTypeSymbol type) =>
         field.ContainingType is { } fieldType && type is { } && !fieldType.Equals(type)

From ccea4bbf37178dabd765a9a2995d3431750cef08 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Fri, 24 Feb 2023 18:23:17 +0100
Subject: [PATCH 15/24] Code review 5

---
 .../Rules/LockedFieldShouldBePrivateAndReadonly.cs          | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
index aa058389533..c4f62d2e132 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
@@ -42,7 +42,7 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
         else
         {
             var symbol = context.SemanticModel.GetSymbolInfo(expression).Symbol;
-            if (IsOfTypeString(context.SemanticModel, expression))
+            if (IsOfTypeString(expression, symbol))
             {
                 ReportIssue("Strings can be interned, and should not be used for locking.");
             }
@@ -70,9 +70,9 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
             SyntaxKind.ImplicitArrayCreationExpression,
             SyntaxKind.QueryExpression);
 
-    private static bool IsOfTypeString(SemanticModel model, ExpressionSyntax expression) =>
+    private static bool IsOfTypeString(ExpressionSyntax expression, ISymbol symbol) =>
         expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression)
-        || expression.IsKnownType(KnownType.System_String, model);
+        || symbol.GetSymbolType().Is(KnownType.System_String);
 
     private static IFieldSymbol FieldNotReadonlyOrNotPrivate(ExpressionSyntax expression, ISymbol symbol) =>
         expression.IsAnyKind(SyntaxKind.IdentifierName, SyntaxKind.SimpleMemberAccessExpression)

From ff3c4f8920676126429cafcfacd78f59bcc4e7ed Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Fri, 24 Feb 2023 18:30:52 +0100
Subject: [PATCH 16/24] Make symbol retrieval lazy

---
 .../LockedFieldShouldBePrivateAndReadonly.cs     | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
index c4f62d2e132..265abf2a8cb 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
@@ -41,16 +41,16 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
         }
         else
         {
-            var symbol = context.SemanticModel.GetSymbolInfo(expression).Symbol;
-            if (IsOfTypeString(expression, symbol))
+            var lazySymbol = new Lazy<ISymbol>(() => context.SemanticModel.GetSymbolInfo(expression).Symbol);
+            if (IsOfTypeString(expression, lazySymbol))
             {
                 ReportIssue("Strings can be interned, and should not be used for locking.");
             }
-            else if (expression is IdentifierNameSyntax && symbol is ILocalSymbol lockedSymbol)
+            else if (expression is IdentifierNameSyntax && lazySymbol.Value is ILocalSymbol lockedSymbol)
             {
                 ReportIssue($"'{lockedSymbol.Name}' is a local variable, and should not be used for locking.");
             }
-            else if (FieldNotReadonlyOrNotPrivate(expression, symbol) is { } lockedField)
+            else if (FieldNotReadonlyOrNotPrivate(expression, lazySymbol) is { } lockedField)
             {
                 ReportIssue(FieldInSameTypeAs(lockedField, context.ContainingSymbol?.ContainingType) is { } containingType
                     ? $"Use members from '{containingType.ToMinimalDisplayString(context.SemanticModel, expression.SpanStart)}' for locking."
@@ -70,13 +70,13 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
             SyntaxKind.ImplicitArrayCreationExpression,
             SyntaxKind.QueryExpression);
 
-    private static bool IsOfTypeString(ExpressionSyntax expression, ISymbol symbol) =>
+    private static bool IsOfTypeString(ExpressionSyntax expression, Lazy<ISymbol> lazySymbol) =>
         expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression)
-        || symbol.GetSymbolType().Is(KnownType.System_String);
+        || lazySymbol.Value.GetSymbolType().Is(KnownType.System_String);
 
-    private static IFieldSymbol FieldNotReadonlyOrNotPrivate(ExpressionSyntax expression, ISymbol symbol) =>
+    private static IFieldSymbol FieldNotReadonlyOrNotPrivate(ExpressionSyntax expression, Lazy<ISymbol> lazySymbol) =>
         expression.IsAnyKind(SyntaxKind.IdentifierName, SyntaxKind.SimpleMemberAccessExpression)
-        && symbol is IFieldSymbol lockedField
+        && lazySymbol.Value is IFieldSymbol lockedField
         && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private)
             ? lockedField
             : null;

From 74c42f8b23e1cb8c726a6909a532bde43cee8d44 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Mon, 27 Feb 2023 12:06:50 +0100
Subject: [PATCH 17/24] Update rspec after review

---
 analyzers/rspec/cs/S2445_c#.html | 18 ++++++++----------
 analyzers/rspec/cs/S2445_c#.json |  2 +-
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/analyzers/rspec/cs/S2445_c#.html b/analyzers/rspec/cs/S2445_c#.html
index aa8f8ab4612..21ef51420e3 100644
--- a/analyzers/rspec/cs/S2445_c#.html
+++ b/analyzers/rspec/cs/S2445_c#.html
@@ -3,25 +3,23 @@
 <ol>
   <li> Locking on a non-<code>readonly</code> field makes it possible for the field’s value to change while a thread is in the code block locked on
   the old value. This allows another thread to lock on the new value and access the same block concurrently. </li>
-  <li> Locking on a <code>readonly</code> non-<code>private</code> field allows, for example, a method from another class to lock on the same field,
-  potentially interfering with the synchronization. </li>
-  <li> Locking on a local variable or a new instance of an object undermines the synchronization because two different threads running the same method
-  in parallel will lock on two different instances of the same object, allowing them to access the synchronized block at the same time. </li>
+  <li> Locking on a local variable or a new instance of an object can undermine synchronization because two different threads running the same method
+  in parallel will potentially lock on different instances of the same object, allowing them to access the synchronized block at the same time. </li>
   <li> Locking on a string literal is also dangerous since, depending on whether the string is interned or not, different threads may or may not
   synchronize on the same object instance. </li>
 </ol>
 <h2>Noncompliant Code Example</h2>
 <pre>
-private Color colorObject = new Color("red");
-private readonly colorString = "red";
+private Color color = new Color("red");
+private readonly string colorString = "red";
 
 private void DoSomething()
 {
-  // Synchronizing access via "colorObject"
-  lock (colorObject) // Noncompliant; lock is actually on object instance "red" referred to by the color field
+  // Synchronizing access via "color"
+  lock (color) // Noncompliant; lock is actually on object instance "red" referred to by the "color" field
   {
     //...
-    colorObject = new Color("green"); // other threads now allowed into this block
+    color = new Color("green"); // other threads now allowed into this block
     // ...
   }
   lock (new object()) // Noncompliant; this is a no-op
@@ -36,7 +34,7 @@ <h2>Noncompliant Code Example</h2>
 </pre>
 <h2>Compliant Solution</h2>
 <pre>
-private Color colorObject = new Color("red");
+private Color color = new Color("red");
 private readonly object lockObj = new object();
 
 private void DoSomething()
diff --git a/analyzers/rspec/cs/S2445_c#.json b/analyzers/rspec/cs/S2445_c#.json
index 3bad3ab9135..d5b4a8d6d3f 100644
--- a/analyzers/rspec/cs/S2445_c#.json
+++ b/analyzers/rspec/cs/S2445_c#.json
@@ -1,5 +1,5 @@
 {
-  "title": "Blocks should be synchronized on \"private readonly\" fields",
+  "title": "Blocks should be synchronized on \"readonly\" fields",
   "type": "BUG",
   "status": "ready",
   "remediation": {

From 5f444c652e02ab8fefd7c7523ed0952cc2a38ef0 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Mon, 27 Feb 2023 12:08:44 +0100
Subject: [PATCH 18/24] Remove top-level statement tests, remove private check
 from rule implementation and names

---
 ...only.cs => LockedFieldShouldBeReadonly.cs} | 20 ++++----
 ....cs => LockedFieldShouldBeReadonlyTest.cs} | 28 ++++-------
 ...eAndReadonly.CSharp9.TopLevelStatements.cs | 48 -------------------
 ...> LockedFieldShouldBeReadonly.CSharp11.cs} |  0
 ...=> LockedFieldShouldBeReadonly.CSharp8.cs} |  0
 ...=> LockedFieldShouldBeReadonly.CSharp9.cs} |  0
 ...only.cs => LockedFieldShouldBeReadonly.cs} |  0
 7 files changed, 18 insertions(+), 78 deletions(-)
 rename analyzers/src/SonarAnalyzer.CSharp/Rules/{LockedFieldShouldBePrivateAndReadonly.cs => LockedFieldShouldBeReadonly.cs} (74%)
 rename analyzers/tests/SonarAnalyzer.UnitTest/Rules/{LockedFieldShouldBePrivateAndReadonlyTest.cs => LockedFieldShouldBeReadonlyTest.cs} (58%)
 delete mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs
 rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/{LockedFieldShouldBePrivateAndReadonly.CSharp11.cs => LockedFieldShouldBeReadonly.CSharp11.cs} (100%)
 rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/{LockedFieldShouldBePrivateAndReadonly.CSharp8.cs => LockedFieldShouldBeReadonly.CSharp8.cs} (100%)
 rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/{LockedFieldShouldBePrivateAndReadonly.CSharp9.cs => LockedFieldShouldBeReadonly.CSharp9.cs} (100%)
 rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/{LockedFieldShouldBePrivateAndReadonly.cs => LockedFieldShouldBeReadonly.cs} (100%)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
similarity index 74%
rename from analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
rename to analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
index 265abf2a8cb..8c954411cc4 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBePrivateAndReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
@@ -21,7 +21,7 @@
 namespace SonarAnalyzer.Rules.CSharp;
 
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
-public sealed class LockedFieldShouldBePrivateAndReadonly : SonarDiagnosticAnalyzer
+public sealed class LockedFieldShouldBeReadonly : SonarDiagnosticAnalyzer
 {
     private const string DiagnosticId = "S2445";
 
@@ -37,24 +37,22 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
         var expression = ((LockStatementSyntax)context.Node).Expression?.RemoveParentheses();
         if (IsCreation(expression))
         {
-            ReportIssue("Locking on a new instance is a no-op.");
+            ReportIssue("Locking on a new instance is a no-op, use a 'readonly' field instead.");
         }
         else
         {
             var lazySymbol = new Lazy<ISymbol>(() => context.SemanticModel.GetSymbolInfo(expression).Symbol);
             if (IsOfTypeString(expression, lazySymbol))
             {
-                ReportIssue("Strings can be interned, and should not be used for locking.");
+                ReportIssue("Strings can be interned, and should not be used for locking. Use a 'readonly' field instead.");
             }
             else if (expression is IdentifierNameSyntax && lazySymbol.Value is ILocalSymbol lockedSymbol)
             {
-                ReportIssue($"'{lockedSymbol.Name}' is a local variable, and should not be used for locking.");
+                ReportIssue($"Do not lock on local variable '{lockedSymbol.Name}', use a 'readonly' field instead.");
             }
-            else if (FieldNotReadonlyOrNotPrivate(expression, lazySymbol) is { } lockedField)
+            else if (FieldNotReadonly(expression, lazySymbol) is { } lockedField)
             {
-                ReportIssue(FieldInSameTypeAs(lockedField, context.ContainingSymbol?.ContainingType) is { } containingType
-                    ? $"Use members from '{containingType.ToMinimalDisplayString(context.SemanticModel, expression.SpanStart)}' for locking."
-                    : $"'{lockedField.Name}' is not 'private readonly', and should not be used for locking.");
+                ReportIssue($"Do not lock on non-'readonly' field '{lockedField.Name}', use a 'readonly' field instead.");
             }
         }
 
@@ -74,10 +72,8 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
         expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression)
         || lazySymbol.Value.GetSymbolType().Is(KnownType.System_String);
 
-    private static IFieldSymbol FieldNotReadonlyOrNotPrivate(ExpressionSyntax expression, Lazy<ISymbol> lazySymbol) =>
-        expression.IsAnyKind(SyntaxKind.IdentifierName, SyntaxKind.SimpleMemberAccessExpression)
-        && lazySymbol.Value is IFieldSymbol lockedField
-        && (!lockedField.IsReadOnly || lockedField.GetEffectiveAccessibility() != Accessibility.Private)
+    private static IFieldSymbol FieldNotReadonly(ExpressionSyntax expression, Lazy<ISymbol> lazySymbol) =>
+        expression.IsAnyKind(SyntaxKind.IdentifierName, SyntaxKind.SimpleMemberAccessExpression) && lazySymbol.Value is IFieldSymbol lockedField && !lockedField.IsReadOnly
             ? lockedField
             : null;
 
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBePrivateAndReadonlyTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBeReadonlyTest.cs
similarity index 58%
rename from analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBePrivateAndReadonlyTest.cs
rename to analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBeReadonlyTest.cs
index f59f72770b5..f5f3a4b7f5d 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBePrivateAndReadonlyTest.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/LockedFieldShouldBeReadonlyTest.cs
@@ -23,40 +23,32 @@
 namespace SonarAnalyzer.UnitTest.Rules;
 
 [TestClass]
-public class LockedFieldShouldBePrivateAndReadonlyTest
+public class LockedFieldShouldBeReadonlyTest
 {
-    private readonly VerifierBuilder builder = new VerifierBuilder<LockedFieldShouldBePrivateAndReadonly>();
+    private readonly VerifierBuilder builder = new VerifierBuilder<LockedFieldShouldBeReadonly>();
 
     [TestMethod]
-    public void LockedFieldShouldBePrivateAndReadonly_CS() =>
-        builder.AddPaths("LockedFieldShouldBePrivateAndReadonly.cs").Verify();
+    public void LockedFieldShouldBeReadonly_CS() =>
+        builder.AddPaths("LockedFieldShouldBeReadonly.cs").Verify();
 
     [TestMethod]
-    public void LockedFieldShouldBePrivateAndReadonly_CSharp8() =>
+    public void LockedFieldShouldBeReadonly_CSharp8() =>
         builder
-            .AddPaths("LockedFieldShouldBePrivateAndReadonly.CSharp8.cs")
+            .AddPaths("LockedFieldShouldBeReadonly.CSharp8.cs")
             .WithOptions(ParseOptionsHelper.FromCSharp8)
             .Verify();
 
     [TestMethod]
-    public void LockedFieldShouldBePrivateAndReadonly_CSharp9() =>
+    public void LockedFieldShouldBeReadonly_CSharp9() =>
         builder
-            .AddPaths("LockedFieldShouldBePrivateAndReadonly.CSharp9.cs")
+            .AddPaths("LockedFieldShouldBeReadonly.CSharp9.cs")
             .WithOptions(ParseOptionsHelper.FromCSharp9)
             .Verify();
 
     [TestMethod]
-    public void LockedFieldShouldBePrivateAndReadonly_CSharp9_TopLevelStatements() =>
+    public void LockedFieldShouldBeReadonly_CSharp11() =>
         builder
-            .AddPaths("LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs")
-            .WithOptions(ParseOptionsHelper.FromCSharp9)
-            .WithTopLevelStatements()
-            .Verify();
-
-    [TestMethod]
-    public void LockedFieldShouldBePrivateAndReadonly_CSharp11() =>
-        builder
-            .AddPaths("LockedFieldShouldBePrivateAndReadonly.CSharp11.cs")
+            .AddPaths("LockedFieldShouldBeReadonly.CSharp11.cs")
             .WithOptions(ParseOptionsHelper.FromCSharp11)
             .Verify();
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs
deleted file mode 100644
index a31ca9aa2f6..00000000000
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.TopLevelStatements.cs
+++ /dev/null
@@ -1,48 +0,0 @@
-lock ("a string") { }              // Noncompliant
-//    ^^^^^^^^^^
-lock (MethodReturningString()) { } // Noncompliant
-lock (new object()) { }            // Noncompliant
-lock (staticReadonlyField) { }
-lock (staticReadWriteField) {}     // Noncompliant
-
-string MethodReturningString() => "a string";
-
-partial class Program
-{
-    static readonly object staticReadonlyField = new();
-    static object staticReadWriteField = new();
-}
-
-class ClassNestedAtTopLevel
-{
-    readonly object readonlyField = new();
-    object readWriteField = new();
-
-    static readonly object staticReadonlyField = new();
-    static object staticReadWriteField = new();
-
-    void Test()
-    {
-        lock (readonlyField) { }
-        lock (readWriteField) { }           // Noncompliant
-        lock (staticReadonlyField) { }
-        lock (staticReadWriteField) { }     // Noncompliant
-    }
-
-    class SecondLevelNesting
-    {
-        readonly object readonlyField = new();
-        object readWriteField = new();
-
-        static readonly object staticReadonlyField = new();
-        static object staticReadWriteField = new();
-
-        void Test()
-        {
-            lock (readonlyField) { }
-            lock (readWriteField) { }       // Noncompliant
-            lock (staticReadonlyField) { }
-            lock (staticReadWriteField) { } // Noncompliant
-        }
-    }
-}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp11.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp11.cs
similarity index 100%
rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp11.cs
rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp11.cs
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp8.cs
similarity index 100%
rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp8.cs
rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp8.cs
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp9.cs
similarity index 100%
rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.CSharp9.cs
rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp9.cs
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
similarity index 100%
rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBePrivateAndReadonly.cs
rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs

From e468e940962dc452a43007c7a13a40d04e6e0550 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Mon, 27 Feb 2023 13:29:39 +0100
Subject: [PATCH 19/24] Update unit tests after removing private check

---
 .../LockedFieldShouldBeReadonly.CSharp8.cs    | 13 -----
 .../TestCases/LockedFieldShouldBeReadonly.cs  | 56 ++++++++++---------
 2 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp8.cs
index ffb6bbd01fe..22b720a2c00 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp8.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp8.cs
@@ -44,16 +44,3 @@ void SwitchExpression(object oPar)
         lock (oPar switch { _ => new object() }) { } // FN, switch expression not supported
     }
 }
-
-class NonPrivateAccessibily
-{
-    private protected readonly object privateProtectedField = null;
-
-    private object PrivateProtectedProperty => null;
-
-    void Test()
-    {
-        lock (privateProtectedField) { }    // Noncompliant
-        lock (PrivateProtectedProperty) { } // Compliant, not a field
-    }
-}
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
index 06796fbbd96..e56c1163dd9 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
@@ -19,33 +19,37 @@ class Test
     void OnAStaticField()
     {
         lock (staticReadonlyField) { }
-        lock (staticReadWriteField) { }                  // Noncompliant {{'staticReadWriteField' is not 'private readonly', and should not be used for locking.}}
+        lock (staticReadWriteField) { }                  // Noncompliant {{Do not lock on non-'readonly' field 'staticReadWriteField', use a 'readonly' field instead.}}
         //    ^^^^^^^^^^^^^^^^^^^^
         lock (Test.staticReadonlyField) { }
-        lock (Test.staticReadWriteField) { }             // Noncompliant {{'staticReadWriteField' is not 'private readonly', and should not be used for locking.}}
+        lock (Test.staticReadWriteField) { }             // Noncompliant {{Do not lock on non-'readonly' field 'staticReadWriteField', use a 'readonly' field instead.}}
         //    ^^^^^^^^^^^^^^^^^^^^^^^^^
-        lock (AnotherClass.staticReadonlyField) { }      // Noncompliant {{Use members from 'Test' for locking.}}
-        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{Use members from 'Test' for locking.}}
+        lock (AnotherClass.staticReadonlyField) { }
+        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{Do not lock on non-'readonly' field 'staticReadWriteField', use a 'readonly' field instead.}}
         //    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     }
 
     void OnAFieldOfSameInstance()
     {
         lock (readonlyField) { }
-        lock ((readonlyField)) { }
-        lock (readonlyStringField) { }                   // Noncompliant {{Strings can be interned, and should not be used for locking.}}
-        lock (readWriteField) { }                        // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
-        lock ((readWriteField)) { }                      // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
+        lock (readonlyStringField) { }                   // Noncompliant {{Strings can be interned, and should not be used for locking. Use a 'readonly' field instead.}}
+        lock (readWriteField) { }                        // Noncompliant {{Do not lock on non-'readonly' field 'readWriteField', use a 'readonly' field instead.}}
         lock (this.readonlyField) { }
-        lock (this.readWriteField) { }                   // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
-        lock ((this.readWriteField)) { }                 // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
+        lock (this.readWriteField) { }                   // Noncompliant {{Do not lock on non-'readonly' field 'readWriteField', use a 'readonly' field instead.}}
+    }
+
+    void OnParenthesizedExpressions()
+    {
+        lock ((readonlyField)) { }
+        lock ((readWriteField)) { }                      // Noncompliant {{Do not lock on non-'readonly' field 'readWriteField', use a 'readonly' field instead.}}
+        lock ((this.readWriteField)) { }                 // Noncompliant {{Do not lock on non-'readonly' field 'readWriteField', use a 'readonly' field instead.}}
     }
 
     void OnAFieldOfDifferentInstance()
     {
         var anotherInstance = new Test();
         lock (anotherInstance.readonlyField) { }
-        lock (anotherInstance.readWriteField) { }        // Noncompliant {{'readWriteField' is not 'private readonly', and should not be used for locking.}}
+        lock (anotherInstance.readWriteField) { }        // Noncompliant {{Do not lock on non-'readonly' field 'readWriteField', use a 'readonly' field instead.}}
         lock (anotherInstance.readonlyField) { }
         lock (anotherInstance?.readWriteField) { }       // FN: ?. not supported
     }
@@ -53,7 +57,7 @@ void OnAFieldOfDifferentInstance()
     void OnALocalVariable()
     {
         object localVarNull = null;
-        lock (localVarNull) { }                          // Noncompliant {{'localVarNull' is a local variable, and should not be used for locking.}}
+        lock (localVarNull) { }                          // Noncompliant {{Do not lock on local variable 'localVarNull', use a 'readonly' field instead.}}
         object localVarReadonlyField = readonlyField;
         lock (localVarReadonlyField) { }                 // Noncompliant, while the local variable references a readonly field, the local variable itself can mutate
         object localVarReadWriteField = readWriteField;
@@ -62,7 +66,7 @@ void OnALocalVariable()
 
     void OnANewInstance()
     {
-        lock (new object()) { }                          // Noncompliant {{Locking on a new instance is a no-op.}}
+        lock (new object()) { }                          // Noncompliant {{Locking on a new instance is a no-op, use a 'readonly' field instead.}}
         lock (new ANamespace.AClass()) { }               // Noncompliant
         lock (new Test[] { }) { }                        // Noncompliant
         lock (new[] { readonlyField }) { }               // Noncompliant
@@ -82,7 +86,7 @@ void OnANewInstance()
 
     void OnAStringInstance()
     {
-        lock ("a string") { }                            // Noncompliant {{Strings can be interned, and should not be used for locking.}}
+        lock ("a string") { }                            // Noncompliant {{Strings can be interned, and should not be used for locking. Use a 'readonly' field instead.}}
         lock ($"an interpolated {"string"}") { }         // Noncompliant
         lock ("a" + "string") { }                        // Noncompliant
         lock (MethodReturningString()) { }               // Noncompliant
@@ -164,7 +168,7 @@ class AnotherClass
     public static explicit operator AnotherClass(Test o) => new AnotherClass();
 }
 
-class NonPrivateAccessibily
+class FieldAccessibily
 {
     private readonly object privateField = null;
     protected readonly object protectedField = null;
@@ -180,17 +184,17 @@ class NonPrivateAccessibily
 
     void Test()
     {
-        lock (privateField) { }              // Compliant
-        lock (protectedField) { }            // Noncompliant
-        lock (protectedInternalField) { }    // Noncompliant
-        lock (internalField) { }             // Noncompliant
-        lock (publicField) { }               // Noncompliant
-
-        lock (PrivateProperty) { }           // Compliant, not a field
-        lock (ProtectedProperty) { }         // Compliant, not a field
-        lock (ProtectedInternalProperty) { } // Compliant, not a field
-        lock (InternalProperty) { }          // Compliant, not a field
-        lock (PublicProperty) { }            // Compliant, not a field
+        lock (privateField) { }
+        lock (protectedField) { }
+        lock (protectedInternalField) { }
+        lock (internalField) { }
+        lock (publicField) { }
+
+        lock (PrivateProperty) { }
+        lock (ProtectedProperty) { }
+        lock (ProtectedInternalProperty) { }
+        lock (InternalProperty) { }
+        lock (PublicProperty) { }
     }
 }
 

From 1ba0266dc864ac3c70efa5987f108d0b6a972acb Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Mon, 27 Feb 2023 13:43:23 +0100
Subject: [PATCH 20/24] Remove unused method

---
 .../Rules/LockedFieldShouldBeReadonly.cs                     | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
index 8c954411cc4..4f1f07a76fa 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
@@ -76,9 +76,4 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
         expression.IsAnyKind(SyntaxKind.IdentifierName, SyntaxKind.SimpleMemberAccessExpression) && lazySymbol.Value is IFieldSymbol lockedField && !lockedField.IsReadOnly
             ? lockedField
             : null;
-
-    private static ITypeSymbol FieldInSameTypeAs(IFieldSymbol field, INamedTypeSymbol type) =>
-        field.ContainingType is { } fieldType && type is { } && !fieldType.Equals(type)
-            ? type
-            : null;
 }

From 3a989378ff4bc97bbd1e7aea74f2cbb8a48cea53 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Mon, 27 Feb 2023 14:17:35 +0100
Subject: [PATCH 21/24] Update rspec

---
 analyzers/rspec/cs/S2445_c#.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/analyzers/rspec/cs/S2445_c#.json b/analyzers/rspec/cs/S2445_c#.json
index d5b4a8d6d3f..afe23a65374 100644
--- a/analyzers/rspec/cs/S2445_c#.json
+++ b/analyzers/rspec/cs/S2445_c#.json
@@ -1,5 +1,5 @@
 {
-  "title": "Blocks should be synchronized on \"readonly\" fields",
+  "title": "Blocks should be synchronized on read-only fields",
   "type": "BUG",
   "status": "ready",
   "remediation": {

From 0162b48ec166a5421d1996f553be3c0341276313 Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Mon, 27 Feb 2023 14:30:00 +0100
Subject: [PATCH 22/24] Update UTs after change of messages in rspec

---
 .../Rules/LockedFieldShouldBeReadonly.cs      | 10 ++++----
 .../TestCases/LockedFieldShouldBeReadonly.cs  | 24 +++++++++----------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
index 4f1f07a76fa..1eecffa3b58 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
@@ -25,7 +25,7 @@ public sealed class LockedFieldShouldBeReadonly : SonarDiagnosticAnalyzer
 {
     private const string DiagnosticId = "S2445";
 
-    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, "{0}");
+    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, "Do not lock on {0}, use a readonly field instead.");
 
     public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
 
@@ -37,22 +37,22 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
         var expression = ((LockStatementSyntax)context.Node).Expression?.RemoveParentheses();
         if (IsCreation(expression))
         {
-            ReportIssue("Locking on a new instance is a no-op, use a 'readonly' field instead.");
+            ReportIssue("a new instance because is a no-op");
         }
         else
         {
             var lazySymbol = new Lazy<ISymbol>(() => context.SemanticModel.GetSymbolInfo(expression).Symbol);
             if (IsOfTypeString(expression, lazySymbol))
             {
-                ReportIssue("Strings can be interned, and should not be used for locking. Use a 'readonly' field instead.");
+                ReportIssue("strings as they can be interned");
             }
             else if (expression is IdentifierNameSyntax && lazySymbol.Value is ILocalSymbol lockedSymbol)
             {
-                ReportIssue($"Do not lock on local variable '{lockedSymbol.Name}', use a 'readonly' field instead.");
+                ReportIssue($"local variable '{lockedSymbol.Name}'");
             }
             else if (FieldNotReadonly(expression, lazySymbol) is { } lockedField)
             {
-                ReportIssue($"Do not lock on non-'readonly' field '{lockedField.Name}', use a 'readonly' field instead.");
+                ReportIssue($"writable field '{lockedField.Name}'");
             }
         }
 
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
index e56c1163dd9..e7b99e929c1 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
@@ -19,37 +19,37 @@ class Test
     void OnAStaticField()
     {
         lock (staticReadonlyField) { }
-        lock (staticReadWriteField) { }                  // Noncompliant {{Do not lock on non-'readonly' field 'staticReadWriteField', use a 'readonly' field instead.}}
+        lock (staticReadWriteField) { }                  // Noncompliant {{Do not lock on writable field 'staticReadWriteField', use a readonly field instead.}}
         //    ^^^^^^^^^^^^^^^^^^^^
         lock (Test.staticReadonlyField) { }
-        lock (Test.staticReadWriteField) { }             // Noncompliant {{Do not lock on non-'readonly' field 'staticReadWriteField', use a 'readonly' field instead.}}
+        lock (Test.staticReadWriteField) { }             // Noncompliant {{Do not lock on writable field 'staticReadWriteField', use a readonly field instead.}}
         //    ^^^^^^^^^^^^^^^^^^^^^^^^^
         lock (AnotherClass.staticReadonlyField) { }
-        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{Do not lock on non-'readonly' field 'staticReadWriteField', use a 'readonly' field instead.}}
+        lock (AnotherClass.staticReadWriteField) { }     // Noncompliant {{Do not lock on writable field 'staticReadWriteField', use a readonly field instead.}}
         //    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     }
 
     void OnAFieldOfSameInstance()
     {
         lock (readonlyField) { }
-        lock (readonlyStringField) { }                   // Noncompliant {{Strings can be interned, and should not be used for locking. Use a 'readonly' field instead.}}
-        lock (readWriteField) { }                        // Noncompliant {{Do not lock on non-'readonly' field 'readWriteField', use a 'readonly' field instead.}}
+        lock (readonlyStringField) { }                   // Noncompliant {{Do not lock on strings as they can be interned, use a readonly field instead.}}
+        lock (readWriteField) { }                        // Noncompliant {{Do not lock on writable field 'readWriteField', use a readonly field instead.}}
         lock (this.readonlyField) { }
-        lock (this.readWriteField) { }                   // Noncompliant {{Do not lock on non-'readonly' field 'readWriteField', use a 'readonly' field instead.}}
+        lock (this.readWriteField) { }                   // Noncompliant {{Do not lock on writable field 'readWriteField', use a readonly field instead.}}
     }
 
     void OnParenthesizedExpressions()
     {
         lock ((readonlyField)) { }
-        lock ((readWriteField)) { }                      // Noncompliant {{Do not lock on non-'readonly' field 'readWriteField', use a 'readonly' field instead.}}
-        lock ((this.readWriteField)) { }                 // Noncompliant {{Do not lock on non-'readonly' field 'readWriteField', use a 'readonly' field instead.}}
+        lock ((readWriteField)) { }                      // Noncompliant {{Do not lock on writable field 'readWriteField', use a readonly field instead.}}
+        lock ((this.readWriteField)) { }                 // Noncompliant {{Do not lock on writable field 'readWriteField', use a readonly field instead.}}
     }
 
     void OnAFieldOfDifferentInstance()
     {
         var anotherInstance = new Test();
         lock (anotherInstance.readonlyField) { }
-        lock (anotherInstance.readWriteField) { }        // Noncompliant {{Do not lock on non-'readonly' field 'readWriteField', use a 'readonly' field instead.}}
+        lock (anotherInstance.readWriteField) { }        // Noncompliant {{Do not lock on writable field 'readWriteField', use a readonly field instead.}}
         lock (anotherInstance.readonlyField) { }
         lock (anotherInstance?.readWriteField) { }       // FN: ?. not supported
     }
@@ -57,7 +57,7 @@ void OnAFieldOfDifferentInstance()
     void OnALocalVariable()
     {
         object localVarNull = null;
-        lock (localVarNull) { }                          // Noncompliant {{Do not lock on local variable 'localVarNull', use a 'readonly' field instead.}}
+        lock (localVarNull) { }                          // Noncompliant {{Do not lock on local variable 'localVarNull', use a readonly field instead.}}
         object localVarReadonlyField = readonlyField;
         lock (localVarReadonlyField) { }                 // Noncompliant, while the local variable references a readonly field, the local variable itself can mutate
         object localVarReadWriteField = readWriteField;
@@ -66,7 +66,7 @@ void OnALocalVariable()
 
     void OnANewInstance()
     {
-        lock (new object()) { }                          // Noncompliant {{Locking on a new instance is a no-op, use a 'readonly' field instead.}}
+        lock (new object()) { }                          // Noncompliant {{Do not lock on a new instance because is a no-op, use a readonly field instead.}}
         lock (new ANamespace.AClass()) { }               // Noncompliant
         lock (new Test[] { }) { }                        // Noncompliant
         lock (new[] { readonlyField }) { }               // Noncompliant
@@ -86,7 +86,7 @@ void OnANewInstance()
 
     void OnAStringInstance()
     {
-        lock ("a string") { }                            // Noncompliant {{Strings can be interned, and should not be used for locking. Use a 'readonly' field instead.}}
+        lock ("a string") { }                            // Noncompliant {{Do not lock on strings as they can be interned, use a readonly field instead.}}
         lock ($"an interpolated {"string"}") { }         // Noncompliant
         lock ("a" + "string") { }                        // Noncompliant
         lock (MethodReturningString()) { }               // Noncompliant

From 829c8925d6774ecd580b919dd6dd99c55266fd6b Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Mon, 27 Feb 2023 17:37:22 +0100
Subject: [PATCH 23/24] Fix ITs after change of messages

---
 .../expected/Automapper/AutoMapper--net461-S2445.json    | 2 +-
 .../Automapper/AutoMapper--netstandard2.0-S2445.json     | 2 +-
 analyzers/its/expected/Nancy/Nancy--net452-S2445.json    | 2 +-
 .../its/expected/Nancy/Nancy--netstandard2.0-S2445.json  | 2 +-
 .../Rules/LockedFieldShouldBeReadonly.cs                 | 4 ++--
 .../TestCases/LockedFieldShouldBeReadonly.cs             | 9 +++++++++
 6 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json b/analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json
index 1dea7b12535..c9f27b7abe2 100644
--- a/analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json
+++ b/analyzers/its/expected/Automapper/AutoMapper--net461-S2445.json
@@ -2,7 +2,7 @@
 "issues":  [
 {
 "id":  "S2445",
-"message":  "'typeMap' is a local variable, and should not be used for locking.",
+"message":  "Do not lock on local variable 'typeMap', use a readonly field instead.",
 "location":  {
 "uri":  "sources\Automapper\src\AutoMapper\Configuration\MapperConfiguration.cs",
 "region":  {
diff --git a/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json b/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json
index 1dea7b12535..c9f27b7abe2 100644
--- a/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json
+++ b/analyzers/its/expected/Automapper/AutoMapper--netstandard2.0-S2445.json
@@ -2,7 +2,7 @@
 "issues":  [
 {
 "id":  "S2445",
-"message":  "'typeMap' is a local variable, and should not be used for locking.",
+"message":  "Do not lock on local variable 'typeMap', use a readonly field instead.",
 "location":  {
 "uri":  "sources\Automapper\src\AutoMapper\Configuration\MapperConfiguration.cs",
 "region":  {
diff --git a/analyzers/its/expected/Nancy/Nancy--net452-S2445.json b/analyzers/its/expected/Nancy/Nancy--net452-S2445.json
index 5f3cf072756..59c67779f40 100644
--- a/analyzers/its/expected/Nancy/Nancy--net452-S2445.json
+++ b/analyzers/its/expected/Nancy/Nancy--net452-S2445.json
@@ -2,7 +2,7 @@
 "issues":  [
 {
 "id":  "S2445",
-"message":  "'entitiesLock' is not 'private readonly', and should not be used for locking.",
+"message":  "Do not lock on writable field 'entitiesLock', use a readonly field instead.",
 "location":  {
 "uri":  "sources\Nancy\src\Nancy\Helpers\HttpEncoder.cs",
 "region":  {
diff --git a/analyzers/its/expected/Nancy/Nancy--netstandard2.0-S2445.json b/analyzers/its/expected/Nancy/Nancy--netstandard2.0-S2445.json
index 5f3cf072756..59c67779f40 100644
--- a/analyzers/its/expected/Nancy/Nancy--netstandard2.0-S2445.json
+++ b/analyzers/its/expected/Nancy/Nancy--netstandard2.0-S2445.json
@@ -2,7 +2,7 @@
 "issues":  [
 {
 "id":  "S2445",
-"message":  "'entitiesLock' is not 'private readonly', and should not be used for locking.",
+"message":  "Do not lock on writable field 'entitiesLock', use a readonly field instead.",
 "location":  {
 "uri":  "sources\Nancy\src\Nancy\Helpers\HttpEncoder.cs",
 "region":  {
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
index 1eecffa3b58..1839386a3c9 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
@@ -50,7 +50,7 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
             {
                 ReportIssue($"local variable '{lockedSymbol.Name}'");
             }
-            else if (FieldNotReadonly(expression, lazySymbol) is { } lockedField)
+            else if (FieldWritable(expression, lazySymbol) is { } lockedField)
             {
                 ReportIssue($"writable field '{lockedField.Name}'");
             }
@@ -72,7 +72,7 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
         expression.IsAnyKind(SyntaxKind.StringLiteralExpression, SyntaxKind.InterpolatedStringExpression)
         || lazySymbol.Value.GetSymbolType().Is(KnownType.System_String);
 
-    private static IFieldSymbol FieldNotReadonly(ExpressionSyntax expression, Lazy<ISymbol> lazySymbol) =>
+    private static IFieldSymbol FieldWritable(ExpressionSyntax expression, Lazy<ISymbol> lazySymbol) =>
         expression.IsAnyKind(SyntaxKind.IdentifierName, SyntaxKind.SimpleMemberAccessExpression) && lazySymbol.Value is IFieldSymbol lockedField && !lockedField.IsReadOnly
             ? lockedField
             : null;
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
index e7b99e929c1..c9bc02391f5 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Linq;
 
 class Test
@@ -64,6 +65,14 @@ void OnALocalVariable()
         lock (localVarReadWriteField) { }                // Noncompliant
     }
 
+    void OnALocalOutVar(Dictionary<int, object> lockObjs)
+    {
+        if (lockObjs.TryGetValue(42, out var lockObj))
+        {
+            lock (lockObj) { }                           // Noncompliant, FP: the lock object is a local variable retrieved from a collection of locks
+        }
+    }
+
     void OnANewInstance()
     {
         lock (new object()) { }                          // Noncompliant {{Do not lock on a new instance because is a no-op, use a readonly field instead.}}

From 52fa81ed809a31fee031641cb3ed199010f10b2e Mon Sep 17 00:00:00 2001
From: Antonio Aversa <antonio.aversa@sonarsource.com>
Date: Wed, 1 Mar 2023 10:33:57 +0100
Subject: [PATCH 24/24] Nitpicks

---
 .../Rules/LockedFieldShouldBeReadonly.cs                  | 8 ++++----
 .../TestCases/LockedFieldShouldBeReadonly.CSharp11.cs     | 6 ------
 .../TestCases/LockedFieldShouldBeReadonly.CSharp8.cs      | 1 -
 .../TestCases/LockedFieldShouldBeReadonly.cs              | 5 ++++-
 4 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
index 1839386a3c9..104b9592a54 100644
--- a/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
+++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/LockedFieldShouldBeReadonly.cs
@@ -46,13 +46,13 @@ private static void CheckLockStatement(SonarSyntaxNodeReportingContext context)
             {
                 ReportIssue("strings as they can be interned");
             }
-            else if (expression is IdentifierNameSyntax && lazySymbol.Value is ILocalSymbol lockedSymbol)
+            else if (expression is IdentifierNameSyntax && lazySymbol.Value is ILocalSymbol localSymbol)
             {
-                ReportIssue($"local variable '{lockedSymbol.Name}'");
+                ReportIssue($"local variable '{localSymbol.Name}'");
             }
-            else if (FieldWritable(expression, lazySymbol) is { } lockedField)
+            else if (FieldWritable(expression, lazySymbol) is { } field)
             {
-                ReportIssue($"writable field '{lockedField.Name}'");
+                ReportIssue($"writable field '{field.Name}'");
             }
         }
 
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp11.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp11.cs
index 395faf70089..fb473e8e122 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp11.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp11.cs
@@ -2,12 +2,6 @@
 
 class Test
 {
-    static readonly object staticReadonlyField = null;
-    static object staticReadWriteField = null;
-
-    readonly object readonlyField = null;
-    object readWriteField = null;
-
     void OnANewInstance()
     {
         lock ("""a raw string literal""") { }                    // Noncompliant
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp8.cs
index 22b720a2c00..21f4b14b44b 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp8.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.CSharp8.cs
@@ -1,7 +1,6 @@
 class Test
 {
     static readonly object staticReadonlyField = null;
-    static object staticReadWriteField = null;
 
     readonly object readonlyField = null;
     object readWriteField = null;
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
index c9bc02391f5..18dbd529ef6 100644
--- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
+++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/LockedFieldShouldBeReadonly.cs
@@ -191,14 +191,17 @@ class FieldAccessibily
     internal object InternalProperty => null;
     public object PublicProperty => null;
 
-    void Test()
+    void FieldAccessibilityDoesntMatter()
     {
         lock (privateField) { }
         lock (protectedField) { }
         lock (protectedInternalField) { }
         lock (internalField) { }
         lock (publicField) { }
+    }
 
+    void RuleDoesntRaiseOnProperties()
+    {
         lock (PrivateProperty) { }
         lock (ProtectedProperty) { }
         lock (ProtectedInternalProperty) { }