Members on Atom and Type include inherited methods from Any #12099

Akirathan · 2025-01-21T18:27:33Z

Fixes #12045 and #6303

Pull Request Description

getMembers interop message on Atom and on Type now include inherited methods from super types.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.
If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
or the Snowflake database integration, a run of the Extra Tests has been scheduled.
- If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

…mbers on Type.

GregoryTravis · 2025-01-22T15:20:33Z

test/Base_Tests/src/Data/Array_Spec.enso

+            vector_methods = Hashset.from_vector <| Meta.meta Vector . methods
+            array_methods = Hashset.from_vector <| Meta.meta Array . methods
+            diff = array_methods.difference vector_methods
+            Test.with_clue "The difference should be empty, but was: "+diff.to_text <|


Would a simple .should_equal work here?

No, because Vector's methods are a superset of Array's methods, which is expected. For example, there is Vector.from_array and Vector.from_polyglot_array which is not on Array, and that is fine.

# Conflicts: # engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/GetTypeMethodsNode.java

JaroslavTulach

I am surprised the code in Type isn't using allTypes(...), but hardcodes anyMethods. I'd expect more generic code.

...ntime-integration-tests/src/test/java/org/enso/interpreter/test/interop/AtomInteropTest.java

JaroslavTulach · 2025-01-23T05:24:24Z

...ntegration-tests/src/test/scala/org/enso/interpreter/test/instrument/RuntimeErrorsTest.scala

@@ -2428,7 +2428,7 @@ class RuntimeErrorsTest
        contextId,
        mainResId,
        Api.ExpressionUpdate.Payload.Panic(
-          "Compile_Error",
+          "Compile_Error.Error",


Why has this changed?

I was about to ask @4e6 to give me a clue why this change is needed. I have no idea which part is responsible for providing this panic payload.

engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Type.java

test/Base_Tests/src/Semantic/Meta_Spec.enso

…reter/test/interop/AtomInteropTest.java Co-authored-by: Jaroslav Tulach <[email protected]>

JaroslavTulach · 2025-01-23T11:40:42Z

engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Type.java

+    var allMethods = new HashMap<String, Function>();
+    for (var type : allTypes(ctx)) {
+      var methodsOnThisType = type.methodsOnThisType(includeStaticMethods);
+      allMethods.putAll(methodsOnThisType);


What if there are overrides? This code will likely put the Any.to_text into the map and not My_Type.to_text... it may not matter as type.getMethods(true).keySet() - e.g. only keys are used, but then this method should be changed to Set<String> and not provide incorrect .values().

Fixed in 80e80e4 - methods with same name from subtypes are kept in the map and not replaced by the methods from supertypes. In other words, overridden methods are preferred.

Akirathan · 2025-01-27T13:39:38Z

Added test for consistency between method invocation on types in pure Enso, and method invocation on types via interop protocol in b95e3ea. I believe that at this moment, this test sufficiently covers the functionality of method dispatch in Type.invokeMember. We should add a similar test for Atoms in separate PR as well. Moreover, this test can, obviously, be extended to provide all the values from ValuesGenerator to increase the coverage even more.

JaroslavTulach · 2025-01-28T03:17:43Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Locale.enso

-        remove_us = locale_meta.methods + ["Value", "new", "default", "from_language_tag", "from_java", "predefined_locale_fields", "default_widget", "widget_options"]
-        Meta.Type.Value (Meta.type_of locale_meta.value) . methods . filter (Filter_Condition.Is_In remove_us ..Remove) . sort
+        methods_to_remove = Vector.build bldr->
+            bldr.append "Value"


This one should be private unless something horribly breaks.

Test members on Atom and Type include inherited methods from Any

87cece0

Akirathan added CI: No changelog needed Do not require a changelog entry for this PR. CI: Clean build required CI runners will be cleaned before and after this PR is built. labels Jan 21, 2025

Akirathan self-assigned this Jan 21, 2025

Akirathan added 10 commits January 21, 2025 19:52

Add test for consistency between Meta.get_type_methods and interop me…

bd458d0

…mbers on Type.

Type has methods from Any as members

58b825b

Atom has instance methods from Any as members

8aa5131

Update AtomInteropTest

ede07e2

Update TypeMembersTest

34bd4e2

Member names are not qualified

d8ebddf

GetTypeMethodsNode delegates to Type.getMethods

8ced357

Update Meta_Spec test

d2c0d2d

Fix Locale.predefined_locale_fields

5bb54a4

Update Array_Spec test

3ec63bc

Akirathan changed the title ~~Test members on Atom and Type include inherited methods from Any~~ Members on Atom and Type include inherited methods from Any Jan 22, 2025

Akirathan marked this pull request as ready for review January 22, 2025 13:32

Akirathan requested review from jdunkerley, radeusgd, GregoryTravis, AdRiley, marthasharkey, 4e6, JaroslavTulach and hubertp as code owners January 22, 2025 13:32

GregoryTravis approved these changes Jan 22, 2025

View reviewed changes

Akirathan added 5 commits January 22, 2025 18:40

Fix constants in RuntimeTypesTest

6ed4a20

Remove member key size test from ExecCompilerTest

3c6050d

Remove prop size test from DebuggingEnsoTest

049dd6d

Fix constants in RuntimeErrorsTest

d5d7d05

Merge branch 'develop' into wip/akirathan/12045-type-get-members

f94e7c1

# Conflicts: # engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/GetTypeMethodsNode.java

JaroslavTulach approved these changes Jan 23, 2025

View reviewed changes

Akirathan and others added 4 commits January 23, 2025 11:02

Update engine/runtime-integration-tests/src/test/java/org/enso/interp…

cae43f7

…reter/test/interop/AtomInteropTest.java Co-authored-by: Jaroslav Tulach <[email protected]>

Add test for members from Number hierarchy

9b98189

Include methods from the whole type hierarchy

106b35d

Collect expected methods

2edaa22

JaroslavTulach reviewed Jan 23, 2025

View reviewed changes

Keep overriden methods.

80e80e4

enso-bot bot mentioned this pull request Jan 24, 2025

Function metadata call #5226

Open

Akirathan added 5 commits January 24, 2025 15:10

Type.Invoke member uses InvokeCallableNode

e69ce1c

Don't replace instance methods with static methods from eigen type

07c403c

Add minimal reproducer for failing RUntimeErrorsTest

8a4d67e

Add tests for MethodResolverNode

9d99e37

Add MethodInvocationOnTypeConsistencyTest

b95e3ea

enso-bot bot mentioned this pull request Jan 27, 2025

getMembers polyglot message of Type should include methods from Any #12045

Open

JaroslavTulach reviewed Jan 28, 2025

View reviewed changes

Merge branch 'develop' into wip/akirathan/12045-type-get-members

d14550b

JaroslavTulach linked an issue Feb 5, 2025 that may be closed by this pull request

Interop Atom's members test should return inherited members as well #6303

Open

Akirathan added 4 commits February 7, 2025 12:39

Merge branch 'develop' into wip/akirathan/12045-type-get-members

af32b43

Add ArgumentMappingTest

def8d62

Use resolveFunction from InvokeMethodNode

63f0df0

Extract shouldPrependSyntheticSelfArg from InvokeMethodNode

8eddeae

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Members on Atom and Type include inherited methods from Any #12099

Members on Atom and Type include inherited methods from Any #12099

Akirathan commented Jan 21, 2025 •

edited

Loading

GregoryTravis Jan 22, 2025

Akirathan Jan 22, 2025

JaroslavTulach left a comment

JaroslavTulach Jan 23, 2025

Akirathan Jan 23, 2025

JaroslavTulach Jan 23, 2025

Akirathan Jan 23, 2025

Akirathan commented Jan 27, 2025

JaroslavTulach Jan 28, 2025

Members on Atom and Type include inherited methods from Any #12099

Are you sure you want to change the base?

Members on Atom and Type include inherited methods from Any #12099

Conversation

Akirathan commented Jan 21, 2025 • edited Loading

Pull Request Description

Important Notes

Checklist

GregoryTravis Jan 22, 2025

Choose a reason for hiding this comment

Akirathan Jan 22, 2025

Choose a reason for hiding this comment

JaroslavTulach left a comment

Choose a reason for hiding this comment

JaroslavTulach Jan 23, 2025

Choose a reason for hiding this comment

Akirathan Jan 23, 2025

Choose a reason for hiding this comment

JaroslavTulach Jan 23, 2025

Choose a reason for hiding this comment

Akirathan Jan 23, 2025

Choose a reason for hiding this comment

Akirathan commented Jan 27, 2025

JaroslavTulach Jan 28, 2025

Choose a reason for hiding this comment

Akirathan commented Jan 21, 2025 •

edited

Loading