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

Jump to bottom

Fix parameters not showing up in 'nameof' on a method #76017

Merged
merged 2 commits into from
Nov 25, 2024
Merged

Fix parameters not showing up in 'nameof' on a method #76017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
61d20bf
Fix parameters not showing up in 'nameof' on a method
CyrusNajmabadi Nov 22, 2024
e504204
lint
CyrusNajmabadi Nov 22, 2024
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
22 changes: 11 additions & 11 deletions ...EditorFeatures/CSharpTest/Completion/CompletionProviders/SymbolCompletionProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Completion.Providers;
using Microsoft.CodeAnalysis.CSharp;
Expand Down Expand Up @@ -12558,18 +12559,17 @@ void M(int parameter) { }
await VerifyItemExistsAsync(MakeMarkup(source, languageVersion: "10"), "parameter");
}

[Fact]
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/60812")]
public async Task ParameterNotAvailableInMethodAttributeNameofWithNoArgument()
{
var source = @"
class C
{
[Some(nameof($$))]
void M(int parameter) { }
}
";
// Tracked by https://github.com/dotnet/roslyn/issues/60812
await VerifyItemIsAbsentAsync(MakeMarkup(source), "parameter");
var source = """
class C
{
[Some(nameof($$))]
void M(int parameter) { }
}
""";
await VerifyItemExistsAsync(MakeMarkup(source), "parameter");
}

[Fact]
Expand Down Expand Up @@ -14075,7 +14075,7 @@ IEnumerable<string> M() => [string.Empty, .. ($$

#endregion

private static string MakeMarkup(string source, string languageVersion = "Preview")
private static string MakeMarkup([StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string source, string languageVersion = "Preview")
{
return $$"""
<Workspace>
Expand Down
10 changes: 3 additions & 7 deletions src/Features/CSharp/Portable/Completion/CompletionProviders/SymbolCompletionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ namespace Microsoft.CodeAnalysis.CSharp.Completion.Providers;
[ExportCompletionProvider(nameof(SymbolCompletionProvider), LanguageNames.CSharp)]
[ExtensionOrder(After = nameof(SpeculativeTCompletionProvider))]
[Shared]
internal sealed class SymbolCompletionProvider : AbstractRecommendationServiceBasedCompletionProvider<CSharpSyntaxContext>
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed class SymbolCompletionProvider() : AbstractRecommendationServiceBasedCompletionProvider<CSharpSyntaxContext>
{
private static readonly Dictionary<(bool importDirective, bool preselect, bool tupleLiteral), CompletionItemRules> s_cachedRules = [];

Expand Down Expand Up @@ -65,12 +67,6 @@ static CompletionItemRules MakeRule((bool importDirective, bool preselect, bool
}
}

[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public SymbolCompletionProvider()
{
}

public override ImmutableHashSet<char> TriggerCharacters { get; } = CompletionUtilities.CommonTriggerCharactersWithArgumentList;

internal override string Language => LanguageNames.CSharp;
Expand Down
10 changes: 3 additions & 7 deletions src/Workspaces/CSharp/Portable/Recommendations/CSharpRecommendationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,10 @@
namespace Microsoft.CodeAnalysis.CSharp.Recommendations;

[ExportLanguageService(typeof(IRecommendationService), LanguageNames.CSharp), Shared]
internal partial class CSharpRecommendationService : AbstractRecommendationService<CSharpSyntaxContext, AnonymousFunctionExpressionSyntax>
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal partial class CSharpRecommendationService() : AbstractRecommendationService<CSharpSyntaxContext, AnonymousFunctionExpressionSyntax>
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public CSharpRecommendationService()
{
}

protected override AbstractRecommendationServiceRunner CreateRunner(CSharpSyntaxContext context, bool filterOutOfScopeLocals, CancellationToken cancellationToken)
=> new CSharpRecommendationServiceRunner(context, filterOutOfScopeLocals, cancellationToken);
}
41 changes: 28 additions & 13 deletions src/Workspaces/CSharp/Portable/Recommendations/CSharpRecommendationServiceRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,10 @@ namespace Microsoft.CodeAnalysis.CSharp.Recommendations;

internal partial class CSharpRecommendationService
{
private sealed partial class CSharpRecommendationServiceRunner : AbstractRecommendationServiceRunner
private sealed partial class CSharpRecommendationServiceRunner(
CSharpSyntaxContext context, bool filterOutOfScopeLocals, CancellationToken cancellationToken)
: AbstractRecommendationServiceRunner(context, filterOutOfScopeLocals, cancellationToken)
{
public CSharpRecommendationServiceRunner(
CSharpSyntaxContext context, bool filterOutOfScopeLocals, CancellationToken cancellationToken)
: base(context, filterOutOfScopeLocals, cancellationToken)
{
}

protected override int GetLambdaParameterCount(AnonymousFunctionExpressionSyntax lambdaSyntax)
=> lambdaSyntax switch
{
Expand Down Expand Up @@ -330,6 +326,9 @@ private ImmutableArray<ISymbol> GetSymbolsForTypeOrNamespaceContext()

private ImmutableArray<ISymbol> GetSymbolsForExpressionOrStatementContext()
{
var contextNode = _context.LeftToken.GetRequiredParent();
var semanticModel = _context.SemanticModel;

// Check if we're in an interesting situation like this:
//
// i // <-- here
Expand All @@ -346,22 +345,38 @@ private ImmutableArray<ISymbol> GetSymbolsForExpressionOrStatementContext()
var filterOutOfScopeLocals = _filterOutOfScopeLocals;
if (filterOutOfScopeLocals)
{
var contextNode = _context.LeftToken.GetRequiredParent();
filterOutOfScopeLocals =
!contextNode.IsFoundUnder<LocalDeclarationStatementSyntax>(d => d.Declaration.Type) &&
!contextNode.IsFoundUnder<DeclarationExpressionSyntax>(d => d.Type);
}

var symbols = !_context.IsNameOfContext && _context.LeftToken.GetRequiredParent().IsInStaticContext()
? _context.SemanticModel.LookupStaticMembers(_context.LeftToken.SpanStart)
: _context.SemanticModel.LookupSymbols(_context.LeftToken.SpanStart);
ImmutableArray<ISymbol> symbols;
if (_context.IsNameOfContext)
{
symbols = semanticModel.LookupSymbols(_context.LeftToken.SpanStart);

// We may be inside of a nameof() on a method. In that case, we want to include the parameters in
// the nameof if LookupSymbols didn't already return them.
var enclosingMethodOrLambdaNode = contextNode.AncestorsAndSelf().FirstOrDefault(n => n is AnonymousFunctionExpressionSyntax or BaseMethodDeclarationSyntax);
var enclosingMethodOrLambda = enclosingMethodOrLambdaNode is null
? null
: semanticModel.GetSymbolInfo(enclosingMethodOrLambdaNode).GetAnySymbol() ?? semanticModel.GetDeclaredSymbol(enclosingMethodOrLambdaNode);
if (enclosingMethodOrLambda is IMethodSymbol method)
symbols = [.. symbols.Concat(method.Parameters)];
Copy link
Contributor

Choose a reason for hiding this comment

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

[..

is this the same as

symbols = symbols.Concat(method.Parameters);

Copy link
Member Author

Choose a reason for hiding this comment

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

likely yes. will fixup.

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. .Concat returns an IEnumerqable here.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's confusing. symbols is an IA, method.Parameters is an IA, isn't this invoking the IA overload for Concat?

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, it's not. i get an error when i try :)

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, I see. One is IA, the other is IA. It needs to be:

symbols.Concat(method.Parameters.Cast());

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, just a sec, I thought that cast call just cast the array, not enumerate

Copy link
Contributor

Choose a reason for hiding this comment

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

Duh, it's CastArray, not Cast

}
else
{
symbols = contextNode.IsInStaticContext()
? semanticModel.LookupStaticMembers(_context.LeftToken.SpanStart)
: semanticModel.LookupSymbols(_context.LeftToken.SpanStart);
}

// Filter out any extension methods that might be imported by a using static directive.
// But include extension methods declared in the context's type or it's parents
var contextOuterTypes = ComputeOuterTypes(_context, _cancellationToken);
var contextEnclosingNamedType = _context.SemanticModel.GetEnclosingNamedType(_context.Position, _cancellationToken);
var contextEnclosingNamedType = semanticModel.GetEnclosingNamedType(_context.Position, _cancellationToken);

return symbols.WhereAsArray(
return symbols.Distinct().WhereAsArray(
static (symbol, args) => !IsUndesirable(args._context, args.contextEnclosingNamedType, args.contextOuterTypes, args.filterOutOfScopeLocals, symbol, args._cancellationToken),
(_context, contextOuterTypes, contextEnclosingNamedType, filterOutOfScopeLocals, _cancellationToken));

Expand Down
Loading