Skip to content

Improve error message when Dagger compiler not run over module - #5245

Open
arimu1 wants to merge 1 commit into
google:masterfrom
arimu1:fix/5146-module-missing-factory-error
Open

Improve error message when Dagger compiler not run over module#5245
arimu1 wants to merge 1 commit into
google:masterfrom
arimu1:fix/5146-module-missing-factory-error

Conversation

@arimu1

@arimu1 arimu1 commented Aug 7, 2026

Copy link
Copy Markdown

Summary

When a @Module lives in a dependency that was compiled without the Dagger annotation processor / KSP plugin, generated component code references missing *Factory types and the user only sees a vague javac "does not exist" error.

This PR detects that situation during component validation and reports a clear diagnostic:

The Dagger factory type lib.LibraryModule_ProvideStringFactory is missing. This usually means the Dagger compiler was not applied to the module lib.LibraryModule where the binding is defined. Ensure that the dependency that contains that module runs the Dagger annotation processor (or KSP plugin).

Approach (aligned with option 1 from #5146, using the existing knownModules set so it works for both Javac and KSP):

  • Modules processed in the current compilation are recorded via ModuleValidator.addKnownModules (as before).
  • For transitive modules on a component that are not from the current compilation, require that each @Provides / @Produces factory exists on the classpath.
  • Skip modules from the current compilation: their factories are generated in-round by ModuleProcessingStep and may not yet be visible via findTypeElement.
  • @Binds-only modules do not require factories and are unaffected.

Fixes #5146

Test plan

When a @module from a dependency is missing its generated *Factory
types (because dagger-compiler/KSP was not applied to that library),
report a clear error instead of letting javac fail with a vague
"does not exist" message on the generated component.

Modules being compiled in the current compilation are skipped, since
their factories are generated in-round by ModuleProcessingStep and may
not yet be visible via findTypeElement.

Fixes google#5146

@bcorso bcorso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Overall, this LGTM.

*/
public boolean isModuleFromCurrentCompilation(XTypeElement module) {
return knownModules.contains(module) || knownModuleNames.contains(module.getQualifiedName());
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to check both knownModules and knowModuleNames?

@arimu1

arimu1 commented Aug 11, 2026

Copy link
Copy Markdown
Author

@bcorso Thanks for the review.

knownModules is the historical Set<XTypeElement> filled by addKnownModules. I added knownModuleNames because in multi-round / multi-step processing the XTypeElement instance for the same module is not always identity-equal to the one stored in knownModules (different rounds can produce distinct element wrappers for the same qualified name). The name set makes “module from current compilation” stable across those wrappers.

If you prefer a single source of truth, I can drop the XTypeElement set for this check and only use qualified names (or the reverse if you know identity is reliable here). Happy to simplify either way.

@bcorso

bcorso commented Aug 11, 2026

Copy link
Copy Markdown

knownModules is the historical Set<XTypeElement> filled by addKnownModules. I added knownModuleNames because in multi-round / multi-step processing the XTypeElement instance for the same module is not always identity-equal to the one stored in knownModules (different rounds can produce distinct element wrappers for the same qualified name). The name set makes “module from current compilation” stable across those wrappers.

Thanks, yeah, I noticed this too while reviewing your change and started a fix to switch over to XTypeElement -> XClassName. Just wanted to make sure there wasn't some other reason you were checking both.

If you prefer a single source of truth, I can drop the XTypeElement set for this check and only use qualified names (or the reverse if you know identity is reliable here). Happy to simplify either way.

Yes, but you can leave it as is since I'll be submitting a separate change to fix the XTypeElement -> XClassName first.

Also, just a heads up, GitHub is not Dagger's source of truth so we can't submit this PR directly. I will need to copy it into Google's repo and then merge the changes into GitHub in a separate PR (though I will attribute the change to you so you get credit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better error message when Dagger compiler not run over module.

2 participants