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

Problem with using functions when Strict option is enabled, since 3.0.26 #857

Closed
aviv86 opened this issue Apr 7, 2021 · 5 comments · Fixed by #859
Closed

Problem with using functions when Strict option is enabled, since 3.0.26 #857

aviv86 opened this issue Apr 7, 2021 · 5 comments · Fixed by #859

Comments

@aviv86
Copy link

aviv86 commented Apr 7, 2021

Hi
The following test passes in version 3.0.25 (and older), but started failing since 3.0.26

using Jint;
using Jint.Native;
using Xunit;

namespace Tryouts
{
    public class Program
    {
        public static void Main()
        {
            var script = @"
function execute(doc, args){
    var i = doc;
    {
        function doSomething() {
            return 'ayende';
        }

        i.Name = doSomething();
    }
}
";

            var engine = new Engine(options =>
            {
                options.Strict();
            });
            engine.Execute(script);

            var user = new User();
            var args = new []
            {
                JsValue.FromObject(engine, user)
            };

            var call = engine.GetValue("execute").TryCast<ICallable>();
            call.Call(Undefined.Instance, args); // throws JavaScriptException: 'doSomething is not defined', since version 3.0.26


            Assert.Equal("ayende", user.Name);

        }

        private class User
        {
            public string Name { get; set; }
        }
    }
}

Note that if we remove from the script the curly brackets that start after var i = doc; then the test passes.
Same if Strict option is disabled.

@lahma
Copy link
Collaborator

lahma commented Apr 7, 2021

I think the curly brackets make the function block scoped function (at least in strict mode) and it will not be hoisted to outside scope, it's ES6 behavior.

It seems that the function should be visible though for the block when it's only called inside of it.

@aviv86
Copy link
Author

aviv86 commented Apr 7, 2021

Chrome console :

Screenshot-js

@lahma
Copy link
Collaborator

lahma commented Apr 7, 2021

I think the problem is here:

internal static void BlockDeclarationInstantiation(
LexicalEnvironment env,
List<VariableDeclaration> lexicalDeclarations)
{
var envRec = env._record;
var boundNames = new List<string>();
for (var i = 0; i < lexicalDeclarations.Count; i++)
{
var variableDeclaration = lexicalDeclarations[i];
boundNames.Clear();
variableDeclaration.GetBoundNames(boundNames);
for (var j = 0; j < boundNames.Count; j++)
{
var dn = boundNames[j];
if (variableDeclaration.Kind == VariableDeclarationKind.Const)
{
envRec.CreateImmutableBinding(dn, strict: true);
}
else
{
envRec.CreateMutableBinding(dn, canBeDeleted: false);
}
}
/* If d is a FunctionDeclaration, a GeneratorDeclaration, an AsyncFunctionDeclaration, or an AsyncGeneratorDeclaration, then
* Let fn be the sole element of the BoundNames of d.
* Let fo be the result of performing InstantiateFunctionObject for d with argument env.
* Perform envRec.InitializeBinding(fn, fo).
*/
}
}

If you'd like to implement the missing bits, a PR would be most welcome.

@Jither
Copy link
Contributor

Jither commented Apr 7, 2021

Note that you can't necessarily compare with browser implementation in this - the ECMAScript spec defines different behavior for block level function declarations in browsers vs other ECMAScript implementations. ES6 strict mode in a browser will create both a block scoped and function scoped binding for the function. Outside a browser, things are simpler - the function will be block scoped (and thus not callable from outside the block) in both strict and non-strict mode. https://262.ecma-international.org/6.0/#sec-block-level-function-declarations-web-legacy-compatibility-semantics

(ETA: that's just a clarification - the code in the example should til work as in the browser).

@aviv86 aviv86 closed this as completed Apr 7, 2021
@aviv86 aviv86 reopened this Apr 7, 2021
@ppekrol
Copy link

ppekrol commented Apr 8, 2021

I think the problem is here:

internal static void BlockDeclarationInstantiation(
LexicalEnvironment env,
List<VariableDeclaration> lexicalDeclarations)
{
var envRec = env._record;
var boundNames = new List<string>();
for (var i = 0; i < lexicalDeclarations.Count; i++)
{
var variableDeclaration = lexicalDeclarations[i];
boundNames.Clear();
variableDeclaration.GetBoundNames(boundNames);
for (var j = 0; j < boundNames.Count; j++)
{
var dn = boundNames[j];
if (variableDeclaration.Kind == VariableDeclarationKind.Const)
{
envRec.CreateImmutableBinding(dn, strict: true);
}
else
{
envRec.CreateMutableBinding(dn, canBeDeleted: false);
}
}
/* If d is a FunctionDeclaration, a GeneratorDeclaration, an AsyncFunctionDeclaration, or an AsyncGeneratorDeclaration, then
* Let fn be the sole element of the BoundNames of d.
* Let fo be the result of performing InstantiateFunctionObject for d with argument env.
* Perform envRec.InitializeBinding(fn, fo).
*/
}
}

If you'd like to implement the missing bits, a PR would be most welcome.

@lahma we are not that familiar with all internals and caveats of JS. I would say that it would be saver if one of the Jint contributors could take a look.

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 a pull request may close this issue.

4 participants