From 7a50223b5164322e46a3feee029a8c688c7c3452 Mon Sep 17 00:00:00 2001
From: Francesco Zappa Nardelli <fzn@meta.com>
Date: Tue, 29 Oct 2024 16:36:41 -0700
Subject: [PATCH] raise notice if Reflection::getConstants is used on a
 enum/enum class that imports constants via enum_inclusion

Summary:
Goal: get data on how often `ReflectionClass::getConstants` is used on enums or enum classes that use/extend other enums.

This diffs updates `getConstants` to `raise_notice` whenever this happens.

Reviewed By: viratyosin

Differential Revision: D65018282

fbshipit-source-id: aeb0f3e286c6a596b3f6bc80acf4f31496dcbb8b
---
 hphp/runtime/base/strings.h                   |  2 +
 .../runtime/ext/reflection/ext_reflection.cpp | 16 ++++++--
 .../test/slow/enum_supertyping/reflection.php | 41 +++++++++++++++++++
 .../enum_supertyping/reflection.php.expectf   | 20 +++++++++
 4 files changed, 76 insertions(+), 3 deletions(-)
 create mode 100644 hphp/test/slow/enum_supertyping/reflection.php
 create mode 100644 hphp/test/slow/enum_supertyping/reflection.php.expectf

diff --git a/hphp/runtime/base/strings.h b/hphp/runtime/base/strings.h
index 63976a99d8d11..5756134016a46 100644
--- a/hphp/runtime/base/strings.h
+++ b/hphp/runtime/base/strings.h
@@ -140,6 +140,8 @@ constexpr char MISSING_DYNAMICALLY_REFERENCED[] =
   "Missing __DynamicallyReferenced attribute on class %s for classname_to_class";
 constexpr char CLASSNAME_TO_CLASS_NOEXIST_EXCEPTION[] =
   "Failed to load class from %s %s for classname_to_class.";
+constexpr char REFLECTION_MISS_CONSTANTS_FROM_INCLUDED_ENUMS[] =
+  "ReflectionClass::getConstants() misses constants from the enums/enum classes included by %s";
 
 } // namespace Strings
 } // namespace HPHP
diff --git a/hphp/runtime/ext/reflection/ext_reflection.cpp b/hphp/runtime/ext/reflection/ext_reflection.cpp
index 3416627a72074..3d4cb503fefeb 100644
--- a/hphp/runtime/ext/reflection/ext_reflection.cpp
+++ b/hphp/runtime/ext/reflection/ext_reflection.cpp
@@ -1628,12 +1628,22 @@ void addClassConstantNames(const Class* cls,
 
   auto numConsts = cls->numConstants();
   const Class::Const* consts = cls->constants();
+  bool has_constants_from_included_enums = false;
   for (size_t i = 0; i < numConsts; i++) {
-    if (consts[i].cls == cls && !consts[i].isAbstractAndUninit()
-        && consts[i].kind() == ConstModifiers::Kind::Value) {
-      st->add(const_cast<StringData*>(consts[i].name.get()));
+    if (!consts[i].isAbstractAndUninit() && 
+        consts[i].kind() == ConstModifiers::Kind::Value) {
+      if (consts[i].cls == cls) { 
+        st->add(const_cast<StringData*>(consts[i].name.get()));
+      } else if (cls->hasIncludedEnums()
+                 && cls->allIncludedEnums().contains(consts[i].cls->name())) {
+        has_constants_from_included_enums = true;   
+      }
     }
   }
+  if (has_constants_from_included_enums) {
+    raise_notice(Strings::REFLECTION_MISS_CONSTANTS_FROM_INCLUDED_ENUMS, 
+                 cls->name()->data());
+  }
 
   auto const& allTraits = cls->usedTraitClasses();
   auto const numTraits = allTraits.size();
diff --git a/hphp/test/slow/enum_supertyping/reflection.php b/hphp/test/slow/enum_supertyping/reflection.php
new file mode 100644
index 0000000000000..37d6ea9341471
--- /dev/null
+++ b/hphp/test/slow/enum_supertyping/reflection.php
@@ -0,0 +1,41 @@
+<?hh
+
+enum Enum1: string as string {
+  A = 'A';
+}
+
+enum Enum2: string as string {
+  B = 'B';
+}
+
+enum Enum3: string as string {
+  C = 'C';
+}
+
+enum CombinedEnums: string as string {
+  use Enum1, Enum2, Enum3;
+  D = 'D';
+}
+
+
+enum class Base: int {
+  int BA = 1;
+}
+
+enum class Extd : int extends Base {
+  int BB = 2;
+}
+
+<<__EntryPoint>>
+function main(): void {
+  $reflector = new ReflectionClass(Enum1::class);
+  var_dump($reflector->getConstants());
+  $reflector = new ReflectionClass(CombinedEnums::class);
+  var_dump($reflector->getConstants());
+
+  $reflector = new ReflectionClass(Base::class);
+  var_dump($reflector->getConstants());
+
+  $reflector = new ReflectionClass(Extd::class);
+  var_dump($reflector->getConstants());
+}
diff --git a/hphp/test/slow/enum_supertyping/reflection.php.expectf b/hphp/test/slow/enum_supertyping/reflection.php.expectf
new file mode 100644
index 0000000000000..bd1b76fef93fe
--- /dev/null
+++ b/hphp/test/slow/enum_supertyping/reflection.php.expectf
@@ -0,0 +1,20 @@
+dict(1) {
+  ["A"]=>
+  string(1) "A"
+}
+
+Notice: ReflectionClass::getConstants() misses constants from the enums/enum classes included by CombinedEnums in %s/enum_supertyping/reflection.php on line 34
+dict(1) {
+  ["D"]=>
+  string(1) "D"
+}
+dict(1) {
+  ["BA"]=>
+  int(1)
+}
+
+Notice: ReflectionClass::getConstants() misses constants from the enums/enum classes included by Extd in %s/reflection.php on line 40
+dict(1) {
+  ["BB"]=>
+  int(2)
+}