enso-org / enso

Enso Analytics is a self-service data prep and analysis platform designed for data teams.
https://ensoanalytics.com
Apache License 2.0
7.37k stars 323 forks source link

Broken redefinition of variable inside if-then expression #6211

Open Akirathan opened 1 year ago

Akirathan commented 1 year ago

Inside then branch of an if expression, a variable assignment is syntactically correct. The following snippet should create a new variable named a inside the then expression, and return Nothing.

a = 5
if False then a = 10 else a = 15

But it fails with

Execution finished with an error: Method `=` of 5 (Integer) could not be found.
JaroslavTulach commented 1 year ago

Looks like the situation is more complex. Looking for help by language and parser experts, CCing @wdanilo and @kazcw. Following program offers "block" and "one liner" version of the same code:

block =
    a = 5
    if a == 3 then a = 10 else
        a = 15
        a # needn't be here, still OK without this line

line =
    a = 5
    if a == 3 then a = 10 else a = 15

Running block is fine. Running line generates an error as described in the issue description.

Of course using a = 10 in the "one liner" makes little sense. However using a = 10 in the block (without a being used further) makes little sense as well.

How shall the engine behave? Should the "one liner" behave just like the "block" (without a being returned)? Thank you in advance for your advises.

JaroslavTulach commented 1 year ago

Wojciech wrote:

these 2 codes should behave the same way

I will modify the code so the "one liner" behaves the same as the block version.

JaroslavTulach commented 1 year ago

The Tree we get from the Rust parser is causing the difference. While in the case of "oneliner" it is:

OprApp[" ", "a = 15", Ident["", "a", Ident["", "a", false, 0, false, false]], 
Either{right=Operator[" ", "=", ]}, Number[" ", "15", null, Digits["", "15", null], null]]

in case of the "block" version it is:

Assignment["        ", "a = 15", Ident["", "a", Ident["", "a", false, 0, false, false]], 
Operator[" ", "=", ], Number[" ", "15", null, Digits["", "15", null], null]]

I believe the right fix is to modify the Rust parser to treat even the "oneliner" as an Assignment. Passing to @kazcw

enso-bot[bot] commented 1 year ago

Jaroslav Tulach reports a new STANDUP for yesterday (2023-05-12):

Progress: - TypeNameTest and RuntimeErrorsTest: https://github.com/enso-org/enso/pull/6584/commits/f2df70c70f1e133f3e40914b0306a663d91baa88

Next Day: Finding a way to join Dmitry's #6655 and my #6584

Discord
Discord - A New Way to Chat with Friends & Communities
Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.