Skip to content
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.

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

Validation of method-based coercion expression is not correct #48

Open
BladeWise opened this issue Feb 17, 2022 · 0 comments
Open

Validation of method-based coercion expression is not correct #48

BladeWise opened this issue Feb 17, 2022 · 0 comments
Assignees

Comments

@BladeWise
Copy link

BladeWise commented Feb 17, 2022

The check to ensure the method singature of a method-based UnaryExpression expression is probably not correct.

The current method is

private static UnaryExpression getMethodBasedCoercionOperator(
final ExpressionType unaryType,
final Expression operand,
final Type convertToType,
final MethodInfo method) {
assert method != null;
validateOperator(method);
final ParameterList parameters = method.getParameters();
if (parameters.size() != 0) {
throw Error.incorrectNumberOfMethodCallArguments(method);
}
final Type returnType = method.getReturnType();
if (TypeUtils.areReferenceAssignable(convertToType, returnType)) {
return new UnaryExpression(unaryType, operand, returnType, method);
}
// check for auto(un)boxing call
if (TypeUtils.isAutoUnboxed(convertToType) && returnType.isPrimitive() &&
TypeUtils.areEquivalent(returnType, TypeUtils.getUnderlyingPrimitive(convertToType))) {
return new UnaryExpression(unaryType, operand, convertToType, method);
}
throw Error.methodBasedOperatorMustHaveValidReturnType(unaryType, method);
}

which means that the coercion method must be:

  • A valid operator (see below)
  • Parameterless
  • A method whose return type is allowed to be assigned to the expected coercion type

To be a valid operator, as per the following function

private static void validateOperator(final MethodInfo method) {
assert method != null;
if (method.isStatic()) {
throw Error.operatorMethodMustNotBeStatic(method);
}
final Type returnType = method.getReturnType();
if (returnType == PrimitiveTypes.Void) {
throw Error.operatorMethodMustNotReturnVoid(method);
}
final ParameterList parameters = method.getParameters();
if (parameters.size() != 0) {
throw Error.operatorMethodParametersMustMatchReturnValue(method);
}
final Type<?> returnClass = TypeUtils.getBoxedTypeOrSelf(returnType);
if (!TypeUtils.areReferenceAssignable(method.getDeclaringType(), returnClass)) {
throw Error.methodBasedOperatorMustHaveValidReturnType(method);
}
}

it must be:

  • Non-Static
  • Non-Void returning
  • Parameterless
  • Declared by a class compatible with its return type

Above criteria do not meet what I would expect from a coercion operator, because there is no way for a non-static parameterless method declared by the expected return type, to convert a value from another type (that cannot be supplied). Eventually, it should be provided by a type assignable from the operand type.

Looking at the 'default' coercion methods

public static MethodInfo getCoercionMethod(final Type<?> source, final Type<?> destination) {
// NOTE: If destination type is an autoboxing type, we will need an implicit box later.
final Type<?> unboxedDestinationType = isAutoUnboxed(destination)
? getUnderlyingPrimitive(destination)
: destination;
if (destination == Types.String) {
return Types.String.getMethod("valueOf", source);
}
/*
if (!destination.isPrimitive()) {
return null;
}
*/
final MethodInfo method;
if (destination == PrimitiveTypes.Integer) {
method = source.getMethod("intValue", BindingFlags.PublicInstance);
}

we can see that the actual criteria could be
either:

  • Static
  • Provided by a type assignable from the coercion type
  • Accepting a parameter compatible with the provided operand type
  • Returning a type compatible with the coercion method
    like String::valueOf
    or
  • Non-Static
  • Provided by a type assignable from the operand type
  • Parameterless
  • Returning a type compatible with the coercion method
    like Long.intValue().

At least, the default coercion methods seem to fall in the above categories (and to be honest, I would avoid the strict check over the declaring type: why a static method provided by another class to perform the conversion is not allowed?).

For reference the current .NET implementation requires the method to be:

  • Static
  • Accepting a single parameter assignable from the operand type
  • Able to return a value assignable from the expected coercion type
@mstrobel mstrobel self-assigned this Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants