Skip to content

Commit

Permalink
New rule: MA0158 - Use System.Threading.Lock instead of System.Object
Browse files Browse the repository at this point in the history
  • Loading branch information
meziantou committed May 23, 2024
1 parent ed6845a commit ef8775e
Show file tree
Hide file tree
Showing 14 changed files with 456 additions and 17 deletions.
12 changes: 6 additions & 6 deletions Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@

<Otherwise>
<ItemGroup>
<PackageReference Update="Microsoft.CodeAnalysis.Analyzers" Version="3.11.0-beta1.24122.1" />
<PackageReference Update="Microsoft.CodeAnalysis.CSharp" Version="4.9.2" />
<PackageReference Update="Microsoft.CodeAnalysis.Workspaces.Common" Version="4.9.2" />
<PackageReference Update="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.9.2" />
<PackageReference Update="Microsoft.CodeAnalysis.Analyzers" Version="3.11.0-beta1.24219.2" />
<PackageReference Update="Microsoft.CodeAnalysis.CSharp" Version="4.10.0-3.final" />
<PackageReference Update="Microsoft.CodeAnalysis.Workspaces.Common" Version="4.10.0-3.final" />
<PackageReference Update="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.10.0-3.final" />
</ItemGroup>
<PropertyGroup>
<DefineConstants>$(DefineConstants);ROSLYN4_8;ROSLYN_4_2_OR_GREATER;ROSLYN_4_4_OR_GREATER;ROSLYN_4_5_OR_GREATER;ROSLYN_4_6_OR_GREATER;ROSLYN_4_8_OR_GREATER</DefineConstants>
<DefineConstants>$(DefineConstants);CSHARP9_OR_GREATER;CSHARP10_OR_GREATER;CSHARP11_OR_GREATER;CSHARP12_OR_GREATER</DefineConstants>
<DefineConstants>$(DefineConstants);ROSLYN4_8;ROSLYN_4_2_OR_GREATER;ROSLYN_4_4_OR_GREATER;ROSLYN_4_5_OR_GREATER;ROSLYN_4_6_OR_GREATER;ROSLYN_4_8_OR_GREATER;ROSLYN_4_10_OR_GREATER</DefineConstants>
<DefineConstants>$(DefineConstants);CSHARP9_OR_GREATER;CSHARP10_OR_GREATER;CSHARP11_OR_GREATER;CSHARP12_OR_GREATER;CSHARP13_OR_GREATER</DefineConstants>
<NoWarn>$(NoWarn);CS0618</NoWarn>
</PropertyGroup>
</Otherwise>
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ If you are already using other analyzers, you can check [which rules are duplica
|[MA0155](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0155.md)|Design|Do not use async void methods|⚠️|||
|[MA0156](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0156.md)|Design|Use 'Async' suffix when a method returns IAsyncEnumerable\<T\>|⚠️|||
|[MA0157](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0157.md)|Design|Do not use 'Async' suffix when a method does not return IAsyncEnumerable\<T\>|⚠️|||
|[MA0158](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0158.md)|Performance|Use System.Threading.Lock|⚠️|✔️||

<!-- rules -->

Expand Down
7 changes: 7 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@
|[MA0155](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0155.md)|Design|Do not use async void methods|<span title='Warning'>⚠️</span>|||
|[MA0156](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0156.md)|Design|Use 'Async' suffix when a method returns IAsyncEnumerable\<T\>|<span title='Warning'>⚠️</span>|||
|[MA0157](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0157.md)|Design|Do not use 'Async' suffix when a method does not return IAsyncEnumerable\<T\>|<span title='Warning'>⚠️</span>|||
|[MA0158](https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0158.md)|Performance|Use System.Threading.Lock|<span title='Warning'>⚠️</span>|✔️||

|Id|Suppressed rule|Justification|
|--|---------------|-------------|
Expand Down Expand Up @@ -634,6 +635,9 @@ dotnet_diagnostic.MA0156.severity = none
# MA0157: Do not use 'Async' suffix when a method does not return IAsyncEnumerable<T>
dotnet_diagnostic.MA0157.severity = none
# MA0158: Use System.Threading.Lock
dotnet_diagnostic.MA0158.severity = warning
```

# .editorconfig - all rules disabled
Expand Down Expand Up @@ -1106,4 +1110,7 @@ dotnet_diagnostic.MA0156.severity = none
# MA0157: Do not use 'Async' suffix when a method does not return IAsyncEnumerable<T>
dotnet_diagnostic.MA0157.severity = none
# MA0158: Use System.Threading.Lock
dotnet_diagnostic.MA0158.severity = none
```
35 changes: 35 additions & 0 deletions docs/Rules/MA0158.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# MA0158 - Use System.Threading.Lock

Starting with .NET 9 and C# 13, you can use `System.Threading.Lock`. When a field or a local variable is only used inside `lock`, this rule will suggest using `System.Threading.Lock` instead of `object`.

````c#
using System.Threading;

class MyClass
{
private object _lock = new(); // non-compliant
public void MyMethod()
{
using (_lock)
{
}
}
}
````

````c#
using System.Threading;

class MyClass
{
private Lock _lock = new(); // ok
public void MyMethod()
{
using (_lock)
{
}
}
}
````
5 changes: 5 additions & 0 deletions src/Meziantou.Analyzer/Internals/LanguageVersionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ public static bool IsCSharp11OrAbove(this LanguageVersion languageVersion)
return languageVersion >= (LanguageVersion)1100;
}

public static bool IsCSharp13OrAbove(this LanguageVersion languageVersion)
{
return languageVersion >= (LanguageVersion)1300;
}

public static bool IsCSharp10OrBellow(this LanguageVersion languageVersion)
{
return languageVersion <= (LanguageVersion)1000;
Expand Down
7 changes: 1 addition & 6 deletions src/Meziantou.Analyzer/Internals/SymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,7 @@ public static bool IsVisibleOutsideOfAssembly([NotNullWhen(true)] this ISymbol?

public static bool IsOperator(this ISymbol? symbol)
{
if (symbol is IMethodSymbol methodSymbol)
{
return methodSymbol.MethodKind == MethodKind.UserDefinedOperator || methodSymbol.MethodKind == MethodKind.Conversion;
}

return false;
return symbol is IMethodSymbol { MethodKind: MethodKind.UserDefinedOperator or MethodKind.Conversion };
}

public static bool IsOverrideOrInterfaceImplementation(this ISymbol? symbol)
Expand Down
1 change: 1 addition & 0 deletions src/Meziantou.Analyzer/RuleIdentifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ internal static class RuleIdentifiers
public const string DoNotUseAsyncVoid = "MA0155";
public const string MethodsReturningIAsyncEnumerableMustHaveTheAsyncSuffix = "MA0156";
public const string MethodsNotReturningIAsyncEnumerableMustNotHaveTheAsyncSuffix = "MA0157";
public const string UseSystemThreadingLockInsteadOfObject = "MA0158";

public static string GetHelpUri(string identifier)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private static void AnalyzeSymbol(SymbolAnalysisContext context)

foreach (var excludedSymbolName in excludedSymbolNamesSplit)
{
if (IsWildcardMatch(symbolName, excludedSymbolName) || IsWildcardMatch(symbolDeclarationId, excludedSymbolName))
if (IsWildcardMatch(symbolName, excludedSymbolName) || (symbolDeclarationId is not null && IsWildcardMatch(symbolDeclarationId, excludedSymbolName)))
matched = true;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Meziantou.Analyzer/Rules/NamedParameterAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ bool IsParams(SyntaxNode node)
if (syntaxContext.Options.TryGetConfigurationValue(expression.SyntaxTree, RuleIdentifiers.UseNamedParameter + ".excluded_methods_regex", out var excludedMethodsRegex))
{
var declarationId = DocumentationCommentId.CreateDeclarationId(invokedMethodSymbol);
if (Regex.IsMatch(declarationId, excludedMethodsRegex, RegexOptions.None, Timeout.InfiniteTimeSpan))
if (declarationId is not null && Regex.IsMatch(declarationId, excludedMethodsRegex, RegexOptions.None, Timeout.InfiniteTimeSpan))
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ public NodeQueuePoolPolicy()
}
}

public Queue<SyntaxNode> Create() => new Queue<SyntaxNode>(capacity: 30);
public Queue<SyntaxNode> Create() => new(capacity: 30);

public bool Return(Queue<SyntaxNode> obj)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Meziantou.Analyzer.Internals;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Meziantou.Analyzer.Rules;

// https://github.com/dotnet/runtime/issues/34812
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class UseSystemThreadingLockInsteadOfObjectAnalyzer : DiagnosticAnalyzer
{
private static readonly DiagnosticDescriptor Rule = new(
RuleIdentifiers.UseSystemThreadingLockInsteadOfObject,
title: "Use System.Threading.Lock",
messageFormat: "Use System.Threading.Lock",
RuleCategories.Performance,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: "",
helpLinkUri: RuleIdentifiers.GetHelpUri(RuleIdentifiers.UseSystemThreadingLockInsteadOfObject));

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);

context.RegisterCompilationStartAction(context =>
{
if (context.Compilation.GetBestTypeByMetadataName("System.Threading.Lock") is null)
return;
if (!context.Compilation.GetCSharpLanguageVersion().IsCSharp13OrAbove())
return;
context.RegisterOperationBlockStartAction(context =>
{
foreach (var block in context.OperationBlocks)
{
if (block.Syntax is StatementSyntax or ExpressionSyntax)
{
var symbols = new SymbolLockContext();
context.RegisterOperationAction(context => symbols.HandleOperation((ILocalReferenceOperation)context.Operation), OperationKind.LocalReference);
context.RegisterOperationBlockEndAction(context => symbols.ReportSymbols(context, Rule));
}
}
});
var symbols = new SymbolLockContext();
context.RegisterOperationAction(context => symbols.HandleOperation((IFieldReferenceOperation)context.Operation), OperationKind.FieldReference);
context.RegisterCompilationEndAction(context => symbols.ReportSymbols(context, Rule));
});
}

private sealed class SymbolLockContext
{
private readonly ConcurrentDictionary<ISymbol, bool> _symbols = new(SymbolEqualityComparer.Default);

public void ReportSymbols(DiagnosticReporter reporter, DiagnosticDescriptor descriptor)
{
foreach (var symbol in _symbols)
{
if (symbol.Value)
{
reporter.ReportDiagnostic(descriptor, symbol.Key);
}
}
}

private static bool IsPotentialSymbol(ISymbol symbol)
{
if (symbol is IFieldSymbol { Type.SpecialType: SpecialType.System_Object } && !symbol.IsVisibleOutsideOfAssembly())
return true;

if (symbol is ILocalSymbol { Type.SpecialType: SpecialType.System_Object })
return true;

return false;
}

public void HandleOperation(ILocalReferenceOperation operation)
{
var symbol = operation.Local;
HandleOperation(symbol, operation);
}

public void HandleOperation(IFieldReferenceOperation operation)
{
var symbol = operation.Field;
HandleOperation(symbol, operation);
}

private void HandleOperation(ISymbol symbol, IOperation operation)
{
if (!IsPotentialSymbol(symbol))
return;

if (operation.Parent is not ILockOperation)
{
ExcludeSymbol(symbol);
}
else
{
AddPotentialSymbol(symbol);
}
}

public void ExcludeSymbol(ISymbol symbol)
{
_ = _symbols.AddOrUpdate(symbol, addValue: false, (_, _) => false);
}

public void AddPotentialSymbol(ISymbol symbol)
{
_ = _symbols.TryAdd(symbol, value: true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ private Task<Project> CreateProject()
AddNuGetReference("Microsoft.NETCore.App.Ref", "8.0.0", "ref/net8.0/");
break;

case TargetFramework.Net9_0:
AddNuGetReference("Microsoft.NETCore.App.Ref", "9.0.0-preview.4.24266.19", "ref/net9.0/");
break;

case TargetFramework.AspNetCore5_0:
AddNuGetReference("Microsoft.NETCore.App.Ref", "5.0.0", "ref/net5.0/");
AddNuGetReference("Microsoft.AspNetCore.App.Ref", "5.0.0", "ref/net5.0/");
Expand Down Expand Up @@ -217,7 +221,7 @@ private Task<Project> CreateProject()

AddNuGetReference("System.Collections.Immutable", "1.5.0", "lib/netstandard2.0/");

if (TargetFramework is not TargetFramework.Net7_0 and not TargetFramework.Net8_0)
if (TargetFramework is not TargetFramework.Net7_0 and not TargetFramework.Net8_0 and not TargetFramework.Net9_0)
{
AddNuGetReference("System.Numerics.Vectors", "4.5.0", "ref/netstandard2.0/");
}
Expand Down
3 changes: 2 additions & 1 deletion tests/Meziantou.Analyzer.Test/Helpers/TargetFramework.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ public enum TargetFramework
Net6_0,
Net7_0,
Net8_0,
NetLatest = Net8_0,
Net9_0,
NetLatest = Net9_0,
AspNetCore5_0,
AspNetCore6_0,
AspNetCore7_0,
Expand Down
Loading

0 comments on commit ef8775e

Please sign in to comment.