make redundant sealed annotation a warning instead of a lint error

Summary:
We used to raise a lint if a classish was sealed against another classish that was not a subtype.  The `tco_enforce_sealed_subclasses` option from GlobalOptions turned the lint into a strict error but defaulted to false.

This diff removes the global option and the strict error, and turns the lint into a warning.  Additionally, fixes the incorrect wording used in a few cases (eg a class does not _extend_ an interface, it _ implements_ it).

Reviewed By: mheiber

Differential Revision: D89376668

fbshipit-source-id: 548553fae16a50b4c741111d9b029069f6c036b2

Author: Francesco Zappa Nardelli

hphp/hack/src/client_and_server/serverConfig.ml

@@ -484,8 +484,6 @@ let load_config (config : Config_file_common.t) (options : GlobalOptions.t) :
       (bool_opt Config_keys.require_extends_implements_ancestors config)
     ?tco_strict_value_equality:
       (bool_opt Config_keys.strict_value_equality config)
-    ?tco_enforce_sealed_subclasses:
-      (bool_opt Config_keys.enforce_sealed_subclasses config)
     ?tco_implicit_inherit_sdt:(bool_opt Config_keys.implicit_inherit_sdt config)
     ?tco_repo_stdlib_path:(string_opt Config_keys.repo_stdlib_path config)
     ?tco_explicit_consistent_constructors:

hphp/hack/src/diagnostics/error_codes.ml

@@ -737,7 +737,7 @@ module Typing = struct
     | RigidTVarEscape [@value 4442]
     | StrictEqValueIncompatibleTypes [@value 4443]
     | ModuleError [@value 4444]
-    | SealedNotSubtype [@value 4445]
+    (* | SealedNotSubtypeDEPRECATED [@value 4445] *)
     | ModuleHintError [@value 4446]
     | MemoizeObjectWithoutGlobals [@value 4447]
     | ExpressionTreeNonPublicProperty [@value 4448]
@@ -833,6 +833,7 @@ module Warning = struct
     | NullsafePipeOnNull [@value 12032]
     | UnboundNameWarning [@value 12033]
     | SetOrKeysetArrayGet [@value 12034]
+    | SealedNotSubtype [@value 12035]
       (* Add new Warning codes here! Comment out when deprecating. *)
   [@@deriving enum, ord, show { with_path = false }]
 end

hphp/hack/src/options/globalOptions.ml

@@ -152,7 +152,6 @@ type t = {
   tco_meth_caller_only_public_visibility: bool;
   tco_require_extends_implements_ancestors: bool;
   tco_strict_value_equality: bool;
-  tco_enforce_sealed_subclasses: bool;
   tco_implicit_inherit_sdt: bool;
   tco_repo_stdlib_path: string option;
   tco_explicit_consistent_constructors: int;
@@ -277,7 +276,6 @@ let default =
     tco_meth_caller_only_public_visibility = true;
     tco_require_extends_implements_ancestors = false;
     tco_strict_value_equality = false;
-    tco_enforce_sealed_subclasses = false;
     tco_implicit_inherit_sdt = false;
     tco_repo_stdlib_path = None;
     tco_explicit_consistent_constructors = 0;
@@ -401,7 +399,6 @@ let set
     ?tco_meth_caller_only_public_visibility
     ?tco_require_extends_implements_ancestors
     ?tco_strict_value_equality
-    ?tco_enforce_sealed_subclasses
     ?tco_implicit_inherit_sdt
     ?tco_repo_stdlib_path
     ?tco_explicit_consistent_constructors
@@ -630,10 +627,6 @@ let set
         options.tco_require_extends_implements_ancestors;
     tco_strict_value_equality =
       setting tco_strict_value_equality options.tco_strict_value_equality;
-    tco_enforce_sealed_subclasses =
-      setting
-        tco_enforce_sealed_subclasses
-        options.tco_enforce_sealed_subclasses;
     tco_implicit_inherit_sdt =
       setting tco_implicit_inherit_sdt options.tco_implicit_inherit_sdt;
     tco_repo_stdlib_path =

hphp/hack/src/options/globalOptions.mli

@@ -193,8 +193,6 @@ type t = {
       (** Consider `require extends` and `require implements` as ancestors when checking a class *)
   tco_strict_value_equality: bool;
       (** Emit an error when "==" or "!=" is used to compare values that are incompatible types *)
-  tco_enforce_sealed_subclasses: bool;
-      (** All member of the __Sealed whitelist should be subclasses*)
   tco_implicit_inherit_sdt: bool;
       (** Inherit SDT from parents, without writing <<__SupportDynamicType>> *)
   tco_repo_stdlib_path: string option;
@@ -383,7 +381,6 @@ val set :
   ?tco_meth_caller_only_public_visibility:bool ->
   ?tco_require_extends_implements_ancestors:bool ->
   ?tco_strict_value_equality:bool ->
-  ?tco_enforce_sealed_subclasses:bool ->
   ?tco_implicit_inherit_sdt:bool ->
   ?tco_repo_stdlib_path:string ->
   ?tco_explicit_consistent_constructors:int ->

hphp/hack/src/options/typecheckerOptions.ml

@@ -198,8 +198,6 @@ let require_extends_implements_ancestors t =
 
 let strict_value_equality t = t.GlobalOptions.tco_strict_value_equality
 
-let enforce_sealed_subclasses t = t.GlobalOptions.tco_enforce_sealed_subclasses
-
 let repo_stdlib_path t = t.GlobalOptions.tco_repo_stdlib_path
 
 let everything_sdt t = t.GlobalOptions.po.ParserOptions.everything_sdt

hphp/hack/src/oxidized/gen/error_codes.rs

@@ -3,7 +3,7 @@
 // This source code is licensed under the MIT license found in the
 // LICENSE file in the "hack" directory of this source tree.
 //
-// @generated SignedSource<<0cc28ae99bc1061db33ccad8126c8ba3>>
+// @generated SignedSource<<1979bd32cde8ff3069353e3a5093c26e>>
 //
 // To regenerate this file, run:
 //   buck run @fbcode//mode/dev-nosan-lg fbcode//hphp/hack/src:oxidized_regen
@@ -519,7 +519,6 @@ pub enum Typing {
     RigidTVarEscape = 4442,
     StrictEqValueIncompatibleTypes = 4443,
     ModuleError = 4444,
-    SealedNotSubtype = 4445,
     ModuleHintError = 4446,
     MemoizeObjectWithoutGlobals = 4447,
     ExpressionTreeNonPublicProperty = 4448,
@@ -625,6 +624,7 @@ pub enum Warning {
     NullsafePipeOnNull = 12032,
     UnboundNameWarning = 12033,
     SetOrKeysetArrayGet = 12034,
+    SealedNotSubtype = 12035,
 }
 impl TrivialDrop for Warning {}
 arena_deserializer::impl_deserialize_in_arena!(Warning);

hphp/hack/src/oxidized/gen/global_options.rs

@@ -3,7 +3,7 @@
 // This source code is licensed under the MIT license found in the
 // LICENSE file in the "hack" directory of this source tree.
 //
-// @generated SignedSource<<1a93b3740f1d35ca3922e713522f453e>>
+// @generated SignedSource<<dca96768b0528150f35179020488742c>>
 //
 // To regenerate this file, run:
 //   buck run @fbcode//mode/dev-nosan-lg fbcode//hphp/hack/src:oxidized_regen
@@ -295,8 +295,6 @@ pub struct GlobalOptions {
     pub tco_require_extends_implements_ancestors: bool,
     /// Emit an error when "==" or "!=" is used to compare values that are incompatible types
     pub tco_strict_value_equality: bool,
-    /// All member of the __Sealed whitelist should be subclasses
-    pub tco_enforce_sealed_subclasses: bool,
     /// Inherit SDT from parents, without writing <<__SupportDynamicType>>
     pub tco_implicit_inherit_sdt: bool,
     /// Directory of HSL wrappers defined in the repo, warns on unbound name.

hphp/hack/src/oxidized/manual/global_options_impl.rs

@@ -103,7 +103,6 @@ impl Default for GlobalOptions {
             tco_meth_caller_only_public_visibility: true,
             tco_require_extends_implements_ancestors: false,
             tco_strict_value_equality: false,
-            tco_enforce_sealed_subclasses: false,
             tco_implicit_inherit_sdt: false,
             tco_explicit_consistent_constructors: 0,
             tco_require_types_class_consts: 0,

hphp/hack/src/typing/typing_class.ml

@@ -445,10 +445,10 @@ let check_parent env class_def class_type =
     ()
 
 (** If the class is sealed, check that the elements of the whitelist
-    are descendants of the class. Error or lint depending on option
-    enforce_sealed_subclasses. *)
-let sealed_subtype (c : Nast.class_) ~is_enum ~hard_error ~env =
+    are descendants of the class and emit a warning if not. *)
+let sealed_subtype (c : Nast.class_) ~is_enum ~env =
   let parent_name = snd c.c_name in
+  let parent_kind = c.c_kind in
   let is_sealed (attr : Nast.user_attribute) =
     String.equal (snd attr.ua_name) SN.UserAttributes.uaSealed
   in
@@ -481,37 +481,30 @@ let sealed_subtype (c : Nast.class_) ~is_enum ~hard_error ~env =
           in
           if not includes_ancestor then
             let parent_pos = pos in
-            let child_pos = Cls.pos decl in
             let child_name = Cls.name decl in
             let class_kind = Cls.kind decl in
             let (child_kind, verb) =
-              match class_kind with
-              | Ast_defs.Cclass _ -> ("Class", "extend")
-              | Ast_defs.Cinterface -> ("Interface", "implement")
-              | Ast_defs.Ctrait -> ("Trait", "use")
-              | Ast_defs.Cenum -> ("Enum", "use")
-              | Ast_defs.Cenum_class _ -> ("Enum Class", "extend")
+              Ast_defs.(
+                match (parent_kind, class_kind) with
+                | (Cinterface, Cclass _) -> ("Class", "implement")
+                | (_, Cclass _) -> ("Class", "extend")
+                | (Cinterface, Cinterface) -> ("Interface", "extend")
+                | (_, Cinterface) -> ("Interface", "implement")
+                | (Cinterface, Ctrait) -> ("Trait", "implement")
+                | (_, Ctrait) -> ("Trait", "use")
+                | (_, Cenum) -> ("Enum", "use")
+                | (_, Cenum_class _) -> ("Enum Class", "extend"))
             in
-            if hard_error then
-              Typing_error_utils.add_typing_error
-                ~env
-                Typing_error.(
-                  primary
-                  @@ Primary.Sealed_not_subtype
-                       {
-                         pos = parent_pos;
-                         child_pos;
-                         name = parent_name;
-                         child_name;
-                         child_kind = class_kind;
-                       })
-            else
-              Lint.sealed_not_subtype
-                verb
-                parent_pos
-                parent_name
-                child_name
-                child_kind)
+            Typing_warning_utils.add
+              env
+              ( parent_pos,
+                Typing_warning.Sealed_not_subtype,
+                {
+                  Typing_warning.Sealed_not_subtype.verb;
+                  parent_name;
+                  child_name;
+                  child_kind;
+                } ))
       (* unit below is fine because error cases are handled as Parsing[1002] *)
       | _ -> ()
     in
@@ -1644,12 +1637,11 @@ let check_override_keyword env c tc =
   check_used_methods_with_override env c tc
 
 (** If the class is sealed, check that the elements of the whitelist
-    are descendants of the class. Error or lint depending on option
-    enforce_sealed_subclasses. *)
+    are descendants of the class and emit a warning if not.
+    *)
 let check_sealed env c =
-  let hard_error = TCO.enforce_sealed_subclasses (Env.get_tcopt env) in
   let is_enum = is_enum_or_enum_class c.c_kind in
-  sealed_subtype c ~is_enum ~hard_error ~env
+  sealed_subtype c ~is_enum ~env
 
 let check_class_require_non_strict_constraints env c tc =
   let req_non_strict_constraints =

hphp/hack/src/typing/typing_error.ml

@@ -1189,13 +1189,6 @@ module Primary = struct
         parent_kind: [ `intf | `trait | `class_ | `enum | `enum_class ];
         verb: [ `extend | `implement | `use ];
       }
-    | Sealed_not_subtype of {
-        pos: Pos.t;
-        name: string;
-        child_kind: Ast_defs.classish_kind;
-        child_pos: Pos_or_decl.t;
-        child_name: string;
-      }
     | Trait_prop_const_class of {
         pos: Pos.t;
         name: string;