Parser: Parse UUIDs; implement comments in AST; implement type annotations and signatures; fix field names #3653

kazcw · 2022-08-16T17:32:15Z

Pull Request Description

Implements:

UUIDs: https://www.pivotaltracker.com/story/show/182931137
Comments: https://www.pivotaltracker.com/story/show/182981779
Type annotations and signatures: https://www.pivotaltracker.com/story/show/182497454
Fix getter names (Parser: implement import #3627 (comment)).

Important Notes

I can't fully test UUIDs; I have tested that the data obtained in Rust matches my understanding of how the format is supposed to work. What remains to be tested is that the data in Java matches the way the old parser handles the format. So @JaroslavTulach, let me know if you see any cases where I'm not returning the same values.
This implementation of type annotations and signatures accepts any expression in type context. It would probably be nice to narrow this down at some point, but for now I have no design info on what specifically should be allowed in type expressions; this implementation should be at least an incremental improvement.

Checklist

Please include the following checklist in your PR:

The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed: Enso GUI was tested when built using BOTH
  ./run ide build and ./run ide watch.

[ci no changelog needed]

JaroslavTulach

I am concerned by the number of JNI calls we may need to do to read metadata...

lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java

lib/rust/parser/tests/parse.rs

…nnotations and signatures; fix field names #3653

JaroslavTulach · 2022-08-22T05:18:16Z

I've just merged this PR into #3611 as d4e5a4b and there is one new failure when running the EnsoCompilerTest:

enso$ sbt --java-home /graalvm
sbt:enso> project runtime
sbt:runtime> testOnly *EnsoCompilerTest*
[error] Test org.enso.compiler.EnsoCompilerTest.testComment failed: IR for ## A type representing computations that may fail.
[error] type Maybe
[error]  shall be equal expected:<...ings = List(
[error]     IR.[Comment.Documentation(
[error]       doc =  A type representing computations that may fail.,
[error]       location = Some(
[error]         IdentifiedLocation[_]
[error]       ),
[error]       passData = Iterable(),
[error]       diagnostics = DiagnosticStorage(
[error]         diagnostics = List()
[error]       ),
[error]       id = _
[error]     ),
[error]     IR.]Module.Scope.Definit...> but was:<...ings = List(
[error]     IR.[]Module.Scope.Definit...>, took 0.076 sec
[error] Failed: Total 12, Failed 1, Errors 0, Passed 10, Skipped 1
[error] Failed tests:
[error]         org.enso.compiler.EnsoCompilerTest

which means the code in this PR no longer recognizes

    ## A type representing computations that may fail.
    type Maybe

as documentation comment. The old parser apparently believes this is a documentation comment.

JaroslavTulach · 2022-08-22T14:20:13Z

lib/rust/parser/tests/parse.rs

+#[test]
+fn type_signatures() {
+    let cases = [
+        ("val : Bool", block![(TypeSignature val ":" (Ident Bool))]),


Note for myself: When a new PR on the Rust side is created, take a look at #[test] section, find out what is new and copy the same tests into #3611: Just like 0869d56

JaroslavTulach · 2022-08-23T10:49:53Z

lib/rust/parser/tests/parse.rs

+
+
+#### METADATA ####
+[[{"index":{"value":7},"size":{"value":8}},"5bad897e-099b-4b00-9348-64092636746d"],[{"index":{"value":15},"size":{"value":1}},"b803f0a9-51bf-4473-8dc0-5163340feb0a"],[{"index":{"value":16},"size":{"value":4}},"e48f96d6-8349-4c28-8c2a-3ac3c447a53c"],[{"index":{"value":20},"size":{"value":1}},"8adaadf0-443a-4598-bc61-4f85e6b518c2"],[{"index":{"value":21},"size":{"value":4}},"4e3ab45b-4333-4cf3-abb7-ced5b26b5d64"],[{"index":{"value":25},"size":{"value":1}},"a450dc37-ed96-470d-a12a-0a98e43a4002"],[{"index":{"value":26},"size":{"value":3}},"379322e5-b4b6-4c96-8404-348cdec4f2e9"],[{"index":{"value":0},"size":{"value":29}},"1db87a0f-3de5-41bc-bd0f-a4d6738dace9"],[{"index":{"value":37},"size":{"value":8}},"b8d0dcc9-298a-448e-b235-59ad71e2ef68"],[{"index":{"value":45},"size":{"value":1}},"1a8db951-33b6-41f7-b84c-2fda9d34dca2"],[{"index":{"value":46},"size":{"value":4}},"d53a8336-5125-48cb-adc0-99b8f6208fa6"],[{"index":{"value":50},"size":{"value":1}},"26032102-59e8-4e30-84f7-04d6410478bc"],[{"index":{"value":51},"size":{"value":4}},"1f5d99d1-680b-47e9-8599-061289a41201"],[{"index":{"value":55},"size":{"value":1}},"c2fe225b-9fe9-4f10-abb0-d83accd42336"],[{"index":{"value":56},"size":{"value":7}},"64907631-01f4-4a02-acab-a0f55e86eed4"],[{"index":{"value":30},"size":{"value":33}},"836d67f0-b0df-421f-8ac3-099e952d0050"],[{"index":{"value":71},"size":{"value":8}},"f7d49db0-6a18-45a0-9279-ac04cb25740b"],[{"index":{"value":79},"size":{"value":1}},"73ade9b0-f3a0-4483-9cfb-a355541cbf6b"],[{"index":{"value":80},"size":{"value":4}},"5d3b79ef-7d37-4a46-8e73-b0c01f8463a9"],[{"index":{"value":84},"size":{"value":1}},"0097b595-7d58-4ebf-bc1a-9252ee7ea4e0"]]


When I copy this source and try to parse it with old the Enso parser, it complains:

org.enso.syntax.text.Parser$ParserError: Internal error in parser Expected two lines after METADATA. at org.enso.syntax.text.Parser.splitMeta(Parser.scala:173) at org.enso.syntax.text.Parser.runWithIds(Parser.scala:200) at org.enso.compiler.EnsoCompilerTest.parseTest(EnsoCompilerTest.java:227) at org.enso.compiler.EnsoCompilerTest.testMetadataCheck(EnsoCompilerTest.java:192) at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:566)

The new parser is more accepting (and for now, it only cares about UUIDs, and ignores the second metadata blob). I'll switch the test case to use an untruncated source file.

JaroslavTulach · 2022-08-23T11:04:03Z

There is no getter for UUID field of Tree as far as I can say.

kazcw · 2022-08-29T16:33:56Z

I've just merged this PR into #3611 as d4e5a4b and there is one new failure when running the EnsoCompilerTest:

enso$ sbt --java-home /graalvm
sbt:enso> project runtime
sbt:runtime> testOnly *EnsoCompilerTest*
[error] Test org.enso.compiler.EnsoCompilerTest.testComment failed: IR for ## A type representing computations that may fail.
[error] type Maybe
[error]  shall be equal expected:<...ings = List(
[error]     IR.[Comment.Documentation(
[error]       doc =  A type representing computations that may fail.,
[error]       location = Some(
[error]         IdentifiedLocation[_]
[error]       ),
[error]       passData = Iterable(),
[error]       diagnostics = DiagnosticStorage(
[error]         diagnostics = List()
[error]       ),
[error]       id = _
[error]     ),
[error]     IR.]Module.Scope.Definit...> but was:<...ings = List(
[error]     IR.[]Module.Scope.Definit...>, took 0.076 sec
[error] Failed: Total 12, Failed 1, Errors 0, Passed 10, Skipped 1
[error] Failed tests:
[error]         org.enso.compiler.EnsoCompilerTest

which means the code in this PR no longer recognizes

    ## A type representing computations that may fail.
    type Maybe

as documentation comment. The old parser apparently believes this is a documentation comment.

Doc comments have not been implemented in the new parser (it's on the roadmap: https://www.pivotaltracker.com/story/show/182497173). That test must have been relying on treating all comments as doc comments, which is no more correct than treating all comments as machine-invisible plain comments. Sorry for the apparent regression, but in the implementation it's progress.

Doc comments lexer implementation - Pivotal Tracker

JaroslavTulach

Let's get this done and use it in #3611

kazcw · 2022-09-02T15:20:04Z

Combined this into #3674 for easier merge.

kazcw force-pushed the wip/kw/parser/uuids branch from 925f9da to df9177d Compare August 16, 2022 17:33

kazcw changed the title ~~Parser: Parse/generate UUIDs; implement comments in AST~~ Parser: Parse UUIDs; implement comments in AST; fix field names Aug 17, 2022

kazcw force-pushed the wip/kw/parser/uuids branch from df9177d to 5818dac Compare August 17, 2022 15:45

Implement comments; impl UUIDs for backend; fix field names

813a954

kazcw force-pushed the wip/kw/parser/uuids branch from 5818dac to 65b9a4a Compare August 17, 2022 15:53

Type annotations and signatures

1a81610

kazcw force-pushed the wip/kw/parser/uuids branch from 65b9a4a to 1a81610 Compare August 17, 2022 17:38

kazcw changed the title ~~Parser: Parse UUIDs; implement comments in AST; fix field names~~ Parser: Parse UUIDs; implement comments in AST; implement type annotations and signatures; fix field names Aug 17, 2022

kazcw marked this pull request as ready for review August 17, 2022 17:41

kazcw requested review from wdanilo, JaroslavTulach, MichaelMauderer, mwu-tow and farmaazon as code owners August 17, 2022 17:41

kazcw self-assigned this Aug 17, 2022

fixup missing file

e92853e

JaroslavTulach approved these changes Aug 18, 2022

View reviewed changes

lib/rust/parser/generate-java/java/org/enso/syntax2/Parser.java Show resolved Hide resolved

lib/rust/parser/tests/parse.rs Show resolved Hide resolved

kazcw added the CI: Ready to merge This PR is eligible for automatic merge label Aug 18, 2022

Merge branch 'develop' into wip/kw/parser/uuids

b4ca537

JaroslavTulach added a commit that referenced this pull request Aug 22, 2022

Merging with Parse UUIDs; implement comments in AST; implement type a…

d4e5a4b

…nnotations and signatures; fix field names #3653

JaroslavTulach reviewed Aug 22, 2022

View reviewed changes

mergify bot added 4 commits August 22, 2022 15:33

Merge branch 'develop' into wip/kw/parser/uuids

65b1395

Merge branch 'develop' into wip/kw/parser/uuids

571d9f7

Merge branch 'develop' into wip/kw/parser/uuids

bd65a82

Merge branch 'develop' into wip/kw/parser/uuids

182ea66

JaroslavTulach reviewed Aug 23, 2022

View reviewed changes

JaroslavTulach self-requested a review August 23, 2022 11:03

Merge branch 'develop' into wip/kw/parser/uuids

b5cd5ef

kazcw removed the CI: Ready to merge This PR is eligible for automatic merge label Aug 29, 2022

kazcw added 4 commits August 29, 2022 17:29

getter for UUIDs

e2e5c44

all JNI is unsafe

39978eb

docs in metadata module

b4e4543

metadata parsing: use test case the old parser hopefully accepts

5aabe5c

kazcw added the CI: Ready to merge This PR is eligible for automatic merge label Aug 29, 2022

mergify bot added 4 commits August 29, 2022 20:28

Merge branch 'develop' into wip/kw/parser/uuids

157e731

Merge branch 'develop' into wip/kw/parser/uuids

791b88e

Merge branch 'develop' into wip/kw/parser/uuids

f82ba02

Merge branch 'develop' into wip/kw/parser/uuids

f859a1f

JaroslavTulach approved these changes Sep 1, 2022

View reviewed changes

Merge branch 'develop' into wip/kw/parser/uuids

b56492d

This was referenced Sep 2, 2022

parser tests: build JNI library before using it enso-org/ci-build#8

Merged

Parse export statements; fix comma parsing #3674

Closed

kazcw closed this Sep 2, 2022

kazcw deleted the wip/kw/parser/uuids branch September 2, 2022 15:19

kazcw restored the wip/kw/parser/uuids branch September 2, 2022 15:20

kazcw reopened this Sep 2, 2022

mergify bot added 2 commits September 2, 2022 20:03

Merge branch 'develop' into wip/kw/parser/uuids

22cd1d1

Merge branch 'develop' into wip/kw/parser/uuids

1bc6597

kazcw removed the CI: Ready to merge This PR is eligible for automatic merge label Sep 2, 2022

windows compatibility

395e68d

kazcw added the CI: Ready to merge This PR is eligible for automatic merge label Sep 2, 2022

mergify bot merged commit c3f758e into develop Sep 3, 2022

mergify bot deleted the wip/kw/parser/uuids branch September 3, 2022 03:15

This was referenced Feb 6, 2023

Type annotation and type signatures support #4504

Closed

Export statement support #4531

Closed

Stdlib should contain a lazy visualization for Scatterplot #4518

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parser: Parse UUIDs; implement comments in AST; implement type annotations and signatures; fix field names #3653

Parser: Parse UUIDs; implement comments in AST; implement type annotations and signatures; fix field names #3653

kazcw commented Aug 16, 2022 •

edited

Loading

JaroslavTulach left a comment

JaroslavTulach commented Aug 22, 2022

JaroslavTulach Aug 22, 2022

JaroslavTulach Aug 23, 2022

kazcw Aug 29, 2022 •

edited

Loading

JaroslavTulach commented Aug 23, 2022

kazcw commented Aug 29, 2022 •

edited by unfurl-links bot

Loading

JaroslavTulach left a comment

kazcw commented Sep 2, 2022



		#### METADATA ####
		[[{"index":{"value":7},"size":{"value":8}},"5bad897e-099b-4b00-9348-64092636746d"],[{"index":{"value":15},"size":{"value":1}},"b803f0a9-51bf-4473-8dc0-5163340feb0a"],[{"index":{"value":16},"size":{"value":4}},"e48f96d6-8349-4c28-8c2a-3ac3c447a53c"],[{"index":{"value":20},"size":{"value":1}},"8adaadf0-443a-4598-bc61-4f85e6b518c2"],[{"index":{"value":21},"size":{"value":4}},"4e3ab45b-4333-4cf3-abb7-ced5b26b5d64"],[{"index":{"value":25},"size":{"value":1}},"a450dc37-ed96-470d-a12a-0a98e43a4002"],[{"index":{"value":26},"size":{"value":3}},"379322e5-b4b6-4c96-8404-348cdec4f2e9"],[{"index":{"value":0},"size":{"value":29}},"1db87a0f-3de5-41bc-bd0f-a4d6738dace9"],[{"index":{"value":37},"size":{"value":8}},"b8d0dcc9-298a-448e-b235-59ad71e2ef68"],[{"index":{"value":45},"size":{"value":1}},"1a8db951-33b6-41f7-b84c-2fda9d34dca2"],[{"index":{"value":46},"size":{"value":4}},"d53a8336-5125-48cb-adc0-99b8f6208fa6"],[{"index":{"value":50},"size":{"value":1}},"26032102-59e8-4e30-84f7-04d6410478bc"],[{"index":{"value":51},"size":{"value":4}},"1f5d99d1-680b-47e9-8599-061289a41201"],[{"index":{"value":55},"size":{"value":1}},"c2fe225b-9fe9-4f10-abb0-d83accd42336"],[{"index":{"value":56},"size":{"value":7}},"64907631-01f4-4a02-acab-a0f55e86eed4"],[{"index":{"value":30},"size":{"value":33}},"836d67f0-b0df-421f-8ac3-099e952d0050"],[{"index":{"value":71},"size":{"value":8}},"f7d49db0-6a18-45a0-9279-ac04cb25740b"],[{"index":{"value":79},"size":{"value":1}},"73ade9b0-f3a0-4483-9cfb-a355541cbf6b"],[{"index":{"value":80},"size":{"value":4}},"5d3b79ef-7d37-4a46-8e73-b0c01f8463a9"],[{"index":{"value":84},"size":{"value":1}},"0097b595-7d58-4ebf-bc1a-9252ee7ea4e0"]]

Parser: Parse UUIDs; implement comments in AST; implement type annotations and signatures; fix field names #3653

Parser: Parse UUIDs; implement comments in AST; implement type annotations and signatures; fix field names #3653

Conversation

kazcw commented Aug 16, 2022 • edited Loading

Pull Request Description

Important Notes

Checklist

JaroslavTulach left a comment

Choose a reason for hiding this comment

JaroslavTulach commented Aug 22, 2022

JaroslavTulach Aug 22, 2022

Choose a reason for hiding this comment

JaroslavTulach Aug 23, 2022

Choose a reason for hiding this comment

kazcw Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

JaroslavTulach commented Aug 23, 2022

kazcw commented Aug 29, 2022 • edited by unfurl-links bot Loading

JaroslavTulach left a comment

Choose a reason for hiding this comment

kazcw commented Sep 2, 2022

kazcw commented Aug 16, 2022 •

edited

Loading

kazcw Aug 29, 2022 •

edited

Loading

kazcw commented Aug 29, 2022 •

edited by unfurl-links bot

Loading