Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Commit

Permalink
Use undefined instead of missing to represent absent field in descrip…
Browse files Browse the repository at this point in the history 
…tors

Flow doesn't support optional fields in classes. This effectively becomes enforced with Babel 7 since it treats the type annotations as field initializers. Which means that these fields always gets created.

So I fixed all the callsites that checks for `hasOwnProperty` or `in` that I could find.
  • Loading branch information
@sebmarkbage
sebmarkbage committed Aug 30, 2018
1 parent 922d40c commit 2b4546d
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 76 deletions.
43 changes: 14 additions & 29 deletions src/descriptors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ export class PropertyDescriptor extends Descriptor {

constructor(desc: DescriptorInitializer | PropertyDescriptor) {
super();
if (desc.hasOwnProperty("writable")) this.writable = desc.writable;
if (desc.hasOwnProperty("enumerable")) this.enumerable = desc.enumerable;
if (desc.hasOwnProperty("configurable")) this.configurable = desc.configurable;
if (desc.hasOwnProperty("value")) this.value = desc.value;
if (desc.hasOwnProperty("get")) this.get = desc.get;
if (desc.hasOwnProperty("set")) this.set = desc.set;
this.writable = desc.writable;
this.enumerable = desc.enumerable;
this.configurable = desc.configurable;
this.value = desc.value;
this.get = desc.get;
this.set = desc.set;
}

throwIfNotConcrete(realm: Realm): PropertyDescriptor {
Expand Down Expand Up @@ -124,28 +124,13 @@ export function cloneDescriptor(d: void | PropertyDescriptor): void | PropertyDe

// does not check if the contents of value properties are the same
export function equalDescriptors(d1: PropertyDescriptor, d2: PropertyDescriptor): boolean {
if (d1.hasOwnProperty("writable")) {
if (!d2.hasOwnProperty("writable")) return false;
if (d1.writable !== d2.writable) return false;
}
if (d1.hasOwnProperty("enumerable")) {
if (!d2.hasOwnProperty("enumerable")) return false;
if (d1.enumerable !== d2.enumerable) return false;
}
if (d1.hasOwnProperty("configurable")) {
if (!d2.hasOwnProperty("configurable")) return false;
if (d1.configurable !== d2.configurable) return false;
}
if (d1.hasOwnProperty("value")) {
if (!d2.hasOwnProperty("value")) return false;
}
if (d1.hasOwnProperty("get")) {
if (!d2.hasOwnProperty("get")) return false;
if (d1.get !== d2.get) return false;
}
if (d1.hasOwnProperty("set")) {
if (!d2.hasOwnProperty("set")) return false;
if (d1.set !== d2.set) return false;
}
if (d1.writable !== d2.writable) return false;
if (d1.enumerable !== d2.enumerable) return false;
if (d1.configurable !== d2.configurable) return false;
if (d1.value !== undefined) {
if (d2.value === undefined) return false;
}
if (d1.get !== d2.get) return false;
if (d1.set !== d2.set) return false;
return true;
}
4 changes: 2 additions & 2 deletions src/methods/is.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ function IsAccessorDescriptorInternal(realm: Realm, Desc: ?Descriptor): boolean

// 2. If both Desc.[[Get]] and Desc.[[Set]] are absent, return false.
Desc = Desc.throwIfNotConcrete(realm);
if (!("get" in Desc) && !("set" in Desc)) return false;
if (Desc.get === undefined && Desc.set === undefined) return false;

// 3. Return true.
return true;
Expand All @@ -83,7 +83,7 @@ function IsDataDescriptorInternal(realm: Realm, Desc: ?Descriptor): boolean {

// If both Desc.[[Value]] and Desc.[[Writable]] are absent, return false.
Desc = Desc.throwIfNotConcrete(realm);
if (!("value" in Desc) && !("writable" in Desc)) return false;
if (Desc.value === undefined && Desc.writable === undefined) return false;

// Return true.
return true;
Expand Down
79 changes: 42 additions & 37 deletions src/methods/properties.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ export class PropertiesImplementation {
invariant(IsAccessorDescriptor(realm, ownDesc), "expected accessor");

// 6. Let setter be ownDesc.[[Set]].
let setter = "set" in ownDesc ? ownDesc.set : undefined;
let setter = ownDesc.set;

// 7. If setter is undefined, return false.
if (!setter || setter instanceof UndefinedValue) return false;
Expand Down Expand Up @@ -662,44 +662,38 @@ export class PropertiesImplementation {

// 4. If Desc has a [[Value]] field, then
let success = true;
if ("value" in Desc) {
invariant(Desc.value instanceof Value);
if (Desc.value !== undefined) {
// a. Perform CreateDataProperty(obj, "value", Desc.[[Value]]).
success = Create.CreateDataProperty(realm, obj, "value", Desc.value) && success;
}

// 5. If Desc has a [[Writable]] field, then
if ("writable" in Desc) {
invariant(Desc.writable !== undefined);
if (Desc.writable !== undefined) {
// a. Perform CreateDataProperty(obj, "writable", Desc.[[Writable]]).
success = Create.CreateDataProperty(realm, obj, "writable", new BooleanValue(realm, Desc.writable)) && success;
}

// 6. If Desc has a [[Get]] field, then
if ("get" in Desc) {
invariant(Desc.get !== undefined);
if (Desc.get !== undefined) {
// a. Perform CreateDataProperty(obj, "get", Desc.[[Get]]).
success = Create.CreateDataProperty(realm, obj, "get", Desc.get) && success;
}

// 7. If Desc has a [[Set]] field, then
if ("set" in Desc) {
invariant(Desc.set !== undefined);
if (Desc.set !== undefined) {
// a. Perform CreateDataProperty(obj, "set", Desc.[[Set]]).
success = Create.CreateDataProperty(realm, obj, "set", Desc.set) && success;
}

// 8. If Desc has an [[Enumerable]] field, then
if ("enumerable" in Desc) {
invariant(Desc.enumerable !== undefined);
if (Desc.enumerable !== undefined) {
// a. Perform CreateDataProperty(obj, "enumerable", Desc.[[Enumerable]]).
success =
Create.CreateDataProperty(realm, obj, "enumerable", new BooleanValue(realm, Desc.enumerable)) && success;
}

// 9. If Desc has a [[Configurable]] field, then
if ("configurable" in Desc) {
invariant(Desc.configurable !== undefined);
if (Desc.configurable !== undefined) {
// a. Perform CreateDataProperty(obj, "configurable", Desc.[[Configurable]]).
success =
Create.CreateDataProperty(realm, obj, "configurable", new BooleanValue(realm, Desc.configurable)) && success;
Expand Down Expand Up @@ -806,22 +800,22 @@ export class PropertiesImplementation {
// 3. If either IsGenericDescriptor(Desc) or IsDataDescriptor(Desc) is true, then
if (IsGenericDescriptor(realm, Desc) || IsDataDescriptor(realm, Desc)) {
// a. If Desc does not have a [[Value]] field, set Desc.[[Value]] to like.[[Value]].
if (!("value" in Desc)) Desc.value = like.value;
if (Desc.value === undefined) Desc.value = like.value;
// b. If Desc does not have a [[Writable]] field, set Desc.[[Writable]] to like.[[Writable]].
if (!("writable" in Desc)) Desc.writable = like.writable;
if (Desc.writable === undefined) Desc.writable = like.writable;
} else {
// 4. Else,
// a. If Desc does not have a [[Get]] field, set Desc.[[Get]] to like.[[Get]].
if (!("get" in Desc)) Desc.get = like.get;
if (Desc.get === undefined) Desc.get = like.get;
// b. If Desc does not have a [[Set]] field, set Desc.[[Set]] to like.[[Set]].
if (!("set" in Desc)) Desc.set = like.set;
if (Desc.set === undefined) Desc.set = like.set;
}

// 5. If Desc does not have an [[Enumerable]] field, set Desc.[[Enumerable]] to like.[[Enumerable]].
if (!("enumerable" in Desc)) Desc.enumerable = like.enumerable;
if (Desc.enumerable === undefined) Desc.enumerable = like.enumerable;

// 6. If Desc does not have a [[Configurable]] field, set Desc.[[Configurable]] to like.[[Configurable]].
if (!("configurable" in Desc)) Desc.configurable = like.configurable;
if (Desc.configurable === undefined) Desc.configurable = like.configurable;

// 7. Return Desc.
return Desc;
Expand Down Expand Up @@ -889,10 +883,10 @@ export class PropertiesImplementation {
O,
P,
new PropertyDescriptor({
value: "value" in Desc ? Desc.value : realm.intrinsics.undefined,
writable: "writable" in Desc ? Desc.writable : false,
enumerable: "enumerable" in Desc ? Desc.enumerable : false,
configurable: "configurable" in Desc ? Desc.configurable : false,
value: Desc.value !== undefined ? Desc.value : realm.intrinsics.undefined,
writable: Desc.writable !== undefined ? Desc.writable : false,
enumerable: Desc.enumerable !== undefined ? Desc.enumerable : false,
configurable: Desc.configurable !== undefined ? Desc.configurable : false,
})
);
InternalUpdatedProperty(realm, O, P, undefined);
Expand All @@ -911,10 +905,10 @@ export class PropertiesImplementation {
O,
P,
new PropertyDescriptor({
get: "get" in Desc ? Desc.get : realm.intrinsics.undefined,
set: "set" in Desc ? Desc.set : realm.intrinsics.undefined,
enumerable: "enumerable" in Desc ? Desc.enumerable : false,
configurable: "configurable" in Desc ? Desc.configurable : false,
get: Desc.get !== undefined ? Desc.get : realm.intrinsics.undefined,
set: Desc.set !== undefined ? Desc.set : realm.intrinsics.undefined,
enumerable: Desc.enumerable !== undefined ? Desc.enumerable : false,
configurable: Desc.configurable !== undefined ? Desc.configurable : false,
})
);
InternalUpdatedProperty(realm, O, P, undefined);
Expand All @@ -929,13 +923,20 @@ export class PropertiesImplementation {
Desc = Desc.throwIfNotConcrete(realm);

// 3. Return true, if every field in Desc is absent.
if (!Object.keys(Desc).length) return true;
let allAbsent = true;
for (let field in Desc) {
if ((Desc: any)[field] !== undefined) {
allAbsent = false;
break;
}
}
if (allAbsent) return true;

// 4. Return true, if every field in Desc also occurs in current and the value of every field in Desc is the
// same value as the corresponding field in current when compared using the SameValue algorithm.
let identical = true;
for (let field in Desc) {
if (!(field in current)) {
if ((current: any)[field] === undefined) {
identical = false;
} else {
let dval = InternalDescriptorPropertyToValue(realm, (Desc: any)[field]);
Expand Down Expand Up @@ -966,7 +967,7 @@ export class PropertiesImplementation {
if (Desc.configurable) return false;

// b. Return false, if the [[Enumerable]] field of Desc is present and the [[Enumerable]] fields of current and Desc are the Boolean negation of each other.
if ("enumerable" in Desc && Desc.enumerable !== current.enumerable) {
if (Desc.enumerable !== undefined && Desc.enumerable !== current.enumerable) {
return false;
}
}
Expand Down Expand Up @@ -1055,8 +1056,8 @@ export class PropertiesImplementation {
// If the property might have been deleted, we need to ensure that either
// the new descriptor overrides any existing values, or always results in
// the default value.
let unknownEnumerable = !("enumerable" in Desc) && !!current.enumerable;
let unknownWritable = !("writable" in Desc) && !!current.writable;
let unknownEnumerable = Desc.enumerable === undefined && !!current.enumerable;
let unknownWritable = Desc.writable === undefined && !!current.writable;
if (unknownEnumerable || unknownWritable) {
let error = new CompilerDiagnostic(
"unknown descriptor attributes on deleted property",
Expand Down Expand Up @@ -1091,7 +1092,11 @@ export class PropertiesImplementation {

// a. For each field of Desc that is present, set the corresponding attribute of the property named P of
// object O to the value of the field.
for (let field in Desc) (current: any)[field] = (Desc: any)[field];
for (let field in Desc) {
if ((Desc: any)[field] !== undefined) {
(current: any)[field] = (Desc: any)[field];
}
}
InternalUpdatedProperty(realm, O, P, oldDesc);
}

Expand Down Expand Up @@ -1341,7 +1346,7 @@ export class PropertiesImplementation {

// 12. If newLenDesc.[[Writable]] is absent or has the value true, let newWritable be true.
let newWritable;
if (!("writable" in newLenDesc) || newLenDesc.writable === true) {
if (newLenDesc.writable === undefined || newLenDesc.writable === true) {
newWritable = true;
} else {
// 13. Else,
Expand Down Expand Up @@ -1594,9 +1599,9 @@ export class PropertiesImplementation {
P,
new PropertyDescriptor({
value: value,
writable: "writable" in X ? X.writable : false,
enumerable: "enumerable" in X ? X.enumerable : false,
configurable: "configurable" in X ? X.configurable : false,
writable: X.writable !== undefined ? X.writable : false,
enumerable: X.enumerable !== undefined ? X.enumerable : false,
configurable: X.configurable !== undefined ? X.configurable : false,
})
);
}
Expand Down
2 changes: 1 addition & 1 deletion src/methods/to.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ export class ToImplementation {
// 15. If either desc.[[Get]] or desc.[[Set]] is present, then
if (desc.get || desc.set) {
// a. If either desc.[[Value]] or desc.[[Writable]] is present, throw a TypeError exception.
if ("value" in desc || "writable" in desc) {
if (desc.value !== undefined || desc.writable !== undefined) {
throw realm.createErrorThrowCompletion(realm.intrinsics.TypeError);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/serializer/ResidualHeapSerializer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ export class ResidualHeapSerializer {

let descriptorsKey = [];
for (let boolKey of boolKeys) {
if (boolKey in desc) {
if ((desc: any)[boolKey] !== undefined) {
let b: boolean = (desc: any)[boolKey];
invariant(b !== undefined);
descProps.push(t.objectProperty(t.identifier(boolKey), t.booleanLiteral(b)));
Expand All @@ -720,7 +720,7 @@ export class ResidualHeapSerializer {
invariant(descriptorId !== undefined);

for (let descKey of valKeys) {
if (descKey in desc) {
if ((desc: any)[descKey] !== undefined) {
let descValue: Value = (desc: any)[descKey];
invariant(descValue instanceof Value);
if (descValue instanceof UndefinedValue) {
Expand Down
13 changes: 9 additions & 4 deletions src/values/AbstractObjectValue.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,16 @@ export default class AbstractObjectValue extends AbstractValue {
firstExistingDesc = firstExistingDesc.throwIfNotConcrete(this.$Realm);
}
let desc = new PropertyDescriptor({
value: "value" in Desc ? Desc.value : this.$Realm.intrinsics.undefined,
writable: "writable" in Desc ? Desc.writable : firstExistingDesc ? firstExistingDesc.writable : false,
enumerable: "enumerable" in Desc ? Desc.enumerable : firstExistingDesc ? firstExistingDesc.enumerable : false,
value: Desc.value !== undefined ? Desc.value : this.$Realm.intrinsics.undefined,
writable: Desc.writable !== undefined ? Desc.writable : firstExistingDesc ? firstExistingDesc.writable : false,
enumerable:
Desc.enumerable !== undefined ? Desc.enumerable : firstExistingDesc ? firstExistingDesc.enumerable : false,
configurable:
"configurable" in Desc ? Desc.configurable : firstExistingDesc ? firstExistingDesc.configurable : false,
Desc.configurable !== undefined
? Desc.configurable
: firstExistingDesc
? firstExistingDesc.configurable
: false,
});
let newVal = desc.value;
if (this.kind === "conditional") {
Expand Down
2 changes: 1 addition & 1 deletion src/values/ProxyValue.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ export default class ProxyValue extends ObjectValue {

// 13. If Desc has a [[Configurable]] field and if Desc.[[Configurable]] is false, then
let settingConfigFalse;
if ("configurable" in Desc && !Desc.throwIfNotConcrete(realm).configurable) {
if (Desc.throwIfNotConcrete(realm).configurable === false) {
// a. Let settingConfigFalse be true.
settingConfigFalse = true;
} else {
Expand Down

0 comments on commit 2b4546d

Please sign in to comment.