From 2a8cab93987a5f20bbaaf209466583c22b0d6b42 Mon Sep 17 00:00:00 2001 From: Ad5001 Date: Tue, 18 Oct 2022 18:31:04 +0200 Subject: [PATCH] Proper error handling and reporting for expressions! Only remaining non convered case is recursive dependencies, but I'm not willing to tackle that yet. --- .../ObjectLists/Editor/CustomPropertyList.qml | 73 +++++++++++----- .../ObjectLists/Editor/Dialog.qml | 6 ++ .../ObjectLists/ObjectRow.qml | 1 - .../ad5001/LogarithmPlotter/js/expr-eval.js | 86 +++++++++++-------- .../LogarithmPlotter/js/math/expression.js | 4 + 5 files changed, 108 insertions(+), 62 deletions(-) diff --git a/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/Editor/CustomPropertyList.qml b/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/Editor/CustomPropertyList.qml index 20f3885..0bb35bb 100644 --- a/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/Editor/CustomPropertyList.qml +++ b/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/Editor/CustomPropertyList.qml @@ -36,7 +36,7 @@ import "../../js/mathlib.js" as MathLib \sa Dialog */ Repeater { - id: dlgCustomProperties + id: root signal changed() @@ -90,21 +90,52 @@ Repeater { } } onChanged: function(newValue) { - var newValue = { - 'Expression': () => new MathLib.Expression(newValue), - 'Domain': () => MathLib.parseDomain(newValue), - 'string': () => newValue, - 'number': () => parseFloat(newValue) - }[propertyType]() - // Ensuring old and new values are different to prevent useless adding to history. - if(obj[propertyName] != newValue) { - history.addToHistory(new HistoryLib.EditedProperty( - obj.name, objType, propertyName, - obj[propertyName], newValue - )) - obj[propertyName] = newValue - Objects.currentObjects[objType][objIndex].update() - objectListList.update() + try { + var newValueParsed = { + 'Expression': () => { + let expr = new MathLib.Expression(newValue) + // Check if the expression is valid, throws error otherwise. + if(!expr.allRequirementsFullfilled()) { + let undefVars = expr.undefinedVariables() + console.log(JSON.stringify(undefVars), undefVars.join(', ')) + if(undefVars.length > 1) + throw new Error(qsTranslate('error', 'No object found with names %1.').arg(undefVars.join(', '))) + else + throw new Error(qsTranslate('error', 'No object found with name %1.').arg(undefVars.join(', '))) + } + if(expr.requiredObjects().includes(obj.name)) + throw new Error(qsTranslate('error', 'Object cannot be dependent on itself.')) + // TODO: Check for recursive dependencies. + return expr + }, + 'Domain': () => MathLib.parseDomain(newValue), + 'string': () => newValue, + 'number': () => parseFloat(newValue) + }[propertyType]() + + // Ensuring old and new values are different to prevent useless adding to history. + if(obj[propertyName] != newValueParsed) { + history.addToHistory(new HistoryLib.EditedProperty( + obj.name, objType, propertyName, + obj[propertyName], newValueParsed + )) + obj[propertyName] = newValueParsed + root.changed() + } + } catch(e) { + // Error in expression or domain + parsingErrorDialog.showDialog(propertyName, newValue, e.message) + } + } + + + D.MessageDialog { + id: parsingErrorDialog + title: qsTr("LogarithmPlotter - Parsing error") + text: "" + function showDialog(propName, propValue, error) { + text = qsTr("Error while parsing expression for property %1:\n%2\n\nEvaluated expression: %3").arg(propName).arg(error).arg(propValue) + open() } } } @@ -126,8 +157,7 @@ Repeater { obj[propertyName], this.checked )) obj[propertyName] = this.checked - Objects.currentObjects[objType][objIndex].update() - objectListList.update() + root.changed() } } } @@ -187,8 +217,7 @@ Repeater { obj[propertyName] = baseModel[newIndex] } // Refreshing - Objects.currentObjects[objType][objIndex].update() - objectListList.update() + root.changed() } } } @@ -217,9 +246,7 @@ Repeater { )) //Objects.currentObjects[objType][objIndex][propertyName] = exported obj[propertyName] = exported - //Objects.currentObjects[objType][objIndex].update() - obj.update() - objectListList.update() + root.changed() } Component.onCompleted: { diff --git a/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/Editor/Dialog.qml b/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/Editor/Dialog.qml index bc6cdd7..d8c319e 100644 --- a/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/Editor/Dialog.qml +++ b/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/Editor/Dialog.qml @@ -120,6 +120,7 @@ D.Dialog { onActivated: function(newIndex) { if(idModel[newIndex] != objEditor.obj.labelContent) { objEditor.obj.labelContent = idModel[newIndex] + objEditor.obj.update() objectListList.update() } } @@ -129,6 +130,11 @@ D.Dialog { CustomPropertyList { id: dlgCustomProperties obj: objEditor.obj + + onChanged: { + obj.update() + objectListList.update() + } } } diff --git a/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/ObjectRow.qml b/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/ObjectRow.qml index ddc73eb..3937c4f 100644 --- a/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/ObjectRow.qml +++ b/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/ObjectLists/ObjectRow.qml @@ -144,7 +144,6 @@ Item { ToolTip.text: qsTr("Set %1 %2 position").arg(obj.constructor.displayType()).arg(obj.name) onClicked: { - console.log(obj.type, obj.name) posPicker.objType = obj.type posPicker.objName = obj.name posPicker.pickX = hasXProp diff --git a/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/js/expr-eval.js b/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/js/expr-eval.js index 16c3940..c615305 100644 --- a/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/js/expr-eval.js +++ b/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/js/expr-eval.js @@ -18,16 +18,16 @@ var IARRAY = 'IARRAY'; // Additional variable characters. var ADDITIONAL_VARCHARS = [ - "α","β","γ","δ","ε","ζ","η", - "π","θ","κ","λ","μ","ξ","ρ", - "ς","σ","τ","φ","χ","ψ","ω", - "Γ","Δ","Θ","Λ","Ξ","Π","Σ", - "Φ","Ψ","Ω","ₐ","ₑ","ₒ","ₓ", - "ₕ","ₖ","ₗ","ₘ","ₙ","ₚ","ₛ", - "ₜ","¹","²","³","⁴","⁵","⁶", - "⁷","⁸","⁹","⁰","₁","₂","₃", - "₄","₅","₆","₇","₈","₉","₀" - ] + "α","β","γ","δ","ε","ζ","η", + "π","θ","κ","λ","μ","ξ","ρ", + "ς","σ","τ","φ","χ","ψ","ω", + "Γ","Δ","Θ","Λ","Ξ","Π","Σ", + "Φ","Ψ","Ω","ₐ","ₑ","ₒ","ₓ", + "ₕ","ₖ","ₗ","ₘ","ₙ","ₚ","ₛ", + "ₜ","¹","²","³","⁴","⁵","⁶", + "⁷","⁸","⁹","⁰","₁","₂","₃", + "₄","₅","₆","₇","₈","₉","₀" +] function Instruction(type, value) { this.type = type; @@ -117,7 +117,11 @@ function simplify(tokens, unaryOps, binaryOps, ternaryOps, values) { newexpression.push(new Instruction(IEXPR, simplify(item.value, unaryOps, binaryOps, ternaryOps, values))); } else if (type === IMEMBER && nstack.length > 0) { n1 = nstack.pop(); - nstack.push(new Instruction(INUMBER, n1.value[item.value])); + //console.log("Getting property ", item.value, "of", n1) + if(item.value in n1.value) + nstack.push(new Instruction(INUMBER, n1.value[item.value])); + else + throw new Error(qsTranslate('error', 'Cannot find property %1 of object %2.').arg(item.value).arg(n1)) } /* else if (type === IARRAY && nstack.length >= item.value) { var length = item.value; while (length-- > 0) { @@ -217,7 +221,7 @@ function evaluate(tokens, expr, values) { if (v !== undefined) { nstack.push(v); } else { - throw new Error('undefined variable: ' + item.value); + throw new Error(qsTranslate('error', 'Undefined variable %1.').arg(item.value)); } } } else if (type === IOP1) { @@ -237,7 +241,7 @@ function evaluate(tokens, expr, values) { // Objects & expressions execution nstack.push(f.execute.apply(f, args)); } else { - throw new Error(f + ' cannot be executed'); + throw new Error(qsTranslate('error', '%1 cannot be executed.').arg(f)); } } else if (type === IFUNDEF) { // Create closure to keep references to arguments and expression @@ -270,7 +274,11 @@ function evaluate(tokens, expr, values) { nstack.push(item); } else if (type === IMEMBER) { n1 = nstack.pop(); - nstack.push(n1[item.value]); + //console.log("Getting property", item.value, "of", n1) + if(item.value in n1) + nstack.push(n1[item.value]); + else + throw new Error(qsTranslate('error', 'Cannot find property %1 of object %2.').arg(item.value).arg(n1)) } else if (type === IENDSTATEMENT) { nstack.pop(); } else if (type === IARRAY) { @@ -281,11 +289,11 @@ function evaluate(tokens, expr, values) { } nstack.push(args); } else { - throw new Error('invalid Expression'); + throw new Error(qsTranslate('error', 'Invalid expression.')); } } if (nstack.length > 1) { - throw new Error('invalid Expression (parity)'); + throw new Error(qsTranslate('error', 'Invalid expression (parity).')); } // Explicitly return zero to avoid test issues caused by -0 return nstack[0] === 0 ? 0 : resolveExpression(nstack[0], values); @@ -361,7 +369,7 @@ function expressionToString(tokens, toJS) { if (f === '?') { nstack.push('(' + n1 + ' ? ' + n2 + ' : ' + n3 + ')'); } else { - throw new Error('invalid Expression'); + throw new Error(qsTranslate('error', 'Invalid expression.')); } } else if (type === IVAR || type === IVARNAME) { nstack.push(item.value); @@ -417,7 +425,7 @@ function expressionToString(tokens, toJS) { } else if (type === IEXPR) { nstack.push('(' + expressionToString(item.value, toJS) + ')'); } else if (type === IENDSTATEMENT) ; else { - throw new Error('invalid Expression'); + throw new Error(qsTranslate('error', 'Invalid expression.')); } } if (nstack.length > 1) { @@ -605,7 +613,7 @@ TokenStream.prototype.next = function () { this.isName()) { return this.current; } else { - this.parseError('Unknown character "' + this.expression.charAt(this.pos) + '"'); + this.parseError(qsTranslate('error', 'Unknown character "%1".').arg(this.expression.charAt(this.pos))); } }; @@ -799,13 +807,13 @@ TokenStream.prototype.unescape = function (v) { // interpret the following 4 characters as the hex of the unicode code point var codePoint = v.substring(index + 1, index + 5); if (!codePointPattern.test(codePoint)) { - this.parseError('Illegal escape sequence: \\u' + codePoint); + this.parseError(qsTranslate('error', 'Illegal escape sequence: %1.').arg("\\u" + codePoint)); } buffer += String.fromCharCode(parseInt(codePoint, 16)); index += 4; break; default: - throw this.parseError('Illegal escape sequence: "\\' + c + '"'); + throw this.parseError(qsTranslate('error', 'Illegal escape sequence: %1.').arg('\\' + c)); } ++index; var backslash = v.indexOf('\\', index); @@ -1007,7 +1015,7 @@ TokenStream.prototype.getCoordinates = function () { TokenStream.prototype.parseError = function (msg) { var coords = this.getCoordinates(); - throw new Error('parse error [' + coords.line + ':' + coords.column + ']: ' + msg); + throw new Error(qsTranslate('error', 'Parse error [%1:%2]: %3').arg(coords.line).arg(coords.column).arg(msg)); }; function ParserState(parser, tokenStream, options) { @@ -1061,7 +1069,9 @@ ParserState.prototype.accept = function (type, value) { ParserState.prototype.expect = function (type, value) { if (!this.accept(type, value)) { var coords = this.tokens.getCoordinates(); - throw new Error('parse error [' + coords.line + ':' + coords.column + ']: Expected ' + (value || type)); + throw new Error(qsTranslate('error', 'Parse error [%1:%2]: %3') + .arg(coords.line).arg(coords.column) + .arg(qsTranslate('error', 'Expected %1').arg(qsTranslate('error',value) || type))); } }; @@ -1088,7 +1098,7 @@ ParserState.prototype.parseAtom = function (instr) { instr.push(new Instruction(IARRAY, argCount)); } } else { - throw new Error('unexpected ' + this.nextToken); + throw new Error(qsTranslate('error', 'Unexpected %1').arg(this.nextToken)); } }; @@ -1145,7 +1155,7 @@ ParserState.prototype.parseVariableAssignmentExpression = function (instr) { var lastInstrIndex = instr.length - 1; if (varName.type === IFUNCALL) { if (!this.tokens.isOperatorEnabled('()=')) { - throw new Error('function definition is not permitted'); + throw new Error(qsTranslate('error', 'Function definition is not permitted.')); } for (var i = 0, len = varName.value + 1; i < len; i++) { var index = lastInstrIndex - i; @@ -1159,7 +1169,7 @@ ParserState.prototype.parseVariableAssignmentExpression = function (instr) { continue; } if (varName.type !== IVAR && varName.type !== IMEMBER) { - throw new Error('expected variable for assignment'); + throw new Error(qsTranslate('error', 'Expected variable for assignment.')); } this.parseVariableAssignmentExpression(varValue); instr.push(new Instruction(IVARNAME, varName.value)); @@ -1323,21 +1333,21 @@ ParserState.prototype.parseMemberExpression = function (instr) { if (op.value === '.') { if (!this.allowMemberAccess) { - throw new Error('unexpected ".", member access is not permitted'); + throw new Error(qsTranslate('error', 'Unexpected ".": member access is not permitted')); } this.expect(TNAME); instr.push(new Instruction(IMEMBER, this.current.value)); } else if (op.value === '[') { if (!this.tokens.isOperatorEnabled('[')) { - throw new Error('unexpected "[]", arrays are disabled'); + throw new Error(qsTranslate('error', 'Unexpected "[]": arrays are disabled.')); } this.parseExpression(instr); this.expect(TBRACKET, ']'); instr.push(binaryInstruction('[')); } else { - throw new Error('unexpected symbol: ' + op.value); + throw new Error(qsTranslate('error', 'Unexpected symbol: %1.').arg(op.value)); } } }; @@ -1614,10 +1624,10 @@ function min(array) { function arrayMap(f, a) { if (typeof f !== 'function') { - throw new Error('First argument to map is not a function'); + throw new Error(qsTranslate('error', 'First argument to map is not a function.')); } if (!Array.isArray(a)) { - throw new Error('Second argument to map is not an array'); + throw new Error(qsTranslate('error', 'Second argument to map is not an array.')); } return a.map(function (x, i) { return f(x, i); @@ -1626,10 +1636,10 @@ function arrayMap(f, a) { function arrayFold(f, init, a) { if (typeof f !== 'function') { - throw new Error('First argument to fold is not a function'); + throw new Error(qsTranslate('error', 'First argument to fold is not a function.')); } if (!Array.isArray(a)) { - throw new Error('Second argument to fold is not an array'); + throw new Error(qsTranslate('error', 'Second argument to fold is not an array.')); } return a.reduce(function (acc, x, i) { return f(acc, x, i); @@ -1638,10 +1648,10 @@ function arrayFold(f, init, a) { function arrayFilter(f, a) { if (typeof f !== 'function') { - throw new Error('First argument to filter is not a function'); + throw new Error(qsTranslate('error', 'First argument to filter is not a function.')); } if (!Array.isArray(a)) { - throw new Error('Second argument to filter is not an array'); + throw new Error(qsTranslate('error', 'Second argument to filter is not an array.')); } return a.filter(function (x, i) { return f(x, i); @@ -1650,7 +1660,7 @@ function arrayFilter(f, a) { function stringOrArrayIndexOf(target, s) { if (!(Array.isArray(s) || typeof s === 'string')) { - throw new Error('Second argument to indexOf is not a string or array'); + throw new Error(qsTranslate('error', 'Second argument to indexOf is not a string or array.')); } return s.indexOf(target); @@ -1658,7 +1668,7 @@ function stringOrArrayIndexOf(target, s) { function arrayJoin(sep, a) { if (!Array.isArray(a)) { - throw new Error('Second argument to join is not an array'); + throw new Error(qsTranslate('error', 'Second argument to join is not an array.')); } return a.join(sep); @@ -1785,7 +1795,7 @@ class Parser { ); parserState.parseExpression(instr); - parserState.expect(TEOF, 'EOF'); + parserState.expect(TEOF, QT_TRANSLATE_NOOP('error','EOF')); return new Expression(instr, this); } diff --git a/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/js/math/expression.js b/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/js/math/expression.js index 88ebac6..a9b008a 100644 --- a/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/js/math/expression.js +++ b/LogarithmPlotter/qml/eu/ad5001/LogarithmPlotter/js/math/expression.js @@ -47,6 +47,10 @@ class Expression { return this.requiredObjects().every(objName => objName in C.currentObjectsByName) } + undefinedVariables() { + return this.requiredObjects().filter(objName => !(objName in C.currentObjectsByName)) + } + recache() { if(this.cached) this.cachedValue = this.calc.evaluate(C.currentObjectsByName)