synvert-hq
unary splat operator is not handled properly
I have seen this with the rails/strong_parameters snippet, but I think the problem is more general.
Basically, given a source like
attr_accessible *args
doing
eval arguments.first.to_source # "*args"
is never going to do the right thing, because eval("*args")is invalid.
This can't be handled easily because the splat operator needs to bubble up the chain of calls, but replacing eval with an appropriate synvert_eval may allow the tool to work while notifying the users, i.e.
def synvert_eval token, expr, file, line
if token[0] == ?*
log "synvert could not handle #{expr} at #{file}:#{line}, look for SYNVERT_FIXME to handle this"
"SYNVERT_FIXME('#{token}',#{file},#{line})"
else
eval(token)
end
end
@riffraff what's attr_accessible *args, I never see it before. If we want to use strong_parameters instead, what it will be?
foo *whatever explodes an Enumerable as multiple arguments, i.e.
def foo(a,b, *rest) p [a,b,rest] end
ary = [1,2,3,4]
foo( ary) # => ArgumentError: wrong number of arguments (1 for 2+)
foo(*ary) # => [1, 2, [3, 4]]
Passing it to .permit would be the same i.e.
params.require(:xx).permit(*whatever)
I have in some code reused the same list both for attr_accessible and for other things, so it's saved in a constant, and passed with a splat, in which case the code would need to be
params.require(:xx).permit(*Model::MY_CONSTANT)
if the data was in a local variable then it wouldn't be possible, and we'd need to get the eval'ed data so it would become
params.require(:xx).permit(all, the , attributes)
# or equivalently, using "`*`" plus `var.to_source`
params.require(:xx).permit(*[all, the, attributes])
but I think it's pretty ok if synvert doesn't handle this case, I just think it would be better to handle it by notifying the user and marking it in the code rather than the current behaviour of crashing (with no references to what made it crash)
@riffraff it's fixed, I have removed eval, used to_source or to_value instead of eval, please run synvert --sync first, then let me know if it works for you.
it sorta works, in the sense that it now does
params.require(:xx).permit(*MY_CONSTANT)
which will fail at runtime cause MY_CONSTANT is from another scope (should be MyModel::MY_CONSTANT) but that is good enough for me thanks!