freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ddek...@apache.org
Subject [17/21] incubator-freemarker git commit: FREEMARKER-63: Lot of refinement in the API-s and implementation. #macro now creates a `TemplateDirectiveModel`, and #function now creates `TemplateFunctionModel` (though the function/method call syntax doesn't ye
Date Mon, 07 Aug 2017 22:32:19 GMT
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-core/src/main/java/org/apache/freemarker/core/model/ArgumentArrayLayout.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/ArgumentArrayLayout.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/ArgumentArrayLayout.java
index 2a34703..4e1149c 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/ArgumentArrayLayout.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/ArgumentArrayLayout.java
@@ -23,33 +23,45 @@ import org.apache.freemarker.core.util.StringToIndexMap;
 
 /**
  * {@link TemplateCallableModel} subinterfaces define a method called {@code execute}, which has an argument array
- * parameter, whose layout this class describes. Each parameter has a constant index in this array, which is the same
- * for all invocations of the same {@link TemplateCallableModel} object (regardless if there are omitted optional
- * parameters). Thus, the argument values can always be accessed at these constant indexes; no runtime name lookup is
- * needed inside the {@code execute} method of the {@link TemplateCallableModel} implementation. The
- * {@link ArgumentArrayLayout} object is usually stored a static final field of the {@link TemplateCallableModel}
- * implementation class.
+ * parameter, whose layout this class describes. The layout specifies the (minimum) array length, what's the index
+ * of which parameters, and if there are varargs parameters, in which case they must not be left {@code null}.
+ * <p>
+ * Each parameter has a constant index in this array, which is the same for all invocations of the same
+ * {@link TemplateCallableModel} object (regardless if there are omitted optional parameters). Thus, the argument
+ * values can always be accessed at these constant indexes; no runtime name lookup is needed inside the {@code
+ * execute} method of the {@link TemplateCallableModel} implementation. The {@link ArgumentArrayLayout} object is
+ * usually stored in a static final field of the {@link TemplateCallableModel} implementation class. Said constant
+ * indexes are alsi usually defined in the {@link TemplateCallableModel} implementation as static final constants
+ * (then feed into the {@link ArgumentArrayLayout}). Some {@link TemplateCallableModel} implementations, such those
+ * stand for macros and functions defined in the template, decide the layout on runtime instead. Note the less, once
+ * the {@link TemplateCallableModel} was crated, the layout is fixed.
  * <p>
  * The layout of the array is as follows:
  * <ol>
  * <li>
  *     {@link #getPredefinedPositionalArgumentCount()} elements for the predefined positional parameters. Index 0
- *     corresponds to the 1st positional parameter. For omitted parameters the corresponding array element is {@code
- *     null}.
+ *     corresponds to the 1st positional parameter, index 1 for the second, etc. For omitted parameters (optional
+ *     parameters usually) the corresponding array element is {@code null}.
  * <li>
- *     {@link #getPredefinedNamedArgumentsMap()}{@code .size()} elements for the predefined named arguments. These are at
- *     the indexes returned by {@link #getPredefinedNamedArgumentsMap()}{@code .get(String name)}. For omitted arguments
- *     the corresponding array element is {@code null}.
+ *     {@link #getPredefinedNamedArgumentsMap()}{@code .size()} elements for the predefined named arguments. These are
+ *     at the indexes returned by {@link #getPredefinedNamedArgumentsMap()}{@code .get(String name)}. Yet again, for
+ *     omitted arguments the corresponding array element is {@code null}. Within this index range reserved for the
+ *     named arguments, the {@link TemplateCallableModel} object is free to chose what index belongs to which name (as
+ *     far as two names don't share the same index).
  * <li>
- *     If there's a positional varargs argument, then one element for the positional varargs parameter, at
- *     index {@link #getPositionalVarargsArgumentIndex()}.
+ *     If there's a positional varargs argument, then 1 element for the positional varargs parameter (whose value
+ *     will be a {@link TemplateSequenceModel}), at index {@link #getPositionalVarargsArgumentIndex()}. This must not
+ *     be left {@code null} in the argument array. In case there are 0 positional varargs, the caller must set it to
+ *     an empty {@link TemplateSequenceModel} (like {@link Constants#EMPTY_SEQUENCE}).
  * <li>
- *     If there's a named varargs argument, then one element for the positional varargs parameter, at
- *     index {@link #getNamedVarargsArgumentIndex()}.
+ *     If there's a named varargs argument, then 1 element for the positional varargs parameter (whose value will be
+ *     a {@link TemplateHashModelEx2}), at index {@link #getNamedVarargsArgumentIndex()}. This must not be left
+ *     {@code null} in the argument array. In case there are 0 named varargs, the caller must set it to an empty
+ *     {@link TemplateHashModelEx2} (like {@link Constants#EMPTY_HASH}).
  * </ol>
  * <p>
- * The length of the array is {@link #getTotalLength()}}, or more, in which case the extra elements should be
- * ignored.
+ * The length of the argument array (allocated by the caller of {@code execute}) is {@link #getTotalLength()}}, or
+ * more (in case you have a longer array for reuse), in which case the extra elements should be ignored by the callee.
  * <p>
  * Instances of this class are immutable, thread-safe objects.
  */
@@ -66,14 +78,24 @@ public final class ArgumentArrayLayout {
             0, false,
             null, false);
 
-    /** Constant to be used when the {@link TemplateCallableModel} has 1 positional parameter, and no others. */
+    /**
+     * Constant to be used when the {@link TemplateCallableModel} has 1 positional parameter, and no others.
+     * (The argument array index of the single positional parameter will be 0.)
+     */
     public static final ArgumentArrayLayout SINGLE_POSITIONAL_PARAMETER = new ArgumentArrayLayout(
             1, false,
             null, false);
 
     /**
-     * Creates a new instance, or returns some of the equivalent static constants (such as {@link #PARAMETERLESS} or
-     * {@link #SINGLE_POSITIONAL_PARAMETER}).
+     * Constant to be used when the {@link TemplateCallableModel} has 1 positional varargs parameter, and no others.
+     * (The argument array index of the positional varargs parameter will be 0.)
+     *  */
+    public static final ArgumentArrayLayout POSITIONAL_VARARGS_PARAMETER_ONLY = new ArgumentArrayLayout(
+            0, true,
+            null, false);
+
+    /**
+     * Creates a new instance, or returns some of the equivalent static constants (for example {@link #PARAMETERLESS}).
      *
      * @param predefinedPositionalArgumentCount
      *         The highest allowed number of positional arguments, not counting the positional varargs argument. The
@@ -107,11 +129,11 @@ public final class ArgumentArrayLayout {
             int predefinedPositionalArgumentCount, boolean hasPositionalVarargsArgument,
             StringToIndexMap predefinedNamedArgumentsMap, boolean hasNamedVarargsArgument) {
         if ((predefinedNamedArgumentsMap == null || predefinedNamedArgumentsMap == StringToIndexMap.EMPTY)
-                && !hasPositionalVarargsArgument && !hasNamedVarargsArgument) {
+                && !hasNamedVarargsArgument) {
             if (predefinedPositionalArgumentCount == 0) {
-                return PARAMETERLESS;
+                return hasPositionalVarargsArgument ? POSITIONAL_VARARGS_PARAMETER_ONLY : PARAMETERLESS;
             }
-            if (predefinedPositionalArgumentCount == 1) {
+            if (predefinedPositionalArgumentCount == 1 && !hasPositionalVarargsArgument) {
                 return SINGLE_POSITIONAL_PARAMETER;
             }
         }
@@ -120,6 +142,11 @@ public final class ArgumentArrayLayout {
                 predefinedNamedArgumentsMap, hasNamedVarargsArgument);
     }
 
+    /**
+     * Creates a new instance. Note that the index layout rules are not internal implementation details; they can't be
+     * changed without breaking backward compatibility. Also some internal parts, such as macro definitions, depend on
+     * those rules.
+     */
     private ArgumentArrayLayout(int predefinedPositionalArgumentCount, boolean hasPositionalVarargsArgument,
             StringToIndexMap predefinedNamedArgumentsMap, boolean hasNamedVarargsArgument) {
         if (predefinedNamedArgumentsMap == null) {
@@ -165,35 +192,37 @@ public final class ArgumentArrayLayout {
      * Returns the index of the varargs argument into which positional arguments that aren't predefined are collected,
      * or -1 if there's no such varargs argument. The value of the positional varargs argument is a {@link
      * TemplateSequenceModel} that collects all positional arguments whose index would be greater than or equal to
-     * {@link #getPredefinedPositionalArgumentCount()}.
+     * {@link #getPredefinedPositionalArgumentCount()}. The value of this argument can't be {@code null}.
      *
-     * @return -1 if there's no such named argument
+     * @return -1 if there's no positional varargs argument
      */
     public int getPositionalVarargsArgumentIndex() {
         return positionalVarargsArgumentIndex;
     }
 
     /**
-     * Returns the index of the varargs argument into which named arguments that aren't predefined are collected, or -1
-     * if there's no such varargs argument. The value of the named varargs argument is a {@link TemplateHashModelEx2}
-     * with string keys that collects all the named arguments that aren't present in the {@link
-     * #getPredefinedNamedArgumentsMap()}. The iteration order of this hash follows the order in which the arguments
-     * were specified on the call site (in the template, typically).
+     * Returns the index of the varargs argument into which named arguments that aren't predefined (via {@link
+     * #getPredefinedNamedArgumentsMap()}) are collected, or -1 if there's no such varargs argument. The value of the
+     * named varargs argument is a {@link TemplateHashModelEx2} with string keys that collects all the named arguments
+     * that aren't present in the {@link #getPredefinedNamedArgumentsMap()}. The iteration order of this hash
+     * corresponds to the order in which the arguments were specified on the call site (in a template, typically).
+     * The value of this argument can't be {@code null}.
      *
-     * @return -1 if there's no such named argument
+     * @return -1 if there's no named varargs argument
      */
     public int getNamedVarargsArgumentIndex() {
         return namedVarargsArgumentIndex;
     }
 
     /**
-     * Returns the required (minimum) length of the {@code args} array that's passed to the {@code execute} method. As
-     * there's an index reserved for each predefined parameters, this length always includes the space reserved for
-     * optional parameters as well; it's not why it's said to be a minimum length. It's a minimum length because a
-     * longer array might be reused for better performance (but {@code execute} should never read those excess
-     * elements).
+     * Returns the required (minimum) length of the {@code args} array that's passed to the {@code execute} method of
+     * the {@link TemplateCallableModel} subinterface. As there's an index reserved for each predefined parameters (and
+     * a varargs parameter always counts as 1 parameter), this length always includes the space reserved for optional
+     * parameters as well; it's not why it's said to be a minimum length. It's a minimum length because a longer array
+     * might be reused for better performance (but {@code execute} should never read those excess elements).
      */
     public int getTotalLength() {
         return arrayLength;
     }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-core/src/main/java/org/apache/freemarker/core/model/CallPlace.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/CallPlace.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/CallPlace.java
deleted file mode 100644
index 749c313..0000000
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/CallPlace.java
+++ /dev/null
@@ -1,179 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.apache.freemarker.core.model;
-
-import java.io.IOException;
-import java.io.Writer;
-import java.util.IdentityHashMap;
-
-import org.apache.freemarker.core.CallPlaceCustomDataInitializationException;
-import org.apache.freemarker.core.Environment;
-import org.apache.freemarker.core.Template;
-import org.apache.freemarker.core.TemplateException;
-import org.apache.freemarker.core.util.CommonSupplier;
-
-/**
- * The place in a template from where a directive (like a macro) or function (like a method) is called;
- * <b>Do not implement this interface yourself</b>, as new methods may be added any time! Only FreeMarker itself
- * should provide implementations.
- */
-// TODO [FM3][CF] Should also replace DirectiveCallPlace
-public interface CallPlace {
-
-    // -------------------------------------------------------------------------------------------------------------
-    // Nested content:
-
-    /**
-     * Tells if there's a non-zero-length nested content. This is {@code false} for {@code <@foo />} or
-     * {@code <@foo></@foo>} or for calls inside expressions (i.e., for function calls).
-     */
-    boolean hasNestedContent();
-
-    /**
-     * The number of nested content parameters in this call (like 2 in {@code <@foo xs; k, v>...</@>}). If you want the
-     * caller to specify a predefined number of nested content parameters, then this is not interesting for you, and
-     * just pass an array of that length to {@link #executeNestedContent(TemplateModel[], Writer, Environment)}. If,
-     * however, you want to allow the caller to declare less parameters, then this is how you know how much
-     * parameters you should omit when calling {@link #executeNestedContent(TemplateModel[], Writer, Environment)}.
-     */
-    int getNestedContentParameterCount();
-
-    /**
-     * Executed the nested content; it there's none, it just does nothing.
-     *
-     * @param nestedContentParamValues
-     *         The nested content parameter values to pass to the nested content (as in {@code <@foo bar; i, j>${i},
-     *         ${j}</@foo>} there are 2 such parameters, whose value you set here), or {@code null} if there's none.
-     *         This array must be {@link #getNestedContentParameterCount()} long, or else {@link TemplateException} will thrown by
-     *         FreeMarker with a descriptive error message that tells to user the they need to declare that many nested
-     *         content parameters as the length of this array. If you want to allow the the caller to not declare some
-     *         of nested content parameters, then you have to make this array shorter according to {@link
-     *         #getNestedContentParameterCount()}.
-     */
-    void executeNestedContent(TemplateModel[] nestedContentParamValues, Writer out, Environment env)
-            throws TemplateException, IOException;
-
-    // -------------------------------------------------------------------------------------------------------------
-    // Source code info:
-
-    /**
-     * The template that contains this call.
-     */
-    Template getTemplate();
-
-    /**
-     * The 1-based column number of the first character of the directive call in the template source code, or -1 if it's
-     * not known.
-     */
-    int getBeginColumn();
-
-    /**
-     * The 1-based line number of the first character of the directive call in the template source code, or -1 if it's
-     * not known.
-     */
-    int getBeginLine();
-
-    /**
-     * The 1-based column number of the last character of the directive call in the template source code, or -1 if it's
-     * not known. If the directive has an end-tag ({@code </@...>}), then it points to the last character of that.
-     */
-    int getEndColumn();
-
-    /**
-     * The 1-based line number of the last character of the directive call in the template source code, or -1 if it's
-     * not known. If the directive has an end-tag ({@code </@...>}), then it points to the last character of that.
-     */
-    int getEndLine();
-
-    // -------------------------------------------------------------------------------------------------------------
-    // Caching:
-
-    /**
-     * Returns the custom data, or if that's {@code null}, then it creates and stores it in an atomic operation then
-     * returns it. This method is thread-safe, however, it doesn't ensure thread safe (like synchronized) access to the
-     * custom data itself. Be sure that the custom data only depends on things that get their final value during
-     * template parsing, not on runtime settings.
-     * <p>
-     * This method will block other calls while the {@code supplier} is executing, thus, the object will be
-     * <em>usually</em> created only once, even if multiple threads request the value when it's still {@code null}. It
-     * doesn't stand though when {@code providerIdentity} mismatches occur (see later). Furthermore, then it's also
-     * possible that multiple objects created by the same {@link CommonSupplier} will be in use on the same time,
-     * because of directive executions already running in parallel, and because of memory synchronization delays
-     * (hardware dependent) between the threads.
-     *
-     * @param providerIdentity
-     *         This is usually the class of the {@link TemplateDirectiveModel} that creates (and uses) the custom data,
-     *         or if you are using your own class for the custom data object (as opposed to a class from some more
-     *         generic API), then that class. This is needed as the same call place might calls different directives
-     *         depending on runtime conditions, and so it must be ensured that these directives won't accidentally read
-     *         each other's custom data, ending up with class cast exceptions or worse. In the current implementation,
-     *         if there's a {@code providerIdentity} mismatch (means, the {@code providerIdentity} object used when the
-     *         custom data was last set isn't the exactly same object as the one provided with the parameter now), the
-     *         previous custom data will be just ignored as if it was {@code null}. So if multiple directives that use
-     *         the custom data feature use the same call place, the caching of the custom data can be inefficient, as
-     *         they will keep overwriting each other's custom data. (In a more generic implementation the {@code
-     *         providerIdentity} would be a key in a {@link IdentityHashMap}, but then this feature would be slower,
-     *         while {@code providerIdentity} mismatches aren't occurring in most applications.)
-     * @param supplier
-     *         Called when the custom data wasn't yet set, to invoke its initial value. If this parameter is {@code
-     *         null} and the custom data wasn't set yet, then {@code null} will be returned. The returned value of
-     *         {@link CommonSupplier#get()} can be any kind of object, but can't be {@code null}.
-     *
-     * @return The current custom data object, or possibly {@code null} if there was no {@link CommonSupplier} provided.
-     *
-     * @throws CallPlaceCustomDataInitializationException
-     *         If the {@link CommonSupplier} had to be invoked but failed.
-     */
-    Object getOrCreateCustomData(Object providerIdentity, CommonSupplier<?> supplier)
-            throws CallPlaceCustomDataInitializationException;
-
-    /**
-     * Tells if the nested content (the body) can be safely cached, as it only depends on the template content (not on
-     * variable values and such) and has no side-effects (other than writing to the output). Examples of cases that give
-     * {@code false}: {@code <@foo>Name: } <tt>${name}</tt>{@code</@foo>},
-     * {@code <@foo>Name: <#if showIt>Joe</#if></@foo>}. Examples of cases that give {@code true}:
-     * {@code <@foo>Name: Joe</@foo>}, {@code <@foo />}. Note that we get {@code true} for no nested content, because
-     * that's equivalent to 0-length nested content.
-     * <p>
-     * This method returns a pessimistic result. For example, if it sees a custom directive call, it can't know what it
-     * does, so it will assume that it's not cacheable.
-     */
-    boolean isNestedOutputCacheable();
-
-    // -------------------------------------------------------------------------------------------------------------
-    // Miscellaneous:
-
-    /**
-     * Used solely for speed optimization (to minimize the number of
-     * {@link #getTargetJavaParameterType(int)} calls).
-     *
-     * @return -1 if no parameter has type hint
-     */
-    int getFirstTargetJavaParameterTypeIndex();
-
-    /**
-     * The type of the parameter in the target Java method; used for overloaded Java method selection.
-     * This optional information is specified by the template author in the source code.
-     *
-     * @return The desired type or {@code null} if this information wasn't specified in the template.
-     */
-    Class<?> getTargetJavaParameterType(int argIndex);
-
-}

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-core/src/main/java/org/apache/freemarker/core/model/Constants.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/Constants.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/Constants.java
index f72d3e6..5e29b6d 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/Constants.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/Constants.java
@@ -50,6 +50,8 @@ public class Constants {
     
     public static final TemplateModelIterator EMPTY_ITERATOR = new EmptyIteratorModel();
 
+    public static final TemplateModel[] EMPTY_TEMPLATE_MODEL_ARRAY = new TemplateModel[0];
+
     private static class EmptyIteratorModel implements TemplateModelIterator, Serializable {
 
         @Override

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateCallableModel.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateCallableModel.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateCallableModel.java
index 2f30981..6aa9445 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateCallableModel.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateCallableModel.java
@@ -20,10 +20,14 @@
 package org.apache.freemarker.core.model;
 
 /**
- * Super interface of {@link TemplateFunctionModel} and {@link TemplateDirectiveModel}.
+ * Super interface of {@link TemplateFunctionModel} and {@link TemplateDirectiveModel}; don' extended (or implement) it
+ * yourself!
  */
 public interface TemplateCallableModel extends TemplateModel {
 
+    /**
+     * Returns the argument array layout to use when calling the {@code execute} method.
+     */
     ArgumentArrayLayout getArgumentArrayLayout();
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java
index 3529c8c..95678d6 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java
@@ -3,35 +3,44 @@ package org.apache.freemarker.core.model;
 import java.io.IOException;
 import java.io.Writer;
 
+import org.apache.freemarker.core.CallPlace;
 import org.apache.freemarker.core.Environment;
 import org.apache.freemarker.core.TemplateException;
 
 /**
- * A {@link TemplateCallableModel} that (progressively) prints it result into the {@code out} object, instead of
- * returning a single result at the end of the execution. Many of these won't print anything, but has other
- * side-effects why it's useful for calling them, or do flow control. They are used in templates like
- * {@code <@myDirective foo=1 bar="wombat">...</@myDirective>} (or as {@code <@myDirective foo=1 bar="wombat" />} -
- * the nested content is optional).
+ * A {@link TemplateCallableModel} that progressively writes it result into the {@code out} object, instead of
+ * returning a single result at the end of the execution. Some directives won't print anything, and you call them from
+ * other side-effects, or do flow control (conditional execution or repetition of the nested content).
  * <p>
- * When called from expression context (and if the template language allows that!), the printed output will be captured,
- * and will be the return value of the call. Depending on the output format of the directive, the type of that value
- * will be {@link TemplateMarkupOutputModel} or {@link String}.
+ * They are not called from expression context, but on the top-level (that is, directly embedded into the
+ * static text). (If some future template language allows calling them from expression context, the printed output
+ * will be captured, and will be the return value of the call. Depending on the output format of the directive, the
+ * type of that value will be {@link TemplateMarkupOutputModel} or {@link String}.)
  * <p>
- * Note that {@link TemplateDirectiveModel} is a relatively low-level interface that puts more emphasis on
- * performance than on ease of implementation. TODO [FM3]: Introduce a more convenient way for implementing directives.
+ * Example usage in a template: {@code <@my.menu style="foo" expand=true>...</@my.menu>},
+ * {@code <@my.menuItem "Some title" icon="some.jpg" />}.
  */
 public interface TemplateDirectiveModel extends TemplateCallableModel {
 
     /**
+     * Invokes the directive.
+     *
      * @param args
      *         The of argument values. Not {@code null}. If a parameter was omitted on the caller side, the
-     *         corresponding array element will be {@code null}. For the indexed of arguments, see argument array layout
-     *         in the {@link TemplateCallableModel} documentation.
+     *         corresponding array element will be {@code null}. The length of the array and the indexes
+     *         correspont to the {@link ArgumentArrayLayout} returned by {@link #getArgumentArrayLayout()}.
+     *         If the caller doesn't keep argument layout rules (such as the array is shorter than
+     *         {@link ArgumentArrayLayout#getTotalLength()}, or the type of the values at
+     *         {@link ArgumentArrayLayout#getPositionalVarargsArgumentIndex()} or at
+     *         {@link ArgumentArrayLayout#getNamedVarargsArgumentIndex()} is improper), this method may
+     *         throws {@link IndexOutOfBoundsException} or {@link ClassCastException}. Thus, user Java code
+     *         that wishes to call {@link TemplateCallableModel}-s is responsible to ensure that the argument array
+     *         follows the layout described be {@link ArgumentArrayLayout}, as the {@code execute} method
+     *         isn't meant to do validations on that level.
      * @param callPlace
      *         The place (in a template, normally) where this directive was called from. Not {@code null}. Note that
      *         {@link CallPlace#executeNestedContent(TemplateModel[], Writer, Environment)} can be used to execute the
-     *         nested content. If the directive doesn't support nested content, it should check {@link
-     *         CallPlace#hasNestedContent()} that return {@code false}, and otherwise throw exception.
+     *         nested content.
      * @param out
      *         Print the output here (if there's any)
      * @param env
@@ -47,14 +56,14 @@ public interface TemplateDirectiveModel extends TemplateCallableModel {
 
     /**
      * Tells if this directive supports having nested content. If {@code false}, yet the caller specifies a non-empty
-     * (strictly 0-length, not even whitespace is allowed), FreeMarker will throw a {@link TemplateException} with
+     * nested content (non-0-length, even whitespace matters), FreeMarker will throw a {@link TemplateException} with
      * descriptive error message, and {@link #execute(TemplateModel[], CallPlace, Writer, Environment)} won't be called.
      * If {@code true}, the author of the directive shouldn't forget calling {@link
      * CallPlace#executeNestedContent(TemplateModel[], Writer, Environment)}, unless the intent was to skip the nested
      * content. (This property was added to prevent the frequent oversight (in FreeMarker 2) where a directive that
      * isn't supposed to have nested content doesn't examine if there's a nested content to throw an exception in that
-     * case. Then if there's nested content, it will be silently skipped during execution, as the directive never
-     * calls {@link CallPlace#executeNestedContent(TemplateModel[], Writer, Environment)}.)
+     * case. Then if there's nested content, it will be silently skipped during execution, as the directive never calls
+     * {@link CallPlace#executeNestedContent(TemplateModel[], Writer, Environment)}.)
      */
     boolean isNestedContentSupported();
 

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateFunctionModel.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateFunctionModel.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateFunctionModel.java
index 2ddfca0..6b42550 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateFunctionModel.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateFunctionModel.java
@@ -1,25 +1,38 @@
 package org.apache.freemarker.core.model;
 
+import java.io.Writer;
+
+import org.apache.freemarker.core.CallPlace;
 import org.apache.freemarker.core.Environment;
-import org.apache.freemarker.core.ProcessingConfiguration;
 import org.apache.freemarker.core.TemplateException;
-import org.apache.freemarker.core.outputformat.OutputFormat;
 
 /**
- * A {@link TemplateCallableModel}, which returns its result as a {@link TemplateModel} at the end of the execution.
- * This is in contrast with {@link TemplateDirectiveModel}, which writes its result progressively to the output.
- *
- * <p>Some template languages may allow function calls directly embedded into static text, as in
- * <code>text#f()text</code>. In that case, the language has to ensure that the return value is formatted according
- * the {@link ProcessingConfiguration} settings (such as {@link ProcessingConfiguration#getNumberFormat()} and
- * {@link ProcessingConfiguration#getDateFormat()}), and is printed to the output after escaping according the
- * {@link OutputFormat} of the context. Some template languages instead require using an explicit expression value
- * printer statement, as in <code>text${f()}text</code>.
+ * A {@link TemplateCallableModel}, which returns its result as a {@link TemplateModel} at the end of its execution.
+ * This is in contrast with {@link TemplateDirectiveModel}, which writes its result progressively to the output, if it
+ * has output at all. Also, {@link TemplateFunctionModel}-s can only be called where an expression is expected. (If
+ * some future template languages allows calling functions outside expression context, on the top-level, then
+ * that's a shorthand to doing that in with interpolation, like {@code ${f()}}.)
+ * <p>
+ * Example usage in templates: {@code < a href="${my.toProductURL(product.id)}">},
+ * {@code <#list my.groupByFirstLetter(products, property="name") as productGroup>}
  */
 public interface TemplateFunctionModel extends TemplateCallableModel {
 
-    TemplateModel execute(
-            TemplateModel[] args, Environment env, CallPlace callPlace)
-            throws TemplateException;
+    /**
+     * Invokes the function.
+     *
+     * @param args
+     *         See the similar parameter of {@link TemplateDirectiveModel#execute(TemplateModel[], CallPlace, Writer,
+     *         Environment)}
+     * @param callPlace
+     *         See the similar parameter of {@link TemplateDirectiveModel#execute(TemplateModel[], CallPlace, Writer,
+     *         Environment)}
+     * @param env
+     *         See the similar parameter of {@link TemplateDirectiveModel#execute(TemplateModel[], CallPlace, Writer,
+     *         Environment)}
+     *
+     * @return The return value of the function.
+     */
+    TemplateModel execute(TemplateModel[] args, CallPlace callPlace, Environment env) throws TemplateException;
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-core/src/main/java/org/apache/freemarker/core/util/DuplicateStringKeyException.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/util/DuplicateStringKeyException.java b/freemarker-core/src/main/java/org/apache/freemarker/core/util/DuplicateStringKeyException.java
new file mode 100644
index 0000000..a598694
--- /dev/null
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/util/DuplicateStringKeyException.java
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.freemarker.core.util;
+
+/**
+ * A string key is duplicated in something where keys must be unique. For example thrown by {@link StringToIndexMap}
+ * creator methods.
+ */
+public class DuplicateStringKeyException extends IllegalArgumentException {
+
+    private final String key;
+
+    public DuplicateStringKeyException(String key) {
+        super("Duplicate key: " + _StringUtil.jQuote(key));
+        this.key = key;
+    }
+
+    public String getKey() {
+        return key;
+    }
+}

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-core/src/main/java/org/apache/freemarker/core/util/FTLUtil.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/util/FTLUtil.java b/freemarker-core/src/main/java/org/apache/freemarker/core/util/FTLUtil.java
index 647ad90..2923647 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/util/FTLUtil.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/util/FTLUtil.java
@@ -25,10 +25,12 @@ import org.apache.freemarker.core.Environment;
 import org.apache.freemarker.core._CoreAPI;
 import org.apache.freemarker.core.model.AdapterTemplateModel;
 import org.apache.freemarker.core.model.TemplateBooleanModel;
+import org.apache.freemarker.core.model.TemplateCallableModel;
 import org.apache.freemarker.core.model.TemplateCollectionModel;
 import org.apache.freemarker.core.model.TemplateCollectionModelEx;
 import org.apache.freemarker.core.model.TemplateDateModel;
 import org.apache.freemarker.core.model.TemplateDirectiveModel;
+import org.apache.freemarker.core.model.TemplateFunctionModel;
 import org.apache.freemarker.core.model.TemplateHashModel;
 import org.apache.freemarker.core.model.TemplateHashModelEx;
 import org.apache.freemarker.core.model.TemplateMarkupOutputModel;
@@ -714,10 +716,6 @@ public final class FTLUtil {
                 appendTemplateModelTypeName(sb, typeNamesAppended, primaryInterface);
             }
 
-            if (_CoreAPI.isMacroOrFunction(tm)) {
-                appendTypeName(sb, typeNamesAppended, _CoreAPI.isFunction(tm) ? "function" : "macro");
-            }
-
             appendTemplateModelTypeName(sb, typeNamesAppended, tm.getClass());
 
             String javaClassName;
@@ -745,6 +743,31 @@ public final class FTLUtil {
     }
 
     /**
+     * Return the template language type name of callable class, as it should be shown in error messages.
+     *
+     * @param callable
+     *         Can't be {@code null}.
+     */
+    public static String getCallableTypeName(TemplateCallableModel callable) {
+        _NullArgumentException.check("callable", callable);
+
+        String result = "callable-of-unknown-kind";
+
+        String d = null;
+        if (callable instanceof TemplateDirectiveModel) {
+            d = _CoreAPI.isMacro(callable.getClass()) ? "macro" : "directive";
+            result = d;
+        }
+
+        if (callable instanceof TemplateFunctionModel) {
+            String f = "function"; // TODO [FM3][CF] Should say "method" sometimes
+            result = d == null ? f : d + "+" + f;
+        }
+
+        return result;
+    }
+
+    /**
      * Returns the {@link TemplateModel} interface that is the most characteristic of the object, or {@code null}.
      */
     private static Class getPrimaryTemplateModelInterface(TemplateModel tm) {
@@ -771,8 +794,13 @@ public final class FTLUtil {
             appendTypeName(sb, typeNamesAppended, "node");
         }
 
-        if (TemplateDirectiveModel.class.isAssignableFrom(cl)) {
-            appendTypeName(sb, typeNamesAppended, "directive");
+        if (TemplateCallableModel.class.isAssignableFrom(cl)) {
+            if (TemplateDirectiveModel.class.isAssignableFrom(cl)) {
+                appendTypeName(sb, typeNamesAppended, _CoreAPI.isMacro(cl) ? "macro" : "directive");
+            }
+            if (TemplateFunctionModel.class.isAssignableFrom(cl)) {
+                appendTypeName(sb, typeNamesAppended, "function"); // TODO [FM3][CF] should say "method" sometimes
+            }
         }
 
         if (TemplateSequenceModel.class.isAssignableFrom(cl)) {

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-core/src/main/java/org/apache/freemarker/core/util/StringToIndexMap.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/util/StringToIndexMap.java b/freemarker-core/src/main/java/org/apache/freemarker/core/util/StringToIndexMap.java
index 9edb980..65e8153 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/util/StringToIndexMap.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/util/StringToIndexMap.java
@@ -102,9 +102,14 @@ public final class StringToIndexMap {
     }
 
     /**
-     * @param entries Contains the entries that will be copied into the created object.
-     * @param entriesLength The method assumes that we only have this many elements; the {@code entries} parameter
-     *                      array might be longer than this.
+     * @param entries
+     *         Contains the entries that will be copied into the created object.
+     * @param entriesLength
+     *         The method assumes that we only have this many elements; the {@code entries} parameter array might be
+     *         longer than this.
+     *
+     * @throws DuplicateStringKeyException
+     *         if the same key occurs twice
      */
     public static StringToIndexMap of(Entry[] entries, int entriesLength) {
         return entriesLength == 0 ? EMPTY
@@ -281,7 +286,7 @@ public final class StringToIndexMap {
     private static void checkNameUnique(Entry entry, String key) {
         while (entry != null) {
             if (entry.key.equals(key)) {
-                throw new IllegalArgumentException("Duplicate key: " + _StringUtil.jQuote(key));
+                throw new DuplicateStringKeyException(key);
             }
             entry = entry.nextInSameBucket;
         }
@@ -311,6 +316,22 @@ public final class StringToIndexMap {
         }
     }
 
+    /** Returns the key for a value, or {@code null} if the value wasn't found; this is a slow (O(N)) operation! */
+    public String getKeyOfValue(int value) {
+        if (buckets == null) {
+            return null;
+        }
+        for (Entry entry : buckets) {
+            while (entry != null) {
+                if (entry.getValue() == value) {
+                    return entry.getKey();
+                }
+                entry = entry.nextInSameBucket;
+            }
+        }
+        return null;
+    }
+
     /*
     // Code used to see how well the elements are spread among the buckets:
 

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-core/src/main/javacc/FTL.jj
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/javacc/FTL.jj b/freemarker-core/src/main/javacc/FTL.jj
index 78cb1fc..5d1b354 100644
--- a/freemarker-core/src/main/javacc/FTL.jj
+++ b/freemarker-core/src/main/javacc/FTL.jj
@@ -947,7 +947,7 @@ TOKEN:
     <DOLLAR_INTERPOLATION_OPENING : "${"> { startInterpolation(matchedToken); }
 }
 
-<FM_EXPRESSION, IN_PAREN, NAMED_PARAMETER_EXPRESSION> SKIP :
+<FM_EXPRESSION, IN_PAREN> SKIP :
 {
     < ( " " | "\t" | "\n" | "\r" )+ >
     |
@@ -967,12 +967,11 @@ TOKEN:
     < "-->" | "--]">
     {
         if (parenthesisNesting > 0) SwitchTo(IN_PAREN);
-        else if (inNamedParameterExpression) SwitchTo(NAMED_PARAMETER_EXPRESSION);
         else SwitchTo(FM_EXPRESSION);
     }
 }
 
-<FM_EXPRESSION, IN_PAREN, NO_SPACE_EXPRESSION, NAMED_PARAMETER_EXPRESSION> TOKEN :
+<FM_EXPRESSION, IN_PAREN, NO_SPACE_EXPRESSION> TOKEN :
 {
     <#ESCAPED_CHAR :
         "\\"
@@ -1093,8 +1092,7 @@ TOKEN:
     {
         --parenthesisNesting;
         if (parenthesisNesting == 0) {
-            if (inNamedParameterExpression) SwitchTo(NAMED_PARAMETER_EXPRESSION);
-            else SwitchTo(FM_EXPRESSION);
+            SwitchTo(FM_EXPRESSION);
         }
     }
     |
@@ -1290,7 +1288,7 @@ TOKEN:
     <#ASCII_DIGIT: ["0" - "9"]>
 }
 
-<FM_EXPRESSION, NO_SPACE_EXPRESSION, NAMED_PARAMETER_EXPRESSION> TOKEN :
+<FM_EXPRESSION, NO_SPACE_EXPRESSION> TOKEN :
 {
     <DIRECTIVE_END : ">">
     {
@@ -1323,11 +1321,6 @@ TOKEN:
     <TERMINATING_WHITESPACE :  (["\n", "\r", "\t", " "])+> : FM_EXPRESSION
 }
 
-<NAMED_PARAMETER_EXPRESSION> TOKEN :
-{
-    <TERMINATING_EXCLAM : "!" (["\n", "\r", "\t", " "])+> : FM_EXPRESSION
-}
-
 <NO_PARSE> TOKEN :
 {
     <TERSE_COMMENT_END : "-->" | "--]">
@@ -1409,7 +1402,7 @@ ASTExpression PrimaryExpression() :
         exp = ASTExpBuiltInVariable()
     )
     (
-        LOOKAHEAD(<DOT> | <OPEN_BRACKET> |<OPEN_PAREN> | <BUILT_IN> | <EXCLAM> | <TERMINATING_EXCLAM> | <EXISTS>)
+        LOOKAHEAD(<DOT> | <OPEN_BRACKET> |<OPEN_PAREN> | <BUILT_IN> | <EXCLAM> | <EXISTS>)
         exp = AddSubExpression(exp)
     )*
     {
@@ -1864,20 +1857,14 @@ ASTExpression DefaultTo(ASTExpression exp) :
     Token t;
 }
 {
+    t = <EXCLAM>
     (
-        t = <TERMINATING_EXCLAM>
+        LOOKAHEAD(<ID><ASSIGNMENT_EQUALS>) { /* Do not consume */ }
         |
-        (
-            t = <EXCLAM>
-            (
-                LOOKAHEAD(<ID><ASSIGNMENT_EQUALS>) { /* Do not consume */ }
-                |
-                [
-                    LOOKAHEAD(ASTExpression())
-                    rhs = ASTExpression()
-                ]
-            )
-        )
+        [
+            LOOKAHEAD(ASTExpression())
+            rhs = ASTExpression()
+        ]
     )
     {
         ASTExpDefault result = new ASTExpDefault(exp, rhs);
@@ -2583,7 +2570,7 @@ ASTDirStop Stop() :
 ASTElement Nested() :
 {
     Token t, end;
-    ArrayList bodyParameters;
+    ArrayList<ASTExpression> nestedContentParams;
     ASTDirNested result = null;
 }
 {
@@ -2598,10 +2585,10 @@ ASTElement Nested() :
         |
         (
             t = <NESTED>
-            bodyParameters = PositionalArgs()
+            nestedContentParams = PositionalArgs()
             end = LooseDirectiveEnd()
             {
-                result = new ASTDirNested(bodyParameters);
+                result = new ASTDirNested(nestedContentParams);
                 result.setLocation(template, t, end);
             }
         )
@@ -2881,112 +2868,220 @@ ASTDirImport Import() :
     }
 }
 
-ASTDirMacro Macro() :
+ASTDirMacroOrFunction Macro() :
 {
-    Token arg, start, end;
-    ASTExpression nameExp;
+    Token tk = null, tk2, start, end;
+    ASTExpression exp;
+
+    Token paramNameTk;
+    Token commaTk = null;
+
+    boolean isFunction;
     String name;
-    ArrayList argNames = new ArrayList();
-    HashMap args = new HashMap();
-    ArrayList defNames = new ArrayList();
-    ASTExpression defValue = null;
+    List<ASTDirMacroOrFunction.ParameterDefinition> predefPosParamDefs = null;
+    ASTDirMacroOrFunction.ParameterDefinition posVarargsParamDef = null;
+    List<ASTDirMacroOrFunction.ParameterDefinition> predefNamedParamDefs = null;
+    ASTDirMacroOrFunction.ParameterDefinition namedVarargsParamDef = null;
+    TemplateElements children;
+
+    boolean isPositional;
+    ASTExpression defaultValueExp = null;
+    boolean isVarargs = false;
+
+    boolean isFirstParam = true;
+    boolean hasOpenParen;
+    boolean hasPendingComma = false;
+
     List lastIteratorBlockContexts;
     int lastBreakableDirectiveNesting;
-    TemplateElements children;
-    boolean isFunction = false, hasDefaults = false;
-    boolean isCatchAll = false;
-    String catchAll = null;
-    boolean hasOpenParen = false;
-    boolean hasComma = false;
-    boolean hasCloseParen = false;
 }
 {
     (
-        start = <MACRO>
+        start = <MACRO> { isFunction = false; }
         |
         start = <FUNCTION> { isFunction = true; }
     )
     {
         if (inMacro || inFunction) {
-            throw new ParseException("Macros cannot be nested.", template, start);
+            throw new ParseException("Macro or function definitions can't be nested into each other.", template, start);
         }
         if (isFunction) inFunction = true; else inMacro = true;
     }
-    nameExp = IdentifierOrStringLiteral()
+
+    exp = IdentifierOrStringLiteral()
     {
-        name = (nameExp instanceof ASTExpStringLiteral)
-                ? ((ASTExpStringLiteral) nameExp).getAsString()
-                : ((ASTExpVariable) nameExp).getName();
+        name = (exp instanceof ASTExpStringLiteral)
+                ? ((ASTExpStringLiteral) exp).getAsString()
+                : ((ASTExpVariable) exp).getName();
     }
-    [<OPEN_PAREN> { hasOpenParen = true; }]
+
+    { hasOpenParen = false; }
+    [ tk = <OPEN_PAREN> { hasOpenParen = true; } ]
     {
         if (hasOpenParen != isFunction) {
-    		throw new ParseException(
-                    isFunction ? "#function must use \"(\" after the function name."
-                            : "#macro can't use \"(\" after the macro name.",
-                            template, start);
+            if (isFunction) {
+                throw new ParseException("#function must use \"(\" after the function name.", template, start);
+            } else {
+    		    throw new ParseException("#macro can't use \"(\" after the macro name.", template, tk);
+            }
         }
     }
+
+
     (
-        arg = <ID> {
-            defValue = null;
-            if (!argNames.isEmpty() && hasComma != isFunction) {
-                throw new ParseException(
-                        isFunction ? "#function must have comma between the declared parameters."
-                                : "#macro must not have comma between the declared parameters.",
-                                template, start);
+        paramNameTk = <ID>
+
+        {
+            // Function parameters are positional by default, while macro parameters are named by default.
+            isPositional = isFunction;
+        }
+        [
+            <OPENING_CURLY_BRACKET>
+            // TODO [FM3] Support multiple options inside the {}, and value assignment to them (`=true` is implied).
+            [
+                tk = <ID>
+                {
+                    switch (tk.image) {
+                        case ASTDirMacroOrFunction.NAMED_PARAMETER_OPTION_NAME:
+                            isPositional = false;
+                            break;
+                        case ASTDirMacroOrFunction.POSITIONAL_PARAMETER_OPTION_NAME:
+                            isPositional = true;
+                            break;
+                        default:
+                            throw new ParseException(
+                                    "Unsupported parameter option " + _StringUtil.jQuote(tk.image) + ". "
+                                    + "The supported options are: \""
+                                    + ASTDirMacroOrFunction.NAMED_PARAMETER_OPTION_NAME + "\", \""
+                                    + ASTDirMacroOrFunction.POSITIONAL_PARAMETER_OPTION_NAME + "\".",
+                                    template, tk);
+                    }
+                }
+            ]
+            <CLOSING_CURLY_BRACKET>
+        ]
+        {
+            if (isPositional) {
+                if (predefNamedParamDefs != null || namedVarargsParamDef != null) {
+                    throw new ParseException(
+                            "Positional parameters must precede named parameters.",
+                            template, paramNameTk);
+                }
+                if (posVarargsParamDef != null) {
+                    throw new ParseException(
+                            "Can't define another positional parameter after the \"positional varargs\" parameter.",
+                            template, paramNameTk);
+                }
+            } else { // named parameter
+                if (namedVarargsParamDef != null) {
+                    throw new ParseException(
+                            "Can't define another parameter after the \"named varargs\" parameter.",
+                            template, paramNameTk);
+                }
             }
-            hasComma = false;
+
+            if (isFunction) {
+                if (!hasPendingComma && !isFirstParam) {
+                    throw new ParseException(
+                            "Function parameters must have comma between them; insert comma before the parameter.",
+                            template, paramNameTk);
+                }
+            } else { // macro
+                if (hasPendingComma) {
+                    if (!isPositional) {
+                        throw new ParseException(
+                                "Named parameters of macros need no comma before them; remove the comma.",
+                                template, commaTk);
+                    }
+                } else {
+                    if (isPositional && !isFirstParam) {
+                        throw new ParseException(
+                                "Positional parameters must have comma between them; "
+                                + "insert comma before the parameter.",
+                                template, paramNameTk);
+                    }
+                }
+            }
+            hasPendingComma = false; // It has found it's role
+        }
+
+        {
+            isVarargs = false;
         }
         [
-            <ELLIPSIS> { isCatchAll = true; }
+            tk = <ELLIPSIS> { isVarargs = true; }
+            // Note that because it's ensured earlier that varargs are the last in the positonal and named sublists,
+            // we don't have to check here if we already have a positional or named varargs parameter.
         ]
+
+        {
+            defaultValueExp = null;
+        }
         [
-            <ASSIGNMENT_EQUALS>
-            defValue = ASTExpression()
+            tk = <ASSIGNMENT_EQUALS>
             {
-                defNames.add(arg.image);
-                hasDefaults = true;
+                if (isVarargs) {
+                    throw new ParseException("Varargs parameters can't have default value.", template, tk);
+                }
             }
+            defaultValueExp = ASTExpression()
         ]
-        [<COMMA> { hasComma = true; } ]
         {
-            if (catchAll != null) {
-                throw new ParseException(
-                "There may only be one \"catch-all\" parameter in a macro declaration, and it must be the last parameter.",
-                template, arg);
-            }
-            if (isCatchAll) {
-                if (defValue != null) {
+            if (defaultValueExp == null && isPositional && !isVarargs) {
+                if (predefPosParamDefs != null
+                        && predefPosParamDefs.get(predefPosParamDefs.size() - 1).getDefaultExpression() != null) {
                     throw new ParseException(
-                    "\"Catch-all\" macro parameter may not have a default value.",
-                    template, arg);
+                            "Parameters without a default value must be before parameters with default values, "
+                            + "among the positional parameters.", template, paramNameTk);
                 }
-                catchAll = arg.image;
-            } else {
-                argNames.add(arg.image);
-                if (hasDefaults && defValue == null) {
-                    throw new ParseException(
-		                    "In a macro declaration, parameters without a default value "
-		                    + "must all occur before the parameters with default values.",
-                    template, arg);
+            }
+        }
+
+        [ commaTk = <COMMA> { hasPendingComma = true; } ]
+
+        {
+            String paramName = paramNameTk.image;
+            if (isPositional) {
+                if (isVarargs) {
+                    posVarargsParamDef = new ASTDirMacroOrFunction.ParameterDefinition(paramName, null);
+                } else {
+                    if (predefPosParamDefs == null) {
+                        predefPosParamDefs = new ArrayList<ASTDirMacroOrFunction.ParameterDefinition>();
+                    }
+                    predefPosParamDefs.add(new ASTDirMacroOrFunction.ParameterDefinition(paramName, defaultValueExp));
+                }
+            } else { // named parameter
+                if (isVarargs) {
+                    namedVarargsParamDef = new ASTDirMacroOrFunction.ParameterDefinition(paramName, null);
+                } else {
+                    if (predefNamedParamDefs == null) {
+                        predefNamedParamDefs = new ArrayList<ASTDirMacroOrFunction.ParameterDefinition>();
+                    }
+                    predefNamedParamDefs.add(new ASTDirMacroOrFunction.ParameterDefinition(paramName, defaultValueExp));
                 }
-                args.put(arg.image, defValue);
             }
+
+            isFirstParam = false;
         }
     )*
-    [<CLOSE_PAREN> { hasCloseParen = true; } ] {
-        if (hasComma) {
-            throw new ParseException("You can't have comma after the last parameter.", template, start);
+
+    { tk = null; }
+    [ tk = <CLOSE_PAREN> ]
+    tk2 = <DIRECTIVE_END>
+    {
+        if (hasPendingComma) {
+            throw new ParseException("Comma without parameter after it.", template, commaTk);
         }
-        if (hasCloseParen != hasOpenParen) {
-    		throw new ParseException(
-                    hasOpenParen ? "Missing closing \")\"."
-                            : "Closing \")\" without corresponding opening \"(\".",
-                            template, start);
+        if ((tk != null) != hasOpenParen) {
+            if (hasOpenParen) {
+                // Note that parenhesis expression mode may aborts at the ">", so we don't get this far.
+                throw new ParseException("Missing closing \")\".", template, tk2);
+            } else {
+        		throw new ParseException("Closing \")\" without corresponding opening \"(\".", template, tk);
+            }
         }
     }
-    <DIRECTIVE_END>
+
     {
         // To prevent parser check loopholes like <#list ...><#macro ...><#break></#macro></#list>.
         lastIteratorBlockContexts = iteratorBlockContexts;
@@ -2998,12 +3093,12 @@ ASTDirMacro Macro() :
     (
         end = <END_MACRO>
         {
-        	if (isFunction) throw new ParseException("Expected function end tag here.", template, start);
+        	if (isFunction) throw new ParseException("Expected function end tag here.", template, end);
     	}
         |
         end = <END_FUNCTION>
         {
-    		if (!isFunction) throw new ParseException("Expected macro end tag here.", template, start);
+    		if (!isFunction) throw new ParseException("Expected macro end tag here.", template, end);
     	}
     )
     {
@@ -3011,7 +3106,19 @@ ASTDirMacro Macro() :
         breakableDirectiveNesting = lastBreakableDirectiveNesting;
         
         inMacro = inFunction = false;
-        ASTDirMacro result = new ASTDirMacro(name, argNames, args, catchAll, isFunction, children);
+
+        ASTDirMacroOrFunction result;
+        try {
+            result = new ASTDirMacroOrFunction(
+                    isFunction,
+                    name,
+                    predefPosParamDefs, posVarargsParamDef,
+                    predefNamedParamDefs, namedVarargsParamDef,
+                    children);
+        } catch (DuplicateStringKeyException e) {
+            throw new ParseException("The parameter list contains the parameter name "
+                    + _StringUtil.jQuote(e.getKey()) + " for multiple times.", template, start);
+        }
         result.setLocation(template, start, end);
         template.addMacro(result);
         return result;
@@ -3217,34 +3324,9 @@ ASTElement DynamicTopLevelCall() :
     }
 }
 
-HashMap NamedArgs() :
-{
-    HashMap result = new HashMap();
-    Token t;
-    ASTExpression exp;
-}
-{
-    (
-        t = <ID>
-        <ASSIGNMENT_EQUALS>
-        {
-            token_source.SwitchTo(token_source.NAMED_PARAMETER_EXPRESSION);
-            token_source.inNamedParameterExpression = true;
-        }             
-        exp = ASTExpression()
-        {
-            result.put(t.image, exp);
-        }
-    )+
-    {
-        token_source.inNamedParameterExpression = false;
-        return result;
-    }
-}
-
-ArrayList PositionalArgs() :
+ArrayList<ASTExpression> PositionalArgs() :
 {
-    ArrayList result = new ArrayList();
+    ArrayList<ASTExpression> result = new ArrayList<ASTExpression>();
     ASTExpression arg;
 }
 {

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-manual/src/test/resources/org/apache/freemarker/manual/examples/AutoEscapingExample-infoBox.ftlh
----------------------------------------------------------------------
diff --git a/freemarker-manual/src/test/resources/org/apache/freemarker/manual/examples/AutoEscapingExample-infoBox.ftlh b/freemarker-manual/src/test/resources/org/apache/freemarker/manual/examples/AutoEscapingExample-infoBox.ftlh
index 8ff93b5..94423dc 100644
--- a/freemarker-manual/src/test/resources/org/apache/freemarker/manual/examples/AutoEscapingExample-infoBox.ftlh
+++ b/freemarker-manual/src/test/resources/org/apache/freemarker/manual/examples/AutoEscapingExample-infoBox.ftlh
@@ -19,7 +19,7 @@
 <@infoBox "Foo & bar" />
 <@infoBox "Foo <b>bar</b>"?noEsc />
 
-<#macro infoBox message>
+<#macro infoBox message{positional}>
   <div class="infoBox">
     ${message}
   </div>

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/IncludePage.java
----------------------------------------------------------------------
diff --git a/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/IncludePage.java b/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/IncludePage.java
index 6199fcc..c500c8c 100644
--- a/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/IncludePage.java
+++ b/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/IncludePage.java
@@ -41,7 +41,7 @@ import org.apache.freemarker.core.TemplateException;
 import org.apache.freemarker.core._DelayedFTLTypeDescription;
 import org.apache.freemarker.core._MiscTemplateException;
 import org.apache.freemarker.core.model.ArgumentArrayLayout;
-import org.apache.freemarker.core.model.CallPlace;
+import org.apache.freemarker.core.CallPlace;
 import org.apache.freemarker.core.model.TemplateBooleanModel;
 import org.apache.freemarker.core.model.TemplateDirectiveModel;
 import org.apache.freemarker.core.model.TemplateModel;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/CustomTagAndELFunctionCombiner.java
----------------------------------------------------------------------
diff --git a/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/CustomTagAndELFunctionCombiner.java b/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/CustomTagAndELFunctionCombiner.java
index 2302f22..fb63309 100644
--- a/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/CustomTagAndELFunctionCombiner.java
+++ b/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/CustomTagAndELFunctionCombiner.java
@@ -26,7 +26,7 @@ import org.apache.freemarker.core.Environment;
 import org.apache.freemarker.core.TemplateException;
 import org.apache.freemarker.core._UnexpectedTypeErrorExplainerTemplateModel;
 import org.apache.freemarker.core.model.ArgumentArrayLayout;
-import org.apache.freemarker.core.model.CallPlace;
+import org.apache.freemarker.core.CallPlace;
 import org.apache.freemarker.core.model.TemplateDirectiveModel;
 import org.apache.freemarker.core.model.TemplateMethodModelEx;
 import org.apache.freemarker.core.model.TemplateModel;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/SimpleTagDirectiveModel.java
----------------------------------------------------------------------
diff --git a/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/SimpleTagDirectiveModel.java b/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/SimpleTagDirectiveModel.java
index 4e7d6c7..6035a1f 100644
--- a/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/SimpleTagDirectiveModel.java
+++ b/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/SimpleTagDirectiveModel.java
@@ -33,7 +33,7 @@ import javax.servlet.jsp.tagext.Tag;
 import org.apache.freemarker.core.Environment;
 import org.apache.freemarker.core.TemplateException;
 import org.apache.freemarker.core.model.ArgumentArrayLayout;
-import org.apache.freemarker.core.model.CallPlace;
+import org.apache.freemarker.core.CallPlace;
 import org.apache.freemarker.core.model.TemplateDirectiveModel;
 import org.apache.freemarker.core.model.TemplateHashModelEx2;
 import org.apache.freemarker.core.model.TemplateModel;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/TagDirectiveModel.java
----------------------------------------------------------------------
diff --git a/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/TagDirectiveModel.java b/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/TagDirectiveModel.java
index 86febfa..ff5b435 100644
--- a/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/TagDirectiveModel.java
+++ b/freemarker-servlet/src/main/java/org/apache/freemarker/servlet/jsp/TagDirectiveModel.java
@@ -34,7 +34,7 @@ import javax.servlet.jsp.tagext.TryCatchFinally;
 import org.apache.freemarker.core.Environment;
 import org.apache.freemarker.core.TemplateException;
 import org.apache.freemarker.core.model.ArgumentArrayLayout;
-import org.apache.freemarker.core.model.CallPlace;
+import org.apache.freemarker.core.CallPlace;
 import org.apache.freemarker.core.model.TemplateDirectiveModel;
 import org.apache.freemarker.core.model.TemplateHashModelEx2;
 import org.apache.freemarker.core.model.TemplateModel;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertDirective.java
----------------------------------------------------------------------
diff --git a/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertDirective.java b/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertDirective.java
index 50ffe60..2e7ecfc 100644
--- a/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertDirective.java
+++ b/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertDirective.java
@@ -22,34 +22,26 @@ package org.apache.freemarker.test.templateutil;
 import java.io.IOException;
 import java.io.Writer;
 
+import org.apache.freemarker.core.CallPlace;
 import org.apache.freemarker.core.Environment;
 import org.apache.freemarker.core.TemplateException;
 import org.apache.freemarker.core.model.ArgumentArrayLayout;
-import org.apache.freemarker.core.model.CallPlace;
 import org.apache.freemarker.core.model.TemplateBooleanModel;
 import org.apache.freemarker.core.model.TemplateDirectiveModel;
 import org.apache.freemarker.core.model.TemplateModel;
 import org.apache.freemarker.core.util.FTLUtil;
-import org.apache.freemarker.core.util.StringToIndexMap;
 
 public class AssertDirective implements TemplateDirectiveModel {
     public static AssertDirective INSTANCE = new AssertDirective();
 
-    private static final String TEST_ARG_NAME = "test";
-    private static final int TEST_ARG_IDX = 0;
-    private static final StringToIndexMap ARG_NAMES_TO_IDX = StringToIndexMap.of(TEST_ARG_NAME, TEST_ARG_IDX);
-    private static final ArgumentArrayLayout ARGS_LAYOUT = ArgumentArrayLayout.create(
-            0, false,
-            ARG_NAMES_TO_IDX, false);
-
     private AssertDirective() { }
     
     @Override
     public void execute(TemplateModel[] args, CallPlace callPlace, Writer out, Environment env)
             throws TemplateException, IOException {
-        TemplateModel test = args[TEST_ARG_IDX];
+        TemplateModel test = args[0];
         if (test == null) {
-            throw new MissingRequiredParameterException(TEST_ARG_NAME, env);
+            throw new MissingRequiredParameterException("test", env);
         }
         if (!(test instanceof TemplateBooleanModel)) {
             throw new AssertationFailedInTemplateException("Assertion failed:\n"
@@ -65,7 +57,7 @@ public class AssertDirective implements TemplateDirectiveModel {
 
     @Override
     public ArgumentArrayLayout getArgumentArrayLayout() {
-        return ARGS_LAYOUT;
+        return ArgumentArrayLayout.SINGLE_POSITIONAL_PARAMETER;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertEqualsDirective.java
----------------------------------------------------------------------
diff --git a/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertEqualsDirective.java b/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertEqualsDirective.java
index fda0d22..6734fbc 100644
--- a/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertEqualsDirective.java
+++ b/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertEqualsDirective.java
@@ -25,7 +25,7 @@ import java.io.Writer;
 import org.apache.freemarker.core.Environment;
 import org.apache.freemarker.core.TemplateException;
 import org.apache.freemarker.core.model.ArgumentArrayLayout;
-import org.apache.freemarker.core.model.CallPlace;
+import org.apache.freemarker.core.CallPlace;
 import org.apache.freemarker.core.model.TemplateBooleanModel;
 import org.apache.freemarker.core.model.TemplateDateModel;
 import org.apache.freemarker.core.model.TemplateDirectiveModel;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertFailsDirective.java
----------------------------------------------------------------------
diff --git a/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertFailsDirective.java b/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertFailsDirective.java
index c089300..e8120fc 100644
--- a/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertFailsDirective.java
+++ b/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/AssertFailsDirective.java
@@ -26,7 +26,7 @@ import java.util.regex.Pattern;
 import org.apache.freemarker.core.Environment;
 import org.apache.freemarker.core.TemplateException;
 import org.apache.freemarker.core.model.ArgumentArrayLayout;
-import org.apache.freemarker.core.model.CallPlace;
+import org.apache.freemarker.core.CallPlace;
 import org.apache.freemarker.core.model.TemplateDirectiveModel;
 import org.apache.freemarker.core.model.TemplateModel;
 import org.apache.freemarker.core.model.TemplateModelException;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3cacd9ed/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/NoOutputDirective.java
----------------------------------------------------------------------
diff --git a/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/NoOutputDirective.java b/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/NoOutputDirective.java
index 7e53023..a8a7bee 100644
--- a/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/NoOutputDirective.java
+++ b/freemarker-test-utils/src/main/java/org/apache/freemarker/test/templateutil/NoOutputDirective.java
@@ -25,7 +25,7 @@ import java.io.Writer;
 import org.apache.freemarker.core.Environment;
 import org.apache.freemarker.core.TemplateException;
 import org.apache.freemarker.core.model.ArgumentArrayLayout;
-import org.apache.freemarker.core.model.CallPlace;
+import org.apache.freemarker.core.CallPlace;
 import org.apache.freemarker.core.model.TemplateDirectiveModel;
 import org.apache.freemarker.core.model.TemplateModel;
 import org.apache.freemarker.core.util._NullWriter;


Mime
View raw message