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.

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

Allow host to handle unknown references #410

Merged
merged 2 commits into from
Aug 26, 2017
Merged

Conversation

ayende
Copy link
Contributor

@ayende ayende commented Aug 25, 2017

See #409

This handle null reference errors as well as methods calls.

@@ -851,7 +852,17 @@ public JsValue EvaluateCallExpression(CallExpression callExpression)

if (!func.IsObject())
{
throw new JavaScriptException(_engine.TypeError, r == null ? "" : string.Format("Property '{0}' of object is not a function", (callee as Reference).GetReferencedName()));
var reference = (Reference) callee;
Copy link
Owner

Choose a reason for hiding this comment

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

This fails some tests with System.InvalidCastException: Unable to cast object of type 'Jint.Native.JsValue' to type 'Jint.Runtime.References.Reference'.

Copy link
Owner

Choose a reason for hiding this comment

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

This might have been hidden in the previous code because it was already throwing an exception. I assume the correct behavior is to throw a TypeError if it's not a reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I assumed that this is correct because it would NRE otherwise. Will fix in a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, there was no NRE, there is a check for r == null there.

@sebastienros
Copy link
Owner

FYI in ES6 this kind of pluggable behavior is part of the language, so you can dynamically change it.

@sebastienros sebastienros merged commit 4223c30 into sebastienros:dev Aug 26, 2017
@sebastienros
Copy link
Owner

sebastienros commented Aug 26, 2017

The dev branch is available on myget here https://www.myget.org/feed/Packages/jint
Published on nuget.org once merged in master, no special condition to do it, just based on time and velocity of changes.

@ayende
Copy link
Contributor Author

ayende commented Aug 26, 2017

Thanks


namespace Jint.Tests.Runtime
{
public class NullPropogation
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this typo be fixed to NullPropagation ?

{
public class NullPropogation
{
public class NullPropgationReferenceResolver : IReferenceResolver
Copy link
Contributor

Choose a reason for hiding this comment

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

And this one to NullPropagationReferenceResolver

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants