Merge pull request isaacphi#7 from virtuald/use-smallest-symbol

clangd: use smallest matching symbol as definition result

Author: virtuald

integrationtests/snapshots/clangd/definition/method.snap

@@ -4,15 +4,7 @@ Symbol: method
 /TEST_OUTPUT/workspace/clangd/src/consumer.cpp
 Kind: Method
 Container Name: TestClass
-Range: L7:C1 - L15:C2
+Range: L14:C1 - L14:C47
 
- 7|class TestClass {
- 8| public:
- 9|  /**
-10|   * @brief A method that takes an integer parameter.
-11|   *
-12|   * @param param The integer parameter to be processed.
-13|   */
 14|  void method(int param) { helperFunction(); }
-15|};
 

integrationtests/snapshots/clangd/definition/nsFunction.snap

@@ -0,0 +1,12 @@
+---
+
+Symbol: nsFunction2
+/TEST_OUTPUT/workspace/clangd/src/namespace.cpp
+Kind: Function
+Container Name: ns
+Range: L11:C1 - L13:C2
+
+11|void nsFunction2() {
+12|    // empty   
+13|}
+

integrationtests/snapshots/python/definition/method.snap

@@ -4,21 +4,8 @@ Symbol: test_method
 /TEST_OUTPUT/workspace/main.py
 Kind: Method
 Container Name: TestClass
-Range: L18:C1 - L59:C22
+Range: L31:C1 - L41:C26
 
-18|class TestClass:
-19|    """A test class with methods and attributes."""
-20|
-21|    class_variable: str = "class variable"
-22|
-23|    def __init__(self, value: int = 0):
-24|        """Initialize the TestClass.
-25|
-26|        Args:
-27|            value: The initial value
-28|        """
-29|        self.value: int = value
-30|
 31|    def test_method(self, increment: int) -> int:
 32|        """Increment the value by the given amount.
 33|
@@ -30,22 +17,4 @@ Range: L18:C1 - L59:C22
 39|        """
 40|        self.value += increment
 41|        return self.value
-42|
-43|    @staticmethod
-44|    def static_method(items: list[str]) -> dict[str, int]:
-45|        """Convert a list of items to a dictionary with item counts.
-46|
-47|        Args:
-48|            items: A list of strings
-49|
-50|        Returns:
-51|            A dictionary mapping items to their counts
-52|        """
-53|        result: dict[str, int] = {}
-54|        for item in items:
-55|            if item in result:
-56|                result[item] += 1
-57|            else:
-58|                result[item] = 1
-59|        return result
 

integrationtests/snapshots/python/definition/static-method.snap

@@ -4,33 +4,8 @@ Symbol: static_method
 /TEST_OUTPUT/workspace/main.py
 Kind: Method
 Container Name: TestClass
-Range: L18:C1 - L59:C22
+Range: L43:C1 - L59:C22
 
-18|class TestClass:
-19|    """A test class with methods and attributes."""
-20|
-21|    class_variable: str = "class variable"
-22|
-23|    def __init__(self, value: int = 0):
-24|        """Initialize the TestClass.
-25|
-26|        Args:
-27|            value: The initial value
-28|        """
-29|        self.value: int = value
-30|
-31|    def test_method(self, increment: int) -> int:
-32|        """Increment the value by the given amount.
-33|
-34|        Args:
-35|            increment: The amount to increment by
-36|
-37|        Returns:
-38|            The new value
-39|        """
-40|        self.value += increment
-41|        return self.value
-42|
 43|    @staticmethod
 44|    def static_method(items: list[str]) -> dict[str, int]:
 45|        """Convert a list of items to a dictionary with item counts.

integrationtests/snapshots/rust/definition/method.snap

@@ -4,39 +4,21 @@ Symbol: method
 /TEST_OUTPUT/workspace/src/types.rs
 Kind: Function
 Container Name: TestStruct
-Range: L18:C1 - L30:C2
+Range: L27:C1 - L29:C6
 
-18|// Implementation for TestStruct
-19|impl TestStruct {
-20|    pub fn new(name: &str, value: i32) -> Self {
-21|        TestStruct {
-22|            name: String::from(name),
-23|            value,
-24|        }
-25|    }
-26|
 27|    pub fn method(&self) -> String {
 28|        format!("{}: {}", self.name, self.value)
 29|    }
-30|}
 
 ---
 
 Symbol: method
 /TEST_OUTPUT/workspace/src/types.rs
 Kind: Function
 Container Name: SharedStruct
-Range: L54:C1 - L64:C2
+Range: L61:C1 - L63:C6
 
-54|impl SharedStruct {
-55|    pub fn new(name: &str) -> Self {
-56|        SharedStruct {
-57|            name: String::from(name),
-58|        }
-59|    }
-60|
 61|    pub fn method(&self) -> String {
 62|        format!("SharedStruct: {}", self.name)
 63|    }
-64|}
 

integrationtests/tests/clangd/definition/definition_test.go

@@ -65,6 +65,12 @@ func TestReadDefinition(t *testing.T) {
 			expectedText: "void method(int param)",
 			snapshotName: "method",
 		},
+		{
+			name:         "Namespace function",
+			symbolName:   "nsFunction2",
+			expectedText: "void nsFunction2()",
+			snapshotName: "nsFunction",
+		},
 		{
 			name:         "Struct",
 			symbolName:   "TestStruct",

integrationtests/workspaces/clangd/Makefile

@@ -29,7 +29,7 @@ TARGET_CLEAN_PROGRAM = clean_program # Assuming this is another program to be bu
 # Listing them explicitly for clarity in target dependencies.
 OBJ_MAIN = $(OBJDIR)/main.o
 # Add other specific object files your 'program' executable depends on
-OTHER_OBJS = $(OBJDIR)/helper.o $(OBJDIR)/types.o $(OBJDIR)/consumer.o $(OBJDIR)/another_consumer.o
+OTHER_OBJS = $(OBJDIR)/helper.o $(OBJDIR)/types.o $(OBJDIR)/consumer.o $(OBJDIR)/another_consumer.o $(OBJDIR)/namespace.o
 OBJ_FOR_CLEAN_PROGRAM = $(OBJDIR)/clean.o
 
 

integrationtests/workspaces/clangd/src/namespace.cpp

@@ -0,0 +1,15 @@
+//
+// A file with a namespace in it
+//
+
+namespace ns {
+
+void nsFunction1() {
+    // empty   
+}
+
+void nsFunction2() {
+    // empty   
+}
+
+}

internal/tools/lsp-utilities.go

@@ -5,14 +5,19 @@ import (
 	"fmt"
 	"net/url"
 	"os"
+	"slices"
 	"strings"
 
 	"github.com/isaacphi/mcp-language-server/internal/lsp"
 	"github.com/isaacphi/mcp-language-server/internal/protocol"
 )
 
-// Gets the full code block surrounding the start of the input location
-func GetFullDefinition(ctx context.Context, client *lsp.Client, startLocation protocol.Location) (string, protocol.Location, protocol.DocumentSymbolResult, error) {
+type match struct {
+	Symbol protocol.DocumentSymbolResult
+	Range  protocol.Range
+}
+
+func identifyOverlappingSymbols(ctx context.Context, client *lsp.Client, startLocation protocol.Location) ([]match, error) {
 	symParams := protocol.DocumentSymbolParams{
 		TextDocument: protocol.TextDocumentIdentifier{
 			URI: startLocation.URI,
@@ -22,45 +27,59 @@ func GetFullDefinition(ctx context.Context, client *lsp.Client, startLocation pr
 	// Get all symbols in document
 	symResult, err := client.DocumentSymbol(ctx, symParams)
 	if err != nil {
-		return "", protocol.Location{}, nil, fmt.Errorf("failed to get document symbols: %w", err)
+		return nil, fmt.Errorf("failed to get document symbols: %w", err)
 	}
 
 	symbols, err := symResult.Results()
 	if err != nil {
-		return "", protocol.Location{}, nil, fmt.Errorf("failed to process document symbols: %w", err)
+		return nil, fmt.Errorf("failed to process document symbols: %w", err)
 	}
 
-	var symbolRange protocol.Range
-	var symbol protocol.DocumentSymbolResult
-	found := false
-
 	// Search for symbol at startLocation
-	var searchSymbols func(symbols []protocol.DocumentSymbolResult) bool
-	searchSymbols = func(symbols []protocol.DocumentSymbolResult) bool {
+	// - multiple symbols might match (for example, a C++ namespace) so find
+	//   all of the matching symbols and use the smallest one (or the first one
+	//   if there is a tie)
+	var matchingSymbols []match
+
+	var searchSymbols func(symbols []protocol.DocumentSymbolResult)
+	searchSymbols = func(symbols []protocol.DocumentSymbolResult) {
 		for _, sym := range symbols {
 			if containsPosition(sym.GetRange(), startLocation.Range.Start) {
-				symbol = sym
-				symbolRange = sym.GetRange()
-				found = true
-				return true
+				matchingSymbols = append(matchingSymbols, match{sym, sym.GetRange()})
 			}
+
 			// Handle nested symbols if it's a DocumentSymbol
 			if ds, ok := sym.(*protocol.DocumentSymbol); ok && len(ds.Children) > 0 {
 				childSymbols := make([]protocol.DocumentSymbolResult, len(ds.Children))
 				for i := range ds.Children {
 					childSymbols[i] = &ds.Children[i]
 				}
-				if searchSymbols(childSymbols) {
-					return true
-				}
+				searchSymbols(childSymbols)
 			}
 		}
-		return false
 	}
 
-	found = searchSymbols(symbols)
+	searchSymbols(symbols)
+	return matchingSymbols, nil
+}
+
+// Gets the full code block surrounding the start of the input location
+func GetFullDefinition(ctx context.Context, client *lsp.Client, startLocation protocol.Location) (string, protocol.Location, protocol.DocumentSymbolResult, error) {
+
+	matchingSymbols, err := identifyOverlappingSymbols(ctx, client, startLocation)
+	if err != nil {
+		return "", protocol.Location{}, nil, err
+	}
+
+	// Identify the smallest overlapping symbol
+	slices.SortStableFunc(matchingSymbols, func(a, b match) int {
+		return int(a.Range.End.Line-a.Range.Start.Line) - int(b.Range.End.Line-b.Range.Start.Line)
+	})
+
+	if len(matchingSymbols) > 0 {
+		symbol := matchingSymbols[0].Symbol
+		symbolRange := matchingSymbols[0].Range
 
-	if found {
 		// Convert URI to filesystem path
 		filePath, err := url.PathUnescape(strings.TrimPrefix(string(startLocation.URI), "file://"))
 		if err != nil {
@@ -125,16 +144,13 @@ func GetFullDefinition(ctx context.Context, client *lsp.Client, startLocation pr
 			}
 		}
 
-		// Update location with new range
-		startLocation.Range = symbolRange
-
 		// Return the text within the range
 		if int(symbolRange.End.Line) >= len(lines) {
 			return "", protocol.Location{}, nil, fmt.Errorf("end line out of range")
 		}
 
 		selectedLines := lines[symbolRange.Start.Line : symbolRange.End.Line+1]
-		return strings.Join(selectedLines, "\n"), startLocation, symbol, nil
+		return strings.Join(selectedLines, "\n"), protocol.Location{URI: startLocation.URI, Range: symbolRange}, symbol, nil
 	}
 
 	return "", protocol.Location{}, nil, fmt.Errorf("symbol not found")
@@ -148,8 +164,8 @@ func GetLineRangesToDisplay(ctx context.Context, client *lsp.Client, locations [
 	// For each location, get its container and add relevant lines
 	for _, loc := range locations {
 		// Use GetFullDefinition to find container
-		_, containerLoc, _, err := GetFullDefinition(ctx, client, loc)
-		if err != nil {
+		matchingSymbols, _ := identifyOverlappingSymbols(ctx, client, loc)
+		if len(matchingSymbols) == 0 {
 			// If container not found, just use the location's line
 			refLine := int(loc.Range.Start.Line)
 			linesToShow[refLine] = true
@@ -163,9 +179,11 @@ func GetLineRangesToDisplay(ctx context.Context, client *lsp.Client, locations [
 			continue
 		}
 
+		containerRange := matchingSymbols[0].Range
+
 		// Add container start and end lines
-		containerStart := int(containerLoc.Range.Start.Line)
-		containerEnd := int(containerLoc.Range.End.Line)
+		containerStart := int(containerRange.Start.Line)
+		containerEnd := int(containerRange.End.Line)
 		linesToShow[containerStart] = true
 		// linesToShow[containerEnd] = true