Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[New] compare .toString, .valueOf, and [Symbol.toPrimitive] completions #79

Merged
merged 1 commit into from
Oct 5, 2023
Merged

[New] compare .toString, .valueOf, and [Symbol.toPrimitive] completions #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .eslintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"id-length": [2, { "min": 1, "max": 23 }],
"max-depth": [2, 5],
"max-len": 0,
"max-lines-per-function": [2, { "max": 250 }],
"max-lines-per-function": [2, { "max": 300 }],
"max-params": 0,
"max-statements": [1, 10],
"max-statements-per-line": [2, { "max": 2 }],
"new-cap": [2, { "capIsNewExceptions": ["BigInt"] }],
Expand All @@ -28,6 +29,7 @@
"files": "test/**",
"rules": {
"func-name-matching": 0,
"no-throw-literal": 0,
"prefer-regex-literals": 0,
},
},
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
],
"dependencies": {
"es-get-iterator": "^1.1.3",
"es-to-primitive": "^1.2.1",
"functions-have-names": "^1.2.3",
"has": "^1.0.3",
"has-bigints": "^1.0.2",
Expand Down
56 changes: 56 additions & 0 deletions test/native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,62 @@ test('bigints', { skip: !hasBigInts }, function (t) {
t.end();
});

test('toPrimitive', function (t) {
t.test('gracefully handles error throwing', function (mt) {
var toStringThrow = {
toString: function () { throw new Error(); }
};
var valueOfThrow = {
valueOf: function () { throw new Error(); }
};
var noThrow = {};

mt.equal(isEqual(toStringThrow, noThrow), false, 'first argument toPrimitive throws, second does not');
mt.equal(isEqual(noThrow, toStringThrow), false, 'second argument toPrimitive throws, first does not');
mt.equal(isEqual(valueOfThrow, noThrow), false, 'first argument toPrimitive throws, second does not');
mt.equal(isEqual(noThrow, valueOfThrow), false, 'second argument toPrimitive throws; first does not');

mt.end();
});

t.test('toPrimitive strings', function (mt) {
var foo1 = {
toString: function () { return 'foo'; }
};
var foo2 = {
toString: function () { return 'foo'; }
};
var bar = {
toString: function () { return 'bar'; }
};

mt.equal(isEqual(foo1, foo2), true, 'both argument toPrimitive values are equal');
mt.equal(isEqual(foo1, bar), false, 'argument toPrimitive values are not equal');
mt.equal(isEqual(bar, foo1), false, 'argument toPrimitive values are not equal');

mt.end();
});

t.test('toPrimitive numbers', function (mt) {
var value1 = {
valueOf: function () { return 1; }
};
var alsoValue1 = {
valueOf: function () { return 1; }
};
var value2 = {
valueOf: function () { return 2; }
};

mt.equal(isEqual(value1, alsoValue1), true, 'both argument toPrimitive values are equal');
mt.equal(isEqual(value1, value2), false, 'argument toPrimitive values are not equal');
mt.equal(isEqual(value2, value1), false, 'argument toPrimitive values are not equal');

mt.end();
});
t.end();
});

var genericIterator = function (obj) {
var entries = objectEntries(obj);
return function iterator() {
Expand Down
106 changes: 106 additions & 0 deletions test/why.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,112 @@ test('bigints', { skip: !hasBigInts }, function (t) {
t.end();
});

test('toPrimitive', function (t) {
t.test('gracefully handles error throwing', function (mt) {
var toStringThrow = {
toString: function () { throw new Error(); }
};
var toStringThrowFalsy = {
toString: function () { throw false; }
};
var toStringThrowFalsy2 = {
toString: function () { throw false; }
};
var valueOfThrow = {
valueOf: function () { throw new Error(); }
};
var noThrow = {};

mt.equal(
isEqualWhy(toStringThrow, toStringThrowFalsy),
'value at key "toString" differs: Function string representations differ',
'both throw; falls back to toString comparison'
);
mt.equal(
isEqualWhy(toStringThrowFalsy, toStringThrowFalsy2),
'',
'both throw; falls back to toString comparison'
);

mt.equal(isEqualWhy(toStringThrow, noThrow), 'first argument toPrimitive (hint String) throws; second does not');
mt.equal(isEqualWhy(noThrow, toStringThrow), 'second argument toPrimitive (hint String) throws; first does not');
mt.equal(isEqualWhy(valueOfThrow, noThrow), 'first argument toPrimitive (hint Number) throws; second does not');
mt.equal(isEqualWhy(noThrow, valueOfThrow), 'second argument toPrimitive (hint Number) throws; first does not');

mt.end();
});

t.test('toPrimitive strings', function (mt) {
var foo1 = {
toString: function () { return 'foo'; }
};
var foo2 = {
toString: function () { return 'foo'; }
};
var bar = {
toString: function () { return 'bar'; }
};

mt.equal(isEqualWhy(foo1, foo2), '');
mt.equal(isEqualWhy(foo1, bar), 'first argument toPrimitive does not match second argument toPrimitive (hint String)');
mt.equal(isEqualWhy(bar, foo1), 'first argument toPrimitive does not match second argument toPrimitive (hint String)');

mt.end();
});

t.test('toPrimitive numbers', function (mt) {
var value1 = {
valueOf: function () { return 1; }
};
var alsoValue1 = {
valueOf: function () { return 1; }
};
var value2 = {
valueOf: function () { return 2; }
};

mt.equal(isEqualWhy(value1, alsoValue1), '');
mt.equal(isEqualWhy(value1, value2), 'first argument toPrimitive does not match second argument toPrimitive (hint Number)');
mt.equal(isEqualWhy(value2, value1), 'first argument toPrimitive does not match second argument toPrimitive (hint Number)');

mt.end();
});

t.test('Symbol.toPrimitive', { skip: !hasSymbols || !Symbol.toPrimitive }, function (st) {
var hints = [];
var obj = {
toString: function () { return 'toString'; },
valueOf: function () { return 'valueOf'; }
};
var obj2 = {
toString: function () { return 'toString'; },
valueOf: function () { return 'valueOf'; }
};
obj2[Symbol.toPrimitive] = function (hint) {
hints.push(hint);
if (hint === 'string') {
return 'toString';
}
if (hint === 'number') {
return 'valueOf';
}
return 'default';
};

st.equal(
isEqualWhy(obj, obj2),
'first argument toPrimitive does not match second argument toPrimitive (hint default)',
'test Symbol.toPrimitive'
);

st.deepEqual(hints, ['string', 'number', 'default'], 'all hints are tested');

st.end();
});

t.end();
});

var genericIterator = function (obj) {
var entries = objectEntries(obj);
return function iterator() {
Expand Down
39 changes: 39 additions & 0 deletions why.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var isSymbol = require('is-symbol');
var isCallable = require('is-callable');
var isBigInt = require('is-bigint');
var getIterator = require('es-get-iterator');
var toPrimitive = require('es-to-primitive/es2015');
var whichCollection = require('which-collection');
var whichBoxedPrimitive = require('which-boxed-primitive');
var getPrototypeOf = require('object.getprototypeof/polyfill')();
Expand All @@ -37,6 +38,32 @@ var normalizeFnWhitespace = function normalizeWhitespace(fnStr) {
return fnStr.replace(/^function ?\(/, 'function (').replace('){', ') {');
};

var testToPrim = function testToPrimitive(value, other, hint, hintName) {
var valPrimitive = NaN;
var valPrimitiveThrows = false;
try {
valPrimitive = toPrimitive(value, hint);
} catch (error) {
valPrimitiveThrows = true;
}

var otherPrimitive = NaN;
var otherPrimitiveThrows = false;
try {
otherPrimitive = toPrimitive(other, hint);
} catch (error) {
otherPrimitiveThrows = true;
}

if (valPrimitiveThrows || otherPrimitiveThrows) {
if (!valPrimitiveThrows) { return 'second argument toPrimitive (hint ' + hintName + ') throws; first does not'; }
if (!otherPrimitiveThrows) { return 'first argument toPrimitive (hint ' + hintName + ') throws; second does not'; }
} else if (valPrimitive !== otherPrimitive) {
return 'first argument toPrimitive does not match second argument toPrimitive (hint ' + hintName + ')';
}
return '';
};

module.exports = function whyNotEqual(value, other) {
if (value === other) { return ''; }
if (value == null || other == null) {
Expand Down Expand Up @@ -205,6 +232,18 @@ module.exports = function whyNotEqual(value, other) {
if (isProto.call(other, value)) { return 'second argument is the [[Prototype]] of the first'; }
if (getPrototypeOf(value) !== getPrototypeOf(other)) { return 'arguments have a different [[Prototype]]'; }

var valueIsFn = typeof value === 'function';
var otherIsFn = typeof other === 'function';

if (!valueIsFn || !otherIsFn) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this line? surely either a function or an object could both have toStrings or valueOfs that need comparing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise functions with same arity/name/body are equal despite whitespace between signature and body will fail because:

> const foo = Object(function(){});
undefined
> const bar = Object(function () {});
undefined
> foo.toString()
'function(){}'
> bar.toString()
'function () {}'

our options are to include this line, or delete that test. including this line is more backwards compatible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aha, hmm. that means this new functionality wouldn't apply to functions, which is weird. at the least, let's document that in the readme?

an alternative, tho, would be to skip the check for a function whenever the function's toString is === to Function.prototype.toString?

var result = testToPrim(value, other, String, 'String')
|| testToPrim(value, other, Number, 'Number')
|| testToPrim(value, other, void undefined, 'default');
if (result) {
return result;
}
}

var valueIterator = getIterator(value);
var otherIterator = getIterator(other);
if (!!valueIterator !== !!otherIterator) {
Expand Down
Loading