Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experimental error message feedback #1769

Open
Chang-Eric opened this issue Mar 13, 2020 · 6 comments
Open

Experimental error message feedback #1769

Chang-Eric opened this issue Mar 13, 2020 · 6 comments

Comments

@Chang-Eric
Copy link
Member

This is a tracking issue for feedback on the experimental error messages.

When using this option, the format and some content of error messages will be changed in order to improve readability. Pending feedback, this format will eventually become the default and replace current error messages.

To opt-in to the new format use -Adagger.experimentalDaggerErrorMessages=enabled after version 2.27.

Currently, this flag:

  • Shortens all class names to the simple class name. A legend is output at the bottom of the errors to map short names to fully qualified names.
  • Reports errors as a single error for each root component.
  • Adds color to error tags to visibly separate individual errors
  • Reduces some extraneous information from some messages.
@Chang-Eric Chang-Eric self-assigned this Mar 13, 2020
@vRallev
Copy link

vRallev commented Mar 16, 2020

We're testing the new error messages.

[Dagger/MissingBinding] FeatureProvider cannot be provided without an @Provides-annotated method.
      FeatureProvider is provided at
          DevelopmentLoginScreenComponent.featureProvider() [DemoLegacyAppComponent → DemoLegacyLoggedInComponent → DemoLegacyMainActivityComponent]

  ======================
  Full classname legend:
  ======================
  DemoLegacyAppComponent:          com.squareup.development.shell.demo.legacy.DemoLegacyAppComponent
  DemoLegacyLoggedInComponent:     com.squareup.development.shell.demo.legacy.DemoLegacyLoggedInComponent
  DemoLegacyMainActivityComponent: com.squareup.development.shell.demo.legacy.DemoLegacyMainActivityComponent
  DevelopmentLoginScreenComponent: com.squareup.development.shell.login.screen.DevelopmentLoginScreenComponent
  FeatureProvider:                 com.squareup.development.shell.login.screen.FeatureProvider

While correct the wording is very confusing. It says: FeatureProvider cannot be provided without... and FeatureProvider is provided at on the next line. Maybe change the 2nd line to FeatureProvider is required for / at.

@Chang-Eric
Copy link
Member Author

Thanks for the feedback. I had noticed that before but forgot to submit that change. Funnily enough that wording is actually in the existing error messages, but I think it gets lost in the noise. I will likely change that wording to "requested at" or "injected at" which hopefully will make more sense.

@vRallev
Copy link

vRallev commented Mar 16, 2020

I was aware that the confusing wording already exists. But I thought now is a good time to report it since it's more prominent. Both "requested at" and "injected at" make sense. I'd prefer the latter, because usually you "@Inject" something. For components providing dependencies "requested at" would make more sense, but that's not the norm.

@tomislavhofman
Copy link

Excuse my ignorance, where do we put -Adagger.experimentalDaggerErrorMessages=enabled

@dmapr
Copy link

dmapr commented Mar 17, 2020

@tomislavhofman it depends on your build system. For Gradle I used this

 javaCompileOptions {
     annotationProcessorOptions {
          arguments = [
               "dagger.experimentalDaggerErrorMessages" : "enabled"
          ]
     }
 }

@JajaComp
Copy link

JajaComp commented Mar 17, 2020

@tomislavhofman it depends on your build system. For Gradle I used this

 javaCompileOptions {
     annotationProcessorOptions {
          arguments = [
               "dagger.experimentalDaggerErrorMessages" : "enabled"
          ]
     }
 }

in android -> defaultConfig section

or

kapt {
        arguments {
            arg("dagger.experimentalDaggerErrorMessages", "enabled")
        }
    }

in android section

nick-someone pushed a commit that referenced this issue Apr 1, 2020
…instead of "provided at" or "produced at". This should better indicate that it is a request for the type.

Issue #1769

RELNOTES=Minor fixes to error messages

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=304057073
nick-someone pushed a commit that referenced this issue Apr 1, 2020
…instead of "provided at" or "produced at". This should better indicate that it is a request for the type.

Issue #1769

RELNOTES=Minor fixes to error messages

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=304057073
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants