[flow][refactor] Rename and make the name already bound error more de…

…tailed

Summary:
In the next diff, we are going to error on libdef overrides using the same signature binding error. This diff improves the current one to give it a more specific name, and also does some driveby cleaning to make the error more detailed by pointing our the existing binding.

Changelog: [internal]

Reviewed By: jbrown215

Differential Revision: D70570825

fbshipit-source-id: 0a75674e81414ade595836d5c86788e4d3a2f8f0

src/parser_utils/signature_builder/signature_error.ml

@@ -15,6 +15,12 @@ type 'loc t =
   | UnexpectedExpression of 'loc * Flow_ast_utils.ExpressionSort.t
 [@@deriving show, iter, map]
 
-type 'loc binding_validation_t = NameAlreadyBound of 'loc [@@deriving show, iter, map]
+type 'loc binding_validation_t =
+  | NamespacedNameAlreadyBound of {
+      name: string;
+      invalid_binding_loc: 'loc;
+      existing_binding_loc: 'loc;
+    }
+[@@deriving show, iter, map]
 
 let compare = Stdlib.compare

src/parser_utils/type_sig/type_sig_parse.ml

@@ -1091,11 +1091,22 @@ module Scope = struct
       let def : _ local_binding = NamespaceBinding { id_loc; name; values; types } in
       let union_values_and_types existing_values existing_types values types =
         let new_binding name (l, _) =
-          if SMap.mem name existing_values || SMap.mem name existing_types then (
-            tbls.additional_errors <- Signature_error.NameAlreadyBound l :: tbls.additional_errors;
+          match SMap.find_opt name existing_values with
+          | Some (existing_binding_loc, _) ->
+            tbls.additional_errors <-
+              Signature_error.NamespacedNameAlreadyBound
+                { name; invalid_binding_loc = l; existing_binding_loc }
+              :: tbls.additional_errors;
             false
-          ) else
-            true
+          | None ->
+            (match SMap.find_opt name existing_types with
+            | Some (existing_binding_loc, _) ->
+              tbls.additional_errors <-
+                Signature_error.NamespacedNameAlreadyBound
+                  { name; invalid_binding_loc = l; existing_binding_loc }
+                :: tbls.additional_errors;
+              false
+            | None -> true)
         in
         let values = SMap.filter new_binding values in
         let types = SMap.filter new_binding types in

src/typing/debug_js.ml

@@ -1636,8 +1636,11 @@ let dump_error_message =
     | EUnexpectedTemporaryBaseType loc ->
       spf "EUnexpectedTemporaryBaseType (%s)" (string_of_aloc loc)
     | ECannotDelete (l1, r1) -> spf "ECannotDelete (%s, %s)" (string_of_aloc l1) (dump_reason cx r1)
-    | ESignatureBindingValidation (Signature_error.NameAlreadyBound l) ->
-      spf "ESignatureBindingValidation (NameAlreadyBound %s)" (string_of_aloc l)
+    | ESignatureBindingValidation
+        (Signature_error.NamespacedNameAlreadyBound { invalid_binding_loc; _ }) ->
+      spf
+        "ESignatureBindingValidation (NamespacedNameAlreadyBound %s)"
+        (string_of_aloc invalid_binding_loc)
     | ESignatureVerification sve ->
       let msg = string_of_signature_error ALoc.debug_to_string sve in
       spf "ESignatureVerification (%s)" msg

src/typing/errors/error_message.ml

@@ -1980,7 +1980,7 @@ let loc_of_msg : 'loc t' -> 'loc option = function
   | ESignatureBindingValidation e ->
     Signature_error.(
       (match e with
-      | NameAlreadyBound loc -> Some loc)
+      | NamespacedNameAlreadyBound { invalid_binding_loc; _ } -> Some invalid_binding_loc)
     )
   | ESignatureVerification sve ->
     Signature_error.(
@@ -2632,8 +2632,10 @@ let friendly_message_of_msg = function
     Normal (MessageCannotExportRenamedDefault { name; is_reexport })
   | EUnexpectedTemporaryBaseType _ -> Normal MessageUnexpectedTemporaryBaseType
   | ECannotDelete (_, expr) -> Normal (MessageCannotDelete expr)
-  | ESignatureBindingValidation (Signature_error.NameAlreadyBound _) ->
-    Normal MessageCannotDeclareAlreadyBoundNameInLibdef
+  | ESignatureBindingValidation
+      (Signature_error.NamespacedNameAlreadyBound { name; existing_binding_loc; _ }) ->
+    let x = mk_reason (RIdentifier (OrdinaryName name)) existing_binding_loc in
+    Normal (MessageCannotDeclareAlreadyBoundNameInNamespace x)
   | ESignatureVerification sve -> Normal (MessageCannotBuildTypedInterface sve)
   | EUnreachable _ -> Normal MessageUnreachableCode
   | EInvalidObjectKit { reason; reason_op = _; use_op } ->

src/typing/errors/flow_intermediate_error.ml

@@ -1637,8 +1637,12 @@ let to_printable_error :
       ]
     | MessageCannotDeclareAlreadyBoundName x ->
       [text "Cannot declare "; Friendly.ref x; text " because the name is already bound."]
-    | MessageCannotDeclareAlreadyBoundNameInLibdef ->
-      [text "Cannot declare the name in library definition because the name is already bound."]
+    | MessageCannotDeclareAlreadyBoundNameInNamespace x ->
+      [
+        text "Cannot declare the name in the namespace because the name ";
+        ref x;
+        text " is already bound.";
+      ]
     | MessageCannotDelete expr ->
       [
         text "Cannot delete ";

src/typing/errors/flow_intermediate_error_types.ml

@@ -396,7 +396,7 @@ type 'loc message =
   | MessageCannotCreateExactType of 'loc virtual_reason
   | MessageCannotDeclareAlreadyBoundGlobal of concrete_reason
   | MessageCannotDeclareAlreadyBoundName of concrete_reason
-  | MessageCannotDeclareAlreadyBoundNameInLibdef
+  | MessageCannotDeclareAlreadyBoundNameInNamespace of 'loc virtual_reason
   | MessageCannotDelete of 'loc virtual_reason
   | MessageCannotDetermineEmptyArrayLiteralType
   | MessageCannotDetermineModuleType

tests/declare_namespace/declare_namespace.exp

@@ -1,18 +1,30 @@
 Error ------------------------------------------------------------------------------------- flow-typed/namespaces.js:8:8
 
-Cannot declare the name in library definition because the name is already bound. [signature-verification-failure]
+Cannot declare the name in the namespace because the name `a` [1] is already bound. [signature-verification-failure]
 
+   flow-typed/namespaces.js:8:8
    8|   type a = string; // error: already bound
              ^
 
+References:
+   flow-typed/namespaces.js:2:17
+   2|   declare const a: string;
+                      ^ [1]
+
 
 Error ------------------------------------------------------------------------------------- flow-typed/namespaces.js:9:8
 
-Cannot declare the name in library definition because the name is already bound. [signature-verification-failure]
+Cannot declare the name in the namespace because the name `b` [1] is already bound. [signature-verification-failure]
 
+   flow-typed/namespaces.js:9:8
    9|   type b = string; // error: already bound
              ^
 
+References:
+   flow-typed/namespaces.js:5:17
+   5|   declare const b: string;
+                      ^ [1]
+
 
 Error ---------------------------------------------------------------------------------------------- binding_test.js:2:6