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

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

Fixes #12045 and #6303

Pull Request Description

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.

JaroslavTulach · 2025-01-23T05:20:22Z

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

-      var member = myTypeAtom.getMember(memberName);
-      assertThat("Member " + memberName + " should be readable", member, is(notNullValue()));
-      assertThat("All fields are numbers", member.isNumber(), is(true));
+    assertThat("Has at least two fields", memberNames.size(), is(greaterThan(2)));


Suggested change

assertThat("Has at least two fields", memberNames.size(), is(greaterThan(2)));

assertThat("Has more than two fields", memberNames.size(), is(greaterThan(2)));

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?

JaroslavTulach · 2025-01-23T05:26:15Z

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

+
+  /**
+   * Returns methods (both instance and static) defined on this type, including the ones inherited
+   * from Any. Instance methods are defined on this type, static methods are defined on its {@link


Does this work properly for Any, Number, Integer or Float hierarchy? E.g. are methods from Number (if any) also included?

JaroslavTulach · 2025-01-23T05:27:51Z

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

+  @TruffleBoundary
+  public Map<String, Function> getMethods(boolean includeStaticMethods) {
+    var ctx = EnsoContext.get(null);
+    var allMethods = new HashMap<>(methodsFromAny(ctx));


There is method allTypes() which returns "All types this type represents including super types." - I guess it should be used here somehow to handle the "numbers hierarchy".

JaroslavTulach · 2025-01-23T05:30:28Z

test/Base_Tests/src/Semantic/Meta_Spec.enso


        group_builder.specify "methods of Integer" <|
-            Meta.meta Integer . methods . sort . should_equal ['%', '*', '+', '-', '/', '<', '<=', '>', '>=', '^', 'abs', 'bit_and', 'bit_not', 'bit_or', 'bit_shift', 'bit_shift_l', 'bit_shift_r', 'bit_xor', 'ceil', 'div', 'fits_in_long', 'floor', 'negate', 'round', 'to_decimal', 'to_float', 'truncate']
+            expected_methods = ['%', '*', '+', '-', '/', '<', '<=', '>', '>=', '^', 'abs', 'bit_and', 'bit_not', 'bit_or', 'bit_shift', 'bit_shift_l', 'bit_shift_r', 'bit_xor', 'ceil', 'div', 'fits_in_long', 'floor', 'negate', 'round', 'to_decimal', 'to_float', 'truncate']


Can you concatenate the expected_methods from three parts?

those coming from Any

those coming from Number

those coming from Integer

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

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

JaroslavTulach Jan 23, 2025

JaroslavTulach Jan 23, 2025

JaroslavTulach Jan 23, 2025

JaroslavTulach Jan 23, 2025

	assertThat("Has at least two fields", memberNames.size(), is(greaterThan(2)));
	assertThat("Has more than two fields", memberNames.size(), is(greaterThan(2)));

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

JaroslavTulach Jan 23, 2025

Choose a reason for hiding this comment

JaroslavTulach Jan 23, 2025

Choose a reason for hiding this comment

JaroslavTulach Jan 23, 2025

Choose a reason for hiding this comment

JaroslavTulach Jan 23, 2025

Choose a reason for hiding this comment

Akirathan commented Jan 21, 2025 •

edited

Loading