Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve return values #7406

Merged
merged 3 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/javascript/interpreter/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export type RuntimeErrorType =
| 'UnexpectedReturnOutsideOfFunction'
| 'ExpectedFunctionNotFound'
| 'ExpectedFunctionHasWrongArguments'
| 'InvalidNestedFunction'

export type StaticErrorType =
| DisabledLanguageFeatureErrorType
Expand Down
13 changes: 7 additions & 6 deletions app/javascript/interpreter/expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,15 @@ export class CallExpression extends Expression {
}

public description(
result: Partial<'value' | EvaluationResultCallExpression>
result: EvaluationResultCallExpression,
includeReturnValue = true
): string {
const argsDescription = '()'
return `<code>${
this.callee.name.lexeme
}${argsDescription}</code> (which returned <code>${formatLiteral(
result.value
)}</code>)`
let desc = `<code>${this.callee.name.lexeme}${argsDescription}</code>`
if (includeReturnValue) {
desc += ` (which returned <code>${formatLiteral(result.value)}</code>)`
}
return desc
}
}

Expand Down
8 changes: 6 additions & 2 deletions app/javascript/interpreter/frames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
SetVariableStatement,
Statement,
ChangeVariableStatement,
ReturnStatement,
} from './statement'

export type FrameType = 'ERROR' | 'REPEAT' | 'EXPRESSION'
Expand Down Expand Up @@ -225,8 +226,11 @@ function describeIfStatement(frame: FrameWithResult) {
return output
}
function describeReturnStatement(frame: FrameWithResult) {
let output = `<p>This returned the value of <code>${frame.result.value.name}</code>, which in this case is <code>${frame.result.value.value}</code>.</p>`
return output
const context = frame.context as ReturnStatement
if (context === undefined) {
return ''
}
return context.description(frame.result)
}
function describeCallExpression(
frame: FrameWithResult,
Expand Down
5 changes: 3 additions & 2 deletions app/javascript/interpreter/locales/en/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"MissingDoubleQuoteToTerminateString": "Did you forget to add end quote? Maybe you meant to write:\n\n```\"{{string}}\"```",
"MissingEndAfterBlock": "Are you missing an `end` to finish the {{type}} body?",
"MissingEndOfLine": "We didn't expect `{{current}}` to appear on this line after the `{{previous}}`. {{suggestion}}",
"MissingExpression": "Expect expression.",
"MissingExpression": "Jiki doesn't know what this means.",
"MissingFieldNameOrIndexAfterLeftBracket": "Expect field name or index after '['.",
"InvalidFunctionName": "Jiki couldn't work out what function you wanted to use here.",
"MissingFunctionName": "Did you forget to give your function a name?",
Expand Down Expand Up @@ -55,7 +55,8 @@
"UnexpectedSpaceInIdentifier": "Did you accidently put a space in the middle of a variable name?",
"UnexpectedVariableExpressionAfterIf": "Did you forget to compare to things? In JikiScript the comparison (`is`, `==`, `>`, etc are always required).",
"UnexpectedVariableExpressionAfterIfWithPotentialTypo": "We were expecting some sort of comparison here. Did you mean to type `{{potential}}` instead of `{{actual}}`?",
"UnexpectedEqualsForEquality": "To say \"equals\" in JikiScript, we use two equals signs (`==`). Did you forget one?"
"UnexpectedEqualsForEquality": "To say \"equals\" in JikiScript, we use two equals signs (`==`). Did you forget one?",
"InvalidNestedFunction": "You can't create a function within a function. Did you mean to create a function outside of this one?"
},
"semantic": {
"CannotAssignToConstant": "Cannot re-assign value of constant.",
Expand Down
3 changes: 2 additions & 1 deletion app/javascript/interpreter/locales/system/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@
"VariableNotDeclared": "VariableNotDeclared: name: {{name}}",
"ExpectedFunctionHasWrongArguments": "ExpectedFunctionHasWrongArguments: name: {{name}}",
"ExpectedFunctionNotFound": "ExpectedFunctionNotFound: name: {{name}}",
"VariableNotAccessibleInFunction": "VariableNotAccessibleInFunction: name: {{name}}"
"VariableNotAccessibleInFunction": "VariableNotAccessibleInFunction: name: {{name}}",
"InvalidNestedFunction": "InvalidNestedFunction"
},
"disabledLanguageFeature": {
"ExcludeListViolation": "ExcludeListViolation: tokenType: {{tokenType}}",
Expand Down
5 changes: 4 additions & 1 deletion app/javascript/interpreter/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export class Parser {
if (this.match('REPEAT_FOREVER')) return this.repeatForeverStatement()
if (this.match('REPEAT_UNTIL_GAME_OVER'))
return this.repeatUntilGameOverStatement()
if (this.match('WHILE')) return this.whileStatement()
// if (this.match('WHILE')) return this.whileStatement()
if (this.match('FOREACH')) return this.foreachStatement()
if (this.match('DO')) return this.blockStatement('do')

Expand Down Expand Up @@ -753,6 +753,9 @@ export class Parser {
)
}

if (this.peek().type == 'FUNCTION') {
this.error('InvalidNestedFunction', this.peek().location)
}
this.error('MissingExpression', this.peek().location)
}

Expand Down
15 changes: 14 additions & 1 deletion app/javascript/interpreter/statement.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import {
EvaluationResult,
EvaluationResultChangeVariableStatement,
EvaluationResultReturnStatement,
EvaluationResultSetVariableStatement,
} from './evaluation-result'
import { Expression } from './expression'
import { Expression, LiteralExpression } from './expression'
import { SomethingWithLocation } from './interpreter'
import { Location } from './location'
import type { Token } from './token'
Expand Down Expand Up @@ -149,6 +150,18 @@ export class ReturnStatement extends Statement {
) {
super('ReturnStatement')
}
public description(result: EvaluationResultReturnStatement) {
if (result.value == undefined) {
return `<p>This exited the function.</p>`
}
if (result.value.type == 'VariableLookupExpression') {
return `<p>This returned the value of <code>${result.value.name}</code>, which in this case is <code>${result.value.value}</code>.</p>`
}
// if(result.value.type == "LiteralExpression") {
else {
return `<p>This returned <code>${result.value.value}</code>.</p>`
}
}
}

export class ForeachStatement extends Statement {
Expand Down
2 changes: 1 addition & 1 deletion app/javascript/interpreter/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export type TokenType =
| 'TO'
| 'TIMES'
| 'TRUE'
| 'WHILE'
// | 'WHILE'
| 'WITH'

// Grouping tokens
Expand Down
2 changes: 1 addition & 1 deletion test/javascript/interpreter/parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ describe('repeat', () => {
})

describe('while', () => {
test('with single statement', () => {
test.skip('with single statement', () => {
const stmts = parse(`
while something is true do
set x to 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ describe('IfStatement', () => {
set name to "Jeremy"
end
`)
console.log(error)
const actual = describeFrame(frames[0], [])
expect(actual).toBe(
`<p>This checked whether <code>true</code> was equal to <code>true</code></p>\n<p>The result was <code>true</code>.</p>`
Expand All @@ -89,7 +88,6 @@ describe('IfStatement', () => {
set name to "Jeremy"
end
`)
console.log(error)
const actual = describeFrame(frames[0], [])
expect(actual).toBe(
`<p>This checked whether both of these were true:</p><ul><li><code>true</code> was equal to <code>true</code></li><li><code>true</code> was equal to <code>true</code></li></ul><p></p>\n<p>The result was <code>true</code>.</p>`
Expand All @@ -104,7 +102,6 @@ describe('IfStatement', () => {
`,
{ externalFunctions: [getTrueFunction] }
)
console.log(frames[0])
const actual = describeFrame(frames[0], [])
expect(actual).toBe(
`<p>This checked whether <code>get_true()</code> (which returned <code>true</code>) was equal to <code>true</code></p>\n<p>The result was <code>true</code>.</p>`
Expand Down Expand Up @@ -149,11 +146,59 @@ describe('IfStatement', () => {
`,
{ externalFunctions: [getTrueFunction, getFalseFunction] }
)
console.log(frames[0])
const actual = describeFrame(frames[0], [])
expect(actual).toBe(
`<p>This checked whether both of these were true:</p><ul><li><code>get_true()</code> (which returned <code>true</code>) was equal to <code>get_true()</code> (which returned <code>true</code>)</li><li><code>get_false()</code> (which returned <code>false</code>) was equal to <code>get_false()</code> (which returned <code>false</code>)</li></ul><p></p>\n<p>The result was <code>true</code>.</p>`
)
})
})
})

describe('ReturnStatement', () => {
test('no value', () => {
const { frames } = interpret(`
function get_name do
return
end
get_name()
`)
const actual = describeFrame(frames[0], [])
expect(actual).toBe('<p>This exited the function.</p>')
})
test('variable', () => {
const { frames } = interpret(`
function get_name do
set x to 1
return x
end
get_name()
`)
const actual = describeFrame(frames[1], [])
expect(actual).toBe(
'<p>This returned the value of <code>x</code>, which in this case is <code>1</code>.</p>'
)
})
test('complex option', () => {
const { frames } = interpret(`
function get_3 do
return 3
end
function get_name do
return get_3()
end
get_name()
`)
const actual = describeFrame(frames[1], [])
expect(actual).toBe('<p>This returned <code>3</code>.</p>')
})
test('literal', () => {
const { frames } = interpret(`
function get_name do
return 1
end
get_name()
`)
const actual = describeFrame(frames[0], [])
expect(actual).toBe('<p>This returned <code>1</code>.</p>')
})
})
16 changes: 14 additions & 2 deletions test/javascript/interpreter/syntaxErrors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ describe('syntax errors', () => {
).toThrow('MissingDoToStartBlock: type: repeat')
})

test('while', () => {
test.skip('while', () => {
expect(() =>
parse(`
while x equals 1
Expand Down Expand Up @@ -330,7 +330,7 @@ describe('syntax errors', () => {
).toThrow('MissingEndAfterBlock: type: repeat')
})

test('while', () => {
test.skip('while', () => {
expect(() =>
parse(`
while x equals 1 do
Expand Down Expand Up @@ -490,4 +490,16 @@ describe('syntax errors', () => {
'MissingRightParenthesisAfterFunctionCall: function: move'
)
})

test('InvalidNestedFunction', () => {
expect(() =>
parse(`
function outer do
function inner do
return 1
end
end
`)
).toThrow('InvalidNestedFunction')
})
})
Loading