Skip to content

Commit fa23fd5

Browse files
Francesco Zappa Nardellifacebook-github-bot
Francesco Zappa Nardelli
authored andcommitted
disable runtime enforcement of require class constraints for mock classes
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 `<<__MockClass>>` 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
1 parent 9e8c173 commit fa23fd5

13 files changed

+64
-11
lines changed

Diff for: hphp/runtime/vm/class.cpp

+11-3
Original file line numberDiff line numberDiff line change
@@ -4393,7 +4393,10 @@ void Class::raiseUnsatisfiedRequirement(const PreClass::ClassRequirement* req)
43934393
assertx(m_preClass->name() != reqName || !(m_preClass->attrs() & AttrFinal));
43944394
for (auto const& traitCls : m_extra->m_usedTraits) {
43954395
if (traitCls->allRequirements().contains(reqName)) {
4396-
raise_error("Trait '%s' may only be used from class '%s', which must be final",
4396+
raise_error("%s class '%s' uses trait '%s', but trait '%s' may only be used from class '%s', which must be final",
4397+
m_preClass->attrs() & AttrFinal ? "Final" : "Non final",
4398+
m_preClass->name()->data(),
4399+
traitCls->preClass()->name()->data(),
43974400
traitCls->preClass()->name()->data(),
43984401
reqName->data());
43994402
}
@@ -4403,7 +4406,9 @@ void Class::raiseUnsatisfiedRequirement(const PreClass::ClassRequirement* req)
44034406
// A result of trait flattening, analogous to the RequirementImplements case above
44044407
assertx(!m_extra || m_extra->m_usedTraits.size() == 0);
44054408
assertx(m_preClass->requirements().size() > 0);
4406-
raise_error("Trait '<<flattened>>' may only be used from class '%s', which must be final",
4409+
raise_error("%s class '%s' uses trait '<<flattened>>', but trait '<<flattened>>' may only be used from class '%s', which must be final",
4410+
m_preClass->attrs() & AttrFinal ? "Final" : "Non final",
4411+
m_preClass->name()->data(),
44074412
reqName->data());
44084413
}
44094414
break;
@@ -4469,7 +4474,10 @@ void Class::checkRequirementConstraints() const {
44694474
break;
44704475
}
44714476
case PreClass::RequirementClass: {
4472-
if (m_preClass->name() != reqName || !(m_preClass->attrs() & AttrFinal)) {
4477+
// remark: `require class` enforcement is disabled for mock classes
4478+
if ((m_preClass->name() != reqName || !(m_preClass->attrs() & AttrFinal)) &&
4479+
(m_preClass->userAttributes().find(s___MockClass.get()) ==
4480+
m_preClass->userAttributes().end())) {
44734481
raiseUnsatisfiedRequirement(req);
44744482
}
44754483
break;

Diff for: hphp/test/slow/traits/requireclass/mock.php

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?hh
2+
// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary.
3+
4+
<<file:__EnableUnstableFeatures('require_class')>>
5+
6+
trait T {
7+
require class C;
8+
}
9+
10+
final class C {
11+
use T;
12+
}
13+
14+
<<__MockClass>>
15+
final class MockC extends C {}
16+
17+
<<__EntryPoint>>
18+
function main() : void {
19+
new Mockc();
20+
echo "done.\n";
21+
}

Diff for: hphp/test/slow/traits/requireclass/mock.php.expectf

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
done.
Original file line numberDiff line numberDiff line change
@@ -1 +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
1+
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
Original file line numberDiff line numberDiff line change
@@ -1 +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
1+
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
Original file line numberDiff line numberDiff line change
@@ -1 +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
1+
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
Original file line numberDiff line numberDiff line change
@@ -1 +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
1+
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
Original file line numberDiff line numberDiff line change
@@ -1 +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
1+
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
Original file line numberDiff line numberDiff line change
@@ -1 +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
1+
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
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?hh
2+
// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
3+
4+
<<file:__EnableUnstableFeatures('require_class')>>
5+
6+
trait T1 {
7+
require class C;
8+
}
9+
10+
trait T2 {
11+
use T1;
12+
}
13+
14+
final class C {
15+
use T2;
16+
}
17+
18+
<<__EntryPoint>>
19+
function main(): void {
20+
new C();
21+
echo "done\n";
22+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
done
Original file line numberDiff line numberDiff line change
@@ -1 +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
1+
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
Original file line numberDiff line numberDiff line change
@@ -1 +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
1+
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

0 commit comments

Comments
 (0)