Deprecate legacy $call syntax

Summary:
Because we represented call signatures as a specially-named `$call` property in
the props map, some people found out about that and started using `$call` in
their type annotations.

Where possible, people should use the actual call property syntax instead. I
will also add support for `[[call]]` internal slot properties in a follow-up.

Note that I moved the support for $call from `Class_sig.elements` to
`Type_annotation.add_interface_properties`, in order to have a handle on the
location of the id. This means that using `$call` properties on an actual
runtime class will not introduce a callable signature. That especially should
never have worked.

Reviewed By: panagosg7

Differential Revision: D8042447

fbshipit-source-id: e45a464a52d0657bd56582f669c023158d67f76a

src/commands/config/flowConfig.ml

@@ -970,6 +970,7 @@ let is_not_comment =
       comment_regexps)
 
 let default_lint_severities = [
+  Lints.DeprecatedCallSyntax, (Severity.Err, None);
 ]
 
 let read filename =

src/common/lints/lints.ml

@@ -21,6 +21,7 @@ type lint_kind =
   | UnsafeGettersSetters
   | InexactSpread
   | UnnecessaryOptionalChain
+  | DeprecatedCallSyntax
 
 let string_of_sketchy_null_kind = function
   | SketchyBool -> "sketchy-null-bool"
@@ -38,6 +39,7 @@ let string_of_kind = function
   | UnsafeGettersSetters -> "unsafe-getters-setters"
   | InexactSpread -> "inexact-spread"
   | UnnecessaryOptionalChain -> "unnecessary-optional-chain"
+  | DeprecatedCallSyntax -> "deprecated-call-syntax"
 
 let kinds_of_string = function
   | "sketchy-null" -> Some [
@@ -58,6 +60,7 @@ let kinds_of_string = function
   | "unsafe-getters-setters" -> Some [UnsafeGettersSetters]
   | "inexact-spread" -> Some [InexactSpread]
   | "unnecessary-optional-chain" -> Some [UnnecessaryOptionalChain]
+  | "deprecated-call-syntax" -> Some [DeprecatedCallSyntax]
   | _ -> None
 
 module LintKind = struct

src/common/lints/lints.mli

@@ -21,6 +21,7 @@ type lint_kind =
  | UnsafeGettersSetters
  | InexactSpread
  | UnnecessaryOptionalChain
+ | DeprecatedCallSyntax
 
 val string_of_kind: lint_kind -> string
 

src/typing/class_sig.ml

@@ -23,7 +23,8 @@ type signature = {
   methods: (Loc.t option * Func_sig.t) Nel.t SMap.t;
   getters: (Loc.t option * Func_sig.t) SMap.t;
   setters: (Loc.t option * Func_sig.t) SMap.t;
-  calls: Func_sig.t list;
+  calls: Type.t list;
+  call_deprecated: Type.t option;
 }
 
 type t = {
@@ -64,6 +65,7 @@ let empty id reason tparams tparams_map super =
     getters = SMap.empty;
     setters = SMap.empty;
     calls = [];
+    call_deprecated = None;
   } in
   let structural, super, ssuper, implements =
     let open Type in
@@ -201,10 +203,21 @@ let append_method ~static name loc fsig x =
     setters = SMap.remove name s.setters;
   }) x
 
-let append_call ~static fsig = map_sig ~static (fun s -> {
-  s with
-  calls = fsig :: s.calls
-})
+let append_call ~static t = map_sig ~static (fun s ->
+  (* Note that $call properties always override the call property syntax.
+     As before, if both are present, the $call property is used and the call
+     property is ignored. *)
+  match s.call_deprecated with
+  | None -> { s with calls = t :: s.calls }
+  | Some _ -> s
+)
+
+let add_call_deprecated ~static t = map_sig ~static (fun s ->
+  (* Note that $call properties always override the call property syntax.
+     As before, if both are present, the $call property is used and the call
+     property is ignored. *)
+  { s with call_deprecated = Some t; calls = [] }
+)
 
 let add_getter ~static name loc fsig x =
   let flat = static || x.structural in
@@ -252,7 +265,8 @@ let subst_sig cx map s =
     methods = SMap.map (Nel.map subst_func_sig) s.methods;
     getters = SMap.map (subst_func_sig) s.getters;
     setters = SMap.map (subst_func_sig) s.setters;
-    calls = List.map (Func_sig.subst cx map) s.calls;
+    calls = List.map (Flow.subst cx map) s.calls;
+    call_deprecated = Option.map ~f:(Flow.subst cx map) s.call_deprecated;
   }
 
 let generate_tests cx f x =
@@ -351,33 +365,24 @@ let elements cx ~tparams_map ?constructor s =
     SMap.add name (to_field fld) acc
   ) s.proto_fields methods in
 
-  let call = match List.rev_map (Func_sig.methodtype cx) s.calls with
-  | [] -> None
-  | [t] -> Some t
-  | t0::t1::ts ->
-    let open Type in
-    let t = DefT (reason_of_t t0, IntersectionT (InterRep.make t0 t1 ts)) in
-    Some t
-  in
-
   (* Previously, call properties were stored in the props map under the key
      $call. Unfortunately, this made it possible to specify call properties
      using this syntax in interfaces, declared classes, and even normal classes.
 
-     Soon, I will deprecate this syntax, but for now the previous behavior is
-     (partially) preserved. A field-like, covariant or invariant field property
-     named $call is special-cased to be a call property. Everything else is left
-     as a normal named property.
-
      Note that $call properties always override the call property syntax
      As before, if both are present, the $call property is used and the call
      property is ignored. *)
-
-  let fields, call = Type.(match SMap.get "$call" fields with
-  | Some (Field (_, t, (Positive | Neutral))) ->
-    SMap.remove "$call" fields, Some t
-  | _ -> fields, call
-  ) in
+  let call = match s.call_deprecated with
+  | Some t -> Some t
+  | None ->
+    match List.rev s.calls with
+    | [] -> None
+    | [t] -> Some t
+    | t0::t1::ts ->
+      let open Type in
+      let t = DefT (reason_of_t t0, IntersectionT (InterRep.make t0 t1 ts)) in
+      Some t
+  in
 
   (* Only un-initialized fields require annotations, so determine now
    * (syntactically) which fields have initializers *)

src/typing/class_sig.mli

@@ -77,7 +77,9 @@ val add_method: static:bool -> string -> Loc.t option -> Func_sig.t -> t -> t
     of a single overloaded method. *)
 val append_method: static:bool -> string -> Loc.t option -> Func_sig.t -> t -> t
 
-val append_call: static:bool -> Func_sig.t -> t -> t
+val append_call: static:bool -> Type.t -> t -> t
+
+val add_call_deprecated: static:bool -> Type.t -> t -> t
 
 (** Add getter to signature. *)
 val add_getter: static:bool -> string -> Loc.t option -> Func_sig.t -> t -> t

src/typing/debug_js.ml

@@ -2739,6 +2739,8 @@ let dump_flow_error =
       spf "EDeprecatedType (%s)" (string_of_loc loc)
     | EUnsafeGettersSetters loc ->
       spf "EUnclearGettersSetters (%s)" (string_of_loc loc)
+    | EDeprecatedCallSyntax loc ->
+      spf "EDeprecatedCallSyntax (%s)" (string_of_loc loc)
     | EUnusedSuppression loc ->
       spf "EUnusedSuppression (%s)" (string_of_loc loc)
     | ELintSetting (loc, kind) ->

src/typing/flow_error.ml

@@ -164,6 +164,7 @@ type error_message =
   | EOptionalChainingMethods of Loc.t
   | EUnnecessaryOptionalChain of Loc.t * reason
   | EInexactSpread of reason * reason
+  | EDeprecatedCallSyntax of Loc.t
 
 and binding_error =
   | ENameAlreadyBound
@@ -384,6 +385,7 @@ let util_use_op_of_msg nope util = function
 | EOptionalChainingMethods _
 | EUnnecessaryOptionalChain _
 | EInexactSpread _
+| EDeprecatedCallSyntax _
   -> nope
 
 (* Rank scores for signals of different strength on an x^2 scale so that greater
@@ -1938,6 +1940,10 @@ let rec error_of_msg ~trace_reasons ~source_file =
     mk_error ~trace_infos ~kind:(LintError Lints.UnsafeGettersSetters) loc
       [text "Getters and setters can have side effects and are unsafe."]
 
+  | EDeprecatedCallSyntax loc ->
+    mk_error ~trace_infos ~kind:(LintError Lints.DeprecatedCallSyntax) loc
+      [text "Deprecated $call syntax. Use callable property syntax instead."]
+
   | EUnusedSuppression loc ->
     mk_error ~trace_infos loc
       [text "Unused suppression comment."]

src/typing/type_annotation.ml

@@ -674,9 +674,13 @@ let rec convert cx tparams_map = Ast.Type.(function
     | Object.CallProperty (_, { Object.CallProperty.static; _ }) -> not static
     | _ -> false
   ) properties in
-  let mk_object ~exact (call_props, dict, props_map, proto) =
+  let mk_object ~exact (call_props, dict, props_map, proto, call_deprecated) =
     let call = match List.rev call_props with
-      | [] -> None
+      | [] ->
+        (* Note that call properties using the call property syntax always override
+           $call properties. Previously, if both were present, the $call property
+           was ignored, but is now left as a named property. *)
+        call_deprecated
       | [t] -> Some t
       | t0::t1::ts ->
         let callable_reason = mk_reason (RCustom "callable object type") loc in
@@ -689,11 +693,6 @@ let rec convert cx tparams_map = Ast.Type.(function
        using this syntax in object types, and some libraries adopted this
        syntax.
 
-       Soon, I will deprecate this syntax, but for now the previous behavior is
-       (partially) preserved. A field-like, covariant or invariant property
-       named $call is special-cased to be a call property. Everything else is
-       left as a normal named property.
-
        Note that call properties using the call property syntax always override
        $call properties. Previously, if both were present, the $call property
        was ignored, but is now left as a named property. *)
@@ -731,14 +730,27 @@ let rec convert cx tparams_map = Ast.Type.(function
     DefT (mk_reason reason_desc loc,
       ObjT (mk_objecttype ~flags ~dict ~call pmap proto))
   in
-  let property loc prop props proto =
+  let property loc prop props proto call_deprecated =
     match prop with
     | { Object.Property.
         key; value = Object.Property.Init value; optional; variance; _method;
         static = _; (* object types don't have static props *)
         proto = _; (* ditto proto props *)
       } ->
       begin match key with
+      (* Previously, call properties were stored in the props map under the key
+         $call. Unfortunately, this made it possible to specify call properties
+         using this syntax in object types, and some libraries adopted this
+         syntax.
+
+         Note that call properties using the call property syntax always override
+         $call properties. Previously, if both were present, the $call property
+         was ignored, but is now left as a named property. *)
+      | Ast.Expression.Object.Property.Identifier (loc, "$call") ->
+          Flow.add_output cx Flow_error.(EDeprecatedCallSyntax loc);
+          let t = convert cx tparams_map value in
+          let t = if optional then Type.optional t else t in
+          props, proto, Some t
       | Ast.Expression.Object.Property.Literal
           (loc, { Ast.Literal.value = Ast.Literal.String name; _ })
       | Ast.Expression.Object.Property.Identifier (loc, name) ->
@@ -750,7 +762,8 @@ let rec convert cx tparams_map = Ast.Type.(function
             let proto = Tvar.mk_where cx reason (fun tout ->
               Flow.flow cx (t, ObjTestProtoT (reason, tout))
             ) in
-            props, Some (Flow.mk_typeof_annotation cx reason proto)
+            let proto = Some (Flow.mk_typeof_annotation cx reason proto) in
+            props, proto, call_deprecated
           else
             let t = if optional then Type.optional t else t in
             let key_loc = match key with
@@ -763,13 +776,13 @@ let rec convert cx tparams_map = Ast.Type.(function
             Env.add_type_table_info cx ~tparams_map key_loc id_info;
             let polarity = if _method then Positive else polarity variance in
             let props = SMap.add name (Field (Some loc, t, polarity)) props in
-            props, proto
+            props, proto, call_deprecated
       | Ast.Expression.Object.Property.Literal (loc, _)
       | Ast.Expression.Object.Property.PrivateName (loc, _)
       | Ast.Expression.Object.Property.Computed (loc, _)
           ->
         Flow.add_output cx (FlowError.EUnsupportedKeyInObjectType loc);
-        props, proto
+        props, proto, call_deprecated
       end
 
     (* unsafe getter property *)
@@ -783,7 +796,7 @@ let rec convert cx tparams_map = Ast.Type.(function
       let id_info = name, return_t, Type_table.Other in
       Env.add_type_table_info cx ~tparams_map id_loc id_info;
       let props = Properties.add_getter name (Some id_loc) return_t props in
-      props, proto
+      props, proto, call_deprecated
 
     (* unsafe setter property *)
     | { Object.Property.
@@ -796,7 +809,7 @@ let rec convert cx tparams_map = Ast.Type.(function
       let id_info = name, param_t, Type_table.Other in
       Env.add_type_table_info cx ~tparams_map id_loc id_info;
       let props = Properties.add_setter name (Some id_loc) param_t props in
-      props, proto
+      props, proto, call_deprecated
 
 
     | { Object.Property.
@@ -805,11 +818,15 @@ let rec convert cx tparams_map = Ast.Type.(function
       } ->
       Flow.add_output cx
         Flow_error.(EUnsupportedSyntax (loc, ObjectPropertyGetSet));
-      props, proto
+      props, proto, call_deprecated
   in
   let add_call c = function
-    | None -> Some ([c], None, SMap.empty, None)
-    | Some (cs, d, pmap, proto) -> Some (c::cs, d, pmap, proto)
+    | None -> Some ([c], None, SMap.empty, None, None)
+    | Some (cs, d, pmap, proto, _) ->
+      (* Note that call properties using the call property syntax always override
+         $call properties. Previously, if both were present, the $call property
+         was ignored, but is now left as a named property. *)
+      Some (c::cs, d, pmap, proto, None)
   in
   let make_dict { Object.Indexer.id; key; value; variance; _ } =
     Some { Type.
@@ -820,20 +837,21 @@ let rec convert cx tparams_map = Ast.Type.(function
     }
   in
   let add_dict loc indexer = function
-    | None -> Some ([], make_dict indexer, SMap.empty, None)
-    | Some (cs, None, pmap, proto) -> Some (cs, make_dict indexer, pmap, proto)
-    | Some (_, Some _, _, _) as o ->
+    | None -> Some ([], make_dict indexer, SMap.empty, None, None)
+    | Some (cs, None, pmap, proto, call_deprecated) ->
+      Some (cs, make_dict indexer, pmap, proto, call_deprecated)
+    | Some (_, Some _, _, _, _) as o ->
       Flow.add_output cx
         FlowError.(EUnsupportedSyntax (loc, MultipleIndexers));
       o
   in
   let add_prop loc p = function
     | None ->
-      let pmap, proto = property loc p SMap.empty None in
-      Some ([], None, pmap, proto)
-    | Some (cs, d, pmap, proto) ->
-      let pmap, proto = property loc p pmap proto in
-      Some (cs, d, pmap, proto)
+      let pmap, proto, call_deprecated = property loc p SMap.empty None None in
+      Some ([], None, pmap, proto, call_deprecated)
+    | Some (cs, d, pmap, proto, call_deprecated) ->
+      let pmap, proto, call_deprecated = property loc p pmap proto call_deprecated in
+      Some (cs, d, pmap, proto, call_deprecated)
   in
   let o, ts, spread = List.fold_left (
     fun (o, ts, spread) -> function
@@ -865,7 +883,7 @@ let rec convert cx tparams_map = Ast.Type.(function
   in
   (match ts with
   | [] ->
-    let t = mk_object ~exact ([], None, SMap.empty, None) in
+    let t = mk_object ~exact ([], None, SMap.empty, None, None) in
     if exact
     then ExactT (mk_reason (RExactType reason_desc) loc, t)
     else t
@@ -1098,9 +1116,9 @@ and mk_interface_super cx tparams_map (loc, {Ast.Type.Generic.id; targs}) =
 and add_interface_properties cx tparams_map properties s =
   let open Class_sig in
   List.fold_left Ast.Type.Object.(fun x -> function
-    | CallProperty (loc, { CallProperty.value = (_, func); static }) ->
-      let fsig = mk_func_sig cx tparams_map loc func in
-      append_call ~static fsig x
+    | CallProperty (loc, { CallProperty.value = (_, ft); static }) ->
+      let t = convert cx tparams_map (loc, Ast.Type.Function ft) in
+      append_call ~static t x
     | Indexer (loc, { Indexer.static; _ }) when mem_field ~static "$key" x ->
       Flow.add_output cx
         Flow_error.(EUnsupportedSyntax (loc, MultipleIndexers));
@@ -1122,6 +1140,21 @@ and add_interface_properties cx tparams_map properties s =
       | _, Property.Computed (loc, _), _ ->
           Flow.add_output cx (Flow_error.EUnsupportedSyntax (loc, Flow_error.IllegalName));
           x
+
+      (* Previously, call properties were stored in the props map under the key
+         $call. Unfortunately, this made it possible to specify call properties
+         using this syntax in interfaces, declared classes, and even normal classes.
+
+         Note that $call properties always override the call property syntax.
+         As before, if both are present, the $call property is used and the call
+         property is ignored. *)
+      | _, Property.Identifier (id_loc, "$call"),
+          Ast.Type.Object.Property.Init value when not proto ->
+          Flow.add_output cx Flow_error.(EDeprecatedCallSyntax id_loc);
+          let t = convert cx tparams_map value in
+          let t = if optional then Type.optional t else t in
+          add_call_deprecated ~static t x
+
       | true, Property.Identifier (id_loc, name),
           Ast.Type.Object.Property.Init (_, Ast.Type.Function func) ->
           let fsig = mk_func_sig cx tparams_map loc func in