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:07 UTC

[freemarker] 01/02: Bit of cleanup to decrease the number of ways InvalidReferenceException can be constructed

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