Fix DeclaringSyntaxReferences returning empty for VB catch variables

[Copilot]

Plan: Fix DeclaringSyntaxReferences for Catch Variables ✅

• Explore repository and understand the issue
• Locate the bug in VariableLocalSymbol.DeclaringSyntaxReferences
• Write a test to demonstrate the issue
• Fix the bug in LocalSymbol.vb
• Verify the fix passes the test
• Run broader tests to ensure no regressions
• Address code review feedback: Add Debug.Assert for catch variable
• Address code review feedback: Remove redundant test comments
• Address code review feedback: Simplify test assertions

Summary

Fixed an issue where DeclaringSyntaxReferences returned an empty list for catch statement variables in Visual Basic.

Solution

Modified the VariableLocalSymbol.DeclaringSyntaxReferences property in LocalSymbol.vb to fall back to using _identifierToken.Parent.GetReference() when _modifiedIdentifierOpt is Nothing.

Code Review Response

1. Investigated all usages of VariableLocalSymbol creation and confirmed that only LocalDeclarationKind.Catch passes Nothing as modifiedIdentifierOpt. Added Debug.Assert(DeclarationKind = LocalDeclarationKind.Catch) to enforce this invariant.
2. Removed redundant comments from test that restated what assertions were doing.
3. Simplified test assertions by using .Single() instead of checking length and indexing, and removed redundant NotNull check since IsType already implies not null.

Analysis of all LocalDeclarationKind cases:

• Catch: Uses Nothing for modifiedIdentifierOpt ✓
• Variable, Constant, Static: Always provide ModifiedIdentifierSyntax
• Using: Always provides ModifiedIdentifierSyntax
• For, ForEach: Always provide ModifiedIdentifierSyntax
• ImplicitVariable, FunctionValue: Use different LocalSymbol types (SourceLocalSymbol), not VariableLocalSymbol

Testing Results ✅

• ✅ All 15 DeclaringSyntaxNodeTests pass
• ✅ All 65 catch-related tests pass
• ✅ Debug assertion does not fire in any tests

Original prompt

---

This section details on the original issue you should resolve

<issue_title>DeclaringSyntaxReferences returns empty list on the underlying ILocalSymbol of a catch statement's variable</issue_title>
<issue_description>In VB.Net with the following input:

Try
  '...
Catch exc As Exception
  Console.WriteLine(exc.ToString())
End Try

when I query the DeclaringSyntaxReferences of the local symbol of exc, an empty list is returned.

The problem seems to be that VariableLocalSymbol expects to have a ModifiedIdentifierSyntax, but in case of the catch statement it has an IdentifierNameSyntax, so it returns empty list: http://source.roslyn.codeplex.com/#Microsoft.CodeAnalysis.VisualBasic/Symbols/Source/LocalSymbol.vb,44cfe5cfa177deb7,references

If I do the same in C#, I get the reference without a problem.
</issue_description>

<agent_instructions>@copilot Write a test to see if there's still a problem here. If not, the test will suffice to close out the issue. Otherwise, see if there is a simple issue in LocalSymbol.vb and attempt a targeted fix that then makes the test succeed.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

• Fixes DeclaringSyntaxReferences returns empty list on the underlying ILocalSymbol of a catch statement's variable #6209

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[CyrusNajmabadi]

@copilot determine what other cases there may be that hit this. if there are none, assert that this is a catch variable. If tehre are others, enumerate them here and add tests.

[Copilot]

Investigated all usages of VariableLocalSymbol and confirmed that only LocalDeclarationKind.Catch passes Nothing for modifiedIdentifierOpt. Added Debug.Assert(DeclarationKind = LocalDeclarationKind.Catch) in commit 8d731e6.

Analysis:

• Catch: Passes Nothing in CatchBlockBinder.vb:55
• Variable, Constant, Static, Using: Always provide ModifiedIdentifierSyntax
• For, ForEach: Always provide ModifiedIdentifierSyntax
• ImplicitVariable, FunctionValue: Use SourceLocalSymbol (different type), not VariableLocalSymbol

All tests pass with the assertion.

[CyrusNajmabadi]

@copilot no need to add test comments that just restate what the test is already doing.

[Copilot]

Removed redundant comments in commit 8566ad4.

[CyrusNajmabadi]

@copilot remove unneeded asserts. Instead, use declaringRefs.Single(). That way it will crash if we don't have exactly one item. Similarly, if we have an IsType check, we don't need a NotNull check.

[Copilot]

Simplified assertions in commit 6e0ec0c:

• Used .Single() instead of checking length and indexing
• Removed redundant Assert.NotNull(declaredSyntax) since Assert.IsType already implies not null

[CyrusNajmabadi]

@dotnet/roslyn-compiler this is ready for review.

[AlekseyTs]

If _modifiedIdentifierOpt IsNot Nothing Then

It looks like the first case is unreachable and the entire method implementation could be:

If _modifiedIdentifierOpt IsNot Nothing Then
    Return ImmutableArray.Create(Of SyntaxReference)(_modifiedIdentifierOpt.GetReference())
Else
    Return MyBase.DeclaringSyntaxReferences
End If

[CyrusNajmabadi]

f8e1b03

[AlekseyTs]

Dim identifierName = token.Parent

This calculation looks unnecessary complicated. We simply could traverse the syntax tree for the first IdentifierNameSyntax with the right identifier value.

[CyrusNajmabadi]

c3ecbf0

[AlekseyTs]

Done with review pass (commit 7)