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/07/01 06:47:00 UTC
[freemarker] branch 2.3-gae updated (364da18 -> 2d61970)
This is an automated email from the ASF dual-hosted git repository.
ddekany pushed a change to branch 2.3-gae
in repository https://gitbox.apache.org/repos/asf/freemarker.git.
from 364da18 Don't fall back to higher scope if the argument in a lambda expression is null. This allows reliably filtering/mapping the elements of a collection that contains some null-s.
new d3c7105 (tab_size was missing from setSetting JavaDoc)
new 2d61970 Added configuration option to disable fallback to higher scopes when accessing an iterator variable that's null (missing). This is a quick initial implementation, that may needs more checking, and need documentation.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/main/java/freemarker/core/BodyInstruction.java | 7 +-
src/main/java/freemarker/core/Configurable.java | 13 +++-
src/main/java/freemarker/core/Environment.java | 2 +-
src/main/java/freemarker/core/IteratorBlock.java | 10 ++-
.../java/freemarker/template/Configuration.java | 47 ++++++++++-
.../java/freemarker/core/NullTransparencyTest.java | 90 +++++++++++++++++++---
6 files changed, 148 insertions(+), 21 deletions(-)
[freemarker] 01/02: (tab_size was missing from setSetting JavaDoc)
Posted by dd...@apache.org.
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 d3c71052680dae461f8ef53936647d2368651c6c
Author: ddekany <dd...@apache.org>
AuthorDate: Sun Jun 30 22:45:11 2019 +0200
(tab_size was missing from setSetting JavaDoc)
---
src/main/java/freemarker/core/Configurable.java | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/main/java/freemarker/core/Configurable.java b/src/main/java/freemarker/core/Configurable.java
index 27e5285..1656ec0 100644
--- a/src/main/java/freemarker/core/Configurable.java
+++ b/src/main/java/freemarker/core/Configurable.java
@@ -2502,6 +2502,9 @@ public class Configurable {
* <br>String value: {@code "default"} (case insensitive) for the default, {@code "default_2_3_0"}
* for {@link freemarker.cache.TemplateNameFormat#DEFAULT_2_3_0}, {@code "default_2_4_0"} for
* {@link freemarker.cache.TemplateNameFormat#DEFAULT_2_4_0}.
+ *
+ * <li><p>{@code "tab_size"}:
+ * See {@link Configuration#setTabSize(int)}.
* </ul>
*
* <p><a name="fm_obe"></a>Regarding <em>object builder expressions</em> (used by the setting values where it was
[freemarker] 02/02: Added configuration option to disable fallback
to higher scopes when accessing an iterator variable that's null (missing).
This is a quick initial implementation, that may needs more checking,
and need documentation.
Posted by dd...@apache.org.
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 2d6197057b7d5e70737dbbe9e53fac3c0a4c207a
Author: ddekany <dd...@apache.org>
AuthorDate: Mon Jul 1 08:42:06 2019 +0200
Added configuration option to disable fallback to higher scopes when accessing an iterator variable that's null (missing). This is a quick initial implementation, that may needs more checking, and need documentation.
---
src/main/java/freemarker/core/BodyInstruction.java | 7 +-
src/main/java/freemarker/core/Configurable.java | 10 ++-
src/main/java/freemarker/core/Environment.java | 2 +-
src/main/java/freemarker/core/IteratorBlock.java | 10 ++-
.../java/freemarker/template/Configuration.java | 47 ++++++++++-
.../java/freemarker/core/NullTransparencyTest.java | 90 +++++++++++++++++++---
6 files changed, 145 insertions(+), 21 deletions(-)
diff --git a/src/main/java/freemarker/core/BodyInstruction.java b/src/main/java/freemarker/core/BodyInstruction.java
index d158822..7cedbfb 100644
--- a/src/main/java/freemarker/core/BodyInstruction.java
+++ b/src/main/java/freemarker/core/BodyInstruction.java
@@ -135,7 +135,12 @@ final class BodyInstruction extends TemplateElement {
if (bodyVars == null) {
bodyVars = env.new Namespace();
}
- bodyVars.put(bodyParameterName, tm);
+ bodyVars.put(
+ bodyParameterName,
+ tm != null
+ ? tm
+ : getTemplate().getConfiguration().getFallbackOnNullLoopVariable()
+ ? null : NullTemplateModel.INSTANCE);
}
}
}
diff --git a/src/main/java/freemarker/core/Configurable.java b/src/main/java/freemarker/core/Configurable.java
index 1656ec0..89f3445 100644
--- a/src/main/java/freemarker/core/Configurable.java
+++ b/src/main/java/freemarker/core/Configurable.java
@@ -2455,7 +2455,7 @@ public class Configurable {
* <br>Note that setting the {@code "tagSyntax"} to {@code "square_bracket"} does <em>not</em> change
* <code>${x}</code> to {@code [=...]}; that's <em>interpolation</em> syntax, so use the
* {@code "interpolation_syntax"} setting for that, not this setting.
- *
+ *
* <li><p>{@code "interpolation_syntax"} (since 2.3.28):
* See {@link Configuration#setInterpolationSyntax(int)}.
* <br>String value: Must be one of
@@ -2468,7 +2468,13 @@ public class Configurable {
* See {@link Configuration#setNamingConvention(int)}.
* <br>String value: Must be one of
* {@code "auto_detect"}, {@code "legacy"}, and {@code "camel_case"}.
- *
+ *
+ * <li><p>{@code "fallback_on_null_loop_variable"}:
+ * See {@link Configuration#setFallbackOnNullLoopVariable(boolean)}.
+ * <br>String value: {@code "true"}, {@code "false"} (also the equivalents: {@code "yes"}, {@code "no"},
+ * {@code "t"}, {@code "f"}, {@code "y"}, {@code "n"}).
+ * Case insensitive.
+ *
* <li><p>{@code "incompatible_improvements"}:
* See {@link Configuration#setIncompatibleImprovements(Version)}.
* <br>String value: version number like {@code 2.3.20}.
diff --git a/src/main/java/freemarker/core/Environment.java b/src/main/java/freemarker/core/Environment.java
index d1ebace..9553d5b 100644
--- a/src/main/java/freemarker/core/Environment.java
+++ b/src/main/java/freemarker/core/Environment.java
@@ -2048,7 +2048,7 @@ public final class Environment extends Configurable {
*
* @since 2.3.29
*/
- final TemplateModel getNullableLocalVariable(String name) throws TemplateModelException {
+ private final TemplateModel getNullableLocalVariable(String name) throws TemplateModelException {
if (localContextStack != null) {
for (int i = localContextStack.size() - 1; i >= 0; i--) {
LocalContext lc = localContextStack.get(i);
diff --git a/src/main/java/freemarker/core/IteratorBlock.java b/src/main/java/freemarker/core/IteratorBlock.java
index aaaaf64..e6d1173 100644
--- a/src/main/java/freemarker/core/IteratorBlock.java
+++ b/src/main/java/freemarker/core/IteratorBlock.java
@@ -461,8 +461,10 @@ final class IteratorBlock extends TemplateElement {
if (name.startsWith(visibleLoopVar1Name)) {
switch(name.length() - visibleLoopVar1Name.length()) {
- case 0:
- return loopVar1Value;
+ case 0:
+ return loopVar1Value != null ? loopVar1Value
+ : getTemplate().getConfiguration().getFallbackOnNullLoopVariable()
+ ? null : NullTemplateModel.INSTANCE;
case 6:
if (name.endsWith(LOOP_STATE_INDEX)) {
return new SimpleNumber(index);
@@ -477,7 +479,9 @@ final class IteratorBlock extends TemplateElement {
}
if (name.equals(loopVar2Name)) {
- return loopVar2Value;
+ return loopVar2Value != null ? loopVar2Value
+ : getTemplate().getConfiguration().getFallbackOnNullLoopVariable()
+ ? null : NullTemplateModel.INSTANCE;
}
return null;
diff --git a/src/main/java/freemarker/template/Configuration.java b/src/main/java/freemarker/template/Configuration.java
index 419332a..21e02a1 100644
--- a/src/main/java/freemarker/template/Configuration.java
+++ b/src/main/java/freemarker/template/Configuration.java
@@ -273,7 +273,7 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
public static final String TAB_SIZE_KEY_CAMEL_CASE = "tabSize";
/** Alias to the {@code ..._SNAKE_CASE} variation. @since 2.3.25 */
public static final String TAB_SIZE_KEY = TAB_SIZE_KEY_SNAKE_CASE;
-
+
/** Legacy, snake case ({@code like_this}) variation of the setting name. @since 2.3.23 */
public static final String TEMPLATE_LOADER_KEY_SNAKE_CASE = "template_loader";
/** Modern, camel case ({@code likeThis}) variation of the setting name. @since 2.3.23 */
@@ -315,12 +315,20 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
/** @deprecated Use {@link #INCOMPATIBLE_IMPROVEMENTS_KEY} instead. */
@Deprecated
public static final String INCOMPATIBLE_ENHANCEMENTS = "incompatible_enhancements";
-
+
+ /** Legacy, snake case ({@code like_this}) variation of the setting name. @since 2.3.29 */
+ public static final String FALLBACK_ON_NULL_LOOP_VARIABLE_KEY_SNAKE_CASE = "fallback_on_null_loop_variable";
+ /** Modern, camel case ({@code likeThis}) variation of the setting name. @since 2.3.29 */
+ public static final String FALLBACK_ON_NULL_LOOP_VARIABLE_KEY_CAMEL_CASE = "fallbackOnNullLoopVariable";
+ /** Alias to the {@code ..._SNAKE_CASE} variation. @since 2.3.25 */
+ public static final String FALLBACK_ON_NULL_LOOP_VARIABLE_KEY = FALLBACK_ON_NULL_LOOP_VARIABLE_KEY_SNAKE_CASE;
+
private static final String[] SETTING_NAMES_SNAKE_CASE = new String[] {
// Must be sorted alphabetically!
AUTO_ESCAPING_POLICY_KEY_SNAKE_CASE,
CACHE_STORAGE_KEY_SNAKE_CASE,
DEFAULT_ENCODING_KEY_SNAKE_CASE,
+ FALLBACK_ON_NULL_LOOP_VARIABLE_KEY_SNAKE_CASE,
INCOMPATIBLE_IMPROVEMENTS_KEY_SNAKE_CASE,
INTERPOLATION_SYNTAX_KEY_SNAKE_CASE,
LOCALIZED_LOOKUP_KEY_SNAKE_CASE,
@@ -344,6 +352,7 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
AUTO_ESCAPING_POLICY_KEY_CAMEL_CASE,
CACHE_STORAGE_KEY_CAMEL_CASE,
DEFAULT_ENCODING_KEY_CAMEL_CASE,
+ FALLBACK_ON_NULL_LOOP_VARIABLE_KEY_CAMEL_CASE,
INCOMPATIBLE_IMPROVEMENTS_KEY_CAMEL_CASE,
INTERPOLATION_SYNTAX_KEY_CAMEL_CASE,
LOCALIZED_LOOKUP_KEY_CAMEL_CASE,
@@ -536,6 +545,7 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
private int interpolationSyntax = LEGACY_INTERPOLATION_SYNTAX;
private int namingConvention = AUTO_DETECT_NAMING_CONVENTION;
private int tabSize = 8; // Default from JavaCC 3.x
+ private boolean fallbackOnNullLoopVariable = true; // Default for backward compatibility
private boolean preventStrippings;
private TemplateCache cache;
@@ -2573,7 +2583,35 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
public int getTabSize() {
return tabSize;
}
-
+
+ /**
+ * The getter pair of {@link #setFallbackOnNullLoopVariable(boolean)}.
+ *
+ * @since 2.3.29
+ */
+ public boolean getFallbackOnNullLoopVariable() {
+ return fallbackOnNullLoopVariable;
+ }
+
+ /**
+ * Specifies the behavior when reading a loop variable (like {@code i} in {@code <#list items as i>}) that's
+ * {@code null} (missing); if {@code true}, FreeMarker will look for a variable with the same name in higher
+ * variable scopes, or if {@code false} the variable will be simply {@code null} (missing).
+ * For backward compatibility the default is {@code true}. The recommended value for new projects is
+ * {@code false}, as otherwise adding new variables to higher scopes (typically to the data-model) can
+ * unintentionally change the behavior of templates. You have to be quite unlucky for that to happen though:
+ * The newly added variable has to have the same name as the loop variable, and there must be some null (missing)
+ * values in what you loop through.
+ *
+ * <p>Note that this doesn't influence the behavior of lambdas, like {@code items?filter(i -> i?hasContent)},
+ * because reading lambda arguments never fall back to higher scopes.
+ *
+ * @since 2.3.29
+ */
+ public void setFallbackOnNullLoopVariable(boolean fallbackOnNullLoopVariable) {
+ this.fallbackOnNullLoopVariable = fallbackOnNullLoopVariable;
+ }
+
/**
* Getter pair of {@link #setPreventStrippings(boolean)}.
*
@@ -3399,6 +3437,9 @@ public class Configuration extends Configurable implements Cloneable, ParserConf
setTemplateConfigurations((TemplateConfigurationFactory) _ObjectBuilderSettingEvaluator.eval(
value, TemplateConfigurationFactory.class, false, _SettingEvaluationEnvironment.getCurrent()));
}
+ } else if (FALLBACK_ON_NULL_LOOP_VARIABLE_KEY_SNAKE_CASE.equals(name)
+ || FALLBACK_ON_NULL_LOOP_VARIABLE_KEY_CAMEL_CASE.equals(name)) {
+ setFallbackOnNullLoopVariable(StringUtil.getYesNo(value));
} else {
unknown = true;
}
diff --git a/src/test/java/freemarker/core/NullTransparencyTest.java b/src/test/java/freemarker/core/NullTransparencyTest.java
index 2f99d0b..24a9792 100644
--- a/src/test/java/freemarker/core/NullTransparencyTest.java
+++ b/src/test/java/freemarker/core/NullTransparencyTest.java
@@ -19,27 +19,95 @@
package freemarker.core;
+import static org.junit.Assert.*;
+
+import java.io.IOException;
import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
import org.junit.Test;
+import freemarker.template.TemplateException;
import freemarker.test.TemplateTest;
public class NullTransparencyTest extends TemplateTest {
+ @Override
+ protected Object createDataModel() {
+ Map<String, Object> dataModel = new HashMap<String, Object>();
+
+ List<String> list = new ArrayList<String>();
+ list.add("a");
+ list.add(null);
+ list.add("b");
+ dataModel.put("list", list);
+
+ Map<String, String> map = new LinkedHashMap<String, String>();
+ map.put("ak", "av");
+ map.put(null, "bv");
+ map.put("ck", null);
+ dataModel.put("map", map);
+
+ return dataModel;
+ }
+
+ @Test
+ public void testWithoutClashingHigherScopeVar() throws Exception {
+
+ assertTrue(getConfiguration().getFallbackOnNullLoopVariable());
+ testLambdaArguments();
+ testLoopVariables("null");
+
+ getConfiguration().setFallbackOnNullLoopVariable(false);
+ testLambdaArguments();
+ testLoopVariables("null");
+ }
+
@Test
- public void test() throws Exception {
- List<String> xs = new ArrayList<String>();
- xs.add("a");
- xs.add(null);
- xs.add("b");
- addToDataModel("xs", xs);
- assertOutput("<#list xs?filter(it -> it??) as it>${it!'null'}<#sep>, </#list>", "a, b");
- assertOutput("<#list xs?map(it -> it!'null') as it>${it}<#sep>, </#list>", "a, null, b");
-
- // Lambdas are always use non-transparent nulls, as there was no backwrad compatibility constraint:
- assertOutput("<#assign it='fallback'><#list xs?map(it -> it!'null') as it>${it}<#sep>, </#list>",
+ public void testWithClashingHigherScopeVar() throws Exception {
+ addToDataModel("it", "fallback");
+
+ assertTrue(getConfiguration().getFallbackOnNullLoopVariable());
+ testLambdaArguments();
+ testLoopVariables("fallback");
+
+ getConfiguration().setFallbackOnNullLoopVariable(false);
+ testLambdaArguments();
+ testLoopVariables("null");
+ }
+
+ // Lambdas arguments never fall back on null, as there was no backward compatibility constraint:
+ protected void testLambdaArguments() throws IOException, TemplateException {
+ assertOutput("<#list list?filter(it -> it??) as it>${it!'null'}<#sep>, </#list>",
+ "a, b");
+ assertOutput("<#list list?map(it -> it!'null') as it>${it}<#sep>, </#list>",
"a, null, b");
}
+
+ // Loop variables by default fallback on null, for backward compatibility
+ protected void testLoopVariables(String expectedFallback) throws IOException, TemplateException {
+ assertOutput("<#list list as it>${it!'null'}<#sep>, </#list>",
+ "a, " + expectedFallback + ", b");
+
+ assertOutput("<#list map?values as it>${it!'null'}<#sep>, </#list>",
+ "av, bv, " + expectedFallback);
+ assertOutput("<#list map as k, it>${k!'null'}=${it!'null'}<#sep>, </#list>",
+ "ak=av, null=bv, ck=" + expectedFallback);
+
+ assertOutput("<#list map?keys as it>${it!'null'}<#sep>, </#list>",
+ "ak, " + expectedFallback + ", ck");
+ assertOutput("<#list map as it, v>${it!'null'}=${v!'null'}<#sep>, </#list>",
+ "ak=av, " + expectedFallback + "=bv, ck=null");
+
+ // TODO #item
+
+ assertOutput("" +
+ "<#macro loop><#nested 1>, <#nested totallyMissing></#macro>\n" +
+ "<@loop; it>${it!'null'}</...@loop>",
+ "1, " + expectedFallback);
+ }
+
}