Bad error Message from INTValueConverter

[cdietrich]

a failing INTValueConverter leeds to following error message

For input string: "999999999999999999999"

it would be nice to have better error messages

[cdietrich]

"Bad" Conversion is done by org.eclipse.xtext.parser.antlr.SyntaxErrorMessageProvider.getSyntaxErrorMessage(IValueConverterErrorContext) calling org.eclipse.xtext.parser.antlr.AbstractInternalAntlrParser.getValueConverterExceptionMessage(ValueConverterException)

[cdietrich]

i am not sure if this should be fixed on ValueConverter side or on SyntaxErrorMessageProvider side

[JanKoehnlein]

I'd say in the value converter. It is closer to the problem implementation-wise. I see SyntaxErrorMessageProvider more as a customization point for overriding defaults.

[cdietrich]

as i said: the value converter message is fine, but the other guys ignore its message and return the root cause

throw new ValueConverterException("Couldn't convert '" + string + "' to an int value.", node, e);

so we should not attach the root cause there?

here is what the parser does

    protected String getValueConverterExceptionMessage(ValueConverterException vce) {
        Exception cause = (Exception) vce.getCause();
        String result = cause != null ? cause.getMessage() : vce.getMessage();
        if (result == null)
            result = vce.getMessage();
        if (result == null)
            result = cause != null ? cause.getClass().getSimpleName() : vce.getClass().getSimpleName();
        return result;
    }

[cdietrich]

any hints?

[szarnekow]

I think the vce's message should win if it's not the default message.

[cdietrich]

hmmm a valueconverter exception is constructed as follows

    public ValueConverterException(String message, INode node, Exception cause) {
        super(message == null && cause != null ? cause.getMessage() : message, cause);
        this.node = node;
    }

=> the exception message contains the causes message if there was no explicit message

[szarnekow]

Meh. Comparing the causes' message with the value converters message doesn't make any sense in getValueConverterExceptionMessage so we should just use the message of the vce.

[cdietrich]

@miklossy do you plan to work on this issue?

[miklossy]

Not in the near future. I will unset the milestone.

[dpetroff]

This error message is a really great way to waste a morning debugging the parser and pulling at your hair when you change some rule names in the grammar and forget that somebody wrote custom value converters 2 years ago.

[cdietrich]

pull request still is welcome