Simplify MapToolLineParser and related code

src/main/java/net/rptools/maptool/client/MapToolVariableResolver.java

@@ -14,13 +14,19 @@
  */
 package net.rptools.maptool.client;
 
+import com.google.gson.Gson;
 import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonNull;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonPrimitive;
+import com.google.gson.TypeAdapter;
+import com.google.gson.stream.JsonReader;
+import java.io.IOException;
+import java.io.StringReader;
 import java.math.BigDecimal;
 import java.util.*;
+import javax.annotation.Nonnull;
 import javax.swing.JOptionPane;
 import net.rptools.CaseInsensitiveHashMap;
 import net.rptools.lib.StringUtil;
@@ -40,6 +46,10 @@ public class MapToolVariableResolver implements VariableResolver {
   // Logger for this class.
   private static final Logger LOGGER = LogManager.getLogger(MapToolVariableResolver.class);
 
+  /** the only way to make Gson apply strict evaluation to JsonObjects, apparently. see #2396 */
+  private static final TypeAdapter<JsonObject> strictGsonObjectAdapter =
+      new Gson().getAdapter(JsonObject.class);
+
   /** The prefix for querying and setting state values . */
   public static final String BAR_PREFIX = "bar.";
 
@@ -94,8 +104,6 @@ public class MapToolVariableResolver implements VariableResolver {
           GM_PANEL);
   private final Map<String, Object> variables = new CaseInsensitiveHashMap<>();
 
-  private List<Runnable> delayedActionList;
-
   private Token tokenInContext;
 
   private boolean autoPrompt;
@@ -126,34 +134,111 @@ public MapToolVariableResolver(Token tokenInContext) {
   }
 
   /**
-   * Initialize this resolver
+   * @return A new resolver with the same token in context.
+   */
+  public MapToolVariableResolver spawn() {
+    return new MapToolVariableResolver(tokenInContext);
+  }
+
+  /**
+   * Looks up a property on the token in context and evaluates it.
    *
-   * @return true if initialization actually happened, false if it's already been initialized
+   * @param key The property name to look up.
+   * @return The evaluate result.
    */
-  public boolean initialize() {
-    if (delayedActionList == null) {
-      delayedActionList = new ArrayList<>();
-      return true;
+  public @Nonnull Object getEvaluatedTokenProperty(@Nonnull String key) {
+    if (tokenInContext == null) {
+      return "";
     }
-    return false;
+
+    Object val = tokenInContext.getProperty(key);
+    if (val == null) {
+      val = tokenInContext.getPropertyDefault(key);
+    }
+    if (val == null) {
+      return "";
+    }
+    return evaluateProperty(key, val);
   }
 
   /**
-   * Add an action to be performed after the full expression has been evaluated.
+   * Looks up a property default on the token in context and evaluates it.
    *
-   * @param runnable the action to be performed.
+   * @param key The property name to look up.
+   * @return The evaluate result.
    */
-  public void addDelayedAction(Runnable runnable) {
-    if (!delayedActionList.contains(runnable)) {
-      delayedActionList.add(runnable);
+  public @Nonnull Object getEvaluatedTokenPropertyDefault(@Nonnull String key) {
+    if (tokenInContext == null) {
+      return "";
     }
+
+    Object val = tokenInContext.getPropertyDefault(key);
+    if (val == null) {
+      return "";
+    }
+    return evaluateProperty(key, val);
   }
 
-  /** Perform any delayed actions. This should called by the command framework only. */
-  public void flush() {
-    for (Runnable r : delayedActionList) {
-      r.run();
+  /**
+   * Evaluate the provided expression with the resolver
+   *
+   * @param key the key of the value
+   * @param val An expression to evaluate
+   * @return The evaluated value
+   */
+  private @Nonnull Object evaluateProperty(@Nonnull String key, @Nonnull Object val) {
+    if (val.toString().trim().startsWith("{")) {
+      /*
+       * The normal Gson evaluator was too lenient in identifying JSON objects, so we had to move
+       * that lower (see #1560). But we would really like to avoid the performance cost of
+       * attempting to parse anything that actually is a proper JSON, so let's try a stricter
+       * evaluation process here first (see #2396).
+       */
+      try {
+        try (JsonReader reader = new JsonReader(new StringReader(val.toString()))) {
+          JsonObject result = strictGsonObjectAdapter.read(reader);
+          // in case of a situation like {"a": 1}{"b": 2}, the above would have stopped at the first
+          // complete object.  This next line will throw an exception on finding another top-level
+          // object, allowing us to move on with other evaluation.
+          reader.hasNext();
+          if (result.isJsonObject()) {
+            return result;
+          }
+        }
+      } catch (IOException e) {
+        // deliberately ignored - continue parsing
+      }
+    }
+    // try to convert it to a JSON array. Fixes #2057.
+    if (val.toString().trim().startsWith("[")) {
+      JsonElement json = JSONMacroFunctions.getInstance().asJsonElement(val.toString());
+      if (json.isJsonArray()) {
+        return json;
+      }
     }
+    try {
+      LOGGER.debug(
+          "Evaluating property: '{}' for token {} ({})----------------------------------------------------------------------------------",
+          key,
+          tokenInContext.getName(),
+          tokenInContext.getId());
+      val = MapTool.getParser().parseLine(this, val.toString(), null);
+    } catch (ParserException pe) {
+      LOGGER.debug("Ignoring Parse Exception, continuing to evaluate {}", key);
+      val = val.toString();
+    }
+    if (val == null) {
+      return "";
+    }
+    // Finally we try convert it to a JSON object. Fixes #1560.
+    if (val.toString().trim().startsWith("{")) {
+      JsonElement json = JSONMacroFunctions.getInstance().asJsonElement(val.toString());
+      if (json.isJsonObject()) {
+        return json;
+      }
+    }
+
+    return val;
   }
 
   public void setAutoPrompt(boolean value) {
@@ -199,7 +284,7 @@ public Object getVariable(String name, VariableModifiers mods) throws ParserExce
 
     boolean evaluate = false; // Should we try to evaluate the value.
 
-    // MT Script doesnt have much in the way of types.
+    // MT Script doesn't have much in the way of types.
     if (name.startsWith(MarkDownFunctions.MARKDOWN_PREFIX)) {
       return new MarkDownFunctions().getMTSTypeLabel(name);
     }
@@ -236,7 +321,7 @@ public Object getVariable(String name, VariableModifiers mods) throws ParserExce
       } // endif
 
       if (this.validTokenProperty(name, tokenInContext)) {
-        result = tokenInContext.getEvaluatedProperty(name);
+        result = getEvaluatedTokenProperty(name);
       } else {
         // If the token has no property of that name check to see if there s a defaulted
         // value for the property for the token type.
@@ -318,7 +403,7 @@ public Object getVariable(String name, VariableModifiers mods) throws ParserExce
       if (evaluate) {
         // Try parse the value if we can not parse it then just return it as a string.
         try {
-          value = MapTool.getParser().parseLine(tokenInContext, result.toString());
+          value = MapTool.getParser().parseLine(spawn(), result.toString(), null);
         } catch (Exception e) {
           value = result.toString();
         }

src/main/java/net/rptools/maptool/client/OptionInfo.java

@@ -23,7 +23,6 @@
 import java.util.regex.Pattern;
 import net.rptools.dicelib.expression.Result;
 import net.rptools.maptool.language.I18N;
-import net.rptools.maptool.model.Token;
 import net.rptools.parser.ParserException;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -430,23 +429,21 @@ public int getIntParam(int index) {
   }
 
   /** Returns a param, parsing it as an expression if it is a string. */
-  public Object getParsedParam(
-      int index, MapToolVariableResolver res, Token tokenInContext, MapToolLineParser parser)
+  public Object getParsedParam(int index, MapToolVariableResolver res, MapToolLineParser parser)
       throws ParserException {
     Object retval = params[index];
     // No parsing is done if the param isn't a String (e.g. it's already a BigDecimal)
     if (params[index] instanceof String) {
-      Result result = parser.parseExpression(res, tokenInContext, (String) params[index], false);
+      Result result = parser.parseExpression(res, (String) params[index], false);
       retval = result.getValue();
     }
     return retval;
   }
 
   /** Returns a param as int, parsing it as an expression if it is a string. */
-  public int getParsedIntParam(
-      int index, MapToolVariableResolver res, Token tokenInContext, MapToolLineParser parser)
+  public int getParsedIntParam(int index, MapToolVariableResolver res, MapToolLineParser parser)
       throws ParserException {
-    Object retval = getParsedParam(index, res, tokenInContext, parser);
+    Object retval = getParsedParam(index, res, parser);
     if (!(retval instanceof BigDecimal)) {
       throw new ParserException(I18N.getText("lineParser.notValidNumber", retval.toString()));
     }

src/main/java/net/rptools/maptool/client/functions/CallFunction.java

@@ -24,7 +24,6 @@
 import net.rptools.maptool.client.functions.json.JSONMacroFunctions;
 import net.rptools.maptool.client.functions.json.JsonArrayFunctions;
 import net.rptools.maptool.language.I18N;
-import net.rptools.maptool.model.Token;
 import net.rptools.maptool.util.FunctionUtil;
 import net.rptools.parser.Parser;
 import net.rptools.parser.ParserException;
@@ -122,8 +121,6 @@ public Object childEvaluate(
   private Object doCall(
       MapToolVariableResolver resolver, String macroName, JsonArray args, boolean defer)
       throws ParserException {
-    Token tokenInContext = resolver.getTokenInContext();
-
     resolver.setVariable(MapToolLineParser.ARGS_PASSED_AS_VARIABLE_NAME, args);
 
     if (defer) {
@@ -133,21 +130,17 @@ private Object doCall(
               MapTool.getParser()
                   .runMacro(
                       resolver,
-                      tokenInContext,
                       macroName,
-                      MapToolLineParser.ARGS_PASSED_AS_VARIABLE_INDICATOR);
+                      MapToolLineParser.ARGS_PASSED_AS_VARIABLE_INDICATOR,
+                      true);
             } catch (ParserException e) {
               MapTool.addErrorMessage(e);
             }
           });
       return ""; // Defered macros will return nothing as the excution will be done later
     } else {
       return MapTool.getParser()
-          .runMacro(
-              resolver,
-              tokenInContext,
-              macroName,
-              MapToolLineParser.ARGS_PASSED_AS_VARIABLE_INDICATOR);
+          .runMacro(resolver, macroName, MapToolLineParser.ARGS_PASSED_AS_VARIABLE_INDICATOR, true);
     }
   }
 

src/main/java/net/rptools/maptool/client/functions/EvalMacroFunctions.java

@@ -21,7 +21,6 @@
 import net.rptools.maptool.client.MapToolMacroContext;
 import net.rptools.maptool.client.MapToolVariableResolver;
 import net.rptools.maptool.language.I18N;
-import net.rptools.maptool.model.Token;
 import net.rptools.parser.Parser;
 import net.rptools.parser.ParserException;
 import net.rptools.parser.VariableResolver;
@@ -54,46 +53,20 @@ public Object childEvaluate(
       throw new ParserException(I18N.getText("macro.function.general.noPerm", functionName));
     }
 
-    Token tokenInContext = ((MapToolVariableResolver) resolver).getTokenInContext();
-
     // execMacro has new variable scope where as evalMacro does not.
+    MapToolVariableResolver mtResolver = (MapToolVariableResolver) resolver;
     if (functionName.equalsIgnoreCase("execMacro")) {
-      return execMacro(tokenInContext, parameters.get(0).toString());
+      return evalMacro(mtResolver.spawn(), parameters.get(0).toString());
     } else if ("evalMacro".equalsIgnoreCase(functionName)) {
-      return evalMacro(
-          (MapToolVariableResolver) resolver, tokenInContext, parameters.get(0).toString());
+      return evalMacro(mtResolver, parameters.get(0).toString());
     }
     throw new ParserException(I18N.getText("macro.function.general.unknownFunction", functionName));
   }
 
-  /**
-   * Executes the macro with a new variable scope.
-   *
-   * @param tokenInContext The token in context.
-   * @param line the macro to execute.
-   * @return the result of the execution.
-   * @throws ParserException if an error occurs.
-   */
-  public static Object execMacro(Token tokenInContext, String line) throws ParserException {
-    return evalMacro(null, tokenInContext, line);
-  }
-
-  /**
-   * Executes the macro with the specified variable scope.
-   *
-   * @param res the {@link MapToolVariableResolver} used to resolve variable values.
-   * @param tokenInContext The token in context.
-   * @param line the macro to execute.
-   * @return the result of the execution.
-   * @throws ParserException if an error occurs.
-   */
-  public static Object evalMacro(MapToolVariableResolver res, Token tokenInContext, String line)
-      throws ParserException {
-    res = res == null ? new MapToolVariableResolver(tokenInContext) : res;
-
+  public static Object evalMacro(MapToolVariableResolver res, String line) throws ParserException {
     MapToolMacroContext context =
         new MapToolMacroContext("<dynamic>", MapTool.getParser().getContext().getSource(), true);
-    String ret = MapTool.getParser().parseLine(res, tokenInContext, line, context);
+    String ret = MapTool.getParser().parseLine(res, line, context);
 
     // Try to convert to a number
     try {

src/main/java/net/rptools/maptool/client/functions/ExecFunction.java

@@ -211,12 +211,10 @@ private static void runExecFunction(String functionName, List<Object> execArgs)
     Parser parser = new ExpressionParserFactory().createMT().getParser();
     Function function = parser.getFunction(functionName);
     var loc = macroLocationFactory.createExecFunctionLocation(functionName);
-    MapTool.getParser().enterTrustedContext(functionName, loc);
-    try {
+    try (var ignored = MapTool.getParser().enterTrustedContext(functionName, loc)) {
       function.evaluate(parser, new MapToolVariableResolver(null), functionName, execArgs);
     } catch (ParserException pe) {
       log.error("execFunction failed:", pe);
     }
-    MapTool.getParser().exitContext();
   }
 }

src/main/java/net/rptools/maptool/client/functions/MacroLinkFunction.java

@@ -497,19 +497,14 @@ private static void doOutput(
       throws ParserException {
 
     var loc = macroLocationFactory.createMacroLinkLocation(macroName);
-    if (MapTool.getParser().isMacroTrusted()) {
-      MapTool.getParser().enterTrustedContext(macroName, loc);
-    } else {
-      MapTool.getParser().enterUntrustedContext(macroName, loc);
-    }
-
     String line = "";
     // Execute the macro
-    try {
+    try (var ignored =
+        MapTool.getParser().isMacroTrusted()
+            ? MapTool.getParser().enterTrustedContext(macroName, loc)
+            : MapTool.getParser().enterUntrustedContext(macroName, loc)) {
       MapToolVariableResolver resolver = new MapToolVariableResolver(token);
-      line = MapTool.getParser().runMacro(resolver, token, macroName, args);
-    } finally {
-      MapTool.getParser().exitContext();
+      line = MapTool.getParser().runMacro(resolver, macroName, args, true);
     }
 
     // Don't output blank messages. Fixes #1867.

src/main/java/net/rptools/maptool/client/functions/TestFunctions.java

@@ -141,9 +141,7 @@ private void runTests(Token token) {
             new MapToolMacroContext(
                 TEST_CONTEXT_NAME, MapTool.getParser().getContext().getSource(), true);
         try {
-          String ret =
-              MapTool.getParser()
-                  .parseLine(resolver, resolver.getTokenInContext(), mbp.getCommand(), context);
+          String ret = MapTool.getParser().parseLine(resolver, mbp.getCommand(), context);
         } catch (Exception e) {
           failures++;
           messages.add(e.getMessage());