Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -6997,6 +6997,40 @@ void N(string? s)
MainDescription($"({FeaturesResources.parameter}) string? s"),
NullabilityAnalysis(string.Format(FeaturesResources._0_may_be_null_here, "s")));

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/42543")]
public Task NullableParameterThatIsMaybeNull_Suppressed1()
=> TestWithOptionsAsync(TestOptions.Regular8,
"""
#nullable enable

class X
{
void N(string? s)
{
string s2 = $$s!;
}
}
""",
MainDescription($"({FeaturesResources.parameter}) string? s"),
NullabilityAnalysis(string.Format(FeaturesResources._0_may_be_null_here, "s")));

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/42543")]
public Task NullableParameterThatIsMaybeNull_Suppressed2()
=> TestWithOptionsAsync(TestOptions.Regular8,
"""
#nullable enable

class X
{
void N(string? s)
{
string s2 = $$s!!;
}
}
""",
MainDescription($"({FeaturesResources.parameter}) string? s"),
NullabilityAnalysis(string.Format(FeaturesResources._0_may_be_null_here, "s")));

[Fact]
public Task NullableParameterThatIsNotNull()
=> TestWithOptionsAsync(TestOptions.Regular8,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,23 +83,23 @@ protected override bool GetBindableNodeForTokenIndicatingMemberAccess(SyntaxToke
protected override bool ShouldCheckPreviousToken(SyntaxToken token)
=> !token.Parent.IsKind(SyntaxKind.XmlCrefAttribute);

protected override (NullableAnnotation, NullableFlowState) GetNullabilityAnalysis(SemanticModel semanticModel, ISymbol symbol, SyntaxNode node, CancellationToken cancellationToken)
protected override (NullableAnnotation, NullableFlowState) GetNullabilityAnalysis(
SemanticModel semanticModel,
ISymbol symbol,
SyntaxNode node,
CancellationToken cancellationToken)
{
// Anything less than C# 8 we just won't show anything, even if the compiler could theoretically give analysis
var parseOptions = (CSharpParseOptions)semanticModel.SyntaxTree!.Options;
if (parseOptions.LanguageVersion < LanguageVersion.CSharp8)
{
return default;
}

// If the user doesn't have nullable enabled, don't show anything. For now we're not trying to be more precise if the user has just annotations or just
// warnings. If the user has annotations off then things that are oblivious might become non-null (which is a lie) and if the user has warnings off then
// that probably implies they're not actually trying to know if their code is correct. We can revisit this if we have specific user scenarios.
var nullableContext = semanticModel.GetNullableContext(node.SpanStart);
if (!nullableContext.WarningsEnabled() || !nullableContext.AnnotationsEnabled())
{
return default;
}

// Although GetTypeInfo can return nullability for uses of all sorts of things, it's not always useful for quick info.
// For example, if you have a call to a method with a nullable return, the fact it can be null is already captured
Expand Down Expand Up @@ -128,9 +128,7 @@ protected override (NullableAnnotation, NullableFlowState) GetNullabilityAnalysi
}

if (symbol.GetMemberType() is { IsValueType: false, NullableAnnotation: NullableAnnotation.None })
{
return (NullableAnnotation.None, NullableFlowState.NotNull);
}

var typeInfo = semanticModel.GetTypeInfo(node, cancellationToken);

Expand All @@ -139,9 +137,7 @@ protected override (NullableAnnotation, NullableFlowState) GetNullabilityAnalysi
// Nullable<int>, which isn't exactly the same. To avoid confusion and
// extra noise, we won't show nullable flow state for value types
if (typeInfo.Type?.IsValueType == true)
{
return default;
}

var nullability = typeInfo.Nullability;
return (nullability.Annotation, nullability.FlowState);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,12 @@

namespace Microsoft.CodeAnalysis.LanguageService;

internal abstract partial class AbstractSymbolDisplayService : ISymbolDisplayService
internal abstract partial class AbstractSymbolDisplayService(LanguageServices services) : ISymbolDisplayService
{
protected readonly LanguageServices LanguageServices;

protected AbstractSymbolDisplayService(LanguageServices services)
{
LanguageServices = services;
}
protected readonly LanguageServices LanguageServices = services;

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

public Task<string> ToDescriptionStringAsync(SemanticModel semanticModel, int position, ISymbol symbol, SymbolDescriptionOptions options, SymbolDescriptionGroups groups, CancellationToken cancellationToken)
=> ToDescriptionStringAsync(semanticModel, position, [symbol], options, groups, cancellationToken);

public async Task<string> ToDescriptionStringAsync(SemanticModel semanticModel, int position, ImmutableArray<ISymbol> symbols, SymbolDescriptionOptions options, SymbolDescriptionGroups groups, CancellationToken cancellationToken)
{
var parts = await ToDescriptionPartsAsync(semanticModel, position, symbols, options, groups, cancellationToken).ConfigureAwait(false);
return parts.ToDisplayString();
}

public Task<ImmutableArray<SymbolDisplayPart>> ToDescriptionPartsAsync(SemanticModel semanticModel, int position, ImmutableArray<ISymbol> symbols, SymbolDescriptionOptions options, SymbolDescriptionGroups groups, CancellationToken cancellationToken)
{
if (symbols.Length == 0)
Expand All @@ -45,9 +31,7 @@ public async Task<IDictionary<SymbolDescriptionGroups, ImmutableArray<TaggedText
SemanticModel semanticModel, int position, ImmutableArray<ISymbol> symbols, SymbolDescriptionOptions options, CancellationToken cancellationToken)
{
if (symbols.Length == 0)
{
return SpecializedCollections.EmptyDictionary<SymbolDescriptionGroups, ImmutableArray<TaggedText>>();
}

var builder = CreateDescriptionBuilder(semanticModel, position, options, cancellationToken);
return await builder.BuildDescriptionSectionsAsync(symbols).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,21 @@ namespace Microsoft.CodeAnalysis.LanguageService;

internal interface ISymbolDisplayService : ILanguageService
{
Task<string> ToDescriptionStringAsync(SemanticModel semanticModel, int position, ISymbol symbol, SymbolDescriptionOptions options, SymbolDescriptionGroups groups = SymbolDescriptionGroups.All, CancellationToken cancellationToken = default);
Task<string> ToDescriptionStringAsync(SemanticModel semanticModel, int position, ImmutableArray<ISymbol> symbols, SymbolDescriptionOptions options, SymbolDescriptionGroups groups = SymbolDescriptionGroups.All, CancellationToken cancellationToken = default);
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 for these to be on the interface. they just defer to the other two methods on the type. moved these to be extensions.

Task<ImmutableArray<SymbolDisplayPart>> ToDescriptionPartsAsync(SemanticModel semanticModel, int position, ImmutableArray<ISymbol> symbols, SymbolDescriptionOptions options, SymbolDescriptionGroups groups = SymbolDescriptionGroups.All, CancellationToken cancellationToken = default);
Task<IDictionary<SymbolDescriptionGroups, ImmutableArray<TaggedText>>> ToDescriptionGroupsAsync(SemanticModel semanticModel, int position, ImmutableArray<ISymbol> symbols, SymbolDescriptionOptions options, CancellationToken cancellationToken = default);
}

internal static class ISymbolDisplayServiceExtensions
{
extension(ISymbolDisplayService service)
{
public Task<string> ToDescriptionStringAsync(SemanticModel semanticModel, int position, ISymbol symbol, SymbolDescriptionOptions options, SymbolDescriptionGroups groups, CancellationToken cancellationToken)
=> service.ToDescriptionStringAsync(semanticModel, position, [symbol], options, groups, cancellationToken);

public async Task<string> ToDescriptionStringAsync(SemanticModel semanticModel, int position, ImmutableArray<ISymbol> symbols, SymbolDescriptionOptions options, SymbolDescriptionGroups groups, CancellationToken cancellationToken)
{
var parts = await service.ToDescriptionPartsAsync(semanticModel, position, symbols, options, groups, cancellationToken).ConfigureAwait(false);
return parts.ToDisplayString();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Linq.Expressions;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.GenerateMember.GenerateVariable;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.PooledObjects;
Expand All @@ -20,6 +23,8 @@ namespace Microsoft.CodeAnalysis.QuickInfo;

internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInfoProvider
{
private static readonly SyntaxAnnotation s_annotation = new();

protected override async Task<QuickInfoItem?> BuildQuickInfoAsync(
QuickInfoContext context, SyntaxToken token)
{
Expand Down Expand Up @@ -186,9 +191,80 @@ protected static Task<QuickInfoItem> CreateContentAsync(
protected virtual Task<OnTheFlyDocsInfo?> GetOnTheFlyDocsInfoAsync(QuickInfoContext context, CancellationToken cancellationToken)
=> Task.FromResult<OnTheFlyDocsInfo?>(null);

protected virtual (NullableAnnotation, NullableFlowState) GetNullabilityAnalysis(SemanticModel semanticModel, ISymbol symbol, SyntaxNode node, CancellationToken cancellationToken) => default;
private (NullableAnnotation, NullableFlowState) GetNullabilityAnalysis(
SolutionServices services, SemanticModel semanticModel, ISymbol symbol, SyntaxToken token, CancellationToken cancellationToken)
{
var languageServices = services.GetLanguageServices(semanticModel.Language);
var syntaxFacts = languageServices.GetRequiredService<ISyntaxFactsService>();

var bindableParent = syntaxFacts.TryGetBindableParent(token);
if (bindableParent is null)
return default;

private TokenInformation BindToken(
return TryGetNullabilityAnalysisForSuppressedExpression(out var analysis)
? analysis
: GetNullabilityAnalysis(semanticModel, symbol, bindableParent, cancellationToken);

bool TryGetNullabilityAnalysisForSuppressedExpression(out (NullableAnnotation, NullableFlowState) analysis)
Copy link
Member Author

Choose a reason for hiding this comment

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

meat of the change. when we try to get nullability info, we first see if we're in a suppressed context, and fork the code so that the suppressions are gone. so we can see waht the lang would think was going on originally.

{
analysis = default;

// Look to see if we're inside a suppression (e.g. `expr!`). The suppression changes the nullability analysis,
// and we don't actually want that here as we want to show the original nullability prior to the suppression applying.
//
// In that case, actually fork the semantic model with the `!` removed and then re-bind the token, getting the
// analysis results from that.
var tokenParent = token.GetRequiredParent();
var parentSuppression = GetOuterSuppression(tokenParent);
if (parentSuppression is null)
return false;

var root = semanticModel.SyntaxTree.GetRoot(cancellationToken);

var editor = new SyntaxEditor(root, services);
// First, mark the token, so we can find it later.
editor.ReplaceNode(
tokenParent, tokenParent.ReplaceToken(token, token.WithAdditionalAnnotations(s_annotation)));

// Now walk upwards, removing all the suppressions until we hit the top of the suppression chain.
for (var currentSuppression = parentSuppression;
currentSuppression is not null;
currentSuppression = GetOuterSuppression(currentSuppression))
{
editor.ReplaceNode(
currentSuppression,
(current, _) => syntaxFacts.GetOperandOfPostfixUnaryExpression(current));
}

// Now fork the semantic model with the new root that has the suppressions removed.
var newRoot = editor.GetChangedRoot();

var newTree = semanticModel.SyntaxTree.WithRootAndOptions(newRoot, semanticModel.SyntaxTree.Options);
var newToken = newTree.GetRoot(cancellationToken).GetAnnotatedTokens(s_annotation).Single();

var newBindableParent = syntaxFacts.TryGetBindableParent(newToken);
if (newBindableParent is null)
return false;

var newCompilation = semanticModel.Compilation.ReplaceSyntaxTree(semanticModel.SyntaxTree, newTree);
semanticModel = newCompilation.GetSemanticModel(newTree);

var symbols = BindSymbols(services, semanticModel, newToken, cancellationToken);
if (symbols.IsEmpty)
return false;

analysis = GetNullabilityAnalysis(semanticModel, symbols[0], newBindableParent, cancellationToken);
return true;

SyntaxNode? GetOuterSuppression(SyntaxNode node)
=> node.Ancestors().FirstOrDefault(a => a.RawKind == syntaxFacts.SyntaxKinds.SuppressNullableWarningExpression);
}
}

protected virtual (NullableAnnotation, NullableFlowState) GetNullabilityAnalysis(SemanticModel semanticModel, ISymbol symbol, SyntaxNode node, CancellationToken cancellationToken)
=> default;

protected ImmutableArray<ISymbol> BindSymbols(
Copy link
Member Author

Choose a reason for hiding this comment

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

Broke BindToken into BindSymbols (just tries to get teh symbols corresponding to a token). That way that helper can be used when we decide to rebind speculatively when inside a suppression

SolutionServices services, SemanticModel semanticModel, SyntaxToken token, CancellationToken cancellationToken)
{
var languageServices = services.GetLanguageServices(semanticModel.Language);
Expand All @@ -203,14 +279,39 @@ private TokenInformation BindToken(
AddSymbols(GetSymbolsFromToken(token, services, semanticModel, cancellationToken), checkAccessibility: true);
AddSymbols(bindableParent != null ? semanticModel.GetMemberGroup(bindableParent, cancellationToken) : [], checkAccessibility: false);

return filteredSymbols.ToImmutableAndClear();

void AddSymbols(ImmutableArray<ISymbol> symbols, bool checkAccessibility)
{
foreach (var symbol in symbols)
{
if (!IsOk(symbol))
continue;

if (checkAccessibility && !IsAccessible(symbol, enclosingType))
continue;

if (symbolSet.Add(symbol))
filteredSymbols.Add(symbol);
}
}
}

private TokenInformation BindToken(
SolutionServices services, SemanticModel semanticModel, SyntaxToken token, CancellationToken cancellationToken)
{
var filteredSymbols = BindSymbols(services, semanticModel, token, cancellationToken);

var languageServices = services.GetLanguageServices(semanticModel.Language);
var syntaxFacts = languageServices.GetRequiredService<ISyntaxFactsService>();

if (filteredSymbols is [var firstSymbol, ..])
{
var isAwait = syntaxFacts.IsAwaitKeyword(token);
var nullabilityInfo = bindableParent != null
? GetNullabilityAnalysis(semanticModel, firstSymbol, bindableParent, cancellationToken)
: default;
var nullabilityInfo = GetNullabilityAnalysis(
services, semanticModel, firstSymbol, token, cancellationToken);

return new TokenInformation(filteredSymbols.ToImmutableAndClear(), isAwait, nullabilityInfo);
return new TokenInformation(filteredSymbols, isAwait, nullabilityInfo);
}

// Couldn't bind the token to specific symbols. If it's an operator, see if we can at
Expand All @@ -223,21 +324,6 @@ private TokenInformation BindToken(
}

return default;

void AddSymbols(ImmutableArray<ISymbol> symbols, bool checkAccessibility)
{
foreach (var symbol in symbols)
{
if (!IsOk(symbol))
continue;

if (checkAccessibility && !IsAccessible(symbol, enclosingType))
continue;

if (symbolSet.Add(symbol))
filteredSymbols.Add(symbol);
}
}
}

private ImmutableArray<ISymbol> GetSymbolsFromToken(SyntaxToken token, SolutionServices services, SemanticModel semanticModel, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1665,6 +1665,13 @@ public void GetPartsOfParenthesizedExpression(
closeParen = parenthesizedExpression.CloseParenToken;
}

public void GetPartsOfPostfixUnaryExpression(SyntaxNode node, out SyntaxNode operand, out SyntaxToken operatorToken)
{
var postfixUnaryExpression = (PostfixUnaryExpressionSyntax)node;
operand = postfixUnaryExpression.Operand;
operatorToken = postfixUnaryExpression.OperatorToken;
}

public void GetPartsOfPrefixUnaryExpression(SyntaxNode node, out SyntaxToken operatorToken, out SyntaxNode operand)
{
var prefixUnaryExpression = (PrefixUnaryExpressionSyntax)node;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace Microsoft.CodeAnalysis.LanguageService;
/// </item>
/// <item>
/// 'GetXxxOfYYY' where 'XXX' matches the name of a property on a 'YYY' syntax construct that both C# and VB have. For
/// example 'GetExpressionOfMemberAccessExpression' corresponding to MemberAccessExpressionsyntax.Expression in both C# and
/// example 'GetExpressionOfMemberAccessExpression' corresponding to MemberAccessExpressionSyntax.Expression in both C# and
/// VB. These functions should throw if passed a node that the corresponding 'IsYYY' did not return <see langword="true"/> for.
/// For nodes that only have a single child, these functions can stay here. For nodes with multiple children, these should migrate
/// to <see cref="ISyntaxFactsExtensions"/> and be built off of 'GetPartsOfXXX'.
Expand Down Expand Up @@ -537,6 +537,7 @@ void GetPartsOfTupleExpression<TArgumentSyntax>(SyntaxNode node,
void GetPartsOfImplicitObjectCreationExpression(SyntaxNode node, out SyntaxToken keyword, out SyntaxNode argumentList, out SyntaxNode? initializer);
void GetPartsOfParameter(SyntaxNode node, out SyntaxToken identifier, out SyntaxNode? @default);
void GetPartsOfParenthesizedExpression(SyntaxNode node, out SyntaxToken openParen, out SyntaxNode expression, out SyntaxToken closeParen);
void GetPartsOfPostfixUnaryExpression(SyntaxNode node, out SyntaxNode operand, out SyntaxToken operatorToken);
void GetPartsOfPrefixUnaryExpression(SyntaxNode node, out SyntaxToken operatorToken, out SyntaxNode operand);
void GetPartsOfQualifiedName(SyntaxNode node, out SyntaxNode left, out SyntaxToken dotToken, out SyntaxNode right);
void GetPartsOfUsingAliasDirective(SyntaxNode node, out SyntaxToken globalKeyword, out SyntaxToken alias, out SyntaxNode name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,12 @@ public static SyntaxNode GetNameOfMemberAccessExpression(this ISyntaxFacts synta
return name;
}

public static SyntaxNode GetOperandOfPostfixUnaryExpression(this ISyntaxFacts syntaxFacts, SyntaxNode node)
{
syntaxFacts.GetPartsOfPostfixUnaryExpression(node, out var operand, out _);
return operand;
}

public static SyntaxNode GetOperandOfPrefixUnaryExpression(this ISyntaxFacts syntaxFacts, SyntaxNode node)
{
syntaxFacts.GetPartsOfPrefixUnaryExpression(node, out _, out var operand);
Expand Down
Loading