dropbox / djinni

A tool for generating cross-language type declarations and interface bindings.
Apache License 2.0
2.88k stars 488 forks source link

Android Parcelable missing ArrayList import #408

Closed skonstant closed 5 years ago

skonstant commented 5 years ago

I have a record that has sets no lists, and when the parcelable code is generated it creates something like:

ArrayList mSubscriptionoasisTemp = new ArrayList(); in.readList(mSubscriptionoasisTemp, getClass().getClassLoader()); this.mSubscriptionoasis = new HashSet(mSubscriptionoasisTemp);

But since there is no list in the record, the import directive for ArrayList is not there, which breaks compilation.

The fix is not simple, I have a quick workaround for now but I don't think it will work with records that do have lists, it might import twice and break compilation again.

In JavaGenerator.scala:

override def generateRecord(origin: String, ident: Ident, doc: Doc, params: Seq[TypeParam], r: Record) { val refs = new JavaRefs() r.fields.foreach(f => refs.find(f.ty))

if (spec.javaImplementAndroidOsParcelable && r.derivingTypes.contains(DerivingType.AndroidParcelable))
  refs.java += "java.util.ArrayList"

The last two lines fix it.

artwyman commented 5 years ago

@skonstant want to submit a PR for this fix?

I also wonder if the simple workaround you have above might be too broad for cases where a record implements Parcelable but doesn't contain any container types at all (sets, maps, lists) so won't need any readList() calls at all.

4brunu commented 5 years ago

I just created a PR https://github.com/dropbox/djinni/pull/434 that fixes this issue.

paulocoutinhox commented 5 years ago

@artwyman can you merge it?