From e9ca03d856ead02eb9845a3d604029846effa5ae Mon Sep 17 00:00:00 2001
From: Max Heiber <mheiber@fb.com>
Date: Fri, 28 Aug 2020 03:11:46 -0700
Subject: [PATCH] Ban instance properties on abstract final classes

Summary:
Ban instance properties on abstract final classes such as:

```
abstract final class Foo {
  public int $foo;
}
```

But *allow* XHP attributes:

```
abstract final class :foo {
  attribute string foobar;
}
class :bar {
  attribute :foo;
}
```

Reviewed By: Wilfred

Differential Revision: D23323178

fbshipit-source-id: 6399b6c511fecd596bd8285c10eeb376dd350ddd
---
 hphp/hack/src/errors/error_codes.ml                               | 3 ++-
 hphp/hack/src/errors/errors.ml                                    | 6 ++++++
 hphp/hack/src/errors/errors.mli                                   | 2 ++
 hphp/hack/src/typing/tast_check/abstract_class_check.ml           | 8 +++++++-
 hphp/hack/test/errors/error_map.ml                                | 1 +
 hphp/hack/test/typecheck/abstract_final_instance_property.php     | 4 ++++
 hphp/hack/test/typecheck/abstract_final_instance_property.php.exp | 2 ++
 hphp/hack/test/typecheck/abstract_final_static_property.exp       | 2 ++
 hphp/hack/test/typecheck/abstract_final_static_property.php       | 4 ++++
 hphp/hack/test/typecheck/abstract_final_static_property.php.exp   | 1 +
 hphp/hack/test/typecheck/abstract_final_xhp_property.php          | 7 +++++++
 hphp/hack/test/typecheck/abstract_final_xhp_property.php.exp      | 1 +
 12 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 hphp/hack/test/typecheck/abstract_final_instance_property.php
 create mode 100644 hphp/hack/test/typecheck/abstract_final_instance_property.php.exp
 create mode 100644 hphp/hack/test/typecheck/abstract_final_static_property.exp
 create mode 100644 hphp/hack/test/typecheck/abstract_final_static_property.php
 create mode 100644 hphp/hack/test/typecheck/abstract_final_static_property.php.exp
 create mode 100644 hphp/hack/test/typecheck/abstract_final_xhp_property.php
 create mode 100644 hphp/hack/test/typecheck/abstract_final_xhp_property.php.exp

diff --git a/hphp/hack/src/errors/error_codes.ml b/hphp/hack/src/errors/error_codes.ml
index 85a0853cc7c..7ead1a37d2d 100644
--- a/hphp/hack/src/errors/error_codes.ml
+++ b/hphp/hack/src/errors/error_codes.ml
@@ -260,7 +260,8 @@ module NastCheck = struct
     | PhpLambdaDisallowed
     | EntryPointArguments
     | VariadicMemoize
-    | AbstractMethodMemoize (* EXTEND HERE WITH NEW VALUES IF NEEDED *)
+    | AbstractMethodMemoize
+    | InstancePropertyInAbstractFinalClass (* EXTEND HERE WITH NEW VALUES IF NEEDED *)
   [@@deriving enum, show { with_path = false }]
 
   let err_code = to_enum
diff --git a/hphp/hack/src/errors/errors.ml b/hphp/hack/src/errors/errors.ml
index ce05144d96e..7dafbb3c899 100644
--- a/hphp/hack/src/errors/errors.ml
+++ b/hphp/hack/src/errors/errors.ml
@@ -2116,6 +2116,12 @@ let abstract_method_memoize pos =
     pos
     "Abstract methods cannot be memoized."
 
+let instance_property_in_abstract_final_class pos =
+  add
+    (NastCheck.err_code NastCheck.InstancePropertyInAbstractFinalClass)
+    pos
+    "Abstract final classes cannot have instance properties."
+
 let inout_params_special pos =
   add
     (NastCheck.err_code NastCheck.InoutParamsSpecial)
diff --git a/hphp/hack/src/errors/errors.mli b/hphp/hack/src/errors/errors.mli
index 7c997a6e251..4d6b1d0d357 100644
--- a/hphp/hack/src/errors/errors.mli
+++ b/hphp/hack/src/errors/errors.mli
@@ -1272,6 +1272,8 @@ val variadic_memoize : Pos.t -> unit
 
 val abstract_method_memoize : Pos.t -> unit
 
+val instance_property_in_abstract_final_class : Pos.t -> unit
+
 val reassign_maybe_mutable_var : in_collection:bool -> Pos.t -> unit
 
 val immutable_argument_mismatch : Pos.t -> Pos.t -> unit
diff --git a/hphp/hack/src/typing/tast_check/abstract_class_check.ml b/hphp/hack/src/typing/tast_check/abstract_class_check.ml
index d206166ae9d..0feb4c02673 100644
--- a/hphp/hack/src/typing/tast_check/abstract_class_check.ml
+++ b/hphp/hack/src/typing/tast_check/abstract_class_check.ml
@@ -45,7 +45,13 @@ let check_class _ c =
     in
     let (c_constructor, _, c_methods) = split_methods c in
     List.iter c_methods err;
-    Option.iter c_constructor err
+    Option.iter c_constructor err;
+
+    let (_, c_instance_vars) = split_vars c in
+    c_instance_vars
+    |> List.filter ~f:(fun var -> Option.is_none var.cv_xhp_attr)
+    |> List.iter ~f:(fun var ->
+           Errors.instance_property_in_abstract_final_class (fst var.cv_id))
   )
 
 let handler =
diff --git a/hphp/hack/test/errors/error_map.ml b/hphp/hack/test/errors/error_map.ml
index 5243514d32f..32be5231267 100644
--- a/hphp/hack/test/errors/error_map.ml
+++ b/hphp/hack/test/errors/error_map.ml
@@ -257,6 +257,7 @@ PhpLambdaDisallowed = 3084
 EntryPointArguments = 3085
 VariadicMemoize = 3086
 AbstractMethodMemoize = 3087
+InstancePropertyInAbstractFinalClass = 3088
 
 Typing Errors:
 AbstractClassFinalDEPRECATED = 4001
diff --git a/hphp/hack/test/typecheck/abstract_final_instance_property.php b/hphp/hack/test/typecheck/abstract_final_instance_property.php
new file mode 100644
index 00000000000..6f99591381f
--- /dev/null
+++ b/hphp/hack/test/typecheck/abstract_final_instance_property.php
@@ -0,0 +1,4 @@
+<?hh
+abstract final class Foo {
+  public int $foobar;
+}
diff --git a/hphp/hack/test/typecheck/abstract_final_instance_property.php.exp b/hphp/hack/test/typecheck/abstract_final_instance_property.php.exp
new file mode 100644
index 00000000000..c0282e922c8
--- /dev/null
+++ b/hphp/hack/test/typecheck/abstract_final_instance_property.php.exp
@@ -0,0 +1,2 @@
+File "abstract_final_instance_property.php", line 3, characters 14-20:
+Abstract final classes cannot have instance properties. (NastCheck[3088])
diff --git a/hphp/hack/test/typecheck/abstract_final_static_property.exp b/hphp/hack/test/typecheck/abstract_final_static_property.exp
new file mode 100644
index 00000000000..deb02e652f6
--- /dev/null
+++ b/hphp/hack/test/typecheck/abstract_final_static_property.exp
@@ -0,0 +1,2 @@
+File "abstract_final_noninstance_property.php", line 3, characters 14-20:
+Abstract final classes cannot have nonstatic methods or properties. (NastCheck[3074])
diff --git a/hphp/hack/test/typecheck/abstract_final_static_property.php b/hphp/hack/test/typecheck/abstract_final_static_property.php
new file mode 100644
index 00000000000..87cb8106a0c
--- /dev/null
+++ b/hphp/hack/test/typecheck/abstract_final_static_property.php
@@ -0,0 +1,4 @@
+<?hh
+abstract final class Foo {
+  public static int $foobar = 3;
+}
diff --git a/hphp/hack/test/typecheck/abstract_final_static_property.php.exp b/hphp/hack/test/typecheck/abstract_final_static_property.php.exp
new file mode 100644
index 00000000000..4269126fceb
--- /dev/null
+++ b/hphp/hack/test/typecheck/abstract_final_static_property.php.exp
@@ -0,0 +1 @@
+No errors
diff --git a/hphp/hack/test/typecheck/abstract_final_xhp_property.php b/hphp/hack/test/typecheck/abstract_final_xhp_property.php
new file mode 100644
index 00000000000..85da7cdc420
--- /dev/null
+++ b/hphp/hack/test/typecheck/abstract_final_xhp_property.php
@@ -0,0 +1,7 @@
+<?hh // partial
+abstract final class :foo {
+  attribute string foobar;
+}
+class :bar {
+  attribute :foo;
+}
diff --git a/hphp/hack/test/typecheck/abstract_final_xhp_property.php.exp b/hphp/hack/test/typecheck/abstract_final_xhp_property.php.exp
new file mode 100644
index 00000000000..4269126fceb
--- /dev/null
+++ b/hphp/hack/test/typecheck/abstract_final_xhp_property.php.exp
@@ -0,0 +1 @@
+No errors
-- 
2.11.4.GIT