You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2019/09/22 21:48:54 UTC

[freemarker] 05/05: Some code cleanup in Macro, UnifiedCall and related parser code.

This is an automated email from the ASF dual-hosted git repository.

ddekany pushed a commit to branch 2.3-gae
in repository https://gitbox.apache.org/repos/asf/freemarker.git

commit d161ece2feb7d0329ef4fe9c3cc2214d4961668b
Author: ddekany <dd...@apache.org>
AuthorDate: Sun Sep 22 14:15:50 2019 +0200

    Some code cleanup in Macro, UnifiedCall and related parser code.
---
 src/main/java/freemarker/core/Environment.java |  2 +-
 src/main/java/freemarker/core/Macro.java       | 42 ++++++++++++++------------
 src/main/java/freemarker/core/UnifiedCall.java |  2 +-
 src/main/javacc/FTL.jj                         | 21 ++++++-------
 4 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/src/main/java/freemarker/core/Environment.java b/src/main/java/freemarker/core/Environment.java
index 9d7a2f6..c7bfc1a 100644
--- a/src/main/java/freemarker/core/Environment.java
+++ b/src/main/java/freemarker/core/Environment.java
@@ -868,7 +868,7 @@ public final class Environment extends Configurable {
             currentNamespace = (Namespace) macroToNamespaceLookup.get(macroOrFunction);
 
             try {
-                macroCtx.sanityCheck(this);
+                macroCtx.checkParamsSetAndApplyDefaults(this);
                 visit(macroOrFunction.getChildBuffer());
             } catch (ReturnInstruction.Return re) {
                 // Not an error, just a <#return>
diff --git a/src/main/java/freemarker/core/Macro.java b/src/main/java/freemarker/core/Macro.java
index d01fef1..c76092b 100644
--- a/src/main/java/freemarker/core/Macro.java
+++ b/src/main/java/freemarker/core/Macro.java
@@ -22,6 +22,7 @@ package freemarker.core;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -32,7 +33,7 @@ import freemarker.template.TemplateModelIterator;
 import freemarker.template.TemplateScalarModel;
 
 /**
- * An element representing a macro declaration.
+ * An element representing a macro or function declaration.
  * 
  * @deprecated Subject to be changed or renamed any time; no "stable" replacement exists yet.
  */
@@ -40,7 +41,6 @@ import freemarker.template.TemplateScalarModel;
 public final class Macro extends TemplateElement implements TemplateModel {
 
     static final Macro DO_NOTHING_MACRO = new Macro(".pass", 
-            Collections.EMPTY_LIST, 
             Collections.EMPTY_MAP,
             null, false,
             TemplateElements.EMPTY);
@@ -50,21 +50,25 @@ public final class Macro extends TemplateElement implements TemplateModel {
     
     private final String name;
     private final String[] paramNames;
-    private final Map paramDefaults;
+    private final Map paramNamesWithDefault;
     private final String catchAllParamName;
     private final boolean function;
 
-    Macro(String name, List argumentNames, Map args, 
+    /**
+     * @param paramNamesWithDefault Maps the parameter names to its default value expression, or to {@code null} if
+     *      there's no default value. As parameter order is significant; use {@link LinkedHashMap} or similar.
+     *      This doesn't include the catch-all parameter (as that can be specified by name on the caller side).
+     */
+    Macro(String name, Map<String, Expression> paramNamesWithDefault,
             String catchAllParamName, boolean function,
             TemplateElements children) {
         this.name = name;
-        this.paramNames = (String[]) argumentNames.toArray(
-                new String[argumentNames.size()]);
-        this.paramDefaults = args;
-        
+        this.paramNamesWithDefault = paramNamesWithDefault;
+        this.paramNames = paramNamesWithDefault.keySet().toArray(new String[0]);
+        this.catchAllParamName = catchAllParamName;
+
         this.function = function;
-        this.catchAllParamName = catchAllParamName; 
-        
+
         this.setChildren(children);
     }
 
@@ -81,7 +85,7 @@ public final class Macro extends TemplateElement implements TemplateModel {
     }
 
     boolean hasArgNamed(String name) {
-        return paramDefaults.containsKey(name);
+        return paramNamesWithDefault.containsKey(name);
     }
     
     public String getName() {
@@ -113,9 +117,9 @@ public final class Macro extends TemplateElement implements TemplateModel {
             }
             String argName = paramNames[i];
             sb.append(_CoreStringUtils.toFTLTopLevelIdentifierReference(argName));
-            if (paramDefaults != null && paramDefaults.get(argName) != null) {
+            if (paramNamesWithDefault != null && paramNamesWithDefault.get(argName) != null) {
                 sb.append('=');
-                Expression defaultExpr = (Expression) paramDefaults.get(argName);
+                Expression defaultExpr = (Expression) paramNamesWithDefault.get(argName);
                 if (function) {
                     sb.append(defaultExpr.getCanonicalForm());
                 } else {
@@ -177,7 +181,7 @@ public final class Macro extends TemplateElement implements TemplateModel {
         }
 
         // Set default parameters, check if all the required parameters are defined.
-        void sanityCheck(Environment env) throws TemplateException {
+        void checkParamsSetAndApplyDefaults(Environment env) throws TemplateException {
             boolean resolvedAnArg, hasUnresolvedArg;
             Expression firstUnresolvedExpression;
             InvalidReferenceException firstReferenceException;
@@ -188,13 +192,13 @@ public final class Macro extends TemplateElement implements TemplateModel {
                 for (int i = 0; i < paramNames.length; ++i) {
                     String argName = paramNames[i];
                     if (localVars.get(argName) == null) {
-                        Expression valueExp = (Expression) paramDefaults.get(argName);
-                        if (valueExp != null) {
+                        Expression defaultValueExp = (Expression) paramNamesWithDefault.get(argName);
+                        if (defaultValueExp != null) {
                             try {
-                                TemplateModel tm = valueExp.eval(env);
+                                TemplateModel tm = defaultValueExp.eval(env);
                                 if (tm == null) {
                                     if (!hasUnresolvedArg) {
-                                        firstUnresolvedExpression = valueExp;
+                                        firstUnresolvedExpression = defaultValueExp;
                                         hasUnresolvedArg = true;
                                     }
                                 } else {
@@ -282,7 +286,7 @@ public final class Macro extends TemplateElement implements TemplateModel {
                 if (idx % 2 != 0) {
                     return paramName;
                 } else {
-                    return paramDefaults.get(paramName);
+                    return paramNamesWithDefault.get(paramName);
                 }
             } else if (idx == argDescsEnd) {
                 return catchAllParamName;
diff --git a/src/main/java/freemarker/core/UnifiedCall.java b/src/main/java/freemarker/core/UnifiedCall.java
index 186fb4e..ccd82ec 100644
--- a/src/main/java/freemarker/core/UnifiedCall.java
+++ b/src/main/java/freemarker/core/UnifiedCall.java
@@ -37,7 +37,7 @@ import freemarker.template.utility.ObjectFactory;
 import freemarker.template.utility.StringUtil;
 
 /**
- * An element for the unified macro/transform syntax. 
+ * An element for calling a macro/directive/transform.
  */
 final class UnifiedCall extends TemplateElement implements DirectiveCallPlace {
 
diff --git a/src/main/javacc/FTL.jj b/src/main/javacc/FTL.jj
index 806d16e..3880ab4 100644
--- a/src/main/javacc/FTL.jj
+++ b/src/main/javacc/FTL.jj
@@ -3434,17 +3434,16 @@ Macro Macro() :
     Token arg, start, end;
     Expression nameExp;
     String name;
-    ArrayList argNames = new ArrayList();
-    HashMap args = new HashMap();
-    ArrayList defNames = new ArrayList();
+    Map<String, Expression> paramNamesWithDefault = new LinkedHashMap<String, Expression>();
     Expression defValue = null;
+    String catchAllParamName = null;
+    boolean isCatchAll = false;
     List lastIteratorBlockContexts;
     int lastBreakableDirectiveNesting;
     int lastContiunableDirectiveNesting;
     TemplateElements children;
-    boolean isFunction = false, hasDefaults = false;
-    boolean isCatchAll = false;
-    String catchAll = null;
+    boolean isFunction = false;
+    boolean hasDefaults = false;
 }
 {
     (
@@ -3474,13 +3473,12 @@ Macro Macro() :
             <EQUALS>
             defValue = Expression()
             {
-                defNames.add(arg.image);
                 hasDefaults = true;
             }
         ]
         [<COMMA>]
         {
-            if (catchAll != null) {
+            if (catchAllParamName != 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);
@@ -3491,16 +3489,15 @@ Macro Macro() :
                     "\"Catch-all\" macro parameter may not have a default value.",
                     template, arg);
                 }
-                catchAll = arg.image;
+                catchAllParamName = 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);
                 }
-                args.put(arg.image, defValue);
+                paramNamesWithDefault.put(arg.image, defValue);
             }
         }
     )*
@@ -3540,7 +3537,7 @@ Macro Macro() :
         }
         
         inMacro = inFunction = false;
-        Macro result = new Macro(name, argNames, args, catchAll, isFunction, children);
+        Macro result = new Macro(name, paramNamesWithDefault, catchAllParamName, isFunction, children);
         result.setLocation(template, start, end);
         template.addMacro(result);
         return result;