Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement early bail-out #5410

Merged
merged 6 commits into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Introduce SafeSyntaxWalker
  • Loading branch information
costin-zaharia-sonarsource committed Feb 18, 2022
commit 5fb0c6be800156079ed0a22fbf4122161e061469
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using SonarAnalyzer.Extensions;

namespace SonarAnalyzer.Helpers
{
Expand Down Expand Up @@ -114,7 +113,7 @@ private static IList<DirectiveTriviaSyntax> CollectPrecedingDirectiveSyntax(Synt
/// Collects all of the #if, #else, #elsif and #endif directives occuring in the
/// syntax tree up to the specified node
/// </summary>
private class BranchingDirectiveCollector : CSharpSyntaxWalker
private sealed class BranchingDirectiveCollector : SafeCSharpSyntaxWalker
{
private readonly SyntaxNode terminatingNode;
private bool found;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace SonarAnalyzer.Helpers
/// Collects all symbol usages from a class declaration. Ignores symbols whose names are not present
/// in the knownSymbolNames collection for performance reasons.
/// </summary>
internal class CSharpSymbolUsageCollector : CSharpSyntaxWalker
internal class CSharpSymbolUsageCollector : SafeCSharpSyntaxWalker
{
[Flags]
private enum SymbolAccess { None = 0, Read = 1, Write = 2, ReadWrite = Read | Write }
Expand All @@ -54,8 +54,8 @@ private enum SymbolAccess { None = 0, Read = 1, Write = 2, ReadWrite = Read | Wr

public ISet<ISymbol> UsedSymbols { get; } = new HashSet<ISymbol>();
public IDictionary<ISymbol, SymbolUsage> FieldSymbolUsages { get; } = new Dictionary<ISymbol, SymbolUsage>();
public HashSet<string> DebuggerDisplayValues { get; } = new HashSet<string>();
public Dictionary<IPropertySymbol, AccessorAccess> PropertyAccess { get; } = new Dictionary<IPropertySymbol, AccessorAccess>();
public HashSet<string> DebuggerDisplayValues { get; } = new();
public Dictionary<IPropertySymbol, AccessorAccess> PropertyAccess { get; } = new();

public CSharpSymbolUsageCollector(Compilation compilation, IEnumerable<ISymbol> knownSymbols)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,21 @@
using System;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using SonarAnalyzer.Common;

namespace SonarAnalyzer.Extensions
namespace SonarAnalyzer.Helpers
{
public static class CSharpSyntaxWalkerExtensions
public class SafeCSharpSyntaxWalker : CSharpSyntaxWalker, ISafeSyntaxWalker
{
public static bool SafeVisit(this CSharpSyntaxWalker syntaxWalker, SyntaxNode syntaxNode)
protected SafeCSharpSyntaxWalker() { }

protected SafeCSharpSyntaxWalker(SyntaxWalkerDepth depth) : base(depth) { }

public bool SafeVisit(SyntaxNode syntaxNode)
{
try
{
syntaxWalker.Visit(syntaxNode);
Visit(syntaxNode);
return true;
}
catch (InsufficientExecutionStackException)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using SonarAnalyzer.Common;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

namespace SonarAnalyzer.Metrics.CSharp
Expand All @@ -43,12 +44,11 @@ public static CognitiveComplexity GetComplexity(SyntaxNode node, bool onlyGlobal
return new CognitiveComplexity(walker.State.Complexity, walker.State.IncrementLocations.ToImmutableArray());
}

private class CognitiveWalker : CSharpSyntaxWalker
private sealed class CognitiveWalker : SafeCSharpSyntaxWalker
{
private readonly bool onlyGlobalStatements;

public CognitiveComplexityWalkerState<MethodDeclarationSyntax> State { get; }
= new CognitiveComplexityWalkerState<MethodDeclarationSyntax>();
public CognitiveComplexityWalkerState<MethodDeclarationSyntax> State { get; } = new();

public CognitiveWalker(bool onlyGlobalStatements) =>
this.onlyGlobalStatements = onlyGlobalStatements;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using SonarAnalyzer.Common;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

Expand Down Expand Up @@ -55,12 +54,11 @@ public static CyclomaticComplexity GetComplexity(SyntaxNode syntaxNode, bool onl
return new CyclomaticComplexity(walker.IncrementLocations.ToImmutableArray());
}

private class CyclomaticWalker : CSharpSyntaxWalker
private sealed class CyclomaticWalker : SafeCSharpSyntaxWalker
{
private readonly bool onlyGlobalStatements;

public List<SecondaryLocation> IncrementLocations { get; }
= new List<SecondaryLocation>();
public List<SecondaryLocation> IncrementLocations { get; } = new();

public CyclomaticWalker(bool onlyGlobalStatements) =>
this.onlyGlobalStatements = onlyGlobalStatements;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

Expand All @@ -43,7 +42,7 @@ public static ImmutableArray<int> GetLineNumbers(SyntaxTree syntaxTree, Semantic
return walker.ExecutableLineNumbers.ToImmutableArray();
}

private class ExecutableLinesWalker : CSharpSyntaxWalker
private sealed class ExecutableLinesWalker : SafeCSharpSyntaxWalker
{
private readonly SemanticModel semanticModel;

Expand All @@ -52,8 +51,7 @@ public ExecutableLinesWalker(SemanticModel semanticModel)
this.semanticModel = semanticModel;
}

public HashSet<int> ExecutableLineNumbers { get; }
= new HashSet<int>();
public HashSet<int> ExecutableLineNumbers { get; } = new();

public override void DefaultVisit(SyntaxNode node)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Common;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

Expand Down Expand Up @@ -140,7 +139,7 @@ private static IEnumerable<INamedTypeSymbol> ExpandGenericTypes(INamedTypeSymbol
private static IEnumerable<INamedTypeSymbol> GetConstraintTypes(ITypeParameterSymbol typeParameter) =>
typeParameter.ConstraintTypes.OfType<INamedTypeSymbol>().SelectMany(ExpandGenericTypes);

private class TypeDependencyCollector : CSharpSyntaxWalker
private sealed class TypeDependencyCollector : SafeCSharpSyntaxWalker
{
private readonly SemanticModel model;
private readonly TypeDeclarationSyntax originalTypeDeclaration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ private static SyntaxNode LookupIdentifierInitializer(IdentifierNameSyntax ident
? equalsValueClause.Value.RemoveParentheses()
: null;

private class InvocationWalker : CSharpSyntaxWalker
private class InvocationWalker : SafeCSharpSyntaxWalker
{
private readonly EndInvokeContext context;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;

namespace SonarAnalyzer.Rules.CSharp
Expand Down Expand Up @@ -84,7 +83,7 @@ protected override void Initialize(SonarAnalysisContext context)
SyntaxKind.MethodDeclaration);
}

private class ConsumeValueTaskWalker : CSharpSyntaxWalker
private sealed class ConsumeValueTaskWalker : SafeCSharpSyntaxWalker
{
private readonly SemanticModel semanticModel;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

Expand Down Expand Up @@ -77,18 +76,18 @@ private static SyntaxNode GetPropertyBody(PropertyDeclarationSyntax property)
.FirstOrDefault();
}

private class PropertyWalker : CSharpSyntaxWalker
private sealed class PropertyWalker : SafeCSharpSyntaxWalker
{
private readonly SemanticModel semanticModel;
private readonly List<Location> locations = new List<Location>();
private readonly List<Location> locations = new();

private static readonly HashSet<SyntaxKind> returnStatements = new HashSet<SyntaxKind>
private static readonly HashSet<SyntaxKind> ReturnStatements = new()
{
SyntaxKind.ReturnStatement,
SyntaxKind.ArrowExpressionClause,
};

public IEnumerable<Location> Locations => this.locations;
public IEnumerable<Location> Locations => locations;

public PropertyWalker(SemanticModel semanticModel)
{
Expand Down Expand Up @@ -127,7 +126,7 @@ private static bool IsReturnOrAssignment(SyntaxNode node)

private static bool IsReturn(SyntaxNode node)
{
return node != null && node.IsAnyKind(returnStatements);
return node != null && node.IsAnyKind(ReturnStatements);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

Expand Down Expand Up @@ -70,7 +69,7 @@ var wrapper when LocalFunctionStatementSyntaxWrapper.IsInstance(wrapper) => ((Lo
_ => Enumerable.Empty<ParameterSyntax>()
};

private sealed class GenericWalker : CSharpSyntaxWalker
private sealed class GenericWalker : SafeCSharpSyntaxWalker
{
private static readonly ImmutableArray<KnownType> ignoredTypes =
KnownType.SystemFuncVariants
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

Expand Down Expand Up @@ -51,7 +50,7 @@ protected override void Initialize(ParameterLoadingAnalysisContext context) =>
SyntaxKind.RemoveAccessorDeclaration,
SyntaxKind.GlobalStatement);

private class NestingDepthWalker : CSharpSyntaxWalker
private sealed class NestingDepthWalker : SafeCSharpSyntaxWalker
{
private readonly NestingDepthCounter counter;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ private static bool ImplementsIDeserializationCallback(ITypeSymbol symbol) =>
/// This walker is responsible to visit all constructor declarations and check if parameters are used in a
/// conditional structure or not.
/// </summary>
private sealed class ConstructorDeclarationWalker : CSharpSyntaxWalker
private sealed class ConstructorDeclarationWalker : SafeCSharpSyntaxWalker
{
private readonly SemanticModel semanticModel;
private readonly List<ConstructorInfo> constructorsInfo = new List<ConstructorInfo>();
private readonly List<ConstructorInfo> constructorsInfo = new();

private bool visitedFirstLevel;

Expand Down Expand Up @@ -237,7 +237,7 @@ private static ImmutableArray<ISymbol> GetConstructorParameterSymbols(BaseMethod
/// This walker is responsible to visit all conditional structures and check if a list of parameters
/// are used or not.
/// </summary>
private sealed class ConditionalsWalker : CSharpSyntaxWalker
private sealed class ConditionalsWalker : SafeCSharpSyntaxWalker
{
private readonly SemanticModel semanticModel;
private readonly ISet<string> parameterNames;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;

namespace SonarAnalyzer.Rules.CSharp
Expand All @@ -37,7 +36,7 @@ protected override void Initialize(SonarAnalysisContext context) =>
c => new ThrowInFinallyWalker(c, Rule).SafeVisit(((FinallyClauseSyntax)c.Node).Block),
SyntaxKind.FinallyClause);

private class ThrowInFinallyWalker : CSharpSyntaxWalker
private sealed class ThrowInFinallyWalker : SafeCSharpSyntaxWalker
{
private readonly SyntaxNodeAnalysisContext context;
private readonly DiagnosticDescriptor rule;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

namespace SonarAnalyzer.Rules.CSharp
{
internal class ParameterValidationInMethodWalker : CSharpSyntaxWalker
internal class ParameterValidationInMethodWalker : SafeCSharpSyntaxWalker
{
private static readonly ISet<SyntaxKind> SubMethodEquivalents =
new HashSet<SyntaxKind>
Expand All @@ -38,11 +38,11 @@ internal class ParameterValidationInMethodWalker : CSharpSyntaxWalker
};

private readonly SemanticModel semanticModel;
private readonly List<Location> argumentExceptionLocations = new List<Location>();
private readonly List<Location> argumentExceptionLocations = new();

private bool keepWalking = true;

public IEnumerable<Location> ArgumentExceptionLocations => this.argumentExceptionLocations;
public IEnumerable<Location> ArgumentExceptionLocations => argumentExceptionLocations;

public ParameterValidationInMethodWalker(SemanticModel semanticModel)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,21 +104,18 @@ bool HasNoAttributes(FieldDeclarationSyntax fieldDeclaration) =>
fieldDeclaration.AttributeLists.Count == 0;
}

private class FieldAccessCollector : CSharpSyntaxWalker
private class FieldAccessCollector : SafeCSharpSyntaxWalker
{
// Contains statements that READ field values. First grouped by field symbol (that is read),
// then by method/property/ctor symbol (that contains the statements)
private readonly Dictionary<IFieldSymbol, Lookup<ISymbol, SyntaxNode>> readsByField =
new Dictionary<IFieldSymbol, Lookup<ISymbol, SyntaxNode>>();
private readonly Dictionary<IFieldSymbol, Lookup<ISymbol, SyntaxNode>> readsByField = new();

// Contains statements that WRITE field values. First grouped by field symbol (that is written),
// then by method/property/ctor symbol (that contains the statements)
private readonly Dictionary<IFieldSymbol, Lookup<ISymbol, SyntaxNode>> writesByField =
new Dictionary<IFieldSymbol, Lookup<ISymbol, SyntaxNode>>();
private readonly Dictionary<IFieldSymbol, Lookup<ISymbol, SyntaxNode>> writesByField = new();

// Contains all method/property invocations grouped by the statement that contains them.
private readonly Lookup<SyntaxNode, ISymbol> invocations =
new Lookup<SyntaxNode, ISymbol>();
private readonly Lookup<SyntaxNode, ISymbol> invocations = new();

private readonly SemanticModel semanticModel;
private readonly IDictionary<IFieldSymbol, VariableDeclaratorSyntax> privateFields;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

Expand Down Expand Up @@ -282,7 +281,7 @@ private static void CheckSealedMemberInSealedClass(SyntaxNodeAnalysisContext con
}
}

private class CheckedWalker : CSharpSyntaxWalker
private sealed class CheckedWalker : SafeCSharpSyntaxWalker
{
private readonly SyntaxNodeAnalysisContext context;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ private bool HasSqlNamespace(SyntaxList<UsingDirectiveSyntax> usings) =>
usings.Select(usingDirective => usingDirective.Name)
.Any(name => SqlNamespaces.Any(sn => SyntaxFactory.AreEquivalent(name, sn)));

private class StringConcatenationWalker : CSharpSyntaxWalker
private class StringConcatenationWalker : SafeCSharpSyntaxWalker
{
private readonly SyntaxNodeAnalysisContext context;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

Expand Down Expand Up @@ -100,7 +99,7 @@ protected override bool TryGetTryAncestorStatements(StatementSyntax node, List<S
return true;
}

private class CsLoopwalker : CSharpSyntaxWalker
private sealed class CsLoopwalker : SafeCSharpSyntaxWalker
{
private readonly LoopWalker walker;

Expand Down
Loading