disable runtime enforcement of require class constraints for mock classes

Francesco Zappa Nardelli · facebook-github-bot · commit fa23fd50c869 · 2022-10-21T11:09:10.000-07:00
Summary:
A trait with a `require class C` constraints can only be used by class `C`, which must be final.  This is enforced by HHVM when loading the class that uses the trait with the constraint.

HHVM enforcement is however too strict in presence of mock classes (see test mock.php in this diff).  This diff disables HHVM enforcement for classes with the `&lt;&lt;__MockClass&gt;&gt;` attribute.

Additionally, this diff improves the error message printed when HHVM enforcement of `require class` constraints detects an error.

Reviewed By: ricklavoie

Differential Revision: D40547447

fbshipit-source-id: 56acc7e2c0a8abdb52b5d465b17b7f8b392bf118
diff --git a/hphp/runtime/vm/class.cpp b/hphp/runtime/vm/class.cpp
@@ -4393,7 +4393,10 @@ void Class::raiseUnsatisfiedRequirement(const PreClass::ClassRequirement* req)
       assertx(m_preClass->name() != reqName || !(m_preClass->attrs() & AttrFinal));
       for (auto const& traitCls : m_extra->m_usedTraits) {
         if (traitCls->allRequirements().contains(reqName)) {
-          raise_error("Trait '%s' may only be used from class '%s', which must be final",
+          raise_error("%s class '%s' uses trait '%s', but trait '%s' may only be used from class '%s', which must be final",
+                      m_preClass->attrs() & AttrFinal ? "Final" : "Non final",
+                      m_preClass->name()->data(),
+                      traitCls->preClass()->name()->data(),
                       traitCls->preClass()->name()->data(),
                       reqName->data());
         }
@@ -4403,7 +4406,9 @@ void Class::raiseUnsatisfiedRequirement(const PreClass::ClassRequirement* req)
         // A result of trait flattening, analogous to the RequirementImplements case above
         assertx(!m_extra || m_extra->m_usedTraits.size() == 0);
         assertx(m_preClass->requirements().size() > 0);
-        raise_error("Trait '<<flattened>>' may only be used from class '%s', which must be final",
+        raise_error("%s class '%s' uses trait '<<flattened>>', but trait '<<flattened>>' may only be used from class '%s', which must be final",
+                    m_preClass->attrs() & AttrFinal ? "Final" : "Non final",
+                    m_preClass->name()->data(),
                     reqName->data());
       }
       break;
@@ -4469,7 +4474,10 @@ void Class::checkRequirementConstraints() const {
         break;
       }
       case PreClass::RequirementClass: {
-        if (m_preClass->name() != reqName || !(m_preClass->attrs() & AttrFinal))  {
+        // remark: `require class` enforcement is disabled for mock classes
+        if ((m_preClass->name() != reqName || !(m_preClass->attrs() & AttrFinal)) &&
+            (m_preClass->userAttributes().find(s___MockClass.get()) ==
+             m_preClass->userAttributes().end())) {
           raiseUnsatisfiedRequirement(req);
         }
         break;
diff --git a/hphp/test/slow/traits/requireclass/mock.php b/hphp/test/slow/traits/requireclass/mock.php
@@ -0,0 +1,21 @@
+<?hh
+// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary.
+
+<<file:__EnableUnstableFeatures('require_class')>>
+
+trait T {
+  require class C;
+}
+
+final class C {
+  use T;
+}
+
+<<__MockClass>>
+final class MockC extends C {}
+
+<<__EntryPoint>>
+function main() : void {
+  new Mockc();
+  echo "done.\n";
+}
diff --git a/hphp/test/slow/traits/requireclass/mock.php.expectf b/hphp/test/slow/traits/requireclass/mock.php.expectf
@@ -0,0 +1 @@
+done.
diff --git a/hphp/test/slow/traits/requireclass/requireclass_abstract_01.bad.php.expectf b/hphp/test/slow/traits/requireclass/requireclass_abstract_01.bad.php.expectf
@@ -1 +1 @@
-Fatal error: Trait 'T' may only be used from class 'C', which must be final in %s/requireclass_abstract_01.bad.php on line 6
+Fatal error: Non final class 'C' uses trait 'T', but trait 'T' may only be used from class 'C', which must be final in %s/requireclass_abstract_01.bad.php on line 6
diff --git a/hphp/test/slow/traits/requireclass/requireclass_abstract_01.bad.php.expectf-repo b/hphp/test/slow/traits/requireclass/requireclass_abstract_01.bad.php.expectf-repo
@@ -1 +1 @@
-Fatal error: Trait '<<flattened>>' may only be used from class 'C', which must be final in %s/requireclass_abstract_01.bad.php on line 6
+Fatal error: Non final class 'C' uses trait '<<flattened>>', but trait '<<flattened>>' may only be used from class 'C', which must be final in %s/requireclass_abstract_01.bad.php on line 6
diff --git a/hphp/test/slow/traits/requireclass/requireclass_hierarchy_02.bad.php.expectf b/hphp/test/slow/traits/requireclass/requireclass_hierarchy_02.bad.php.expectf
@@ -1 +1 @@
-Fatal error: Trait 'T' may only be used from class 'C', which must be final in %s/requireclass_hierarchy_02.bad.php on line 12
+Fatal error: Non final class 'D' uses trait 'T', but trait 'T' may only be used from class 'C', which must be final in %s/requireclass_hierarchy_02.bad.php on line 12
diff --git a/hphp/test/slow/traits/requireclass/requireclass_hierarchy_02.bad.php.expectf-repo b/hphp/test/slow/traits/requireclass/requireclass_hierarchy_02.bad.php.expectf-repo
@@ -1 +1 @@
-Fatal error: Trait '<<flattened>>' may only be used from class 'C', which must be final in %s/requireclass_hierarchy_02.bad.php on line 12
+Fatal error: Non final class 'D' uses trait '<<flattened>>', but trait '<<flattened>>' may only be used from class 'C', which must be final in %s/requireclass_hierarchy_02.bad.php on line 12
diff --git a/hphp/test/slow/traits/requireclass/requireclass_hierarchy_03.bad.php.expectf b/hphp/test/slow/traits/requireclass/requireclass_hierarchy_03.bad.php.expectf
@@ -1 +1 @@
-Fatal error: Trait 'T' may only be used from class 'C', which must be final in %s/requireclass_hierarchy_03.bad.php on line 12
+Fatal error: Non final class 'D' uses trait 'T', but trait 'T' may only be used from class 'C', which must be final in %s/requireclass_hierarchy_03.bad.php on line 12
diff --git a/hphp/test/slow/traits/requireclass/requireclass_hierarchy_03.bad.php.expectf-repo b/hphp/test/slow/traits/requireclass/requireclass_hierarchy_03.bad.php.expectf-repo
@@ -1 +1 @@
-Fatal error: Trait '<<flattened>>' may only be used from class 'C', which must be final in %s/requireclass_hierarchy_03.bad.php on line 12
+Fatal error: Non final class 'D' uses trait '<<flattened>>', but trait '<<flattened>>' may only be used from class 'C', which must be final in %s/requireclass_hierarchy_03.bad.php on line 12
diff --git a/hphp/test/slow/traits/requireclass/requireclass_hierarchy_04.good.php b/hphp/test/slow/traits/requireclass/requireclass_hierarchy_04.good.php
@@ -0,0 +1,22 @@
+<?hh
+// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+
+<<file:__EnableUnstableFeatures('require_class')>>
+
+trait T1 {
+  require class C;
+}
+
+trait T2 {
+  use T1;
+}
+
+final class C {
+  use T2;
+}
+
+<<__EntryPoint>>
+function main(): void {
+  new C();
+  echo "done\n";
+}
diff --git a/hphp/test/slow/traits/requireclass/requireclass_hierarchy_04.good.php.expectf b/hphp/test/slow/traits/requireclass/requireclass_hierarchy_04.good.php.expectf
@@ -0,0 +1 @@
+done
diff --git a/hphp/test/slow/traits/requireclass/requireclass_nonfinal_01.bad.php.expectf b/hphp/test/slow/traits/requireclass/requireclass_nonfinal_01.bad.php.expectf
@@ -1 +1 @@
-Fatal error: Trait 'T' may only be used from class 'C', which must be final in %s/requireclass_nonfinal_01.bad.php on line 6
+Fatal error: Non final class 'C' uses trait 'T', but trait 'T' may only be used from class 'C', which must be final in %s/requireclass_nonfinal_01.bad.php on line 6
diff --git a/hphp/test/slow/traits/requireclass/requireclass_nonfinal_01.bad.php.expectf-repo b/hphp/test/slow/traits/requireclass/requireclass_nonfinal_01.bad.php.expectf-repo
@@ -1 +1 @@
-Fatal error: Trait '<<flattened>>' may only be used from class 'C', which must be final in %s/requireclass/requireclass_nonfinal_01.bad.php on line 6
+Fatal error: Non final class 'C' uses trait '<<flattened>>', but trait '<<flattened>>' may only be used from class 'C', which must be final in %s/requireclass_nonfinal_01.bad.php on line 6
