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

More classification perf fixes. #73535

Merged
merged 13 commits into from
May 17, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void VisitTokens(SyntaxNode node)
while (stack.TryPop(out var currentNodeOrToken))
{
_cancellationToken.ThrowIfCancellationRequested();
if (currentNodeOrToken.Span.IntersectsWith(_textSpan))
if (currentNodeOrToken.FullSpan.IntersectsWith(_textSpan))
{
if (currentNodeOrToken.IsNode)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,36 +29,21 @@ public override void AddClassifications(
SegmentedList<ClassifiedSpan> result,
CancellationToken cancellationToken)
{
if (syntax is NameSyntax name)
{
if (syntax is SimpleNameSyntax name)
ClassifyTypeSyntax(name, semanticModel, result, cancellationToken);
}
}

public override ImmutableArray<Type> SyntaxNodeTypes { get; } =
[
typeof(AliasQualifiedNameSyntax),
typeof(GenericNameSyntax),
typeof(IdentifierNameSyntax),
typeof(QualifiedNameSyntax),
typeof(SimpleNameSyntax),
typeof(GenericNameSyntax),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to check these other 'composed name types'. We only care about X and X<...> (and we only classifiy the 'X' part of each). Every other name type composes into one of these two name types that we actually classify here.

this addresses a lot of callbacks for dotted names that serve no purpose.

];

protected override int? GetRightmostNameArity(SyntaxNode node)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused method.

{
if (node is ExpressionSyntax expressionSyntax)
{
return expressionSyntax.GetRightmostName()?.Arity;
}

return null;
}

protected override bool IsParentAnAttribute(SyntaxNode node)
=> node.IsParentKind(SyntaxKind.Attribute);

private void ClassifyTypeSyntax(
NameSyntax name,
SimpleNameSyntax name,
SemanticModel semanticModel,
SegmentedList<ClassifiedSpan> result,
CancellationToken cancellationToken)
Expand All @@ -73,7 +58,7 @@ private void ClassifyTypeSyntax(
}

private bool TryClassifySymbol(
NameSyntax name,
SimpleNameSyntax name,
SymbolInfo symbolInfo,
SegmentedList<ClassifiedSpan> result)
{
Expand Down Expand Up @@ -104,7 +89,7 @@ CandidateReason.Ambiguous or
}

private static bool TryClassifyAmbiguousSymbol(
NameSyntax name,
SimpleNameSyntax name,
SymbolInfo symbolInfo,
SegmentedList<ClassifiedSpan> result)
{
Expand Down Expand Up @@ -138,19 +123,19 @@ private static bool TryClassifyAmbiguousSymbol(
}

private static bool TryClassifySymbol(
NameSyntax name,
SimpleNameSyntax name,
[NotNullWhen(returnValue: true)] ISymbol? symbol,
out ClassifiedSpan classifiedSpan)
{
// For Namespace parts, we want don't want to classify the QualifiedNameSyntax
// nodes, we instead wait for the each IdentifierNameSyntax node to avoid
// creating overlapping ClassifiedSpans.
if (symbol is INamespaceSymbol namespaceSymbol &&
name is IdentifierNameSyntax identifierNameSyntax)
name is IdentifierNameSyntax)
{
// Do not classify the global:: namespace. It is already syntactically classified as a keyword.
var isGlobalNamespace = namespaceSymbol.IsGlobalNamespace &&
identifierNameSyntax.Identifier.IsKind(SyntaxKind.GlobalKeyword);
name.Identifier.IsKind(SyntaxKind.GlobalKeyword);
if (isGlobalNamespace)
{
classifiedSpan = default;
Expand Down Expand Up @@ -281,7 +266,7 @@ private static string GetClassificationForMethod(IMethodSymbol methodSymbol)
: ClassificationTypeNames.MethodName;
}

private static bool IsInVarContext(NameSyntax name)
private static bool IsInVarContext(SimpleNameSyntax name)
{
return
name.CheckParent<RefTypeSyntax>(v => v.Type == name) ||
Expand All @@ -293,18 +278,18 @@ private static bool IsInVarContext(NameSyntax name)
}

private static bool TryClassifyFromIdentifier(
NameSyntax name,
SimpleNameSyntax name,
SymbolInfo symbolInfo,
SegmentedList<ClassifiedSpan> result)
{
// Okay - it wasn't a type. If the syntax matches "var q = from" or "q = from", and from
// doesn't bind to anything then optimistically color from as a keyword.
if (name is IdentifierNameSyntax identifierName &&
identifierName.Identifier.HasMatchingText(SyntaxKind.FromKeyword) &&
if (name is IdentifierNameSyntax &&
name.Identifier.HasMatchingText(SyntaxKind.FromKeyword) &&
symbolInfo.Symbol == null)
{
var token = identifierName.Identifier;
if (identifierName.IsRightSideOfAnyAssignExpression() || identifierName.IsVariableDeclaratorValue())
var token = name.Identifier;
if (name.IsRightSideOfAnyAssignExpression() || name.IsVariableDeclaratorValue())
{
result.Add(new ClassifiedSpan(token.Span, ClassificationTypeNames.Keyword));
return true;
Expand All @@ -315,21 +300,20 @@ private static bool TryClassifyFromIdentifier(
}

private static bool TryClassifyValueIdentifier(
NameSyntax name,
SimpleNameSyntax name,
SymbolInfo symbolInfo,
SegmentedList<ClassifiedSpan> result)
{
if (name is IdentifierNameSyntax identifierName &&
symbolInfo.Symbol.IsImplicitValueParameter())
if (symbolInfo.Symbol.IsImplicitValueParameter())
{
result.Add(new ClassifiedSpan(identifierName.Identifier.Span, ClassificationTypeNames.Keyword));
result.Add(new ClassifiedSpan(name.Identifier.Span, ClassificationTypeNames.Keyword));
return true;
}

return false;
}

private static bool TryClassifySomeContextualKeywordIdentifiersAsKeywords(NameSyntax name, SymbolInfo symbolInfo, SegmentedList<ClassifiedSpan> result)
private static bool TryClassifySomeContextualKeywordIdentifiersAsKeywords(SimpleNameSyntax name, SymbolInfo symbolInfo, SegmentedList<ClassifiedSpan> result)
{
// Simple approach, if the user ever types one of identifiers from the list and it doesn't actually bind to anything, presume that
// they intend to use it as a keyword. This works for all error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ namespace Microsoft.CodeAnalysis.Classification.Classifiers;

internal abstract class AbstractNameSyntaxClassifier : AbstractSyntaxClassifier
{
protected abstract int? GetRightmostNameArity(SyntaxNode node);
protected abstract bool IsParentAnAttribute(SyntaxNode node);

protected ISymbol? TryGetSymbol(SyntaxNode node, SymbolInfo symbolInfo)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Classification.Classifiers
End If
End Sub

Protected Overrides Function GetRightmostNameArity(node As SyntaxNode) As Integer?
If TypeOf (node) Is ExpressionSyntax Then
Return DirectCast(node, ExpressionSyntax).GetRightmostName()?.Arity
End If

Return Nothing
End Function

Protected Overrides Function IsParentAnAttribute(node As SyntaxNode) As Boolean
Return node.IsParentKind(SyntaxKind.Attribute)
End Function
Expand Down
Loading