From 829c8925d6774ecd580b919dd6dd99c55266fd6b Mon Sep 17 00:00:00 2001 From: Antonio Aversa Date: Mon, 27 Feb 2023 17:37:22 +0100 Subject: [PATCH] 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 lazySymbol) => + private static IFieldSymbol FieldWritable(ExpressionSyntax expression, Lazy 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 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.}}