Skip to content

Commit

Permalink
Merge pull request #68452 from jasonmalinowski/symbol-display-service…
Browse files Browse the repository at this point in the history 
…-cleanups-and-perf-improvements

Ensure we only ask for a symbol's documentation comment once
  • Loading branch information
@jasonmalinowski
jasonmalinowski authored Jun 7, 2023
2 parents 4e1b952 + 950bb60 commit 91afe09
Show file tree
Hide file tree
Showing 14 changed files with 88 additions and 118 deletions.
1 change: 1 addition & 0 deletions .editorconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ root = true
[*]
indent_style = space
# (Please don't specify an indent_size here; that has too many unintended consequences.)
spelling_exclusion_path = SpellingExclusions.dic

# Code files
[*.{cs,csx,vb,vbx}]
Expand Down
1 change: 1 addition & 0 deletions SpellingExclusions.dic
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
awaitable
20 changes: 7 additions & 13 deletions ...s/CSharp/Portable/LanguageServices/CSharpSymbolDisplayService.SymbolDescriptionBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,13 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Classification;
using Microsoft.CodeAnalysis.Classification.Classifiers;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;
Expand Down Expand Up @@ -41,11 +36,10 @@ protected class SymbolDescriptionBuilder : AbstractSymbolDescriptionBuilder
public SymbolDescriptionBuilder(
SemanticModel semanticModel,
int position,
SolutionServices services,
IStructuralTypeDisplayService structuralTypeDisplayService,
Host.LanguageServices languageServices,
SymbolDescriptionOptions options,
CancellationToken cancellationToken)
: base(semanticModel, position, services, structuralTypeDisplayService, options, cancellationToken)
: base(semanticModel, position, languageServices, options, cancellationToken)
{
}

Expand Down Expand Up @@ -116,13 +110,13 @@ protected override Task<ImmutableArray<SymbolDisplayPart>> GetInitializerSourceP
protected override ImmutableArray<SymbolDisplayPart> ToMinimalDisplayParts(ISymbol symbol, SemanticModel semanticModel, int position, SymbolDisplayFormat format)
=> CodeAnalysis.CSharp.SymbolDisplay.ToMinimalDisplayParts(symbol, semanticModel, position, format);

protected override string GetNavigationHint(ISymbol symbol)
protected override string? GetNavigationHint(ISymbol symbol)
=> symbol == null ? null : CodeAnalysis.CSharp.SymbolDisplay.ToDisplayString(symbol, SymbolDisplayFormat.MinimallyQualifiedFormat);

private async Task<ImmutableArray<SymbolDisplayPart>> GetInitializerSourcePartsAsync(
IFieldSymbol symbol)
{
EqualsValueClauseSyntax initializer = null;
EqualsValueClauseSyntax? initializer = null;

var variableDeclarator = await GetFirstDeclarationAsync<VariableDeclaratorSyntax>(symbol).ConfigureAwait(false);
if (variableDeclarator != null)
Expand Down Expand Up @@ -171,7 +165,7 @@ private async Task<ImmutableArray<SymbolDisplayPart>> GetInitializerSourcePartsA
return ImmutableArray<SymbolDisplayPart>.Empty;
}

private async Task<T> GetFirstDeclarationAsync<T>(ISymbol symbol) where T : SyntaxNode
private async Task<T?> GetFirstDeclarationAsync<T>(ISymbol symbol) where T : SyntaxNode
{
foreach (var syntaxRef in symbol.DeclaringSyntaxReferences)
{
Expand All @@ -186,15 +180,15 @@ private async Task<T> GetFirstDeclarationAsync<T>(ISymbol symbol) where T : Synt
}

private async Task<ImmutableArray<SymbolDisplayPart>> GetInitializerSourcePartsAsync(
EqualsValueClauseSyntax equalsValue)
EqualsValueClauseSyntax? equalsValue)
{
if (equalsValue != null && equalsValue.Value != null)
{
var semanticModel = GetSemanticModel(equalsValue.SyntaxTree);
if (semanticModel != null)
{
return await Classifier.GetClassifiedSymbolDisplayPartsAsync(
Services, semanticModel, equalsValue.Value.Span,
LanguageServices, semanticModel, equalsValue.Value.Span,
Options.ClassificationOptions, cancellationToken: CancellationToken).ConfigureAwait(false);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Features/CSharp/Portable/LanguageServices/CSharpSymbolDisplayService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ public CSharpSymbolDisplayService(Host.LanguageServices services)
}

protected override AbstractSymbolDescriptionBuilder CreateDescriptionBuilder(SemanticModel semanticModel, int position, SymbolDescriptionOptions options, CancellationToken cancellationToken)
=> new SymbolDescriptionBuilder(semanticModel, position, Services.SolutionServices, AnonymousTypeDisplayService, options, cancellationToken);
=> new SymbolDescriptionBuilder(semanticModel, position, LanguageServices, options, cancellationToken);
}
}
100 changes: 31 additions & 69 deletions ...ces/SymbolDisplayService/AbstractSymbolDisplayService.AbstractSymbolDescriptionBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,18 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Classification;
using Microsoft.CodeAnalysis.Classification.Classifiers;
using Microsoft.CodeAnalysis.DocumentationComments;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.QuickInfo;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.LanguageService
Expand Down Expand Up @@ -83,25 +79,22 @@ protected abstract partial class AbstractSymbolDescriptionBuilder

private readonly SemanticModel _semanticModel;
private readonly int _position;
private readonly IStructuralTypeDisplayService _structuralTypeDisplayService;
private readonly Dictionary<SymbolDescriptionGroups, IList<SymbolDisplayPart>> _groupMap = new();
private readonly Dictionary<SymbolDescriptionGroups, ImmutableArray<TaggedText>> _documentationMap = new();
private readonly Func<ISymbol, string> _getNavigationHint;
private readonly Func<ISymbol, string?> _getNavigationHint;

protected readonly SolutionServices Services;
protected readonly LanguageServices LanguageServices;
protected readonly SymbolDescriptionOptions Options;
protected readonly CancellationToken CancellationToken;

protected AbstractSymbolDescriptionBuilder(
SemanticModel semanticModel,
int position,
SolutionServices services,
IStructuralTypeDisplayService structuralTypeDisplayService,
LanguageServices languageServices,
SymbolDescriptionOptions options,
CancellationToken cancellationToken)
{
_structuralTypeDisplayService = structuralTypeDisplayService;
Services = services;
LanguageServices = languageServices;
Options = options;
CancellationToken = cancellationToken;
_semanticModel = semanticModel;
Expand All @@ -116,7 +109,7 @@ protected AbstractSymbolDescriptionBuilder(
protected abstract void AddEnumUnderlyingTypeSeparator();
protected abstract Task<ImmutableArray<SymbolDisplayPart>> GetInitializerSourcePartsAsync(ISymbol symbol);
protected abstract ImmutableArray<SymbolDisplayPart> ToMinimalDisplayParts(ISymbol symbol, SemanticModel semanticModel, int position, SymbolDisplayFormat format);
protected abstract string GetNavigationHint(ISymbol symbol);
protected abstract string? GetNavigationHint(ISymbol symbol);

protected abstract SymbolDisplayFormat MinimallyQualifiedFormat { get; }
protected abstract SymbolDisplayFormat MinimallyQualifiedFormatWithConstants { get; }
Expand Down Expand Up @@ -151,48 +144,31 @@ protected SemanticModel GetSemanticModel(SyntaxTree tree)
return null;
}

protected Compilation GetCompilation()
protected Compilation Compilation
=> _semanticModel.Compilation;

private async Task AddPartsAsync(ImmutableArray<ISymbol> symbols)
{
var firstSymbol = symbols[0];

// Grab the doc comment once as computing it for each portion we're concatenating can be expensive for
// LSIF (which does this for every symbol in an entire solution).
var firstSymbolDocumentationComment = firstSymbol.GetAppropriateDocumentationComment(Compilation, CancellationToken);

await AddDescriptionPartAsync(firstSymbol).ConfigureAwait(false);

AddOverloadCountPart(symbols);
FixAllStructuralTypes(firstSymbol);
AddExceptions(firstSymbol);
AddExceptions(firstSymbolDocumentationComment);
AddCaptures(firstSymbol);

AddDocumentationContent(firstSymbol);
AddDocumentationContent(firstSymbol, firstSymbolDocumentationComment);
}

private void AddDocumentationContent(ISymbol symbol)
private void AddDocumentationContent(ISymbol symbol, DocumentationComment documentationComment)
{
var formatter = Services.GetRequiredLanguageService<IDocumentationCommentFormattingService>(_semanticModel.Language);

if (symbol is IParameterSymbol or ITypeParameterSymbol)
{
// Can just defer to the standard helper here. We only want to get the summary portion for just the
// param/type-param and we have no need for remarks/returns/value.
_documentationMap.Add(
SymbolDescriptionGroups.Documentation,
symbol.GetDocumentationParts(_semanticModel, _position, formatter, CancellationToken));
return;
}

if (symbol is IAliasSymbol alias)
symbol = alias.Target;

var original = symbol.OriginalDefinition;
var formatter = LanguageServices.GetRequiredService<IDocumentationCommentFormattingService>();
var format = ISymbolExtensions2.CrefFormat;
var compilation = _semanticModel.Compilation;

// Grab the doc comment once as computing it for each portion we're concatenating can be expensive for
// lsif (which does this for every symbol in an entire solution).
var documentationComment = original is IMethodSymbol method
? ISymbolExtensions2.GetMethodDocumentation(method, compilation, CancellationToken)
: original.GetDocumentationComment(compilation, expandIncludes: true, expandInheritdoc: true, cancellationToken: CancellationToken);

_documentationMap.Add(
SymbolDescriptionGroups.Documentation,
Expand All @@ -202,54 +178,40 @@ private void AddDocumentationContent(ISymbol symbol)
SymbolDescriptionGroups.RemarksDocumentation,
formatter.Format(documentationComment.RemarksText, symbol, _semanticModel, _position, format, CancellationToken));

AddReturnsDocumentationParts(symbol, formatter);
AddValueDocumentationParts(symbol, formatter);
AddDocumentationPartsWithPrefix(documentationComment.ReturnsText, SymbolDescriptionGroups.ReturnsDocumentation, FeaturesResources.Returns_colon);
AddDocumentationPartsWithPrefix(documentationComment.ValueText, SymbolDescriptionGroups.ValueDocumentation, FeaturesResources.Value_colon);

return;

void AddReturnsDocumentationParts(ISymbol symbol, IDocumentationCommentFormattingService formatter)
void AddDocumentationPartsWithPrefix(string? rawXmlText, SymbolDescriptionGroups group, string prefix)
{
var parts = formatter.Format(documentationComment.ReturnsText, symbol, _semanticModel, _position, format, CancellationToken);
if (!parts.IsDefaultOrEmpty)
{
using var _ = ArrayBuilder<TaggedText>.GetInstance(out var builder);

builder.Add(new TaggedText(TextTags.Text, FeaturesResources.Returns_colon));
builder.AddRange(LineBreak().ToTaggedText());
builder.Add(new TaggedText(TextTags.ContainerStart, " "));
builder.AddRange(parts);
builder.Add(new TaggedText(TextTags.ContainerEnd, string.Empty));

_documentationMap.Add(SymbolDescriptionGroups.ReturnsDocumentation, builder.ToImmutable());
}
}
if (string.IsNullOrEmpty(rawXmlText))
return;

void AddValueDocumentationParts(ISymbol symbol, IDocumentationCommentFormattingService formatter)
{
var parts = formatter.Format(documentationComment.ValueText, symbol, _semanticModel, _position, format, CancellationToken);
var parts = formatter.Format(rawXmlText, symbol, _semanticModel, _position, format, CancellationToken);
if (!parts.IsDefaultOrEmpty)
{
using var _ = ArrayBuilder<TaggedText>.GetInstance(out var builder);
builder.Add(new TaggedText(TextTags.Text, FeaturesResources.Value_colon));

builder.Add(new TaggedText(TextTags.Text, prefix));
builder.AddRange(LineBreak().ToTaggedText());
builder.Add(new TaggedText(TextTags.ContainerStart, " "));
builder.AddRange(parts);
builder.Add(new TaggedText(TextTags.ContainerEnd, string.Empty));

_documentationMap.Add(SymbolDescriptionGroups.ValueDocumentation, builder.ToImmutable());
_documentationMap.Add(group, builder.ToImmutable());
}
}
}

private void AddExceptions(ISymbol symbol)
private void AddExceptions(DocumentationComment documentationComment)
{
var exceptionTypes = symbol.GetDocumentationComment(GetCompilation(), expandIncludes: true, expandInheritdoc: true).ExceptionTypes;
if (exceptionTypes.Any())
if (documentationComment.ExceptionTypes.Any())
{
var parts = new List<SymbolDisplayPart>();
parts.AddLineBreak();
parts.AddText(WorkspacesResources.Exceptions_colon);
foreach (var exceptionString in exceptionTypes)
foreach (var exceptionString in documentationComment.ExceptionTypes)
{
parts.AddRange(LineBreak());
parts.AddRange(Space(count: 2));
Expand Down Expand Up @@ -509,7 +471,7 @@ private void AddDescriptionForNamedType(INamedTypeSymbol symbol)
AddTypeParameterMapPart(allTypeParameters, allTypeArguments);
}

if (symbol.IsEnumType() && symbol.EnumUnderlyingType.SpecialType != SpecialType.System_Int32)
if (symbol.IsEnumType() && symbol.EnumUnderlyingType!.SpecialType != SpecialType.System_Int32)
{
AddEnumUnderlyingTypeSeparator();
var underlyingTypeDisplayParts = symbol.EnumUnderlyingType.ToDisplayParts(s_descriptionStyle.WithMiscellaneousOptions(SymbolDisplayMiscellaneousOptions.UseSpecialTypes));
Expand Down Expand Up @@ -837,13 +799,13 @@ protected static IEnumerable<SymbolDisplayPart> Space(int count = 1)
yield return new SymbolDisplayPart(SymbolDisplayPartKind.Space, null, new string(' ', count));
}

protected ImmutableArray<SymbolDisplayPart> ToMinimalDisplayParts(ISymbol symbol, SymbolDisplayFormat format = null)
protected ImmutableArray<SymbolDisplayPart> ToMinimalDisplayParts(ISymbol symbol, SymbolDisplayFormat? format = null)
{
format ??= MinimallyQualifiedFormat;
return ToMinimalDisplayParts(symbol, _semanticModel, _position, format);
}

private static IEnumerable<SymbolDisplayPart> Part(SymbolDisplayPartKind kind, ISymbol symbol, string text)
private static IEnumerable<SymbolDisplayPart> Part(SymbolDisplayPartKind kind, ISymbol? symbol, string text)
{
yield return new SymbolDisplayPart(kind, symbol, text);
}
Expand Down
2 changes: 1 addition & 1 deletion ...able/LanguageServices/SymbolDisplayService/AbstractSymbolDisplayService.AnonymousTypes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ where part.Symbol.IsAnonymousType() || part.Symbol.IsTupleType()
if (firstSymbol.IsAnonymousDelegateType())
directStructuralTypes = directStructuralTypes.Except(new[] { (INamedTypeSymbol)firstSymbol });

var info = _structuralTypeDisplayService.GetTypeDisplayInfo(
var info = LanguageServices.GetRequiredService<IStructuralTypeDisplayService>().GetTypeDisplayInfo(
firstSymbol, directStructuralTypes.ToImmutableArrayOrEmpty(), _semanticModel, _position);

if (info.TypesParts.Count > 0)
Expand Down
10 changes: 2 additions & 8 deletions ...tures/Core/Portable/LanguageServices/SymbolDisplayService/AbstractSymbolDisplayService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,22 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Classification;
using Microsoft.CodeAnalysis.Classification.Classifiers;
using Microsoft.CodeAnalysis.Host;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.LanguageService
{
internal abstract partial class AbstractSymbolDisplayService : ISymbolDisplayService
{
protected readonly LanguageServices Services;
protected readonly IStructuralTypeDisplayService AnonymousTypeDisplayService;
protected readonly LanguageServices LanguageServices;

protected AbstractSymbolDisplayService(LanguageServices services)
{
Services = services;
AnonymousTypeDisplayService = services.GetService<IStructuralTypeDisplayService>();
LanguageServices = services;
}

protected abstract AbstractSymbolDescriptionBuilder CreateDescriptionBuilder(SemanticModel semanticModel, int position, SymbolDescriptionOptions options, CancellationToken cancellationToken);
Expand Down
Loading

0 comments on commit 91afe09

Please sign in to comment.