source-academy / frontend

Frontend of Source Academy, an online experiential environment for computational thinking (React, Redux, Saga, Blueprint)
https://sourceacademy.org
Apache License 2.0
103 stars 166 forks source link

Missing type environment when evaluating editorPrelude #1995

Closed thomastanck closed 4 months ago

thomastanck commented 2 years ago

Background

When running missions/quests/etc it's common for there to be a prelude which runs import statements and other testing code that's accessible to the student. To support certain use cases, this prelude sometimes has to be run at a higher Source chapter, which is the reason why elevated context exists. See https://github.com/source-academy/frontend/pull/833 for the original PR introducing elevated context.

The elevated context currently doesn't proxy the type environment and other type-system related member variables in the Context object, resulting in missions that use import statements in the prelude causing type hint error messages.

Currently, elevated context is used in WorkspaceSaga: https://github.com/source-academy/frontend/blob/54647d78a2d33bdab36992a6fc34c19287f21b82/src/commons/sagas/WorkspaceSaga.ts#L556-L561

It is created in JsSlangHelper: https://github.com/source-academy/frontend/blob/54647d78a2d33bdab36992a6fc34c19287f21b82/src/commons/utils/JsSlangHelper.ts#L174

The bug report

To reproduce, set up both frontend and backend, and upload this quest:

<?xml version="1.0"?>
<CONTENT xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">

<TASK
  number="Q1"
  title="Buggy Quest">
  <READING>Reading</READING>

  <WEBSUMMARY>Summary</WEBSUMMARY>

    <TEXT>Text</TEXT>
    <PROBLEMS>
        <PROBLEM maxxp="300" type="programming">
          <TEXT>Text</TEXT>
            <SNIPPET>
<PREPEND>
import {
x_of,
y_of,
z_of
} from "curve";
</PREPEND>
<TEMPLATE>const x_of_copy = x_of;
1+"";</TEMPLATE>
                <SOLUTION></SOLUTION>
                <GRADER>
true;
                </GRADER>
            </SNIPPET>
        </PROBLEM>
    </PROBLEMS>
    <TEXT>Text</TEXT>

    <DEPLOYMENT interpreter="1">
    </DEPLOYMENT>

    <GRADERDEPLOYMENT interpreter="1">
</GRADERDEPLOYMENT>

</TASK>

</CONTENT>

Running the template code as is produces the following type hints:

Hints:
Line 1: Undeclared name 'x_of' detected
Line 2: A type mismatch was detected in the binary expression:
  1 + ""
The binary operator (+) expected two operands with types:
  addable + addable
but instead it received two operands of types:
  number + string

While the 2nd error message is correct, the first is not, as x_of is properly imported in the prelude.

Possible solutions:

1) Create proxies for the type system related member variables, see this part of the Context type: https://github.com/source-academy/js-slang/blob/master/src/types.ts#L157-L158 2) Rather than proxying things that should be copied over to the original context (whitelist solution), how about the opposite where proxies are added for things that shouldn't be copied over to the original context? (blacklist solution) 3) Replace elevatedContext with a better solution? ideas welcome.

(Maybe this is a good first issue? cc. @Aulud)

martin-henz commented 2 years ago

I don't think this is a good first issue, but it should be done. Any ideas welcome.

martin-henz commented 4 months ago

Hints re removed already. Validation and type checking is uniformly done on the prelude and the program.