Stop collecting floating comments in LSP hover (#4259)

Author: stefanvanburen

private/buf/buflsp/hover_test.go

@@ -34,17 +34,18 @@ func TestHover(t *testing.T) {
 	clientJSONConn, testURI := setupLSPServer(t, testProtoPath)
 
 	tests := []struct {
-		name             string
-		line             uint32
-		character        uint32
-		expectedContains string
-		expectNoHover    bool
+		name                string
+		line                uint32
+		character           uint32
+		expectedContains    string
+		expectedNotContains string
+		expectNoHover       bool
 	}{
 		{
 			name:             "hover_on_user_message",
-			line:             7, // Line with "message User {"
-			character:        8, // On the word "User"
-			expectedContains: "User represents a user in the system",
+			line:             7,               // Line with "message User {"
+			character:        8,               // On the word "User"
+			expectedContains: "system.\nThis", // Ensure newline between comment lines
 		},
 		{
 			name:             "hover_on_id_field",
@@ -76,6 +77,16 @@ func TestHover(t *testing.T) {
 			character:        6,  // On "GetUser"
 			expectedContains: "GetUser retrieves a user by their ID",
 		},
+		{
+			name:      "hover_on_deprecated_option",
+			line:      37, // Line with "option deprecated = true;"
+			character: 11, // On "deprecated"
+			// We don't want the hover info to include the floating comment that is separated by newlines
+			// from the comment above the option.
+			// Ref: https://buf.build/protocolbuffers/wellknowntypes/file/main:google/protobuf/descriptor.proto#L946
+			expectedNotContains: "Buffers.", // From last line of the previous floating comment.
+			expectedContains:    "Is this method deprecated?",
+		},
 		{
 			name:             "hover_on_status_type_reference",
 			line:             15, // Line with "Status status = 3;"
@@ -84,7 +95,7 @@ func TestHover(t *testing.T) {
 		},
 		{
 			name:             "hover_on_user_type_reference",
-			line:             45, // Line with "User user = 1;"
+			line:             50, // Line with "User user = 1;"
 			character:        2,  // On "User" type
 			expectedContains: "User represents a user in the system",
 		},
@@ -145,10 +156,17 @@ func TestHover(t *testing.T) {
 
 			if tt.expectNoHover {
 				assert.Nil(t, hover, "expected no hover information")
-			} else if tt.expectedContains != "" {
-				require.NotNil(t, hover, "expected hover to be non-nil")
-				assert.Equal(t, protocol.Markdown, hover.Contents.Kind)
-				assert.Contains(t, hover.Contents.Value, tt.expectedContains)
+			} else {
+				if tt.expectedContains != "" {
+					require.NotNil(t, hover, "expected hover to be non-nil")
+					assert.Equal(t, protocol.Markdown, hover.Contents.Kind)
+					assert.Contains(t, hover.Contents.Value, tt.expectedContains)
+				}
+				if tt.expectedNotContains != "" {
+					require.NotNil(t, hover, "expected hover to be non-nil")
+					assert.Equal(t, protocol.Markdown, hover.Contents.Kind)
+					assert.NotContains(t, hover.Contents.Value, tt.expectedNotContains)
+				}
 			}
 		})
 	}

private/buf/buflsp/symbol.go

@@ -453,32 +453,29 @@ func (s *symbol) getDocsFromComments() string {
 
 	var comments []string
 	// We drop the other side of "Around" because we only care about the beginning -- we're
-	// traversing backwards for leading comemnts only.
-	_, start := def.Context().Stream().Around(def.Span().Start)
-	cursor := token.NewCursorAt(start)
-	t := cursor.PrevSkippable()
-	var addNewline bool
-	for !t.IsZero() {
-		switch t.Kind() {
-		case token.Comment:
-			text := t.Text()
-			if addNewline {
+	// traversing backwards for leading comments only.
+	tok, _ := def.Context().Stream().Around(def.Span().Start)
+	cursor := token.NewCursorAt(tok)
+	var seenNewline bool
+	for {
+		t := cursor.PrevSkippable()
+		if t.Kind() == token.Comment {
+			text := commentToMarkdown(t.Text())
+			if seenNewline {
 				text += "\n"
-				addNewline = false
 			}
-			comments = append(comments, commentToMarkdown(text))
-		case token.Space:
-			// If the space token only contains spaces (e.g. code indentation), then we drop it.
-			// If the space token ends in a new-line, we append it to the comment above for formatting.
-			if strings.HasSuffix(t.Text(), "\n") {
-				addNewline = true
+			seenNewline = false
+			comments = append(comments, text)
+		} else if t.Kind() == token.Space {
+			if strings.Contains(t.Text(), "\n") {
+				if seenNewline {
+					break
+				}
+				seenNewline = true
 			}
-		}
-		prev := cursor.PeekPrevSkippable()
-		if !prev.Kind().IsSkippable() {
+		} else {
 			break
 		}
-		t = cursor.PrevSkippable()
 	}
 	comments = lineUpComments(comments)
 	// Reverse the list and return joined.

private/buf/buflsp/testdata/hover/test.proto

@@ -32,6 +32,11 @@ enum Status {
 service UserService {
   // GetUser retrieves a user by their ID.
   rpc GetUser(GetUserRequest) returns (GetUserResponse);
+
+  // DeleteUser deletes a user by their ID.
+  rpc DeleteUser(DeleteUserRequest) returns (DeleteUserResponse) {
+    option deprecated = true;
+  }
 }
 
 // GetUserRequest is the request message for GetUser.
@@ -45,3 +50,15 @@ message GetUserResponse {
   // The retrieved user.
   User user = 1;
 }
+
+// DeleteUserRequest is the request message for DeleteUser.
+message DeleteUserRequest {
+  // The ID of the user to delete.
+  string user_id = 1;
+}
+
+// DeleteUserResponse is the response message for DeleteUser.
+message DeleteUserResponse {
+  // Whether the deletion was successful.
+  bool success = 1;
+}