Fix references alternate values implementation, with some more test cases

3 files changed, 36 insertions, 15 deletions

diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java
index 3d37c43e..d4ddda9d 100644
--- a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java
+++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java
@@ -308,15 +308,6 @@ public class ASTReference extends SimpleNode
 
             Object result = getRootVariableValue(context);
 
-            /* a reference which has been provided an alternate value
-             * is *knowingly* potentially null and should be accepted
-             * in strict mode (except if the alternate value is null)
-             */
-            if (astAlternateValue != null && (!DuckType.asBoolean(result, false)))
-            {
-                result = astAlternateValue.value(context);
-            }
-
             if (result == null && !strictRef)
             {
                 /*
@@ -332,6 +323,11 @@ public class ASTReference extends SimpleNode
                             rsvc.getParserConfiguration().getDollarChar() + rootString, null, null, uberInfo);
                 }
 
+                if (astAlternateValue != null && (!DuckType.asBoolean(result, true)))
+                {
+                    result = astAlternateValue.value(context);
+                }
+
                 return result;
             }
 
@@ -370,10 +366,6 @@ public class ASTReference extends SimpleNode
                     }
                     previousResult = result;
                     result = jjtGetChild(i).execute(result,context);
-                    if (astAlternateValue != null && (!DuckType.asBoolean(result, checkEmpty)))
-                    {
-                        result = astAlternateValue.value(context);
-                    }
                     if (result == null && !strictRef)  // If strict and null then well catch this
                                                        // next time through the loop
                     {
@@ -440,6 +432,12 @@ public class ASTReference extends SimpleNode
                     }
                 }
 
+                // Check alternate value at the end of the evaluation
+                if (astAlternateValue != null && (!DuckType.asBoolean(result, true)))
+                {
+                    result = astAlternateValue.value(context);
+                }
+
                 return result;
             }
             catch(MethodInvocationException mie)
diff --git a/velocity-engine-core/src/test/java/org/apache/velocity/test/AlternateValuesTestCase.java b/velocity-engine-core/src/test/java/org/apache/velocity/test/AlternateValuesTestCase.java
index 877b93ac..a141c065 100644
--- a/velocity-engine-core/src/test/java/org/apache/velocity/test/AlternateValuesTestCase.java
+++ b/velocity-engine-core/src/test/java/org/apache/velocity/test/AlternateValuesTestCase.java
@@ -47,4 +47,15 @@ public class AlternateValuesTestCase extends BaseTestCase
         assertEvalEquals("<1.1>", "<${foo|1.1}>");
     }
 
+    public void testComplexEval()
+    {
+        assertEvalEquals("<no date tool>", "<${date.format('medium', $date.date)|'no date tool'}>");
+        assertEvalEquals("true", "#set($val=false)${val.toString().replace(\"false\", \"true\")|'so what'}");
+        assertEvalEquals("so what", "#set($foo='foo')${foo.contains('bar')|'so what'}");
+        assertEvalEquals("so what", "#set($val=false)${val.toString().contains('bar')|'so what'}");
+        assertEvalEquals("true", "#set($val=false)${val.toString().contains('false')|'so what'}");
+        assertEvalEquals("", "$!{null|$null}");
+        assertEvalEquals("null", "$!{null|'null'}");
+        assertEvalEquals("so what", "#set($spaces='   ')${spaces.trim()|'so what'}");
+    }
 }
diff --git a/velocity-engine-core/src/test/java/org/apache/velocity/test/StrictAlternateValuesTestCase.java b/velocity-engine-core/src/test/java/org/apache/velocity/test/StrictAlternateValuesTestCase.java
index d5c7ff4e..36fe903b 100644
--- a/velocity-engine-core/src/test/java/org/apache/velocity/test/StrictAlternateValuesTestCase.java
+++ b/velocity-engine-core/src/test/java/org/apache/velocity/test/StrictAlternateValuesTestCase.java
@@ -56,9 +56,21 @@ public class StrictAlternateValuesTestCase extends BaseTestCase
         assertEvalEquals("<foo>", "<${foo|'foo'}>");
         assertEvalEquals("bar", "#set($bar='bar')${foo|$bar}");
         assertEvalEquals("bar", "#set($bar='bar')${foo|${bar}}");
-        assertEvalException ("${foo.bar.baz()[5]|'hop'}", VelocityException.class);
+        assertEvalException("${foo.bar.baz()[5]|'hop'}", VelocityException.class);
         assertEvalEquals("{foo}", "{${foo|'foo'}}");
-        assertEvalException ("$foo", VelocityException.class);
+        assertEvalException("$foo", VelocityException.class);
+    }
+
+    public void testComplexEval()
+    {
+        assertEvalException("<${date.format('medium', $date.date)|'no date tool'}>", VelocityException.class);
+        assertEvalEquals("true", "#set($val=false)${val.toString().replace(\"false\", \"true\")|'so what'}");
+        assertEvalEquals("so what", "#set($foo='foo')${foo.contains('bar')|'so what'}");
+        assertEvalEquals("so what", "#set($val=false)${val.toString().contains('bar')|'so what'}");
+        assertEvalEquals("true", "#set($val=false)${val.toString().contains('false')|'so what'}");
+        assertEvalException("$!{null|$null}", VelocityException.class);
+        assertEvalEquals("null", "$!{null|'null'}");
+        assertEvalEquals("so what", "#set($spaces='   ')${spaces.trim()|'so what'}");
     }
 
 }