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

The toString() of 'class' and 'function' return SourceText #1726

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

The toString() of 'class' and 'function' return SourceText #1726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
30643e9
The toString() of 'class' and 'function' return SourceText
scgm0 Jan 5, 2024
c2058a9
Fix method names in SourceText
scgm0 Jan 6, 2024
e017b1d
The toString() of 'class' and 'function' return SourceText
scgm0 Jan 5, 2024
1165659
Fix methodDef.Closure._sourceText Regex
scgm0 Feb 8, 2024
cd2e567
Using String Replacement Instead of Regular Replacement
scgm0 Feb 8, 2024
40afaef
Fix getter/setter and private method
scgm0 Feb 8, 2024
2c2fa8d
Update Test262Harness.settings.json
scgm0 Feb 9, 2024
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
49 changes: 48 additions & 1 deletion Jint.Tests.Test262/Test262Harness.settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,53 @@
"language/statements/function/scope-param-elem-var-open.js",
"language/statements/function/scope-param-rest-elem-var-close.js",
"language/statements/function/scope-param-rest-elem-var-open.js",
"language/statements/with/cptn-abrupt-empty.js"
"language/statements/with/cptn-abrupt-empty.js",

// need to wait for further updates from Esprima
"built-ins/Function/prototype/toString/arrow-function.js",
"built-ins/Function/prototype/toString/async-arrow-function.js",
"built-ins/Function/prototype/toString/async-function-declaration.js",
"built-ins/Function/prototype/toString/async-function-expression.js",
"built-ins/Function/prototype/toString/async-method-class-expression-static.js",
"built-ins/Function/prototype/toString/async-method-class-expression.js",
"built-ins/Function/prototype/toString/async-method-class-statement-static.js",
"built-ins/Function/prototype/toString/async-method-class-statement.js",
"built-ins/Function/prototype/toString/async-method-object.js",
"built-ins/Function/prototype/toString/AsyncFunction.js",
"built-ins/Function/prototype/toString/class-declaration-complex-heritage.js",
"built-ins/Function/prototype/toString/class-declaration-explicit-ctor.js",
"built-ins/Function/prototype/toString/class-declaration-implicit-ctor.js",
"built-ins/Function/prototype/toString/class-expression-explicit-ctor.js",
"built-ins/Function/prototype/toString/class-expression-implicit-ctor.js",
"built-ins/Function/prototype/toString/function-declaration-non-simple-parameter-list.js",
"built-ins/Function/prototype/toString/function-declaration.js",
"built-ins/Function/prototype/toString/function-expression.js",
"built-ins/Function/prototype/toString/Function.js",
"built-ins/Function/prototype/toString/generator-function-declaration.js",
"built-ins/Function/prototype/toString/generator-function-expression.js",
"built-ins/Function/prototype/toString/generator-method.js",
"built-ins/Function/prototype/toString/getter-class-expression-static.js",
"built-ins/Function/prototype/toString/getter-class-expression.js",
"built-ins/Function/prototype/toString/getter-class-statement-static.js",
"built-ins/Function/prototype/toString/getter-class-statement.js",
"built-ins/Function/prototype/toString/getter-object.js",
"built-ins/Function/prototype/toString/line-terminator-normalisation-CR-LF.js",
"built-ins/Function/prototype/toString/line-terminator-normalisation-CR.js",
"built-ins/Function/prototype/toString/line-terminator-normalisation-LF.js",
"built-ins/Function/prototype/toString/method-class-expression-static.js",
"built-ins/Function/prototype/toString/method-class-expression.js",
"built-ins/Function/prototype/toString/method-class-statement-static.js",
"built-ins/Function/prototype/toString/method-class-statement.js",
"built-ins/Function/prototype/toString/method-object.js",
"built-ins/Function/prototype/toString/private-method-class-expression.js",
"built-ins/Function/prototype/toString/private-method-class-statement.js",
"built-ins/Function/prototype/toString/private-static-method-class-expression.js",
"built-ins/Function/prototype/toString/private-static-method-class-statement.js",
"built-ins/Function/prototype/toString/setter-class-expression-static.js",
"built-ins/Function/prototype/toString/setter-class-expression.js",
"built-ins/Function/prototype/toString/setter-class-statement-static.js",
"built-ins/Function/prototype/toString/setter-class-statement.js",
"built-ins/Function/prototype/toString/setter-object.js",
"built-ins/Function/prototype/toString/unicode.js"
]
}
9 changes: 9 additions & 0 deletions Jint/Native/Function/ClassDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Text.RegularExpressions;
using Esprima;
using Esprima.Ast;
using Esprima.Utils;
Expand All @@ -19,6 +20,7 @@ internal sealed class ClassDefinition
internal static readonly MethodDefinition _emptyConstructor;

internal readonly string? _className;
private readonly string _classSource;
private readonly Expression? _superClass;
private readonly ClassBody _body;

Expand All @@ -39,10 +41,12 @@ static MethodDefinition CreateConstructorMethodDefinition(string source)

public ClassDefinition(
string? className,
string classSource,
Expression? superClass,
ClassBody body)
{
_className = className;
_classSource = classSource;
_superClass = superClass;
_body = body;
}
Expand Down Expand Up @@ -145,6 +149,7 @@ public JsValue BuildConstructor(EvaluationContext context, Environment env)
F = constructorInfo.Closure;

F.SetFunctionName(_className ?? "");
F._sourceText = _classSource;

F.MakeConstructor(writableProperty: false, proto);
F._constructorKind = _superClass is null ? ConstructorKind.Base : ConstructorKind.Derived;
Expand Down Expand Up @@ -359,6 +364,10 @@ public ClassStaticBlockFunction(StaticBlock staticBlock) : base(Nodes.StaticBloc
{
var methodDef = method.DefineMethod(obj);
methodDef.Closure.SetFunctionName(methodDef.Key);
// TODO currently in the SourceText retrieved from Esprima, the method name is incorrect, so for now, use a string replacement.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it make more sense to fix the wrong name on Esprima side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I don't know how to fix it in Esprima (

Copy link
Collaborator

Choose a reason for hiding this comment

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

There are a lot of example test cases. You would just need to check the produced AST and that the identifier is correct. Minimal test case helps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried it, and after debugging for a while, I gave up. Maybe you can take a look? I'll try it again later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I generally have a lot more free time for OSS if someone takes care of my child and cleans the house.

var oldSourceText = methodDef.Closure.ToString();
var index = oldSourceText.IndexOf("function", StringComparison.Ordinal);
methodDef.Closure._sourceText = index > -1 ? oldSourceText.Remove(index, 8).Insert(index, methodDef.Closure._functionDefinition.Name ?? string.Empty) : oldSourceText;
return DefineMethodProperty(obj, methodDef.Key, methodDef.Closure, enumerable);
}

Expand Down
25 changes: 23 additions & 2 deletions Jint/Native/Function/Function.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public abstract partial class Function : ObjectInstance, ICallable

internal Realm _realm;
internal PrivateEnvironment? _privateEnvironment;
internal string? _sourceText;
private readonly IScriptOrModule? _scriptOrModule;

protected Function(
Expand Down Expand Up @@ -206,6 +207,18 @@ internal void SetFunctionName(JsValue name, string? prefix = null, bool force =
if (!string.IsNullOrWhiteSpace(prefix))
{
name = prefix + " " + name;
if (prefix is "get" or "set")
{
// TODO currently in the SourceText retrieved from Esprima, the method name is incorrect, so for now, use a string replacement.
var oldSourceText = ToString();
var index = oldSourceText.IndexOf("function", StringComparison.Ordinal);
_sourceText = index > -1 ? oldSourceText.Remove(index, 8).Insert(index, name.AsString()) : oldSourceText;
}
}

if (_functionDefinition != null)
{
_functionDefinition.Name = name.AsString();
}

_nameDescriptor = new PropertyDescriptor(name, PropertyFlag.Configurable);
Expand Down Expand Up @@ -367,7 +380,15 @@ internal void SetFunctionLength(JsNumber length)

public override string ToString()
{
// TODO no way to extract SourceText from Esprima at the moment, just returning native code
if (_sourceText != null)
{
return _sourceText;
}
if ((_sourceText = _functionDefinition?.Function?.ToString()) != null)
{
return _sourceText;
}

var nameValue = _nameDescriptor != null ? UnwrapJsValue(_nameDescriptor) : JsString.Empty;
var name = "";
if (!nameValue.IsUndefined())
Expand All @@ -377,7 +398,7 @@ public override string ToString()

name = name.TrimStart(_functionNameTrimStartChars);

return "function " + name + "() { [native code] }";
return _sourceText = "function " + name + "() { [native code] }";
}

private sealed class ObjectInstanceWithConstructor : ObjectInstance
Expand Down
3 changes: 2 additions & 1 deletion Jint/Runtime/Interpreter/Expressions/JintClassExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Esprima.Ast;
using Esprima.Utils;
using Jint.Native.Function;

namespace Jint.Runtime.Interpreter.Expressions
Expand All @@ -9,7 +10,7 @@ internal sealed class JintClassExpression : JintExpression

public JintClassExpression(ClassExpression expression) : base(expression)
{
_classDefinition = new ClassDefinition(expression.Id?.Name, expression.SuperClass, expression.Body);
_classDefinition = new ClassDefinition(className: expression.Id?.Name, classSource: expression.ToString(), superClass: expression.SuperClass, body: expression.Body);
}

protected override object EvaluateInternal(EvaluationContext context)
Expand Down
2 changes: 1 addition & 1 deletion Jint/Runtime/Interpreter/JintFunctionDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal sealed class JintFunctionDefinition
private JintExpression? _bodyExpression;
private JintStatementList? _bodyStatementList;

public readonly string? Name;
public string? Name;
public readonly IFunction Function;

public JintFunctionDefinition(IFunction function)
Expand Down
3 changes: 2 additions & 1 deletion Jint/Runtime/Interpreter/Statements/JintClassDeclarationStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Esprima.Ast;
using Esprima.Utils;
using Jint.Native;
using Jint.Native.Function;

Expand All @@ -10,7 +11,7 @@ internal sealed class JintClassDeclarationStatement : JintStatement<ClassDeclara

public JintClassDeclarationStatement(ClassDeclaration classDeclaration) : base(classDeclaration)
{
_classDefinition = new ClassDefinition(className: classDeclaration.Id?.Name, classDeclaration.SuperClass, classDeclaration.Body);
_classDefinition = new ClassDefinition(className: classDeclaration.Id?.Name, classSource:classDeclaration.ToString(), superClass: classDeclaration.SuperClass, body: classDeclaration.Body);
}

protected override Completion ExecuteInternal(EvaluationContext context)
Expand Down
3 changes: 2 additions & 1 deletion Jint/Runtime/Interpreter/Statements/JintExportDefaultDeclaration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Esprima.Ast;
using Esprima.Utils;
using Jint.Native;
using Jint.Native.Function;
using Jint.Runtime.Interpreter.Expressions;
Expand All @@ -21,7 +22,7 @@ protected override void Initialize(EvaluationContext context)
{
if (_statement.Declaration is ClassDeclaration classDeclaration)
{
_classDefinition = new ClassDefinition(className: classDeclaration.Id?.Name ?? "default", classDeclaration.SuperClass, classDeclaration.Body);
_classDefinition = new ClassDefinition(className: classDeclaration.Id?.Name ?? "default", classSource: classDeclaration.ToString(), superClass: classDeclaration.SuperClass, body: classDeclaration.Body);
}
else if (_statement.Declaration is FunctionDeclaration functionDeclaration)
{
Expand Down