Permalink
Browse files

Fix this typehints for mocking.

Summary:
Mock objects used in WWW testing caused issues in multiple places.
This diff should address those issues and allow us to reenable this typehint
checking in dev, and sandcastle.

Reviewed By: markw65

Differential Revision: D6705918

fbshipit-source-id: d6c332a8c8a5813108ca97dbec3d15aebd7b0e8d
  • Loading branch information...
mofarrell authored and hhvm-bot committed Jan 16, 2018
1 parent 64a7311 commit 66a9dcaa276c4f3875537f02650864318b039612
@@ -471,6 +471,14 @@ struct ClassInfo {
*/
std::unordered_map<SString,PublicSPropEntry> publicStaticProps;
/*
* Flags to track if this class is mocked, or if any of its dervied classes
* are mocked.
*/
bool isMocked{false};
bool isDerivedMocked{false};
/*
* Flags about the existence of various magic methods, or whether
* any derived classes may have those methods. The non-derived
@@ -617,6 +625,24 @@ bool Class::couldHaveMagicBool() const {
);
}
bool Class::couldHaveMockedDerivedClass() const {
return val.match(
[] (SString) { return true;},
[] (borrowed_ptr<ClassInfo> cinfo) {
return cinfo->isDerivedMocked;
}
);
}
bool Class::couldBeMocked() const {
return val.match(
[] (SString) { return true;},
[] (borrowed_ptr<ClassInfo> cinfo) {
return cinfo->isMocked;
}
);
}
folly::Optional<Class> Class::commonAncestor(const Class& o) const {
if (val.left() || o.val.left()) return folly::none;
auto const c1 = val.right();
@@ -2189,6 +2215,17 @@ void find_magic_methods(IndexData& index) {
}
}
void find_mocked_classes(IndexData& index) {
for (auto& cinfo : index.allClassInfos) {
if (is_mock_class(cinfo->cls) && cinfo->parent) {
cinfo->parent->isMocked = true;
for (auto c = cinfo->parent; c; c = c->parent) {
c->isDerivedMocked = true;
}
}
}
}
void mark_no_override_classes(IndexData& index) {
for (auto& cinfo : index.allClassInfos) {
auto const set = [&] {
@@ -2676,6 +2713,7 @@ Index::Index(borrowed_ptr<php::Program> program,
mark_no_override_methods(*m_data); // uses AttrUnique
define_func_families(*m_data); // uses AttrNoOverride functions
find_magic_methods(*m_data); // uses the subclass lists
find_mocked_classes(*m_data);
compute_iface_vtables(*m_data);
check_invariants(*m_data);
@@ -191,6 +191,17 @@ struct Class {
*/
bool couldHaveMagicBool() const;
/*
* Whether this class could possibly have a derived class that is mocked.
* Including itself.
*/
bool couldHaveMockedDerivedClass() const;
/*
* Whether this class could possibly be mocked.
*/
bool couldBeMocked() const;
/*
* Returns the Class that is the first common ancestor between 'this' and 'o'.
* If there is no common ancestor folly::none is returned
@@ -3589,6 +3589,19 @@ void in(ISS& env, const bc::OODeclExists& op) {
} ());
}
namespace {
bool couldBeMocked(const Type& t) {
if (is_specialized_cls(t)) {
return dcls_of(t).cls.couldBeMocked();
} else if (is_specialized_obj(t)) {
return dobj_of(t).cls.couldBeMocked();
}
// In practice this should not occur since this is used mostly on the result
// of looked up type constraints.
return true;
}
}
void in(ISS& env, const bc::VerifyParamType& op) {
if (env.ctx.func->isMemoizeImpl &&
!locCouldBeRef(env, op.loc1) &&
@@ -3628,6 +3641,9 @@ void in(ISS& env, const bc::VerifyParamType& op) {
!constraint.isTypeConstant()) {
auto t =
loosen_dvarrayness(env.index.lookup_constraint(env.ctx, constraint));
if (constraint.isThis() && couldBeMocked(t)) {
t = unctx(std::move(t));
}
if (t.subtypeOf(TBottom)) unreachable(env);
FTRACE(2, " {} ({})\n", constraint.fullName(), show(t));
setLoc(env, op.loc1, std::move(t));
@@ -851,6 +851,13 @@ void do_optimize(const Index& index, FuncAnalysis&& ainfo, bool isFinal) {
}
auto fixTypeConstraint = [&] (TypeConstraint& tc, const Type& candidate) {
auto t = index.lookup_constraint(ainfo.ctx, tc, candidate);
if (is_specialized_obj(t) &&
!dobj_of(t).cls.couldHaveMockedDerivedClass()) {
tc.setNoMockObjects();
}
if (!tc.hasConstraint() ||
tc.isSoft() ||
tc.isTypeVar() ||
@@ -860,7 +867,6 @@ void do_optimize(const Index& index, FuncAnalysis&& ainfo, bool isFinal) {
return;
}
auto t = index.lookup_constraint(ainfo.ctx, tc, candidate);
auto const nullable = is_opt(t);
if (nullable) t = unopt(std::move(t));
@@ -2368,8 +2368,13 @@ Type return_with_context(Type t, Type context) {
if (((is_specialized_obj(t) && t.m_data.dobj.isCtx) ||
(is_specialized_cls(t) && t.m_data.dcls.isCtx)) &&
context.subtypeOfAny(TCls, TObj) && context != TBottom) {
context = toobj(context);
if (is_specialized_obj(context) && dobj_of(context).type == DObj::Exact &&
dobj_of(context).cls.couldBeMocked()) {
context = subObj(dobj_of(context).cls);
}
bool o = is_opt(t);
t = intersection_of(unctx(t), toobj(context));
t = intersection_of(unctx(std::move(t)), context);
// We must preserve optional typing, as this is not included in the
// context type.
return (o && canBeOptional(t.m_bits)) ? opt(t) : t;
@@ -58,6 +58,7 @@ const StaticString s_86pinit("86pinit");
const StaticString s_86sinit("86sinit");
const StaticString s___destruct("__destruct");
const StaticString s___OptionalDestruct("__OptionalDestruct");
const StaticString s___MockClass("__MockClass");
Mutex g_classesMutex;
@@ -1331,7 +1332,6 @@ void Class::setInstanceBitsImpl() {
//
// These are mostly for the class creation path.
static const StaticString s___MockClass("__MockClass");
void Class::setParent() {
// Cache m_preClass->attrs()
m_attrCopy = m_preClass->attrs();
@@ -48,6 +48,8 @@
namespace HPHP {
///////////////////////////////////////////////////////////////////////////////
extern const StaticString s___MockClass;
struct Class;
struct ClassInfo;
struct EnumValues;
@@ -179,10 +179,29 @@ void verifyTypeImpl(IRGS& env, int32_t const id, bool isReturnType,
curUnit(env)->isHHFile() ||
!RuntimeOption::PHP7_ScalarTypes;
auto const thisFailsHard = [&] {
switch (RuntimeOption::EvalThisTypeHintLevel) {
case 0:
// We are not checking this typehints.
case 2:
// We are warning on this typehint failures.
return false;
case 1:
// We are checking this typehints like self typehints.
return true;
case 3:
// If we know there are no mock classes for the current class, it is
// okay to fail hard. Otherwise, mock objects may still pass, and we
// have to be ready for execution to resume.
return !tc.couldSeeMockObject();
}
always_assert(false);
};
auto const failHard = strictTypes
&& RuntimeOption::RepoAuthoritative
&& !tc.isSoft()
&& !(tc.isThis() && RuntimeOption::EvalThisTypeHintLevel == 2)
&& (!tc.isThis() || thisFailsHard())
// If we're warning on d/varray mismatches, any array type-hint will
// always fail, so regardless of other settings, we can't assume its a
// hard failure.
@@ -519,7 +519,7 @@ void TypeConstraint::verifyParamFail(const Func* func, TypedValue* tv,
int paramNum, bool useStrictTypes) const {
verifyFail(func, tv, paramNum, useStrictTypes);
assertx(
isSoft() || !RuntimeOption::EvalHardTypeHints ||
isSoft() || !RuntimeOption::EvalHardTypeHints || (isThis() && couldSeeMockObject()) ||
(RuntimeOption::EvalHackArrCompatTypeHintNotices &&
isArrayType(tv->m_type)) ||
check(tv, func)
@@ -636,6 +636,16 @@ void TypeConstraint::verifyFail(const Func* func, TypedValue* tv,
}();
if (done) return;
if (UNLIKELY(isThis() && c->m_type == KindOfObject)) {
Class* cls = c->m_data.pobj->getVMClass();
const Class* thisClass = nullptr;
thisToClass(&thisClass);
if (cls->preClass()->userAttributes().count(s___MockClass.get()) &&
cls->parent() == thisClass) {
return;
}
}
if (UNLIKELY(!useStrictTypes)) {
if (auto dt = underlyingDataType()) {
// In non-strict mode we may be able to coerce a type failure. For object
@@ -93,7 +93,13 @@ struct TypeConstraint {
* and the actual type is in m_type. When set, Object is guaranteed
* to be an object, not a type-alias.
*/
Resolved = 0x40
Resolved = 0x40,
/*
* Indicates that no mock object can satisfy this constraint. This is
* resolved by HHBBC.
*/
NoMockObjects = 0x80
};
/*
@@ -149,6 +155,11 @@ struct TypeConstraint {
m_type = t;
}
void setNoMockObjects() {
auto flags = m_flags | Flags::NoMockObjects;
m_flags = static_cast<Flags>(flags);
}
/*
* Returns: whether this constraint implies any runtime checking at
* all. If this function returns false, it means the
@@ -197,6 +208,7 @@ struct TypeConstraint {
bool isTypeVar() const { return m_flags & TypeVar; }
bool isTypeConstant() const { return m_flags & TypeConstant; }
bool isResolved() const { return m_flags & Resolved; }
bool couldSeeMockObject() const { return !(m_flags & NoMockObjects); }
bool isPrecise() const { return metaType() == MetaType::Precise; }
bool isMixed() const { return m_type == Type::Mixed; }
@@ -0,0 +1,21 @@
<?hh
final class A {
static public $a;
static public function setSingleton(this $a): void {
static::$a = $a;
}
static public function getMeAnA(): this {
return static::$a;
}
}
<<__MockClass>>
final class MockA extends A {
}
$m = new MockA();
A::setSingleton($m);
var_dump(A::getMeAnA());
@@ -0,0 +1,2 @@
object(MockA)#1 (0) {
}
@@ -0,0 +1,2 @@
-vHardReturnTypeHints=1
-vRuntime.Eval.ThisTypeHintLevel=3
@@ -0,0 +1,2 @@
-vEval.CheckReturnTypeHints=3
-vEval.ThisTypeHintLevel=3

0 comments on commit 66a9dca

Please sign in to comment.