Repository: PebbleTemplates/pebble (java)

Issue:
Allow override core operators
As of right now operator overrides are disabled. This means that one cannot change the behaviour of operators like `==` or any others.

While the use cases might be very marginal I do have a use case for this: I need to make `==` operator less strict on the types of the arguments. e.g. 

```java
//  default behavior
1 == "1" // false

// desired behavior
1 == "1" // true
```

Problem is that by design everything can be extended or overridden but the unary & binary operators.

My idea is to add a flag to enable the operator overrides. So users can override core operators if they want to. i.e.
```java
@Override
  public List<BinaryOperator> getBinaryOperators() {
    List<BinaryOperator> operators = new ArrayList<>();

    operators.add(new BinaryOperatorImpl("==", 30, CustomEqualsExpression.class, Associativity.LEFT));

    return operators;
  }
```
where `CustomEqualsExpression.class` would be the override for the `==` core operator. 

Pull request:
Allow override core operators
Resolves #455.

Added `allowOverrideCoreOperators` flag to `PebbleEngine.Builder`. It defaults to `false` to preserve backwards compatibility.

Example usage:

```java
    PebbleEngine pebble = new PebbleEngine.Builder()
        .loader(new StringLoader())
        .extension(new TestExtension())
        .allowOverrideCoreOperators(true)
        .build();
```

If enabled, it will apply **all** operators defined in provided extension class.
Otherwise it will only apply those operators that are **not already defined** in `CoreExtension.class` operators.



Patch:
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/PebbleEngine.java b/pebble/src/main/java/com/mitchellbosecke/pebble/PebbleEngine.java
index 0232dba3..4f62f942 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/PebbleEngine.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/PebbleEngine.java
@@ -79,9 +79,8 @@ public class PebbleEngine {
    * Constructor for the Pebble Engine given an instantiated Loader. This method does only load
    * those userProvidedExtensions listed here.
    *
-   * @param loader     The template loader for this engine
-   * @param syntax     the syntax to use for parsing the templates.
-   * @param extensions The userProvidedExtensions which should be loaded.
+   * @param loader The template loader for this engine
+   * @param syntax the syntax to use for parsing the templates.
    */
   private PebbleEngine(Loader<?> loader,
       Syntax syntax,
@@ -90,7 +89,7 @@ public class PebbleEngine {
       PebbleCache<CacheKey, Object> tagCache,
       PebbleCache<Object, PebbleTemplate> templateCache,
       ExecutorService executorService,
-      Collection<? extends Extension> extensions,
+      ExtensionRegistry extensionRegistry,
       ParserOptions parserOptions,
       EvaluationOptions evaluationOptions) {
 
@@ -101,7 +100,7 @@ public class PebbleEngine {
     this.tagCache = tagCache;
     this.executorService = executorService;
     this.templateCache = templateCache;
-    this.extensionRegistry = new ExtensionRegistry(extensions);
+    this.extensionRegistry = extensionRegistry;
     this.parserOptions = parserOptions;
     this.evaluationOptions = evaluationOptions;
   }
@@ -283,6 +282,8 @@ public class PebbleEngine {
 
     private boolean greedyMatchMethod = false;
 
+    private boolean allowOverrideCoreOperators = false;
+
     /**
      * Creates the builder.
      */
@@ -422,6 +423,17 @@ public class PebbleEngine {
       return this;
     }
 
+    /**
+     * Sets whether or not core operators overrides should be allowed.
+     *
+     * @param allowOverrideCoreOperators Whether or not core operators overrides should be allowed.
+     * @return This builder object
+     */
+    public Builder allowOverrideCoreOperators(boolean allowOverrideCoreOperators) {
+      this.allowOverrideCoreOperators = allowOverrideCoreOperators;
+      return this;
+    }
+
     /**
      * Sets the default escaping strategy of the built-in escaper extension.
      *
@@ -436,7 +448,7 @@ public class PebbleEngine {
     /**
      * Adds an escaping strategy to the built-in escaper extension.
      *
-     * @param name     The name of the escaping strategy
+     * @param name The name of the escaping strategy
      * @param strategy The strategy implementation
      * @return This builder object
      */
@@ -472,7 +484,7 @@ public class PebbleEngine {
      * Enable/disable treat literal decimal as Integer. Default is disabled, treated as Long.
      *
      * @param literalDecimalTreatedAsInteger toggle to enable/disable literal decimal treated as
-     *                                       integer
+     * integer
      * @return This builder object
      */
     public Builder literalDecimalTreatedAsInteger(boolean literalDecimalTreatedAsInteger) {
@@ -513,13 +525,7 @@ public class PebbleEngine {
      */
     public PebbleEngine build() {
 
-      // core extensions
-      List<Extension> extensions = new ArrayList<>();
-      extensions.add(new CoreExtension());
-      extensions.add(this.escaperExtension);
-      extensions.add(new I18nExtension());
-      extensions.addAll(this.userProvidedExtensions);
-      extensions.add(new AttributeResolverExtension());
+      ExtensionRegistry extensionRegistry = buildExtensionRegistry();
 
       // default loader
       if (this.loader == null) {
@@ -562,7 +568,27 @@ public class PebbleEngine {
 
       return new PebbleEngine(this.loader, this.syntax, this.strictVariables, this.defaultLocale,
           this.tagCache, this.templateCache,
-          this.executorService, extensions, parserOptions, evaluationOptions);
+          this.executorService, extensionRegistry, parserOptions, evaluationOptions);
+    }
+
+    private ExtensionRegistry buildExtensionRegistry() {
+      ExtensionRegistry extensionRegistry = new ExtensionRegistry();
+
+      extensionRegistry.addExtension(new CoreExtension());
+      extensionRegistry.addExtension(this.escaperExtension);
+      extensionRegistry.addExtension(new I18nExtension());
+
+      for (Extension userProvidedExtension : this.userProvidedExtensions) {
+        if (allowOverrideCoreOperators) {
+          extensionRegistry.addOperatorOverridingExtension(userProvidedExtension);
+        } else {
+          extensionRegistry.addExtension(userProvidedExtension);
+        }
+      }
+
+      extensionRegistry.addExtension(new AttributeResolverExtension());
+
+      return extensionRegistry;
     }
   }
 
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/extension/ExtensionRegistry.java b/pebble/src/main/java/com/mitchellbosecke/pebble/extension/ExtensionRegistry.java
index 71e47234..15469002 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/extension/ExtensionRegistry.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/extension/ExtensionRegistry.java
@@ -59,74 +59,91 @@ public class ExtensionRegistry {
 
   private final List<AttributeResolver> attributeResolver = new ArrayList<>();
 
+  public ExtensionRegistry() {
+  }
+
   public ExtensionRegistry(Collection<? extends Extension> extensions) {
 
     for (Extension extension : extensions) {
-      // token parsers
-      List<TokenParser> tokenParsers = extension.getTokenParsers();
-      if (tokenParsers != null) {
-        for (TokenParser tokenParser : tokenParsers) {
-          this.tokenParsers.put(tokenParser.getTag(), tokenParser);
-        }
+      addExtension(extension);
+    }
+  }
+
+  public void addOperatorOverridingExtension(Extension extension) {
+    addExtension(extension, true);
+  }
+
+  public void addExtension(Extension extension) {
+    addExtension(extension, false);
+  }
+
+  private void addExtension(Extension extension, boolean operatorOverriding) {
+    // token parsers
+    List<TokenParser> tokenParsers = extension.getTokenParsers();
+    if (tokenParsers != null) {
+      for (TokenParser tokenParser : tokenParsers) {
+        this.tokenParsers.put(tokenParser.getTag(), tokenParser);
       }
+    }
 
-      // binary operators
-      List<BinaryOperator> binaryOperators = extension.getBinaryOperators();
-      if (binaryOperators != null) {
-        for (BinaryOperator operator : binaryOperators) {
-          if (!this.binaryOperators
-              .containsKey(operator.getSymbol())) { // disallow overriding core operators
-            this.binaryOperators.put(operator.getSymbol(), operator);
-          }
+    // binary operators
+    List<BinaryOperator> binaryOperators = extension.getBinaryOperators();
+    if (binaryOperators != null) {
+      for (BinaryOperator operator : binaryOperators) {
+        if (operatorOverriding) {
+          this.binaryOperators.put(operator.getSymbol(), operator);
+        } else {
+          this.binaryOperators.putIfAbsent(operator.getSymbol(), operator);
         }
       }
+    }
 
-      // unary operators
-      List<UnaryOperator> unaryOperators = extension.getUnaryOperators();
-      if (unaryOperators != null) {
-        for (UnaryOperator operator : unaryOperators) {
-          if (!this.unaryOperators
-              .containsKey(operator.getSymbol())) { // disallow override core operators
-            this.unaryOperators.put(operator.getSymbol(), operator);
-          }
+    // unary operators
+    List<UnaryOperator> unaryOperators = extension.getUnaryOperators();
+    if (unaryOperators != null) {
+      for (UnaryOperator operator : unaryOperators) {
+        if (operatorOverriding) {
+          this.unaryOperators.put(operator.getSymbol(), operator);
+        } else {
+          this.unaryOperators.putIfAbsent(operator.getSymbol(), operator);
         }
       }
+    }
 
-      // filters
-      Map<String, Filter> filters = extension.getFilters();
-      if (filters != null) {
-        this.filters.putAll(filters);
-      }
+    // filters
+    Map<String, Filter> filters = extension.getFilters();
+    if (filters != null) {
+      this.filters.putAll(filters);
+    }
 
-      // tests
-      Map<String, Test> tests = extension.getTests();
-      if (tests != null) {
-        this.tests.putAll(tests);
-      }
+    // tests
+    Map<String, Test> tests = extension.getTests();
+    if (tests != null) {
+      this.tests.putAll(tests);
+    }
 
-      // tests
-      Map<String, Function> functions = extension.getFunctions();
-      if (functions != null) {
-        this.functions.putAll(functions);
-      }
+    // functions
+    Map<String, Function> functions = extension.getFunctions();
+    if (functions != null) {
+      this.functions.putAll(functions);
+    }
 
-      // global variables
-      Map<String, Object> globalVariables = extension.getGlobalVariables();
-      if (globalVariables != null) {
-        this.globalVariables.putAll(globalVariables);
-      }
+    // global variables
+    Map<String, Object> globalVariables = extension.getGlobalVariables();
+    if (globalVariables != null) {
+      this.globalVariables.putAll(globalVariables);
+    }
 
-      // node visitors
-      List<NodeVisitorFactory> nodeVisitors = extension.getNodeVisitors();
-      if (nodeVisitors != null) {
-        this.nodeVisitors.addAll(nodeVisitors);
-      }
+    // node visitors
+    List<NodeVisitorFactory> nodeVisitors = extension.getNodeVisitors();
+    if (nodeVisitors != null) {
+      this.nodeVisitors.addAll(nodeVisitors);
+    }
 
-      // attribute resolver
-      List<AttributeResolver> attributeResolvers = extension.getAttributeResolver();
-      if (attributeResolvers != null) {
-        this.attributeResolver.addAll(attributeResolvers);
-      }
+    // attribute resolver
+    List<AttributeResolver> attributeResolvers = extension.getAttributeResolver();
+    if (attributeResolvers != null) {
+      this.attributeResolver.addAll(attributeResolvers);
     }
   }
 


Test patch:
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/OverrideCoreExtensionTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/OverrideCoreExtensionTest.java
index 941d95ef..a1333d0d 100644
--- a/pebble/src/test/java/com/mitchellbosecke/pebble/OverrideCoreExtensionTest.java
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/OverrideCoreExtensionTest.java
@@ -1,32 +1,40 @@
 package com.mitchellbosecke.pebble;
 
+import static java.util.Collections.singletonList;
+import static org.junit.Assert.assertEquals;
+
 import com.mitchellbosecke.pebble.error.PebbleException;
 import com.mitchellbosecke.pebble.extension.AbstractExtension;
 import com.mitchellbosecke.pebble.extension.Filter;
 import com.mitchellbosecke.pebble.extension.Function;
 import com.mitchellbosecke.pebble.loader.StringLoader;
+import com.mitchellbosecke.pebble.node.expression.BinaryExpression;
+import com.mitchellbosecke.pebble.node.expression.UnaryExpression;
+import com.mitchellbosecke.pebble.operator.Associativity;
+import com.mitchellbosecke.pebble.operator.BinaryOperator;
+import com.mitchellbosecke.pebble.operator.BinaryOperatorImpl;
+import com.mitchellbosecke.pebble.operator.UnaryOperator;
+import com.mitchellbosecke.pebble.operator.UnaryOperatorImpl;
 import com.mitchellbosecke.pebble.template.EvaluationContext;
+import com.mitchellbosecke.pebble.template.EvaluationContextImpl;
 import com.mitchellbosecke.pebble.template.PebbleTemplate;
-
-import org.junit.Test;
-
+import com.mitchellbosecke.pebble.template.PebbleTemplateImpl;
 import java.io.IOException;
 import java.io.StringWriter;
 import java.io.Writer;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-
-import static org.junit.Assert.assertEquals;
+import org.junit.Test;
 
 public class OverrideCoreExtensionTest {
+
   @Test
   public void testOverrideCodeExtensionFunction() throws IOException {
     PebbleEngine pebble = new PebbleEngine.Builder()
-            .loader(new StringLoader())
-            .extension(new TestExtension())
-            .strictVariables(false)
-            .build();
+        .loader(new StringLoader())
+        .extension(new TestExtension())
+        .build();
 
     PebbleTemplate template = pebble.getTemplate("{{i18n()}}");
 
@@ -38,10 +46,9 @@ public class OverrideCoreExtensionTest {
   @Test
   public void testOverrideCodeExtensionFilter() throws IOException {
     PebbleEngine pebble = new PebbleEngine.Builder()
-            .loader(new StringLoader())
-            .extension(new TestExtension())
-            .strictVariables(false)
-            .build();
+        .loader(new StringLoader())
+        .extension(new TestExtension())
+        .build();
 
     PebbleTemplate template = pebble.getTemplate("{{ null | date }}");
 
@@ -50,6 +57,64 @@ public class OverrideCoreExtensionTest {
     assertEquals("custom date filter", writer.toString());
   }
 
+  @Test
+  public void testOverrideCoreExtensionUnaryOperator() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder()
+        .loader(new StringLoader())
+        .extension(new TestExtension())
+        .allowOverrideCoreOperators(true)
+        .build();
+
+    PebbleTemplate template = pebble.getTemplate("{{ not true }}");
+
+    Writer writer = new StringWriter();
+    template.evaluate(writer);
+    assertEquals("custom unary operator", writer.toString());
+  }
+
+  @Test
+  public void testByDefaultPreventsOverrideCoreExtensionUnaryOperator() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder()
+        .loader(new StringLoader())
+        .extension(new TestExtension())
+        .build();
+
+    PebbleTemplate template = pebble.getTemplate("{{ not true }}");
+
+    Writer writer = new StringWriter();
+    template.evaluate(writer);
+    assertEquals("false", writer.toString());
+  }
+
+  @Test
+  public void testOverrideCoreExtensionBinaryOperator() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder()
+        .loader(new StringLoader())
+        .extension(new TestExtension())
+        .allowOverrideCoreOperators(true)
+        .build();
+
+    PebbleTemplate template = pebble.getTemplate("{{ 2 == 2 }}");
+
+    Writer writer = new StringWriter();
+    template.evaluate(writer);
+    assertEquals("custom binary operator", writer.toString());
+  }
+
+  @Test
+  public void testByDefaultPreventsOverrideCoreExtensionBinaryOperator() throws IOException {
+    PebbleEngine pebble = new PebbleEngine.Builder()
+        .loader(new StringLoader())
+        .extension(new TestExtension())
+        .build();
+
+    PebbleTemplate template = pebble.getTemplate("{{ 2 == 2 }}");
+
+    Writer writer = new StringWriter();
+    template.evaluate(writer);
+    assertEquals("true", writer.toString());
+  }
+
   private static class TestExtension extends AbstractExtension {
 
     @Override
@@ -65,11 +130,29 @@ public class OverrideCoreExtensionTest {
       filters.put("date", new CustomDateFilter());
       return filters;
     }
+
+    @Override
+    public List<BinaryOperator> getBinaryOperators() {
+      BinaryOperatorImpl equalsOperator = new BinaryOperatorImpl(
+          "==", 30, FakeEqualsExpression.class, Associativity.LEFT);
+
+      return singletonList(equalsOperator);
+    }
+
+    @Override
+    public List<UnaryOperator> getUnaryOperators() {
+      UnaryOperatorImpl equalsOperator = new UnaryOperatorImpl(
+          "not", 500, FakeUnaryNotExpression.class);
+
+      return singletonList(equalsOperator);
+    }
   }
 
   private static class CustomI18nFunction implements Function {
+
     @Override
-    public Object execute(Map<String, Object> args, PebbleTemplate self, EvaluationContext context, int lineNumber) {
+    public Object execute(Map<String, Object> args, PebbleTemplate self, EvaluationContext context,
+        int lineNumber) {
       return "custom i18n function";
     }
 
@@ -79,9 +162,27 @@ public class OverrideCoreExtensionTest {
     }
   }
 
+  public static class FakeEqualsExpression extends BinaryExpression<String> {
+
+    @Override
+    public String evaluate(PebbleTemplateImpl self, EvaluationContextImpl context) {
+      return "custom binary operator";
+    }
+  }
+
+  public static class FakeUnaryNotExpression extends UnaryExpression {
+
+    @Override
+    public String evaluate(PebbleTemplateImpl self, EvaluationContextImpl context) {
+      return "custom unary operator";
+    }
+  }
+
   private static class CustomDateFilter implements Filter {
+
     @Override
-    public Object apply(Object input, Map<String, Object> args, PebbleTemplate self, EvaluationContext context, int lineNumber) throws PebbleException {
+    public Object apply(Object input, Map<String, Object> args, PebbleTemplate self,
+        EvaluationContext context, int lineNumber) throws PebbleException {
       return "custom date filter";
     }