Dagger: Scopes.getReadableSource() should output annotation values.

[pyricau]

In Square Register, we use a single scope annotation for most scopes and rely on annotation equality that Dagger implements correctly. Our annotation is @SingleIn:

/**
 * A class annotated with SingleIn(foo) is a singleton scoped to (ie, held by) the
 * {@link dagger.Component} that is also annotated with SingleIn(foo).
 */
@Scope public @interface SingleIn {
  Class<?> value();
}

Usage example: @SingleIn(LoggedInScope.class)

This has two advantages:

• No need to create new annotations for every scope (we have a lot of scope)
• We can command + click the class, which is usually an outer class of where the component is defined, so that provides easy navigation.

This works great. Unfortunately, Dagger error messages do not output the annotation values:

/src/main/java/com/squareup/FooComponent.java:18: error: com.squareup.FooComponent scoped with @com.squareup.dagger.SingleIn may not reference bindings with different scopes:
@Component(
^
      @com.squareup.dagger.SingleIn class com.squareup.bar.Bar
1 error

Ideally this would be:

/src/main/java/com/squareup/FooComponent.java:18: error: com.squareup.FooComponent scoped with @com.squareup.dagger.SingleIn(com.squareup.AppScope.class) may not reference bindings with different scopes:
@Component(
^
      @com.squareup.dagger.SingleIn(com.squareup.LoggedInScope.class) class com.squareup.bar.Bar
1 error

Scopes.getReadableSource() is currently implemented like so:

  /**
   * Returns the readable source representation (name with @ prefix) of the scope's annotation type.
   *
   * <p>It's readable source because it has had common package prefixes removed, e.g.
   * {@code @javax.inject.Singleton} is returned as {@code @Singleton}.
   *
   * <p>Does not return any annotation values, since {@link javax.inject.Scope @Scope} annotations
   * are not supposed to have any.
   */
  static String getReadableSource(Scope scope) {
    return stripCommonTypePrefixes("@" + scope.scopeAnnotationElement().getQualifiedName());
  }

While the Javax scope annotation javadoc states it should not have attributes, Dagger doesn't enforce that and it gives us much more flexibility.

[JakeWharton]

s/at Square/in Square Register/

On Mon, Apr 30, 2018, 8:24 PM Pierre-Yves Ricau [e-mail]
wrote:

At Square, we use a single scope annotation for most scopes and rely on
annotation equality that Dagger implements correctly. Our annotation is
@SingleIn:

/**

• A class annotated with SingleIn(foo) is a singleton scoped to (ie, held by) the
• {@link dagger.Component} that is also annotated with SingleIn(foo).
  */
  @Scope public @interface SingleIn {
  Class value();
  }

Usage example: @SingleIn(LoggedInScope.class)

This has two advantages:

• No need to create new annotations for every scope (we have a lot of
  scope)
• We can command + click the class, which is usually an outer class of
  where the component is defined, so that provides easy navigation.

This works great. Unfortunately, Dagger error messages do not output the
annotation values:

/src/main/java/com/squareup/FooComponent.java:18: error: com.squareup.FooComponent scoped with @com.squareup.dagger.SingleIn may not reference bindings with different scopes:
@Component(
^
@com.squareup.dagger.SingleIn class com.squareup.bar.Bar
1 error

Ideally this would be:

/src/main/java/com/squareup/FooComponent.java:18: error: com.squareup.FooComponent scoped with @com.squareup.dagger.SingleIn(com.squareup.AppScope.class) may not reference bindings with different scopes:
@Component(
^
@com.squareup.dagger.SingleIn(com.squareup.LoggedInScope.class) class com.squareup.bar.Bar
1 error

Scopes.getReadableSource()
https://github.com/google/dagger/blob/master/java/dagger/internal/codegen/Scopes.java#L75
is currently implemented like so:

/**

• Returns the readable source representation (name with @ prefix) of the scope's annotation type.
  *

• It's readable source because it has had common package prefixes removed, e.g.

• {@code @javax.inject.Singleton} is returned as {@code @Singleton}.
  *

• Does not return any annotation values, since {@link javax.inject.Scope @Scope} annotations

• are not supposed to have any.
  */
  static String getReadableSource(Scope scope) {
  return stripCommonTypePrefixes("@" + scope.scopeAnnotationElement().getQualifiedName());
  }

While the Javax scope annotation javadoc states it should not have
attributes., Dagger doesn't enforce that and it gives us much more
flexibility.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/dagger/issues/1160, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEEEUILtEiLIRyrVbhQxZ6jkHmNVj9Mks5tt6tHgaJpZM4Ttac2
.

[ronshapiro]

You're right that we don't go out of our way to not support it, but we have in fact considered doing so either in ErrorProne or Dagger.

[pyricau]

The JSR 330 doesn't provide the reasoning behind that decision. Dagger always took some liberties with the JSR, not sure why that'd change now. Definitely wouldn't recommend an error prone for that, there are legitimate reasons to rely on attribute annotations for scopes. It's been a great experience to have that feature, it would a step back to add an error prone just because someone didn't envision a clear reason to have attributes on scoping annotations. As it is, attributes are supported, it's effectively part of the Dagger API, so printing scopes annotations should print attributes.

The change would be small in scope, I'm happy to open up a PR. That'd be really beneficial for us, and better than having to maintain a fork again.

[ronshapiro]

@swankjesse @gk5885 do you have insight into _why_ that provision was made in JSR 330?

Either way, this may be one of the things that we may still want to not allow, because it may work in some injectors and not others.

[swankjesse]

We wanted to narrow the number of things the injector had to do. At the time we were imagining just a few scopes: singleton, session, and request.

But this is a compelling use case and I don’t see any reason to read much into the ancient words of JSR-330.

[pyricau]

💃