Skip to content

Fix filter error translation for context evaluate of variable expression #143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dylanahsmith merged 2 commits into from
Feb 9, 2021
Merged

Fix filter error translation for context evaluate of variable expression #143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d592c1
Refactor to provide internal_variable_expression_evaluate for context.c
dylanahsmith Feb 8, 2021
a076511
Fix filter error translation for context evaluate of variable expression
dylanahsmith Feb 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions ext/liquid_c/context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,14 @@ static VALUE context_evaluate(VALUE self, VALUE expression)
switch (RB_BUILTIN_TYPE(expression)) {
case T_DATA:
{
if (RTYPEDDATA_P(expression) && RTYPEDDATA_TYPE(expression) == &expression_data_type)
return internal_expression_evaluate(DATA_PTR(expression), self);
if (RTYPEDDATA_P(expression) && RTYPEDDATA_TYPE(expression) == &expression_data_type) {
if (RBASIC_CLASS(expression) == cLiquidCExpression) {
return internal_expression_evaluate(DATA_PTR(expression), self);
} else {
assert(RBASIC_CLASS(expression) == cLiquidCVariableExpression);
return internal_variable_expression_evaluate(DATA_PTR(expression), self);
}
}
break; // e.g. BigDecimal
}
case T_OBJECT: // may be Liquid::VariableLookup or Liquid::RangeLookup
Expand Down
2 changes: 0 additions & 2 deletions ext/liquid_c/expression.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ static VALUE expression_strict_parse(VALUE klass, VALUE markup)
return expr_obj;
}

#define Expression_Get_Struct(obj, sval) TypedData_Get_Struct(obj, expression_t, &expression_data_type, sval)

VALUE expression_evaluate(VALUE self, VALUE context)
{
expression_t *expression;
Expand Down
3 changes: 3 additions & 0 deletions ext/liquid_c/expression.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ typedef struct expression {
vm_assembler_t code;
} expression_t;

extern const rb_data_type_t expression_data_type;
#define Expression_Get_Struct(obj, sval) TypedData_Get_Struct(obj, expression_t, &expression_data_type, sval)

void init_liquid_expression();

VALUE expression_new(VALUE klass, expression_t **expression_ptr);
Expand Down
17 changes: 12 additions & 5 deletions ext/liquid_c/variable.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,14 @@ static VALUE variable_strict_parse_method(VALUE self, VALUE markup)
}

typedef struct {
VALUE self;
expression_t *expression;
VALUE context;
} variable_expression_evaluate_args_t;

static VALUE try_variable_expression_evaluate(VALUE uncast_args)
{
variable_expression_evaluate_args_t *args = (void *)uncast_args;
return expression_evaluate(args->self, args->context);
return liquid_vm_evaluate(args->context, &args->expression->code);
}

static VALUE rescue_variable_expression_evaluate(VALUE uncast_args, VALUE exception)
Expand All @@ -189,12 +189,19 @@ static VALUE rescue_variable_expression_evaluate(VALUE uncast_args, VALUE except
rb_exc_raise(exception);
}

static VALUE variable_expression_evaluate(VALUE self, VALUE context)
VALUE internal_variable_expression_evaluate(expression_t *expression, VALUE context)
{
variable_expression_evaluate_args_t args = { self, context };
variable_expression_evaluate_args_t args = { expression, context };
return rb_rescue(try_variable_expression_evaluate, (VALUE)&args, rescue_variable_expression_evaluate, (VALUE)&args);
}

static VALUE variable_expression_evaluate_method(VALUE self, VALUE context)
{
expression_t *expression;
Expression_Get_Struct(self, expression);
return internal_variable_expression_evaluate(expression, context);
}

void init_liquid_variable(void)
{
id_rescue_strict_parse_syntax_error = rb_intern("rescue_strict_parse_syntax_error");
Expand All @@ -211,6 +218,6 @@ void init_liquid_variable(void)

cLiquidCVariableExpression = rb_define_class_under(mLiquidC, "VariableExpression", cLiquidCExpression);
rb_global_variable(&cLiquidCVariableExpression);
rb_define_method(cLiquidCVariableExpression, "evaluate", variable_expression_evaluate, 1);
rb_define_method(cLiquidCVariableExpression, "evaluate", variable_expression_evaluate_method, 1);
}

2 changes: 2 additions & 0 deletions ext/liquid_c/variable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "vm_assembler.h"
#include "block.h"
#include "expression.h"

extern VALUE cLiquidCVariableExpression;

Expand All @@ -17,6 +18,7 @@ typedef struct variable_parse_args {
void init_liquid_variable(void);
void internal_variable_compile(variable_parse_args_t *parse_args, unsigned int line_number);
void internal_variable_compile_evaluate(variable_parse_args_t *parse_args);
VALUE internal_variable_expression_evaluate(expression_t *expression, VALUE context);

#endif

7 changes: 7 additions & 0 deletions test/unit/variable_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,13 @@ def test_render_variable_object
assert_equal('Liquid error: concat filter requires an array argument', exc.message)
end

def test_filter_argument_error_translation
variable = Liquid::Variable.new("'some words' | split", Liquid::ParseContext.new)
context = Liquid::Context.new
exc = assert_raises(Liquid::ArgumentError) { variable.render(context) }
assert_equal('Liquid error: wrong number of arguments (given 1, expected 2)', exc.message)
end

private

def variable_strict_parse(markup)
Expand Down