basic type checking

- logical expressions now properly report their type (Boolean) - functions return their declared types - minimal expression walker checks type equality on operations and adds errors if necessary - fixed tests
Graylog2 · Dec 31, 2015 · 2ef8d6c · 2ef8d6c
1 parent 4cb223d
commit 2ef8d6c
Show file tree

Hide file tree

Showing 22 changed files with 267 additions and 48 deletions.
diff --git a/src/main/antlr4/org/graylog/plugins/messageprocessor/parser/RuleLang.g4 b/src/main/antlr4/org/graylog/plugins/messageprocessor/parser/RuleLang.g4
@@ -29,6 +29,10 @@
 
 grammar RuleLang;
 
+@header {
+import org.graylog.plugins.messageprocessor.ast.expressions.Expression;
+}
+
 ruleDeclaration
     :   Rule name=String
         (During Stage stage=Integer)?

diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/AndExpression.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/AndExpression.java
@@ -19,6 +19,11 @@ public boolean evaluateBool(EvaluationContext context, Message message) {
         return ((LogicalExpression)left).evaluateBool(context, message) && ((LogicalExpression)right).evaluateBool(context, message);
     }
 
+    @Override
+    public Class getType() {
+        return Boolean.class;
+    }
+
     @Override
     public String toString() {
         return left.toString() + " AND " + right.toString();

diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/BinaryExpression.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/BinaryExpression.java
@@ -14,4 +14,7 @@ public boolean isConstant() {
         return left.isConstant() && right.isConstant();
     }
 
+    public Expression left() {
+        return left;
+    }
 }
diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/ComparisonExpression.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/ComparisonExpression.java
@@ -19,6 +19,11 @@ public Object evaluate(EvaluationContext context, Message message) {
         return evaluateBool(context, message);
     }
 
+    @Override
+    public Class getType() {
+        return Boolean.class;
+    }
+
     @Override
     public boolean evaluateBool(EvaluationContext context, Message message) {
         if (!numericArgs) {

diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/EqualityExpression.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/EqualityExpression.java
@@ -16,6 +16,11 @@ public Object evaluate(EvaluationContext context, Message message) {
         return evaluateBool(context, message);
     }
 
+    @Override
+    public Class getType() {
+        return Boolean.class;
+    }
+
     @Override
     public boolean evaluateBool(EvaluationContext context, Message message) {
         final boolean equals = left.evaluate(context, message).equals(right.evaluate(context, message));

diff --git a/...main/java/org/graylog/plugins/messageprocessor/ast/expressions/FieldAccessExpression.java b/...main/java/org/graylog/plugins/messageprocessor/ast/expressions/FieldAccessExpression.java
@@ -1,13 +1,12 @@
 package org.graylog.plugins.messageprocessor.ast.expressions;
 
-import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.beanutils.PropertyUtils;
 import org.graylog.plugins.messageprocessor.EvaluationContext;
 import org.graylog2.plugin.Message;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
 
 public class FieldAccessExpression implements Expression {
     private static final Logger log = LoggerFactory.getLogger(FieldAccessExpression.class);
@@ -30,10 +29,11 @@ public Object evaluate(EvaluationContext context, Message message) {
         final Object bean = this.object.evaluate(context, message);
         final String fieldName = field.evaluate(context, message).toString();
         try {
-            final Method method = bean.getClass().getMethod("get"+ StringUtils.capitalize(fieldName));
-            return method.invoke(bean);
+            final Object property = PropertyUtils.getProperty(bean, fieldName);
+            log.debug("[field access] property {} of bean {}: {}", fieldName, bean.getClass().getTypeName(), property);
+            return property;
         } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
-            log.error("Oops");
+            log.error("Unable to read property {} from {}", fieldName, bean);
             return null;
         }
     }

diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/FunctionExpression.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/FunctionExpression.java
@@ -2,18 +2,23 @@
 
 import com.google.common.base.Joiner;
 import org.graylog.plugins.messageprocessor.EvaluationContext;
-import org.graylog.plugins.messageprocessor.FieldSet;
+import org.graylog.plugins.messageprocessor.ast.functions.Function;
+import org.graylog.plugins.messageprocessor.ast.functions.FunctionDescriptor;
 import org.graylog2.plugin.Message;
 
 import java.util.Map;
 
 public class FunctionExpression implements Expression {
     private final String name;
     private final Map<String, Expression> args;
+    private final Function function;
+    private final FunctionDescriptor descriptor;
 
-    public FunctionExpression(String name, Map<String, Expression> args) {
+    public FunctionExpression(String name, Map<String, Expression> args, Function function) {
         this.name = name;
         this.args = args;
+        this.function = function;
+        this.descriptor = function.descriptor();
     }
 
     @Override
@@ -23,12 +28,12 @@ public boolean isConstant() {
 
     @Override
     public Object evaluate(EvaluationContext context, Message message) {
-        return context.invokeFunction(context, message, name, args);
+        return descriptor.returnType().cast(function.evaluate(args, context, message));
     }
 
     @Override
     public Class getType() {
-        return FieldSet.class;
+        return descriptor.returnType();
     }
 
     @Override

diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/NotExpression.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/NotExpression.java
@@ -18,6 +18,11 @@ public boolean evaluateBool(EvaluationContext context, Message message) {
         return !((LogicalExpression)right).evaluateBool(context, message);
     }
 
+    @Override
+    public Class getType() {
+        return Boolean.class;
+    }
+
     @Override
     public String toString() {
         return "NOT " + right.toString();

diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/OrExpression.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/OrExpression.java
@@ -19,6 +19,11 @@ public boolean evaluateBool(EvaluationContext context, Message message) {
         return ((LogicalExpression)left).evaluateBool(context, message) || ((LogicalExpression)right).evaluateBool(context, message);
     }
 
+    @Override
+    public Class getType() {
+        return Boolean.class;
+    }
+
     @Override
     public String toString() {
         return left.toString() + " OR " + right.toString();

diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/UnaryExpression.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/expressions/UnaryExpression.java
@@ -17,4 +17,8 @@ public boolean isConstant() {
     public Class getType() {
         return right.getType();
     }
+
+    public Expression right() {
+        return right;
+    }
 }
diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/functions/Function.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/functions/Function.java
@@ -1,15 +1,32 @@
 package org.graylog.plugins.messageprocessor.ast.functions;
 
+import com.google.common.collect.ImmutableList;
 import org.graylog.plugins.messageprocessor.EvaluationContext;
 import org.graylog.plugins.messageprocessor.ast.expressions.Expression;
 import org.graylog2.plugin.Message;
 
 import java.util.Map;
 
-public interface Function {
+public interface Function<T> {
 
-    Object evaluate(Map<String, Expression> args, EvaluationContext context, Message message);
+    Function ERROR_FUNCTION = new Function<Void>() {
+        @Override
+        public Void evaluate(Map args, EvaluationContext context, Message message) {
+            return null;
+        }
 
-    FunctionDescriptor descriptor();
+        @Override
+        public FunctionDescriptor<Void> descriptor() {
+            return FunctionDescriptor.<Void>builder()
+                    .name("__unresolved_function")
+                    .returnType(Void.class)
+                    .params(ImmutableList.of())
+                    .build();
+        }
+    };
+
+    T evaluate(Map<String, Expression> args, EvaluationContext context, Message message);
+
+    FunctionDescriptor<T> descriptor();
 
 }
diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/functions/FunctionDescriptor.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/functions/FunctionDescriptor.java
@@ -4,27 +4,28 @@
 import com.google.common.collect.ImmutableList;
 
 @AutoValue
-public abstract class FunctionDescriptor {
+public abstract class FunctionDescriptor<T> {
 
     public abstract String name();
 
     public abstract boolean pure();
 
-    public abstract Class returnType();
+    public abstract Class<T> returnType();
 
     public abstract ImmutableList<ParameterDescriptor> params();
 
-    public static Builder builder() {
+    public static <T> Builder<T> builder() {
+        //noinspection unchecked
         return new AutoValue_FunctionDescriptor.Builder().pure(false);
     }
 
     @AutoValue.Builder
-    public static abstract class Builder {
-        public abstract FunctionDescriptor build();
+    public static abstract class Builder<T> {
+        public abstract FunctionDescriptor<T> build();
 
-        public abstract Builder name(String name);
-        public abstract Builder pure(boolean pure);
-        public abstract Builder returnType(Class type);
-        public abstract Builder params(ImmutableList<ParameterDescriptor> params);
+        public abstract Builder<T> name(String name);
+        public abstract Builder<T> pure(boolean pure);
+        public abstract Builder<T> returnType(Class<T> type);
+        public abstract Builder<T> params(ImmutableList<ParameterDescriptor> params);
     }
 }
diff --git a/.../java/org/graylog/plugins/messageprocessor/ast/functions/builtin/DropMessageFunction.java b/.../java/org/graylog/plugins/messageprocessor/ast/functions/builtin/DropMessageFunction.java
@@ -9,7 +9,7 @@
 
 import java.util.Map;
 
-public class DropMessageFunction implements Function {
+public class DropMessageFunction implements Function<Void> {
 
     public static final String NAME = "drop_message";
 
@@ -20,8 +20,8 @@ public Void evaluate(Map<String, Expression> args, EvaluationContext context, Me
     }
 
     @Override
-    public FunctionDescriptor descriptor() {
-        return FunctionDescriptor.builder()
+    public FunctionDescriptor<Void> descriptor() {
+        return FunctionDescriptor.<Void>builder()
                 .name(NAME)
                 .pure(true)
                 .returnType(Void.class)

diff --git a/src/main/java/org/graylog/plugins/messageprocessor/ast/functions/builtin/InputFunction.java b/src/main/java/org/graylog/plugins/messageprocessor/ast/functions/builtin/InputFunction.java
@@ -15,7 +15,7 @@
 
 import static com.google.common.collect.ImmutableList.of;
 
-public class InputFunction implements Function {
+public class InputFunction implements Function<MessageInput> {
 
     public static final String NAME = "input";
 
@@ -27,15 +27,15 @@ public InputFunction(InputRegistry inputRegistry) {
     }
 
     @Override
-    public Object evaluate(Map<String, Expression> args, EvaluationContext context, Message message) {
+    public MessageInput evaluate(Map<String, Expression> args, EvaluationContext context, Message message) {
         final Object id = args.get("id").evaluate(context, message);
         final IOState<MessageInput> inputState = inputRegistry.getInputState(id.toString());
         return inputState.getStoppable();
     }
 
     @Override
-    public FunctionDescriptor descriptor() {
-        return FunctionDescriptor.builder()
+    public FunctionDescriptor<MessageInput> descriptor() {
+        return FunctionDescriptor.<MessageInput>builder()
                 .name(NAME)
                 .returnType(MessageInput.class)
                 .params(of(ParameterDescriptor.string("id")))

diff --git a/src/main/java/org/graylog/plugins/messageprocessor/parser/FunctionRegistry.java b/src/main/java/org/graylog/plugins/messageprocessor/parser/FunctionRegistry.java
@@ -6,6 +6,7 @@
 import java.util.Map;
 
 public class FunctionRegistry {
+
     private final Map<String, Function> functions;
 
     @Inject
@@ -17,4 +18,12 @@ public FunctionRegistry(Map<String, Function> functions) {
     public Function resolve(String name) {
         return functions.get(name);
     }
+
+    public Function resolveOrError(String name) {
+        final Function function = resolve(name);
+        if (function == null) {
+            return Function.ERROR_FUNCTION;
+        }
+        return function;
+    }
 }
diff --git a/src/main/java/org/graylog/plugins/messageprocessor/parser/IncompatibleTypes.java b/src/main/java/org/graylog/plugins/messageprocessor/parser/IncompatibleTypes.java
@@ -0,0 +1,28 @@
+package org.graylog.plugins.messageprocessor.parser;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import org.graylog.plugins.messageprocessor.ast.expressions.BinaryExpression;
+import org.graylog.plugins.messageprocessor.ast.expressions.Expression;
+
+public class IncompatibleTypes extends ParseError {
+    private final RuleLangParser.ExpressionContext ctx;
+    private final BinaryExpression binaryExpr;
+
+    public IncompatibleTypes(RuleLangParser.ExpressionContext ctx, BinaryExpression binaryExpr) {
+        super("incompatible_types", ctx);
+        this.ctx = ctx;
+        this.binaryExpr = binaryExpr;
+    }
+
+    @JsonProperty("reason")
+    @Override
+    public String toString() {
+        return "Incompatible types " + exprString(binaryExpr.left()) + " <=> " + exprString(binaryExpr.right()) + positionString();
+    }
+
+    private String exprString(Expression e) {
+        return "(" + e.toString() + ") : " + e.getType().getSimpleName();
+    }
+
+
+}
diff --git a/src/main/java/org/graylog/plugins/messageprocessor/parser/ParseError.java b/src/main/java/org/graylog/plugins/messageprocessor/parser/ParseError.java
@@ -4,6 +4,8 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import org.antlr.v4.runtime.ParserRuleContext;
 
+import java.util.Objects;
+
 public abstract class ParseError {
 
     @JsonProperty
@@ -32,4 +34,18 @@ protected String positionString() {
                 " line " + line() +
                 " pos " + positionInLine();
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof ParseError)) return false;
+        ParseError that = (ParseError) o;
+        return Objects.equals(type, that.type) &&
+                Objects.equals(ctx, that.ctx);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(type, ctx);
+    }
 }
diff --git a/src/main/java/org/graylog/plugins/messageprocessor/parser/ParseException.java b/src/main/java/org/graylog/plugins/messageprocessor/parser/ParseException.java
@@ -1,15 +1,15 @@
 package org.graylog.plugins.messageprocessor.parser;
 
-import java.util.List;
+import java.util.Set;
 
 public class ParseException extends RuntimeException {
-    private final List<ParseError> errors;
+    private final Set<ParseError> errors;
 
-    public ParseException(List<ParseError> errors) {
+    public ParseException(Set<ParseError> errors) {
         this.errors = errors;
     }
 
-    public List<ParseError> getErrors() {
+    public Set<ParseError> getErrors() {
         return errors;
     }