[GR-54995] Should '0' should not be able to map to int? type coercion

[in-fke]

Describe GraalVM and your environment :

• GraalVM version or commit id if built from source: 23.0.4
• CE or EE: CE
• JDK version: 17
• OS and OS Version: Windows
• Architecture: x64
• The output of java -Xinternalversion:

<using stock VM>

Have you verified this issue still happens when using the latest snapshot? You can find snapshot builds here: https://github.com/graalvm/graalvm-ce-dev-builds/releases No, but I have looked at the code and it's the same: https://github.com/oracle/graal/blob/master/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostUtil.java#L141

Describe the issue Rhino can "cast" to int, while Graal / Truffel does not, see code below.

Get ClassCastException with Cannot convert 'com.oracle.truffle.api.strings.TruffleString'(language: Java, type: com.oracle.truffle.api.strings.TruffleString) to Java type 'int': Invalid or lossy primitive coercion. Code snippet or code repository that reproduces the issue

    public static class TakesInt {
        @Export
        public void method(int arg) {};
    }
    
    @Test
    public void testTakesInt() {
        Context context = Context.create();
        context.getBindings("js").putMember("takesInt", new TakesInt());
        context.eval("js", "takesInt.method('0');");
    }

Steps to reproduce the issue Run the above test.

Expected behavior I would assume that Truffle would cast that (as long as it's compliant?). Caller is com.oracle.truffle.host.HostToTypeNode.convertImpl() Maybe https://github.com/oracle/graal/blob/master/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostUtil.java#L141 should support other target types (such as int?).

Additional context Add any other context about the problem here. Specially important are stack traces or log output. Feel free to link to gists or to screenshots if necesary

Details

TypeError: invokeMember (method) on ScriptingServiceGraalStandaloneTest$TakesInt failed due to: Cannot convert 'com.oracle.truffle.api.strings.TruffleString'(language: Java, type: com.oracle.truffle.api.strings.TruffleString) to Java type 'int': Invalid or lossy primitive coercion.
	at <js> :program(Unnamed:1:0-19)
	at org.graalvm.polyglot.Context.eval(Context.java:429)
	at ScriptingServiceGraalStandaloneTest.testTakesInt(ScriptingServiceGraalStandaloneTest.java:144)

[in-fke]

https://github.com/oracle/graaljs/issues/272

[in-fke]

I will probably use this workaround (targetTypeMapping), question is: is it valid, that Graal does not do this out-of-the-box?

    public static class TakesInt {
        @Export
        public void method(int arg) {};
    }

    @Test
    public void testTakesIntWithCoercion() {
        // https://github.com/oracle/graal/issues/9096
        AtomicInteger predicateSucceededCount = new AtomicInteger();
        AtomicInteger predicateFailedCount = new AtomicInteger();
        AtomicInteger conversionCount = new AtomicInteger();        
        HostAccess hostAccess = HostAccess.newBuilder(HostAccess.ALL)
                // Rhino would do the same!?
                .targetTypeMapping(String.class, Integer.class, target -> {
                    boolean result;
                    try {
                        double d = Double.valueOf(target);
                        if (Math.floor(d) != Math.ceil(d)) {
                            result = false;
                        } else if (d > Integer.MAX_VALUE) {
                            result = false;
                        } else if (d < Integer.MIN_VALUE) {
                            result = false;
                        } else {
                            result = true;
                        }
                    } catch (NumberFormatException e) {
                        result = false;
                    }
                    if (result) {
                        predicateSucceededCount.incrementAndGet();
                    } else {
                        predicateFailedCount.incrementAndGet();
                    }
                    return result;
                }, target -> {
                    conversionCount.incrementAndGet();
                    return Double.valueOf(target).intValue();
                })
                //
                .build();        
        Context context = Context.newBuilder("js")
                .allowHostAccess(hostAccess)
                // required to declare class with Java.type
                .allowHostClassLookup(s -> true).build();                
        Value bindings = context.getBindings("js");
        bindings.putMember("takesInt", new TakesInt());
        context.eval("js", "const Integer = Java.type('" + Integer.class.getName() + "');");
        // these must not throw
        context.eval("js", "takesInt.method('0');");
        context.eval("js", "takesInt.method('' + Integer.MAX_VALUE);");
        context.eval("js", "takesInt.method('' + Integer.MIN_VALUE);");
        assertEquals(3, predicateSucceededCount.get());
        assertEquals(0, predicateFailedCount.get());        
        assertEquals(3, conversionCount.get());
        List.of(
                // too small
                "takesInt.method('' + (Integer.MIN_VALUE - 1));",
                // too large
                "takesInt.method('' + (Integer.MAX_VALUE + 1));",
                // not integer
                "takesInt.method('1.5');")
                //
                .forEach((script) -> {
                    try {
                        context.eval("js", script);
                        fail("for script [" + script + "]: expected excpetion to be thrown");
                    } catch (Exception e) {
                        assertEquals("for script [" + script + "]: exception class mismatch", PolyglotException.class, e.getClass());
                        assertTrue("for script [" + script + "]: unexpected message: [" + e.getMessage() + "]", StringUtils.contains(e.getMessage(), "Invalid or lossy primitive coercion"));
                    }
                });
        assertEquals(3, predicateSucceededCount.get());
        assertEquals(3, predicateFailedCount.get());        
        assertEquals(3, conversionCount.get());
    }