search

JakeWharton / dagger-reflect

A reflection-based implementation of the Dagger dependency injection library for fast IDE builds.
Apache License 2.0
654 stars 44 forks source link

Full reflection with obfuscation #209

Open EricKuck opened 3 years ago

EricKuck commented 3 years ago

We've been wanting to switch to full reflection for a while, but are worried about not being able to obfuscate the class names of our components. The concern here is that if we have a component named SecretExperimentalFeatureComponent, we may expose info on a feature that is hidden behind an experiment.

With the current approach this library takes there's really no getting around the requirement for unobfuscated component class names, so I was wondering how a new approach would be received.

The idea is that we add a new annotation processor that would be run in release mode (or whenever the current dagger-codegen artifact would be used). This processor would go module by module, creating a registry for each. The registry would essentially be a Map<Class, () -> (Component|Builder|Factory)>. An example entry in this map would be key: MyComponent.Factory.class value: { DaggerMyComponent.factory() }. This creates a mapping to generated components that would survive obfuscation. The top-level component would then aggregate these registries and would be able to discover all dagger-generated components using the same Dagger.create, Dagger.factory, and Dagger.builder calls that are already used.

A minor secondary benefit to this approach would be that we'd be back to using no reflection at all for release builds. An obvious downside is that more codegen is required, but it would only be needed for release mode.

I'd be happy to open a PR if there's agreement on this approach. Also very interested to hear what other opinions people might have on this.

runningcode commented 3 years ago

The readme states that this library is for fast IDE builds and tests. What is the use case for dagger reflect in release builds?

EricKuck commented 3 years ago

The readme states that this library is for fast IDE builds and tests. What is the use case for dagger reflect in release builds?

Using full reflection requires code changes for all builds, not just debug. I don't want to use reflection in release builds, but you still have to use this library's Dagger.create, Dagger.factory, or Dagger.builder calls in release mode. That's what I'm proposing a change for.

JakeWharton commented 3 years ago

You don't have to, there's already an annotation processor which generates bridges into the reflection code.

2 would also solve this if you're using Kotlin

EricKuck commented 3 years ago

To be clear here, our goal is to switch to full reflection so we can completely disable kapt for most modules while in debug mode. We're currently using the partial reflection approach and it's been great. I don't think there's a way to use full reflection in debug mode without also forcing the requirement that component names avoid obfuscation. Maybe I'm missing something about another setup that isn't listed in the readme?

The code in DaggerCodegen.java can't work with obfuscation enabled. The deduceImplementationClassName method takes the component's classname and prepends Dagger to it. With obfuscation enabled, MyComponent would be renamed to something like a and DaggerMyComponent would be renamed to something like b, which would cause this to fail. Even if the class names happened to work, the invokeStatic calls would fail due to method names being rewritten.

My proposal is to work around having to deduce class or method names with reflection, which is what would make this work with obfuscation.