FR: correctly handle constructor-injection for records

[chris-gray-aem]

In #4344 @bcorso says

you may run into issues trying to use constructor injection with a record class (which probably doesn't make sense anyway, but just as an example).

I think it does make sense and would reduce some boilerplate.

For example, if you have

public class FooService {
  private final FooRepository fooRepository;

  @Inject
  public FooService(FooRepository fooRepository) {
    this.fooRepository = fooRepository;
  }
}

that's way more verbose than

public record FooService(FooRepository fooRepository) {
  @Inject
  public FooService {}
}

[bcorso]

Yes, we can certainly reconsider this.

At the time, I hadn't thought about it much, but my thought was that record/data classes were mostly for cases where you care about equals/hashcode, and it wouldn't make sense to use it for DI since every instance created will be identical (w.r.t. equals/hashcode of a record) because the values are all determined statically at compile time.

However, I see that that assumption could be wrong. For example, even if you don't care about equals/hashcode records could be useful for immutability (i.e. classes that just inject other dependencies with no internal state of its own), readability (it's more concise than normal classes), and efficiency (it seems like the JVM may have an easier time optimizing records compared to normal classes).

Anyway, happy to keep this open and consider supporting it.

[Chang-Eric]

I think it does make sense and would reduce some boilerplate.

The example you have focuses on the reducing boilerplate, but I'm not sure it is clear it makes sense? For example, does something that is a FooService really match the concept behind record classes? From https://openjdk.org/jeps/395, records are described with: "...records, which are classes that act as transparent carriers for immutable data. Records can be thought of as nominal tuples."

This is not a description that fits most service types or types instantiated by Dagger. So I feel like supporting this would be taking advantage of records as a means to reduce boilerplate, but not really using it correctly for the concept. I feel like this could later run into problems with some of the special rules that records have around constructors or with later updates or restrictions around the capabilities of records. As an example that isn't breaking but is sort of illustrative, I think record constructors have to match the visibility of the record class, whereas we generally recommend @Inject constructors be package-private because only Dagger should be instantiating them.

So if the main reason really is just to reduce boilerplate, I don't think that is a good enough reason to support this. In fact, in https://openjdk.org/jeps/395, they even mention that supporting records as a means to reduce boilerplate in general for objects is a non-goal. "While records do offer improved concision when declaring data carrier classes, it is not a goal to declare a "war on boilerplate". In particular, it is not a goal to address the problems of mutable classes which use the JavaBeans naming conventions."