From 97a01e6ad9d3a075b1fc10c088d8a224ef15c4bf Mon Sep 17 00:00:00 2001 From: Jeremy Walker Date: Wed, 22 Jan 2025 12:44:40 +0000 Subject: [PATCH 1/2] Fix missing error message about callables --- app/javascript/interpreter/executor.ts | 9 +++++++ .../interpreter/locales/en/translation.json | 2 +- .../interpreter/interpreter.test.ts | 26 +++++++++++++------ 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/app/javascript/interpreter/executor.ts b/app/javascript/interpreter/executor.ts index 0c59b03d8c..eae7367680 100644 --- a/app/javascript/interpreter/executor.ts +++ b/app/javascript/interpreter/executor.ts @@ -218,6 +218,15 @@ export class Executor }) } const value = this.evaluate(statement.initializer).value + if (isCallable(value)) { + this.error( + 'MissingParenthesesForFunctionCall', + statement.initializer.location, + { + name: (statement.initializer as VariableExpression).name.lexeme, + } + ) + } this.environment.define(statement.name.lexeme, value) return { diff --git a/app/javascript/interpreter/locales/en/translation.json b/app/javascript/interpreter/locales/en/translation.json index 05046b4d91..cd3dbe4726 100644 --- a/app/javascript/interpreter/locales/en/translation.json +++ b/app/javascript/interpreter/locales/en/translation.json @@ -79,7 +79,7 @@ "InvalidNumberOfArgumentsWithOptionalArguments": "Expected between {{minArity} and {{maxArity}} arguments but got {{numberOfArgs}}.", "InvalidUnaryOperator": "Invalid unary operator", "LogicError": "{{message}}", - "MissingParenthesesForFunctionCall": "Did you forget the parenthesis when trying to call the function?", + "MissingParenthesesForFunctionCall": "Did you forget the parenthesis when trying to use this function?\n\nDid you mean:\n\n```{{name}}()```", "NonCallableTarget": "Can only call functions.", "OperandMustBeBoolean": "Operand must be a boolean.", "OperandMustBeNumber": "Operand must be a number.", diff --git a/test/javascript/interpreter/interpreter.test.ts b/test/javascript/interpreter/interpreter.test.ts index 0b6a99ee4e..b9d179a413 100644 --- a/test/javascript/interpreter/interpreter.test.ts +++ b/test/javascript/interpreter/interpreter.test.ts @@ -834,14 +834,12 @@ describe('frames', () => { expect(frames[0].variables).toBeEmpty() }) - test('variable', () => { - const { frames } = interpret('set x to 1') - expect(frames).toBeArrayOfSize(1) - expect(frames[0].line).toBe(1) - expect(frames[0].status).toBe('SUCCESS') - expect(frames[0].code).toBe('set x to 1') - expect(frames[0].error).toBeNil() - expect(frames[0].variables).toMatchObject({ x: 1 }) + test('not using brackets when calling a function', () => { + const { error, frames } = interpret(` + set x to y + `) + console.log(error, frames) + expect(frames[0].status).toBe('ERROR') }) }) @@ -1191,6 +1189,18 @@ describe('errors', () => { expect(error).toBeNull() }) + test('forgetting brackets', () => { + const echoFunction = (_interpreter: any, _n: any) => {} + const context = { + externalFunctions: [ + { name: 'echo', func: echoFunction, description: '' }, + ], + } + const { frames } = interpret('set x to echo', context) + console.log(frames) + expect(frames[0].status).toBe('ERROR') + }) + describe('arity', () => { describe('no optional parameters', () => { test('too many arguments', () => { From 5b97d901b9c460de0bbc0d8d1195066d549769db Mon Sep 17 00:00:00 2001 From: Jeremy Walker Date: Wed, 22 Jan 2025 13:18:01 +0000 Subject: [PATCH 2/2] Improve error messages --- app/javascript/interpreter/executor.ts | 2 +- .../interpreter/locales/en/translation.json | 6 ++-- .../interpreter/interpreter.test.ts | 33 ++++++++++++------- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/app/javascript/interpreter/executor.ts b/app/javascript/interpreter/executor.ts index eae7367680..5837a90a3e 100644 --- a/app/javascript/interpreter/executor.ts +++ b/app/javascript/interpreter/executor.ts @@ -203,7 +203,7 @@ export class Executor if (statement.expression instanceof VariableExpression) this.error('MissingParenthesesForFunctionCall', statement.location, { - expression: statement.expression, + name: statement.expression.name.lexeme, }) return result diff --git a/app/javascript/interpreter/locales/en/translation.json b/app/javascript/interpreter/locales/en/translation.json index cd3dbe4726..b096a9f541 100644 --- a/app/javascript/interpreter/locales/en/translation.json +++ b/app/javascript/interpreter/locales/en/translation.json @@ -66,9 +66,9 @@ }, "runtime": { "CouldNotEvaluateFunction": "Could not evaluate function", - "CouldNotFindFunctionWithName": "Could not find function with name.", - "CouldNotFindFunctionWithNameSuggestion": "We don't know what `{{name}}` means. Maybe you meant to use the `{{suggestion}}` function instead?", - "CouldNotFindValueWithName": "Could not find value with name '{{name}}'.", + "CouldNotFindFunctionWithName": "Jiki couldn't find a function with the name `{{name}}`.", + "CouldNotFindFunctionWithNameSuggestion": "Jiki couldn't find a function with the name `{{name}}`. Maybe you meant to use the `{{suggestion}}` function instead?", + "CouldNotFindValueWithName": "Jiki couldn't find a variable or function with name `{{name}}`.", "InfiniteLoop": "Your code ran for too long and we stopped it to prevent an infinite loop.", "InvalidBinaryExpression": "Invalid binary expression", "InvalidExpression": "Invalid expression", diff --git a/test/javascript/interpreter/interpreter.test.ts b/test/javascript/interpreter/interpreter.test.ts index b9d179a413..73b434dfa3 100644 --- a/test/javascript/interpreter/interpreter.test.ts +++ b/test/javascript/interpreter/interpreter.test.ts @@ -833,14 +833,6 @@ describe('frames', () => { expect(frames[0].error).toBeNil() expect(frames[0].variables).toBeEmpty() }) - - test('not using brackets when calling a function', () => { - const { error, frames } = interpret(` - set x to y - `) - console.log(error, frames) - expect(frames[0].status).toBe('ERROR') - }) }) describe('multiple statements', () => { @@ -1197,7 +1189,6 @@ describe('errors', () => { ], } const { frames } = interpret('set x to echo', context) - console.log(frames) expect(frames[0].status).toBe('ERROR') }) @@ -1281,7 +1272,7 @@ describe('errors', () => { expect(frames[0].code).toBe('foobor()') expect(frames[0].error).not.toBeNull() expect(frames[0].error!.message).toBe( - "We don't know what `foobor` means. Maybe you meant to use the `foobar` function instead?" + "Jiki couldn't find a function with the name `foobor`. Maybe you meant to use the `foobar` function instead?" ) expect(frames[0].error!.category).toBe('RuntimeError') expect(frames[0].error!.type).toBe( @@ -1305,7 +1296,27 @@ describe('errors', () => { expect(frames[0].code).toBe('foo') expect(frames[0].error).not.toBeNull() expect(frames[0].error!.message).toBe( - 'Did you forget the parenthesis when trying to call the function?' + 'Did you forget the parenthesis when trying to use this function?\n\nDid you mean:\n\n```foo()```' + ) + expect(frames[0].error!.category).toBe('RuntimeError') + expect(frames[0].error!.type).toBe('MissingParenthesesForFunctionCall') + expect(error).toBeNull() + }) + + test('missing parentheses within set', () => { + const context = { + externalFunctions: [ + { name: 'y', func: (_: any) => {}, description: '' }, + ], + } + const { error, frames } = interpret(`set x to y`, context) + expect(frames).toBeArrayOfSize(1) + expect(frames[0].line).toBe(1) + expect(frames[0].status).toBe('ERROR') + expect(frames[0].code).toBe('set x to y') + expect(frames[0].error).not.toBeNull() + expect(frames[0].error!.message).toBe( + 'Did you forget the parenthesis when trying to use this function?\n\nDid you mean:\n\n```y()```' ) expect(frames[0].error!.category).toBe('RuntimeError') expect(frames[0].error!.type).toBe('MissingParenthesesForFunctionCall')