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

Warning due to missing explicit constructor in generated classes #4573

Open
gtache opened this issue Jan 13, 2025 · 2 comments
Open

Warning due to missing explicit constructor in generated classes #4573

gtache opened this issue Jan 13, 2025 · 2 comments

Comments

@gtache
Copy link

gtache commented Jan 13, 2025

When compiling with all the warnings enabled on JDK16+ while using JPMS, we get a bunch of warnings such as :
[WARNING] /abc/target/generated-sources/annotations/abc/RemoteModule_ProvidesObjectMapperFactory.java:[23,14] class pkg.RemoteModule_ProvidesObjectMapperFactory in exported package pkg declares no explicit constructors, thereby exposing a default constructor to clients of module module
[WARNING] /abc/target/generated-sources/annotations/abc/RemoteTranslator_Factory.java:[21,14] class pkg.RemoteTranslator_Factory in exported package pkg declares no explicit constructors, thereby exposing a default constructor to clients of module module
...

I think the easiest way to fix it would be by adding "missing-explicit-ctor" to the existing SuppressWarnings.

@bcorso
Copy link

bcorso commented Jan 14, 2025

Thanks, looks like the case where we're missing the explicit constructor is here:

https://github.com/google/dagger/blob/master/java/dagger/internal/codegen/writing/FactoryGenerator.java#L146-L152

I think we should just fix this in the code to always generate a constructor, and in fact even the constructors we generate currently are public so we should change them to always be private, so I'll make that change too.

@gtache
Copy link
Author

gtache commented Jan 15, 2025

Indeed, if always adding the constructors is not complicated then it's clearly the best solution. Thank you for looking into this!
I'll close my PR then.

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 a pull request may close this issue.

2 participants