Skip to content

Refactor - adding different scala object detecting #656

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 10 commits into
base: 2.17
Choose a base branch
from

Conversation

marcinbelicki
Copy link

@marcinbelicki marcinbelicki commented Nov 30, 2023

I'm adding ScalaObject along with ScalaObjectTest to detect whether given Class[_] is a scala singleton object. I'm not sure if clazz can be a null but I'm adding Option(clazz) in ScalaObjectDeserializerResolver just in case.


case object TestCaseObject

class TestClass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not add to src/test/scala ? does this not work with scala 3?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it the classes with names ending in $ - maybe scala3 doesn't allow this?

It is weird to have classes ending in $ - is there any way not to use these in tests?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are not necessary in tests but they are used just to make sure ScalaObject.unapply do not detect classes with names that end with a $ as scala singleton objects

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These class names are probably legal in scala 2 but illegal in scala 3. Since I haven't seen anyone in their right mind to use such class names I'm deleting them.

@pjfanning
Copy link
Member

build fails with

[info] com.fasterxml.jackson.module.scala.ScalaObjectMapperTest *** ABORTED ***
[info]   java.lang.InternalError: Malformed class name
[info]   at java.lang.Class.getSimpleName(Class.java:1330)
[info]   at com.fasterxml.jackson.module.scala.util.ScalaObject$.unapply(ScalaObject.scala:27)

@marcinbelicki
Copy link
Author

I've corrected clazz.getSimpleName to clazz.getName - I wasn't aware that classes from inside of scala singleton objects cant produce propper simple name :/

@pjfanning
Copy link
Member

@marcinbelicki I've started #657 - I may grab more of the changes in this PR but there I don't like all the new classes in this PR

@marcinbelicki
Copy link
Author

Ok, I see your point

@pjfanning
Copy link
Member

@marcinbelicki #657 is merged and will appear in v2.16.1 release - thanks for driving this

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants