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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reflector: use companion object apply method #487

Merged
merged 2 commits into from
Nov 26, 2017

Conversation

shanielh
Copy link
Contributor

@shanielh shanielh commented Nov 14, 2017

even when constructors are available.

This is because companion.apply(...) in Scala has more power than secondary c'tors.

  • Changed Reflector
  • Added Test

Maintainers: I had to change reflector to lookup the type parameters by name and not by instance because the apply method in scala (that is generated for case classes) has different instances of type parameters than those of the class. That means that if someone will create Foo.apply[A](..) for case class Foo[B](..), it might not work. I can throw exception in that case, or whatever - Just let me know what you want, if you want this pull request 馃槃

even when constructors are available
@seratch
Copy link
Member

seratch commented Nov 26, 2017

Thanks, LGTM.

@seratch seratch merged commit af8697d into json4s:3.6 Nov 26, 2017
@shanielh shanielh deleted the feature/apply-as-ctor branch November 26, 2017 13:25
@shanielh
Copy link
Contributor Author

Thanks for merging 馃帄

shanielh added a commit to shanielh/json4s that referenced this pull request Nov 26, 2017
@pwyczes
Copy link

pwyczes commented Mar 13, 2018

Hi @shanielh - thanks for this PR. I think it is very valuable.

Unfortunately I have a suspicion, that it breaks Extraction.decompose() for the case, when case class is generic and apply() method in companion object has different signature than constructor of the case class.

Something like

object CaseClassWithCompanion {
  def apply[A](v: A): CaseClassWithCompanion[A] = CaseClassWithCompanion(v, "Bar")
}
case class CaseClassWithCompanion[A](value: A, other: String)

It throws

org.json4s.package$MappingException: Can't find constructor for CaseClassWithCompanion
	at org.json4s.reflect.package$.fail(package.scala:95)
	at org.json4s.reflect.ScalaSigReader$.$anonfun$readConstructor$3(ScalaSigReader.scala:21)
	at scala.Option.getOrElse(Option.scala:121)
	at org.json4s.reflect.ScalaSigReader$.readConstructor(ScalaSigReader.scala:21)
	at org.json4s.reflect.Reflector$ClassDescriptorBuilder.ctorParamType(Reflector.scala:93)
	at org.json4s.reflect.Reflector$ClassDescriptorBuilder.$anonfun$createConstructorDescriptors$6(Reflector.scala:153)
	at scala.collection.TraversableLike.$anonfun$map$1(TraversableLike.scala:234)
	at scala.collection.mutable.ResizableArray.foreach(ResizableArray.scala:59)
	at scala.collection.mutable.ResizableArray.foreach$(ResizableArray.scala:52)
	at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:48)
	at scala.collection.TraversableLike.map(TraversableLike.scala:234)
	at scala.collection.TraversableLike.map$(TraversableLike.scala:227)
	at scala.collection.AbstractTraversable.map(Traversable.scala:104)
	at org.json4s.reflect.Reflector$ClassDescriptorBuilder.$anonfun$createConstructorDescriptors$3(Reflector.scala:139)
	at scala.collection.TraversableLike.$anonfun$map$1(TraversableLike.scala:234)
	at scala.collection.IndexedSeqOptimized.foreach(IndexedSeqOptimized.scala:32)
	at scala.collection.IndexedSeqOptimized.foreach$(IndexedSeqOptimized.scala:29)
	at scala.collection.mutable.WrappedArray.foreach(WrappedArray.scala:38)
	at scala.collection.TraversableLike.map(TraversableLike.scala:234)
	at scala.collection.TraversableLike.map$(TraversableLike.scala:227)
	at scala.collection.AbstractTraversable.map(Traversable.scala:104)
	at org.json4s.reflect.Reflector$ClassDescriptorBuilder.createConstructorDescriptors(Reflector.scala:133)
	at org.json4s.reflect.Reflector$ClassDescriptorBuilder.constructorsAndCompanion(Reflector.scala:129)
	at org.json4s.reflect.Reflector$ClassDescriptorBuilder.result(Reflector.scala:180)
	at org.json4s.reflect.Reflector$.createDescriptor(Reflector.scala:53)
	at org.json4s.reflect.Reflector$.$anonfun$describe$1(Reflector.scala:48)
	at org.json4s.reflect.package$Memo.apply(package.scala:36)
	at org.json4s.reflect.Reflector$.describe(Reflector.scala:48)
	at org.json4s.Extraction$.decomposeObject$1(Extraction.scala:119)
	at org.json4s.Extraction$.internalDecomposeWithBuilder(Extraction.scala:230)
	at org.json4s.Extraction$.decomposeWithBuilder(Extraction.scala:65)
	at org.json4s.Extraction$.decompose(Extraction.scala:244)
	at org.json4s.ExtractionBugs.$anonfun$new$101(ExtractionBugs.scala:299)
	at org.specs2.matcher.MatchResult$$anon$12.$anonfun$asResult$1(MatchResult.scala:344)
	at org.specs2.execute.ResultExecution.execute(ResultExecution.scala:23)
	at org.specs2.execute.ResultExecution.execute$(ResultExecution.scala:21)
	at org.specs2.execute.ResultExecution$.execute(ResultExecution.scala:118)
	at org.specs2.execute.Result$$anon$11.asResult(Result.scala:246)
	at org.specs2.execute.AsResult$.apply(AsResult.scala:32)
	at org.specs2.matcher.MatchResult$$anon$12.asResult(MatchResult.scala:344)
	at org.specs2.execute.AsResult$.apply(AsResult.scala:32)
	at org.specs2.specification.core.AsExecution$$anon$1.$anonfun$execute$1(AsExecution.scala:15)
	at org.specs2.execute.ResultExecution.execute(ResultExecution.scala:23)
	at org.specs2.execute.ResultExecution.execute$(ResultExecution.scala:21)
	at org.specs2.execute.ResultExecution$.execute(ResultExecution.scala:118)
	at org.specs2.execute.Result$$anon$11.asResult(Result.scala:246)
	at org.specs2.execute.AsResult$.apply(AsResult.scala:32)
	at org.specs2.execute.AsResult$.$anonfun$safely$1(AsResult.scala:40)
	at org.specs2.execute.ResultExecution.execute(ResultExecution.scala:23)
	at org.specs2.execute.ResultExecution.execute$(ResultExecution.scala:21)
	at org.specs2.execute.ResultExecution$.execute(ResultExecution.scala:118)
	at org.specs2.execute.AsResult$.safely(AsResult.scala:40)
	at org.specs2.specification.core.Execution$.$anonfun$result$1(Execution.scala:305)
	at org.specs2.specification.core.Execution$.$anonfun$withEnvSync$3(Execution.scala:323)
	at org.specs2.execute.ResultExecution.execute(ResultExecution.scala:23)
	at org.specs2.execute.ResultExecution.execute$(ResultExecution.scala:21)
	at org.specs2.execute.ResultExecution$.execute(ResultExecution.scala:118)
	at org.specs2.execute.Result$$anon$11.asResult(Result.scala:246)
	at org.specs2.execute.AsResult$.apply(AsResult.scala:32)
	at org.specs2.execute.AsResult$.$anonfun$safely$1(AsResult.scala:40)
	at org.specs2.execute.ResultExecution.execute(ResultExecution.scala:23)
	at org.specs2.execute.ResultExecution.execute$(ResultExecution.scala:21)
	at org.specs2.execute.ResultExecution$.execute(ResultExecution.scala:118)
	at org.specs2.execute.AsResult$.safely(AsResult.scala:40)
	at org.specs2.specification.core.Execution$.$anonfun$withEnvSync$2(Execution.scala:323)
	at org.specs2.specification.core.Execution.$anonfun$startExecution$3(Execution.scala:135)
	at scala.util.Success.$anonfun$map$1(Try.scala:251)
	at scala.util.Success.map(Try.scala:209)
	at scala.concurrent.Future.$anonfun$map$1(Future.scala:289)
	at scala.concurrent.impl.Promise.liftedTree1$1(Promise.scala:29)
	at scala.concurrent.impl.Promise.$anonfun$transform$1(Promise.scala:29)
	at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:60)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)

Do you think that this is a small fix, or maybe I should create another issue for this case?

@shanielh
Copy link
Contributor Author

@eventuallyinconsistent Hi! I created a test case which doesn't seem to work as you told. I'll try to figure out what's wrong with it in the weekend. You should open an issue for this case 馃憤

@pwyczes
Copy link

pwyczes commented Mar 18, 2018

Thank you @shanielh :)
I created an issue: #507

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants