From e96bffc14b1ae903ce3b7e1b52240ee9fc70628b Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sat, 15 Aug 2026 00:57:14 +0200 Subject: [PATCH 1/7] refactor: replace [ThreadStatic] walker cache with a shared object pool The hand-rolled GetInstance/Free pattern was repeated on 23 types, and its reuse safety rested entirely on Free manually nulling every field, guarded only by Debug.Assert. Four call sites had no try/finally at all, and none of the collection-owning walkers capped the buffer they retained per thread. Pooling is kept only where the pooled object owns a growable buffer. Those eight types now implement IResettable and rent through ObjectPool, whose PooledObject makes the release unconditional via using, and whose CanBeCached check drops an instance whose collection outgrew ObjectPool.MaxCachedBufferSize instead of pinning it for the life of the thread. The remaining 15 walkers take their state through constructors, so they can no longer arrive dirty. This also fixes RCS1187, which was reporting inconsistently: a fresh UseConstantInsteadOfFieldWalker started with CanBeConvertedToConstant set to false, suppressing the diagnostic, while a recycled one started with the value Free had reset it to and reported normally. Co-authored-by: Cursor --- ChangeLog.md | 1 + .../Analysis/InlineLocalVariableAnalyzer.cs | 96 ++++--------- .../Analysis/MakeClassStaticAnalyzer.cs | 69 +++------ .../MakeMemberReadOnlyAnalyzer.cs | 17 +-- .../MakeMemberReadOnlyWalker.cs | 37 ++--- .../MarkLocalVariableAsConstAnalyzer.cs | 37 ++--- .../MarkLocalVariableAsConstWalker.cs | 57 +++----- .../Analysis/OptimizeMethodCallAnalysis.cs | 8 +- .../Analysis/RefReadOnlyParameterAnalyzer.cs | 131 ++++++++---------- ...inalExceptionFromThrowStatementAnalyzer.cs | 70 +++------- .../RemoveRedundantAssignmentAnalyzer.cs | 89 +++--------- ...eturnCompletedTaskInsteadOfNullAnalyzer.cs | 31 +---- ...ecessaryExplicitUseOfEnumeratorAnalyzer.cs | 21 +-- .../UnnecessaryUsageOfEnumeratorWalker.cs | 48 ++----- .../UnusedMember/UnusedMemberAnalyzer.cs | 85 ++++-------- .../UnusedMember/UnusedMemberWalker.cs | 34 +---- .../UnusedParameterAnalyzer.cs | 45 ++---- .../UnusedParameter/UnusedParameterWalker.cs | 54 +++----- .../CSharp/Analysis/UseAsyncAwaitAnalyzer.cs | 66 ++++----- .../Analysis/UseAutoPropertyAnalyzer.cs | 108 ++++----------- .../Analysis/UseExceptionFilterAnalyzer.cs | 64 ++------- ...tatementInsteadOfWhileStatementAnalyzer.cs | 52 +------ ...tternMatchingInsteadOfIsAndCastAnalyzer.cs | 21 +-- .../UsePatternMatchingWalker.cs | 59 ++------ .../ValidateArgumentsCorrectlyAnalyzer.cs | 4 +- .../SyntaxWalkers/ContainsCommentWalker.cs | 35 +---- ...ContainsLocalOrParameterReferenceWalker.cs | 49 +------ .../SyntaxWalkers/ContainsYieldWalker.cs | 41 +----- .../MethodReferencedAsMethodGroupWalker.cs | 73 +++------- .../CSharp/UsePatternMatchingAnalyzer.cs | 33 ++--- .../Analysis/RemoveAsyncAwaitAnalysis.cs | 45 +++--- ...oveRedundantYieldBreakStatementAnalysis.cs | 14 +- .../UseConstantInsteadOfFieldAnalysis.cs | 70 +++------- .../SyntaxWalkers/AwaitExpressionWalker.cs | 42 ++---- src/Core/IResettable.cs | 14 ++ src/Core/ObjectPool.cs | 13 ++ src/Core/ObjectPool`1.cs | 31 +++++ src/Core/PooledObject`1.cs | 20 +++ .../CSharp/UseSpacesInsteadOfTabAnalyzer.cs | 38 +---- .../ConvertWhileToForRefactoring.cs | 37 ++--- .../CSharp/Refactorings/RefactoringContext.cs | 36 +---- .../RCS1187UseConstantInsteadOfFieldTests.cs | 26 ++++ 42 files changed, 580 insertions(+), 1341 deletions(-) create mode 100644 src/Core/IResettable.cs create mode 100644 src/Core/ObjectPool.cs create mode 100644 src/Core/ObjectPool`1.cs create mode 100644 src/Core/PooledObject`1.cs diff --git a/ChangeLog.md b/ChangeLog.md index 905732712e..bd450769dd 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fix analyzer [RCS1187](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1187) to report a field consistently when the containing type has a static constructor that does not assign the field - Fix analyzer [RCS1060](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1060) to not report a file that contains only multiple partial declarations of the same type ([PR](https://github.com/dotnet/roslynator/pull/1798)) - Fix analyzer [RCS1231](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1231) to not suggest `in` for `ref struct` parameters ([#1725](https://github.com/dotnet/roslynator/issues/1725)) ([PR](https://github.com/dotnet/roslynator/pull/1807)) - Fix analyzer [RCS1260](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1260) false positive for `omit_when_single_line` on multi-line object/collection initializers ([#1439](https://github.com/dotnet/roslynator/issues/1439)) ([PR](https://github.com/dotnet/roslynator/pull/1808)) diff --git a/src/Analyzers/CSharp/Analysis/InlineLocalVariableAnalyzer.cs b/src/Analyzers/CSharp/Analysis/InlineLocalVariableAnalyzer.cs index dbbe30558d..49728474e4 100644 --- a/src/Analyzers/CSharp/Analysis/InlineLocalVariableAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/InlineLocalVariableAnalyzer.cs @@ -154,29 +154,19 @@ private static void AnalyzeLocalDeclarationStatement(SyntaxNodeAnalysisContext c if (localSymbol?.IsErrorType() != false) return; - ContainsLocalOrParameterReferenceWalker walker = null; + var walker = new ContainsLocalOrParameterReferenceWalker(localSymbol, context.SemanticModel, context.CancellationToken); - try - { - walker = ContainsLocalOrParameterReferenceWalker.GetInstance(localSymbol, context.SemanticModel, context.CancellationToken); - - walker.Visit(forEachStatement.Statement); + walker.Visit(forEachStatement.Statement); - if (!walker.Result - && index < statements.Count - 2) - { - walker.VisitList(statements, index + 2); - } - - if (!walker.Result) - ReportDiagnostic(context, localDeclarationInfo, forEachStatement.Expression); - } - finally + if (!walker.Result + && index < statements.Count - 2) { - if (walker is not null) - ContainsLocalOrParameterReferenceWalker.Free(walker); + walker.VisitList(statements, index + 2); } + if (!walker.Result) + ReportDiagnostic(context, localDeclarationInfo, forEachStatement.Expression); + break; } case SyntaxKind.SwitchStatement: @@ -194,29 +184,19 @@ private static void AnalyzeLocalDeclarationStatement(SyntaxNodeAnalysisContext c if (localSymbol?.IsErrorType() != false) return; - ContainsLocalOrParameterReferenceWalker walker = null; + var walker = new ContainsLocalOrParameterReferenceWalker(localSymbol, context.SemanticModel, context.CancellationToken); - try - { - walker = ContainsLocalOrParameterReferenceWalker.GetInstance(localSymbol, context.SemanticModel, context.CancellationToken); - - walker.VisitList(switchStatement.Sections); + walker.VisitList(switchStatement.Sections); - if (!walker.Result - && index < statements.Count - 2) - { - walker.VisitList(statements, index + 2); - } - - if (!walker.Result) - ReportDiagnostic(context, localDeclarationInfo, switchStatement.Expression); - } - finally + if (!walker.Result + && index < statements.Count - 2) { - if (walker is not null) - ContainsLocalOrParameterReferenceWalker.Free(walker); + walker.VisitList(statements, index + 2); } + if (!walker.Result) + ReportDiagnostic(context, localDeclarationInfo, switchStatement.Expression); + break; } } @@ -250,28 +230,18 @@ private static void Analyze( if (localSymbol?.IsErrorType() != false) return; - ContainsLocalOrParameterReferenceWalker walker = null; + var walker = new ContainsLocalOrParameterReferenceWalker(localSymbol, context.SemanticModel, context.CancellationToken); - try - { - walker = ContainsLocalOrParameterReferenceWalker.GetInstance(localSymbol, context.SemanticModel, context.CancellationToken); - - walker.Visit(assignment.Left); + walker.Visit(assignment.Left); - if (!walker.Result - && index < statements.Count - 2) - { - walker.VisitList(statements, index + 2); - } - - if (!walker.Result) - ReportDiagnostic(context, localDeclarationInfo, identifierName); - } - finally + if (!walker.Result + && index < statements.Count - 2) { - if (walker is not null) - ContainsLocalOrParameterReferenceWalker.Free(walker); + walker.VisitList(statements, index + 2); } + + if (!walker.Result) + ReportDiagnostic(context, localDeclarationInfo, identifierName); } private static void Analyze( @@ -306,22 +276,12 @@ private static void Analyze( if (index < statements.Count - 2) { - ContainsLocalOrParameterReferenceWalker walker = null; + var walker = new ContainsLocalOrParameterReferenceWalker(localSymbol, context.SemanticModel, context.CancellationToken); - try - { - walker = ContainsLocalOrParameterReferenceWalker.GetInstance(localSymbol, context.SemanticModel, context.CancellationToken); - - walker.VisitList(statements, index + 2); + walker.VisitList(statements, index + 2); - if (walker.Result) - return; - } - finally - { - if (walker is not null) - ContainsLocalOrParameterReferenceWalker.Free(walker); - } + if (walker.Result) + return; } ReportDiagnostic(context, localDeclarationInfo, identifierName); diff --git a/src/Analyzers/CSharp/Analysis/MakeClassStaticAnalyzer.cs b/src/Analyzers/CSharp/Analysis/MakeClassStaticAnalyzer.cs index e1d4f5921b..e1eda92139 100644 --- a/src/Analyzers/CSharp/Analysis/MakeClassStaticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/MakeClassStaticAnalyzer.cs @@ -68,29 +68,11 @@ private static void AnalyzeClassDeclaration(SyntaxNodeAnalysisContext context) if (!AnalyzeMembers(members)) return; - bool canBeMadeStatic; - MakeClassStaticWalker walker = null; + var walker = new MakeClassStaticWalker(symbol, context.SemanticModel, context.CancellationToken); - try - { - walker = MakeClassStaticWalker.GetInstance(); - - walker.CanBeMadeStatic = true; - walker.Symbol = symbol; - walker.SemanticModel = context.SemanticModel; - walker.CancellationToken = context.CancellationToken; - - walker.Visit(classDeclaration); + walker.Visit(classDeclaration); - canBeMadeStatic = walker.CanBeMadeStatic; - } - finally - { - if (walker is not null) - MakeClassStaticWalker.Free(walker); - } - - if (canBeMadeStatic) + if (walker.CanBeMadeStatic) DiagnosticHelpers.ReportDiagnostic(context, DiagnosticRules.MakeClassStatic, classDeclaration.Identifier); } @@ -173,16 +155,23 @@ public static bool AnalyzeMembers(ImmutableArray members) private class MakeClassStaticWalker : TypeSyntaxWalker { - [ThreadStatic] - private static MakeClassStaticWalker _cachedInstance; + public MakeClassStaticWalker( + INamedTypeSymbol symbol, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + Symbol = symbol; + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } - public bool CanBeMadeStatic { get; set; } + public bool CanBeMadeStatic { get; private set; } = true; - public INamedTypeSymbol Symbol { get; set; } + public INamedTypeSymbol Symbol { get; } - public SemanticModel SemanticModel { get; set; } + public SemanticModel SemanticModel { get; } - public CancellationToken CancellationToken { get; set; } + public CancellationToken CancellationToken { get; } protected override bool ShouldVisit => CanBeMadeStatic; @@ -210,31 +199,5 @@ protected override void VisitType(TypeSyntax node) } } } - - public static MakeClassStaticWalker GetInstance() - { - MakeClassStaticWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.Symbol is null); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new MakeClassStaticWalker(); - } - - public static void Free(MakeClassStaticWalker walker) - { - walker.Symbol = null; - walker.SemanticModel = null; - walker.CancellationToken = default; - - _cachedInstance = walker; - } } } diff --git a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyAnalyzer.cs b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyAnalyzer.cs index bc574183f6..d5a22765e1 100644 --- a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyAnalyzer.cs @@ -58,22 +58,13 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) if (typeDeclaration.Modifiers.Contains(SyntaxKind.PartialKeyword)) return; - MakeMemberReadOnlyWalker walker = null; + using PooledObject pooledWalker = ObjectPool.Rent(); - try - { - walker = MakeMemberReadOnlyWalker.GetInstance(); + MakeMemberReadOnlyWalker walker = pooledWalker.Value; - walker.SemanticModel = context.SemanticModel; - walker.CancellationToken = context.CancellationToken; + walker.Initialize(context.SemanticModel, context.CancellationToken); - AnalyzeTypeDeclaration(context, typeDeclaration, walker); - } - finally - { - if (walker is not null) - MakeMemberReadOnlyWalker.Free(walker); - } + AnalyzeTypeDeclaration(context, typeDeclaration, walker); } private static void AnalyzeTypeDeclaration( diff --git a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs index 9ae1e736a2..52429a5936 100644 --- a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs +++ b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs @@ -11,48 +11,33 @@ namespace Roslynator.CSharp.Analysis.MakeMemberReadOnly; -internal class MakeMemberReadOnlyWalker : AssignedExpressionWalker +internal class MakeMemberReadOnlyWalker : AssignedExpressionWalker, IResettable { private int _classOrStructDepth; private int _localFunctionDepth; private int _anonymousFunctionDepth; private bool _isInInstanceConstructor; private bool _isInStaticConstructor; + private bool _canBeCached = true; - [ThreadStatic] - private static MakeMemberReadOnlyWalker _cachedInstance; + public SemanticModel SemanticModel { get; private set; } - public SemanticModel SemanticModel { get; set; } - - public CancellationToken CancellationToken { get; set; } + public CancellationToken CancellationToken { get; private set; } public Dictionary Symbols { get; } = []; - public static MakeMemberReadOnlyWalker GetInstance() - { - MakeMemberReadOnlyWalker walker = _cachedInstance; + public bool CanBeCached => _canBeCached; - if (walker is not null) - { - Debug.Assert(walker.Symbols.Count == 0); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new MakeMemberReadOnlyWalker(); - } - - public static void Free(MakeMemberReadOnlyWalker walker) + public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) { - walker.Reset(); - _cachedInstance = walker; + SemanticModel = semanticModel; + CancellationToken = cancellationToken; } - private void Reset() + public void Reset() { + _canBeCached = Symbols.Count <= ObjectPool.MaxCachedBufferSize; + Symbols.Clear(); SemanticModel = null; CancellationToken = default; diff --git a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs index 865a7b0afe..2eea96803e 100644 --- a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs @@ -100,35 +100,26 @@ private static bool CanBeMarkedAsConst( SyntaxList statements, int startIndex) { - MarkLocalVariableAsConstWalker walker = null; + using PooledObject pooledWalker = ObjectPool.Rent(); - try - { - walker = MarkLocalVariableAsConstWalker.GetInstance(); - - walker.SemanticModel = context.SemanticModel; - walker.CancellationToken = context.CancellationToken; + MarkLocalVariableAsConstWalker walker = pooledWalker.Value; - foreach (VariableDeclaratorSyntax variable in variables) - { - var symbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken) as ILocalSymbol; - - if (symbol is not null) - walker.Identifiers[variable.Identifier.ValueText] = symbol; - } + walker.Initialize(context.SemanticModel, context.CancellationToken); - for (int i = startIndex; i < statements.Count; i++) - { - walker.Visit(statements[i]); + foreach (VariableDeclaratorSyntax variable in variables) + { + var symbol = context.SemanticModel.GetDeclaredSymbol(variable, context.CancellationToken) as ILocalSymbol; - if (walker.Result) - return false; - } + if (symbol is not null) + walker.Identifiers[variable.Identifier.ValueText] = symbol; } - finally + + for (int i = startIndex; i < statements.Count; i++) { - if (walker is not null) - MarkLocalVariableAsConstWalker.Free(walker); + walker.Visit(statements[i]); + + if (walker.Result) + return false; } return true; diff --git a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs index 8ef6f53e6d..eed6f591d8 100644 --- a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs +++ b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Threading; using Microsoft.CodeAnalysis; @@ -12,21 +10,38 @@ namespace Roslynator.CSharp.Analysis.MarkLocalVariableAsConst; -internal class MarkLocalVariableAsConstWalker : AssignedExpressionWalker +internal class MarkLocalVariableAsConstWalker : AssignedExpressionWalker, IResettable { - [ThreadStatic] - private static MarkLocalVariableAsConstWalker _cachedInstance; + private bool _canBeCached = true; public Dictionary Identifiers { get; } = []; - public SemanticModel SemanticModel { get; set; } + public SemanticModel SemanticModel { get; private set; } - public CancellationToken CancellationToken { get; set; } + public CancellationToken CancellationToken { get; private set; } - public bool Result { get; set; } + public bool Result { get; private set; } protected override bool ShouldVisit => !Result; + public bool CanBeCached => _canBeCached; + + public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) + { + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } + + public void Reset() + { + _canBeCached = Identifiers.Count <= ObjectPool.MaxCachedBufferSize; + + Identifiers.Clear(); + SemanticModel = null; + CancellationToken = default; + Result = false; + } + public override void VisitAssignedExpression(ExpressionSyntax expression) { if (IsLocalReference(expression)) @@ -82,30 +97,4 @@ private bool IsLocalReference(IdentifierNameSyntax identifierName) return Identifiers.TryGetValue(identifierName.Identifier.ValueText, out ILocalSymbol symbol) && SymbolEqualityComparer.Default.Equals(symbol, SemanticModel.GetSymbol(identifierName, CancellationToken)); } - - public static MarkLocalVariableAsConstWalker GetInstance() - { - MarkLocalVariableAsConstWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.Identifiers.Count == 0); - Debug.Assert(!walker.Result); - - _cachedInstance = null; - return walker; - } - - return new MarkLocalVariableAsConstWalker(); - } - - public static void Free(MarkLocalVariableAsConstWalker walker) - { - walker.Identifiers.Clear(); - walker.SemanticModel = null; - walker.CancellationToken = default; - walker.Result = false; - - _cachedInstance = walker; - } } diff --git a/src/Analyzers/CSharp/Analysis/OptimizeMethodCallAnalysis.cs b/src/Analyzers/CSharp/Analysis/OptimizeMethodCallAnalysis.cs index d06733fe05..54f67517d6 100644 --- a/src/Analyzers/CSharp/Analysis/OptimizeMethodCallAnalysis.cs +++ b/src/Analyzers/CSharp/Analysis/OptimizeMethodCallAnalysis.cs @@ -365,15 +365,11 @@ public static void OptimizeAdd(SyntaxNodeAnalysisContext context, in SimpleMembe if (forEachVariableSymbol is not null) { - ContainsLocalOrParameterReferenceWalker walker = ContainsLocalOrParameterReferenceWalker.GetInstance(forEachVariableSymbol, semanticModel, cancellationToken); + var walker = new ContainsLocalOrParameterReferenceWalker(forEachVariableSymbol, semanticModel, cancellationToken); walker.Visit(invocationInfo.Expression); - bool containsReference = walker.Result; - - ContainsLocalOrParameterReferenceWalker.Free(walker); - - if (!containsReference) + if (!walker.Result) { DiagnosticHelpers.ReportDiagnostic( context, diff --git a/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs index 2587f62fcd..e242cb0195 100644 --- a/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs @@ -113,7 +113,11 @@ private static void Analyze( var methodSymbol = (IMethodSymbol)semanticModel.GetDeclaredSymbol(declaration, cancellationToken); - RefReadOnlyParameterWalker walker = null; + using PooledObject pooledWalker = ObjectPool.Rent(); + + RefReadOnlyParameterWalker walker = pooledWalker.Value; + + var isFirstCandidate = true; foreach (IParameterSymbol parameter in methodSymbol.Parameters) { @@ -157,12 +161,12 @@ private static void Analyze( continue; } - if (walker is null) + if (isFirstCandidate) { if (methodSymbol.ImplementsInterfaceMember(allInterfaces: true)) break; - walker = RefReadOnlyParameterWalker.GetInstance(); + isFirstCandidate = false; } else if (walker.Parameters.ContainsKey(parameter.Name)) { @@ -173,67 +177,56 @@ private static void Analyze( walker.Parameters.Add(parameter.Name, parameter); } - if (walker is null) - return; - - try + if (walker.Parameters.Count > 0) { + walker.Initialize(semanticModel, cancellationToken); + + if (bodyOrExpressionBody.IsKind(SyntaxKind.Block)) + { + walker.VisitBlock((BlockSyntax)bodyOrExpressionBody); + } + else + { + walker.VisitArrowExpressionClause((ArrowExpressionClauseSyntax)bodyOrExpressionBody); + } + if (walker.Parameters.Count > 0) { - walker.SemanticModel = semanticModel; - walker.CancellationToken = cancellationToken; + DataFlowAnalysis analysis = (bodyOrExpressionBody.IsKind(SyntaxKind.Block)) + ? semanticModel.AnalyzeDataFlow((BlockSyntax)bodyOrExpressionBody) + : semanticModel.AnalyzeDataFlow(((ArrowExpressionClauseSyntax)bodyOrExpressionBody).Expression); - if (bodyOrExpressionBody.IsKind(SyntaxKind.Block)) - { - walker.VisitBlock((BlockSyntax)bodyOrExpressionBody); - } - else - { - walker.VisitArrowExpressionClause((ArrowExpressionClauseSyntax)bodyOrExpressionBody); - } + bool? isReferencedAsMethodGroup = null; - if (walker.Parameters.Count > 0) + foreach (KeyValuePair kvp in walker.Parameters) { - DataFlowAnalysis analysis = (bodyOrExpressionBody.IsKind(SyntaxKind.Block)) - ? semanticModel.AnalyzeDataFlow((BlockSyntax)bodyOrExpressionBody) - : semanticModel.AnalyzeDataFlow(((ArrowExpressionClauseSyntax)bodyOrExpressionBody).Expression); - - bool? isReferencedAsMethodGroup = null; + var isAssigned = false; - foreach (KeyValuePair kvp in walker.Parameters) + foreach (ISymbol assignedSymbol in analysis.AlwaysAssigned) { - var isAssigned = false; - - foreach (ISymbol assignedSymbol in analysis.AlwaysAssigned) + if (SymbolEqualityComparer.Default.Equals(assignedSymbol, kvp.Value)) { - if (SymbolEqualityComparer.Default.Equals(assignedSymbol, kvp.Value)) - { - isAssigned = true; - break; - } + isAssigned = true; + break; } + } - if (isAssigned) - continue; + if (isAssigned) + continue; - if (isReferencedAsMethodGroup ??= IsReferencedAsMethodGroup()) - break; + if (isReferencedAsMethodGroup ??= IsReferencedAsMethodGroup()) + break; - if (kvp.Value.GetSyntaxOrDefault(cancellationToken) is ParameterSyntax parameter) - { - DiagnosticHelpers.ReportDiagnostic( - context, - DiagnosticRules.MakeParameterRefReadOnly, - parameter.Identifier); - } + if (kvp.Value.GetSyntaxOrDefault(cancellationToken) is ParameterSyntax parameter) + { + DiagnosticHelpers.ReportDiagnostic( + context, + DiagnosticRules.MakeParameterRefReadOnly, + parameter.Identifier); } } } } - finally - { - RefReadOnlyParameterWalker.Free(walker); - } bool IsReferencedAsMethodGroup() { @@ -249,22 +242,30 @@ bool IsReferencedAsMethodGroup() } } - private class RefReadOnlyParameterWalker : BaseCSharpSyntaxWalker + private class RefReadOnlyParameterWalker : BaseCSharpSyntaxWalker, IResettable { - [ThreadStatic] - private static RefReadOnlyParameterWalker _cachedInstance; - private int _localFunctionDepth; private int _anonymousFunctionDepth; + private bool _canBeCached = true; public Dictionary Parameters { get; } = []; - public SemanticModel SemanticModel { get; set; } + public SemanticModel SemanticModel { get; private set; } + + public CancellationToken CancellationToken { get; private set; } - public CancellationToken CancellationToken { get; set; } + public bool CanBeCached => _canBeCached; + + public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) + { + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } public void Reset() { + _canBeCached = Parameters.Count <= ObjectPool.MaxCachedBufferSize; + Parameters.Clear(); SemanticModel = null; CancellationToken = default; @@ -333,29 +334,5 @@ public override void VisitLocalFunctionStatement(LocalFunctionStatementSyntax no base.VisitLocalFunctionStatement(node); _localFunctionDepth--; } - - public static RefReadOnlyParameterWalker GetInstance() - { - RefReadOnlyParameterWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.Parameters.Count == 0); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new RefReadOnlyParameterWalker(); - } - - public static void Free(RefReadOnlyParameterWalker walker) - { - walker.Reset(); - - _cachedInstance = walker; - } } } diff --git a/src/Analyzers/CSharp/Analysis/RemoveOriginalExceptionFromThrowStatementAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveOriginalExceptionFromThrowStatementAnalyzer.cs index 652f1ed392..743b127b87 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveOriginalExceptionFromThrowStatementAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveOriginalExceptionFromThrowStatementAnalyzer.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Collections.Immutable; -using System.Diagnostics; using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -51,26 +49,11 @@ private static void AnalyzeCatchClause(SyntaxNodeAnalysisContext context) if (symbol?.IsErrorType() != false) return; - ExpressionSyntax expression = null; - Walker walker = null; + var walker = new Walker(symbol, semanticModel, cancellationToken); - try - { - walker = Walker.GetInstance(); - - walker.Symbol = symbol; - walker.SemanticModel = semanticModel; - walker.CancellationToken = cancellationToken; - - walker.VisitBlock(catchClause.Block); + walker.VisitBlock(catchClause.Block); - expression = walker.ThrowStatement?.Expression; - } - finally - { - if (walker is not null) - Walker.Free(walker); - } + ExpressionSyntax expression = walker.ThrowStatement?.Expression; if (expression is not null) { @@ -83,16 +66,23 @@ private static void AnalyzeCatchClause(SyntaxNodeAnalysisContext context) private class Walker : CSharpSyntaxWalker { - [ThreadStatic] - private static Walker _cachedInstance; + public Walker( + ISymbol symbol, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + Symbol = symbol; + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } - public ThrowStatementSyntax ThrowStatement { get; set; } + public ThrowStatementSyntax ThrowStatement { get; private set; } - public ISymbol Symbol { get; set; } + public ISymbol Symbol { get; } - public SemanticModel SemanticModel { get; set; } + public SemanticModel SemanticModel { get; } - public CancellationToken CancellationToken { get; set; } + public CancellationToken CancellationToken { get; } public override void VisitCatchClause(CatchClauseSyntax node) { @@ -112,33 +102,5 @@ public override void VisitThrowStatement(ThrowStatementSyntax node) base.VisitThrowStatement(node); } - - public static Walker GetInstance() - { - Walker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.Symbol is null); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - Debug.Assert(walker.ThrowStatement is null); - - _cachedInstance = null; - return walker; - } - - return new Walker(); - } - - public static void Free(Walker walker) - { - walker.Symbol = null; - walker.SemanticModel = null; - walker.CancellationToken = default; - walker.ThrowStatement = null; - - _cachedInstance = walker; - } } } diff --git a/src/Analyzers/CSharp/Analysis/RemoveRedundantAssignmentAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveRedundantAssignmentAnalyzer.cs index 8d07b9c910..be363d2aad 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveRedundantAssignmentAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveRedundantAssignmentAnalyzer.cs @@ -227,29 +227,11 @@ private static void AnalyzeSimpleAssignment(SyntaxNodeAnalysisContext context) if (IsAssignedInsideAnonymousFunctionButDeclaredOutsideOfIt()) return; - bool result; - RemoveRedundantAssignmentWalker walker = null; + var walker = new RemoveRedundantAssignmentWalker(symbol, context.SemanticModel, context.CancellationToken); - try - { - walker = RemoveRedundantAssignmentWalker.GetInstance(); - - walker.Symbol = symbol; - walker.SemanticModel = context.SemanticModel; - walker.CancellationToken = context.CancellationToken; - walker.Result = false; - - walker.Visit(assignmentInfo.Right); - - result = walker.Result; - } - finally - { - if (walker is not null) - RemoveRedundantAssignmentWalker.Free(walker); - } + walker.Visit(assignmentInfo.Right); - if (result) + if (walker.Result) return; if (IsDeclaredInTryStatementOrCatchClauseAndReferencedInFinallyClause(context, assignmentInfo.Statement, symbol)) @@ -324,22 +306,12 @@ private static bool IsDeclaredInTryStatementOrCatchClauseAndReferencedInFinallyC if (block is not null) { - ContainsLocalOrParameterReferenceWalker walker = null; + var walker = new ContainsLocalOrParameterReferenceWalker(symbol, context.SemanticModel, context.CancellationToken); - try - { - walker = ContainsLocalOrParameterReferenceWalker.GetInstance(symbol, context.SemanticModel, context.CancellationToken); - - walker.VisitBlock(block); + walker.VisitBlock(block); - if (walker.Result) - return true; - } - finally - { - if (walker is not null) - ContainsLocalOrParameterReferenceWalker.Free(walker); - } + if (walker.Result) + return true; } } @@ -351,18 +323,25 @@ private static bool IsDeclaredInTryStatementOrCatchClauseAndReferencedInFinallyC private class RemoveRedundantAssignmentWalker : LocalOrParameterReferenceWalker { - [ThreadStatic] - private static RemoveRedundantAssignmentWalker _cachedInstance; - private int _anonymousFunctionDepth; - public bool Result { get; set; } + public RemoveRedundantAssignmentWalker( + ISymbol symbol, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + Symbol = symbol; + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } + + public bool Result { get; private set; } - public ISymbol Symbol { get; set; } + public ISymbol Symbol { get; } - public SemanticModel SemanticModel { get; set; } + public SemanticModel SemanticModel { get; } - public CancellationToken CancellationToken { get; set; } + public CancellationToken CancellationToken { get; } protected override bool ShouldVisit => !Result; @@ -400,31 +379,5 @@ public override void VisitParenthesizedLambdaExpression(ParenthesizedLambdaExpre base.VisitParenthesizedLambdaExpression(node); _anonymousFunctionDepth--; } - - public static RemoveRedundantAssignmentWalker GetInstance() - { - RemoveRedundantAssignmentWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.Symbol is null); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new RemoveRedundantAssignmentWalker(); - } - - public static void Free(RemoveRedundantAssignmentWalker walker) - { - walker.Symbol = null; - walker.SemanticModel = null; - walker.CancellationToken = default; - - _cachedInstance = walker; - } } } diff --git a/src/Analyzers/CSharp/Analysis/ReturnCompletedTaskInsteadOfNullAnalyzer.cs b/src/Analyzers/CSharp/Analysis/ReturnCompletedTaskInsteadOfNullAnalyzer.cs index 6a476f5d83..c7c6935bba 100644 --- a/src/Analyzers/CSharp/Analysis/ReturnCompletedTaskInsteadOfNullAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/ReturnCompletedTaskInsteadOfNullAnalyzer.cs @@ -1,9 +1,7 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.Diagnostics; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -318,7 +316,7 @@ private static void AnalyzeBlock(SyntaxNodeAnalysisContext context, BlockSyntax if (body is null) return; - SyntaxWalker walker = SyntaxWalker.GetInstance(); + var walker = new SyntaxWalker(); walker.VisitBlock(body); @@ -327,8 +325,6 @@ private static void AnalyzeBlock(SyntaxNodeAnalysisContext context, BlockSyntax foreach (ExpressionSyntax expression in walker.Expressions) ReportDiagnostic(context, expression); } - - SyntaxWalker.Free(walker); } public static bool IsTaskOrTaskOfT(ITypeSymbol typeSymbol) @@ -355,9 +351,6 @@ private static void ReportDiagnostic(SyntaxNodeAnalysisContext context, Expressi private class SyntaxWalker : StatementWalker { - [ThreadStatic] - private static SyntaxWalker _cachedInstance; - public List Expressions { get; private set; } public override void VisitReturnStatement(ReturnStatementSyntax node) @@ -377,27 +370,5 @@ public override void VisitReturnStatement(ReturnStatementSyntax node) public override void VisitLocalFunctionStatement(LocalFunctionStatementSyntax node) { } - - public static SyntaxWalker GetInstance() - { - SyntaxWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.Expressions is null || walker.Expressions.Count == 0); - - _cachedInstance = null; - return walker; - } - - return new SyntaxWalker(); - } - - public static void Free(SyntaxWalker walker) - { - walker.Expressions?.Clear(); - - _cachedInstance = walker; - } } } diff --git a/src/Analyzers/CSharp/Analysis/UnnecessaryExplicitUseOfEnumeratorAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UnnecessaryExplicitUseOfEnumeratorAnalyzer.cs index 06af60fc52..e7e09cacd6 100644 --- a/src/Analyzers/CSharp/Analysis/UnnecessaryExplicitUseOfEnumeratorAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UnnecessaryExplicitUseOfEnumeratorAnalyzer.cs @@ -70,26 +70,11 @@ private static void AnalyzeUsingStatement(SyntaxNodeAnalysisContext context) if (!string.Equals(invocationInfo2.NameText, WellKnownMemberNames.GetEnumeratorMethodName, StringComparison.Ordinal)) return; - bool? isFixable; - UnnecessaryUsageOfEnumeratorWalker walker = null; + var walker = new UnnecessaryUsageOfEnumeratorWalker(declarator, context.SemanticModel, context.CancellationToken); - try - { - walker = UnnecessaryUsageOfEnumeratorWalker.GetInstance(); - - walker.SetValues(declarator, context.SemanticModel, context.CancellationToken); - - walker.Visit(whileStatement.Statement); - - isFixable = walker.IsFixable; - } - finally - { - if (walker is not null) - UnnecessaryUsageOfEnumeratorWalker.Free(walker); - } + walker.Visit(whileStatement.Statement); - if (isFixable == true) + if (walker.IsFixable == true) { DiagnosticHelpers.ReportDiagnostic(context, DiagnosticRules.UnnecessaryExplicitUseOfEnumerator, usingStatement.UsingKeyword); } diff --git a/src/Analyzers/CSharp/Analysis/UnnecessaryUsageOfEnumeratorWalker.cs b/src/Analyzers/CSharp/Analysis/UnnecessaryUsageOfEnumeratorWalker.cs index d785b13fdf..4c60b431dc 100644 --- a/src/Analyzers/CSharp/Analysis/UnnecessaryUsageOfEnumeratorWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UnnecessaryUsageOfEnumeratorWalker.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Diagnostics; using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -12,31 +11,25 @@ namespace Roslynator.CSharp.Analysis; internal class UnnecessaryUsageOfEnumeratorWalker : BaseCSharpSyntaxWalker { - [ThreadStatic] - private static UnnecessaryUsageOfEnumeratorWalker _cachedInstance; - + private readonly VariableDeclaratorSyntax _variableDeclarator; + private readonly string _name; + private readonly SemanticModel _semanticModel; + private readonly CancellationToken _cancellationToken; private ISymbol _symbol; - private VariableDeclaratorSyntax _variableDeclarator; - private string _name; - private SemanticModel _semanticModel; - private CancellationToken _cancellationToken; - - public bool? IsFixable { get; private set; } - public void SetValues( + public UnnecessaryUsageOfEnumeratorWalker( VariableDeclaratorSyntax variableDeclarator, SemanticModel semanticModel, CancellationToken cancellationToken) { - IsFixable = null; - - _symbol = null; - _name = variableDeclarator?.Identifier.ValueText; _variableDeclarator = variableDeclarator; + _name = variableDeclarator?.Identifier.ValueText; _semanticModel = semanticModel; _cancellationToken = cancellationToken; } + public bool? IsFixable { get; private set; } + protected override bool ShouldVisit => IsFixable != false; public override void VisitIdentifierName(IdentifierNameSyntax node) @@ -86,29 +79,4 @@ public override void VisitIdentifierName(IdentifierNameSyntax node) IsFixable = true; } - - public static UnnecessaryUsageOfEnumeratorWalker GetInstance() - { - UnnecessaryUsageOfEnumeratorWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker._symbol is null); - Debug.Assert(walker._variableDeclarator is null); - Debug.Assert(walker._semanticModel is null); - Debug.Assert(walker._cancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new UnnecessaryUsageOfEnumeratorWalker(); - } - - public static void Free(UnnecessaryUsageOfEnumeratorWalker walker) - { - walker.SetValues(default(VariableDeclaratorSyntax), default(SemanticModel), default(CancellationToken)); - - _cachedInstance = walker; - } } diff --git a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs index ac71ac28ba..f097210fe8 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs @@ -76,7 +76,9 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) SyntaxList members = typeDeclaration.Members; - UnusedMemberWalker walker = null; + using PooledObject pooledWalker = ObjectPool.Rent(); + + UnusedMemberWalker walker = pooledWalker.Value; foreach (MemberDeclarationSyntax member in members) { @@ -91,12 +93,7 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) case DelegateDeclarationSyntax declaration: { if (SyntaxAccessibility.Instance.GetAccessibility(declaration) == Accessibility.Private) - { - if (walker is null) - walker = UnusedMemberWalker.GetInstance(); - walker.AddDelegate(declaration.Identifier.ValueText, declaration); - } break; } @@ -105,9 +102,6 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) if (declaration.ExplicitInterfaceSpecifier is null && SyntaxAccessibility.Instance.GetAccessibility(declaration) == Accessibility.Private) { - if (walker is null) - walker = UnusedMemberWalker.GetInstance(); - walker.AddNode(declaration.Identifier.ValueText, declaration); } @@ -116,12 +110,7 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) case EventFieldDeclarationSyntax declaration: { if (SyntaxAccessibility.Instance.GetAccessibility(declaration) == Accessibility.Private) - { - if (walker is null) - walker = UnusedMemberWalker.GetInstance(); - walker.AddNodes(declaration.Declaration); - } break; } @@ -130,12 +119,7 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) SyntaxTokenList modifiers = declaration.Modifiers; if (SyntaxAccessibility.Instance.GetAccessibility(declaration) == Accessibility.Private) - { - if (walker is null) - walker = UnusedMemberWalker.GetInstance(); - walker.AddNodes(declaration.Declaration, isConst: modifiers.Contains(SyntaxKind.ConstKeyword)); - } break; } @@ -175,9 +159,6 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) } } - if (walker is null) - walker = UnusedMemberWalker.GetInstance(); - walker.AddNode(methodName, declaration); break; @@ -187,9 +168,6 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) if (declaration.ExplicitInterfaceSpecifier is null && SyntaxAccessibility.Instance.GetAccessibility(declaration) == Accessibility.Private) { - if (walker is null) - walker = UnusedMemberWalker.GetInstance(); - walker.AddNode(declaration.Identifier.ValueText, declaration); } @@ -198,44 +176,37 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) } } - if (walker is null) + Collection nodes = walker.Nodes; + + if (nodes.Count == 0) return; - try + if (ShouldAnalyzeDebuggerDisplayAttribute() + && nodes.Any(f => f.CanBeInDebuggerDisplayAttribute)) { - Collection nodes = walker.Nodes; - - if (ShouldAnalyzeDebuggerDisplayAttribute() - && nodes.Any(f => f.CanBeInDebuggerDisplayAttribute)) - { - if (attributes.IsDefault) - attributes = semanticModel.GetDeclaredSymbol(typeDeclaration, cancellationToken).GetAttributes(); - - string value = attributes - .FirstOrDefault(f => f.AttributeClass.HasMetadataName(MetadataNames.System_Diagnostics_DebuggerDisplayAttribute))? - .ConstructorArguments - .SingleOrDefault(shouldThrow: false) - .Value? - .ToString(); - - if (value is not null) - RemoveMethodsAndPropertiesThatAreInDebuggerDisplayAttributeValue(value, ref nodes); - } + if (attributes.IsDefault) + attributes = semanticModel.GetDeclaredSymbol(typeDeclaration, cancellationToken).GetAttributes(); + + string value = attributes + .FirstOrDefault(f => f.AttributeClass.HasMetadataName(MetadataNames.System_Diagnostics_DebuggerDisplayAttribute))? + .ConstructorArguments + .SingleOrDefault(shouldThrow: false) + .Value? + .ToString(); + + if (value is not null) + RemoveMethodsAndPropertiesThatAreInDebuggerDisplayAttributeValue(value, ref nodes); + } - if (nodes.Count > 0) - { - walker.SemanticModel = semanticModel; - walker.CancellationToken = cancellationToken; + if (nodes.Count > 0) + { + walker.SemanticModel = semanticModel; + walker.CancellationToken = cancellationToken; - walker.Visit(typeDeclaration); + walker.Visit(typeDeclaration); - foreach (NodeSymbolInfo node in nodes) - ReportDiagnostic(context, node.Node); - } - } - finally - { - UnusedMemberWalker.Free(walker); + foreach (NodeSymbolInfo node in nodes) + ReportDiagnostic(context, node.Node); } bool ShouldAnalyzeDebuggerDisplayAttribute() diff --git a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs index 065c8ff071..fbe1120122 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs @@ -1,6 +1,5 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Collections.Immutable; using System.Collections.ObjectModel; using System.Diagnostics; @@ -12,12 +11,10 @@ namespace Roslynator.CSharp.Analysis.UnusedMember; -internal class UnusedMemberWalker : TypeSyntaxWalker +internal class UnusedMemberWalker : TypeSyntaxWalker, IResettable { - [ThreadStatic] - private static UnusedMemberWalker _cachedInstance; - private bool _isEmpty; + private bool _canBeCached = true; private IMethodSymbol _containingMethodSymbol; @@ -36,8 +33,11 @@ protected override bool ShouldVisit get { return !_isEmpty; } } + public bool CanBeCached => _canBeCached; + public void Reset() { + _canBeCached = Nodes.Count <= ObjectPool.MaxCachedBufferSize; _isEmpty = false; _containingMethodSymbol = null; @@ -604,28 +604,4 @@ private void VisitAttributeLists(SyntaxList attributeLists) } } - public static UnusedMemberWalker GetInstance() - { - UnusedMemberWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker._containingMethodSymbol is null); - Debug.Assert(walker.Nodes.Count == 0); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new UnusedMemberWalker(); - } - - public static void Free(UnusedMemberWalker walker) - { - walker.Reset(); - - _cachedInstance = walker; - } } diff --git a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterAnalyzer.cs index 196d82024b..2a935e7a56 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterAnalyzer.cs @@ -143,27 +143,19 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) if (methodSymbol.ImplementsInterfaceMember(allInterfaces: true)) return; - UnusedParameterWalker walker = null; + using PooledObject pooledWalker = ObjectPool.Rent(); - try - { - walker = UnusedParameterWalker.GetInstance(); + UnusedParameterWalker walker = pooledWalker.Value; - walker.SetValues(context.SemanticModel, context.CancellationToken); + walker.Initialize(context.SemanticModel, context.CancellationToken); - FindUnusedNodes(parameterInfo, walker, methodSymbol); + FindUnusedNodes(parameterInfo, walker, methodSymbol); - if (walker.Nodes.Count > 0 - && !MethodReferencedAsMethodGroupWalker.IsReferencedAsMethodGroup(methodDeclaration, methodSymbol, context.SemanticModel, context.CancellationToken)) - { - foreach (KeyValuePair kvp in walker.Nodes) - ReportDiagnostic(context, kvp.Value.Node); - } - } - finally + if (walker.Nodes.Count > 0 + && !MethodReferencedAsMethodGroupWalker.IsReferencedAsMethodGroup(methodDeclaration, methodSymbol, context.SemanticModel, context.CancellationToken)) { - if (walker is not null) - UnusedParameterWalker.Free(walker); + foreach (KeyValuePair kvp in walker.Nodes) + ReportDiagnostic(context, kvp.Value.Node); } } @@ -337,23 +329,16 @@ private static void AnalyzeAnonymousMethodExpression(SyntaxNodeAnalysisContext c private static void Analyze(SyntaxNodeAnalysisContext context, in ParameterInfo parameterInfo, bool isIndexer = false) { - UnusedParameterWalker walker = null; + using PooledObject pooledWalker = ObjectPool.Rent(); - try - { - walker = UnusedParameterWalker.GetInstance(); - walker.SetValues(context.SemanticModel, context.CancellationToken, isIndexer); + UnusedParameterWalker walker = pooledWalker.Value; - FindUnusedNodes(parameterInfo, walker); + walker.Initialize(context.SemanticModel, context.CancellationToken, isIndexer); - foreach (KeyValuePair kvp in walker.Nodes) - ReportDiagnostic(context, kvp.Value.Node); - } - finally - { - if (walker is not null) - UnusedParameterWalker.Free(walker); - } + FindUnusedNodes(parameterInfo, walker); + + foreach (KeyValuePair kvp in walker.Nodes) + ReportDiagnostic(context, kvp.Value.Node); } private static void FindUnusedNodes(in ParameterInfo parameterInfo, UnusedParameterWalker walker, IMethodSymbol methodSymbol = null) diff --git a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs index bd3bf1bf89..654170079d 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs @@ -2,7 +2,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -11,35 +10,43 @@ namespace Roslynator.CSharp.Analysis.UnusedParameter; -internal class UnusedParameterWalker : TypeSyntaxWalker +internal class UnusedParameterWalker : TypeSyntaxWalker, IResettable { - [ThreadStatic] - private static UnusedParameterWalker _cachedInstance; - private static readonly StringComparer _ordinalComparer = StringComparer.Ordinal; private bool _isEmpty; + private bool _canBeCached = true; public Dictionary Nodes { get; } = new(_ordinalComparer); - public SemanticModel SemanticModel { get; set; } + public SemanticModel SemanticModel { get; private set; } - public CancellationToken CancellationToken { get; set; } + public CancellationToken CancellationToken { get; private set; } - public bool IsIndexer { get; set; } + public bool IsIndexer { get; private set; } public bool IsAnyTypeParameter { get; set; } protected override bool ShouldVisit => !_isEmpty; - public void SetValues(SemanticModel semanticModel, CancellationToken cancellationToken, bool isIndexer = false) - { - _isEmpty = false; + public bool CanBeCached => _canBeCached; - Nodes.Clear(); + public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken, bool isIndexer = false) + { SemanticModel = semanticModel; CancellationToken = cancellationToken; IsIndexer = isIndexer; + } + + public void Reset() + { + _canBeCached = Nodes.Count <= ObjectPool.MaxCachedBufferSize; + _isEmpty = false; + + Nodes.Clear(); + SemanticModel = null; + CancellationToken = default; + IsIndexer = false; IsAnyTypeParameter = false; } @@ -198,27 +205,4 @@ public override void VisitTypeParameterConstraintClause(TypeParameterConstraintC } } - public static UnusedParameterWalker GetInstance() - { - UnusedParameterWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.Nodes.Count == 0); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new UnusedParameterWalker(); - } - - public static void Free(UnusedParameterWalker walker) - { - walker.SetValues(default(SemanticModel), default(CancellationToken)); - - _cachedInstance = walker; - } } diff --git a/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs index 14cf301009..96d4c7f564 100644 --- a/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs @@ -154,35 +154,29 @@ private static void AnalyzeAnonymousMethodExpression(SyntaxNodeAnalysisContext c private static bool IsFixable(BlockSyntax body, SyntaxNodeAnalysisContext context) { - UseAsyncAwaitWalker walker = null; + using PooledObject pooledWalker = ObjectPool.Rent(); - try - { - walker = UseAsyncAwaitWalker.GetInstance(context.SemanticModel, context.CancellationToken); + UseAsyncAwaitWalker walker = pooledWalker.Value; - walker.VisitBlock(body); + walker.Initialize(context.SemanticModel, context.CancellationToken); - return walker.ReturnStatement is not null - && !CSharpUtility.IsInUnsafeContext(body); - } - finally - { - if (walker is not null) - UseAsyncAwaitWalker.Free(walker); - } + walker.VisitBlock(body); + + return walker.ReturnStatement is not null + && !CSharpUtility.IsInUnsafeContext(body); } - private class UseAsyncAwaitWalker : StatementWalker + private class UseAsyncAwaitWalker : StatementWalker, IResettable { - [ThreadStatic] - private static UseAsyncAwaitWalker _cachedInstance; - + private readonly List _usingDeclarations = []; private int _usingOrTryStatementDepth; private bool _shouldVisit = true; - private readonly List _usingDeclarations = []; + private bool _canBeCached = true; public override bool ShouldVisit => _shouldVisit; + public bool CanBeCached => _canBeCached; + public ReturnStatementSyntax ReturnStatement { get; private set; } public SemanticModel SemanticModel { get; private set; } @@ -330,34 +324,22 @@ public override void VisitParenthesizedLambdaExpression(ParenthesizedLambdaExpre { } - public static UseAsyncAwaitWalker GetInstance(SemanticModel semanticModel, CancellationToken cancellationToken) + public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) { - UseAsyncAwaitWalker walker = _cachedInstance; - - if (walker is not null) - { - _cachedInstance = null; - } - else - { - walker = new UseAsyncAwaitWalker(); - } - - walker.SemanticModel = semanticModel; - walker.CancellationToken = cancellationToken; - - return walker; + SemanticModel = semanticModel; + CancellationToken = cancellationToken; } - public static void Free(UseAsyncAwaitWalker walker) + public void Reset() { - walker._shouldVisit = true; - walker._usingDeclarations.Clear(); - walker.ReturnStatement = null; - walker.SemanticModel = null; - walker.CancellationToken = default; - - _cachedInstance = walker; + _canBeCached = _usingDeclarations.Count <= ObjectPool.MaxCachedBufferSize; + _shouldVisit = true; + _usingOrTryStatementDepth = 0; + + _usingDeclarations.Clear(); + ReturnStatement = null; + SemanticModel = null; + CancellationToken = default; } } } diff --git a/src/Analyzers/CSharp/Analysis/UseAutoPropertyAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UseAutoPropertyAnalyzer.cs index 61b8e8aa9f..d8bd90d314 100644 --- a/src/Analyzers/CSharp/Analysis/UseAutoPropertyAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UseAutoPropertyAnalyzer.cs @@ -212,50 +212,34 @@ private static bool IsFixableBackingField( && !propertySymbol.IsStatic && (propertySymbol.IsVirtual || propertySymbol.IsOverride); - var isFixable = false; - UseAutoPropertyWalker walker = null; + ImmutableArray syntaxReferences = containingType.DeclaringSyntaxReferences; - try + if (syntaxReferences.Length == 1) { - walker = UseAutoPropertyWalker.GetInstance(); + var walker = new UseAutoPropertyWalker(fieldSymbol, shouldSearchForReferenceInInstanceConstructor, semanticModel, cancellationToken); - ImmutableArray syntaxReferences = containingType.DeclaringSyntaxReferences; + walker.Visit(propertyDeclaration.Parent); - if (syntaxReferences.Length == 1) - { - walker.SetValues(fieldSymbol, shouldSearchForReferenceInInstanceConstructor, semanticModel, cancellationToken); + return walker.Success; + } - walker.Visit(propertyDeclaration.Parent); + var isFixable = false; - isFixable = walker.Success; - } - else - { - foreach (SyntaxReference syntaxReference in syntaxReferences) - { - SyntaxNode typeDeclaration = syntaxReference.GetSyntax(cancellationToken); + foreach (SyntaxReference syntaxReference in syntaxReferences) + { + SyntaxNode typeDeclaration = syntaxReference.GetSyntax(cancellationToken); - if (typeDeclaration.SyntaxTree != semanticModel.SyntaxTree) - { - isFixable = false; - break; - } + if (typeDeclaration.SyntaxTree != semanticModel.SyntaxTree) + return false; - walker.SetValues(fieldSymbol, shouldSearchForReferenceInInstanceConstructor, semanticModel, cancellationToken); + var walker = new UseAutoPropertyWalker(fieldSymbol, shouldSearchForReferenceInInstanceConstructor, semanticModel, cancellationToken); - walker.Visit(typeDeclaration); + walker.Visit(typeDeclaration); - isFixable = walker.Success; + isFixable = walker.Success; - if (!isFixable) - break; - } - } - } - finally - { - if (walker is not null) - UseAutoPropertyWalker.Free(walker); + if (!isFixable) + break; } return isFixable; @@ -461,19 +445,7 @@ private class UseAutoPropertyWalker : BaseCSharpSyntaxWalker { private bool _isInInstanceConstructor; - public IFieldSymbol FieldSymbol { get; private set; } - - public bool ShouldSearchForReferenceInInstanceConstructor { get; private set; } - - public SemanticModel SemanticModel { get; private set; } - - public CancellationToken CancellationToken { get; private set; } - - public bool Success { get; set; } = true; - - protected override bool ShouldVisit => Success; - - public void SetValues( + public UseAutoPropertyWalker( IFieldSymbol fieldSymbol, bool shouldSearchForReferenceInInstanceConstructor, SemanticModel semanticModel, @@ -483,9 +455,20 @@ public void SetValues( ShouldSearchForReferenceInInstanceConstructor = shouldSearchForReferenceInInstanceConstructor; SemanticModel = semanticModel; CancellationToken = cancellationToken; - Success = true; } + public IFieldSymbol FieldSymbol { get; } + + public bool ShouldSearchForReferenceInInstanceConstructor { get; } + + public SemanticModel SemanticModel { get; } + + public CancellationToken CancellationToken { get; } + + public bool Success { get; private set; } = true; + + protected override bool ShouldVisit => Success; + public override void VisitArgument(ArgumentSyntax node) { CancellationToken.ThrowIfCancellationRequested(); @@ -654,36 +637,5 @@ private bool IsBackingFieldReference(IdentifierNameSyntax identifierName) return string.Equals(identifierName.Identifier.ValueText, FieldSymbol.Name, StringComparison.Ordinal) && SymbolEqualityComparer.Default.Equals(SemanticModel.GetSymbol(identifierName, CancellationToken), FieldSymbol); } - - [ThreadStatic] - private static UseAutoPropertyWalker _cachedInstance; - - public static UseAutoPropertyWalker GetInstance() - { - UseAutoPropertyWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.FieldSymbol is null); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new UseAutoPropertyWalker(); - } - - public static void Free(UseAutoPropertyWalker walker) - { - walker.SetValues( - default(IFieldSymbol), - false, - default(SemanticModel), - default(CancellationToken)); - - _cachedInstance = walker; - } } } diff --git a/src/Analyzers/CSharp/Analysis/UseExceptionFilterAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UseExceptionFilterAnalyzer.cs index d1235fa538..ba77716969 100644 --- a/src/Analyzers/CSharp/Analysis/UseExceptionFilterAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UseExceptionFilterAnalyzer.cs @@ -2,7 +2,6 @@ using System; using System.Collections.Immutable; -using System.Diagnostics; using System.Text.RegularExpressions; using System.Threading; using Microsoft.CodeAnalysis; @@ -55,27 +54,11 @@ private static void AnalyzeCatchClause(SyntaxNodeAnalysisContext context) if (IsThrowStatementWithoutExpression(ifStatement.Statement.SingleNonBlockStatementOrDefault()) ^ IsThrowStatementWithoutExpression(ifStatement.Else?.Statement.SingleNonBlockStatementOrDefault())) { - bool canUseExceptionFilter; - UseExceptionFilterWalker walker = null; + var walker = new UseExceptionFilterWalker(context.SemanticModel, context.CancellationToken); - try - { - walker = UseExceptionFilterWalker.GetInstance(); - - walker.SemanticModel = context.SemanticModel; - walker.CancellationToken = context.CancellationToken; - - walker.Visit(ifStatement.Condition); - - canUseExceptionFilter = walker.CanUseExceptionFilter; - } - finally - { - if (walker is not null) - UseExceptionFilterWalker.Free(walker); - } + walker.Visit(ifStatement.Condition); - if (!canUseExceptionFilter) + if (!walker.CanUseExceptionFilter) return; if (ifStatement.ContainsUnbalancedIfElseDirectives()) @@ -93,16 +76,19 @@ private static bool IsThrowStatementWithoutExpression(StatementSyntax statement) private class UseExceptionFilterWalker : BaseCSharpSyntaxWalker { - [ThreadStatic] - private static UseExceptionFilterWalker _cachedInstance; - private static readonly Regex _exceptionElementRegex = new(@"\<(?i:exception)\ +cref=(?:""|')"); - public bool CanUseExceptionFilter { get; set; } = true; + public UseExceptionFilterWalker(SemanticModel semanticModel, CancellationToken cancellationToken) + { + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } + + public bool CanUseExceptionFilter { get; private set; } = true; - public SemanticModel SemanticModel { get; set; } + public SemanticModel SemanticModel { get; } - public CancellationToken CancellationToken { get; set; } + public CancellationToken CancellationToken { get; } protected override bool ShouldVisit => CanUseExceptionFilter; @@ -172,31 +158,5 @@ public override void VisitSimpleLambdaExpression(SimpleLambdaExpressionSyntax no public override void VisitParenthesizedLambdaExpression(ParenthesizedLambdaExpressionSyntax node) { } - - public static UseExceptionFilterWalker GetInstance() - { - UseExceptionFilterWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.CanUseExceptionFilter = true); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new UseExceptionFilterWalker(); - } - - public static void Free(UseExceptionFilterWalker walker) - { - walker.CanUseExceptionFilter = true; - walker.SemanticModel = null; - walker.CancellationToken = default; - - _cachedInstance = walker; - } } } diff --git a/src/Analyzers/CSharp/Analysis/UseForStatementInsteadOfWhileStatementAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UseForStatementInsteadOfWhileStatementAnalyzer.cs index 46ef3e9de0..21596d0158 100644 --- a/src/Analyzers/CSharp/Analysis/UseForStatementInsteadOfWhileStatementAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UseForStatementInsteadOfWhileStatementAnalyzer.cs @@ -116,43 +116,26 @@ private static void AnalyzeWhileStatement(SyntaxNodeAnalysisContext context) bool ContainsContinueStatement() { - ContainsContinueStatementWalker walker = ContainsContinueStatementWalker.GetInstance(); - walker.ContainsContinueStatement = false; - - var containsContinueStatement = false; + var walker = new ContainsContinueStatementWalker(); foreach (StatementSyntax innerStatement in innerStatements) { walker.Visit(innerStatement); if (walker.ContainsContinueStatement) - { - containsContinueStatement = true; - break; - } + return true; } - ContainsContinueStatementWalker.Free(walker); - - return containsContinueStatement; + return false; } bool IsLocalVariableReferencedAfterWhileStatement() { - ContainsLocalOrParameterReferenceWalker walker = null; - try - { - walker = ContainsLocalOrParameterReferenceWalker.GetInstance(symbol, semanticModel, cancellationToken); + var walker = new ContainsLocalOrParameterReferenceWalker(symbol, semanticModel, cancellationToken); - walker.VisitList(outerStatements, index + 1); + walker.VisitList(outerStatements, index + 1); - return walker.Result; - } - finally - { - if (walker is not null) - ContainsLocalOrParameterReferenceWalker.Free(walker); - } + return walker.Result; } } @@ -182,10 +165,7 @@ private static SingleLocalDeclarationStatementInfo GetLocalInfo(StatementSyntax private class ContainsContinueStatementWalker : BaseCSharpSyntaxWalker { - [ThreadStatic] - private static ContainsContinueStatementWalker _cachedInstance; - - public bool ContainsContinueStatement { get; set; } + public bool ContainsContinueStatement { get; private set; } protected override bool ShouldVisit => !ContainsContinueStatement; @@ -213,23 +193,5 @@ public override void VisitForEachVariableStatement(ForEachVariableStatementSynta public override void VisitWhileStatement(WhileStatementSyntax node) { } - - public static ContainsContinueStatementWalker GetInstance() - { - ContainsContinueStatementWalker walker = _cachedInstance; - - if (walker is not null) - { - _cachedInstance = null; - return walker; - } - - return new ContainsContinueStatementWalker(); - } - - public static void Free(ContainsContinueStatementWalker walker) - { - _cachedInstance = walker; - } } } diff --git a/src/Analyzers/CSharp/Analysis/UsePatternMatching/UsePatternMatchingInsteadOfIsAndCastAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UsePatternMatching/UsePatternMatchingInsteadOfIsAndCastAnalyzer.cs index ced4beb69f..2cee7344f9 100644 --- a/src/Analyzers/CSharp/Analysis/UsePatternMatching/UsePatternMatchingInsteadOfIsAndCastAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UsePatternMatching/UsePatternMatchingInsteadOfIsAndCastAnalyzer.cs @@ -135,25 +135,10 @@ private static bool IsFixable( SemanticModel semanticModel, CancellationToken cancellationToken) { - bool isFixable; - UsePatternMatchingWalker walker = null; + var walker = new UsePatternMatchingWalker(identifierName, semanticModel, cancellationToken); - try - { - walker = UsePatternMatchingWalker.GetInstance(); - - walker.SetValues(identifierName, semanticModel, cancellationToken); - - walker.Visit(node); - - isFixable = walker.IsFixable.GetValueOrDefault(); - } - finally - { - if (walker is not null) - UsePatternMatchingWalker.Free(walker); - } + walker.Visit(node); - return isFixable; + return walker.IsFixable.GetValueOrDefault(); } } diff --git a/src/Analyzers/CSharp/Analysis/UsePatternMatching/UsePatternMatchingWalker.cs b/src/Analyzers/CSharp/Analysis/UsePatternMatching/UsePatternMatchingWalker.cs index 429db7b61d..d6bcb675e0 100644 --- a/src/Analyzers/CSharp/Analysis/UsePatternMatching/UsePatternMatchingWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UsePatternMatching/UsePatternMatchingWalker.cs @@ -1,7 +1,5 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; -using System.Diagnostics; using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -12,36 +10,30 @@ namespace Roslynator.CSharp.Analysis.UsePatternMatching; internal class UsePatternMatchingWalker : BaseCSharpSyntaxWalker { - [ThreadStatic] - private static UsePatternMatchingWalker _cachedInstance; - + private readonly IdentifierNameSyntax _identifierName; + private readonly string _name; + private readonly SemanticModel _semanticModel; + private readonly CancellationToken _cancellationToken; private ISymbol _symbol; - private IdentifierNameSyntax _identifierName; - private string _name; - private SemanticModel _semanticModel; - private CancellationToken _cancellationToken; - - public bool? IsFixable { get; private set; } - - protected override bool ShouldVisit - { - get { return IsFixable != false; } - } - public void SetValues( + public UsePatternMatchingWalker( IdentifierNameSyntax identifierName, SemanticModel semanticModel, CancellationToken cancellationToken) { - IsFixable = null; - - _symbol = null; - _name = identifierName?.Identifier.ValueText; _identifierName = identifierName; + _name = identifierName?.Identifier.ValueText; _semanticModel = semanticModel; _cancellationToken = cancellationToken; } + public bool? IsFixable { get; private set; } + + protected override bool ShouldVisit + { + get { return IsFixable != false; } + } + public override void VisitIdentifierName(IdentifierNameSyntax node) { _cancellationToken.ThrowIfCancellationRequested(); @@ -79,29 +71,4 @@ public override void VisitIdentifierName(IdentifierNameSyntax node) } } } - - public static UsePatternMatchingWalker GetInstance() - { - UsePatternMatchingWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker._symbol is null); - Debug.Assert(walker._identifierName is null); - Debug.Assert(walker._semanticModel is null); - Debug.Assert(walker._cancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new UsePatternMatchingWalker(); - } - - public static void Free(UsePatternMatchingWalker walker) - { - walker.SetValues(default(IdentifierNameSyntax), default(SemanticModel), default(CancellationToken)); - - _cachedInstance = walker; - } } diff --git a/src/Analyzers/CSharp/Analysis/ValidateArgumentsCorrectlyAnalyzer.cs b/src/Analyzers/CSharp/Analysis/ValidateArgumentsCorrectlyAnalyzer.cs index 299d702056..470d0e574b 100644 --- a/src/Analyzers/CSharp/Analysis/ValidateArgumentsCorrectlyAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/ValidateArgumentsCorrectlyAnalyzer.cs @@ -81,14 +81,12 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) context.CancellationToken.ThrowIfCancellationRequested(); - ContainsYieldWalker walker = ContainsYieldWalker.GetInstance(); + var walker = new ContainsYieldWalker(); walker.VisitBlock(body); YieldStatementSyntax yieldStatement = walker.YieldStatement; - ContainsYieldWalker.Free(walker); - if (yieldStatement is null) return; diff --git a/src/CSharp/CSharp/SyntaxWalkers/ContainsCommentWalker.cs b/src/CSharp/CSharp/SyntaxWalkers/ContainsCommentWalker.cs index a737671b65..71daba5bd2 100644 --- a/src/CSharp/CSharp/SyntaxWalkers/ContainsCommentWalker.cs +++ b/src/CSharp/CSharp/SyntaxWalkers/ContainsCommentWalker.cs @@ -1,6 +1,5 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Text; @@ -9,9 +8,6 @@ namespace Roslynator.CSharp.SyntaxWalkers; internal sealed class ContainsCommentWalker : CSharpSyntaxWalker { - [ThreadStatic] - private static ContainsCommentWalker? _cachedInstance; - public ContainsCommentWalker(TextSpan span) : base(SyntaxWalkerDepth.Trivia) { @@ -20,7 +16,7 @@ public ContainsCommentWalker(TextSpan span) public bool Result { get; set; } - public TextSpan Span { get; set; } + public TextSpan Span { get; } public override void VisitTrivia(SyntaxTrivia trivia) { @@ -46,35 +42,10 @@ public static bool ContainsComment(SyntaxNode node) public static bool ContainsComment(SyntaxNode node, TextSpan span) { - ContainsCommentWalker walker = GetInstance(span); + var walker = new ContainsCommentWalker(span); walker.Visit(node); - bool result = walker.Result; - - Free(walker); - - return result; - } - - public static ContainsCommentWalker GetInstance(TextSpan span) - { - ContainsCommentWalker? walker = _cachedInstance; - - if (walker is not null) - { - _cachedInstance = null; - walker.Result = false; - walker.Span = span; - - return walker; - } - - return new ContainsCommentWalker(span); - } - - public static void Free(ContainsCommentWalker walker) - { - _cachedInstance = walker; + return walker.Result; } } diff --git a/src/CSharp/CSharp/SyntaxWalkers/ContainsLocalOrParameterReferenceWalker.cs b/src/CSharp/CSharp/SyntaxWalkers/ContainsLocalOrParameterReferenceWalker.cs index f08426c737..379f773fc7 100644 --- a/src/CSharp/CSharp/SyntaxWalkers/ContainsLocalOrParameterReferenceWalker.cs +++ b/src/CSharp/CSharp/SyntaxWalkers/ContainsLocalOrParameterReferenceWalker.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Diagnostics; using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -10,13 +9,10 @@ namespace Roslynator.CSharp.SyntaxWalkers; internal sealed class ContainsLocalOrParameterReferenceWalker : LocalOrParameterReferenceWalker { - [ThreadStatic] - private static ContainsLocalOrParameterReferenceWalker? _cachedInstance; - public ContainsLocalOrParameterReferenceWalker( ISymbol symbol, SemanticModel semanticModel, - CancellationToken cancellationToken) + CancellationToken cancellationToken = default) { Symbol = symbol; SemanticModel = semanticModel; @@ -25,11 +21,11 @@ public ContainsLocalOrParameterReferenceWalker( public bool Result { get; set; } - public ISymbol? Symbol { get; private set; } + public ISymbol Symbol { get; } - public SemanticModel? SemanticModel { get; private set; } + public SemanticModel SemanticModel { get; } - public CancellationToken CancellationToken { get; private set; } + public CancellationToken CancellationToken { get; } protected override bool ShouldVisit { @@ -40,8 +36,8 @@ public override void VisitIdentifierName(IdentifierNameSyntax node) { CancellationToken.ThrowIfCancellationRequested(); - if (string.Equals(node.Identifier.ValueText, Symbol!.Name, StringComparison.Ordinal) - && SymbolEqualityComparer.Default.Equals(SemanticModel!.GetSymbol(node, CancellationToken), Symbol)) + if (string.Equals(node.Identifier.ValueText, Symbol.Name, StringComparison.Ordinal) + && SymbolEqualityComparer.Default.Equals(SemanticModel.GetSymbol(node, CancellationToken), Symbol)) { Result = true; } @@ -119,37 +115,4 @@ public void VisitList(SeparatedSyntaxList statements, int startInd } } - public static ContainsLocalOrParameterReferenceWalker GetInstance( - ISymbol symbol, - SemanticModel semanticModel, - CancellationToken cancellationToken = default) - { - ContainsLocalOrParameterReferenceWalker? walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.Symbol is null); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - walker.Symbol = symbol; - walker.SemanticModel = semanticModel; - walker.CancellationToken = cancellationToken; - - return walker; - } - - return new ContainsLocalOrParameterReferenceWalker(symbol, semanticModel, cancellationToken); - } - - public static void Free(ContainsLocalOrParameterReferenceWalker walker) - { - walker.Result = false; - walker.Symbol = null; - walker.SemanticModel = null; - walker.CancellationToken = default; - - _cachedInstance = walker; - } } diff --git a/src/CSharp/CSharp/SyntaxWalkers/ContainsYieldWalker.cs b/src/CSharp/CSharp/SyntaxWalkers/ContainsYieldWalker.cs index ca10ee2927..208370d848 100644 --- a/src/CSharp/CSharp/SyntaxWalkers/ContainsYieldWalker.cs +++ b/src/CSharp/CSharp/SyntaxWalkers/ContainsYieldWalker.cs @@ -9,9 +9,6 @@ namespace Roslynator.CSharp.SyntaxWalkers; internal sealed class ContainsYieldWalker : StatementWalker { - [ThreadStatic] - private static ContainsYieldWalker? _cachedInstance; - public ContainsYieldWalker( bool searchForYieldBreak = true, bool searchForYieldReturn = true) @@ -25,9 +22,9 @@ public override bool ShouldVisit get { return YieldStatement is null; } } - public bool SearchForYieldBreak { get; private set; } + public bool SearchForYieldBreak { get; } - public bool SearchForYieldReturn { get; private set; } + public bool SearchForYieldReturn { get; } public YieldStatementSyntax? YieldStatement { get; private set; } @@ -36,17 +33,11 @@ public static bool ContainsYield(StatementSyntax statement, bool searchForYieldR if (statement is null) throw new ArgumentNullException(nameof(statement)); - ContainsYieldWalker walker = GetInstance(); - walker.SearchForYieldBreak = searchForYieldBreak; - walker.SearchForYieldReturn = searchForYieldReturn; + var walker = new ContainsYieldWalker(searchForYieldBreak, searchForYieldReturn); walker.VisitStatement(statement); - bool success = walker.YieldStatement is not null; - - Free(walker); - - return success; + return walker.YieldStatement is not null; } public override void VisitYieldStatement(YieldStatementSyntax node) @@ -70,28 +61,4 @@ public override void VisitYieldStatement(YieldStatementSyntax node) public override void VisitLocalFunctionStatement(LocalFunctionStatementSyntax node) { } - - public static ContainsYieldWalker GetInstance() - { - ContainsYieldWalker? walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.YieldStatement is null); - - _cachedInstance = null; - return walker; - } - - return new ContainsYieldWalker(); - } - - public static void Free(ContainsYieldWalker walker) - { - walker.SearchForYieldBreak = true; - walker.SearchForYieldReturn = true; - walker.YieldStatement = null; - - _cachedInstance = walker; - } } diff --git a/src/CSharp/CSharp/SyntaxWalkers/MethodReferencedAsMethodGroupWalker.cs b/src/CSharp/CSharp/SyntaxWalkers/MethodReferencedAsMethodGroupWalker.cs index d74d8776f6..27ea92a4cb 100644 --- a/src/CSharp/CSharp/SyntaxWalkers/MethodReferencedAsMethodGroupWalker.cs +++ b/src/CSharp/CSharp/SyntaxWalkers/MethodReferencedAsMethodGroupWalker.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Diagnostics; using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -11,16 +10,23 @@ namespace Roslynator.CSharp.SyntaxWalkers; internal class MethodReferencedAsMethodGroupWalker : BaseCSharpSyntaxWalker { - [ThreadStatic] - private static MethodReferencedAsMethodGroupWalker? _cachedInstance; + public MethodReferencedAsMethodGroupWalker( + IMethodSymbol symbol, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + Symbol = symbol; + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } public bool Result { get; set; } - public IMethodSymbol? Symbol { get; set; } + public IMethodSymbol Symbol { get; } - public SemanticModel? SemanticModel { get; set; } + public SemanticModel SemanticModel { get; } - public CancellationToken CancellationToken { get; set; } + public CancellationToken CancellationToken { get; } protected override bool ShouldVisit => !Result; @@ -28,9 +34,9 @@ public override void VisitIdentifierName(IdentifierNameSyntax node) { CancellationToken.ThrowIfCancellationRequested(); - if (string.Equals(Symbol!.Name, node.Identifier.ValueText, StringComparison.Ordinal) + if (string.Equals(Symbol.Name, node.Identifier.ValueText, StringComparison.Ordinal) && !IsInvoked(node) - && SymbolEqualityComparer.Default.Equals(SemanticModel!.GetSymbol(node, CancellationToken), Symbol)) + && SymbolEqualityComparer.Default.Equals(SemanticModel.GetSymbol(node, CancellationToken), Symbol)) { Result = true; } @@ -87,55 +93,10 @@ public static bool IsReferencedAsMethodGroup( SemanticModel semanticModel, CancellationToken cancellationToken) { - var result = false; - - MethodReferencedAsMethodGroupWalker? walker = null; - - try - { - walker = GetInstance(); - - walker.Symbol = methodSymbol; - walker.SemanticModel = semanticModel; - walker.CancellationToken = cancellationToken; - - walker.Visit(node); - - result = walker.Result; - } - finally - { - if (walker is not null) - Free(walker); - } - - return result; - } - - private static MethodReferencedAsMethodGroupWalker GetInstance() - { - MethodReferencedAsMethodGroupWalker? walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.Symbol is null); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - return walker; - } + var walker = new MethodReferencedAsMethodGroupWalker(methodSymbol, semanticModel, cancellationToken); - return new MethodReferencedAsMethodGroupWalker(); - } - - private static void Free(MethodReferencedAsMethodGroupWalker walker) - { - walker.Result = false; - walker.Symbol = null; - walker.SemanticModel = null; - walker.CancellationToken = default; + walker.Visit(node); - _cachedInstance = walker; + return walker.Result; } } diff --git a/src/CodeAnalysis.Analyzers/CSharp/UsePatternMatchingAnalyzer.cs b/src/CodeAnalysis.Analyzers/CSharp/UsePatternMatchingAnalyzer.cs index 9877f70866..7f5acf9b3e 100644 --- a/src/CodeAnalysis.Analyzers/CSharp/UsePatternMatchingAnalyzer.cs +++ b/src/CodeAnalysis.Analyzers/CSharp/UsePatternMatchingAnalyzer.cs @@ -361,37 +361,24 @@ private static bool IsLocalVariableReferenced( if (localSymbol.IsKind(SymbolKind.Local)) { - bool isReferenced; - ContainsLocalOrParameterReferenceWalker walker = null; + var walker = new ContainsLocalOrParameterReferenceWalker(localSymbol, context.SemanticModel, context.CancellationToken); - try - { - walker = ContainsLocalOrParameterReferenceWalker.GetInstance(localSymbol, context.SemanticModel, context.CancellationToken); + walker.VisitList(switchStatement.Sections); - walker.VisitList(switchStatement.Sections); + if (!walker.Result) + { + StatementListInfo statementsInfo = SyntaxInfo.StatementListInfo(switchStatement); - if (!walker.Result) + if (statementsInfo.Success) { - StatementListInfo statementsInfo = SyntaxInfo.StatementListInfo(switchStatement); - - if (statementsInfo.Success) - { - int index = statementsInfo.IndexOf(switchStatement); + int index = statementsInfo.IndexOf(switchStatement); - if (index < statementsInfo.Count - 1) - walker.VisitList(statementsInfo.Statements, index + 1); - } + if (index < statementsInfo.Count - 1) + walker.VisitList(statementsInfo.Statements, index + 1); } - - isReferenced = walker.Result; - } - finally - { - if (walker is not null) - ContainsLocalOrParameterReferenceWalker.Free(walker); } - return isReferenced; + return walker.Result; } return false; diff --git a/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs b/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs index 59d438ab97..72526d8835 100644 --- a/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs +++ b/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs @@ -13,14 +13,18 @@ namespace Roslynator.CSharp.Analysis; internal readonly struct RemoveAsyncAwaitAnalysis : IDisposable { - private RemoveAsyncAwaitAnalysis(AwaitExpressionWalker walker) + private readonly PooledObject _pooledWalker; + + private RemoveAsyncAwaitAnalysis(PooledObject pooledWalker) { - Walker = walker; + _pooledWalker = pooledWalker; + Walker = pooledWalker.Value; AwaitExpression = null; } private RemoveAsyncAwaitAnalysis(AwaitExpressionSyntax awaitExpression) { + _pooledWalker = default; AwaitExpression = awaitExpression; Walker = null; } @@ -181,14 +185,14 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( if (awaitExpression is null) return default; - AwaitExpressionWalker walker = VisitStatements(); + PooledObject pooledWalker = VisitStatements(); - HashSet awaitExpressions = walker.AwaitExpressions; + HashSet awaitExpressions = pooledWalker.Value.AwaitExpressions; if (awaitExpressions.Count == 1) { if (VerifyTypes(node, awaitExpression, semanticModel, cancellationToken)) - return new RemoveAsyncAwaitAnalysis(walker); + return new RemoveAsyncAwaitAnalysis(pooledWalker); } else if (awaitExpressions.Count > 1) { @@ -201,7 +205,7 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( if (VerifyIfStatement((IfStatementSyntax)prevStatement, awaitExpressions.Count - 1, endsWithElse: false) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return new RemoveAsyncAwaitAnalysis(walker); + return new RemoveAsyncAwaitAnalysis(pooledWalker); } break; @@ -211,7 +215,7 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( if (VerifySwitchStatement((SwitchStatementSyntax)prevStatement, awaitExpressions.Count - 1, containsDefaultSection: false) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return new RemoveAsyncAwaitAnalysis(walker); + return new RemoveAsyncAwaitAnalysis(pooledWalker); } break; @@ -219,49 +223,52 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( } } + pooledWalker.Dispose(); return default; } case SyntaxKind.IfStatement: { - AwaitExpressionWalker walker = VisitStatements(); + PooledObject pooledWalker = VisitStatements(); - HashSet awaitExpressions = walker.AwaitExpressions; + HashSet awaitExpressions = pooledWalker.Value.AwaitExpressions; if (awaitExpressions.Count > 0 && VerifyIfStatement((IfStatementSyntax)statement, awaitExpressions.Count, endsWithElse: true) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return new RemoveAsyncAwaitAnalysis(walker); + return new RemoveAsyncAwaitAnalysis(pooledWalker); } + pooledWalker.Dispose(); return default; } case SyntaxKind.SwitchStatement: { - AwaitExpressionWalker walker = VisitStatements(); + PooledObject pooledWalker = VisitStatements(); - HashSet awaitExpressions = walker.AwaitExpressions; + HashSet awaitExpressions = pooledWalker.Value.AwaitExpressions; if (awaitExpressions.Count > 0 && VerifySwitchStatement((SwitchStatementSyntax)statement, awaitExpressions.Count, containsDefaultSection: true) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return new RemoveAsyncAwaitAnalysis(walker); + return new RemoveAsyncAwaitAnalysis(pooledWalker); } + pooledWalker.Dispose(); return default; } } return default; - AwaitExpressionWalker VisitStatements() + PooledObject VisitStatements() { - AwaitExpressionWalker walker = AwaitExpressionWalker.GetInstance(); + PooledObject pooledWalker = ObjectPool.Rent(); - walker.VisitStatements(statements, statement); + pooledWalker.Value.VisitStatements(statements, statement); - return walker; + return pooledWalker; } } @@ -451,8 +458,6 @@ private static IMethodSymbol GetMethodSymbol( public void Dispose() { if (Walker is not null) - { - AwaitExpressionWalker.Free(Walker); - } + _pooledWalker.Dispose(); } } diff --git a/src/Common/CSharp/Analysis/RemoveRedundantStatement/RemoveRedundantYieldBreakStatementAnalysis.cs b/src/Common/CSharp/Analysis/RemoveRedundantStatement/RemoveRedundantYieldBreakStatementAnalysis.cs index 908f1b0bb2..5d82a05cd2 100644 --- a/src/Common/CSharp/Analysis/RemoveRedundantStatement/RemoveRedundantYieldBreakStatementAnalysis.cs +++ b/src/Common/CSharp/Analysis/RemoveRedundantStatement/RemoveRedundantYieldBreakStatementAnalysis.cs @@ -29,9 +29,7 @@ protected override bool IsFixable(StatementSyntax statement, StatementSyntax con if (object.ReferenceEquals(statements.SingleOrDefault(ignoreLocalFunctions: true, shouldThrow: false), containingStatement)) return false; - ContainsYieldWalker walker = ContainsYieldWalker.GetInstance(); - - var success = false; + var walker = new ContainsYieldWalker(); int index = statements.IndexOf(containingStatement); @@ -39,14 +37,10 @@ protected override bool IsFixable(StatementSyntax statement, StatementSyntax con { walker.VisitStatement(statements[i]); - success = walker.YieldStatement is not null; - - if (success) - break; + if (walker.YieldStatement is not null) + return true; } - ContainsYieldWalker.Free(walker); - - return success; + return false; } } diff --git a/src/Common/CSharp/Analysis/UseConstantInsteadOfFieldAnalysis.cs b/src/Common/CSharp/Analysis/UseConstantInsteadOfFieldAnalysis.cs index 2cab51d568..a6917253a0 100644 --- a/src/Common/CSharp/Analysis/UseConstantInsteadOfFieldAnalysis.cs +++ b/src/Common/CSharp/Analysis/UseConstantInsteadOfFieldAnalysis.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Diagnostics; using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -112,28 +111,11 @@ public static bool IsFixable( if (body is not null) { - bool canBeConvertedToConstant; - UseConstantInsteadOfFieldWalker walker = null; + var walker = new UseConstantInsteadOfFieldWalker(fieldSymbol, semanticModel, cancellationToken); - try - { - walker = UseConstantInsteadOfFieldWalker.GetInstance(); + walker.VisitBlock(body); - walker.FieldSymbol = fieldSymbol; - walker.SemanticModel = semanticModel; - walker.CancellationToken = cancellationToken; - - walker.VisitBlock(body); - - canBeConvertedToConstant = walker.CanBeConvertedToConstant; - } - finally - { - if (walker is not null) - UseConstantInsteadOfFieldWalker.Free(walker); - } - - if (!canBeConvertedToConstant) + if (!walker.CanBeConvertedToConstant) return false; } } @@ -144,16 +126,23 @@ public static bool IsFixable( private class UseConstantInsteadOfFieldWalker : AssignedExpressionWalker { - [ThreadStatic] - private static UseConstantInsteadOfFieldWalker _cachedInstance; + public UseConstantInsteadOfFieldWalker( + IFieldSymbol fieldSymbol, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + FieldSymbol = fieldSymbol; + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } - public IFieldSymbol FieldSymbol { get; set; } + public IFieldSymbol FieldSymbol { get; } - public SemanticModel SemanticModel { get; set; } + public SemanticModel SemanticModel { get; } - public CancellationToken CancellationToken { get; set; } + public CancellationToken CancellationToken { get; } - public bool CanBeConvertedToConstant { get; set; } + public bool CanBeConvertedToConstant { get; private set; } = true; protected override bool ShouldVisit { @@ -204,32 +193,5 @@ public override void VisitArgument(ArgumentSyntax node) base.VisitArgument(node); } - - public static UseConstantInsteadOfFieldWalker GetInstance() - { - UseConstantInsteadOfFieldWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.FieldSymbol is null); - Debug.Assert(walker.SemanticModel is null); - Debug.Assert(walker.CancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new UseConstantInsteadOfFieldWalker(); - } - - public static void Free(UseConstantInsteadOfFieldWalker walker) - { - walker.FieldSymbol = null; - walker.SemanticModel = null; - walker.CancellationToken = default; - walker.CanBeConvertedToConstant = true; - - _cachedInstance = walker; - } } } diff --git a/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs b/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs index 67e2505c20..438e730fae 100644 --- a/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs +++ b/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs @@ -1,6 +1,5 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Collections.Generic; using System.Diagnostics; using Microsoft.CodeAnalysis; @@ -8,12 +7,10 @@ namespace Roslynator.CSharp.SyntaxWalkers; -internal class AwaitExpressionWalker : BaseCSharpSyntaxWalker +internal class AwaitExpressionWalker : BaseCSharpSyntaxWalker, IResettable { - [ThreadStatic] - private static AwaitExpressionWalker _cachedInstance; - private bool _shouldVisit = true; + private bool _canBeCached = true; public HashSet AwaitExpressions { get; } = []; @@ -21,8 +18,11 @@ internal class AwaitExpressionWalker : BaseCSharpSyntaxWalker protected override bool ShouldVisit => _shouldVisit; + public bool CanBeCached => _canBeCached; + public void Reset() { + _canBeCached = AwaitExpressions.Count <= ObjectPool.MaxCachedBufferSize; _shouldVisit = true; StopOnFirstAwaitExpression = false; AwaitExpressions.Clear(); @@ -30,18 +30,16 @@ public void Reset() public static bool ContainsAwaitExpression(ExpressionSyntax expression) { - AwaitExpressionWalker walker = GetInstance(); + using PooledObject pooledWalker = ObjectPool.Rent(); + + AwaitExpressionWalker walker = pooledWalker.Value; walker.StopOnFirstAwaitExpression = true; walker.Visit(expression); Debug.Assert(walker.AwaitExpressions.Count <= 1); - bool result = walker.AwaitExpressions.Count == 1; - - Free(walker); - - return result; + return walker.AwaitExpressions.Count == 1; } public void VisitStatements(SyntaxList statements, StatementSyntax lastStatement) @@ -107,26 +105,4 @@ public override void VisitParenthesizedLambdaExpression(ParenthesizedLambdaExpre public override void VisitLocalFunctionStatement(LocalFunctionStatementSyntax node) { } - - public static AwaitExpressionWalker GetInstance() - { - AwaitExpressionWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.AwaitExpressions.Count == 0); - - _cachedInstance = null; - return walker; - } - - return new AwaitExpressionWalker(); - } - - public static void Free(AwaitExpressionWalker walker) - { - walker.Reset(); - - _cachedInstance = walker; - } } diff --git a/src/Core/IResettable.cs b/src/Core/IResettable.cs new file mode 100644 index 0000000000..e07611471b --- /dev/null +++ b/src/Core/IResettable.cs @@ -0,0 +1,14 @@ +// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Roslynator; + +internal interface IResettable +{ + void Reset(); + + /// + /// Gets a value indicating whether the instance can be retained by . + /// Implementations should return false when the instance holds a buffer that grew too large to be worth retaining. + /// + bool CanBeCached { get; } +} diff --git a/src/Core/ObjectPool.cs b/src/Core/ObjectPool.cs new file mode 100644 index 0000000000..1604db08ca --- /dev/null +++ b/src/Core/ObjectPool.cs @@ -0,0 +1,13 @@ +// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Roslynator; + +internal static class ObjectPool +{ + /// + /// Maximum number of items a pooled object's buffer may have held to be worth retaining. + /// Clearing a collection does not shrink its backing array, so an instance that grew beyond + /// this size would keep that array alive for the lifetime of the thread. + /// + public const int MaxCachedBufferSize = 256; +} diff --git a/src/Core/ObjectPool`1.cs b/src/Core/ObjectPool`1.cs new file mode 100644 index 0000000000..5ab0ed66e9 --- /dev/null +++ b/src/Core/ObjectPool`1.cs @@ -0,0 +1,31 @@ +// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Diagnostics; + +namespace Roslynator; + +internal static class ObjectPool where T : class, IResettable, new() +{ + [ThreadStatic] + private static T? _cachedInstance; + + public static PooledObject Rent() + { + T? instance = _cachedInstance; + + _cachedInstance = null; + + return new PooledObject(instance ?? new T()); + } + + internal static void Free(T instance) + { + Debug.Assert(!ReferenceEquals(_cachedInstance, instance), $"'{typeof(T).Name}' freed twice."); + + instance.Reset(); + + if (instance.CanBeCached) + _cachedInstance = instance; + } +} diff --git a/src/Core/PooledObject`1.cs b/src/Core/PooledObject`1.cs new file mode 100644 index 0000000000..5cf86a2a70 --- /dev/null +++ b/src/Core/PooledObject`1.cs @@ -0,0 +1,20 @@ +// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; + +namespace Roslynator; + +internal readonly struct PooledObject : IDisposable where T : class, IResettable, new() +{ + internal PooledObject(T value) + { + Value = value; + } + + public T Value { get; } + + public void Dispose() + { + ObjectPool.Free(Value); + } +} diff --git a/src/Formatting.Analyzers/CSharp/UseSpacesInsteadOfTabAnalyzer.cs b/src/Formatting.Analyzers/CSharp/UseSpacesInsteadOfTabAnalyzer.cs index 4db7ea1461..65f3985208 100644 --- a/src/Formatting.Analyzers/CSharp/UseSpacesInsteadOfTabAnalyzer.cs +++ b/src/Formatting.Analyzers/CSharp/UseSpacesInsteadOfTabAnalyzer.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Collections.Immutable; -using System.Diagnostics; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Diagnostics; @@ -41,25 +39,20 @@ private static void AnalyzeSyntaxTree(SyntaxTreeAnalysisContext context) if (!tree.TryGetRoot(out SyntaxNode root)) return; - UseSpacesInsteadOfTabWalker walker = UseSpacesInsteadOfTabWalker.GetInstance(); + var walker = new UseSpacesInsteadOfTabWalker(context); - walker.AnalysisContext = context; walker.Visit(root); - - UseSpacesInsteadOfTabWalker.Free(walker); } private class UseSpacesInsteadOfTabWalker : CSharpSyntaxWalker { - [ThreadStatic] - private static UseSpacesInsteadOfTabWalker _cachedInstance; - - public UseSpacesInsteadOfTabWalker() + public UseSpacesInsteadOfTabWalker(SyntaxTreeAnalysisContext analysisContext) : base(SyntaxWalkerDepth.StructuredTrivia) { + AnalysisContext = analysisContext; } - public SyntaxTreeAnalysisContext AnalysisContext { get; set; } + public SyntaxTreeAnalysisContext AnalysisContext { get; } public override void VisitTrivia(SyntaxTrivia trivia) { @@ -86,28 +79,5 @@ public override void VisitTrivia(SyntaxTrivia trivia) } } } - - public static UseSpacesInsteadOfTabWalker GetInstance() - { - UseSpacesInsteadOfTabWalker walker = _cachedInstance; - - if (walker is not null) - { - Debug.Assert(walker.AnalysisContext.Tree is null); - Debug.Assert(walker.AnalysisContext.CancellationToken == default); - - _cachedInstance = null; - return walker; - } - - return new UseSpacesInsteadOfTabWalker(); - } - - public static void Free(UseSpacesInsteadOfTabWalker walker) - { - walker.AnalysisContext = default; - - _cachedInstance = walker; - } } } diff --git a/src/Refactorings/CSharp/Refactorings/ConvertWhileToForRefactoring.cs b/src/Refactorings/CSharp/Refactorings/ConvertWhileToForRefactoring.cs index 5eae162eb7..ad49a3364f 100644 --- a/src/Refactorings/CSharp/Refactorings/ConvertWhileToForRefactoring.cs +++ b/src/Refactorings/CSharp/Refactorings/ConvertWhileToForRefactoring.cs @@ -116,38 +116,25 @@ private static int FindLocalDeclarationStatementIndex( return resultIndex; } - ContainsLocalOrParameterReferenceWalker walker = null; + var walker = new ContainsLocalOrParameterReferenceWalker(symbol, semanticModel, cancellationToken); - try + if (mustBeReferencedInsideWhileStatement) { - walker = ContainsLocalOrParameterReferenceWalker.GetInstance(symbol, semanticModel, cancellationToken); + walker.VisitWhileStatement(whileStatement); - if (mustBeReferencedInsideWhileStatement) - { - walker.VisitWhileStatement(whileStatement); - - if (!walker.Result) - { - ContainsLocalOrParameterReferenceWalker.Free(walker); - return resultIndex; - } - } + if (!walker.Result) + return resultIndex; + } - walker.Result = false; + walker.Result = false; - if (whileStatementIndex == -1) - whileStatementIndex = statements.IndexOf(whileStatement); + if (whileStatementIndex == -1) + whileStatementIndex = statements.IndexOf(whileStatement); - walker.VisitList(statements, whileStatementIndex + 1); + walker.VisitList(statements, whileStatementIndex + 1); - if (walker.Result) - return resultIndex; - } - finally - { - if (walker is not null) - ContainsLocalOrParameterReferenceWalker.Free(walker); - } + if (walker.Result) + return resultIndex; resultIndex = i; } diff --git a/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs b/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs index dddf8a6e18..b11cad37a3 100644 --- a/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs +++ b/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs @@ -414,8 +414,9 @@ public async Task ComputeRefactoringsForNodeAsync() if (node is null) return; - RefactoringFlags flags = RefactoringFlagsCache.GetInstance(); - flags.Reset(); + using PooledObject pooledFlags = ObjectPool.Rent(); + + RefactoringFlags flags = pooledFlags.Value; SyntaxNode firstNode = node; @@ -1036,14 +1037,12 @@ public async Task ComputeRefactoringsForNodeAsync() } } - RefactoringFlagsCache.Free(flags); - await SelectedLinesRefactoring.ComputeRefactoringsAsync(this, firstNode).ConfigureAwait(false); CommentTriviaRefactoring.ComputeRefactorings(this, firstNode); } - private class RefactoringFlags + private class RefactoringFlags : IResettable { private readonly BitArray _flags; @@ -1052,6 +1051,9 @@ public RefactoringFlags() _flags = new BitArray((int)Flag.Count); } + // The bit array has a fixed size, so the instance never outgrows the pool. + public bool CanBeCached => true; + public bool IsSet(Flag flag) { return _flags.Get((int)flag); @@ -1068,30 +1070,6 @@ public void Reset() } } - private static class RefactoringFlagsCache - { - [ThreadStatic] - private static RefactoringFlags _cachedInstance; - - public static RefactoringFlags GetInstance() - { - RefactoringFlags instance = _cachedInstance; - - if (instance is not null) - { - _cachedInstance = null; - return instance; - } - - return new RefactoringFlags(); - } - - public static void Free(RefactoringFlags instance) - { - _cachedInstance = instance; - } - } - private enum Flag { None = 0, diff --git a/src/Tests/Analyzers.Tests/RCS1187UseConstantInsteadOfFieldTests.cs b/src/Tests/Analyzers.Tests/RCS1187UseConstantInsteadOfFieldTests.cs index 71439070a6..fcd81fd2fa 100644 --- a/src/Tests/Analyzers.Tests/RCS1187UseConstantInsteadOfFieldTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1187UseConstantInsteadOfFieldTests.cs @@ -12,6 +12,32 @@ public class RCS1187UseConstantInsteadOfFieldTests : AbstractCSharpDiagnosticVer { public override DiagnosticDescriptor Descriptor { get; } = DiagnosticRules.UseConstantInsteadOfField; + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UseConstantInsteadOfField)] + public async Task Test_StaticConstructorThatDoesNotAssignField() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + [|private static readonly int _f = 1;|] + + static C() + { + var x = 1; + } +} +", @" +class C +{ + private const int _f = 1; + + static C() + { + var x = 1; + } +} +"); + } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UseConstantInsteadOfField)] public async Task TestNoDiagnostic_AssignmentInInStaticConstructor() { From b8978636006d9db892638e545fcfe3e055f491c7 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sat, 15 Aug 2026 01:12:59 +0200 Subject: [PATCH 2/7] fix: remove trailing blank lines flagged by RCS1036 Co-authored-by: Cursor --- src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs | 1 - .../CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs | 1 - .../SyntaxWalkers/ContainsLocalOrParameterReferenceWalker.cs | 1 - 3 files changed, 3 deletions(-) diff --git a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs index fbe1120122..c85d9517aa 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs @@ -603,5 +603,4 @@ private void VisitAttributeLists(SyntaxList attributeLists) VisitAttributeList(attributeList); } } - } diff --git a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs index 654170079d..f62da8db3a 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs @@ -204,5 +204,4 @@ public override void VisitTypeParameterConstraintClause(TypeParameterConstraintC base.VisitTypeParameterConstraintClause(node); } } - } diff --git a/src/CSharp/CSharp/SyntaxWalkers/ContainsLocalOrParameterReferenceWalker.cs b/src/CSharp/CSharp/SyntaxWalkers/ContainsLocalOrParameterReferenceWalker.cs index 379f773fc7..bc50ebe709 100644 --- a/src/CSharp/CSharp/SyntaxWalkers/ContainsLocalOrParameterReferenceWalker.cs +++ b/src/CSharp/CSharp/SyntaxWalkers/ContainsLocalOrParameterReferenceWalker.cs @@ -114,5 +114,4 @@ public void VisitList(SeparatedSyntaxList statements, int startInd break; } } - } From 68a91239d7114c826e4b0e49acf55229ebae04e9 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sat, 15 Aug 2026 01:17:13 +0200 Subject: [PATCH 3/7] fix: reword comment that spellcheck flagged as a typo Co-authored-by: Cursor --- src/Refactorings/CSharp/Refactorings/RefactoringContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs b/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs index b11cad37a3..dfc537ffc5 100644 --- a/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs +++ b/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs @@ -1051,7 +1051,7 @@ public RefactoringFlags() _flags = new BitArray((int)Flag.Count); } - // The bit array has a fixed size, so the instance never outgrows the pool. + // The bit array has a fixed size, so the instance never grows beyond the pool limit. public bool CanBeCached => true; public bool IsSet(Flag flag) From de601a7900487dce2a256ad94662d89ded643891 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sun, 16 Aug 2026 16:41:20 +0200 Subject: [PATCH 4/7] fix: drop oversized UseAsyncAwaitWalker buffers and harden PooledObject CanBeCached was checking List.Count after a balanced visit (always ~0), so oversized walkers were never discarded; use Capacity instead. Make PooledObject.Dispose idempotent, and align UnusedMemberWalker with Initialize. Co-authored-by: Cursor --- .../UnusedMember/UnusedMemberAnalyzer.cs | 3 +-- .../Analysis/UnusedMember/UnusedMemberWalker.cs | 10 ++++++++-- .../CSharp/Analysis/UseAsyncAwaitAnalyzer.cs | 3 ++- src/Core/PooledObject`1.cs | 16 ++++++++++++---- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs index f097210fe8..24a0376bac 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs @@ -200,8 +200,7 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) if (nodes.Count > 0) { - walker.SemanticModel = semanticModel; - walker.CancellationToken = cancellationToken; + walker.Initialize(semanticModel, cancellationToken); walker.Visit(typeDeclaration); diff --git a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs index c85d9517aa..07c959c60e 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs @@ -20,9 +20,9 @@ internal class UnusedMemberWalker : TypeSyntaxWalker, IResettable public Collection Nodes { get; } = []; - public SemanticModel SemanticModel { get; set; } + public SemanticModel SemanticModel { get; private set; } - public CancellationToken CancellationToken { get; set; } + public CancellationToken CancellationToken { get; private set; } public bool IsAnyNodeConst { get; private set; } @@ -35,6 +35,12 @@ protected override bool ShouldVisit public bool CanBeCached => _canBeCached; + public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) + { + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } + public void Reset() { _canBeCached = Nodes.Count <= ObjectPool.MaxCachedBufferSize; diff --git a/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs index 96d4c7f564..ecb7803495 100644 --- a/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs @@ -332,7 +332,8 @@ public void Initialize(SemanticModel semanticModel, CancellationToken cancellati public void Reset() { - _canBeCached = _usingDeclarations.Count <= ObjectPool.MaxCachedBufferSize; + // Count is always 0 after a balanced visit; Capacity is the retained buffer size. + _canBeCached = _usingDeclarations.Capacity <= ObjectPool.MaxCachedBufferSize; _shouldVisit = true; _usingOrTryStatementDepth = 0; diff --git a/src/Core/PooledObject`1.cs b/src/Core/PooledObject`1.cs index 5cf86a2a70..8aa31a47e9 100644 --- a/src/Core/PooledObject`1.cs +++ b/src/Core/PooledObject`1.cs @@ -4,17 +4,25 @@ namespace Roslynator; -internal readonly struct PooledObject : IDisposable where T : class, IResettable, new() +internal struct PooledObject : IDisposable where T : class, IResettable, new() { + private T? _value; + internal PooledObject(T value) { - Value = value; + _value = value; } - public T Value { get; } + public readonly T Value => _value!; public void Dispose() { - ObjectPool.Free(Value); + T? value = _value; + + if (value is null) + return; + + _value = null; + ObjectPool.Free(value); } } From fde7d1322ca5a5b085a02da56d85334c68c41e15 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sun, 16 Aug 2026 17:06:39 +0200 Subject: [PATCH 5/7] fix: make the object pool copy-safe and fold cache eligibility into Reset PooledObject is now a ref struct so it cannot be stored in a field. Reset returns whether to cache, Free is a no-op on null or an already-cached instance, and RemoveAsyncAwaitAnalysis takes the walker out of the rental instead of embedding PooledObject. Co-authored-by: Cursor --- .../MakeMemberReadOnlyWalker.cs | 9 +- .../MarkLocalVariableAsConstWalker.cs | 10 +-- .../Analysis/RefReadOnlyParameterAnalyzer.cs | 9 +- .../UnusedMember/UnusedMemberWalker.cs | 9 +- .../UnusedParameter/UnusedParameterWalker.cs | 9 +- .../CSharp/Analysis/UseAsyncAwaitAnalyzer.cs | 9 +- .../Analysis/RemoveAsyncAwaitAnalysis.cs | 49 ++++++++--- .../SyntaxWalkers/AwaitExpressionWalker.cs | 8 +- src/Core/IResettable.cs | 9 +- src/Core/ObjectPool`1.cs | 18 ++-- src/Core/PooledObject`1.cs | 4 +- .../CSharp/Refactorings/RefactoringContext.cs | 21 +++-- src/Tests/Core.Tests/ObjectPoolTests.cs | 87 +++++++++++++++++++ 13 files changed, 180 insertions(+), 71 deletions(-) create mode 100644 src/Tests/Core.Tests/ObjectPoolTests.cs diff --git a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs index 52429a5936..e75817ca84 100644 --- a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs +++ b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs @@ -18,7 +18,6 @@ internal class MakeMemberReadOnlyWalker : AssignedExpressionWalker, IResettable private int _anonymousFunctionDepth; private bool _isInInstanceConstructor; private bool _isInStaticConstructor; - private bool _canBeCached = true; public SemanticModel SemanticModel { get; private set; } @@ -26,17 +25,15 @@ internal class MakeMemberReadOnlyWalker : AssignedExpressionWalker, IResettable public Dictionary Symbols { get; } = []; - public bool CanBeCached => _canBeCached; - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) { SemanticModel = semanticModel; CancellationToken = cancellationToken; } - public void Reset() + public bool Reset() { - _canBeCached = Symbols.Count <= ObjectPool.MaxCachedBufferSize; + bool canBeCached = Symbols.Count <= ObjectPool.MaxCachedBufferSize; Symbols.Clear(); SemanticModel = null; @@ -46,6 +43,8 @@ public void Reset() _anonymousFunctionDepth = 0; _isInInstanceConstructor = false; _isInStaticConstructor = false; + + return canBeCached; } public override void VisitAssignedExpression(ExpressionSyntax expression) diff --git a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs index eed6f591d8..ef7eb7dec8 100644 --- a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs +++ b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs @@ -12,8 +12,6 @@ namespace Roslynator.CSharp.Analysis.MarkLocalVariableAsConst; internal class MarkLocalVariableAsConstWalker : AssignedExpressionWalker, IResettable { - private bool _canBeCached = true; - public Dictionary Identifiers { get; } = []; public SemanticModel SemanticModel { get; private set; } @@ -24,22 +22,22 @@ internal class MarkLocalVariableAsConstWalker : AssignedExpressionWalker, IReset protected override bool ShouldVisit => !Result; - public bool CanBeCached => _canBeCached; - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) { SemanticModel = semanticModel; CancellationToken = cancellationToken; } - public void Reset() + public bool Reset() { - _canBeCached = Identifiers.Count <= ObjectPool.MaxCachedBufferSize; + bool canBeCached = Identifiers.Count <= ObjectPool.MaxCachedBufferSize; Identifiers.Clear(); SemanticModel = null; CancellationToken = default; Result = false; + + return canBeCached; } public override void VisitAssignedExpression(ExpressionSyntax expression) diff --git a/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs index e242cb0195..709ade9efd 100644 --- a/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs @@ -246,7 +246,6 @@ private class RefReadOnlyParameterWalker : BaseCSharpSyntaxWalker, IResettable { private int _localFunctionDepth; private int _anonymousFunctionDepth; - private bool _canBeCached = true; public Dictionary Parameters { get; } = []; @@ -254,23 +253,23 @@ private class RefReadOnlyParameterWalker : BaseCSharpSyntaxWalker, IResettable public CancellationToken CancellationToken { get; private set; } - public bool CanBeCached => _canBeCached; - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) { SemanticModel = semanticModel; CancellationToken = cancellationToken; } - public void Reset() + public bool Reset() { - _canBeCached = Parameters.Count <= ObjectPool.MaxCachedBufferSize; + bool canBeCached = Parameters.Count <= ObjectPool.MaxCachedBufferSize; Parameters.Clear(); SemanticModel = null; CancellationToken = default; _localFunctionDepth = 0; _anonymousFunctionDepth = 0; + + return canBeCached; } protected override bool ShouldVisit => Parameters.Count > 0; diff --git a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs index 07c959c60e..6907fb5f43 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs @@ -14,7 +14,6 @@ namespace Roslynator.CSharp.Analysis.UnusedMember; internal class UnusedMemberWalker : TypeSyntaxWalker, IResettable { private bool _isEmpty; - private bool _canBeCached = true; private IMethodSymbol _containingMethodSymbol; @@ -33,17 +32,15 @@ protected override bool ShouldVisit get { return !_isEmpty; } } - public bool CanBeCached => _canBeCached; - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) { SemanticModel = semanticModel; CancellationToken = cancellationToken; } - public void Reset() + public bool Reset() { - _canBeCached = Nodes.Count <= ObjectPool.MaxCachedBufferSize; + bool canBeCached = Nodes.Count <= ObjectPool.MaxCachedBufferSize; _isEmpty = false; _containingMethodSymbol = null; @@ -52,6 +49,8 @@ public void Reset() CancellationToken = default; IsAnyNodeConst = false; IsAnyNodeDelegate = false; + + return canBeCached; } public void AddDelegate(string name, SyntaxNode node) diff --git a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs index f62da8db3a..fb8fa85909 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs @@ -15,7 +15,6 @@ internal class UnusedParameterWalker : TypeSyntaxWalker, IResettable private static readonly StringComparer _ordinalComparer = StringComparer.Ordinal; private bool _isEmpty; - private bool _canBeCached = true; public Dictionary Nodes { get; } = new(_ordinalComparer); @@ -29,8 +28,6 @@ internal class UnusedParameterWalker : TypeSyntaxWalker, IResettable protected override bool ShouldVisit => !_isEmpty; - public bool CanBeCached => _canBeCached; - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken, bool isIndexer = false) { SemanticModel = semanticModel; @@ -38,9 +35,9 @@ public void Initialize(SemanticModel semanticModel, CancellationToken cancellati IsIndexer = isIndexer; } - public void Reset() + public bool Reset() { - _canBeCached = Nodes.Count <= ObjectPool.MaxCachedBufferSize; + bool canBeCached = Nodes.Count <= ObjectPool.MaxCachedBufferSize; _isEmpty = false; Nodes.Clear(); @@ -48,6 +45,8 @@ public void Reset() CancellationToken = default; IsIndexer = false; IsAnyTypeParameter = false; + + return canBeCached; } public void AddParameter(ParameterSyntax parameter) diff --git a/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs index ecb7803495..34e6e27594 100644 --- a/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs @@ -171,12 +171,9 @@ private class UseAsyncAwaitWalker : StatementWalker, IResettable private readonly List _usingDeclarations = []; private int _usingOrTryStatementDepth; private bool _shouldVisit = true; - private bool _canBeCached = true; public override bool ShouldVisit => _shouldVisit; - public bool CanBeCached => _canBeCached; - public ReturnStatementSyntax ReturnStatement { get; private set; } public SemanticModel SemanticModel { get; private set; } @@ -330,10 +327,10 @@ public void Initialize(SemanticModel semanticModel, CancellationToken cancellati CancellationToken = cancellationToken; } - public void Reset() + public bool Reset() { // Count is always 0 after a balanced visit; Capacity is the retained buffer size. - _canBeCached = _usingDeclarations.Capacity <= ObjectPool.MaxCachedBufferSize; + bool canBeCached = _usingDeclarations.Capacity <= ObjectPool.MaxCachedBufferSize; _shouldVisit = true; _usingOrTryStatementDepth = 0; @@ -341,6 +338,8 @@ public void Reset() ReturnStatement = null; SemanticModel = null; CancellationToken = default; + + return canBeCached; } } } diff --git a/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs b/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs index 72526d8835..e067b9eff7 100644 --- a/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs +++ b/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs @@ -13,27 +13,25 @@ namespace Roslynator.CSharp.Analysis; internal readonly struct RemoveAsyncAwaitAnalysis : IDisposable { - private readonly PooledObject _pooledWalker; + private readonly Holder _holder; - private RemoveAsyncAwaitAnalysis(PooledObject pooledWalker) + private RemoveAsyncAwaitAnalysis(AwaitExpressionWalker walker) { - _pooledWalker = pooledWalker; - Walker = pooledWalker.Value; + _holder = new Holder(walker); AwaitExpression = null; } private RemoveAsyncAwaitAnalysis(AwaitExpressionSyntax awaitExpression) { - _pooledWalker = default; + _holder = null; AwaitExpression = awaitExpression; - Walker = null; } public bool Success => AwaitExpression is not null || Walker?.AwaitExpressions.Count > 0; public AwaitExpressionSyntax AwaitExpression { get; } - public AwaitExpressionWalker Walker { get; } + public AwaitExpressionWalker Walker => _holder?.Walker; public static RemoveAsyncAwaitAnalysis Create( MethodDeclarationSyntax methodDeclaration, @@ -192,7 +190,7 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( if (awaitExpressions.Count == 1) { if (VerifyTypes(node, awaitExpression, semanticModel, cancellationToken)) - return new RemoveAsyncAwaitAnalysis(pooledWalker); + return Take(ref pooledWalker); } else if (awaitExpressions.Count > 1) { @@ -205,7 +203,7 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( if (VerifyIfStatement((IfStatementSyntax)prevStatement, awaitExpressions.Count - 1, endsWithElse: false) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return new RemoveAsyncAwaitAnalysis(pooledWalker); + return Take(ref pooledWalker); } break; @@ -215,7 +213,7 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( if (VerifySwitchStatement((SwitchStatementSyntax)prevStatement, awaitExpressions.Count - 1, containsDefaultSection: false) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return new RemoveAsyncAwaitAnalysis(pooledWalker); + return Take(ref pooledWalker); } break; @@ -236,7 +234,7 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( && VerifyIfStatement((IfStatementSyntax)statement, awaitExpressions.Count, endsWithElse: true) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return new RemoveAsyncAwaitAnalysis(pooledWalker); + return Take(ref pooledWalker); } pooledWalker.Dispose(); @@ -252,7 +250,7 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( && VerifySwitchStatement((SwitchStatementSyntax)statement, awaitExpressions.Count, containsDefaultSection: true) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return new RemoveAsyncAwaitAnalysis(pooledWalker); + return Take(ref pooledWalker); } pooledWalker.Dispose(); @@ -272,6 +270,13 @@ PooledObject VisitStatements() } } + private static RemoveAsyncAwaitAnalysis Take(ref PooledObject pooledWalker) + { + AwaitExpressionWalker walker = pooledWalker.Value; + pooledWalker = default; + return new RemoveAsyncAwaitAnalysis(walker); + } + private static bool VerifyIfStatement( IfStatementSyntax ifStatement, int expectedCount, @@ -457,7 +462,23 @@ private static IMethodSymbol GetMethodSymbol( public void Dispose() { - if (Walker is not null) - _pooledWalker.Dispose(); + Holder holder = _holder; + AwaitExpressionWalker walker = holder?.Walker; + + if (walker is null) + return; + + holder.Walker = null; + ObjectPool.Free(walker); + } + + private sealed class Holder + { + public Holder(AwaitExpressionWalker walker) + { + Walker = walker; + } + + public AwaitExpressionWalker Walker; } } diff --git a/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs b/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs index 438e730fae..1fc4fbc243 100644 --- a/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs +++ b/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs @@ -10,7 +10,6 @@ namespace Roslynator.CSharp.SyntaxWalkers; internal class AwaitExpressionWalker : BaseCSharpSyntaxWalker, IResettable { private bool _shouldVisit = true; - private bool _canBeCached = true; public HashSet AwaitExpressions { get; } = []; @@ -18,14 +17,13 @@ internal class AwaitExpressionWalker : BaseCSharpSyntaxWalker, IResettable protected override bool ShouldVisit => _shouldVisit; - public bool CanBeCached => _canBeCached; - - public void Reset() + public bool Reset() { - _canBeCached = AwaitExpressions.Count <= ObjectPool.MaxCachedBufferSize; + bool canBeCached = AwaitExpressions.Count <= ObjectPool.MaxCachedBufferSize; _shouldVisit = true; StopOnFirstAwaitExpression = false; AwaitExpressions.Clear(); + return canBeCached; } public static bool ContainsAwaitExpression(ExpressionSyntax expression) diff --git a/src/Core/IResettable.cs b/src/Core/IResettable.cs index e07611471b..b35cc4ae4f 100644 --- a/src/Core/IResettable.cs +++ b/src/Core/IResettable.cs @@ -4,11 +4,10 @@ namespace Roslynator; internal interface IResettable { - void Reset(); - /// - /// Gets a value indicating whether the instance can be retained by . - /// Implementations should return false when the instance holds a buffer that grew too large to be worth retaining. + /// Clears instance state. Returns true if the instance is worth retaining. + /// Implementations must snapshot buffer size (capacity when available) before clearing + /// and return false when it exceeded . /// - bool CanBeCached { get; } + bool Reset(); } diff --git a/src/Core/ObjectPool`1.cs b/src/Core/ObjectPool`1.cs index 5ab0ed66e9..a24876fa2a 100644 --- a/src/Core/ObjectPool`1.cs +++ b/src/Core/ObjectPool`1.cs @@ -1,7 +1,6 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Diagnostics; namespace Roslynator; @@ -11,21 +10,28 @@ namespace Roslynator; private static T? _cachedInstance; public static PooledObject Rent() + { + return new PooledObject(RentInstance()); + } + + public static T RentInstance() { T? instance = _cachedInstance; _cachedInstance = null; - return new PooledObject(instance ?? new T()); + return instance ?? new T(); } - internal static void Free(T instance) + internal static void Free(T? instance) { - Debug.Assert(!ReferenceEquals(_cachedInstance, instance), $"'{typeof(T).Name}' freed twice."); + if (instance is null) + return; - instance.Reset(); + if (ReferenceEquals(_cachedInstance, instance)) + return; - if (instance.CanBeCached) + if (instance.Reset()) _cachedInstance = instance; } } diff --git a/src/Core/PooledObject`1.cs b/src/Core/PooledObject`1.cs index 8aa31a47e9..e635c6bcc9 100644 --- a/src/Core/PooledObject`1.cs +++ b/src/Core/PooledObject`1.cs @@ -1,10 +1,8 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; - namespace Roslynator; -internal struct PooledObject : IDisposable where T : class, IResettable, new() +internal ref struct PooledObject where T : class, IResettable, new() { private T? _value; diff --git a/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs b/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs index dfc537ffc5..fd3a86689e 100644 --- a/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs +++ b/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs @@ -414,10 +414,19 @@ public async Task ComputeRefactoringsForNodeAsync() if (node is null) return; - using PooledObject pooledFlags = ObjectPool.Rent(); - - RefactoringFlags flags = pooledFlags.Value; + RefactoringFlags flags = ObjectPool.RentInstance(); + try + { + await ComputeRefactoringsForNodeAsync(node, flags).ConfigureAwait(false); + } + finally + { + ObjectPool.Free(flags); + } + } + private async Task ComputeRefactoringsForNodeAsync(SyntaxNode node, RefactoringFlags flags) + { SyntaxNode firstNode = node; for (; node is not null; node = node.GetParent(ascendOutOfTrivia: true)) @@ -1051,9 +1060,6 @@ public RefactoringFlags() _flags = new BitArray((int)Flag.Count); } - // The bit array has a fixed size, so the instance never grows beyond the pool limit. - public bool CanBeCached => true; - public bool IsSet(Flag flag) { return _flags.Get((int)flag); @@ -1064,9 +1070,10 @@ public void Set(Flag flag) _flags.Set((int)flag, true); } - public void Reset() + public bool Reset() { _flags.SetAll(false); + return true; } } diff --git a/src/Tests/Core.Tests/ObjectPoolTests.cs b/src/Tests/Core.Tests/ObjectPoolTests.cs new file mode 100644 index 0000000000..cd9f14bfcd --- /dev/null +++ b/src/Tests/Core.Tests/ObjectPoolTests.cs @@ -0,0 +1,87 @@ +// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using Xunit; + +namespace Roslynator.Testing.CSharp; + +public static class ObjectPoolTests +{ + [Fact] + public static void Rent_Free_ReusesInstance() + { + PooledBuffer first; + using (PooledObject pooled = ObjectPool.Rent()) + { + first = pooled.Value; + } + + using PooledObject pooled2 = ObjectPool.Rent(); + + Assert.Same(first, pooled2.Value); + } + + [Fact] + public static void Free_DropsOversizedInstance() + { + PooledBuffer first; + using (PooledObject pooled = ObjectPool.Rent()) + { + first = pooled.Value; + first.Grow(ObjectPool.MaxCachedBufferSize + 1); + } + + using PooledObject pooled2 = ObjectPool.Rent(); + + Assert.NotSame(first, pooled2.Value); + } + + [Fact] + public static void Dispose_SecondCallOnSameLocal_IsNoOp() + { + PooledObject pooled = ObjectPool.Rent(); + PooledBuffer first = pooled.Value; + + pooled.Dispose(); + pooled.Dispose(); + + using PooledObject pooled2 = ObjectPool.Rent(); + + Assert.Same(first, pooled2.Value); + } + + [Fact] + public static void Free_DroppedInstance_DoesNotRecache() + { + PooledBuffer first; + using (PooledObject pooled = ObjectPool.Rent()) + { + first = pooled.Value; + first.Grow(ObjectPool.MaxCachedBufferSize + 1); + } + + ObjectPool.Free(first); + + using PooledObject pooled2 = ObjectPool.Rent(); + + Assert.NotSame(first, pooled2.Value); + } + + private sealed class PooledBuffer : IResettable + { + public List Items { get; } = []; + + public void Grow(int count) + { + for (int i = 0; i < count; i++) + Items.Add(i); + } + + public bool Reset() + { + bool canBeCached = Items.Capacity <= ObjectPool.MaxCachedBufferSize; + Items.Clear(); + return canBeCached; + } + } +} From 0e420b5bdce88d9e77d62955b1e8b6b3839dffe9 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sun, 16 Aug 2026 17:18:26 +0200 Subject: [PATCH 6/7] refactor: drop walker pooling and allocate on demand The ThreadStatic cache and the ObjectPool that replaced it were more protocol than the allocations were worth next to semantic-model work. Walkers take their state in constructors; StringBuilderCache is unchanged. Co-authored-by: Cursor --- .../MakeMemberReadOnlyAnalyzer.cs | 6 +- .../MakeMemberReadOnlyWalker.cs | 30 ++----- .../MarkLocalVariableAsConstAnalyzer.cs | 6 +- .../MarkLocalVariableAsConstWalker.cs | 30 ++----- .../Analysis/RefReadOnlyParameterAnalyzer.cs | 29 ++----- .../RemoveRedundantAsyncAwaitAnalyzer.cs | 32 +++---- .../UnusedMember/UnusedMemberAnalyzer.cs | 6 +- .../UnusedMember/UnusedMemberWalker.cs | 33 ++----- .../UnusedParameterAnalyzer.cs | 12 +-- .../UnusedParameter/UnusedParameterWalker.cs | 36 +++----- .../CSharp/Analysis/UseAsyncAwaitAnalyzer.cs | 39 +++------ .../Analysis/RemoveAsyncAwaitAnalysis.cs | 72 ++++----------- .../SyntaxWalkers/AwaitExpressionWalker.cs | 23 ++--- src/Core/IResettable.cs | 13 --- src/Core/ObjectPool.cs | 13 --- src/Core/ObjectPool`1.cs | 37 -------- src/Core/PooledObject`1.cs | 26 ------ .../CSharp/Refactorings/RefactoringContext.cs | 19 +--- .../RemoveAsyncAwaitRefactoring.cs | 40 ++++----- src/Tests/Core.Tests/ObjectPoolTests.cs | 87 ------------------- 20 files changed, 113 insertions(+), 476 deletions(-) delete mode 100644 src/Core/IResettable.cs delete mode 100644 src/Core/ObjectPool.cs delete mode 100644 src/Core/ObjectPool`1.cs delete mode 100644 src/Core/PooledObject`1.cs delete mode 100644 src/Tests/Core.Tests/ObjectPoolTests.cs diff --git a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyAnalyzer.cs b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyAnalyzer.cs index d5a22765e1..11a4ef7216 100644 --- a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyAnalyzer.cs @@ -58,11 +58,7 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) if (typeDeclaration.Modifiers.Contains(SyntaxKind.PartialKeyword)) return; - using PooledObject pooledWalker = ObjectPool.Rent(); - - MakeMemberReadOnlyWalker walker = pooledWalker.Value; - - walker.Initialize(context.SemanticModel, context.CancellationToken); + var walker = new MakeMemberReadOnlyWalker(context.SemanticModel, context.CancellationToken); AnalyzeTypeDeclaration(context, typeDeclaration, walker); } diff --git a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs index e75817ca84..414812c30c 100644 --- a/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs +++ b/src/Analyzers/CSharp/Analysis/MakeMemberReadOnly/MakeMemberReadOnlyWalker.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Collections.Generic; -using System.Diagnostics; using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -11,7 +9,7 @@ namespace Roslynator.CSharp.Analysis.MakeMemberReadOnly; -internal class MakeMemberReadOnlyWalker : AssignedExpressionWalker, IResettable +internal class MakeMemberReadOnlyWalker : AssignedExpressionWalker { private int _classOrStructDepth; private int _localFunctionDepth; @@ -19,33 +17,17 @@ internal class MakeMemberReadOnlyWalker : AssignedExpressionWalker, IResettable private bool _isInInstanceConstructor; private bool _isInStaticConstructor; - public SemanticModel SemanticModel { get; private set; } - - public CancellationToken CancellationToken { get; private set; } - - public Dictionary Symbols { get; } = []; - - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) + public MakeMemberReadOnlyWalker(SemanticModel semanticModel, CancellationToken cancellationToken) { SemanticModel = semanticModel; CancellationToken = cancellationToken; } - public bool Reset() - { - bool canBeCached = Symbols.Count <= ObjectPool.MaxCachedBufferSize; - - Symbols.Clear(); - SemanticModel = null; - CancellationToken = default; - _classOrStructDepth = 0; - _localFunctionDepth = 0; - _anonymousFunctionDepth = 0; - _isInInstanceConstructor = false; - _isInStaticConstructor = false; + public SemanticModel SemanticModel { get; } - return canBeCached; - } + public CancellationToken CancellationToken { get; } + + public Dictionary Symbols { get; } = []; public override void VisitAssignedExpression(ExpressionSyntax expression) { diff --git a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs index 2eea96803e..d9ffc36728 100644 --- a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstAnalyzer.cs @@ -100,11 +100,7 @@ private static bool CanBeMarkedAsConst( SyntaxList statements, int startIndex) { - using PooledObject pooledWalker = ObjectPool.Rent(); - - MarkLocalVariableAsConstWalker walker = pooledWalker.Value; - - walker.Initialize(context.SemanticModel, context.CancellationToken); + var walker = new MarkLocalVariableAsConstWalker(context.SemanticModel, context.CancellationToken); foreach (VariableDeclaratorSyntax variable in variables) { diff --git a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs index ef7eb7dec8..b38d50c623 100644 --- a/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs +++ b/src/Analyzers/CSharp/Analysis/MarkLocalVariableAsConst/MarkLocalVariableAsConstWalker.cs @@ -10,35 +10,23 @@ namespace Roslynator.CSharp.Analysis.MarkLocalVariableAsConst; -internal class MarkLocalVariableAsConstWalker : AssignedExpressionWalker, IResettable +internal class MarkLocalVariableAsConstWalker : AssignedExpressionWalker { - public Dictionary Identifiers { get; } = []; - - public SemanticModel SemanticModel { get; private set; } - - public CancellationToken CancellationToken { get; private set; } - - public bool Result { get; private set; } - - protected override bool ShouldVisit => !Result; - - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) + public MarkLocalVariableAsConstWalker(SemanticModel semanticModel, CancellationToken cancellationToken) { SemanticModel = semanticModel; CancellationToken = cancellationToken; } - public bool Reset() - { - bool canBeCached = Identifiers.Count <= ObjectPool.MaxCachedBufferSize; + public Dictionary Identifiers { get; } = []; - Identifiers.Clear(); - SemanticModel = null; - CancellationToken = default; - Result = false; + public SemanticModel SemanticModel { get; } - return canBeCached; - } + public CancellationToken CancellationToken { get; } + + public bool Result { get; private set; } + + protected override bool ShouldVisit => !Result; public override void VisitAssignedExpression(ExpressionSyntax expression) { diff --git a/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs index 709ade9efd..6c7b3b378e 100644 --- a/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RefReadOnlyParameterAnalyzer.cs @@ -113,9 +113,7 @@ private static void Analyze( var methodSymbol = (IMethodSymbol)semanticModel.GetDeclaredSymbol(declaration, cancellationToken); - using PooledObject pooledWalker = ObjectPool.Rent(); - - RefReadOnlyParameterWalker walker = pooledWalker.Value; + var walker = new RefReadOnlyParameterWalker(semanticModel, cancellationToken); var isFirstCandidate = true; @@ -179,8 +177,6 @@ private static void Analyze( if (walker.Parameters.Count > 0) { - walker.Initialize(semanticModel, cancellationToken); - if (bodyOrExpressionBody.IsKind(SyntaxKind.Block)) { walker.VisitBlock((BlockSyntax)bodyOrExpressionBody); @@ -242,35 +238,22 @@ bool IsReferencedAsMethodGroup() } } - private class RefReadOnlyParameterWalker : BaseCSharpSyntaxWalker, IResettable + private class RefReadOnlyParameterWalker : BaseCSharpSyntaxWalker { private int _localFunctionDepth; private int _anonymousFunctionDepth; - public Dictionary Parameters { get; } = []; - - public SemanticModel SemanticModel { get; private set; } - - public CancellationToken CancellationToken { get; private set; } - - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) + public RefReadOnlyParameterWalker(SemanticModel semanticModel, CancellationToken cancellationToken) { SemanticModel = semanticModel; CancellationToken = cancellationToken; } - public bool Reset() - { - bool canBeCached = Parameters.Count <= ObjectPool.MaxCachedBufferSize; + public Dictionary Parameters { get; } = []; - Parameters.Clear(); - SemanticModel = null; - CancellationToken = default; - _localFunctionDepth = 0; - _anonymousFunctionDepth = 0; + public SemanticModel SemanticModel { get; } - return canBeCached; - } + public CancellationToken CancellationToken { get; } protected override bool ShouldVisit => Parameters.Count > 0; diff --git a/src/Analyzers/CSharp/Analysis/RemoveRedundantAsyncAwaitAnalyzer.cs b/src/Analyzers/CSharp/Analysis/RemoveRedundantAsyncAwaitAnalyzer.cs index be076ad9ba..5fd16b60cc 100644 --- a/src/Analyzers/CSharp/Analysis/RemoveRedundantAsyncAwaitAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/RemoveRedundantAsyncAwaitAnalyzer.cs @@ -87,11 +87,9 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) if (!asyncKeyword.IsKind(SyntaxKind.AsyncKeyword)) return; - using (RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(methodDeclaration, context.SemanticModel, context.CancellationToken)) - { - if (analysis.Success) - ReportDiagnostic(context, asyncKeyword, analysis); - } + RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(methodDeclaration, context.SemanticModel, context.CancellationToken); + if (analysis.Success) + ReportDiagnostic(context, asyncKeyword, analysis); } private static void AnalyzeLocalFunctionStatement(SyntaxNodeAnalysisContext context) @@ -106,11 +104,9 @@ private static void AnalyzeLocalFunctionStatement(SyntaxNodeAnalysisContext cont if (!asyncKeyword.IsKind(SyntaxKind.AsyncKeyword)) return; - using (RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(localFunction, context.SemanticModel, context.CancellationToken)) - { - if (analysis.Success) - ReportDiagnostic(context, asyncKeyword, analysis); - } + RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(localFunction, context.SemanticModel, context.CancellationToken); + if (analysis.Success) + ReportDiagnostic(context, asyncKeyword, analysis); } private static void AnalyzeAnonymousMethodExpression(SyntaxNodeAnalysisContext context) @@ -125,11 +121,9 @@ private static void AnalyzeAnonymousMethodExpression(SyntaxNodeAnalysisContext c if (!asyncKeyword.IsKind(SyntaxKind.AsyncKeyword)) return; - using (RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(anonymousMethod, context.SemanticModel, context.CancellationToken)) - { - if (analysis.Success) - ReportDiagnostic(context, asyncKeyword, analysis); - } + RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(anonymousMethod, context.SemanticModel, context.CancellationToken); + if (analysis.Success) + ReportDiagnostic(context, asyncKeyword, analysis); } private static void AnalyzeLambdaExpression(SyntaxNodeAnalysisContext context) @@ -144,11 +138,9 @@ private static void AnalyzeLambdaExpression(SyntaxNodeAnalysisContext context) if (!asyncKeyword.IsKind(SyntaxKind.AsyncKeyword)) return; - using (RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(lambda, context.SemanticModel, context.CancellationToken)) - { - if (analysis.Success) - ReportDiagnostic(context, asyncKeyword, analysis); - } + RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(lambda, context.SemanticModel, context.CancellationToken); + if (analysis.Success) + ReportDiagnostic(context, asyncKeyword, analysis); } private static void ReportDiagnostic(SyntaxNodeAnalysisContext context, SyntaxToken asyncKeyword, RemoveAsyncAwaitAnalysis analysis) diff --git a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs index 24a0376bac..99d0ff88e8 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberAnalyzer.cs @@ -76,9 +76,7 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) SyntaxList members = typeDeclaration.Members; - using PooledObject pooledWalker = ObjectPool.Rent(); - - UnusedMemberWalker walker = pooledWalker.Value; + var walker = new UnusedMemberWalker(semanticModel, cancellationToken); foreach (MemberDeclarationSyntax member in members) { @@ -200,8 +198,6 @@ private static void AnalyzeTypeDeclaration(SyntaxNodeAnalysisContext context) if (nodes.Count > 0) { - walker.Initialize(semanticModel, cancellationToken); - walker.Visit(typeDeclaration); foreach (NodeSymbolInfo node in nodes) diff --git a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs index 6907fb5f43..9d17a855da 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedMember/UnusedMemberWalker.cs @@ -11,17 +11,23 @@ namespace Roslynator.CSharp.Analysis.UnusedMember; -internal class UnusedMemberWalker : TypeSyntaxWalker, IResettable +internal class UnusedMemberWalker : TypeSyntaxWalker { private bool _isEmpty; private IMethodSymbol _containingMethodSymbol; + public UnusedMemberWalker(SemanticModel semanticModel, CancellationToken cancellationToken) + { + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } + public Collection Nodes { get; } = []; - public SemanticModel SemanticModel { get; private set; } + public SemanticModel SemanticModel { get; } - public CancellationToken CancellationToken { get; private set; } + public CancellationToken CancellationToken { get; } public bool IsAnyNodeConst { get; private set; } @@ -32,27 +38,6 @@ protected override bool ShouldVisit get { return !_isEmpty; } } - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) - { - SemanticModel = semanticModel; - CancellationToken = cancellationToken; - } - - public bool Reset() - { - bool canBeCached = Nodes.Count <= ObjectPool.MaxCachedBufferSize; - _isEmpty = false; - _containingMethodSymbol = null; - - Nodes.Clear(); - SemanticModel = null; - CancellationToken = default; - IsAnyNodeConst = false; - IsAnyNodeDelegate = false; - - return canBeCached; - } - public void AddDelegate(string name, SyntaxNode node) { AddNode(name, node); diff --git a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterAnalyzer.cs index 2a935e7a56..6c27afdf83 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterAnalyzer.cs @@ -143,11 +143,7 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context) if (methodSymbol.ImplementsInterfaceMember(allInterfaces: true)) return; - using PooledObject pooledWalker = ObjectPool.Rent(); - - UnusedParameterWalker walker = pooledWalker.Value; - - walker.Initialize(context.SemanticModel, context.CancellationToken); + var walker = new UnusedParameterWalker(context.SemanticModel, context.CancellationToken); FindUnusedNodes(parameterInfo, walker, methodSymbol); @@ -329,11 +325,7 @@ private static void AnalyzeAnonymousMethodExpression(SyntaxNodeAnalysisContext c private static void Analyze(SyntaxNodeAnalysisContext context, in ParameterInfo parameterInfo, bool isIndexer = false) { - using PooledObject pooledWalker = ObjectPool.Rent(); - - UnusedParameterWalker walker = pooledWalker.Value; - - walker.Initialize(context.SemanticModel, context.CancellationToken, isIndexer); + var walker = new UnusedParameterWalker(context.SemanticModel, context.CancellationToken, isIndexer); FindUnusedNodes(parameterInfo, walker); diff --git a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs index fb8fa85909..8ca4786145 100644 --- a/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs +++ b/src/Analyzers/CSharp/Analysis/UnusedParameter/UnusedParameterWalker.cs @@ -10,44 +10,30 @@ namespace Roslynator.CSharp.Analysis.UnusedParameter; -internal class UnusedParameterWalker : TypeSyntaxWalker, IResettable +internal class UnusedParameterWalker : TypeSyntaxWalker { private static readonly StringComparer _ordinalComparer = StringComparer.Ordinal; private bool _isEmpty; - public Dictionary Nodes { get; } = new(_ordinalComparer); - - public SemanticModel SemanticModel { get; private set; } - - public CancellationToken CancellationToken { get; private set; } - - public bool IsIndexer { get; private set; } - - public bool IsAnyTypeParameter { get; set; } - - protected override bool ShouldVisit => !_isEmpty; - - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken, bool isIndexer = false) + public UnusedParameterWalker(SemanticModel semanticModel, CancellationToken cancellationToken, bool isIndexer = false) { SemanticModel = semanticModel; CancellationToken = cancellationToken; IsIndexer = isIndexer; } - public bool Reset() - { - bool canBeCached = Nodes.Count <= ObjectPool.MaxCachedBufferSize; - _isEmpty = false; + public Dictionary Nodes { get; } = new(_ordinalComparer); - Nodes.Clear(); - SemanticModel = null; - CancellationToken = default; - IsIndexer = false; - IsAnyTypeParameter = false; + public SemanticModel SemanticModel { get; } - return canBeCached; - } + public CancellationToken CancellationToken { get; } + + public bool IsIndexer { get; } + + public bool IsAnyTypeParameter { get; set; } + + protected override bool ShouldVisit => !_isEmpty; public void AddParameter(ParameterSyntax parameter) { diff --git a/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs index 34e6e27594..9fea85b0c3 100644 --- a/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/UseAsyncAwaitAnalyzer.cs @@ -154,11 +154,7 @@ private static void AnalyzeAnonymousMethodExpression(SyntaxNodeAnalysisContext c private static bool IsFixable(BlockSyntax body, SyntaxNodeAnalysisContext context) { - using PooledObject pooledWalker = ObjectPool.Rent(); - - UseAsyncAwaitWalker walker = pooledWalker.Value; - - walker.Initialize(context.SemanticModel, context.CancellationToken); + var walker = new UseAsyncAwaitWalker(context.SemanticModel, context.CancellationToken); walker.VisitBlock(body); @@ -166,19 +162,25 @@ private static bool IsFixable(BlockSyntax body, SyntaxNodeAnalysisContext contex && !CSharpUtility.IsInUnsafeContext(body); } - private class UseAsyncAwaitWalker : StatementWalker, IResettable + private class UseAsyncAwaitWalker : StatementWalker { private readonly List _usingDeclarations = []; private int _usingOrTryStatementDepth; private bool _shouldVisit = true; + public UseAsyncAwaitWalker(SemanticModel semanticModel, CancellationToken cancellationToken) + { + SemanticModel = semanticModel; + CancellationToken = cancellationToken; + } + public override bool ShouldVisit => _shouldVisit; public ReturnStatementSyntax ReturnStatement { get; private set; } - public SemanticModel SemanticModel { get; private set; } + public SemanticModel SemanticModel { get; } - public CancellationToken CancellationToken { get; private set; } + public CancellationToken CancellationToken { get; } public override void VisitUsingStatement(UsingStatementSyntax node) { @@ -320,26 +322,5 @@ public override void VisitSimpleLambdaExpression(SimpleLambdaExpressionSyntax no public override void VisitParenthesizedLambdaExpression(ParenthesizedLambdaExpressionSyntax node) { } - - public void Initialize(SemanticModel semanticModel, CancellationToken cancellationToken) - { - SemanticModel = semanticModel; - CancellationToken = cancellationToken; - } - - public bool Reset() - { - // Count is always 0 after a balanced visit; Capacity is the retained buffer size. - bool canBeCached = _usingDeclarations.Capacity <= ObjectPool.MaxCachedBufferSize; - _shouldVisit = true; - _usingOrTryStatementDepth = 0; - - _usingDeclarations.Clear(); - ReturnStatement = null; - SemanticModel = null; - CancellationToken = default; - - return canBeCached; - } } } diff --git a/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs b/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs index e067b9eff7..ed9df6a9d6 100644 --- a/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs +++ b/src/Common/CSharp/Analysis/RemoveAsyncAwaitAnalysis.cs @@ -11,27 +11,25 @@ namespace Roslynator.CSharp.Analysis; -internal readonly struct RemoveAsyncAwaitAnalysis : IDisposable +internal readonly struct RemoveAsyncAwaitAnalysis { - private readonly Holder _holder; - private RemoveAsyncAwaitAnalysis(AwaitExpressionWalker walker) { - _holder = new Holder(walker); + Walker = walker; AwaitExpression = null; } private RemoveAsyncAwaitAnalysis(AwaitExpressionSyntax awaitExpression) { - _holder = null; AwaitExpression = awaitExpression; + Walker = null; } public bool Success => AwaitExpression is not null || Walker?.AwaitExpressions.Count > 0; public AwaitExpressionSyntax AwaitExpression { get; } - public AwaitExpressionWalker Walker => _holder?.Walker; + public AwaitExpressionWalker Walker { get; } public static RemoveAsyncAwaitAnalysis Create( MethodDeclarationSyntax methodDeclaration, @@ -183,14 +181,14 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( if (awaitExpression is null) return default; - PooledObject pooledWalker = VisitStatements(); + AwaitExpressionWalker walker = VisitStatements(); - HashSet awaitExpressions = pooledWalker.Value.AwaitExpressions; + HashSet awaitExpressions = walker.AwaitExpressions; if (awaitExpressions.Count == 1) { if (VerifyTypes(node, awaitExpression, semanticModel, cancellationToken)) - return Take(ref pooledWalker); + return new RemoveAsyncAwaitAnalysis(walker); } else if (awaitExpressions.Count > 1) { @@ -203,7 +201,7 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( if (VerifyIfStatement((IfStatementSyntax)prevStatement, awaitExpressions.Count - 1, endsWithElse: false) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return Take(ref pooledWalker); + return new RemoveAsyncAwaitAnalysis(walker); } break; @@ -213,7 +211,7 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( if (VerifySwitchStatement((SwitchStatementSyntax)prevStatement, awaitExpressions.Count - 1, containsDefaultSection: false) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return Take(ref pooledWalker); + return new RemoveAsyncAwaitAnalysis(walker); } break; @@ -221,62 +219,52 @@ private static RemoveAsyncAwaitAnalysis AnalyzeMethodBody( } } - pooledWalker.Dispose(); return default; } case SyntaxKind.IfStatement: { - PooledObject pooledWalker = VisitStatements(); + AwaitExpressionWalker walker = VisitStatements(); - HashSet awaitExpressions = pooledWalker.Value.AwaitExpressions; + HashSet awaitExpressions = walker.AwaitExpressions; if (awaitExpressions.Count > 0 && VerifyIfStatement((IfStatementSyntax)statement, awaitExpressions.Count, endsWithElse: true) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return Take(ref pooledWalker); + return new RemoveAsyncAwaitAnalysis(walker); } - pooledWalker.Dispose(); return default; } case SyntaxKind.SwitchStatement: { - PooledObject pooledWalker = VisitStatements(); + AwaitExpressionWalker walker = VisitStatements(); - HashSet awaitExpressions = pooledWalker.Value.AwaitExpressions; + HashSet awaitExpressions = walker.AwaitExpressions; if (awaitExpressions.Count > 0 && VerifySwitchStatement((SwitchStatementSyntax)statement, awaitExpressions.Count, containsDefaultSection: true) && VerifyTypes(node, awaitExpressions, semanticModel, cancellationToken)) { - return Take(ref pooledWalker); + return new RemoveAsyncAwaitAnalysis(walker); } - pooledWalker.Dispose(); return default; } } return default; - PooledObject VisitStatements() + AwaitExpressionWalker VisitStatements() { - PooledObject pooledWalker = ObjectPool.Rent(); + var walker = new AwaitExpressionWalker(); - pooledWalker.Value.VisitStatements(statements, statement); + walker.VisitStatements(statements, statement); - return pooledWalker; + return walker; } } - private static RemoveAsyncAwaitAnalysis Take(ref PooledObject pooledWalker) - { - AwaitExpressionWalker walker = pooledWalker.Value; - pooledWalker = default; - return new RemoveAsyncAwaitAnalysis(walker); - } - private static bool VerifyIfStatement( IfStatementSyntax ifStatement, int expectedCount, @@ -459,26 +447,4 @@ private static IMethodSymbol GetMethodSymbol( throw new InvalidOperationException(); } - - public void Dispose() - { - Holder holder = _holder; - AwaitExpressionWalker walker = holder?.Walker; - - if (walker is null) - return; - - holder.Walker = null; - ObjectPool.Free(walker); - } - - private sealed class Holder - { - public Holder(AwaitExpressionWalker walker) - { - Walker = walker; - } - - public AwaitExpressionWalker Walker; - } } diff --git a/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs b/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs index 1fc4fbc243..0c00dbb689 100644 --- a/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs +++ b/src/Common/CSharp/SyntaxWalkers/AwaitExpressionWalker.cs @@ -7,32 +7,25 @@ namespace Roslynator.CSharp.SyntaxWalkers; -internal class AwaitExpressionWalker : BaseCSharpSyntaxWalker, IResettable +internal class AwaitExpressionWalker : BaseCSharpSyntaxWalker { private bool _shouldVisit = true; + public AwaitExpressionWalker(bool stopOnFirstAwaitExpression = false) + { + StopOnFirstAwaitExpression = stopOnFirstAwaitExpression; + } + public HashSet AwaitExpressions { get; } = []; - private bool StopOnFirstAwaitExpression { get; set; } + private bool StopOnFirstAwaitExpression { get; } protected override bool ShouldVisit => _shouldVisit; - public bool Reset() - { - bool canBeCached = AwaitExpressions.Count <= ObjectPool.MaxCachedBufferSize; - _shouldVisit = true; - StopOnFirstAwaitExpression = false; - AwaitExpressions.Clear(); - return canBeCached; - } - public static bool ContainsAwaitExpression(ExpressionSyntax expression) { - using PooledObject pooledWalker = ObjectPool.Rent(); - - AwaitExpressionWalker walker = pooledWalker.Value; + var walker = new AwaitExpressionWalker(stopOnFirstAwaitExpression: true); - walker.StopOnFirstAwaitExpression = true; walker.Visit(expression); Debug.Assert(walker.AwaitExpressions.Count <= 1); diff --git a/src/Core/IResettable.cs b/src/Core/IResettable.cs deleted file mode 100644 index b35cc4ae4f..0000000000 --- a/src/Core/IResettable.cs +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Roslynator; - -internal interface IResettable -{ - /// - /// Clears instance state. Returns true if the instance is worth retaining. - /// Implementations must snapshot buffer size (capacity when available) before clearing - /// and return false when it exceeded . - /// - bool Reset(); -} diff --git a/src/Core/ObjectPool.cs b/src/Core/ObjectPool.cs deleted file mode 100644 index 1604db08ca..0000000000 --- a/src/Core/ObjectPool.cs +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Roslynator; - -internal static class ObjectPool -{ - /// - /// Maximum number of items a pooled object's buffer may have held to be worth retaining. - /// Clearing a collection does not shrink its backing array, so an instance that grew beyond - /// this size would keep that array alive for the lifetime of the thread. - /// - public const int MaxCachedBufferSize = 256; -} diff --git a/src/Core/ObjectPool`1.cs b/src/Core/ObjectPool`1.cs deleted file mode 100644 index a24876fa2a..0000000000 --- a/src/Core/ObjectPool`1.cs +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; - -namespace Roslynator; - -internal static class ObjectPool where T : class, IResettable, new() -{ - [ThreadStatic] - private static T? _cachedInstance; - - public static PooledObject Rent() - { - return new PooledObject(RentInstance()); - } - - public static T RentInstance() - { - T? instance = _cachedInstance; - - _cachedInstance = null; - - return instance ?? new T(); - } - - internal static void Free(T? instance) - { - if (instance is null) - return; - - if (ReferenceEquals(_cachedInstance, instance)) - return; - - if (instance.Reset()) - _cachedInstance = instance; - } -} diff --git a/src/Core/PooledObject`1.cs b/src/Core/PooledObject`1.cs deleted file mode 100644 index e635c6bcc9..0000000000 --- a/src/Core/PooledObject`1.cs +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Roslynator; - -internal ref struct PooledObject where T : class, IResettable, new() -{ - private T? _value; - - internal PooledObject(T value) - { - _value = value; - } - - public readonly T Value => _value!; - - public void Dispose() - { - T? value = _value; - - if (value is null) - return; - - _value = null; - ObjectPool.Free(value); - } -} diff --git a/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs b/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs index fd3a86689e..159893ea0c 100644 --- a/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs +++ b/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs @@ -414,15 +414,8 @@ public async Task ComputeRefactoringsForNodeAsync() if (node is null) return; - RefactoringFlags flags = ObjectPool.RentInstance(); - try - { - await ComputeRefactoringsForNodeAsync(node, flags).ConfigureAwait(false); - } - finally - { - ObjectPool.Free(flags); - } + RefactoringFlags flags = new(); + await ComputeRefactoringsForNodeAsync(node, flags).ConfigureAwait(false); } private async Task ComputeRefactoringsForNodeAsync(SyntaxNode node, RefactoringFlags flags) @@ -1051,7 +1044,7 @@ private async Task ComputeRefactoringsForNodeAsync(SyntaxNode node, RefactoringF CommentTriviaRefactoring.ComputeRefactorings(this, firstNode); } - private class RefactoringFlags : IResettable + private class RefactoringFlags { private readonly BitArray _flags; @@ -1069,12 +1062,6 @@ public void Set(Flag flag) { _flags.Set((int)flag, true); } - - public bool Reset() - { - _flags.SetAll(false); - return true; - } } private enum Flag diff --git a/src/Refactorings/CSharp/Refactorings/RemoveAsyncAwaitRefactoring.cs b/src/Refactorings/CSharp/Refactorings/RemoveAsyncAwaitRefactoring.cs index 03e2ef6cb8..cd0bcb7323 100644 --- a/src/Refactorings/CSharp/Refactorings/RemoveAsyncAwaitRefactoring.cs +++ b/src/Refactorings/CSharp/Refactorings/RemoveAsyncAwaitRefactoring.cs @@ -18,11 +18,9 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, Sy { SemanticModel semanticModel = await context.GetSemanticModelAsync().ConfigureAwait(false); - using (RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(methodDeclaration, semanticModel, context.CancellationToken)) - { - if (analysis.Success) - RegisterRefactoring(); - } + RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(methodDeclaration, semanticModel, context.CancellationToken); + if (analysis.Success) + RegisterRefactoring(); return; } @@ -30,11 +28,9 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, Sy { SemanticModel semanticModel = await context.GetSemanticModelAsync().ConfigureAwait(false); - using (RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(localFunction, semanticModel, context.CancellationToken)) - { - if (analysis.Success) - RegisterRefactoring(); - } + RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(localFunction, semanticModel, context.CancellationToken); + if (analysis.Success) + RegisterRefactoring(); return; } @@ -42,11 +38,9 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, Sy { SemanticModel semanticModel = await context.GetSemanticModelAsync().ConfigureAwait(false); - using (RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(parenthesizedLambda, semanticModel, context.CancellationToken)) - { - if (analysis.Success) - RegisterRefactoring(); - } + RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(parenthesizedLambda, semanticModel, context.CancellationToken); + if (analysis.Success) + RegisterRefactoring(); return; } @@ -54,11 +48,9 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, Sy { SemanticModel semanticModel = await context.GetSemanticModelAsync().ConfigureAwait(false); - using (RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(simpleLambda, semanticModel, context.CancellationToken)) - { - if (analysis.Success) - RegisterRefactoring(); - } + RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(simpleLambda, semanticModel, context.CancellationToken); + if (analysis.Success) + RegisterRefactoring(); return; } @@ -66,11 +58,9 @@ public static async Task ComputeRefactoringsAsync(RefactoringContext context, Sy { SemanticModel semanticModel = await context.GetSemanticModelAsync().ConfigureAwait(false); - using (RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(anonymousMethod, semanticModel, context.CancellationToken)) - { - if (analysis.Success) - RegisterRefactoring(); - } + RemoveAsyncAwaitAnalysis analysis = RemoveAsyncAwaitAnalysis.Create(anonymousMethod, semanticModel, context.CancellationToken); + if (analysis.Success) + RegisterRefactoring(); return; } diff --git a/src/Tests/Core.Tests/ObjectPoolTests.cs b/src/Tests/Core.Tests/ObjectPoolTests.cs deleted file mode 100644 index cd9f14bfcd..0000000000 --- a/src/Tests/Core.Tests/ObjectPoolTests.cs +++ /dev/null @@ -1,87 +0,0 @@ -// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; -using Xunit; - -namespace Roslynator.Testing.CSharp; - -public static class ObjectPoolTests -{ - [Fact] - public static void Rent_Free_ReusesInstance() - { - PooledBuffer first; - using (PooledObject pooled = ObjectPool.Rent()) - { - first = pooled.Value; - } - - using PooledObject pooled2 = ObjectPool.Rent(); - - Assert.Same(first, pooled2.Value); - } - - [Fact] - public static void Free_DropsOversizedInstance() - { - PooledBuffer first; - using (PooledObject pooled = ObjectPool.Rent()) - { - first = pooled.Value; - first.Grow(ObjectPool.MaxCachedBufferSize + 1); - } - - using PooledObject pooled2 = ObjectPool.Rent(); - - Assert.NotSame(first, pooled2.Value); - } - - [Fact] - public static void Dispose_SecondCallOnSameLocal_IsNoOp() - { - PooledObject pooled = ObjectPool.Rent(); - PooledBuffer first = pooled.Value; - - pooled.Dispose(); - pooled.Dispose(); - - using PooledObject pooled2 = ObjectPool.Rent(); - - Assert.Same(first, pooled2.Value); - } - - [Fact] - public static void Free_DroppedInstance_DoesNotRecache() - { - PooledBuffer first; - using (PooledObject pooled = ObjectPool.Rent()) - { - first = pooled.Value; - first.Grow(ObjectPool.MaxCachedBufferSize + 1); - } - - ObjectPool.Free(first); - - using PooledObject pooled2 = ObjectPool.Rent(); - - Assert.NotSame(first, pooled2.Value); - } - - private sealed class PooledBuffer : IResettable - { - public List Items { get; } = []; - - public void Grow(int count) - { - for (int i = 0; i < count; i++) - Items.Add(i); - } - - public bool Reset() - { - bool canBeCached = Items.Capacity <= ObjectPool.MaxCachedBufferSize; - Items.Clear(); - return canBeCached; - } - } -} From 94c27b5cea4cf0f5fb5ba893ffd48f3f0471060b Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sun, 16 Aug 2026 17:32:25 +0200 Subject: [PATCH 7/7] fix: use explicit object creation for RefactoringFlags dotnet format --severity info failed RCS1250 on target-typed new(). Co-authored-by: Cursor --- src/Refactorings/CSharp/Refactorings/RefactoringContext.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs b/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs index 159893ea0c..e725fa9d00 100644 --- a/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs +++ b/src/Refactorings/CSharp/Refactorings/RefactoringContext.cs @@ -414,7 +414,7 @@ public async Task ComputeRefactoringsForNodeAsync() if (node is null) return; - RefactoringFlags flags = new(); + var flags = new RefactoringFlags(); await ComputeRefactoringsForNodeAsync(node, flags).ConfigureAwait(false); }