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

Include CodeLens on remaining types and members #69608

Merged
merged 1 commit into from
Aug 18, 2023
Merged

Include CodeLens on remaining types and members #69608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 37 additions & 0 deletions src/Features/CSharp/Portable/CodeLens/CSharpCodeLensMemberFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public override void VisitInterfaceDeclaration(InterfaceDeclarationSyntax node)
public override void VisitEnumDeclaration(EnumDeclarationSyntax node)
{
_memberBuilder.Add(new CodeLensMember(node, node.Identifier.Span));
base.VisitEnumDeclaration(node);
}

public override void VisitPropertyDeclaration(PropertyDeclarationSyntax node)
Expand Down Expand Up @@ -83,5 +84,41 @@ public override void VisitRecordDeclaration(RecordDeclarationSyntax node)
_memberBuilder.Add(new CodeLensMember(node, node.Identifier.Span));
base.VisitRecordDeclaration(node);
}

public override void VisitDelegateDeclaration(DelegateDeclarationSyntax node)
{
_memberBuilder.Add(new CodeLensMember(node, node.Identifier.Span));
}
Copy link
Member

Choose a reason for hiding this comment

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

definitely approve of this.


public override void VisitEnumMemberDeclaration(EnumMemberDeclarationSyntax node)
{
_memberBuilder.Add(new CodeLensMember(node, node.Identifier.Span));
}

public override void VisitFieldDeclaration(FieldDeclarationSyntax node)
{
foreach (var variable in node.Declaration.Variables)
{
_memberBuilder.Add(new CodeLensMember(variable, variable.Identifier.Span));
}
}
Copy link
Member

Choose a reason for hiding this comment

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

  1. what's teh consequence of this in VS for multi-declarator members?
  2. does thsi light up code-lens for fields for all users in VS. if so, i think this should be gated behind a user option peopel can opt into.

Copy link
Member Author

@sharwell sharwell Aug 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

what's teh consequence of this in VS [Code] for multi-declarator members?

I'm not sure how to test the UI in VS Code, but I would expect it to be able to handle this since today it's already possible to declare multiple properties on the same line of code.

does thsi light up code-lens for fields for all users in VS

This has no impact on Visual Studio. This is a VS Code feature.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how to test the UI in VS Code

@dibarbet can you guide sam on this?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how to test the UI in VS Code, but I would expect it to be able to handle this since today it's already possible to declare multiple properties on the same line of code.

If you mean seeing the actual code lens items inside vscode, it should be relatively straightforward. Basically:

  1. Make sure the C# extension is installed and you can open a C# project
  2. Point the C# extension to your locally built server as described here - https://github.com/dotnet/vscode-csharp/blob/main/CONTRIBUTING.md#using-a-locally-developed-roslyn-server
  3. Reload the window

Copy link
Member Author

Choose a reason for hiding this comment

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

It's going to end up working like this:
image


public override void VisitEventFieldDeclaration(EventFieldDeclarationSyntax node)
{
foreach (var variable in node.Declaration.Variables)
{
_memberBuilder.Add(new CodeLensMember(variable, variable.Identifier.Span));
}
}

public override void VisitEventDeclaration(EventDeclarationSyntax node)
{
_memberBuilder.Add(new CodeLensMember(node, node.Identifier.Span));
}

public override void VisitDestructorDeclaration(DestructorDeclarationSyntax node)
Copy link
Member

Choose a reason for hiding this comment

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

note: won't ever have results. so this may be likely to annoy people as it will always have "0 references" showing for it. Would prefer to not do this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's fine, for a few reasons:

  1. Behavior matches current Visual Studio behavior
  2. Destructors are rare and generally discouraged, so most users won't ever see the difference
  3. For the remaining users, it's less likely someone will ask why it shows 0 than for someone to ask why it's missing

{
_memberBuilder.Add(new CodeLensMember(node, node.Identifier.Span));
}
}
}
169 changes: 168 additions & 1 deletion src/Features/LanguageServer/ProtocolUnitTests/CodeLens/CSharpCodeLensTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using Microsoft.CodeAnalysis.LanguageServer.Handler.CodeLens;
using Newtonsoft.Json;
using Roslyn.Test.Utilities;
using StreamJsonRpc;
using Xunit;
using Xunit.Abstractions;
using LSP = Microsoft.VisualStudio.LanguageServer.Protocol;
Expand Down Expand Up @@ -134,6 +133,19 @@ public async Task TestEnumDeclarationAsync(bool lspMutatingWorkspace)
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestEnumMemberDeclarationAsync(bool lspMutatingWorkspace)
{
var markup =
@"enum A
{
{|codeLens:One|}
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
public async Task TestPropertyDeclarationAsync(bool lspMutatingWorkspace)
{
Expand All @@ -146,6 +158,136 @@ public async Task TestPropertyDeclarationAsync(bool lspMutatingWorkspace)
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestEventDeclarationAsync(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
public event System.EventHandler {|codeLens:I|} { add { } remove { } }
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestEventFieldDeclaration1Async(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
public event System.EventHandler {|codeLens:I|};
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestEventFieldDeclaration2Async(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
public event System.EventHandler {|codeLens:I|}, I2;
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestEventFieldDeclaration3Async(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
public event System.EventHandler I, {|codeLens:I2|};
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestFieldDeclaration1Async(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
public int {|codeLens:I|};
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestFieldDeclaration2Async(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
public int {|codeLens:I|}, I2;
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestFieldDeclaration3Async(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
public int I, {|codeLens:I2|};
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestConstantDeclaration1Async(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
public const int {|codeLens:I|} = 0;
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestConstantDeclaration2Async(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
public const int {|codeLens:I|} = 0, I2 = 0;
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestConstantDeclaration3Async(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
public const int I = 0, {|codeLens:I2|} = 0;
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
public async Task TestMethodDeclarationAsync(bool lspMutatingWorkspace)
{
Expand All @@ -171,6 +313,16 @@ public async Task TestStructDeclarationAsync(bool lspMutatingWorkspace)
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestDelegateDeclarationAsync(bool lspMutatingWorkspace)
{
var markup =
@"delegate void {|codeLens:A|}();";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
public async Task TestConstructorDeclarationAsync(bool lspMutatingWorkspace)
{
Expand All @@ -185,6 +337,21 @@ public async Task TestConstructorDeclarationAsync(bool lspMutatingWorkspace)
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
[WorkItem("https://github.com/dotnet/roslyn/issues/69583")]
public async Task TestDestructorDeclarationAsync(bool lspMutatingWorkspace)
{
var markup =
@"class A
{
~{|codeLens:A|}()
{
}
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, lspMutatingWorkspace, CapabilitiesWithVSExtensions);
await VerifyCodeLensAsync(testLspServer, expectedNumberOfReferences: 0);
}

[Theory, CombinatorialData]
public async Task TestRecordDeclarationAsync(bool lspMutatingWorkspace)
{
Expand Down
Loading