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 <[email protected]> Authored: Thu Jul 20 17:21:03 2017 +0200 Committer: ddekany <[email protected]> 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>.
