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.

Sign up for GitHub

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

Improve performance for SymbolReferenceAnalyzer #5548

Merged
merged 9 commits into from
Apr 12, 2022
Merged

Improve performance for SymbolReferenceAnalyzer #5548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
357dc14
Filter declarations at syntax level
costin-zaharia-sonarsource Apr 5, 2022
8e89cc4
Ignore casing when comparing visual basic identifiers
costin-zaharia-sonarsource Apr 8, 2022
8999b05
Use HashSet to avoid loop
costin-zaharia-sonarsource Apr 8, 2022
707f1ef
Use ILanguageFacade to get the string comparer
costin-zaharia-sonarsource Apr 11, 2022
4c64f75
Add test cases for VB
costin-zaharia-sonarsource Apr 11, 2022
bd51ce7
Add test cases also for C#
costin-zaharia-sonarsource Apr 11, 2022
6315f8b
Use for instead of foreach to improve perf
costin-zaharia-sonarsource Apr 11, 2022
8796377
Replace foreach with for & remove groupby call
costin-zaharia-sonarsource Apr 11, 2022
302abbe
Remove intermediate variable
costin-zaharia-sonarsource Apr 12, 2022
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
13 changes: 7 additions & 6 deletions analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,16 @@ namespace SonarAnalyzer.Helpers
{
internal sealed class CSharpFacade : ILanguageFacade<SyntaxKind>
{
private static readonly Lazy<CSharpFacade> Singleton = new Lazy<CSharpFacade>(() => new CSharpFacade());
private static readonly Lazy<AssignmentFinder> AssignmentFinderLazy = new Lazy<AssignmentFinder>(() => new CSharpAssignmentFinder());
private static readonly Lazy<IExpressionNumericConverter> ExpressionNumericConverterLazy = new Lazy<IExpressionNumericConverter>(() => new CSharpExpressionNumericConverter());
private static readonly Lazy<SyntaxFacade<SyntaxKind>> SyntaxLazy = new Lazy<SyntaxFacade<SyntaxKind>>(() => new CSharpSyntaxFacade());
private static readonly Lazy<ISyntaxKindFacade<SyntaxKind>> SyntaxKindLazy = new Lazy<ISyntaxKindFacade<SyntaxKind>>(() => new CSharpSyntaxKindFacade());
private static readonly Lazy<ITrackerFacade<SyntaxKind>> TrackerLazy = new Lazy<ITrackerFacade<SyntaxKind>>(() => new CSharpTrackerFacade());
private static readonly Lazy<CSharpFacade> Singleton = new(() => new CSharpFacade());
private static readonly Lazy<AssignmentFinder> AssignmentFinderLazy = new(() => new CSharpAssignmentFinder());
private static readonly Lazy<IExpressionNumericConverter> ExpressionNumericConverterLazy = new(() => new CSharpExpressionNumericConverter());
private static readonly Lazy<SyntaxFacade<SyntaxKind>> SyntaxLazy = new(() => new CSharpSyntaxFacade());
private static readonly Lazy<ISyntaxKindFacade<SyntaxKind>> SyntaxKindLazy = new(() => new CSharpSyntaxKindFacade());
private static readonly Lazy<ITrackerFacade<SyntaxKind>> TrackerLazy = new(() => new CSharpTrackerFacade());

public AssignmentFinder AssignmentFinder => AssignmentFinderLazy.Value;
public StringComparison NameComparison => StringComparison.Ordinal;
public StringComparer NameComparer => StringComparer.Ordinal;
public GeneratedCodeRecognizer GeneratedCodeRecognizer => CSharpGeneratedCodeRecognizer.Instance;
public IExpressionNumericConverter ExpressionNumericConverter => ExpressionNumericConverterLazy.Value;
public SyntaxFacade<SyntaxKind> Syntax => SyntaxLazy.Value;
Expand Down
71 changes: 65 additions & 6 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/Utilities/SymbolReferenceAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Collections.Generic;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Extensions;
using SonarAnalyzer.Helpers;
using StyleCop.Analyzers.Lightup;

namespace SonarAnalyzer.Rules.CSharp
{
Expand All @@ -36,11 +38,68 @@ public class SymbolReferenceAnalyzer : SymbolReferenceAnalyzerBase<SyntaxKind>
protected override SyntaxNode GetBindableParent(SyntaxToken token) =>
token.GetBindableParent();

protected override SyntaxToken? GetSetKeyword(ISymbol valuePropertySymbol) =>
IsValuePropertyParameter(valuePropertySymbol)
&& valuePropertySymbol.ContainingSymbol is IMethodSymbol methodSymbol
&& methodSymbol.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax() is AccessorDeclarationSyntax accessor
? accessor.Keyword
: null;
protected override ReferenceInfo[] CreateDeclarationReferenceInfo(SyntaxNode node, SemanticModel model) =>
node switch
{
BaseTypeDeclarationSyntax typeDeclaration => new[] { CreateDeclarationReferenceInfo(node, typeDeclaration.Identifier, model) },
VariableDeclarationSyntax variableDeclaration => CreateDeclarationReferenceInfo(variableDeclaration, model),
MethodDeclarationSyntax methodDeclaration => new[] { CreateDeclarationReferenceInfo(node, methodDeclaration.Identifier, model) },
ParameterSyntax parameterSyntax => new[] { CreateDeclarationReferenceInfo(node, parameterSyntax.Identifier, model) },
LocalDeclarationStatementSyntax localDeclarationStatement => CreateDeclarationReferenceInfo(localDeclarationStatement.Declaration, model),
PropertyDeclarationSyntax propertyDeclaration => new[] { CreateDeclarationReferenceInfo(node, propertyDeclaration.Identifier, model) },
TypeParameterSyntax typeParameterSyntax => new[] { CreateDeclarationReferenceInfo(node, typeParameterSyntax.Identifier, model) },
var localFunction when LocalFunctionStatementSyntaxWrapper.IsInstance(localFunction) =>
new[] { CreateDeclarationReferenceInfo(node, ((LocalFunctionStatementSyntaxWrapper)localFunction).Identifier, model) },
var singleVariableDesignation when SingleVariableDesignationSyntaxWrapper.IsInstance(singleVariableDesignation) =>
new[] { CreateDeclarationReferenceInfo(node, ((SingleVariableDesignationSyntaxWrapper)singleVariableDesignation).Identifier, model) },
_ => null
};

protected override IList<SyntaxNode> GetDeclarations(SyntaxNode node)
{
var walker = new DeclarationsFinder();
walker.SafeVisit(node);
return walker.Declarations;
}

private static ReferenceInfo[] CreateDeclarationReferenceInfo(VariableDeclarationSyntax declaration, SemanticModel model) =>
declaration.Variables.Select(x => CreateDeclarationReferenceInfo(x, x.Identifier, model)).ToArray();

private static ReferenceInfo CreateDeclarationReferenceInfo(SyntaxNode node, SyntaxToken identifier, SemanticModel model) =>
new(node, identifier, model.GetDeclaredSymbol(node), true);

private sealed class DeclarationsFinder : SafeCSharpSyntaxWalker
{
private readonly ISet<ushort> declarationKinds = new HashSet<SyntaxKind>
{
SyntaxKind.ClassDeclaration,
SyntaxKind.DelegateDeclaration,
SyntaxKind.EnumDeclaration,
SyntaxKind.EventDeclaration,
SyntaxKind.InterfaceDeclaration,
SyntaxKind.LocalDeclarationStatement,
SyntaxKind.MethodDeclaration,
SyntaxKind.Parameter,
SyntaxKind.PropertyDeclaration,
SyntaxKind.StructDeclaration,
SyntaxKind.TypeParameter,
SyntaxKind.VariableDeclaration,
SyntaxKindEx.LocalFunctionStatement,
SyntaxKindEx.RecordClassDeclaration,
SyntaxKindEx.RecordStructDeclaration,
SyntaxKindEx.SingleVariableDesignation
}.Cast<ushort>().ToHashSet();

public readonly List<SyntaxNode> Declarations = new();

public override void Visit(SyntaxNode node)
{
if (declarationKinds.Contains((ushort)node.RawKind))
{
Declarations.Add(node);
}
base.Visit(node);
}
}
}
}
1 change: 1 addition & 0 deletions analyzers/src/SonarAnalyzer.Common/Facade/ILanguageFacade.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public interface ILanguageFacade
{
AssignmentFinder AssignmentFinder { get; }
StringComparison NameComparison { get; }
StringComparer NameComparer { get; }
GeneratedCodeRecognizer GeneratedCodeRecognizer { get; }
IExpressionNumericConverter ExpressionNumericConverter { get; }
ResourceManager RspecResources { get; }
Expand Down
145 changes: 67 additions & 78 deletions analyzers/src/SonarAnalyzer.Common/Rules/Utilities/SymbolReferenceAnalyzerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Text;
using SonarAnalyzer.Helpers;
using SonarAnalyzer.Protobuf;

namespace SonarAnalyzer.Rules
Expand All @@ -33,40 +34,24 @@ public abstract class SymbolReferenceAnalyzerBase<TSyntaxKind> : UtilityAnalyzer
private const string Title = "Symbol reference calculator";
private const int TokenCountThreshold = 40_000;

private readonly ISet<SymbolKind> declarationKinds = new HashSet<SymbolKind>
{
SymbolKind.Event,
SymbolKind.Field,
SymbolKind.Local,
SymbolKind.Method,
SymbolKind.NamedType,
SymbolKind.Parameter,
SymbolKind.Property,
SymbolKind.TypeParameter
};
protected sealed override string FileName => "symrefs.pb";

protected abstract SyntaxNode GetBindableParent(SyntaxToken token);

protected sealed override string FileName => "symrefs.pb";
protected abstract ReferenceInfo[] CreateDeclarationReferenceInfo(SyntaxNode node, SemanticModel model);
csaba-sagi-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

protected abstract IList<SyntaxNode> GetDeclarations(SyntaxNode node);

protected SymbolReferenceAnalyzerBase() : base(DiagnosticId, Title) { }

protected sealed override SymbolReferenceInfo CreateMessage(SyntaxTree syntaxTree, SemanticModel semanticModel)
{
var allReferences = new List<SymRefInfo>();
var tokens = syntaxTree.GetRoot().DescendantTokens();
foreach (var token in tokens)
{
if (GetSymRefInfo(token, semanticModel) is { } reference)
{
allReferences.Add(reference);
}
}

var symbolReferenceInfo = new SymbolReferenceInfo { FilePath = syntaxTree.FilePath };
foreach (var allReference in allReferences.GroupBy(r => r.Symbol))
var references = GetReferences(syntaxTree.GetRoot(), semanticModel);

foreach (var symbol in references.Keys)
{
if (GetSymbolReference(allReference.ToArray(), syntaxTree) is { } reference)
if (GetSymbolReference(references[symbol], syntaxTree) is { } reference)
{
symbolReferenceInfo.Reference.Add(reference);
}
Expand All @@ -79,89 +64,93 @@ protected override bool ShouldGenerateMetrics(SyntaxTree tree) =>
base.ShouldGenerateMetrics(tree)
&& !HasTooManyTokens(tree);

protected virtual SyntaxToken? GetSetKeyword(ISymbol valuePropertySymbol) => null;

protected static bool IsValuePropertyParameter(ISymbol symbol) =>
symbol is IParameterSymbol {IsImplicitlyDeclared: true, Name: "value"};

private SymbolReferenceInfo.Types.SymbolReference GetSymbolReference(SymRefInfo[] allReference, SyntaxTree tree)
private Dictionary<ISymbol, List<ReferenceInfo>> GetReferences(SyntaxNode root, SemanticModel model)
{
TextSpan declarationSpan;
if (allReference.FirstOrDefault(r => r.IsDeclaration) is { } declaration)
{
declarationSpan = declaration.IdentifierToken.Span;
}
else
var references = new Dictionary<ISymbol, List<ReferenceInfo>>();
var knownIdentifiers = new HashSet<string>(Language.NameComparer);
var knownNodes = new List<SyntaxNode>();
var declarations = GetDeclarations(root);

for (var i = 0; i < declarations.Count; i++)
{
if (allReference.FirstOrDefault() is { } reference && GetSetKeyword(reference.Symbol) is { } setKeyword)
var declarationReferences = CreateDeclarationReferenceInfo(declarations[i], model);
if (declarationReferences == null)
{
declarationSpan = setKeyword.Span;
continue;
}
else

for (var j = 0; j < declarationReferences.Length; j++)
{
return null;
var currentDeclaration = declarationReferences[j];
if (currentDeclaration.Symbol != null)
{
references.GetOrAdd(currentDeclaration.Symbol, _ => new List<ReferenceInfo>())
.Add(currentDeclaration);
knownNodes.Add(currentDeclaration.Node);
knownIdentifiers.Add(currentDeclaration.Identifier.ValueText);
}
}
}

var sr = new SymbolReferenceInfo.Types.SymbolReference { Declaration = GetTextRange(Location.Create(tree, declarationSpan).GetLineSpan()) };
foreach (var reference in allReference.Where(r => !r.IsDeclaration).Select(r => r.IdentifierToken))
foreach (var token in root.DescendantTokens())
{
sr.Reference.Add(GetTextRange(Location.Create(tree, reference.Span).GetLineSpan()));
if (Language.Syntax.IsKind(token, Language.SyntaxKind.IdentifierToken)
&& knownIdentifiers.Contains(token.Text)
&& GetBindableParent(token) is { } parent
&& !knownNodes.Contains(parent)
&& GetReferenceSymbol(parent, model) is { } symbol
&& references.ContainsKey(symbol)
&& references[symbol] is { } symbolRefs)
{
symbolRefs.Add(new ReferenceInfo(parent, token, symbol, false));
}
}
return sr;

return references;
}

private SymRefInfo GetSymRefInfo(SyntaxToken token, SemanticModel semanticModel)
private static ISymbol GetReferenceSymbol(SyntaxNode node, SemanticModel model) =>
model.GetSymbolInfo(node).Symbol switch
{
IMethodSymbol { MethodKind: MethodKind.Constructor, IsImplicitlyDeclared: true } constructor => constructor.ContainingType,
var symbol => symbol
};

private static SymbolReferenceInfo.Types.SymbolReference GetSymbolReference(List<ReferenceInfo> references, SyntaxTree tree)
{
if (!Language.Syntax.IsKind(token, Language.SyntaxKind.IdentifierToken))
var declarationSpan = GetDeclarationSpan(references);
if (!declarationSpan.HasValue)
{
// For the time being, we only handle identifier tokens.
// We could also handle keywords, such as this, base
return null;
}

if (semanticModel.GetDeclaredSymbol(token.Parent) is { } declaredSymbol)
var symbolReference = new SymbolReferenceInfo.Types.SymbolReference { Declaration = GetTextRange(Location.Create(tree, declarationSpan.Value).GetLineSpan()) };
for (var i = 0; i < references.Count; i++)
{
return declarationKinds.Contains(declaredSymbol.Kind)
? new SymRefInfo(token, declaredSymbol, true)
: null;
var reference = references[i];
if (!reference.IsDeclaration)
{
symbolReference.Reference.Add(GetTextRange(Location.Create(tree, reference.Identifier.Span).GetLineSpan()));
}
}
return symbolReference;
}

if (GetBindableParent(token) is { } node)
private static TextSpan? GetDeclarationSpan(List<ReferenceInfo> references)
{
for (var i = 0; i < references.Count; i++)
{
var symbol = semanticModel.GetSymbolInfo(node).Symbol;
if (symbol == null)
{
return null;
}
else if (symbol.DeclaringSyntaxReferences.Any() || IsValuePropertyParameter(symbol))
if (references[i].IsDeclaration)
{
return new SymRefInfo(token, symbol);
}
else if (symbol is IMethodSymbol ctorSymbol && ctorSymbol.MethodKind == MethodKind.Constructor && ctorSymbol.IsImplicitlyDeclared)
{
return new SymRefInfo(token, ctorSymbol.ContainingType);
return references[i].Identifier.Span;
}
}

return null;
}

private static bool HasTooManyTokens(SyntaxTree syntaxTree) =>
syntaxTree.GetRoot().DescendantTokens().Count() > TokenCountThreshold;
csaba-sagi-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

private class SymRefInfo
{
public SyntaxToken IdentifierToken { get; }
public ISymbol Symbol { get; }
public bool IsDeclaration { get; }

public SymRefInfo(SyntaxToken identifierToken, ISymbol symbol, bool isDeclaration = false)
{
IdentifierToken = identifierToken;
Symbol = symbol;
IsDeclaration = isDeclaration;
}
}
protected sealed record ReferenceInfo(SyntaxNode Node, SyntaxToken Identifier, ISymbol Symbol, bool IsDeclaration);
}
}
13 changes: 7 additions & 6 deletions analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicFacade.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,16 @@ namespace SonarAnalyzer.Helpers
{
internal sealed class VisualBasicFacade : ILanguageFacade<SyntaxKind>
{
private static readonly Lazy<VisualBasicFacade> Singleton = new Lazy<VisualBasicFacade>(() => new VisualBasicFacade());
private static readonly Lazy<AssignmentFinder> AssignmentFinderLazy = new Lazy<AssignmentFinder>(() => new VisualBasicAssignmentFinder());
private static readonly Lazy<IExpressionNumericConverter> ExpressionNumericConverterLazy = new Lazy<IExpressionNumericConverter>(() => new VisualBasicExpressionNumericConverter());
private static readonly Lazy<SyntaxFacade<SyntaxKind>> SyntaxLazy = new Lazy<SyntaxFacade<SyntaxKind>>(() => new VisualBasicSyntaxFacade());
private static readonly Lazy<ISyntaxKindFacade<SyntaxKind>> SyntaxKindLazy = new Lazy<ISyntaxKindFacade<SyntaxKind>>(() => new VisualBasicSyntaxKindFacade());
private static readonly Lazy<ITrackerFacade<SyntaxKind>> TrackerLazy = new Lazy<ITrackerFacade<SyntaxKind>>(() => new VisualBasicTrackerFacade());
private static readonly Lazy<VisualBasicFacade> Singleton = new(() => new VisualBasicFacade());
private static readonly Lazy<AssignmentFinder> AssignmentFinderLazy = new(() => new VisualBasicAssignmentFinder());
private static readonly Lazy<IExpressionNumericConverter> ExpressionNumericConverterLazy = new(() => new VisualBasicExpressionNumericConverter());
private static readonly Lazy<SyntaxFacade<SyntaxKind>> SyntaxLazy = new(() => new VisualBasicSyntaxFacade());
private static readonly Lazy<ISyntaxKindFacade<SyntaxKind>> SyntaxKindLazy = new(() => new VisualBasicSyntaxKindFacade());
private static readonly Lazy<ITrackerFacade<SyntaxKind>> TrackerLazy = new(() => new VisualBasicTrackerFacade());

public AssignmentFinder AssignmentFinder => AssignmentFinderLazy.Value;
public StringComparison NameComparison => StringComparison.OrdinalIgnoreCase;
public StringComparer NameComparer => StringComparer.OrdinalIgnoreCase;
public GeneratedCodeRecognizer GeneratedCodeRecognizer => VisualBasicGeneratedCodeRecognizer.Instance;
public IExpressionNumericConverter ExpressionNumericConverter => ExpressionNumericConverterLazy.Value;
public SyntaxFacade<SyntaxKind> Syntax => SyntaxLazy.Value;
Expand Down
Loading