Skip to content

Commit dc9b477

Browse files
openorcloseopenorclose
authored
Improve error messages, fix naming bug and assignment bug in native (#362)
* 0.3.4 * Improve native error messages * Bump package.json * Access actual name instead of unclashed name for operators * Add tests * Fix assignment bug on native
1 parent 9e505b4 commit dc9b477

File tree

11 files changed

+319
-18
lines changed

11 files changed

+319
-18
lines changed

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "js-slang",
3-
"version": "0.3.3",
3+
"version": "0.3.5",
44
"description": "Javascript-based interpreter for slang, written in Typescript",
55
"author": {
66
"name": "Source Academy",

‎src/__tests__/__snapshots__/interpreter-errors.ts.snap‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,195 @@ map(f, list(1, 2));",
6767
}
6868
`;
6969

70+
exports[`Cascading js errors work properly 1: expectParsedError 1`] = `
71+
Object {
72+
"alertResult": Array [],
73+
"code": "function make_alternating_stream(stream) {
74+
return pair(head(stream), () => make_alternating_stream(
75+
negate_whole_stream(
76+
stream_tail(stream))));
77+
}
78+
79+
function negate_whole_stream(stream) {
80+
return pair(-head(stream), () => negate_whole_stream(stream_tail(stream)));
81+
}
82+
83+
const ones = pair(1, () => ones);
84+
eval_stream(make_alternating_stream(enum_stream(1, 9)), 9);",
85+
"displayResult": Array [],
86+
"errors": Array [
87+
ExceptionError {
88+
"error": [Error: head(xs) expects a pair as argument xs, but encountered null],
89+
"location": SourceLocation {
90+
"end": Position {
91+
"column": 29,
92+
"line": 8,
93+
},
94+
"start": Position {
95+
"column": 17,
96+
"line": 8,
97+
},
98+
},
99+
"severity": "Error",
100+
"type": "Runtime",
101+
},
102+
],
103+
"parsedErrors": "Line 8: Error: head(xs) expects a pair as argument xs, but encountered null",
104+
"result": undefined,
105+
"resultStatus": "error",
106+
"transpiled": "const native = $$NATIVE_STORAGE;
107+
const callIfFuncAndRightArgs = native.operators.get(\\"callIfFuncAndRightArgs\\");
108+
const boolOrErr = native.operators.get(\\"boolOrErr\\");
109+
const wrap = native.operators.get(\\"wrap\\");
110+
const unaryOp = native.operators.get(\\"unaryOp\\");
111+
const binaryOp = native.operators.get(\\"binaryOp\\");
112+
const throwIfTimeout = native.operators.get(\\"throwIfTimeout\\");
113+
const setProp = native.operators.get(\\"setProp\\");
114+
const getProp = native.operators.get(\\"getProp\\");
115+
let lastStatementResult = undefined;
116+
const globals = $NATIVE_STORAGE.globals;
117+
(( <globals redacted> ) => {
118+
return (() => {
119+
{
120+
{
121+
const make_alternating_stream = wrap(stream => {
122+
return {
123+
isTail: true,
124+
function: pair,
125+
functionName: \\"pair\\",
126+
arguments: [callIfFuncAndRightArgs(head, 2, 14, stream), wrap(() => ({
127+
isTail: true,
128+
function: make_alternating_stream,
129+
functionName: \\"make_alternating_stream\\",
130+
arguments: [callIfFuncAndRightArgs(negate_whole_stream, 3, 36, callIfFuncAndRightArgs(stream_tail, 4, 40, stream))],
131+
line: 2,
132+
column: 34
133+
}), \\"() => make_alternating_stream(negate_whole_stream(stream_tail(stream)))\\")],
134+
line: 2,
135+
column: 9
136+
};
137+
}, \\"function make_alternating_stream(stream) {\\\\n return pair(head(stream), () => make_alternating_stream(negate_whole_stream(stream_tail(stream))));\\\\n}\\");
138+
const negate_whole_stream = wrap(stream => {
139+
return {
140+
isTail: true,
141+
function: pair,
142+
functionName: \\"pair\\",
143+
arguments: [unaryOp(\\"-\\", callIfFuncAndRightArgs(head, 8, 17, stream), 8, 16), wrap(() => ({
144+
isTail: true,
145+
function: negate_whole_stream,
146+
functionName: \\"negate_whole_stream\\",
147+
arguments: [callIfFuncAndRightArgs(stream_tail, 8, 57, stream)],
148+
line: 8,
149+
column: 37
150+
}), \\"() => negate_whole_stream(stream_tail(stream))\\")],
151+
line: 8,
152+
column: 11
153+
};
154+
}, \\"function negate_whole_stream(stream) {\\\\n return pair(-head(stream), () => negate_whole_stream(stream_tail(stream)));\\\\n}\\");
155+
const ones = callIfFuncAndRightArgs(pair, 11, 13, 1, wrap(() => ({
156+
isTail: false,
157+
value: ones
158+
}), \\"() => ones\\"));
159+
lastStatementResult = eval(\\"callIfFuncAndRightArgs(eval_stream, 12, 0, callIfFuncAndRightArgs(make_alternating_stream, 12, 12, callIfFuncAndRightArgs(enum_stream, 12, 36, 1, 9)), 9);\\");
160+
globals.variables.set(\\"make_alternating_stream\\", {
161+
kind: \\"const\\",
162+
getValue: () => {
163+
return make_alternating_stream;
164+
}
165+
});
166+
globals.variables.set(\\"negate_whole_stream\\", {
167+
kind: \\"const\\",
168+
getValue: () => {
169+
return negate_whole_stream;
170+
}
171+
});
172+
globals.variables.set(\\"ones\\", {
173+
kind: \\"const\\",
174+
getValue: () => {
175+
return ones;
176+
}
177+
});
178+
}
179+
}
180+
return lastStatementResult;
181+
})();
182+
})();
183+
",
184+
"visualiseListResult": Array [],
185+
}
186+
`;
187+
188+
exports[`Cascading js errors work properly: expectParsedError 1`] = `
189+
Object {
190+
"alertResult": Array [],
191+
"code": "function h(p) {
192+
return head(p);
193+
}
194+
195+
h(null);",
196+
"displayResult": Array [],
197+
"errors": Array [
198+
ExceptionError {
199+
"error": [Error: head(xs) expects a pair as argument xs, but encountered null],
200+
"location": SourceLocation {
201+
"end": Position {
202+
"column": 16,
203+
"line": 2,
204+
},
205+
"start": Position {
206+
"column": 9,
207+
"line": 2,
208+
},
209+
},
210+
"severity": "Error",
211+
"type": "Runtime",
212+
},
213+
],
214+
"parsedErrors": "Line 2: Error: head(xs) expects a pair as argument xs, but encountered null",
215+
"result": undefined,
216+
"resultStatus": "error",
217+
"transpiled": "const native = $$NATIVE_STORAGE;
218+
const callIfFuncAndRightArgs = native.operators.get(\\"callIfFuncAndRightArgs\\");
219+
const boolOrErr = native.operators.get(\\"boolOrErr\\");
220+
const wrap = native.operators.get(\\"wrap\\");
221+
const unaryOp = native.operators.get(\\"unaryOp\\");
222+
const binaryOp = native.operators.get(\\"binaryOp\\");
223+
const throwIfTimeout = native.operators.get(\\"throwIfTimeout\\");
224+
const setProp = native.operators.get(\\"setProp\\");
225+
const getProp = native.operators.get(\\"getProp\\");
226+
let lastStatementResult = undefined;
227+
const globals = $NATIVE_STORAGE.globals;
228+
(( <globals redacted> ) => {
229+
return (() => {
230+
{
231+
{
232+
const h = wrap(p => {
233+
return {
234+
isTail: true,
235+
function: head,
236+
functionName: \\"head\\",
237+
arguments: [p],
238+
line: 2,
239+
column: 9
240+
};
241+
}, \\"function h(p) {\\\\n return head(p);\\\\n}\\");
242+
lastStatementResult = eval(\\"callIfFuncAndRightArgs(h, 5, 0, null);\\");
243+
globals.variables.set(\\"h\\", {
244+
kind: \\"const\\",
245+
getValue: () => {
246+
return h;
247+
}
248+
});
249+
}
250+
}
251+
return lastStatementResult;
252+
})();
253+
})();
254+
",
255+
"visualiseListResult": Array [],
256+
}
257+
`;
258+
70259
exports[`Error when accessing inherited property of object: expectParsedError 1`] = `
71260
Object {
72261
"alertResult": Array [],
@@ -3097,6 +3286,28 @@ Object {
30973286
"parsedErrors": "Line 1: Expected number on right hand side of operation, got string.",
30983287
"result": undefined,
30993288
"resultStatus": "error",
3289+
"transpiled": "const native = $$NATIVE_STORAGE;
3290+
const callIfFuncAndRightArgs = native.operators.get(\\"callIfFuncAndRightArgs\\");
3291+
const boolOrErr = native.operators.get(\\"boolOrErr\\");
3292+
const wrap = native.operators.get(\\"wrap\\");
3293+
const unaryOp = native.operators.get(\\"unaryOp\\");
3294+
const binaryOp = native.operators.get(\\"binaryOp\\");
3295+
const throwIfTimeout = native.operators.get(\\"throwIfTimeout\\");
3296+
const setProp = native.operators.get(\\"setProp\\");
3297+
const getProp = native.operators.get(\\"getProp\\");
3298+
let lastStatementResult = undefined;
3299+
const globals = $NATIVE_STORAGE.globals;
3300+
(( <globals redacted> ) => {
3301+
return (() => {
3302+
{
3303+
{
3304+
lastStatementResult = eval(\\"binaryOp(\\\\\\"*\\\\\\", 12, 'string', 1, 0);\\");
3305+
}
3306+
}
3307+
return lastStatementResult;
3308+
})();
3309+
})();
3310+
",
31003311
"visualiseListResult": Array [],
31013312
}
31023313
`;

‎src/__tests__/__snapshots__/transpiled-code.ts.snap‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22

33
exports[`Ensure no name clashes 1`] = `
44
"const native0 = $$NATIVE_STORAGE;
5-
const callIfFuncAndRightArgs0 = native.operators.get(\\"callIfFuncAndRightArgs0\\");
6-
const boolOrErr0 = native.operators.get(\\"boolOrErr0\\");
7-
const wrap90 = native.operators.get(\\"wrap90\\");
5+
const callIfFuncAndRightArgs0 = native.operators.get(\\"callIfFuncAndRightArgs\\");
6+
const boolOrErr0 = native.operators.get(\\"boolOrErr\\");
7+
const wrap90 = native.operators.get(\\"wrap\\");
88
const unaryOp = native.operators.get(\\"unaryOp\\");
99
const binaryOp = native.operators.get(\\"binaryOp\\");
1010
const throwIfTimeout = native.operators.get(\\"throwIfTimeout\\");

‎src/__tests__/interpreter-errors.ts‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -963,6 +963,43 @@ test('Type error with <number> * <nonnumber>, error line at <number>, not <nonnu
963963
*
964964
'string';
965965
`,
966-
{ chapter: 1 }
966+
{ chapter: 1, native: true }
967967
).toMatchInlineSnapshot(`"Line 1: Expected number on right hand side of operation, got string."`)
968968
})
969+
970+
test('Cascading js errors work properly 1', () => {
971+
return expectParsedError(
972+
stripIndent`
973+
function make_alternating_stream(stream) {
974+
return pair(head(stream), () => make_alternating_stream(
975+
negate_whole_stream(
976+
stream_tail(stream))));
977+
}
978+
979+
function negate_whole_stream(stream) {
980+
return pair(-head(stream), () => negate_whole_stream(stream_tail(stream)));
981+
}
982+
983+
const ones = pair(1, () => ones);
984+
eval_stream(make_alternating_stream(enum_stream(1, 9)), 9);
985+
`,
986+
{ chapter: 3, native: true }
987+
).toMatchInlineSnapshot(
988+
`"Line 8: Error: head(xs) expects a pair as argument xs, but encountered null"`
989+
)
990+
})
991+
992+
test('Cascading js errors work properly', () => {
993+
return expectParsedError(
994+
stripIndent`
995+
function h(p) {
996+
return head(p);
997+
}
998+
999+
h(null);
1000+
`,
1001+
{ chapter: 2, native: true }
1002+
).toMatchInlineSnapshot(
1003+
`"Line 2: Error: head(xs) expects a pair as argument xs, but encountered null"`
1004+
)
1005+
})

‎src/__tests__/native.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,3 +81,33 @@ test('test proper setting of variables in an outer scope', async () => {
8181
expect(result.status).toBe('finished')
8282
expect((result as Finished).value).toBe('new')
8383
})
84+
85+
test('using internal names still work', async () => {
86+
const context = mockContext(3)
87+
const result = await runInContext(
88+
stripIndent`
89+
const boolOrErr = 1;
90+
function wrap() {
91+
return boolOrErr;
92+
}
93+
wrap();
94+
`,
95+
context
96+
)
97+
expect(result.status).toBe('finished')
98+
expect((result as Finished).value).toBe(1)
99+
})
100+
101+
test('assigning a = b where b was from a previous program call works', async () => {
102+
const context = mockContext(3)
103+
const result = await runInContext(
104+
stripIndent`
105+
let b = null;
106+
b = pair;
107+
b = 1;
108+
`,
109+
context
110+
)
111+
expect(result.status).toBe('finished')
112+
expect((result as Finished).value).toBe(1)
113+
})

‎src/index.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,19 @@ export async function runInContext(
201201
value: sandboxedEval(transpiled)
202202
} as Result)
203203
} catch (error) {
204-
if (error instanceof RuntimeSourceError || error instanceof ExceptionError) {
204+
if (error instanceof RuntimeSourceError) {
205205
context.errors.push(error)
206206
return resolvedErrorPromise
207207
}
208+
if (error instanceof ExceptionError) {
209+
// if we know the location of the error, just throw it
210+
if (error.location.start.line !== -1) {
211+
context.errors.push(error)
212+
return resolvedErrorPromise
213+
} else {
214+
error = error.error // else we try to get the location from source map
215+
}
216+
}
208217
const errorStack = error.stack
209218
const match = /<anonymous>:(\d+):(\d+)/.exec(errorStack)
210219
if (match === null) {

‎src/interpreter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,7 @@ export function* apply(
619619
)
620620

621621
const loc = node ? node.loc! : constants.UNKNOWN_LOCATION
622-
if (!(e instanceof errors.RuntimeSourceError)) {
622+
if (!(e instanceof errors.RuntimeSourceError || e instanceof errors.ExceptionError)) {
623623
// The error could've arisen when the builtin called a source function which errored.
624624
// If the cause was a source error, we don't want to include the error.
625625
// However if the error came from the builtin itself, we need to handle it.

‎src/stdlib/__tests__/__snapshots__/list.ts.snap‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ Object {
2222
"type": "Runtime",
2323
},
2424
],
25-
"parsedErrors": "native:\\"Line -1: Error: head(xs) expects a pair as argument xs, but encountered null\\"
26-
interpreted:\\"Line 147: Error: head(xs) expects a pair as argument xs, but encountered null\\"",
25+
"parsedErrors": "Line 147: Error: head(xs) expects a pair as argument xs, but encountered null",
2726
"result": undefined,
2827
"resultStatus": "error",
2928
"transpiled": "const native = $$NATIVE_STORAGE;

‎src/stdlib/__tests__/list.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -524,10 +524,9 @@ describe('These tests are reporting weird line numbers, as list functions are no
524524
list_ref(list(1, 2, 3), 3);
525525
`,
526526
{ chapter: 2, native: true }
527-
).toMatchInlineSnapshot(`
528-
"native:\\"Line -1: Error: head(xs) expects a pair as argument xs, but encountered null\\"
529-
interpreted:\\"Line 147: Error: head(xs) expects a pair as argument xs, but encountered null\\""
530-
`)
527+
).toMatchInlineSnapshot(
528+
`"Line 147: Error: head(xs) expects a pair as argument xs, but encountered null"`
529+
)
531530
})
532531

533532
test('bad index error list_ref', () => {

‎src/transpiler.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ export function checkForUndefinedVariablesAndTransformAssignmentsToPropagateBack
364364
if (previousVariablesToAst.has(name)) {
365365
const lastAncestor: es.Node = ancestors[ancestors.length - 2]
366366
const { isConstant, variableLocationId } = previousVariablesToAst.get(name)!
367-
if (lastAncestor.type === 'AssignmentExpression') {
367+
if (lastAncestor.type === 'AssignmentExpression' && lastAncestor.left === identifier) {
368368
// if this is an assignment expression, we want to propagate back the change
369369
if (isConstant) {
370370
throw new ConstAssignment(identifier, name)
@@ -717,7 +717,7 @@ function getDeclarationsToAccessTranspilerInternals(): es.VariableDeclaration[]
717717
),
718718
'get'
719719
),
720-
[create.literal(name)]
720+
[create.literal(key)]
721721
)
722722
}
723723
return create.declaration(name, kind, value)

0 commit comments

Comments
 (0)