Verifying argument types (A call to arms)

[Ola Bini <ola.bini@gmail.com>]

Hi,

Before I release Ioke S, I would like to fix one thing that has bugged 
me a bit lately. Namely, many of the implementations of core methods in 
Java assume that they get the correct argument type, using casts 
directly. In some places I've avoided it, but in many more places I haven't.
These are all over the place, really, and I need help fixing it up - and 
writing tests for it. As a typical example, take List#+, which takes a 
list and creates a new list with its contents and the receivers 
contents. The actual code looks like this:

List<Object> args = new ArrayList<Object>();
getArguments().getEvaluatedArguments(context, message, on, args, new 
HashMap<String, Object>());

List<Object> newList = new ArrayList<Object>();
newList.addAll(((IokeList)IokeObject.data(on)).getList());
newList.addAll(((IokeList)IokeObject.data(args.get(0))).getList());
return context.runtime.newList(newList, IokeObject.as(on));


If you haven't looked at the Java implementations, some pointers might 
be in order. The getArguments() call will return an instance of 
DefaultArgumentsDefinition. This is used to evaluate arguments, and then 
add them to the "args" list. The definition of which kind of arguments 
are valid happens outside the actual method body, in a definition that 
looks like this:

private final DefaultArgumentsDefinition ARGUMENTS = 
DefaultArgumentsDefinition
        .builder()
        .withRequiredPositional("otherList")
        .getArguments();


Of course, since Ioke is dynamically typed, the name "otherList" is just 
documentation, nothing the implementation can use.

The first line of the actual code creates a new ArrayList.
The second line extracts the internal list from "on", and adds the 
contents to the new list. And the third line extracts the internal list 
from the argument and adds the content to the new list.
The final line creates a new Ioke list from the ArrayList, using the 
receiver "on" as mimic.

The problematic pieces are
(IokeList)IokeObject.data(on)
(IokeList)IokeObject.data(args.get(0))

If I do something like this:
 foo = Origin mimic
 foo cell("+") = List cell("+")
 foo + [1,2,3]
it will blow up like this:
 *** - java.lang.ClassCastException: ioke.lang.IokeData$1 cannot be 
cast to ioke.lang.IokeList (Condition Error JavaException)

The exact same thing happens if I do:
 [1,2,3] + :foo

Of course, these things shouldn't actually work, but they should give 
reasonable error messages.

So, the actions that need to be taken are to go through everything, 
remove all places that cast data like this, and instead make use of 
converters.

There are some specialized converters, for converting to rationals for 
example. These use methods on IokeData that has been overridden to do 
the right thing.
You can see how this works in any of the number implementations. Try to do
  1 + :foo
and you will get
 *** - condition reported (Condition Error Type IncorrectType)
This is because the + method does something like this if it doesn't get 
the correct type:                  
  arg = IokeObject.convertToRational(arg, message, context, true);

In the case of lists, I haven't created anything yet, and I thought it 
would be great to have a general conversion mechanism. I'm thinking of 
having an API that basically looks like this:
  IokeObject.convertTo("List", obj, true, "asList", message, context);

The first parameter is the kind name to look for. This could also be an 
object that must be mimicked. The second argument is the object to 
convert. The third argument is whether to signal a condition or just 
return null. The fourth argument is the conversion method to call. If 
null, no such conversion will happen. The message and context parameters 
are just standard arguments to make condition signalling work. I will 
add this protocol sometime today. In most cases it might not be 
interesting to actually provided a conversion method name at all, but 
this method makes it possible to give good error messages anyway.

So, in retrospect, the above code should probably have been written like 
this:

List<Object> args = new ArrayList<Object>();
getArguments().getEvaluatedArguments(context, message, on, args, new 
HashMap<String, Object>());

List<Object> newList = new ArrayList<Object>();
Object onAsList = on;
if(!(IokeObject.data(onAsList) instanceof IokeList)) {
 onAsList = IokeObject.convertTo(context.runtime.list, onAsList, true, 
null, message, context)
}
newList.addAll(((IokeList)IokeObject.data(onAsList)).getList());
Object argAsList = args.get(0);
if(!(IokeObject.data(argAsList) instanceof IokeList)) {
 argAsList = IokeObject.convertTo(context.runtime.list, argAsList, 
true, null, message, context)
}
newList.addAll(((IokeList)IokeObject.data(argAsList)).getList());
return context.runtime.newList(newList, IokeObject.as(onAsList));


As you might understand, adding this kind of code and tests for it all 
over the place is kinda substantial, which is why I'm asking for help on 
this. =)


Cheers

- 
Ola Bini (http://olabini.com) 
JRuby Core Developer
Developer, ThoughtWorks Studios (http://studios.thoughtworks.com)
Practical JRuby on Rails (http://apress.com/book/view/9781590598818)

"Yields falsehood when quined" yields falsehood when quined.