From 3627f619773aa5b1684ce6d46e3e43e372dcb051 Mon Sep 17 00:00:00 2001
From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com>
Date: Mon, 2 Feb 2026 22:28:56 +0000
Subject: [PATCH] Address PR review: use iter_symbols() and make hover tests
 assertive

- Replace get_all_symbols_and_roots() with iter_symbols() to avoid
  redundant tuple unpacking
- Make hover tests assert that hover_info is not None instead of
  defensive conditionals
---
 .../csharp_language_server.py                 |  7 +--
 test/solidlsp/csharp/test_csharp_basic.py     | 43 ++++++++++---------
 2 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/src/solidlsp/language_servers/csharp_language_server.py b/src/solidlsp/language_servers/csharp_language_server.py
index e99bdcbf..58ea6e0c 100644
--- a/src/solidlsp/language_servers/csharp_language_server.py
+++ b/src/solidlsp/language_servers/csharp_language_server.py
@@ -200,12 +200,7 @@ class CSharpLanguageServer(SolidLanguageServer):
         symbols = super().request_document_symbols(relative_file_path, file_buffer)
 
         # Normalize all symbols recursively
-        # DocumentSymbols.get_all_symbols_and_roots() returns tuple of (all_symbols, root_symbols)
-        all_symbols_tuple = symbols.get_all_symbols_and_roots()
-        all_symbols, _ = all_symbols_tuple
-
-        # Process each symbol (which is a UnifiedSymbolInformation dict)
-        for symbol in all_symbols:
+        for symbol in symbols.iter_symbols():
             self._normalize_symbol_name(symbol, relative_file_path)
 
         return symbols
diff --git a/test/solidlsp/csharp/test_csharp_basic.py b/test/solidlsp/csharp/test_csharp_basic.py
index 95aae116..ef478dbf 100644
--- a/test/solidlsp/csharp/test_csharp_basic.py
+++ b/test/solidlsp/csharp/test_csharp_basic.py
@@ -142,35 +142,36 @@ class TestCSharpLanguageServer:
         # public string Name { get; set; }
         hover_info = language_server.request_hover(file_path, 6, 23)
 
-        if hover_info is not None:
-            # Verify hover has content
-            assert isinstance(hover_info, dict), "Hover should be a dict"
-            assert "contents" in hover_info, "Hover should have contents"
+        # Verify hover has content
+        assert hover_info is not None, "Hover should return information for Name property"
+        assert isinstance(hover_info, dict), "Hover should be a dict"
+        assert "contents" in hover_info, "Hover should have contents"
 
-            contents = hover_info["contents"]
-            if isinstance(contents, dict) and "value" in contents:
-                hover_text = contents["value"]
-                assert hover_text is not None and len(hover_text) > 0, "Hover should have content"
+        contents = hover_info["contents"]
+        if isinstance(contents, dict) and "value" in contents:
+            hover_text = contents["value"]
+            assert hover_text is not None and len(hover_text) > 0, "Hover should have content"
 
-                # Check if the injected type annotation appears in hover
-                # Original name should be "Name : string" which we inject at the top
-                # Note: This might not always appear if Roslyn's hover already includes it
-                # So we just verify there's meaningful content
-                assert len(hover_text) > 10, "Hover should have substantial content"
+            # Check if the injected type annotation appears in hover
+            # Original name should be "Name : string" which we inject at the top
+            # Note: This might not always appear if Roslyn's hover already includes it
+            # So we just verify there's meaningful content
+            assert len(hover_text) > 10, "Hover should have substantial content"
 
         # Test 2: Hover over the IsAdult method name (line 22, column 21 - on "IsAdult")
         # public bool IsAdult()
         hover_method = language_server.request_hover(file_path, 22, 21)
 
-        if hover_method is not None:
-            assert isinstance(hover_method, dict), "Hover should be a dict"
-            assert "contents" in hover_method, "Hover should have contents"
+        # Verify method hover has content
+        assert hover_method is not None, "Hover should return information for IsAdult method"
+        assert isinstance(hover_method, dict), "Hover should be a dict"
+        assert "contents" in hover_method, "Hover should have contents"
 
-            contents = hover_method["contents"]
-            if isinstance(contents, dict) and "value" in contents:
-                method_hover_text = contents["value"]
-                # Original name should be "IsAdult() : bool" injected at the top
-                assert method_hover_text is not None and len(method_hover_text) > 10, "Method hover should have substantial content"
+        contents = hover_method["contents"]
+        if isinstance(contents, dict) and "value" in contents:
+            method_hover_text = contents["value"]
+            # Original name should be "IsAdult() : bool" injected at the top
+            assert method_hover_text is not None and len(method_hover_text) > 10, "Method hover should have substantial content"
 
 
 @pytest.mark.csharp