Codegen should survive ProGuard

[swankjesse]

See this G+ thread & branch from @cermakcz:
https://plus.google.com/u/0/117878637395157279424/posts/HMiCxi4wh7J
https://github.com/cermakcz/dagger/compare/obfuscatable

[JakeWharton]

Adding this to the 1.0 milestone. I'd love to see this land in the next two or three weeks!

[swankjesse]

That's pretty ambitious. I'm not ready to ship this yet!

[mgrzechocinski]

Hey,

I'd like to know if there are any plans to rebase & integrate this pull request? I'd love to use Dagger in my project but one of the critical requirements is obfuscation.

M.

[orip]

@mgrzechocinski see this SO answer and the comments on this G+ thread.
People have gotten Dagger 1.1.0 working reliably with obfuscation, with some extra ProGuard rules.

[mgrzechocinski]

Thanks for those links, they are very helpful. I have a requirement that the app has to be obfuscated as much as possible. Keeping e.g. names of classes which contains injects (coffee.* in sample app) seems to be unacceptable. I definitely need to play with it.

[cermakcz]

Dagger's codegen changed quite a bit since I've forked it and I'm afraid I now don't have the time to rebase my changes. I also lack a will to do it, since it's clearly not a way Dagger's creators meant for it to go. Otherwise they'd have probably already done it.

[nimeacuerdo]

This is still an open enhancement issue for milestone 2.0, but it looks like we should we forget about any other approach in dagger regarding ProGuard integration rather than tweaking the latter (as my SO question mentioned above suggests).

[JurgenCruz]

I have been trying to make a generic proguard file that makes it possible to use dagger-compiler.
So far here is what I got:

#Keep the annotated things annotated
-keepattributes *Annotation*

#Keep the dagger annotation classes themselves
-keep @interface dagger.*,javax.inject.*

#Keep the Modules intact
-keep @dagger.Module class *

#-Keep the fields annotated with @Inject of any class that is not deleted.
-keepclassmembers class * {
  @javax.inject.* <fields>;
}

#-Keep the names of classes that have fields annotated with @Inject and the fields themselves.
-keepclasseswithmembernames class * {
  @javax.inject.* <fields>;
}

# Keep the generated classes by dagger-compile
-keep class **$$ModuleAdapter
-keep class **$$InjectAdapter
-keep class **$$StaticInjection

I'm sure I'm missing more things, but the one that is currently blocking me is this use case:

If I have a base class:

public class Parent {
  @Inject Something something;
  ...
}

And an inherited class:

public class Child extends Parent {
  ...
}

I will have to declare both in the Module like this:

@Module(injects = {Parent.class, Child.class})
public class Module {
  @Provide
  Something provideSomething() {
    return new Something();
  }
}

Now, the parent class is covered by the fact that it has an @Inject annotated field. but the Child will get obfuscated making the IllegalArgumentException: No inject registered for members/a.a. ... to be thrown.

This could be solved kind of easy if the dagger-compiler generated a Proguard config file which simply adds all classes declared in the injects = ... attribute of @Module.

So far my proguard file covers all modules (and its provide methods) and all classes with Injected methods. Don't know what other use cases are needed to be covered.

What you think of generating this config file?

[JakeWharton]

That's not the direction we want to go for ProGuard safety, but it would work in the interim.

[JurgenCruz]

Oh, what's the direction you aim at?

[JakeWharton]

Using "members/" + Foo.class.getCanonicalName(); so it would just be obfuscated to "members/" + A.class.getCanonicalName().

[JurgenCruz]

Is this already implemented in a snapshot or is it a work in progress, our
is it a distant future?

[JakeWharton]

Distant. Nobody is currently working on this.

Additionally, this type of thing is not required with the current version 2 design.

[JurgenCruz]

What do you mean is not requiered in version 2? will version not have
problems with proguard?

Also, if that is the case, will a PR for version 1.1.x be welcome?

[JakeWharton]

will version not have problems with proguard?

No. There will be zero reflection.

Also, if that is the case, will a PR for version 1.1.x be welcome?

Yes. We can make a v1.3 with something like this if it proves to be valuable without causing runtime penalty.

[JurgenCruz]

Well, If I where to submit a PR it would be the generation of a Proguard
config file based on the dagger annotations just to prevent obfuscation in
those specific classes and members. I see no penalty there. But it is nice
to know Version 2 won't have those troubles. Any ETA on V2?

[cgruber]

V2.0 should be mostly in usable shape by end of Q2, though a good chunk
of it should be usable earlier than that. Should be some directly
visible activity in the next few weeks.

[JurgenCruz]

Another question. Will version 2 require to change annotations in the code?
Or will it be backwards compatible and just need to change the dependency?

[JakeWharton]

It is 100% backwards incompatible. You can learn more at #366.

[cgruber]

Eh. We've been tossing around some compatibility aids to make it less
than 100% incompatible. :) And the migration surface should be small,
if you're building an app with reasonable best-practices.

[JurgenCruz]

From what I see in the document, classes injected would stay very similar.
Same for modules with providers. The biggest change I notice is the switch
from object graph to these new components. But as you say it should not be
a big problem.

[JurgenCruz]

Ok, trying to figure out what rules would be missing in my generic proguard config, I've found the following:

• Proguard needs not to remove the annotations from code.
• Proguard should not remove, nor rename the @Module annotated classes.
• Proguard should not remove, nor rename the fields of classes annotated with @Inject
• Proguard should not rename the classes that are being injected (either the types of the fields annotated with @Inject or the types of the methods annotated with @Provide
• Proguard should not remove nor rename the generated Adapters.
• Proguard should not rename the entry points (the classes declared in the @Module(injects = {...}) section)
• Proguard should not rename the first super class of any entrypoint. (apparently, every InjectAdapter generated makes a key referencing the super class by name even if the super class has no injected fields. But I don't understand why only the first super type is strongly referenced but the rest of the hierarchy isn't a problem if it is obfuscated.)

I do not use the @Inject annotation in constructors so I'm probably missing a use case there.

Of all this points, the ones that would need to be incorporated in the generation of a proguard config file would be the entrypoints, the first father of the entrypoints and the types that the providers return.

I'm not sure if sending a PR for this or waiting for version 2 is a better idea.

[chrisjenx]

@SuperJugy your generic config works pretty well for me.

[JurgenCruz]

Are you using the objectGraph.inject(this) in conjunction with

@Module(injects=AnotherClassThatInheritsFromAnInjectedClass.class)
public class Module {
  @Provide
  Something provideSomething() {
    ...
  }
}

Because with the above approach, proGuard obfuscates the class Something (and then the module can't find the provided class) and the class AnotherClassThatInheritsFromAnInjectedClass because it doesn't have any @Injectable parameters, rather implements one that does (Making the objectGraph not finding the entry point).

Notice that I'm using the dagger-compiler to avoid reflection as much as I can. But I really want to try out dagger 2.0 with no reflection and more intuitive API.

My current approach was to remove all the classes from @Module(injects=...) and instead, in the constructor of such classes, use mSomething = objectGraph.get(Something.class); to inject my fields. But even with this approach, the class Something is still getting obfuscated. I can obviously add manually the class but I was looking for a more generic config or automated generation that doesn't require to manually add this dependencies.

[chrisjenx]

@SuperJugy yeah I'm still testing it.

I pretty much exclusively use:

@Inject
public Something(){
}

for my none injects={} classes.

Interestingly I think because I very rarely do:

@Provides
Something providesSomething(){
  return new SomethingImpl();
}

Or I have explicitly defined @Inject SomethingImpl something elsewhere, your rules work.

[chrisjenx]

@SuperJugy I don't do: @Module(injects=AnotherClassThatInheritsFromAnInjectedClass.class) which is why I guess your config works for me.

I just made the following change to make sure @Inject constructor classes are not changed.

#-Keep the names of classes that have fields annotated with @Inject and the fields themselves.
-keepclasseswithmembernames class * {
  @javax.inject.* <fields>;
  @javax.inject.* <init>(...);
}

Noticed though that barely anything is obfuscated. Not ideal but better than nothing. Guess the full swing to 2.0 will improve this greatly when very little reflection is done.

[JurgenCruz]

Yeah I really hope for it.

[mttkay]

What's the state of this? I've been through many variations of PG config, none of which worked.

Still trying to get to the bottom of it, but what I believe happens is that ProGuard renames a base type from which injectable classes inherit, but the inject adapters are still referring to that base type by its unobfuscated name. I'm not sure why Dagger is even looking at that class, as it has neither Inject annotations nor injectable fields or constructors (it's a plain old base class), but that's what's happening.

[mttkay]

Ah, it's because of this:

// $$InjectAdapter
private Binding<com.soundcloud.android.main.DefaultLifeCycleComponent> supertype;

so what we'd need in addition to what's been mentioned here is a rule that says: whenever a class is used as part of injection, keep the super class, too.

[chrisjenx]

I've got a pro guard config that seems to work well. I'll post it in a sec.
On 19 Aug 2014 07:07, "Matthias Käppler" notifications@github.com wrote:

Ah, it's because of this:

// $$InjectAdapter
private Binding<com.soundcloud.android.main.DefaultLifeCycleComponent> supertype;

so what we'd need in additional to what's been mentioned here is a rule
that says: whenever a class is used as part of injection, keep the super
class, too.

—
Reply to this email directly or view it on GitHub
#202 (comment).

[mttkay]

@chrisjenx I had some success with this config:

-keepnames com.soundcloud.android.**
...
-keepclassmembers,allowobfuscation class * {
    @javax.inject.* *;
    @dagger.* *;
    <init>();
}

-keep class javax.inject.** { *; }
-keep class **$$ModuleAdapter
-keep class **$$InjectAdapter
-keep class **$$StaticInjection
-keep class dagger.** { *; }

meaning, I couldn't make it work without turning off obfuscation entirely for any classes referenced by Dagger. Good enough I guess.

[mttkay]

Oh and, I had to add exceptions for any classes that PG decided to completely remove (or merge with others) and which were referenced by Dagger. We only had the one I mentioned above though, so I had to fully -keep it, which is perfectly fine, however.

[alexandru-calinoiu]

@chrisjenx Can you please post the config that you got working?

[IlyaEremin]

+1 @Balauru
cc @chrisjenx

[JakeWharton]

See Dagger 2.