From 87f77ba94928241c66fa2858b8345457d3a70455 Mon Sep 17 00:00:00 2001
From: Wilfred Hughes <wilfred@fb.com>
Date: Wed, 30 Mar 2022 11:48:01 -0700
Subject: [PATCH] Skip docblocks when hovering on declarations

Summary: Previously we would show docblocks whenever we hovered over a definition, even if we were already at the definition site. This was redundant and slightly confusing.

Reviewed By: hgoldstein

Differential Revision: D35160615

fbshipit-source-id: 08ae8eeee78b50f42c825165a30b1c72dbf63479
---
 hphp/hack/src/server/serverHover.ml                |  8 +++-
 hphp/hack/test/hover/method.php.exp                |  1 -
 hphp/hack/test/integration_ml/test_server_hover.ml | 50 +++++++++++-----------
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/hphp/hack/src/server/serverHover.ml b/hphp/hack/src/server/serverHover.ml
index a91f46943fb..6ae7ec2329c 100644
--- a/hphp/hack/src/server/serverHover.ml
+++ b/hphp/hack/src/server/serverHover.ml
@@ -28,7 +28,9 @@ let filter_class_and_constructor results =
 
 let make_hover_doc_block ctx entry occurrence def_opt =
   match def_opt with
-  | Some def ->
+  | Some def when not occurrence.SymbolOccurrence.is_declaration ->
+    (* The docblock is useful at the call site, but it's redundant at
+       the definition site. *)
     let base_class_name = SymbolOccurrence.enclosing_class occurrence in
     ServerDocblockAt.go_comments_for_symbol_ctx
       ~ctx
@@ -36,7 +38,9 @@ let make_hover_doc_block ctx entry occurrence def_opt =
       ~def
       ~base_class_name
     |> Option.to_list
-  | None -> []
+  | None
+  | Some _ ->
+    []
 
 let make_hover_const_definition entry def_opt =
   match def_opt with
diff --git a/hphp/hack/test/hover/method.php.exp b/hphp/hack/test/hover/method.php.exp
index de7ca560549..4e91ae243d7 100644
--- a/hphp/hack/test/hover/method.php.exp
+++ b/hphp/hack/test/hover/method.php.exp
@@ -1,2 +1 @@
 Foo::bar
-Do stuff.
diff --git a/hphp/hack/test/integration_ml/test_server_hover.ml b/hphp/hack/test/integration_ml/test_server_hover.ml
index e2099a75c66..8d47d8b0c2f 100644
--- a/hphp/hack/test/integration_ml/test_server_hover.ml
+++ b/hphp/hack/test/integration_ml/test_server_hover.ml
@@ -379,31 +379,31 @@ class DocBlockDerived extends DocBlockBase {}
 
 // We don't want the line comment above to be part of this docblock.
 // We do want both these lines though.
-function line_comment_with_break(): void {}
-//       89^:10
+function line_comment_with_break(): void { line_comment_with_break(); }
+//                                       89^:44
 
 // This is another special case.
 /** Only this should be part of the docblock. */
-function two_comment_types(): void {}
-//       ^94:10
+function two_comment_types(): void { two_comment_types(); }
+//                                   ^94:38
 
 // There are too many blank lines between this comment and what it's commenting
 // on.
 
 
-function too_many_blank_lines(): void {}
-//       ^101:10
+function too_many_blank_lines(): void { too_many_blank_lines(); }
+//                                      ^101:41
 
 // For legacy reasons, we have to support a single linebreak between a docblock
 // and the item the docblock is for.
 
-function one_linebreak_is_okay(): void {}
-//       ^107:10
+function one_linebreak_is_okay(): void { one_linebreak_is_okay(); }
+//                                       ^107:42
 
 /** A function with an HH_FIXME. */
 /* HH_FIXME[4030] Missing return type hint. */
-function needs_fixing() {}
-//       ^112:10
+function needs_fixing() { needs_fixing(); }
+//                        ^112:27
 "
 
 let docblock_cases =
@@ -536,50 +536,50 @@ the other stars.";
           pos = pos_at (27, 14) (27, 28);
         };
       ] );
-    ( ("docblock.php", 89, 10),
+    ( ("docblock.php", 89, 44),
       [
         {
-          snippet = "line_comment_with_break";
+          snippet = "function line_comment_with_break(): void";
           addendum =
             [
               "We don't want the line comment above to be part of this docblock.\nWe do want both these lines though.";
             ];
-          pos = pos_at (89, 10) (89, 32);
+          pos = pos_at (89, 44) (89, 66);
         };
       ] );
-    ( ("docblock.php", 94, 10),
+    ( ("docblock.php", 94, 38),
       [
         {
-          snippet = "two_comment_types";
+          snippet = "function two_comment_types(): void";
           addendum = ["Only this should be part of the docblock."];
-          pos = pos_at (94, 10) (94, 26);
+          pos = pos_at (94, 38) (94, 54);
         };
       ] );
-    ( ("docblock.php", 101, 10),
+    ( ("docblock.php", 101, 41),
       [
         {
-          snippet = "too_many_blank_lines";
+          snippet = "function too_many_blank_lines(): void";
           addendum = [];
-          pos = pos_at (101, 10) (101, 29);
+          pos = pos_at (101, 41) (101, 60);
         };
       ] );
-    ( ("docblock.php", 107, 10),
+    ( ("docblock.php", 107, 42),
       [
         {
-          snippet = "one_linebreak_is_okay";
+          snippet = "function one_linebreak_is_okay(): void";
           addendum =
             [
               "For legacy reasons, we have to support a single linebreak between a docblock\nand the item the docblock is for.";
             ];
-          pos = pos_at (107, 10) (107, 30);
+          pos = pos_at (107, 42) (107, 62);
         };
       ] );
-    ( ("docblock.php", 112, 10),
+    ( ("docblock.php", 112, 27),
       [
         {
-          snippet = "needs_fixing";
+          snippet = "function needs_fixing(): _";
           addendum = ["A function with an HH_FIXME."];
-          pos = pos_at (112, 10) (112, 21);
+          pos = pos_at (112, 27) (112, 38);
         };
       ] );
   ]
-- 
2.11.4.GIT