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

ClassCastException: class cannot be cast to class scala.runtime.Nothing$ #22204

Open
jtjeferreira opened this issue Dec 12, 2024 · 12 comments
Open
Labels
compat:java itype:bug itype:soundness Soundness bug (it lets us compile code that crashes at runtime with a ClassCastException) stat:needs minimization Needs a self contained minimization

Comments

@jtjeferreira
Copy link
Contributor

Compiler version

Scala 3.6.2, 3.5.2, 3.3.4

Minimized code

import com.fasterxml.jackson.databind.JsonNode
import com.fasterxml.jackson.databind.node.{
  JsonNodeFactory,
  ObjectNode,
  TextNode
}

val json: ObjectNode = JsonNodeFactory.instance.objectNode()

def foo(a: String): Unit = {
  a match {
    case "a" =>
      json.set("type", TextNode.valueOf("value"))
    case _ =>
      json.set("type", TextNode.valueOf("value"))

  }
}
foo("")

method set is a java method:

public <T extends JsonNode> T set(String propertyName, JsonNode value)

https://scastie.scala-lang.org/i507EyzXTdO1BM362YaOhA

Output

Caused by: java.lang.ClassCastException: class com.fasterxml.jackson.databind.node.ObjectNode cannot be cast to class scala.runtime.Nothing$

Expectation

That it does not crash like in scala2 https://scastie.scala-lang.org/gvqImCf7QeKBXahfClCZHA

@jtjeferreira jtjeferreira added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Dec 12, 2024
@Gedochao
Copy link
Contributor

Gedochao commented Dec 16, 2024

Scala CLI repro:

//> using dep com.fasterxml.jackson.module::jackson-module-scala:2.15.4
import com.fasterxml.jackson.databind.JsonNode
import com.fasterxml.jackson.databind.node.{
  JsonNodeFactory,
  ObjectNode,
  TextNode
}

val json: ObjectNode = JsonNodeFactory.instance.objectNode()

def foo(a: String): Unit = {
  a match {
    case "a" =>
      json.set("type", TextNode.valueOf("value"))
    case _ =>
      json.set("type", TextNode.valueOf("value"))

  }
}
@main def main = foo("")

Fails at runtime with:

Exception in thread "main" java.lang.ClassCastException: class com.fasterxml.jackson.databind.node.ObjectNode cannot be cast to class scala.runtime.Nothing$ (com.fasterxml.jackson.databind.node.ObjectNode and scala.runtime.Nothing$ are in unnamed module of loader 'app')
        at repro$package$.foo(repro.scala:16)
        at repro$package$.main(repro.scala:20)
        at main.main(repro.scala:20)

Optimally, we'd like to minimize without the fasterxml dependency.

@Gedochao Gedochao added itype:soundness Soundness bug (it lets us compile code that crashes at runtime with a ClassCastException) compat:java stat:needs minimization Needs a self contained minimization and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Dec 16, 2024
@Gedochao
Copy link
Contributor

cc @sjrd

@He-Pin
Copy link

He-Pin commented Dec 16, 2024

seems like the return type is convert to nothing instead insert a ()

@He-Pin
Copy link

He-Pin commented Dec 16, 2024

a quic fix :

import com.fasterxml.jackson.databind.JsonNode
import com.fasterxml.jackson.databind.node.{
  JsonNodeFactory,
  ObjectNode,
  TextNode
}

val json: ObjectNode = JsonNodeFactory.instance.objectNode()

def foo(a: String): Unit = {
  a match {
    case "a" =>
      json.set("type", TextNode.valueOf("value"))
    case _ =>
      json.set("type", TextNode.valueOf("value"))
      ()
  }
}
foo("")

This is a bug I think even I insert the () at the end of the method block will not work.

@sjrd
Copy link
Member

sjrd commented Dec 16, 2024

The signature of set makes no sense. It is not possible for such a method to return a valid T for all T extending JsonNode. The Scala compiler chooses to instantiate T = Nothing, and as expected, set is not able to fulfill its promise, so you get a CCE.

There's no language bug, here.

Explicitly write the type parameter to something that set actually returns. Presumably set[JsonNode](...).

@He-Pin
Copy link

He-Pin commented Dec 16, 2024

That's seems true

abstract class BaseJsonNode
    extends JsonNode
abstract class ContainerNode<T extends ContainerNode<T>>
    extends BaseJsonNode
class ObjectNode
    extends ContainerNode<ObjectNode>

and In ObjectNode:

    public <T extends JsonNode> T set(String propertyName, JsonNode value)
    {
        if (value == null) {
            value = nullNode();
        }
        _children.put(propertyName, value);
        return (T) this;
    }

I think the code should be written to :

    public ObjectNode set(String propertyName, JsonNode value)
    {
        if (value == null) {
            value = nullNode();
        }
        _children.put(propertyName, value);
        return (T) this;
    }

@pjfanning cc

@He-Pin
Copy link

He-Pin commented Dec 16, 2024

The current implementation in Jackson will cause error anyway, eg:

ArrayNode arrayNode = objectNode.set(..)

which will throw ClassCustException too, but the compiler will not complain.

@pjfanning
Copy link
Contributor

If anyone is suggesting that it is Jackson that gets changed, I honestly don't think the Jackson team (which includes me) will be changing the Java code in Jackson to suit the Scala 3 compiler.

Scala 3 users form a tiny minority of Jackson users and it's hard to justify discommoding other Jackson users by changing Jackson APIs.

Jackson 3 might be a place where we can make a change but there is no date for a GA release of Jackson 3.

@pjfanning
Copy link
Contributor

Interestingly, Jackson 3 code has been changed so that ObjectNode.set and setAll returns ObjectNode explicitly.

https://github.com/FasterXML/jackson-databind/blob/9b93072efe469d7bb9c3281e90a022f4e83d4443/src/main/java/tools/jackson/databind/node/ObjectNode.java#L537

There are snapshots published.
Note the s01.oss.sonatype.org and the groupId has changed to tools.jackson (and the package names too).

https://s01.oss.sonatype.org/content/repositories/snapshots/tools/jackson/core/jackson-databind/3.0.0-SNAPSHOT/

@pjfanning
Copy link
Contributor

@jtjeferreira a messy workaround might be feasible. Maybe a Java class in your code base called something like ObjectNodeWrapper that wraps an ObjectNode and that adds set and setAll methods that delegate to the ObjectNode methods. The ObjectNodeWrapper methods would return ObjectNode explicitly.

@sjrd
Copy link
Member

sjrd commented Dec 16, 2024

@jtjeferreira a messy workaround might be feasible. Maybe a Java class in your code base called something like ObjectNodeWrapper that wraps an ObjectNode and that adds set and setAll methods that delegate to the ObjectNode methods. The ObjectNodeWrapper methods would return ObjectNode explicitly.

The correct workaround is much simpler. I mentioned it above: call set with an explicit type argument.

@jtjeferreira
Copy link
Contributor Author

The correct workaround is much simpler. I mentioned it above: call set with an explicit type argument.

yes this workaround works and thats what I am using. I just wanted this to work as in scala2... If that not feasible, feel free to close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat:java itype:bug itype:soundness Soundness bug (it lets us compile code that crashes at runtime with a ClassCastException) stat:needs minimization Needs a self contained minimization
Projects
None yet
Development

No branches or pull requests

5 participants