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

Codegen: Transient private vals in super constructor of data class #767

Closed
remcomokveld opened this issue Dec 4, 2018 · 3 comments
Closed

Comments

@remcomokveld
Copy link

remcomokveld commented Dec 4, 2018

I have a base class which has private vals defined in its constructor. and a data class subclass which passes these values to the super constructor.

abstract class MyAbstractClass(
  @Transient private val texts: Map<String, String>
  @Transient private val imageUrls: Map<String, String>
) {
  val title: String? get() = texts["title"]
  val posterImage: String? get() = images["poster"]
}

@JsonClass(generateAdapter = true)
data class MyConcreteClass(
  val texts: Map<String, String>
) : MyAbstractClass(texts)

Right now it is then required to annotate all private properties of MyAbstractClass as @Transient because the codegen will otherwise throw an exception.

Since having all primary constructor properties being a val is a requirement of a data class I think the moshi codegen module should be able to figure out that the private property in the super class should never be (de)serialized and thus there is no need to crash when the property is private.

@ZacSweers
Copy link
Collaborator

To make sure I'm reading this right: is the ask namely to just treat private, non-mutable, supertype properties as implicitly transient?

I'm kind of on board with that, but wondering about the implications of:

@swankjesse
Copy link
Member

I don’t think we treat data classes as special. (And I think we can also remove TargetType.isDataClass which is unused).

@swankjesse
Copy link
Member

We conferred. It’s a good proposal in isolation, but it brings up some potentially surprising consequences...

Lets start with this class:

open class BaseServerResponse(
  var error_message: String?
)

Servers send the error_message field down sometimes when something breaks. Client developers are like “hey, why is this a val? nobody writes it!” and they flip it to a val.

open class BaseServerResponse(
  val error_message: String?
)

With this change the system appears to work normally but now Moshi ignores this field! Bad!

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

No branches or pull requests

3 participants