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

fix interface methods #1638

Merged
merged 7 commits into from Sep 26, 2023
Merged

fix interface methods #1638

merged 7 commits into from Sep 26, 2023

Conversation

hyzx86
Copy link
Contributor

@hyzx86 hyzx86 commented Sep 19, 2023

fix #1637

@Genteure
Copy link
Contributor

I'm not familiar with this area so I can't comment on whether the fix itself is appropriate or not.

Please use simple loops instead of LINQ as LINQ use much more memory and CPU cycles compared to simple for loops.
Please consider adding tests to verify your changes and to prevent regression.

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 20, 2023

@Genteure , That object is too complicated. So I don't know how to instantiate it....

using System.Linq.Expressions;

namespace Jint.Tests.Runtime.TestClasses
{
    public class InterfaceTestModel
    {
        public string Name { get; set; }
        public int Age { get; set; }

        public static ISelect<InterfaceTestModel> BuildSelect()
        {
            var select = new SelectProvider<InterfaceTestModel>();
            return  select;
        }
    }

    public class SelectProvider<T1> : Select0Provider<ISelect<T1>, T1>, ISelect<T1>
    {
        public bool Any(Expression<Func<T1, bool>> exp)
        {
            return true;
        }
    }

    public class Select0Provider<TSelect, T1>
    {
        public bool Any()
        {
            return true;
        }
    }
    public class InterfaceTest<TSelect, T1> : ISelect0<TSelect, T1>
    {
        public bool Any()
        {
            return true;
        }
    }


    public interface ISelect0 { }
    public partial interface ISelect0<TSelect, T1> : ISelect0
    {
        bool Any();
    }

    public interface ISelect<T1> : ISelect0<ISelect<T1>, T1>
    {
        bool Any(Expression<Func<T1, bool>> exp);

    }
}

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 20, 2023

@2881099 ,Can you help me see how this thing can be instantiated?

I have been unable to reproduce the problem using the following code simulation, I should be instantiating the wrong way

    [Fact]
    public void InterfaceMethodTest()
    {
        Assert.Equal(true, InterfaceTestModel.BuildSelect().Any());

        var engine = new Engine();
        engine.SetValue("mySelect", InterfaceTestModel.BuildSelect());
        var result = engine.Evaluate("mySelect.any()").IsBoolean();
        Assert.Equal(true, result);
    }

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 20, 2023

Hi @lahma ,Could you please review this PR? Thank you.

I don't know why PR Check tasks don't pass to windows tasks,

But on my computer, I passed most of the unit tests and failed only a few that involved language and culture.
image

@viruscamp
Copy link
Contributor

viruscamp commented Sep 21, 2023

@hyzx86 , I provide you the tests. But there is still something unresolved, maybe you should mark CallSubPropertySuperMethod_SubProperty and CallSubPropertySuperMethod_SuperMethod as [Fact(Skip = "TODO")].

diff --git a/Jint.Tests/Runtime/InteropExplicitTypeTests.cs b/Jint.Tests/Runtime/InteropExplicitTypeTests.cs
index eef485018..a5be5e201 100644
--- a/Jint.Tests/Runtime/InteropExplicitTypeTests.cs
+++ b/Jint.Tests/Runtime/InteropExplicitTypeTests.cs
@@ -1,13 +1,23 @@
 namespace Jint.Tests.Runtime;
 
 using Jint.Native;
+using Jint.Runtime;
 using Jint.Runtime.Interop;
 
 public class InteropExplicitTypeTests
 {
-    public interface I1
+    public interface I0
+    {
+        string NameI0 { get => "I0.Name"; }
+        string OverloadSuperMethod() => "I0.OverloadSuperMethod()";
+        string SubPropertySuperMethod() => "I0.SubPropertySuperMethod()";
+    }
+
+    public interface I1 : I0
     {
         string Name { get; }
+        string OverloadSuperMethod(int x) => $"I1.OverloadSuperMethod(int {x})";
+        new string SubPropertySuperMethod => "I1.SubPropertySuperMethod";
     }
 
     public class Super
@@ -120,6 +130,42 @@ public class InteropExplicitTypeTests
         Assert.Equal(holder.IndexerI1[0].Name, _engine.Evaluate("holder.IndexerI1[0].Name"));
     }
 
+    [Fact]
+    public void CallSuperPropertyFromInterface()
+    {
+        Assert.Equal(holder.I1.NameI0, _engine.Evaluate("holder.I1.NameI0"));
+    }
+
+    [Fact]
+    public void CallOverloadSuperMethod()
+    {
+        Assert.Equal(
+            holder.I1.OverloadSuperMethod(1),
+            _engine.Evaluate("holder.I1.OverloadSuperMethod(1)"));
+        Assert.Equal(
+            holder.I1.OverloadSuperMethod(),
+            _engine.Evaluate("holder.I1.OverloadSuperMethod()"));
+    }
+
+    [Fact]
+    public void CallSubPropertySuperMethod_SubProperty()
+    {
+        Assert.Equal(
+            holder.I1.SubPropertySuperMethod,
+            _engine.Evaluate("holder.I1.SubPropertySuperMethod"));
+    }
+
+    [Fact]
+    public void CallSubPropertySuperMethod_SuperMethod()
+    {
+        var ex = Assert.Throws<JavaScriptException>(() =>
+        {
+            Assert.Equal(
+                holder.I1.SubPropertySuperMethod(),
+                _engine.Evaluate("holder.I1.SubPropertySuperMethod()"));
+        });
+        Assert.Equal("Property 'SubPropertySuperMethod' of object is not a function", ex.Message);
+    }
 
     [Fact]
     public void SuperClassFromField()

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 21, 2023

CallSubPropertySuperMethod_SubProperty

Hi @viruscamp , Thanks for your support , I have done the UnitTest ,
But I added precompiled instructions at the beginning and end of the class file to solve the following problem.
"The default interface implementation is not supported by the target runtime."

And all of the UnitTests in interopTests are passed on my pc

image

@viruscamp
Copy link
Contributor

I wrote the test in Linux, so I wrote interface default methods which won't run in net462.

Just copy the 5 methods with body in I0 and I1 to class C1, add I1. or I0. to methods name, and then remove the methods body =>"..." in interfaces.

restore InteropExplicitTypeTests changes
@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 21, 2023

@viruscamp I have created a new file as Jint.Tests/Runtime/InterfaceTests.cs

I ran the 4 tests you mentioned on my computer and they all passed ,Whether it's net462 or net6.0

image
image

Copy link
Contributor

@viruscamp viruscamp Sep 22, 2023

Choose a reason for hiding this comment

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

Would you like to rename the class to a clearer name like InteropInterfaceExtendTests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you please remove this change from the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@viruscamp viruscamp left a comment

Choose a reason for hiding this comment

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

OK

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 25, 2023

Hi @lahma ,Could you please review this PR? Thank you.

@lahma lahma merged commit da88d1e into sebastienros:main Sep 26, 2023
3 checks passed
@lahma
Copy link
Collaborator

lahma commented Sep 26, 2023

Thank you.

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.

error No public methods with the specified arguments were found
4 participants