From f1772746cc51121e06291a3312db62000515359b Mon Sep 17 00:00:00 2001 From: aNNiMON Date: Sat, 27 Jan 2024 20:39:05 +0200 Subject: [PATCH] Improve errors displaying for container expressions --- .../ownlang/modules/std/std_range.java | 19 ++++-- .../ownlang/exceptions/TypeException.java | 6 ++ .../com/annimon/ownlang/parser/Parser.java | 26 ++++---- .../parser/ast/AssignmentExpression.java | 32 +++++++--- .../parser/ast/ContainerAccessExpression.java | 63 +++++++++++++++---- .../parser/ast/ObjectCreationExpression.java | 7 +-- .../optimization/OptimizationVisitor.java | 8 +-- .../annimon/ownlang/parser/ast/ASTHelper.java | 2 +- 8 files changed, 115 insertions(+), 48 deletions(-) diff --git a/modules/main/src/main/java/com/annimon/ownlang/modules/std/std_range.java b/modules/main/src/main/java/com/annimon/ownlang/modules/std/std_range.java index 1feb1b9..67fc30b 100644 --- a/modules/main/src/main/java/com/annimon/ownlang/modules/std/std_range.java +++ b/modules/main/src/main/java/com/annimon/ownlang/modules/std/std_range.java @@ -9,9 +9,9 @@ final class std_range implements Function { public Value execute(Value[] args) { Arguments.checkRange(1, 3, args.length); return switch (args.length) { - default -> RangeValue.of(0, getLong(args[0]), 1); case 2 -> RangeValue.of(getLong(args[0]), getLong(args[1]), 1); case 3 -> RangeValue.of(getLong(args[0]), getLong(args[1]), getLong(args[2])); + default -> RangeValue.of(0, getLong(args[0]), 1); }; } @@ -41,9 +41,13 @@ final class std_range implements Function { this.from = from; this.to = to; this.step = step; - final long base = (from < to) ? (to - from) : (from - to); + + final long base = (from < to) + ? Math.subtractExact(to, from) + : Math.subtractExact(from, to); final long absStep = (step < 0) ? -step : step; - this.size = (int) (base / absStep + (base % absStep == 0 ? 0 : 1)); + final long longSize = (base / absStep + (base % absStep == 0 ? 0 : 1)); + this.size = longSize > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) longSize; } @Override @@ -67,8 +71,9 @@ final class std_range implements Function { private boolean isIntegerRange() { if (to > 0) { return (from > Integer.MIN_VALUE && to < Integer.MAX_VALUE); + } else { + return (to > Integer.MIN_VALUE && from < Integer.MAX_VALUE); } - return (to > Integer.MIN_VALUE && from < Integer.MAX_VALUE); } @Override @@ -78,10 +83,12 @@ final class std_range implements Function { @Override public Value get(int index) { + long value = from + index * step; if (isIntegerRange()) { - return NumberValue.of((int) (from + index * step)); + return NumberValue.of((int) (value)); + } else { + return NumberValue.of(value); } - return NumberValue.of(from + (long) index * step); } @Override diff --git a/ownlang-core/src/main/java/com/annimon/ownlang/exceptions/TypeException.java b/ownlang-core/src/main/java/com/annimon/ownlang/exceptions/TypeException.java index e532307..4b4b562 100644 --- a/ownlang-core/src/main/java/com/annimon/ownlang/exceptions/TypeException.java +++ b/ownlang-core/src/main/java/com/annimon/ownlang/exceptions/TypeException.java @@ -1,8 +1,14 @@ package com.annimon.ownlang.exceptions; +import com.annimon.ownlang.util.Range; + public final class TypeException extends OwnLangRuntimeException { public TypeException(String message) { super(message); } + + public TypeException(String message, Range range) { + super(message, range); + } } diff --git a/ownlang-parser/src/main/java/com/annimon/ownlang/parser/Parser.java b/ownlang-parser/src/main/java/com/annimon/ownlang/parser/Parser.java index 271c3ee..45bc8a7 100644 --- a/ownlang-parser/src/main/java/com/annimon/ownlang/parser/Parser.java +++ b/ownlang-parser/src/main/java/com/annimon/ownlang/parser/Parser.java @@ -353,10 +353,10 @@ public final class Parser { if (lookMatch(0, TokenType.LPAREN)) { // next function call - return functionChain(new ContainerAccessExpression(expr, indices)); + return functionChain(new ContainerAccessExpression(expr, indices, getRange())); } // container access - return new ContainerAccessExpression(expr, indices); + return new ContainerAccessExpression(expr, indices, getRange()); } return expr; } @@ -532,7 +532,7 @@ public final class Parser { final BinaryExpression.Operator op = ASSIGN_OPERATORS.get(currentType); final Node expression = expression(); - return new AssignmentExpression(op, (Accessible) targetExpr, expression); + return new AssignmentExpression(op, (Accessible) targetExpr, expression, getRange(position, index)); } private Node ternary() { @@ -765,9 +765,7 @@ public final class Parser { args.add(expression()); match(TokenType.COMMA); } - final var expr = new ObjectCreationExpression(className, args); - expr.setRange(getRange(startTokenIndex, index - 1)); - return expr; + return new ObjectCreationExpression(className, args, getRange(startTokenIndex, index - 1)); } return unary(); @@ -859,12 +857,13 @@ public final class Parser { if (!match(TokenType.WORD)) return null; final List indices = variableSuffix(); + final var variable = new VariableExpression(current.text()); + variable.setRange(getRange(startTokenIndex, index - 1)); if (indices == null || indices.isEmpty()) { - final var variable = new VariableExpression(current.text()); - variable.setRange(getRange(startTokenIndex, index - 1)); return variable; + } else { + return new ContainerAccessExpression(variable, indices, variable.getRange()); } - return new ContainerAccessExpression(current.text(), indices); } private List variableSuffix() { @@ -905,15 +904,16 @@ public final class Parser { if (lookMatch(1, TokenType.WORD) && lookMatch(2, TokenType.LPAREN)) { match(TokenType.DOT); return functionChain(new ContainerAccessExpression( - strExpr, Collections.singletonList( - new ValueExpression(consume(TokenType.WORD).text()) - ))); + strExpr, + Collections.singletonList(new ValueExpression(consume(TokenType.WORD).text())), + getRange() + )); } final List indices = variableSuffix(); if (indices == null || indices.isEmpty()) { return strExpr; } - return new ContainerAccessExpression(strExpr, indices); + return new ContainerAccessExpression(strExpr, indices, getRange()); } return strExpr; } diff --git a/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/AssignmentExpression.java b/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/AssignmentExpression.java index e76c0af..ae2892c 100644 --- a/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/AssignmentExpression.java +++ b/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/AssignmentExpression.java @@ -1,22 +1,32 @@ package com.annimon.ownlang.parser.ast; +import com.annimon.ownlang.exceptions.OwnLangRuntimeException; import com.annimon.ownlang.lib.EvaluableValue; import com.annimon.ownlang.lib.Value; +import com.annimon.ownlang.util.Range; +import com.annimon.ownlang.util.SourceLocation; /** * * @author aNNiMON */ -public final class AssignmentExpression extends InterruptableNode implements Statement, EvaluableValue { +public final class AssignmentExpression extends InterruptableNode implements Statement, EvaluableValue, SourceLocation { public final Accessible target; public final BinaryExpression.Operator operation; public final Node expression; + private final Range range; - public AssignmentExpression(BinaryExpression.Operator operation, Accessible target, Node expr) { + public AssignmentExpression(BinaryExpression.Operator operation, Accessible target, Node expr, Range range) { this.operation = operation; this.target = target; this.expression = expr; + this.range = range; + } + + @Override + public Range getRange() { + return range; } @Override @@ -24,13 +34,21 @@ public final class AssignmentExpression extends InterruptableNode implements Sta super.interruptionCheck(); if (operation == null) { // Simple assignment - return target.set(expression.eval()); + return target.set(checkNonNull(expression.eval(), "Assignment expression")); } - final Node expr1 = new ValueExpression(target.get()); - final Node expr2 = new ValueExpression(expression.eval()); - return target.set(new BinaryExpression(operation, expr1, expr2).eval()); + final Node expr1 = new ValueExpression(checkNonNull(target.get(), "Assignment target")); + final Node expr2 = new ValueExpression(checkNonNull(expression.eval(), "Assignment expression")); + final Value result = new BinaryExpression(operation, expr1, expr2).eval(); + return target.set(result); } - + + private Value checkNonNull(Value value, String message) { + if (value == null) { + throw new OwnLangRuntimeException(message + " evaluates to null", range); + } + return value; + } + @Override public void accept(Visitor visitor) { visitor.visit(this); diff --git a/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/ContainerAccessExpression.java b/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/ContainerAccessExpression.java index 09b3f99..f0ddac1 100644 --- a/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/ContainerAccessExpression.java +++ b/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/ContainerAccessExpression.java @@ -1,7 +1,10 @@ package com.annimon.ownlang.parser.ast; +import com.annimon.ownlang.exceptions.OwnLangRuntimeException; import com.annimon.ownlang.exceptions.TypeException; import com.annimon.ownlang.lib.*; +import com.annimon.ownlang.util.Range; +import com.annimon.ownlang.util.SourceLocation; import java.util.List; import java.util.regex.Pattern; @@ -9,7 +12,7 @@ import java.util.regex.Pattern; * * @author aNNiMON */ -public final class ContainerAccessExpression implements Node, Accessible { +public final class ContainerAccessExpression implements Node, Accessible, SourceLocation { private static final Pattern PATTERN_SIMPLE_INDEX = Pattern.compile("^\"[a-zA-Z$_]\\w*\""); @@ -17,15 +20,13 @@ public final class ContainerAccessExpression implements Node, Accessible { public final List indices; private final boolean[] simpleIndices; private final boolean rootIsVariable; + private final Range range; - public ContainerAccessExpression(String variable, List indices) { - this(new VariableExpression(variable), indices); - } - - public ContainerAccessExpression(Node root, List indices) { + public ContainerAccessExpression(Node root, List indices, Range range) { rootIsVariable = root instanceof VariableExpression; this.root = root; this.indices = indices; + this.range = range; simpleIndices = precomputeSimpleIndices(); } @@ -33,6 +34,11 @@ public final class ContainerAccessExpression implements Node, Accessible { return rootIsVariable; } + @Override + public Range getRange() { + return range; + } + public Node getRoot() { return root; } @@ -47,11 +53,24 @@ public final class ContainerAccessExpression implements Node, Accessible { final Value container = getContainer(); final Value lastIndex = lastIndex(); return switch (container.type()) { - case Types.ARRAY -> ((ArrayValue) container).get(lastIndex); + case Types.ARRAY -> { + final ArrayValue arr = (ArrayValue) container; + final int size = arr.size(); + if (lastIndex.type() != Types.NUMBER) { + yield arr.get(lastIndex); + } else { + final int index = lastIndex.asInt(); + if (0 <= index && index < size) { + yield arr.get(index); + } else { + throw outOfBounds(index, size); + } + } + } case Types.MAP -> ((MapValue) container).get(lastIndex); case Types.STRING -> ((StringValue) container).access(lastIndex); case Types.CLASS -> ((ClassInstance) container).access(lastIndex); - default -> throw new TypeException("Array or map expected. Got " + Types.typeToString(container.type())); + default -> throw arrayOrMapExpected(container, " while accessing a container"); }; } @@ -60,14 +79,23 @@ public final class ContainerAccessExpression implements Node, Accessible { final Value container = getContainer(); final Value lastIndex = lastIndex(); switch (container.type()) { - case Types.ARRAY -> ((ArrayValue) container).set(lastIndex.asInt(), value); + case Types.ARRAY -> { + final ArrayValue arr = (ArrayValue) container; + final int size = arr.size(); + final int index = lastIndex.asInt(); + if (0 <= index && index < size) { + arr.set(index, value); + } else { + throw outOfBounds(index, size); + } + } case Types.MAP -> ((MapValue) container).set(lastIndex, value); case Types.CLASS -> ((ClassInstance) container).set(lastIndex, value); - default -> throw new TypeException("Array or map expected. Got " + container.type()); + default -> throw arrayOrMapExpected(container, " while setting a value to container"); } return value; } - + public Value getContainer() { Value container = root.eval(); final int last = indices.size() - 1; @@ -76,7 +104,7 @@ public final class ContainerAccessExpression implements Node, Accessible { container = switch (container.type()) { case Types.ARRAY -> ((ArrayValue) container).get(index.asInt()); case Types.MAP -> ((MapValue) container).get(index); - default -> throw new TypeException("Array or map expected"); + default -> throw arrayOrMapExpected(container, " while resolving a container"); }; } return container; @@ -96,6 +124,17 @@ public final class ContainerAccessExpression implements Node, Accessible { } return (MapValue) value; } + + private OwnLangRuntimeException outOfBounds(int index, int size) { + return new OwnLangRuntimeException( + "Index %d is out of bounds for array length %d".formatted(index, size), range); + } + + private TypeException arrayOrMapExpected(Value v, String message) { + return new TypeException("Array or map expected" + + (message == null ? "" : message) + + ". Got " + Types.typeToString(v.type()), range); + } @Override public void accept(Visitor visitor) { diff --git a/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/ObjectCreationExpression.java b/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/ObjectCreationExpression.java index a19bbf6..82d0e93 100644 --- a/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/ObjectCreationExpression.java +++ b/ownlang-parser/src/main/java/com/annimon/ownlang/parser/ast/ObjectCreationExpression.java @@ -11,14 +11,11 @@ public final class ObjectCreationExpression implements Node, SourceLocation { public final String className; public final List constructorArguments; - private Range range; + private final Range range; - public ObjectCreationExpression(String className, List constructorArguments) { + public ObjectCreationExpression(String className, List constructorArguments, Range range) { this.className = className; this.constructorArguments = constructorArguments; - } - - public void setRange(Range range) { this.range = range; } diff --git a/ownlang-parser/src/main/java/com/annimon/ownlang/parser/optimization/OptimizationVisitor.java b/ownlang-parser/src/main/java/com/annimon/ownlang/parser/optimization/OptimizationVisitor.java index 29a90d3..d2c2947 100644 --- a/ownlang-parser/src/main/java/com/annimon/ownlang/parser/optimization/OptimizationVisitor.java +++ b/ownlang-parser/src/main/java/com/annimon/ownlang/parser/optimization/OptimizationVisitor.java @@ -31,7 +31,7 @@ public abstract class OptimizationVisitor implements ResultVisitor { final Node exprNode = s.expression.accept(this, t); final Node targetNode = s.target.accept(this, t); if ( (exprNode != s.expression || targetNode != s.target) && (targetNode instanceof Accessible) ) { - return new AssignmentExpression(s.operation, (Accessible) targetNode, exprNode); + return new AssignmentExpression(s.operation, (Accessible) targetNode, exprNode, s.getRange()); } return s; } @@ -79,7 +79,7 @@ public abstract class OptimizationVisitor implements ResultVisitor { final AssignmentExpression newField; if (fieldExpr != field.expression) { changed = true; - newField = new AssignmentExpression(field.operation, field.target, fieldExpr); + newField = new AssignmentExpression(field.operation, field.target, fieldExpr, field.getRange()); } else { newField = field; } @@ -126,7 +126,7 @@ public abstract class OptimizationVisitor implements ResultVisitor { indices.add(node); } if (changed) { - return new ContainerAccessExpression(root, indices); + return new ContainerAccessExpression(root, indices, s.getRange()); } return s; } @@ -356,7 +356,7 @@ public abstract class OptimizationVisitor implements ResultVisitor { } if (changed) { - return new ObjectCreationExpression(s.className, args); + return new ObjectCreationExpression(s.className, args, s.getRange()); } return s; } diff --git a/ownlang-parser/src/test/java/com/annimon/ownlang/parser/ast/ASTHelper.java b/ownlang-parser/src/test/java/com/annimon/ownlang/parser/ast/ASTHelper.java index ef65b33..d078748 100644 --- a/ownlang-parser/src/test/java/com/annimon/ownlang/parser/ast/ASTHelper.java +++ b/ownlang-parser/src/test/java/com/annimon/ownlang/parser/ast/ASTHelper.java @@ -44,7 +44,7 @@ public final class ASTHelper { } public static AssignmentExpression assign(BinaryExpression.Operator op, Accessible accessible, Node expr) { - return new AssignmentExpression(op, accessible, expr); + return new AssignmentExpression(op, accessible, expr, null); } public static BinaryExpression operator(BinaryExpression.Operator op, Node left, Node right) {