diff --git a/src/main/java/com/googlecode/aviator/BaseExpression.java b/src/main/java/com/googlecode/aviator/BaseExpression.java index 5e7ac393..340c979d 100644 --- a/src/main/java/com/googlecode/aviator/BaseExpression.java +++ b/src/main/java/com/googlecode/aviator/BaseExpression.java @@ -153,7 +153,9 @@ public int compare(final VariableMeta o1, final VariableMeta o2) { List newFullNames = new ArrayList<>(fullNames.size()); for (VariableMeta meta : metas) { - newFullNames.add(meta.getName()); + // Drop the null-safe navigation marker so the public name reflects the real property path, + // e.g. "a?.b" is reported as "a.b". + newFullNames.add(meta.getName().replace("?.", ".")); } this.varFullNames = newFullNames; diff --git a/src/main/java/com/googlecode/aviator/code/CodeGenerator.java b/src/main/java/com/googlecode/aviator/code/CodeGenerator.java index 05d5df50..254b4388 100644 --- a/src/main/java/com/googlecode/aviator/code/CodeGenerator.java +++ b/src/main/java/com/googlecode/aviator/code/CodeGenerator.java @@ -91,6 +91,12 @@ public interface CodeGenerator { public void onJoinRight(Token lookahead); + public void onNullCoalesceLeft(Token lookahead); + + + public void onNullCoalesceRight(Token lookahead); + + public void onEq(Token lookahead); diff --git a/src/main/java/com/googlecode/aviator/code/LambdaGenerator.java b/src/main/java/com/googlecode/aviator/code/LambdaGenerator.java index 8aac66b1..b6ea591d 100644 --- a/src/main/java/com/googlecode/aviator/code/LambdaGenerator.java +++ b/src/main/java/com/googlecode/aviator/code/LambdaGenerator.java @@ -349,6 +349,18 @@ public void onJoinRight(final Token lookahead) { } + @Override + public void onNullCoalesceLeft(final Token lookahead) { + this.codeGenerator.onNullCoalesceLeft(lookahead); + } + + + @Override + public void onNullCoalesceRight(final Token lookahead) { + this.codeGenerator.onNullCoalesceRight(lookahead); + } + + @Override public void onEq(final Token lookahead) { diff --git a/src/main/java/com/googlecode/aviator/code/NoneCodeGenerator.java b/src/main/java/com/googlecode/aviator/code/NoneCodeGenerator.java index a8daa669..bb79a6b1 100644 --- a/src/main/java/com/googlecode/aviator/code/NoneCodeGenerator.java +++ b/src/main/java/com/googlecode/aviator/code/NoneCodeGenerator.java @@ -149,6 +149,18 @@ public void onJoinLeft(final Token lookahead) { public void onJoinRight(final Token lookahead) { + } + + @Override + public void onNullCoalesceLeft(final Token lookahead) { + + + } + + @Override + public void onNullCoalesceRight(final Token lookahead) { + + } @Override diff --git a/src/main/java/com/googlecode/aviator/code/OptimizeCodeGenerator.java b/src/main/java/com/googlecode/aviator/code/OptimizeCodeGenerator.java index ddfbfb4e..7ea1b67c 100644 --- a/src/main/java/com/googlecode/aviator/code/OptimizeCodeGenerator.java +++ b/src/main/java/com/googlecode/aviator/code/OptimizeCodeGenerator.java @@ -594,6 +594,12 @@ private void callASM(final Map variables, case Ternary_End: this.codeGen.onTernaryEnd(realToken); break; + case NullCoalesce_Left: + this.codeGen.onNullCoalesceLeft(realToken); + break; + case NullCoalesce_Right: + this.codeGen.onNullCoalesceRight(realToken); + break; } break; @@ -711,6 +717,18 @@ public void onJoinRight(final Token lookahead) { } + @Override + public void onNullCoalesceLeft(final Token lookahead) { + this.tokenList.add(new DelegateToken(lookahead, DelegateTokenType.NullCoalesce_Left)); + } + + + @Override + public void onNullCoalesceRight(final Token lookahead) { + this.tokenList.add(new DelegateToken(lookahead, DelegateTokenType.NullCoalesce_Right)); + } + + @Override public void onLe(final Token lookahead) { this.tokenList.add(new OperatorToken(lookahead, OperatorType.LE)); diff --git a/src/main/java/com/googlecode/aviator/code/asm/ASMCodeGenerator.java b/src/main/java/com/googlecode/aviator/code/asm/ASMCodeGenerator.java index e430d004..0e3a26d4 100644 --- a/src/main/java/com/googlecode/aviator/code/asm/ASMCodeGenerator.java +++ b/src/main/java/com/googlecode/aviator/code/asm/ASMCodeGenerator.java @@ -553,6 +553,37 @@ private void visitLeftBranch(final Token lookahead, final int ints, this.popOperand(); } + /** + * Null-coalescing operator "??" left operand: keep the left value if it is not null, otherwise + * evaluate the right operand. + */ + @Override + public void onNullCoalesceLeft(final Token lookahead) { + this.checkExecutionTimeout(); + // stack: [left] + this.mv.visitInsn(DUP); + this.pushOperand(); + loadEnv(); + visitLineNumber(lookahead); + this.mv.visitMethodInsn(INVOKEVIRTUAL, OBJECT_OWNER, "isNull", "(Ljava/util/Map;)Z"); + this.popOperand(2); + this.pushOperand(); + Label end = makeLabel(); + pushLabel0(end); + // left is not null: jump to end keeping left on stack, skip the right operand. + this.mv.visitJumpInsn(IFEQ, end); + this.popOperand(); + // left is null: discard it, the right operand will be evaluated next. + this.mv.visitInsn(POP); + this.popOperand(); + } + + @Override + public void onNullCoalesceRight(final Token lookahead) { + visitLineNumber(lookahead); + visitLabel(popLabel0()); + } + @Override public void onEq(final Token lookahead) { doCompareAndJump(lookahead, IFNE, OperatorType.EQ); diff --git a/src/main/java/com/googlecode/aviator/code/interpreter/InterpretCodeGenerator.java b/src/main/java/com/googlecode/aviator/code/interpreter/InterpretCodeGenerator.java index dc28d606..a34c6145 100644 --- a/src/main/java/com/googlecode/aviator/code/interpreter/InterpretCodeGenerator.java +++ b/src/main/java/com/googlecode/aviator/code/interpreter/InterpretCodeGenerator.java @@ -20,6 +20,7 @@ import com.googlecode.aviator.code.interpreter.ir.AssertTypeIR; import com.googlecode.aviator.code.interpreter.ir.AssertTypeIR.AssertTypes; import com.googlecode.aviator.code.interpreter.ir.BranchIfIR; +import com.googlecode.aviator.code.interpreter.ir.BranchIfNotNilIR; import com.googlecode.aviator.code.interpreter.ir.BranchUnlessIR; import com.googlecode.aviator.code.interpreter.ir.ClearIR; import com.googlecode.aviator.code.interpreter.ir.GotoIR; @@ -292,6 +293,21 @@ public void onJoinRight(final Token lookahead) { } } + @Override + public void onNullCoalesceLeft(final Token lookahead) { + Label label = makeLabel(); + pushLabel0(label); + this.instruments + .add(new BranchIfNotNilIR(label, new SourceInfo(this.sourceFile, lookahead.getLineNo()))); + emit(PopIR.INSTANCE); + } + + @Override + public void onNullCoalesceRight(final Token lookahead) { + Label label = popLabel0(); + visitLabel(label); + } + @Override public void onEq(final Token lookahead) { emit(OperatorIR.EQ); diff --git a/src/main/java/com/googlecode/aviator/code/interpreter/ir/BranchIfNotNilIR.java b/src/main/java/com/googlecode/aviator/code/interpreter/ir/BranchIfNotNilIR.java new file mode 100644 index 00000000..644e138c --- /dev/null +++ b/src/main/java/com/googlecode/aviator/code/interpreter/ir/BranchIfNotNilIR.java @@ -0,0 +1,61 @@ +package com.googlecode.aviator.code.interpreter.ir; + +import com.googlecode.aviator.code.interpreter.IR; +import com.googlecode.aviator.code.interpreter.InterpretContext; +import com.googlecode.aviator.runtime.type.AviatorObject; + +/** + * Branch to the target if the top operand is not null, keeping it on the stack. Used to implement + * the null-coalescing operator "??". + * + * @author dennis(killme2008@gmail.com) + * + */ +public class BranchIfNotNilIR implements IR, JumpIR { + private static final long serialVersionUID = -6217439483027456339L; + private int pc; + private final Label label; + private final SourceInfo sourceInfo; + + public BranchIfNotNilIR(final Label label, final SourceInfo sourceInfo) { + super(); + this.label = label; + this.sourceInfo = sourceInfo; + } + + public int getPc() { + return this.pc; + } + + @Override + public void setPc(final int pc) { + this.pc = pc; + } + + @Override + public Label getLabel() { + return this.label; + } + + @Override + public void eval(final InterpretContext context) { + AviatorObject top = context.peek(); + if (!top.isNull(context.getEnv())) { + context.jumpTo(this.pc); + context.dispatch(false); + } else { + context.dispatch(); + } + } + + @Override + public boolean mayBeCost() { + return true; + } + + @Override + public String toString() { + return "branch_if_not_nil " + this.pc + " [" + this.label + "] " + this.sourceInfo; + } + +} diff --git a/src/main/java/com/googlecode/aviator/lexer/ExpressionLexer.java b/src/main/java/com/googlecode/aviator/lexer/ExpressionLexer.java index 392492e9..542daf1e 100644 --- a/src/main/java/com/googlecode/aviator/lexer/ExpressionLexer.java +++ b/src/main/java/com/googlecode/aviator/lexer/ExpressionLexer.java @@ -444,9 +444,13 @@ private Token scanVariable() { } sb.append(this.peek); nextChar(); + } else if (this.peek == '?' && tryAbsorbNullSafeDot(sb)) { + // The "?." text has been appended and the lexer now points at the next + // property segment, which the next iteration consumes. + hasDot = true; } else if (hasDot && this.peek == '[' && tryAbsorbChainedIndex(sb)) { // The "[digits]" text has been appended and the lexer now points at the - // following '.', which the next iteration consumes. + // following '.' or '?', which the next iteration consumes. } else { break; } @@ -498,6 +502,32 @@ private boolean tryAbsorbChainedIndex(final StringBuilder sb) { } + /** + * Try to absorb a null-safe navigation operator "?." into the variable lexeme, e.g. the "?." in + * "a?.b". The '?' is kept as a marker on the preceding segment so that the runtime can tell which + * segments should short-circuit to null instead of throwing on a null intermediate value. + * + *

+ * Only "?." (a '?' immediately followed by '.') is absorbed. A lone '?' (ternary operator) or + * "??" (null-coalescing operator) is left untouched for the parser to handle. + * + * @param sb the variable lexeme being built, positioned at '?' + * @return true if a null-safe dot was absorbed + */ + private boolean tryAbsorbNullSafeDot(final StringBuilder sb) { + int mark = this.iterator.getIndex(); // points at '?' + nextChar(); // skip '?' + if (this.peek == '.') { + sb.append("?."); + nextChar(); // skip '.' + return true; + } + // Not a null-safe dot (e.g. ternary '?' or '??'): restore the lexer to the original '?'. + this.peek = this.iterator.setIndex(mark); + return false; + } + + /** * Scan operator character. * diff --git a/src/main/java/com/googlecode/aviator/lexer/token/DelegateToken.java b/src/main/java/com/googlecode/aviator/lexer/token/DelegateToken.java index 02ea9795..f29dbab1 100644 --- a/src/main/java/com/googlecode/aviator/lexer/token/DelegateToken.java +++ b/src/main/java/com/googlecode/aviator/lexer/token/DelegateToken.java @@ -33,7 +33,8 @@ public void setLambdaFunctionBootstrap(final LambdaFunctionBootstrap lambdaFunct public static enum DelegateTokenType { And_Left, Join_Left, Ternary_Boolean, Ternary_Left, Array, Index_Start, // Method_Name, Method_Param, Lambda_New, // - Ternary_End + Ternary_End, // + NullCoalesce_Left, NullCoalesce_Right } diff --git a/src/main/java/com/googlecode/aviator/parser/ExpressionParser.java b/src/main/java/com/googlecode/aviator/parser/ExpressionParser.java index 8472c8a0..03bb2b1f 100644 --- a/src/main/java/com/googlecode/aviator/parser/ExpressionParser.java +++ b/src/main/java/com/googlecode/aviator/parser/ExpressionParser.java @@ -241,6 +241,18 @@ public boolean parseTernary() { Token opToken = this.lookahead; if (expectChar('?')) { move(true); + if (expectChar('?')) { + // null-coalescing operator '??': right-associative, short-circuits the right operand when + // the left operand is not null. + move(true); + CodeGenerator cg = getCodeGeneratorWithTimes(); + cg.onNullCoalesceLeft(opToken); + if (!parseTernary()) { + reportSyntaxError("invalid token for '??' operator"); + } + cg.onNullCoalesceRight(opToken); + return gcTimes < this.getCGTimes; + } CodeGenerator cg = getCodeGeneratorWithTimes(); cg.onTernaryBoolean(opToken); if (!parseTernary()) { @@ -418,6 +430,11 @@ public void parseEquality() { // this.back(); // assignment + if (prevToken.getType() == TokenType.Variable && prevToken.getLexeme().contains("?.")) { + reportSyntaxError("can't assign value to a null-safe navigation expression: " + + prevToken.getLexeme()); + } + boolean isVar = false; if (prevToken.getType() == TokenType.Variable) { isVar = true; @@ -1063,18 +1080,27 @@ private boolean isValidPropertySegment(final String segment) { return false; } - int bracketIdx = segment.indexOf('['); + // A trailing '?' marks a null-safe navigation segment (the "a?.b" operator). + String seg = segment; + if (seg.charAt(seg.length() - 1) == '?') { + seg = seg.substring(0, seg.length() - 1); + if (seg.isEmpty()) { + return false; + } + } + + int bracketIdx = seg.indexOf('['); if (bracketIdx < 0) { - return isJavaIdentifier(segment); + return isJavaIdentifier(seg); } if (bracketIdx == 0) { - return segment.endsWith("]"); // "[0]" format + return seg.endsWith("]"); // "[0]" format } // "bars[0]" format - String baseName = segment.substring(0, bracketIdx); - return isJavaIdentifier(baseName) && segment.endsWith("]"); + String baseName = seg.substring(0, bracketIdx); + return isJavaIdentifier(baseName) && seg.endsWith("]"); } private void methodInvokeOrArrayAccess() { diff --git a/src/main/java/com/googlecode/aviator/utils/Reflector.java b/src/main/java/com/googlecode/aviator/utils/Reflector.java index e41daecf..05d16d39 100644 --- a/src/main/java/com/googlecode/aviator/utils/Reflector.java +++ b/src/main/java/com/googlecode/aviator/utils/Reflector.java @@ -768,8 +768,16 @@ public static Object fastGetProperty(final String name, final String[] names, final int offset, final int len) { int max = Math.min(offset + len, names.length); for (int i = offset; i < max; i++) { - String rName = AviatorJavaType.reserveName(names[i]); - rName = rName != null ? rName : names[i]; + String seg = names[i]; + // A trailing '?' marks a null-safe segment: if its value is null, the whole property + // navigation short-circuits to null instead of throwing (the "a?.b" operator). + boolean nullSafe = false; + if (!seg.isEmpty() && seg.charAt(seg.length() - 1) == '?') { + nullSafe = true; + seg = seg.substring(0, seg.length() - 1); + } + String rName = AviatorJavaType.reserveName(seg); + rName = rName != null ? rName : seg; int arrayIndex = -1; String keyIndex = null; @@ -868,6 +876,9 @@ public static Object fastGetProperty(final String name, final String[] names, target.innerEnv = null; target.targetObject = null; } else if (val == null) { + if (nullSafe) { + return null; + } throw new NullPointerException(rName); } else { target.targetObject = val; diff --git a/src/test/java/com/googlecode/aviator/NullSafeInterpretUnitTest.java b/src/test/java/com/googlecode/aviator/NullSafeInterpretUnitTest.java new file mode 100644 index 00000000..207f55be --- /dev/null +++ b/src/test/java/com/googlecode/aviator/NullSafeInterpretUnitTest.java @@ -0,0 +1,13 @@ +package com.googlecode.aviator; + +import org.junit.Before; + +public class NullSafeInterpretUnitTest extends NullSafeUnitTest { + + @Override + @Before + public void setup() { + super.setup(); + this.instance.setOption(Options.EVAL_MODE, EvalMode.INTERPRETER); + } +} diff --git a/src/test/java/com/googlecode/aviator/NullSafeUnitTest.java b/src/test/java/com/googlecode/aviator/NullSafeUnitTest.java new file mode 100644 index 00000000..23f52629 --- /dev/null +++ b/src/test/java/com/googlecode/aviator/NullSafeUnitTest.java @@ -0,0 +1,200 @@ +package com.googlecode.aviator; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import java.util.HashMap; +import java.util.Map; +import org.junit.Before; +import org.junit.Test; +import com.googlecode.aviator.exception.ExpressionRuntimeException; +import com.googlecode.aviator.exception.ExpressionSyntaxErrorException; +import com.googlecode.aviator.runtime.function.AbstractFunction; +import com.googlecode.aviator.runtime.type.AviatorObject; + +/** + * Tests for null-safe navigation ("a?.b") and the null-coalescing operator ("a ?? b"), in ASM mode. + * The interpreter mode is covered by the subclass. + */ +public class NullSafeUnitTest { + + protected AviatorEvaluatorInstance instance; + + public static class Node { + private final String name; + private final Node next; + + public Node(final String name, final Node next) { + this.name = name; + this.next = next; + } + + public String getName() { + return this.name; + } + + public Node getNext() { + return this.next; + } + } + + @Before + public void setup() { + this.instance = AviatorEvaluator.newInstance(); + this.instance.addFunction(new AbstractFunction() { + @Override + public String getName() { + return "boom"; + } + + @Override + public AviatorObject call(final Map env) { + throw new IllegalStateException("boom should not be evaluated"); + } + }); + } + + private Object exec(final String expression, final Map env) { + return this.instance.compile(expression, true).execute(env); + } + + private Object exec(final String expression) { + return exec(expression, new HashMap()); + } + + private Map nestedMapEnv() { + Map env = new HashMap<>(); + Map a = new HashMap<>(); + Map b = new HashMap<>(); + b.put("c", 42L); + a.put("b", b); + a.put("nb", null); + env.put("a", a); + env.put("nullA", null); + return env; + } + + @Test + public void testNullSafeShortCircuitOnNullRoot() { + Map env = nestedMapEnv(); + assertNull(exec("nullA?.b", env)); + assertNull(exec("nullA?.b.c", env)); + assertNull(exec("nullA?.b?.c", env)); + } + + @Test + public void testNullSafeNonNullChain() { + Map env = nestedMapEnv(); + assertEquals(42L, exec("a?.b?.c", env)); + assertEquals(42L, exec("a.b.c", env)); + assertEquals(42L, exec("a?.b.c", env)); + } + + @Test + public void testNullSafeMidChainNull() { + Map env = nestedMapEnv(); + // "nb" is null: guarding it short-circuits. + assertNull(exec("a?.nb?.c", env)); + assertNull(exec("a.nb?.c", env)); + } + + @Test + public void testUnguardedMidChainNullThrows() { + Map env = nestedMapEnv(); + // "nb" is null and not guarded, so navigating ".c" must fail. + try { + exec("a.nb.c", env); + fail("should throw"); + } catch (ExpressionRuntimeException e) { + // expected + } + try { + exec("a?.nb.c", env); + fail("should throw"); + } catch (ExpressionRuntimeException e) { + // expected + } + } + + @Test + public void testNullSafeOnJavaBean() { + Node tail = new Node("tail", null); + Node head = new Node("head", tail); + Map env = new HashMap<>(); + env.put("head", head); + env.put("nullNode", null); + + assertEquals("tail", exec("head?.next?.name", env)); + assertNull(exec("head?.next?.next?.name", env)); + assertNull(exec("nullNode?.next?.name", env)); + } + + @Test + public void testNullCoalesce() { + Map env = nestedMapEnv(); + assertEquals(10L, exec("nil ?? 10")); + assertEquals(3L, exec("3 ?? 10")); + assertEquals(42L, exec("a?.b?.c ?? 99", env)); + assertEquals(99L, exec("nullA?.b ?? 99", env)); + assertEquals(99L, exec("a?.nb ?? 99", env)); + } + + @Test + public void testNullCoalesceRightAssociative() { + assertEquals(7L, exec("nil ?? nil ?? 7")); + assertEquals(1L, exec("1 ?? nil ?? 7")); + } + + @Test + public void testNullCoalesceShortCircuit() { + // The right operand must not be evaluated when the left operand is not null. + assertEquals(1L, exec("1 ?? boom()")); + try { + exec("nil ?? boom()"); + fail("should throw"); + } catch (IllegalStateException e) { + assertEquals("boom should not be evaluated", e.getMessage()); + } + } + + @Test + public void testTernaryStillWorks() { + assertEquals(1L, exec("true ? 1 : 2")); + assertEquals(2L, exec("false ? 1 : 2")); + // ternary combined with null-coalesce + assertEquals(5L, exec("(nil ?? 5) > 0 ? nil ?? 5 : 0")); + } + + @Test + public void testAssignToNullSafeTargetIsSyntaxError() { + try { + this.instance.compile("a?.b = 1"); + fail("should throw"); + } catch (ExpressionSyntaxErrorException e) { + assertTrue(e.getMessage().contains("null-safe")); + } + } + + @Test + public void testCompileOptimizeLevel() { + // Exercise the direct code-generator path (no OptimizeCodeGenerator wrapper). + this.instance.setOption(Options.OPTIMIZE_LEVEL, AviatorEvaluator.COMPILE); + Map env = nestedMapEnv(); + assertNull(exec("nullA?.b?.c", env)); + assertEquals(42L, exec("a?.b?.c", env)); + assertEquals(10L, exec("nullA?.b ?? 10", env)); + assertEquals(1L, exec("1 ?? boom()")); + } + + @Test + public void testVariableNamesStripNullSafeMark() { + Expression exp = this.instance.compile("a?.b?.c + d"); + assertTrue(exp.getVariableFullNames().contains("a.b.c")); + assertTrue(exp.getVariableNames().contains("a")); + assertTrue(exp.getVariableNames().contains("d")); + for (String name : exp.getVariableFullNames()) { + assertTrue("leaked '?': " + name, !name.contains("?")); + } + } +} diff --git a/src/test/java/com/googlecode/aviator/parser/FakeCodeGenerator.java b/src/test/java/com/googlecode/aviator/parser/FakeCodeGenerator.java index 39bd06c4..55673569 100644 --- a/src/test/java/com/googlecode/aviator/parser/FakeCodeGenerator.java +++ b/src/test/java/com/googlecode/aviator/parser/FakeCodeGenerator.java @@ -103,6 +103,18 @@ public void onJoinRight(final Token lookahead) { } + @Override + public void onNullCoalesceLeft(final Token lookahead) { + + } + + + @Override + public void onNullCoalesceRight(final Token lookahead) { + appendToken("??"); + } + + @Override public void onTernaryBoolean(final Token lookahead) {