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 reference finding for tuple fields and anonymous type properties #69804

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Fix reference finding for tuple fields and anonymous type properties #69804

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
29de8bb
Fix FAR in simple expr implying tuple name
Rekkonnect Sep 2, 2023
1e5132f
Fix anonymous type property name references
Rekkonnect Sep 2, 2023
2b9c62a
Review comments + one more test
Rekkonnect Sep 3, 2023
08fcd62
Review comments + improve tests
Rekkonnect Sep 3, 2023
071e032
Remove extra parentheses
Rekkonnect Sep 3, 2023
e72fae9
Merge branch 'main' into fix/52621
Rekkonnect Sep 16, 2023
d951355
Adjust tests and finding references
Rekkonnect Sep 19, 2023
1d2622a
Adjust implementation using cascade
Rekkonnect Sep 19, 2023
6c3cfe7
Adjust test side effect
Rekkonnect Sep 28, 2023
51b9253
Fix property with methods replacer for anon type props
Rekkonnect Sep 28, 2023
cf79756
Merge branch 'main' into fix/52621
Rekkonnect Aug 28, 2024
38309f1
Fix outdated code
Rekkonnect Aug 28, 2024
1c3dfd1
Fix formatting
Rekkonnect Aug 28, 2024
70754f3
Fix broken test
Rekkonnect Aug 28, 2024
5adca8f
Merge branch 'main' into fix/52621
CyrusNajmabadi Oct 19, 2024
dbad8fb
Use `using`
Rekkonnect Oct 19, 2024
8ba66b3
Remove compilation from implied symbol discovery
Rekkonnect Oct 19, 2024
7b40bc8
Use channel instead of queue
Rekkonnect Oct 19, 2024
48efc1f
Add comments and simplify
Rekkonnect Oct 19, 2024
df8c23e
Use ProducerConsumer
Rekkonnect Oct 24, 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
53 changes: 53 additions & 0 deletions src/EditorFeatures/Test2/FindReferences/FindReferencesTests.AnonymousTypeSymbols.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Imports Microsoft.CodeAnalysis.Remote.Testing
Namespace Microsoft.CodeAnalysis.Editor.UnitTests.FindReferences
<Trait(Traits.Feature, Traits.Features.FindReferences)>
Partial Public Class FindReferencesTests
#Region "Visual Basic"
<WorkItem("http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/542553")>
<WpfTheory, CombinatorialData>
Public Async Function TestAnonymousType1(kind As TestKind, host As TestHost) As Task
Expand Down Expand Up @@ -165,5 +166,57 @@ End Class
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function
#End Region

#Region "C#"
<WorkItem("https://github.com/dotnet/roslyn/issues/20115")>
<WpfTheory, CombinatorialData>
Public Async Function TestAnonymousTypesCSharp_SimpleExpression(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferences="true">
<Document>
<![CDATA[
class C
{
void M(string {|Definition:a|})
{
int length = [|a|].Length;
var r = new { [|a|], Length = 1 };
r = new { $$[|a|], [|a|].Length };
Copy link
Member

Choose a reason for hiding this comment

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

add an example where there is a named-arg for 'a'. like new { a = "", ...}

Copy link
Member

Choose a reason for hiding this comment

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

add test where the $$ is on the defintion. add test where $$ is on the a in new { a = "", ...

}
}
]]>
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function

<WorkItem("https://github.com/dotnet/roslyn/issues/20115")>
<WpfTheory, CombinatorialData>
Public Async Function TestAnonymousTypesCSharp_PropertyAccess(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferences="true">
<Document>
<![CDATA[
class C
{
void M(string a)
{
int length = a.[|Length|];
var r = new { a, Length = 1 };
r = new { a, a.$$[|Length|] };
r = new { a, Length = a.[|Length|] };
Rekkonnect marked this conversation as resolved.
Show resolved Hide resolved
}
}
]]>
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function
#End Region
End Class
End Namespace
219 changes: 219 additions & 0 deletions src/EditorFeatures/Test2/FindReferences/FindReferencesTests.Tuples.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,225 @@ partial class Program
]]>
</DocumentFromSourceGenerator>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function

<WorkItem("https://github.com/dotnet/roslyn/issues/52621")>
<WpfTheory, CombinatorialData>
Public Async Function TestImplicitlyNamedTuples(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferencesNetCoreApp="true">
<Document><![CDATA[
using System;

class Program
{
static void Main()
{
int {|Definition:x|} = 4, y = 5;
var z = ($$[|x|], y);
Rekkonnect marked this conversation as resolved.
Show resolved Hide resolved
}
}
]]>
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function

<WorkItem("https://github.com/dotnet/roslyn/issues/52621")>
<WpfTheory, CombinatorialData>
Public Async Function TestImplicitTupleSwitchStatement(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferencesNetCoreApp="true">
<Document><![CDATA[
using System;

class Program
{
static void Main()
{
int {|Definition:x|} = 4, y = 5;
int z = 3;
switch ($$[|x|], y)
{
case (1, 0):
z += [|x|];
break;
case (1, 1):
z += [|x|];
break;
default:
break;
}
}
}
]]>
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function

<WorkItem("https://github.com/dotnet/roslyn/issues/52621")>
<WpfTheory, CombinatorialData>
Public Async Function TestTupleDeconstruction(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferencesNetCoreApp="true">
<Document><![CDATA[
using System;

class C
{
(int, int) M()
{
return (1, 1);
}

void M2()
{
int {|Definition:x|};
int y;

($$[|x|], y) = M();

[|x|] = 0;
}
}
]]>
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function

<WorkItem("https://github.com/dotnet/roslyn/issues/52621")>
<WpfTheory, CombinatorialData>
Public Async Function TestTupleSwappedFields(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferencesNetCoreApp="true">
<Document><![CDATA[
using System;

class C
{
void M(int {|Definition:left|}, int right)
{
var r = ($$[|left|], right);
r = (right, [|left|]);
}
}
]]>
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function

<WorkItem("https://github.com/dotnet/roslyn/issues/52621")>
<WpfTheory, CombinatorialData>
Public Async Function TestTupleNonLocal(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferencesNetCoreApp="true">
<Document><![CDATA[
using System;

class C
{
int {|Definition:Property|} { get; set; }

void M(string a)
{
var r = (a.Length, $$[|Property|]);
r = ([|Property|], Property: [|Property|]);
}
}
]]>
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function

<WorkItem("https://github.com/dotnet/roslyn/issues/52621")>
<WpfTheory, CombinatorialData>
Public Async Function TestTupleExplicitNames(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferencesNetCoreApp="true">
<Document><![CDATA[
using System;

class C
{
void M(int a, int b)
{
var t = ($$[|{|Definition:x|}|]: a, y: b);
t = ([|x|]: b, y: a);
b = t.[|x|];
}
}
]]>
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function

<WorkItem("https://github.com/dotnet/roslyn/issues/52621")>
<WpfTheory, CombinatorialData>
Public Async Function TestTupleExplicitNamesSameAsLocals01(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferencesNetCoreApp="true">
<Document><![CDATA[
using System;

class C
{
void M(int x, int y)
{
var t = ($$[|{|Definition:x|}|]: x, y: y);
t = ([|x|]: y, y: x);
Copy link
Member

Choose a reason for hiding this comment

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

test of x:x as well please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

x: x is included above, it's the definition, do you also want a test that doesn't involve the definition itself?

Copy link
Member

Choose a reason for hiding this comment

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

yes.

Copy link
Member

Choose a reason for hiding this comment

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

add test where there is also an implicit name. i.e. (x, ...)

t = ([|x|]: x, y: y);
b = t.[|x|];
}
}
]]>
</Document>
</Project>
</Workspace>
Await TestAPIAndFeature(input, kind, host)
End Function

<WorkItem("https://github.com/dotnet/roslyn/issues/52621")>
<WpfTheory, CombinatorialData>
Public Async Function TestTupleExplicitNamesSameAsLocals02(kind As TestKind, host As TestHost) As Task
Dim input =
<Workspace>
<Project Language="C#" CommonReferencesNetCoreApp="true">
<Document><![CDATA[
using System;

class C
{
void M(int {|Definition:x|}, int y)
{
var t = (x: $$[|x|], y: y);
t = (x: y, y: [|x|]);
t = (x: [|x|], y: y);
b = t.x;
}
}
]]>
</Document>
</Project>
</Workspace>
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 finding the tests hard to totally understand because they don't show both implicit and explicit named-props in anonymous-types, and implicit and explicit arguments for tuples.

Await TestAPIAndFeature(input, kind, host)
End Function
Expand Down
55 changes: 43 additions & 12 deletions ...haredUtilitiesAndExtensions/Compiler/CSharp/Services/SemanticFacts/CSharpSemanticFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,54 @@ public bool CanReplaceWithRValue(SemanticModel semanticModel, [NotNullWhen(true)
{
var location = token.GetLocation();

foreach (var ancestor in token.GetAncestors<SyntaxNode>())
var tokenParent = token.Parent;
var sourceAncestor = tokenParent;
Rekkonnect marked this conversation as resolved.
Show resolved Hide resolved
// Skip the tuple expression if the identifier is part of the simple identifier expression inside a tuple,
// but not the identifier of the field
// For example, we are evaluating `x` in `(x, y)`, or the expression after the colon in `(x: x, y: y)`
// but not the name of the field
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't explain the 'why' this is important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment ptal

if (tokenParent is IdentifierNameSyntax { Parent: ArgumentSyntax { Parent: TupleExpressionSyntax parentTuple } })
{
sourceAncestor = parentTuple.Parent;
}

if (sourceAncestor is null)
return null;

foreach (var ancestor in sourceAncestor.AncestorsAndSelf())
{
var symbol = semanticModel.GetDeclaredSymbol(ancestor, cancellationToken);
if (symbol != null)
{
if (symbol is IMethodSymbol { MethodKind: MethodKind.Conversion })
{
// The token may be part of a larger name (for example, `int` in `public static operator int[](Goo g);`.
// So check if the symbol's location encompasses the span of the token we're asking about.
if (symbol.Locations.Any(static (loc, location) => loc.SourceTree == location.SourceTree && loc.SourceSpan.Contains(location.SourceSpan), location))
return symbol;
}
else
switch (symbol)
{
// For any other symbols, we only care if the name directly matches the span of the token
if (symbol.Locations.Contains(location))
return symbol;
case IMethodSymbol { MethodKind: MethodKind.Conversion }:
{
// The token may be part of a larger name (for example, `int` in `public static operator int[](Goo g);`.
// So check if the symbol's location encompasses the span of the token we're asking about.
if (symbol.Locations.Any(static (loc, location) => loc.SourceTree == location.SourceTree && loc.SourceSpan.Contains(location.SourceSpan), location))
return symbol;
break;
}

case IPropertySymbol { ContainingType: ITypeSymbol { IsAnonymousType: true } }:
{
// If the span is part of the property's assignment expression, it's not our target result
// For example, we are evaluating `a.Length` in `new { a.Length }`, or in `new { Length = a.Length }`
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't say 'why' though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment ptal

if (ancestor is AnonymousObjectMemberDeclaratorSyntax declarator &&
declarator.Expression.Span.Contains(location.SourceSpan))
{
break;
}

return symbol;
}

default:
// For any other symbols, we only care if the name directly matches the span of the token
if (symbol.Locations.Contains(location))
return symbol;
break;
}

// We found some symbol, but it defined something else. We're not going to have a higher node defining _another_ symbol with this token, so we can stop now.
Expand Down