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/08/25 20:32:06 UTC
[freemarker] branch 3 updated (60944fc -> e11170a)
This is an automated email from the ASF dual-hosted git repository.
ddekany pushed a change to branch 3
in repository https://gitbox.apache.org/repos/asf/freemarker.git.
from 60944fc Increased minimum Java version to 8
new 310cb70 Bit of cleanup to decrease the number of ways InvalidReferenceException can be constructed
new e11170a Very basic (incomplete) support for TemplateNullModel, and the null literal in the template language. This change also means that reading null loop variables will never fall back to higher scopes anymore.
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:
FM3-CHANGE-LOG.txt | 7 ++
.../apache/freemarker/core/NullLiteralTest.java | 54 ++++++++++++++
.../freemarker/core/NullTransparencyTest.java | 84 ++++++++++++++++++++++
.../org/apache/freemarker/core/ASTDirList.java | 9 ++-
.../org/apache/freemarker/core/ASTExpDefault.java | 6 +-
.../org/apache/freemarker/core/ASTExpExists.java | 4 +-
...pBooleanLiteral.java => ASTExpNullLiteral.java} | 25 ++-----
.../apache/freemarker/core/BuiltInForHashEx.java | 22 +++---
.../apache/freemarker/core/BuiltInsForHashes.java | 8 ++-
.../org/apache/freemarker/core/Environment.java | 61 +++++++++++-----
.../freemarker/core/InvalidReferenceException.java | 6 +-
.../org/apache/freemarker/core/_EvalUtils.java | 15 ++--
.../freemarker/core/model/TemplateNullModel.java | 46 ++++++++++++
.../core/model/impl/DefaultObjectWrapper.java | 4 +-
.../core/util/TemplateLanguageUtils.java | 5 ++
freemarker-core/src/main/javacc/FTL.jj | 51 ++++++++++---
16 files changed, 325 insertions(+), 82 deletions(-)
create mode 100644 freemarker-core-test/src/test/java/org/apache/freemarker/core/NullLiteralTest.java
create mode 100644 freemarker-core-test/src/test/java/org/apache/freemarker/core/NullTransparencyTest.java
copy freemarker-core/src/main/java/org/apache/freemarker/core/{ASTExpBooleanLiteral.java => ASTExpNullLiteral.java} (72%)
create mode 100644 freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateNullModel.java
[freemarker] 02/02: Very basic (incomplete) support for
TemplateNullModel,
and the null literal in the template language. This change also means that
reading null loop variables will never fall back to higher scopes anymore.
Posted by dd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
ddekany pushed a commit to branch 3
in repository https://gitbox.apache.org/repos/asf/freemarker.git
commit e11170a84fb9a25c1bf8289afaf563fc0f7cc12a
Author: ddekany <dd...@apache.org>
AuthorDate: Sun Aug 25 17:29:54 2019 +0200
Very basic (incomplete) support for TemplateNullModel, and the null literal in the template language. This change also means that reading null loop variables will never fall back to higher scopes anymore.
---
FM3-CHANGE-LOG.txt | 7 ++
.../apache/freemarker/core/NullLiteralTest.java | 54 ++++++++++++++
.../freemarker/core/NullTransparencyTest.java | 84 ++++++++++++++++++++++
.../org/apache/freemarker/core/ASTDirList.java | 9 ++-
.../org/apache/freemarker/core/ASTExpDefault.java | 6 +-
.../org/apache/freemarker/core/ASTExpExists.java | 4 +-
.../apache/freemarker/core/ASTExpNullLiteral.java | 71 ++++++++++++++++++
.../org/apache/freemarker/core/Environment.java | 61 +++++++++++-----
.../org/apache/freemarker/core/_EvalUtils.java | 7 +-
.../freemarker/core/model/TemplateNullModel.java | 46 ++++++++++++
.../core/model/impl/DefaultObjectWrapper.java | 4 +-
.../core/util/TemplateLanguageUtils.java | 5 ++
freemarker-core/src/main/javacc/FTL.jj | 51 ++++++++++---
13 files changed, 372 insertions(+), 37 deletions(-)
diff --git a/FM3-CHANGE-LOG.txt b/FM3-CHANGE-LOG.txt
index c532c5c..20bbf95 100644
--- a/FM3-CHANGE-LOG.txt
+++ b/FM3-CHANGE-LOG.txt
@@ -365,6 +365,8 @@ Core / Configuration
- Removed hasCustomFormats() from configuration related API-s (we don't need it anymore)
- The tagSyntax and interpolationSyntax settings were moved inside the templateLanguage setting, as they are only
applicable if the templateLanguage is a DefaultTemplateLanguage instance.
+- Removed Configuration.fallbackOnNullLoopVariable, FM3 never falls back on such loop variables (as if the FM2
+ setting was set to false)
Core / Models and Object wrapping
@@ -488,6 +490,11 @@ Core / Models and Object wrapping
Thus, `${obj.voidReturningMethod()}` still works (it prints nothing, just as in FM2), but things like
`x + obj.voidReturningMethod()` now fail (unlike in FM2), as they are probably oversights. This all applies to
Java methods wrapped by the DefaultObjectWrapper (which, in FM3, is also used in place of the FM2 BeansWrapper).
+- Added `null` literal (so null is now a keyword), and TemplateNullModel (it wasn't public in FM2).
+ [TODO] This change is incomplete, and eventually it will mean that the only legal way of representing a
+ Java null where a TemplateModel is expected will be TemplateNullModel.INSTANCE, not a Java null.
+- Whem listing something that contains a null, reading the loop variable will never fall back to higher scopes
+ (like when configuration.fallbackOnNullLoopVariable was set to false in FM2)
Core / Template loading and caching
...................................
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/NullLiteralTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/NullLiteralTest.java
new file mode 100644
index 0000000..5509ad4
--- /dev/null
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/NullLiteralTest.java
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.freemarker.core;
+
+import java.io.IOException;
+
+import org.apache.freemarker.test.TemplateTest;
+import org.junit.Test;
+
+public class NullLiteralTest extends TemplateTest {
+
+ @SuppressWarnings("ThrowableNotThrown")
+ @Test
+ public void basicTest() throws Exception {
+ assertOutput("<#list ['a', null, 'b']><#items as it>${it!'-'}<#sep>, </#items></#list>",
+ "a, -, b");
+ assertOutput("<#assign m = {'a': 'A', 'b': null}>a=${m.a}, b=${m.b!'-'}",
+ "a=A, b=-");
+ assertOutput("<#assign a = 'A'><#assign b=null>a=${a}, b=${b!'-'}",
+ "a=A, b=-");
+
+ addToDataModel("obj", new TestBean());
+ assertOutput("${obj.m(1)}, ${obj.m(null)}", "n=1, n=null");
+
+ assertErrorContains("${null + 1}", InvalidReferenceException.class);
+ assertErrorContains("${1 + null}", InvalidReferenceException.class);
+ assertErrorContains("${null + 's'}", InvalidReferenceException.class);
+ assertErrorContains("${'s' + null}", InvalidReferenceException.class);
+ assertErrorContains("${null + null}", InvalidReferenceException.class);
+ }
+
+ public static class TestBean {
+ public String m(Integer n) {
+ return "n=" + n;
+ }
+ }
+}
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/NullTransparencyTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/NullTransparencyTest.java
new file mode 100644
index 0000000..93ea669
--- /dev/null
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/NullTransparencyTest.java
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.freemarker.core;
+
+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.apache.freemarker.test.TemplateTest;
+import org.junit.Test;
+
+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 testLoopVariables() throws IOException, TemplateException {
+ addToDataModel("it", "fallback");
+
+ assertOutput("<#list list as it>${it!'null'}<#sep>, </#list>",
+ "a, null, b");
+ assertOutput("<#list list><#items as it>${it!'null'}<#sep>, </#items></#list>",
+ "a, null, b");
+
+ assertOutput("<#list map?values as it>${it!'null'}<#sep>, </#list>",
+ "av, bv, null");
+ assertOutput("<#list map as k, it>${k!'null'}=${it!'null'}<#sep>, </#list>",
+ "ak=av, null=bv, ck=null");
+ assertOutput("<#list map><#items as k, it>${k!'null'}=${it!'null'}<#sep>, </#items></#list>",
+ "ak=av, null=bv, ck=null");
+
+ assertOutput("<#list map?keys as it>${it!'null'}<#sep>, </#list>",
+ "ak, null, ck");
+ assertOutput("<#list map as it, v>${it!'null'}=${v!'null'}<#sep>, </#list>",
+ "ak=av, null=bv, ck=null");
+ assertOutput("<#list map><#items as it, v>${it!'null'}=${v!'null'}<#sep>, </#items></#list>",
+ "ak=av, null=bv, ck=null");
+
+ assertOutput("" +
+ "<#macro loop><#nested 1>, <#nested totallyMissing></#macro>\n" +
+ "<@loop; it>${it!'null'}</...@loop>",
+ "1, null");
+ }
+
+}
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirList.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirList.java
index 362b117..1bb2b9a 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirList.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirList.java
@@ -29,6 +29,8 @@ import org.apache.freemarker.core.model.TemplateHashModelEx;
import org.apache.freemarker.core.model.TemplateIterableModel;
import org.apache.freemarker.core.model.TemplateModel;
import org.apache.freemarker.core.model.TemplateModelIterator;
+import org.apache.freemarker.core.model.TemplateNodeModel;
+import org.apache.freemarker.core.model.TemplateNullModel;
import org.apache.freemarker.core.model.impl.SimpleNumber;
import org.apache.freemarker.core.util._StringUtils;
@@ -353,8 +355,9 @@ final class ASTDirList extends ASTDirective {
String nestedContentParamName = this.nestedContentParam1Name;
if (name.startsWith(nestedContentParamName)) {
switch(name.length() - nestedContentParamName.length()) {
- case 0:
- return nestedContentParam;
+ case 0:
+ // TODO [FM3][null] Later nestedContentParam == null will mean undefined, which should be an error
+ return nestedContentParam != null ? nestedContentParam : TemplateNullModel.INSTANCE;
case 6:
if (name.endsWith(LOOP_STATE_INDEX)) {
return new SimpleNumber(index);
@@ -369,7 +372,7 @@ final class ASTDirList extends ASTDirective {
}
if (name.equals(nestedContentParam2Name)) {
- return nestedContentParam2;
+ return nestedContentParam2 != null ? nestedContentParam2 : TemplateNullModel.INSTANCE;
}
return null;
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpDefault.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpDefault.java
index 0aac874..bac976c 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpDefault.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpDefault.java
@@ -31,6 +31,7 @@ import org.apache.freemarker.core.model.TemplateFunctionModel;
import org.apache.freemarker.core.model.TemplateHashModelEx;
import org.apache.freemarker.core.model.TemplateModel;
import org.apache.freemarker.core.model.TemplateModelIterator;
+import org.apache.freemarker.core.model.TemplateNullModel;
import org.apache.freemarker.core.model.TemplateSequenceModel;
import org.apache.freemarker.core.model.TemplateStringModel;
@@ -157,8 +158,9 @@ class ASTExpDefault extends ASTExpression {
} else {
left = lho.eval(env);
}
-
- if (left != null) return left;
+
+ // TODO [FM3][null] Later left == null will mean undefined, which we shouldn't handle with this operator
+ if (left != null && left != TemplateNullModel.INSTANCE) return left;
else if (rho == null) return EMPTY_STRING_AND_SEQUENCE_AND_HASH;
else return rho.eval(env);
}
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpExists.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpExists.java
index ce96532..b5b9d41 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpExists.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpExists.java
@@ -22,6 +22,7 @@ package org.apache.freemarker.core;
import org.apache.freemarker.core.model.TemplateBooleanModel;
import org.apache.freemarker.core.model.TemplateModel;
+import org.apache.freemarker.core.model.TemplateNullModel;
/**
* AST expression node: {@code ??} operator.
@@ -49,7 +50,8 @@ class ASTExpExists extends ASTExpression {
} else {
tm = exp.eval(env);
}
- return tm == null ? TemplateBooleanModel.FALSE : TemplateBooleanModel.TRUE;
+ // TODO [FM3][null] null will mean undefined, which we shouldn't handle here
+ return tm == null || tm == TemplateNullModel.INSTANCE ? TemplateBooleanModel.FALSE : TemplateBooleanModel.TRUE;
}
@Override
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpNullLiteral.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpNullLiteral.java
new file mode 100644
index 0000000..bc52d0e
--- /dev/null
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpNullLiteral.java
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.freemarker.core;
+
+import org.apache.freemarker.core.model.TemplateNullModel;
+import org.apache.freemarker.core.model.TemplateModel;
+
+/**
+ * AST expression node: boolean literal
+ */
+final class ASTExpNullLiteral extends ASTExpression {
+
+ @Override
+ public String getCanonicalForm() {
+ return "null";
+ }
+
+ @Override
+ public String getLabelWithoutParameters() {
+ return getCanonicalForm();
+ }
+
+ @Override
+ TemplateModel _eval(Environment env) {
+ return TemplateNullModel.INSTANCE;
+ }
+
+ @Override
+ boolean isLiteral() {
+ return true;
+ }
+
+ @Override
+ ASTExpression deepCloneWithIdentifierReplaced_inner(
+ String replacedIdentifier, ASTExpression replacement, ReplacemenetState replacementState) {
+ return new ASTExpNullLiteral();
+ }
+
+ @Override
+ int getParameterCount() {
+ return 0;
+ }
+
+ @Override
+ Object getParameterValue(int idx) {
+ throw new IndexOutOfBoundsException();
+ }
+
+ @Override
+ ParameterRole getParameterRole(int idx) {
+ throw new IndexOutOfBoundsException();
+ }
+
+}
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/Environment.java b/freemarker-core/src/main/java/org/apache/freemarker/core/Environment.java
index f0138c8..ddddf4b 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/Environment.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/Environment.java
@@ -59,6 +59,7 @@ import org.apache.freemarker.core.model.TemplateModel;
import org.apache.freemarker.core.model.TemplateModelIterator;
import org.apache.freemarker.core.model.TemplateModelWithOriginName;
import org.apache.freemarker.core.model.TemplateNodeModel;
+import org.apache.freemarker.core.model.TemplateNullModel;
import org.apache.freemarker.core.model.TemplateNumberModel;
import org.apache.freemarker.core.model.TemplateSequenceModel;
import org.apache.freemarker.core.model.TemplateStringModel;
@@ -468,7 +469,12 @@ public final class Environment extends MutableProcessingConfiguration<Environmen
@Override
public TemplateModel getLocalVariable(String name) throws TemplateException {
int index = nestedContentParamNames.get(name);
- return index != -1 ? nestedContentParamValues[index] : null;
+ if (index == -1) {
+ return null;
+ }
+ TemplateModel paramValue = nestedContentParamValues[index];
+ // TODO [FM3][null] Later paramValue == null will mean undefined, which should be an error
+ return paramValue != null ? paramValue : TemplateNullModel.INSTANCE;
}
@Override
@@ -1971,7 +1977,7 @@ public final class Environment extends MutableProcessingConfiguration<Environmen
* (Note that the misnomer is kept for backward compatibility: nested content parameters are not local variables
* according to our terminology.)
*/
- // TODO [FM3] Don't return nested content params anymore (see JavaDoc)
+ // TODO [FM3] Should be called getBlockVariable (see JavaDoc)
public TemplateModel getLocalVariable(String name) throws TemplateException {
if (localContextStack != null) {
for (int i = localContextStack.size() - 1; i >= 0; i--) {
@@ -2004,29 +2010,53 @@ public final class Environment extends MutableProcessingConfiguration<Environmen
*/
public TemplateModel getVariable(String name) throws TemplateException {
TemplateModel result = getLocalVariable(name);
- if (result == null) {
- result = currentNamespace.get(name);
+ if (result != null) {
+ return result != TemplateNullModel.INSTANCE ? result : null;
}
- if (result == null) {
- result = getGlobalVariable(name);
+
+ result = currentNamespace.get(name);
+ if (result != null) {
+ return result;
}
- return result;
+
+ return getGlobalVariable(name);
}
/**
* Returns the globally visible variable of the given name (or null). This is correspondent to FTL
* <code>.globals.<i>name</i></code>. This will first look at variables that were assigned globally via: <#global
- * ...> and then at the data model exposed to the template.
+ * ...> and then at the data model exposed to the template, and then at the
+ * {@linkplain Configuration.Builder#setSharedVariables(Map)} shared variables} in the {@link Configuration}.
*/
public TemplateModel getGlobalVariable(String name) throws TemplateException {
- TemplateModel result = globalNamespace.get(name);
- if (result == null) {
- result = rootDataModel.get(name);
+ TemplateModel globalVal = globalNamespace.get(name);
+ // Null model stops fallback, so <#global foo = null> will stop the fallback to the // data-model.
+ if (globalVal != null) {
+ return globalVal;
}
- if (result == null) {
- result = configuration.getWrappedSharedVariable(name);
+
+ return getDataModelOrSharedVariable(name);
+ }
+
+ /**
+ * Returns the variable from the data-model, or if it's not there, then from the
+ * {@linkplain Configuration.Builder#setSharedVariables(Map)} shared variables}
+ */
+ public TemplateModel getDataModelOrSharedVariable(String name) throws TemplateException {
+ TemplateModel dataModelVal = rootDataModel.get(name);
+ // As the data-model is typically a Map, we fall back even if it returns a null model.
+ // TODO [FM3] What if this is a "strict" map (a bean), and so we want null model to not fall back?
+ if (dataModelVal != null && dataModelVal != TemplateNullModel.INSTANCE) {
+ return dataModelVal;
}
- return result;
+
+ TemplateModel sharedVal = configuration.getWrappedSharedVariable(name);
+ if (sharedVal != null) {
+ return sharedVal;
+ }
+
+ // If the data model was "strict", we stay strict, otherwise we mimic Map logic.
+ return dataModelVal == null ? null : TemplateNullModel.INSTANCE;
}
/**
@@ -2330,8 +2360,7 @@ public final class Environment extends MutableProcessingConfiguration<Environmen
@Override
public TemplateModel get(String key) throws TemplateException {
- TemplateModel value = rootDataModel.get(key);
- return value != null ? value : configuration.getWrappedSharedVariable(key);
+ return getDataModelOrSharedVariable(key);
}
// NB: The methods below do not take into account
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/_EvalUtils.java b/freemarker-core/src/main/java/org/apache/freemarker/core/_EvalUtils.java
index 69e30ea..1f87d8c 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/_EvalUtils.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/_EvalUtils.java
@@ -33,6 +33,7 @@ import org.apache.freemarker.core.model.TemplateDateModel;
import org.apache.freemarker.core.model.TemplateIterableModel;
import org.apache.freemarker.core.model.TemplateMarkupOutputModel;
import org.apache.freemarker.core.model.TemplateModel;
+import org.apache.freemarker.core.model.TemplateNullModel;
import org.apache.freemarker.core.model.TemplateNumberModel;
import org.apache.freemarker.core.model.TemplateStringModel;
import org.apache.freemarker.core.model.WrapperTemplateModel;
@@ -454,13 +455,13 @@ public class _EvalUtils {
throws TemplateException {
if (tm instanceof TemplateStringModel) {
return modelToString((TemplateStringModel) tm, exp);
- } else if (tm == null) {
+ } else if (tm == null || tm == TemplateNullModel.INSTANCE) { // TODO [FM3][null] null shouldn't reach this
throw InvalidReferenceException.getInstance(exp, env);
} else if (tm instanceof TemplateBooleanModel) {
- // TODO [FM3] It would be more logical if it's before TemplateStringModel (number etc. are before it as
+ // TODO [FM3][null] Handing booleans should be before TemplateStringModel (number etc. are before it as
// well). But currently, in FM3, `exp!` returns a multi-typed value that's also a boolean `false`, and so
// `${missing!}` wouldn't print `""` anymore if we reorder these. But, if and when `null` handling is
- // reworked ("checked nulls"), this problem should go away, and so we should move this.
+ // reworked ("checked nulls"), this problem should go away, and so we should move this.
boolean booleanValue = ((TemplateBooleanModel) tm).getAsBoolean();
return env.formatBoolean(booleanValue);
} else {
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateNullModel.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateNullModel.java
new file mode 100644
index 0000000..d7bbb8d
--- /dev/null
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateNullModel.java
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.freemarker.core.model;
+
+/**
+ * Where an API declares {@link TemplateModel} (or a subclass of it), a {@link TemplateNullModel#INSTANCE} represents
+ * the Java {@code null} value, and {@code null} represents that the value (or rather what holds it) doesn't exist
+ * at all.
+ *
+ * <p>Some examples to make this more understandable:<p>
+ * <ul>
+ * <li>If you try to read the variable {@code myVar}, but such a variable was never declared anywhere, then that's
+ * a {@code null}. If the variable itself exists, but was set to {@code null}, then that's a
+ * {@link TemplateNullModel#INSTANCE}.
+ * <li>An {@link ObjectWrapper} can also distinguish the two cases.
+ * Let's say you have Java Bean class {@code MyBean}, and an instance of that in the data-model called {@code
+ * myBean}. You have {@code myBean.foo} in the template, which on the FreeMarker API level is translated to a
+ * {@link TemplateHashModel#get(String)} call with key {@code "foo"}. If the {@code MyBean} class doesn't have
+ * a {@code public T getFoo()} method (or a matching readable bean property declared otherwise), then
+ * {@link TemplateHashModel#get(String)} should return {@code null}, otherwise if {@code T getFoo()} returns
+ * {@code null}, then {@link TemplateHashModel#get(String)} should return {@link TemplateNullModel#INSTANCE}.
+ * Note that an {@link ObjectWrapper} has the freedom to follow a different logic; this is just a likely logic.
+ * </ul>
+ */
+public final class TemplateNullModel implements TemplateModel {
+ public static final TemplateNullModel INSTANCE = new TemplateNullModel();
+ private TemplateNullModel() { }
+}
+
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java
index 0540af8..26d5cc2 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java
@@ -64,6 +64,7 @@ import org.apache.freemarker.core.model.TemplateIterableModel;
import org.apache.freemarker.core.model.TemplateModel;
import org.apache.freemarker.core.model.TemplateModelAdapter;
import org.apache.freemarker.core.model.TemplateModelIterator;
+import org.apache.freemarker.core.model.TemplateNullModel;
import org.apache.freemarker.core.model.TemplateNumberModel;
import org.apache.freemarker.core.model.TemplateSequenceModel;
import org.apache.freemarker.core.model.TemplateStringModel;
@@ -575,7 +576,8 @@ public class DefaultObjectWrapper implements RichObjectWrapper {
private Object tryUnwrapTo(final TemplateModel model, Class<?> targetClass, final int typeFlags,
final Map<Object, Object> recursionStops)
throws TemplateException {
- if (model == null) {
+ // TODO [FM3][null] Later model == null will mean undefined, which shouldn't get to here
+ if (model == null || model == TemplateNullModel.INSTANCE) {
return null;
}
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/util/TemplateLanguageUtils.java b/freemarker-core/src/main/java/org/apache/freemarker/core/util/TemplateLanguageUtils.java
index 208b39e..4c70568 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/util/TemplateLanguageUtils.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/util/TemplateLanguageUtils.java
@@ -38,6 +38,7 @@ import org.apache.freemarker.core.model.TemplateModel;
import org.apache.freemarker.core.model.TemplateModelIterator;
import org.apache.freemarker.core.model.TemplateNodeModel;
import org.apache.freemarker.core.model.TemplateNodeModelEx;
+import org.apache.freemarker.core.model.TemplateNullModel;
import org.apache.freemarker.core.model.TemplateNumberModel;
import org.apache.freemarker.core.model.TemplateSequenceModel;
import org.apache.freemarker.core.model.TemplateStringModel;
@@ -898,6 +899,10 @@ public final class TemplateLanguageUtils {
appendTypeName(sb, typeNamesAppended, "markupOutput");
}
+ if (TemplateNullModel.class.isAssignableFrom(cl)) {
+ appendTypeName(sb, typeNamesAppended, "null");
+ }
+
if (sb.length() == initialLength) {
appendTypeName(sb, typeNamesAppended, "miscTemplateModel");
}
diff --git a/freemarker-core/src/main/javacc/FTL.jj b/freemarker-core/src/main/javacc/FTL.jj
index 837d769..8abd3e8 100644
--- a/freemarker-core/src/main/javacc/FTL.jj
+++ b/freemarker-core/src/main/javacc/FTL.jj
@@ -246,6 +246,15 @@ public class FMParser {
}
/**
+ * Throw an exception if the expression passed in is a null literal
+ */
+ private void notNullLiteral(ASTExpression exp, String expected) throws ParseException {
+ if (exp instanceof ASTExpNullLiteral) {
+ throw new ParseException("Found: " + exp.getCanonicalForm() + ". Expecting " + expected, exp);
+ }
+ }
+
+ /**
* Throw an exception if the expression passed in is a Hash Literal
*/
private void notHashLiteral(ASTExpression exp, String expected) throws ParseException {
@@ -277,6 +286,7 @@ public class FMParser {
notListLiteral(exp, "number");
notHashLiteral(exp, "number");
notBooleanLiteral(exp, "number");
+ notNullLiteral(exp, "number");
}
/**
@@ -287,6 +297,7 @@ public class FMParser {
notListLiteral(exp, "string");
notHashLiteral(exp, "string");
notBooleanLiteral(exp, "string");
+ notNullLiteral(exp, "number");
}
/**
@@ -297,6 +308,7 @@ public class FMParser {
notListLiteral(exp, "boolean (true/false)");
notHashLiteral(exp, "boolean (true/false)");
notNumberLiteral(exp, "boolean (true/false)");
+ notNullLiteral(exp, "boolean (true/false)");
}
private ASTExpression escapedExpression(ASTExpression exp) {
@@ -959,6 +971,8 @@ TOKEN:
|
<RAW_STRING : "r" (("\"" (~["\""])* "\"") | ("'" (~["'"])* "'"))>
|
+ <NULL : "null">
+ |
<FALSE : "false">
|
<TRUE : "true">
@@ -1416,7 +1430,9 @@ ASTExpression AtomicExpression() :
exp = HashLiteral()
|
exp = StringLiteral(true)
- |
+ |
+ exp = NullLiteral()
+ |
exp = BooleanLiteral()
|
exp = ListLiteral()
@@ -1600,10 +1616,10 @@ ASTExpression EqualityExpression() :
)
rhs = RelationalExpression()
{
- notHashLiteral(lhs, "string");
- notHashLiteral(rhs, "string");
- notListLiteral(lhs, "string");
- notListLiteral(rhs, "string");
+ notHashLiteral(lhs, "different type for equality check");
+ notHashLiteral(rhs, "different type for equality check");
+ notListLiteral(lhs, "different type for equality check");
+ notListLiteral(rhs, "different type for equality check");
result = new ASTExpComparison(lhs, rhs, t.image);
result.setLocation(template, lhs, rhs);
}
@@ -1637,12 +1653,8 @@ ASTExpression RelationalExpression() :
)
rhs = RangeExpression()
{
- notHashLiteral(lhs, "number");
- notHashLiteral(rhs, "number");
- notListLiteral(lhs, "number");
- notListLiteral(rhs, "number");
- notStringLiteral(lhs, "number");
- notStringLiteral(rhs, "number");
+ numberLiteralOnly(lhs);
+ numberLiteralOnly(rhs);
result = new ASTExpComparison(lhs, rhs, t.image);
result.setLocation(template, lhs, rhs);
}
@@ -1996,6 +2008,8 @@ ASTExpression DotVariable(ASTExpression exp) :
|
t = <ESCAPED_GTE>
|
+ t = <NULL>
+ |
t = <FALSE>
|
t = <TRUE>
@@ -2016,6 +2030,7 @@ ASTExpression DotVariable(ASTExpression exp) :
notListLiteral(exp, "hash");
notStringLiteral(exp, "hash");
notBooleanLiteral(exp, "hash");
+ notNullLiteral(exp, "hash");
ASTExpDot dot = new ASTExpDot(exp, t.image);
dot.setLocation(template, exp, t);
return dot;
@@ -2038,6 +2053,7 @@ ASTExpression DynamicKey(ASTExpression exp) :
{
notBooleanLiteral(exp, "list or hash");
notNumberLiteral(exp, "list or hash");
+ notNullLiteral(exp, "list or hash");
ASTExpDynamicKeyName dkn = new ASTExpDynamicKeyName(exp, arg);
dkn.setLocation(template, exp, t);
return dkn;
@@ -2191,6 +2207,19 @@ ASTExpStringLiteral StringLiteral(boolean interpolate) :
}
}
+ASTExpression NullLiteral() :
+{
+ Token t;
+}
+{
+ t = <NULL>
+ {
+ ASTExpression result = new ASTExpNullLiteral();
+ result.setLocation(template, t, t);
+ return result;
+ }
+}
+
ASTExpression BooleanLiteral() :
{
Token t;
[freemarker] 01/02: Bit of cleanup to decrease the number of ways
InvalidReferenceException can be constructed
Posted by dd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
ddekany pushed a commit to branch 3
in repository https://gitbox.apache.org/repos/asf/freemarker.git
commit 310cb703e4b9406fa82e7eeeb6a0308bb592066a
Author: ddekany <dd...@apache.org>
AuthorDate: Mon Aug 19 16:17:05 2019 +0200
Bit of cleanup to decrease the number of ways InvalidReferenceException can be constructed
---
.../apache/freemarker/core/BuiltInForHashEx.java | 22 +++++++++-------------
.../apache/freemarker/core/BuiltInsForHashes.java | 8 ++++++--
.../freemarker/core/InvalidReferenceException.java | 6 +++---
.../org/apache/freemarker/core/_EvalUtils.java | 8 +-------
4 files changed, 19 insertions(+), 25 deletions(-)
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/BuiltInForHashEx.java b/freemarker-core/src/main/java/org/apache/freemarker/core/BuiltInForHashEx.java
index b5efa95..9512761 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/BuiltInForHashEx.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/BuiltInForHashEx.java
@@ -36,19 +36,15 @@ abstract class BuiltInForHashEx extends ASTExpBuiltIn {
abstract TemplateModel calculateResult(TemplateHashModelEx hashExModel, Environment env)
throws TemplateException, InvalidReferenceException;
- protected InvalidReferenceException newNullPropertyException(
- String propertyName, TemplateModel tm, Environment env) {
- if (env.getFastInvalidReferenceExceptions()) {
- return InvalidReferenceException.FAST_INSTANCE;
- } else {
- return new InvalidReferenceException(
- new _ErrorDescriptionBuilder(
- "The exteneded hash (of class ", tm.getClass().getName(), ") has returned null for its \"",
- propertyName,
- "\" property. This is maybe a bug. The extended hash was returned by this expression:")
- .blame(target),
- env, this);
- }
+ protected TemplateException newWrongTemplateModelMethodReturnValueException(
+ String methodName, TemplateModel tm, Environment env) {
+ return new TemplateException(
+ new _ErrorDescriptionBuilder(
+ "The exteneded hash (of class ", tm.getClass().getName(), ") " +
+ methodName + " method has returned null, which is illegal. The extended hash was " +
+ "returned by this expression:")
+ .blame(target),
+ env, this);
}
}
\ No newline at end of file
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/BuiltInsForHashes.java b/freemarker-core/src/main/java/org/apache/freemarker/core/BuiltInsForHashes.java
index eaf12f5..a1c9e1f 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/BuiltInsForHashes.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/BuiltInsForHashes.java
@@ -34,7 +34,9 @@ class BuiltInsForHashes {
TemplateModel calculateResult(TemplateHashModelEx hashExModel, Environment env)
throws TemplateException, InvalidReferenceException {
TemplateCollectionModel keys = hashExModel.keys();
- if (keys == null) throw newNullPropertyException("keys", hashExModel, env);
+ if (keys == null) {
+ throw newWrongTemplateModelMethodReturnValueException("keys", hashExModel, env);
+ }
return keys;
}
@@ -45,7 +47,9 @@ class BuiltInsForHashes {
TemplateModel calculateResult(TemplateHashModelEx hashExModel, Environment env)
throws TemplateException, InvalidReferenceException {
TemplateCollectionModel values = hashExModel.values();
- if (values == null) throw newNullPropertyException("values", hashExModel, env);
+ if (values == null) {
+ throw newWrongTemplateModelMethodReturnValueException("values", hashExModel, env);
+ }
return values;
}
}
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/InvalidReferenceException.java b/freemarker-core/src/main/java/org/apache/freemarker/core/InvalidReferenceException.java
index 7edd087..5d9b5c1 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/InvalidReferenceException.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/InvalidReferenceException.java
@@ -71,7 +71,7 @@ public class InvalidReferenceException extends TemplateException {
* Creates and invalid reference exception that contains no information about what was missing or null.
* As such, try to avoid this constructor.
*/
- public InvalidReferenceException(Environment env) {
+ private InvalidReferenceException(Environment env) {
super("Invalid reference: The expression has evaluated to null or refers to something that doesn't exist.",
env);
}
@@ -81,7 +81,7 @@ public class InvalidReferenceException extends TemplateException {
* blamed expression. As such, try to avoid this constructor, unless need to raise this expression from outside
* the FreeMarker core.
*/
- public InvalidReferenceException(String description, Environment env) {
+ private InvalidReferenceException(String description, Environment env) {
super(description, env);
}
@@ -92,7 +92,7 @@ public class InvalidReferenceException extends TemplateException {
* the failing one, like in {@code goodStep.failingStep.furtherStep} it should only contain
* {@code goodStep.failingStep}.
*/
- InvalidReferenceException(_ErrorDescriptionBuilder description, Environment env, ASTExpression expression) {
+ private InvalidReferenceException(_ErrorDescriptionBuilder description, Environment env, ASTExpression expression) {
super(null, env, expression, description);
}
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/_EvalUtils.java b/freemarker-core/src/main/java/org/apache/freemarker/core/_EvalUtils.java
index c5602ce..69e30ea 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/_EvalUtils.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/_EvalUtils.java
@@ -455,13 +455,7 @@ public class _EvalUtils {
if (tm instanceof TemplateStringModel) {
return modelToString((TemplateStringModel) tm, exp);
} else if (tm == null) {
- if (exp != null) {
- throw InvalidReferenceException.getInstance(exp, env);
- } else {
- throw new InvalidReferenceException(
- "Null/missing value (no more information available)",
- env);
- }
+ throw InvalidReferenceException.getInstance(exp, env);
} else if (tm instanceof TemplateBooleanModel) {
// TODO [FM3] It would be more logical if it's before TemplateStringModel (number etc. are before it as
// well). But currently, in FM3, `exp!` returns a multi-typed value that's also a boolean `false`, and so