Return null when passed a non-function argument

[lucasfcosta]

This PR aims to solve the issue described at chaijs/chai#893.

I just added a simple instanceof check to the main function in this module in order to detect when something that is not an instance of a function is passed. When that happens we return null.

Also, this should be enough to cover all cases since the @@hasInstance symbol needs to be implemented on the "father" functions in order for it to recognize other instances as their children, so if anyone implemented @@hasInstance on the Function constructor they really expect whatever that symbol returns to be the final verdict whether something should be considered a function or not and therefore we shall not worry about those cases.

I also added tests for that and updated the docs.

I will also submit a PR to the main repo later.

[shvaikalesh]

I would rather typeof aFunc == 'function'. Function#toString expects an object to have some internal methods only functions have, typeof is the most accurate way to check for them.

Current solution will throw on edge cases like { __proto__: function() {} }.

[shvaikalesh]

However, even typeof == 'function' does not guarantee Function#toString won't throw. typeof == 'function' checks for [[Call]], but Function#toString requires [[ECMAScriptCode]]. let proxy = new Proxy(() => {}, {}) sets [[Call]], but not [[ECMAScriptCode]], so proxied functions does not work with toString.

[shvaikalesh]

Maybe we should early return for non-functions? Like if (typeof aFunc != 'function') return null. Otherwise nesting is rather deep.

[lucasfcosta]

I have just updated this.

[shvaikalesh]

Thanks for going forward on the instanceOf issue, @lucasfcosta.

I have a feeling that get-func-name should return undefined instead of null.

get-func-name essentially returns name property. When property is missing, internal [[Get]] returns undefined. Maybe we should too? null is used (in DOM API, and in Object.getPrototypeOf, for example) to highlight an absence of Object type.

[lucasfcosta]

@shvaikalesh hmmm, I'm not sure about that change.

Thanks for your feedback, I'm not strongly against it, but I feel like using undefined may give the impression that this went unhandled. Returning null is like saying "Hey, we've done something but there's no result", while returning undefined feels like "maybe we did something, maybe we just didn't notice this".

[meeber]

LGTM. I also prefer null over undefined due to the ambiguity of undefined in the language. I think it's okay for us to deviate in our function from the behavior of the engine when returning a missing property.

[shvaikalesh]

OK then, I have no strong opinion on this. LGTM.

[vieiralucas]

Maybe just nitpicking, but we could avoid unnecessary initializations of name here:

var name;
if (typeof aFunc !== 'function') {
  return null;
}

name = '';

or

if (typeof aFunc !== 'function') {
  return null;
}

var name = '';

[lucasfcosta]

Any byte counts.
Thanks!

[meeber]

RIP

Err... I mean, LGTM!

[vieiralucas]

LGTM

[lucasfcosta]

Thanks for your approval, friends :)