freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ddek...@apache.org
Subject [3/3] incubator-freemarker git commit: Tightened syntax inside the #macro and #function start tags:
Date Thu, 20 Jul 2017 15:23:13 GMT
Tightened syntax inside the #macro and #function start tags:

1. In `#macro` and `#function` the comma must only be used between by-position parameters (earlier the comma was optional). As of this writing, the by-position VS by-name feature isn't implemented yet, so comma is to be used for `#function`, and comma must not be used for `#macro` (even though for now macros are always allowed to be called with by-position parameters as well, just as in FM2).
2. In `#function`, parentheses are now required around parameter list (as in `<#function f(a, b)>`), even if there are zero parameters (as in `<#function f()>`). In `#macro`, parentheses are not allowed around parameter list anymore (so `#macro m(a b)` becomes to `#macro m a b`). This is to remain more consistent with the look-and-feel of the invocation of the function or macro.


Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/4306b7d7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/4306b7d7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/4306b7d7

Branch: refs/heads/3
Commit: 4306b7d7376870e7e28fc196277effc50acef5be
Parents: 6af006c
Author: ddekany <ddekany@apache.org>
Authored: Thu Jul 20 17:21:03 2017 +0200
Committer: ddekany <ddekany@apache.org>
Committed: Thu Jul 20 17:21:03 2017 +0200

----------------------------------------------------------------------
 FM3-CHANGE-LOG.txt                              |   9 +
 .../core/FM2ASTToFM3SourceConverter.java        | 176 ++++++++++++-------
 .../converter/FM2ToFM3ConverterTest.java        |  52 ++++--
 .../EnvironmentGetTemplateVariantsTest.java     |   6 +-
 .../apache/freemarker/core/ListErrorsTest.java  |   2 +-
 .../core/ParsingErrorMessagesTest.java          |   2 +-
 .../freemarker/core/RemovedFM2SyntaxTest.java   |  14 ++
 .../core/TheadInterruptingSupportTest.java      |   3 +-
 .../org/apache/freemarker/core/ast-1.ftl        |   2 +-
 .../core/cano-identifier-escaping.ftl           |   2 +-
 .../templates/existence-operators.ftl           |   4 +-
 .../templates/identifier-escaping.ftl           |   2 +-
 .../core/templatesuite/templates/import_lib.ftl |   2 +-
 .../core/templatesuite/templates/list2.ftl      |   2 +-
 .../core/templatesuite/templates/macros.ftl     |   4 +-
 .../templatesuite/templates/range-common.ftl    |   2 +-
 .../core/templatesuite/templates/range.ftl      |   2 +-
 .../templatesuite/templates/switch-builtin.ftl  |   2 +-
 .../templatesuite/templates/then-builtin.ftl    |   4 +-
 freemarker-core/src/main/javacc/FTL.jj          |  38 +++-
 20 files changed, 233 insertions(+), 97 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/FM3-CHANGE-LOG.txt
----------------------------------------------------------------------
diff --git a/FM3-CHANGE-LOG.txt b/FM3-CHANGE-LOG.txt
index 7f927ac..c357c28 100644
--- a/FM3-CHANGE-LOG.txt
+++ b/FM3-CHANGE-LOG.txt
@@ -69,6 +69,15 @@ Node: Changes already mentioned above aren't repeated here!
 - Removed long deprecated `#{}` interpolations. They are treated as plain static text now.
   Converter note: The template converter tool translates these to `${}` interpolations. For example `#{x}` is simply
   translated to `${b}`, while `#{x; m1M3}` is translated to `${x?string('0.0##')}`). The output should remain the same.
+- In `#macro` and `#function` the comma must only be used between by-position parameters (earlier the comma was
+  optional).
+  TODO As of this writing, the by-position VS by-name feature isn't implemented yet, so
+  comma is to be used for `#function`, and comma must not be used for `#macro` (even though for now macros are always
+  allowed to be called with by-position parameters as well, just as in FM2).
+- In `#function`, parentheses are now required around parameter list (as in `<#function f(a, b)>`), even if there are
+  zero parameters (as in `<#function f()>`). In `#macro`, parentheses are not allowed around parameter list anymore
+  (so `#macro m(a b)` becomes to `#macro m a b`). This is to remain more consistent with the look-and-feel of the
+  invocation of the function or macro.
 - Removed some long deprecated built-ins:
   - `webSafe` (converted to `html`)
   - TODO Add the further ones here as they are removed

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-converter/src/main/java/freemarker/core/FM2ASTToFM3SourceConverter.java
----------------------------------------------------------------------
diff --git a/freemarker-converter/src/main/java/freemarker/core/FM2ASTToFM3SourceConverter.java b/freemarker-converter/src/main/java/freemarker/core/FM2ASTToFM3SourceConverter.java
index 4b62179..26bbe55 100644
--- a/freemarker-converter/src/main/java/freemarker/core/FM2ASTToFM3SourceConverter.java
+++ b/freemarker-converter/src/main/java/freemarker/core/FM2ASTToFM3SourceConverter.java
@@ -207,7 +207,8 @@ public class FM2ASTToFM3SourceConverter {
                 overlayTemplateLoader.putTemplate(
                         template.getName(),
                         tagBeginChar + "#ftl" + tagEndChar
-                        + tagBeginChar + "@ftl" + src.substring(pos, tagEnd) + (hasSlash ? "" : "/") + tagEndChar);
+                                + tagBeginChar + "@ftl" + src.substring(pos, tagEnd) + (hasSlash ? "" : "/")
+                                + tagEndChar);
                 try {
                     customFtlDirSrcConverter = new FM2ASTToFM3SourceConverter(
                             fm2Cfg.getTemplate(template.getName()),
@@ -649,10 +650,10 @@ public class FM2ASTToFM3SourceConverter {
             printExp(passedValue);
             pos = getEndPositionExclusive(passedValue);
             if (paramIdx < paramCnt - 1) {
-                _ObjectHolder<Boolean> hadSeparatorSymbol = new _ObjectHolder<>(null);
+                _ObjectHolder<Integer> separatorPosInOutput = new _ObjectHolder<>(null);
                 int spacingStartPos = out.length();
-                pos = printOptionalSeparatorAndWSAndExpComments(pos, ",", hadSeparatorSymbol);
-                if (!hadSeparatorSymbol.get()) {
+                pos = printOptionalSeparatorAndWSAndExpComments(pos, ",", separatorPosInOutput);
+                if (separatorPosInOutput.get() == null) {
                     insertOmittedSeparatorComma(spacingStartPos);
                 }
             }
@@ -1097,14 +1098,19 @@ public class FM2ASTToFM3SourceConverter {
     private void printDirMacroOrFunction(Macro node) throws ConverterException {
         int paramCnt = node.getParameterCount();
 
-        int subtype = getParam(node, paramCnt - 1, ParameterRole.AST_NODE_SUBTYPE, Integer.class);
         String tagName;
-        if (subtype == Macro.TYPE_MACRO) {
-            tagName = "macro";
-        } else if (subtype == Macro.TYPE_FUNCTION) {
-            tagName = "function";
-        } else {
-            throw new UnexpectedNodeContentException(node, "Unhandled node subtype: {}", subtype);
+        boolean function;
+        {
+            int subtype = getParam(node, paramCnt - 1, ParameterRole.AST_NODE_SUBTYPE, Integer.class);
+            if (subtype == Macro.TYPE_MACRO) {
+                tagName = "macro";
+                function = false;
+            } else if (subtype == Macro.TYPE_FUNCTION) {
+                tagName = "function";
+                function = true;
+            } else {
+                throw new UnexpectedNodeContentException(node, "Unhandled node subtype: {}", subtype);
+            }
         }
 
         int pos = printDirStartTagPartBeforeParams(node, tagName);
@@ -1113,7 +1119,35 @@ public class FM2ASTToFM3SourceConverter {
         print(FTLUtil.escapeIdentifier(assignedName));
         pos = getPositionAfterAssignmentTargetIdentifier(pos);
 
-        pos = printOptionalSeparatorAndWSAndExpComments(pos, "(");
+        {
+            int separatorStartPos = out.length();
+            _ObjectHolder<Integer> separatorPosInOutput = new _ObjectHolder<>(null);
+            pos = printOptionalSeparatorAndWSAndExpComments(pos, "(", separatorPosInOutput);
+            if (function && separatorPosInOutput.get() == null) {
+                if (separatorStartPos < out.length() && out.charAt(separatorStartPos) == ' ') {
+                    // `#function f p` -> `#function f(p)`
+                    out.setCharAt(separatorStartPos, '(');
+                } else {
+                    // `#function f\n\tp` -> `#function f(\n\tp`
+                    out.insert(separatorStartPos, '(');
+                }
+            } else if (!function && separatorPosInOutput.get() != null) {
+                Integer sepPos = separatorPosInOutput.get();
+                // `#macro m(p` -> `#macro mp` would be wrong; we must ensure that the separating WS is there:
+                boolean spaceBefore = sepPos - 1 >= 0 && Character.isWhitespace(out.charAt(sepPos - 1));
+                boolean spaceAfter = sepPos + 1 < out.length() && Character.isWhitespace(out.charAt(sepPos + 1));
+                boolean hasParams = node.getParameterRole(1) != ParameterRole.CATCH_ALL_PARAMETER_NAME
+                        || node.getParameterValue(1) != null;
+                if (spaceBefore || spaceAfter || !hasParams) {
+                    out.deleteCharAt(sepPos);
+                    if (spaceBefore && spaceAfter) {
+                        out.deleteCharAt(sepPos); // avoid double space in case of `#macro m ( p`
+                    }
+                } else {
+                    out.setCharAt(sepPos, ' ');
+                }
+            }
+        }
 
         int paramIdx = 1;
         while (node.getParameterRole(paramIdx) == ParameterRole.PARAMETER_NAME) {
@@ -1128,8 +1162,13 @@ public class FM2ASTToFM3SourceConverter {
                 pos = getEndPositionExclusive(paramDefault);
             }
 
+            int outPosBeforeWS = out.length();
             pos = printWSAndExpComments(pos);
             {
+                // Is it the last param, if we count in the final catch-all parameter as well?
+                boolean isLastParam = node.getParameterRole(paramIdx) == ParameterRole.CATCH_ALL_PARAMETER_NAME
+                        && node.getParameterValue(paramIdx) == null;
+
                 char c = src.charAt(pos);
                 assertNodeContent(
                         c == ',' || c == ')' || isTagEndChar(c)
@@ -1137,17 +1176,24 @@ public class FM2ASTToFM3SourceConverter {
                         node,
                         "Unexpected character: {}", c);
                 if (c == ',') {
-                    print(c);
+                    if (function) {
+                        print(c);
+                    } else if (!Character.isWhitespace(src.charAt(pos - 1))
+                            && !Character.isWhitespace(src.charAt(pos + 1))) {
+                        print(' ');
+                    }
                     pos++;
 
                     pos = printWSAndExpComments(pos);
+                } else if (!isLastParam && function) {
+                    out.insert(outPosBeforeWS, ",");
                 }
                 if (c == ')') {
-                    assertNodeContent(node.getParameterRole(paramIdx) != ParameterRole.PARAMETER_NAME, node,
+                    assertNodeContent(isLastParam, node,
                             "Expected no parameter after \"(\"");
                 }
             }
-        }
+        } // while has more parameters
 
         {
             ParameterRole parameterRole = node.getParameterRole(paramIdx);
@@ -1168,7 +1214,18 @@ public class FM2ASTToFM3SourceConverter {
         assertNodeContent(paramIdx == paramCnt - 1, node,
                 "Expected AST parameter at index {} to be the last one", paramIdx);
 
-        pos = printOptionalSeparatorAndWSAndExpComments(pos, ")");
+        _ObjectHolder<Integer> separatorPosInOutput = new _ObjectHolder<>(null);
+        pos = printOptionalSeparatorAndWSAndExpComments(pos, ")", separatorPosInOutput);
+        if (function && separatorPosInOutput.get() == null) {
+            out.append(')');
+        } else if (!function && separatorPosInOutput.get() != null) {
+            Integer sepPos = separatorPosInOutput.get();
+            out.deleteCharAt(sepPos);
+            if (sepPos - 1 > 0 && out.charAt(sepPos - 1) == ' ') {
+                out.deleteCharAt(sepPos - 1);
+            }
+        }
+
         assertNodeContent(isTagEndChar(src.charAt(pos)), node, "Tag end not found");
         print(tagEndChar);
 
@@ -1181,7 +1238,7 @@ public class FM2ASTToFM3SourceConverter {
         boolean ftlDirMode = printNextCustomDirAsFtlDir;
         printNextCustomDirAsFtlDir = false;
 
-        boolean legacyCallDirMode = src.startsWith("#call" , getStartPosition(node) + 1);
+        boolean legacyCallDirMode = src.startsWith("#call", getStartPosition(node) + 1);
 
         print(tagBeginChar);
         print(ftlDirMode ? '#' : '@');
@@ -1217,10 +1274,10 @@ public class FM2ASTToFM3SourceConverter {
 
             int spacingEndPos;
             if (paramIdx > 1) {
-                _ObjectHolder<Boolean> hadSeparatorSymbol = new _ObjectHolder<>(null);
+                _ObjectHolder<Integer> separatorPosInOutput = new _ObjectHolder<>(null);
                 int spacingStartPos = out.length();
-                spacingEndPos = printOptionalSeparatorAndWSAndExpComments(lastParamEnd, ",", hadSeparatorSymbol);
-                if (!hadSeparatorSymbol.get()) {
+                spacingEndPos = printOptionalSeparatorAndWSAndExpComments(lastParamEnd, ",", separatorPosInOutput);
+                if (separatorPosInOutput.get() == null) {
                     insertOmittedSeparatorComma(spacingStartPos);
                 }
             } else {
@@ -1575,6 +1632,7 @@ public class FM2ASTToFM3SourceConverter {
     }
 
     private static final Map<String, String> DOM_KEY_MAPPING;
+
     static {
         Map<String, String> domKeyMapping = new HashMap<>();
         for (String atAtKey : AtAtKeyAccessor.getAtAtKeys()) {
@@ -1675,10 +1733,10 @@ public class FM2ASTToFM3SourceConverter {
                 Expression item = getParam(node, paramIdx, ParameterRole.ITEM_VALUE, Expression.class);
 
                 if (paramIdx != 0) {
-                    _ObjectHolder<Boolean> hadSeparatorSymbol = new _ObjectHolder<>(null);
+                    _ObjectHolder<Integer> separatorPosInOutput = new _ObjectHolder<>(null);
                     int spacingStartPos = out.length();
-                    printOptionalSeparatorAndWSAndExpComments(pos, ",", hadSeparatorSymbol);
-                    if (!hadSeparatorSymbol.get()) {
+                    printOptionalSeparatorAndWSAndExpComments(pos, ",", separatorPosInOutput);
+                    if (separatorPosInOutput.get() == null) {
                         insertOmittedSeparatorComma(spacingStartPos);
                     }
                 }
@@ -1795,7 +1853,7 @@ public class FM2ASTToFM3SourceConverter {
     private int stringLiteralNestingLevel;
 
     private void printExpStringLiteral(StringLiteral node) throws ConverterException {
-        printExpStringLiteral(node, node.isLiteral()     ? node.getAsString() : null);
+        printExpStringLiteral(node, node.isLiteral() ? node.getAsString() : null);
     }
 
     private void printExpStringLiteral(StringLiteral node, String value) throws ConverterException {
@@ -1983,7 +2041,7 @@ public class FM2ASTToFM3SourceConverter {
         if (fm2TagNames.size() == 1 && containsUpperCaseLetter(fm3TagName)) {
             throw new IllegalArgumentException(
                     "You must specify multiple FM2 tag names when the FM3 tag name ("
-                    + fm3TagName + ") contains upper case letters");
+                            + fm3TagName + ") contains upper case letters");
         }
         printDirEndTag(node, fm2TagNames, fm3TagName, false);
     }
@@ -2251,7 +2309,7 @@ public class FM2ASTToFM3SourceConverter {
      */
     private int getPosition(int column, int row) {
         if (row == 0) {
-            return  -1;
+            return -1;
         }
         if (rowStartPositions == null) {
             rowStartPositions = new ArrayList<>();
@@ -2315,31 +2373,6 @@ public class FM2ASTToFM3SourceConverter {
         return src.substring(startPos, getPositionAfterWSAndExpComments(startPos));
     }
 
-    private String readSeparatorAndWSAndExpComments(int startPos, String separatorSymbol, boolean separatorOptional,
-            _ObjectHolder<Boolean> hadSeparatorSymbol)
-            throws ConverterException {
-        int pos = getPositionAfterWSAndExpComments(startPos);
-
-        if (pos == src.length() || !src.startsWith(separatorSymbol, pos)) {
-            if (!separatorOptional) {
-                throw new ConverterException(
-                        "Expected separator " + _StringUtil.jQuote(separatorSymbol) + " at position " + pos + ".");
-            }
-            if (hadSeparatorSymbol != null) {
-                hadSeparatorSymbol.set(false);
-            }
-            return src.substring(startPos, pos);
-        }
-        pos += separatorSymbol.length();
-        if (hadSeparatorSymbol != null) {
-            hadSeparatorSymbol.set(true);
-        }
-
-        pos = getPositionAfterWSAndExpComments(pos);
-
-        return src.substring(startPos, pos);
-    }
-
     private int printWSAndExpComments(int pos) throws ConverterException {
         String sep = readWSAndExpComments(pos);
         printWithConvertedExpComments(sep);
@@ -2358,18 +2391,39 @@ public class FM2ASTToFM3SourceConverter {
     }
 
     private int printOptionalSeparatorAndWSAndExpComments(
-            int pos, String separator, _ObjectHolder<Boolean> hadSeparatorSymbol)
+            int pos, String separator, _ObjectHolder<Integer> separatorPosInOutput)
             throws ConverterException {
-        return printSeparatorAndWSAndExpComments(pos, separator, true, hadSeparatorSymbol);
+        return printSeparatorAndWSAndExpComments(pos, separator, true, separatorPosInOutput);
     }
 
-    private int printSeparatorAndWSAndExpComments(int pos, String separator, boolean sepOptional,
-            _ObjectHolder<Boolean> hadSeparatorSymbol)
+    private int printSeparatorAndWSAndExpComments(int startPos, String separatorSymbol, boolean separatorOptional,
+            _ObjectHolder<Integer> separatorPosInOutput)
             throws ConverterException {
-        String sep = readSeparatorAndWSAndExpComments(pos, separator, sepOptional, hadSeparatorSymbol);
-        printWithConvertedExpComments(sep);
-        pos += sep.length();
-        return pos;
+
+        int pos = getPositionAfterWSAndExpComments(startPos);
+
+        if (pos == src.length() || !src.startsWith(separatorSymbol, pos)) {
+            if (!separatorOptional) {
+                throw new ConverterException(
+                        "Expected separator " + _StringUtil.jQuote(separatorSymbol) + " at position " + pos + ".");
+            }
+            if (separatorPosInOutput != null) {
+                separatorPosInOutput.set(null);
+            }
+            printWithConvertedExpComments(src.substring(startPos, pos));
+            return pos;
+        } else { // Has separator symbol
+            int sepPos = pos;
+            printWithConvertedExpComments(src.substring(startPos, pos));
+            if (separatorPosInOutput != null) {
+                separatorPosInOutput.set(out.length());
+            }
+
+            pos = getPositionAfterWSAndExpComments(pos + separatorSymbol.length());
+
+            printWithConvertedExpComments(src.substring(sepPos, pos));
+            return pos;
+        }
     }
 
     private int getPositionAfterIdentifier(int startPos) throws ConverterException {
@@ -2426,7 +2480,8 @@ public class FM2ASTToFM3SourceConverter {
             c = src.charAt(pos++);
             if (c == quotationC && !escaped) {
                 return pos;
-            } if (c == '\\' && !escaped && !raw) {
+            }
+            if (c == '\\' && !escaped && !raw) {
                 escaped = true;
             } else {
                 escaped = false;
@@ -2509,5 +2564,4 @@ public class FM2ASTToFM3SourceConverter {
     }
 
 
-
 }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-converter/src/test/java/org/freemarker/converter/FM2ToFM3ConverterTest.java
----------------------------------------------------------------------
diff --git a/freemarker-converter/src/test/java/org/freemarker/converter/FM2ToFM3ConverterTest.java b/freemarker-converter/src/test/java/org/freemarker/converter/FM2ToFM3ConverterTest.java
index cd70303..61eaa35 100644
--- a/freemarker-converter/src/test/java/org/freemarker/converter/FM2ToFM3ConverterTest.java
+++ b/freemarker-converter/src/test/java/org/freemarker/converter/FM2ToFM3ConverterTest.java
@@ -189,27 +189,55 @@ public class FM2ToFM3ConverterTest extends ConverterTest {
 
         assertConvertedSame("<#macro m>body</#macro>");
         assertConvertedSame("<#macro <#--1--> m <#--2-->></#macro >");
-        assertConvertedSame("<#macro m()></#macro>");
-        assertConvertedSame("<#macro m <#--1--> ( <#--2--> ) <#--3--> ></#macro>");
+        assertConverted("<#macro m <#--1--> <#--2--> <#--3--> ></#macro>", "<#macro m <#--1--> ( <#--2--> ) "
+                + "<#--3--> ></#macro>");
         assertConvertedSame("<#macro m p1></#macro>");
-        assertConvertedSame("<#macro m(p1)></#macro>");
+        assertConverted("<#macro m p1></#macro>", "<#macro m(p1)></#macro>");
         assertConvertedSame("<#macro m p1 p2 p3></#macro>");
         assertConvertedSame("<#macro m p1 <#--1--> p2 <#--2--> p3 <#--3-->></#macro>");
-        assertConvertedSame("<#macro m(p1<#--1-->,<#--2--> p2<#--3-->,<#--4-->"
-                + " p5<#--5-->)<#--6-->></#macro>");
+        assertConverted(
+                "<#macro m<#--0--> p1<#--1--> <#--2-->p2<#--3--> <#--4--> p5<#--5--><#--6-->></#macro>",
+                "<#macro m<#--0-->(p1<#--1-->,<#--2-->p2<#--3-->,<#--4--> p5<#--5-->)<#--6-->></#macro>");
         assertConvertedSame("<#macro m p1=11 p2=foo p3=a+b></#macro>");
-        assertConvertedSame("<#macro m(p1=11, p2=foo, p3=a+b)></#macro>");
-        assertConvertedSame("<#macro m p1<#--1-->=<#--2-->11<#--3-->,<#--4-->p2=22></#macro>");
+        assertConverted("<#macro m p1=11 p2=foo p3=a+b></#macro>", "<#macro m(p1=11, p2=foo, p3=a+b)></#macro>");
+        assertConverted(
+                "<#macro m p1<#--1-->=<#--2-->11<#--3--> <#--4-->p2=22></#macro>",
+                "<#macro m p1<#--1-->=<#--2-->11<#--3-->,<#--4-->p2=22></#macro>");
         assertConvertedSame("<#macro m others...></#macro>");
         assertConvertedSame("<#macro m p1 others...></#macro>");
         assertConvertedSame("<#macro m p1 p2=22 others...></#macro>");
-        assertConvertedSame("<#macro m(others...)></#macro>");
-        assertConvertedSame("<#macro m(others <#--1--> ... <#--2--> )></#macro>");
-        assertConvertedSame("<#function f x y><#return x + y></#function>");
-        assertConvertedSame("<#function f(x, y=0 <#--0-->)><#return <#--1--> x + y <#--2-->></#function>");
+        assertConverted("<#macro m others...></#macro>", "<#macro m(others...)></#macro>");
+        assertConverted(
+                "<#macro m others <#--1--> ... <#--2-->></#macro>",
+                "<#macro m(others <#--1--> ... <#--2--> )></#macro>");
         assertConvertedSame("<#macro m\\-1 p\\-1></#macro>");
         // Only works with " now, as it doesn't keep the literal kind. Later we will escape differently anyway:
         assertConvertedSame("<#macro \"m 1\"></#macro>");
+        // Tests made for macro definition syntax tightening (may unintendedly repeats earlier tests...):
+        assertConverted("<#macro m></#macro>", "<#macro m()></#macro>");
+        assertConverted("<#macro m></#macro>", "<#macro m ( )></#macro>");
+        assertConverted("<#macro m p></#macro>", "<#macro m(p)></#macro>");
+        assertConverted("<#macro m p></#macro>", "<#macro m ( p)></#macro>");
+        assertConverted("<#macro m p></#macro>", "<#macro m (p)></#macro>");
+        assertConverted("<#macro m p></#macro>", "<#macro m( p)></#macro>");
+        assertConverted("<#macro m p></#macro>", "<#macro m(p )></#macro>");
+        assertConverted("<#macro m p1 p2 p3></#macro>", "<#macro m(p1, p2, p3)></#macro>");
+        assertConverted("<#macro m p1 p2 p3></#macro>", "<#macro m p1, p2, p3></#macro>");
+        assertConverted("<#macro m p1 p2 p3></#macro>", "<#macro m p1,p2,p3></#macro>");
+        assertConverted("<#macro m p1 p2=2 p3=3></#macro>", "<#macro m p1, p2=2, p3=3></#macro>");
+        assertConverted("<#macro m p1 others...></#macro>", "<#macro m p1, others...></#macro>");
+        assertConverted("<#macro m p1 others...></#macro>", "<#macro m(p1, others...)></#macro>");
+
+        assertConvertedSame("<#function f(x, y=0 <#--0-->)><#return <#--1--> x + y <#--2-->></#function>");
+        assertConverted("<#function f(x, y)><#return x + y></#function>",
+                "<#function f x y><#return x + y></#function>");
+        // Tests made for function definition syntax tightening (may unintendedly repeats earlier tests...):
+        assertConverted("<#function f(p)></#function>", "<#function f p></#function>");
+        assertConverted("<#function f()></#function>", "<#function f></#function>");
+        assertConverted("<#function f(p1, p2, p3)></#function>", "<#function f p1 p2 p3></#function>");
+        assertConverted("<#function f(p1, p2, p3)></#function>", "<#function f(p1 p2 p3)></#function>");
+        assertConverted("<#function f ( p1, p2, p3 ) ></#function>", "<#function f ( p1 p2 p3 ) ></#function>");
+
         assertConvertedSame("<#macro m><#nested x + 1, 2, 3></#macro>");
         assertConvertedSame("<#macro m><#nested <#--1--> x + 1 <#--2-->, <#--3--> 2 <#--4-->></#macro>");
         assertConverted(
@@ -629,7 +657,7 @@ public class FM2ToFM3ConverterTest extends ConverterTest {
         converter.setSource(srcFile);
         converter.setDestinationDirectory(dstDir);
         converter.setInclude(null);
-        // converter.setValidateOutput(false);
+        converter.setValidateOutput(false); //!!T
         Properties properties = new Properties();
         properties.setProperty(Configuration.DEFAULT_ENCODING_KEY, UTF_8.name());
         if (squareBracketTagSyntax) {

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/java/org/apache/freemarker/core/EnvironmentGetTemplateVariantsTest.java
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/EnvironmentGetTemplateVariantsTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/EnvironmentGetTemplateVariantsTest.java
index b79559c..fc3bbf5 100644
--- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/EnvironmentGetTemplateVariantsTest.java
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/EnvironmentGetTemplateVariantsTest.java
@@ -66,7 +66,7 @@ public class EnvironmentGetTemplateVariantsTest extends TemplateTest {
                 + "<@mainM><@tNames /> <#include 'inc4'> <@tNames /></@>\n"
                 + "<@tNames />\n"
                 + "---8---\n"
-                + "<#function mainF>"
+                + "<#function mainF()>"
                     + "<@tNames />"
                     + "<#return lastTNamesResult>"
                 + "</#function>"
@@ -77,7 +77,7 @@ public class EnvironmentGetTemplateVariantsTest extends TemplateTest {
                 + "<#macro incM>"
                     + "[incM: <@tNames /> {<#nested>}]"
                 + "</#macro>"
-                + "<#function incF>"
+                + "<#function incF()>"
                     + "<@tNames />"
                     + "<#return lastTNamesResult>"
                 + "</#function>"
@@ -99,7 +99,7 @@ public class EnvironmentGetTemplateVariantsTest extends TemplateTest {
                     + "<@i2.imp2M><@tNames /></@>\n"
                     + "]"
                 + "</#macro>"
-                + "<#function impF>"
+                + "<#function impF()>"
                     + "<@tNames />"
                     + "<#return lastTNamesResult>"
                 + "</#function>"

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/java/org/apache/freemarker/core/ListErrorsTest.java
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/ListErrorsTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/ListErrorsTest.java
index 67d8f59..8986efe 100644
--- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/ListErrorsTest.java
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/ListErrorsTest.java
@@ -88,7 +88,7 @@ public class ListErrorsTest extends TemplateTest {
                 "?index", "foo" , "no loop variable");
         assertErrorContains("<#list foos as foo><#macro m>${foo?index}</#macro></#list>",
                 "?index", "foo" , "no loop variable");
-        assertErrorContains("<#list foos as foo><#function f>${foo?index}</#function></#list>",
+        assertErrorContains("<#list foos as foo><#function f()>${foo?index}</#function></#list>",
                 "?index", "foo" , "no loop variable");
         assertErrorContains("<#list xs as x>${foo?index}</#list>",
                 "?index", "foo" , "no loop variable");

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/java/org/apache/freemarker/core/ParsingErrorMessagesTest.java
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/ParsingErrorMessagesTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/ParsingErrorMessagesTest.java
index 6b77d97..77dbe0a 100644
--- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/ParsingErrorMessagesTest.java
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/ParsingErrorMessagesTest.java
@@ -62,7 +62,7 @@ public class ParsingErrorMessagesTest {
     @Test
     public void testUnclosedDirectives() {
         assertErrorContains("<#macro x>", "#macro", "unclosed");
-        assertErrorContains("<#function x>", "#macro", "unclosed");
+        assertErrorContains("<#function x()>", "#macro", "unclosed");
         assertErrorContains("<#assign x>", "#assign", "unclosed");
         assertErrorContains("<#macro m><#local x>", "#local", "unclosed");
         assertErrorContains("<#global x>", "#global", "unclosed");

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/java/org/apache/freemarker/core/RemovedFM2SyntaxTest.java
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/RemovedFM2SyntaxTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/RemovedFM2SyntaxTest.java
index cd9923d..40f684e 100644
--- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/RemovedFM2SyntaxTest.java
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/RemovedFM2SyntaxTest.java
@@ -48,4 +48,18 @@ public class RemovedFM2SyntaxTest extends TemplateTest {
         assertOutput("<#macro m><#nested 1, 2></#macro>", "");
     }
 
+    @Test
+    public void testDefinitionSyntax() throws IOException, TemplateException {
+        assertOutput("<#macro m a b></#macro>", "");
+        assertErrorContains("<#macro m a, b></#macro>", ParseException.class);
+        assertErrorContains("<#macro m(a b)></#macro>", ParseException.class);
+        assertErrorContains("<#macro m a b)></#macro>", ParseException.class);
+
+        assertOutput("<#function f(a, b)><#return 0></#function>", "");
+        assertErrorContains("<#function f(a, b,)><#return 0></#function>", ParseException.class);
+        assertErrorContains("<#function f(a, b><#return 0></#function>", ParseException.class);
+        assertErrorContains("<#function f(a b)><#return 0></#function>", ParseException.class);
+        assertErrorContains("<#function f a, b><#return 0></#function>", ParseException.class);
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/java/org/apache/freemarker/core/TheadInterruptingSupportTest.java
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/TheadInterruptingSupportTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/TheadInterruptingSupportTest.java
index a0174b3..003f66a 100644
--- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/TheadInterruptingSupportTest.java
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/TheadInterruptingSupportTest.java
@@ -53,7 +53,8 @@ public class TheadInterruptingSupportTest {
         assertCanBeInterrupted("<@customLoopDirective>x</@>");
         assertCanBeInterrupted("<@customLoopDirective><#if true>x</#if></@>");
         assertCanBeInterrupted("<#macro selfCalling><@sleepDirective/><@selfCalling /></#macro><@selfCalling />");
-        assertCanBeInterrupted("<#function selfCalling><@sleepDirective/>${selfCalling()}</#function>${selfCalling()}");
+        assertCanBeInterrupted("<#function selfCalling()><@sleepDirective/>${selfCalling()}</#function>"
+                + "${selfCalling()}");
         assertCanBeInterrupted("<#list 1.. as _><#attempt><@sleepDirective/><#recover>suppress</#attempt></#list>");
         assertCanBeInterrupted("<#attempt><#list 1.. as _></#list><#recover>suppress</#attempt>");
     }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/ast-1.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/ast-1.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/ast-1.ftl
index f57e65e..f9af656 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/ast-1.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/ast-1.ftl
@@ -22,7 +22,7 @@
 4 <#if x + 1 == 0>foo${y}bar<#else>${"static"}${'x${baaz * 10}y'}</#if>
 5 <#switch x><#case 1>one<#case 2>two<#default>more</#switch>
 6 <#macro foo x y=2 z=y+1 q...><#nested x, y></#macro>
-7 <#function foo x y><#local x = 123><#return 1></#function>
+7 <#function foo(x, y)><#local x = 123><#return 1></#function>
 8 <#list xs as x></#list>
 9 <#list xs>[<#items as x>${x}<#sep>, </#items>]<#else>None</#list>
 10 <#-- A comment -->

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/cano-identifier-escaping.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/cano-identifier-escaping.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/cano-identifier-escaping.ftl
index 9ce7403..41923cc 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/cano-identifier-escaping.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/cano-identifier-escaping.ftl
@@ -25,7 +25,7 @@
 
 <@m\-a data\-color="red"; loop\-var, loopVar2>${loop\-var}</@>
 
-<#function f\-a p\-a>
+<#function f\-a(p\-a)>
     <#return p\-a + " works">
 </#function>
 ${f\-a("f-a")}

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/existence-operators.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/existence-operators.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/existence-operators.ftl
index d517557..d8122bf 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/existence-operators.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/existence-operators.ftl
@@ -134,8 +134,8 @@ ${(callMacroFromExpression(attemptTest))!}
 
 <#macro isIRE><@assertFails exception="InvalidReferenceException"><#nested></@></#macro>
 <#macro isNonFastIRE><@assertFails exception="InvalidReferenceException" message="Tip:"><#nested></@></#macro>
-<#function isNonFastIREMessage msg><#return msg?contains('Tip:') && msg?contains('null or missing')></#function>
-<#function callMacroFromExpression m>
+<#function isNonFastIREMessage(msg)><#return msg?contains('Tip:') && msg?contains('null or missing')></#function>
+<#function callMacroFromExpression(m)>
   <#local captured><@m /></#local>
   <#return captured>
 </#function>

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/identifier-escaping.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/identifier-escaping.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/identifier-escaping.ftl
index 372994a..b221deb 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/identifier-escaping.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/identifier-escaping.ftl
@@ -25,7 +25,7 @@
 
 <@m\-a data\-color="red"; loop\-var>${loop\-var}</@>
 
-<#function f\-a p\-a>
+<#function f\-a(p\-a)>
     <#return p\-a + " works">
 </#function>
 ${f\-a("f-a")}

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/import_lib.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/import_lib.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/import_lib.ftl
index 3329af9..68ba07f 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/import_lib.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/import_lib.ftl
@@ -24,7 +24,7 @@
   </#if>
 </#macro>
 
-<#function doubleUp foo>
+<#function doubleUp(foo)>
    <#return foo+foo>
 </#function>
 

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/list2.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/list2.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/list2.ftl
index db64a4b..6ac21d5 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/list2.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/list2.ftl
@@ -80,7 +80,7 @@
 --
 </#macro>
 
-<#function resolve xs>
+<#function resolve(xs)>
     <#assign resolveCallCnt = (resolveCallCnt!0) + 1>
     <#if xs?isMethod>
         <#return xs()>

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/macros.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/macros.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/macros.ftl
index 9ceaa43..b746321 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/macros.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/macros.ftl
@@ -31,7 +31,7 @@
 <#assign urls = {"home" : "/home.html", "about" : "/about.html"}>
 <#assign images = {"home" : "/images/home.png", "about" : "/image/about-us.jpeg"}>
 <#assign preferences = {"showImages" : true}>
-<#assign español = français><#macro français(url, image, alt)>
+<#assign español = français><#macro français url image alt>
     <#local var = "Kilroy">
     <a href="${url}">
     <#if preferences.showImages>
@@ -63,7 +63,7 @@
 
 <p>A recursive function call:</p>
 
-<#macro recurse(dummy, a=3)>
+<#macro recurse dummy a=3>
     <#if (a > 0)>
         <@recurse dummy, a - 1 />
     </#if>

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/range-common.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/range-common.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/range-common.ftl
index 142435b..bda67de 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/range-common.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/range-common.ftl
@@ -17,7 +17,7 @@
   under the License.
 -->
 <#-- A version of "?join" that fails at null-s in the sequence: -->
-<#function join seq sep=''>
+<#function join(seq, sep='')>
   <#local r = "">
   <#list seq as i>
     <#local r = r + i>

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/range.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/range.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/range.ftl
index d2f5450..18c035d 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/range.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/range.ftl
@@ -38,7 +38,7 @@
 <@assertEquals actual=r[0] expected=-2 />
 <@assertEquals actual=r[1] expected=-1 />
 
-<#function limitedJoin range limit>
+<#function limitedJoin(range, limit)>
 	<#assign joined="">
 	<#list range as i>
 		<#assign joined = joined + i?c>

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/switch-builtin.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/switch-builtin.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/switch-builtin.ftl
index 7441733..f79f639 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/switch-builtin.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/switch-builtin.ftl
@@ -37,7 +37,7 @@
 </#list>
 <@assertEquals expected="low;low;medium;medium;high;high;" actual=out />
 
-<#function f x>
+<#function f(x)>
   <#assign fInvocationCnt++>
   <#return x>
 </#function>

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/then-builtin.ftl
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/then-builtin.ftl b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/then-builtin.ftl
index de809e1..ecd18ac 100644
--- a/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/then-builtin.ftl
+++ b/freemarker-core-test/src/test/resources/org/apache/freemarker/core/templatesuite/templates/then-builtin.ftl
@@ -26,12 +26,12 @@
 <@assertEquals expected=2 actual=f1InvocationCnt />
 <@assertEquals expected=2 actual=f2InvocationCnt />
 
-<#function f1>
+<#function f1()>
   <#assign f1InvocationCnt++>
   <#return "f1 " + f1InvocationCnt>
 </#function>
 
-<#function f2>
+<#function f2()>
   <#assign f2InvocationCnt++>
   <#return "f2 " + f2InvocationCnt>
 </#function>

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4306b7d7/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 cedc199..13655d6 100644
--- a/freemarker-core/src/main/javacc/FTL.jj
+++ b/freemarker-core/src/main/javacc/FTL.jj
@@ -2837,6 +2837,9 @@ ASTDirMacro Macro() :
     boolean isFunction = false, hasDefaults = false;
     boolean isCatchAll = false;
     String catchAll = null;
+    boolean hasOpenParen = false;
+    boolean hasComma = false;
+    boolean hasCloseParen = false;
 }
 {
     (
@@ -2856,9 +2859,26 @@ ASTDirMacro Macro() :
                 ? ((ASTExpStringLiteral) nameExp).getAsString()
                 : ((ASTExpVariable) nameExp).getName();
     }
-    [<OPEN_PAREN>]
+    [<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);
+        }
+    }
     (
-        arg = <ID> { defValue = null; }
+        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);
+            }
+            hasComma = false;
+        }
         [
             <ELLIPSIS> { isCatchAll = true; }
         ]
@@ -2870,7 +2890,7 @@ ASTDirMacro Macro() :
                 hasDefaults = true;
             }
         ]
-        [<COMMA>]
+        [<COMMA> { hasComma = true; } ]
         {
             if (catchAll != null) {
                 throw new ParseException(
@@ -2896,7 +2916,17 @@ ASTDirMacro Macro() :
             }
         }
     )*
-    [<CLOSE_PAREN>]
+    [<CLOSE_PAREN> { hasCloseParen = true; } ] {
+        if (hasComma) {
+            throw new ParseException("You can't have comma after the last parameter.", template, start);
+        }
+        if (hasCloseParen != hasOpenParen) {
+    		throw new ParseException(
+                    hasOpenParen ? "Missing closing \")\"."
+                            : "Closing \")\" without corresponding opening \"(\".",
+                            template, start);
+        }
+    }
     <DIRECTIVE_END>
     {
         // To prevent parser check loopholes like <#list ...><#macro ...><#break></#macro></#list>.



Mime
View raw message