Repository navigation
[BUG][JAVA] Unused Locale Imports #22313
Description
Activity
Hello @Chrimle, from my POV having unused imports in generated DTOs is not a big deal. It does not break compilation or cause any slow-downs and especially if the imports are from a Java standard library. In my opinion this is not a bug and can be closed without fixing. If you have issues with e.g. linters flagging the files, you can probably exclude the generated files from checking.
I mean - carefully crafting the template files to only conditionally import what is needed can end up really complicating the templates and makes them harder to maintain.
But as always - this is just my opinion and I would like to hear what others think.
Reacted by Jochen Schalanda@Picazsoo Agree that this is a nitpick. But unused imports is a code smell. Also, this project should adhere and conform to
google-java-format, see Style Guide. Although, I am fully aware of MANY violations of this already...Since it's
.java-files that are generated, it's trivial to just runspotlessafter the generation to get the files properly formatted before the build is finished. But then there may as well not be a style guide. Not sure what the purpose of it is, if it's ignored?Any feature/requirement introduces "complications". From a style-guide perspective, it's a bug. But if it's considered not worth the effort, or should not be considered, it may be closed.
I guess the difference is that I never commit generated code into repo. So I almost never see it.
Me neither.
One solution would be to stick to fully qualified names - then there is no need to import. But I don't know how well that plays with google style guide.
Reacted by Christopher MolinIf anyone finds an issue with the formatting they are free to suggest changes to the structure. Given the size of this project and the number of contributions that occur, there will of course be cases where indents and imports are missed. Especially given the number of possible combinations that can occur for all the mustache-templates.
I have myself never worked in a project where the style guide is adhered to all the time if it isn't defined as a commit hook or similar. And given that this is a mustache template, I do not see how it could be automated easily.
I would say that the general expectation is that the client that generates code is expected to conduct their own formatting if they deem it valuable/necessary.
Reacted by Jachym Metlicka@Mattias-Sehlstedt very confusing comment. I will not suggest a change to fix something that isn't deemed a bug. The fact that this issue (among real issues) slips through shows the flaw of not splitting this project up per language. This could easily be fully regression tested. But this is an entirely different discussion.
To reiterate; if it's not a bug, it's not a bug.
This could easily be fully regression tested
If you have an idea for an easy fix then I believe the maintainer would be more than interested to hear about it. To my knowledge their current pipeline mostly check changes by generating defined samples and making sure that they compile.
What I am saying is that you get both benefits and drawbacks with having an open source project, especially if you want to allow people with less experience to contribute. So the "issue" is less that of not having language segmentation, but rather that almost anyone can contribute. But I would assume that that is an explicit choice from the maintainer.
I will not suggest a change to fix something that isn't deemed a bug
It is possible to submit styling suggestions too. I have myself adjusted the indentation in some PRs that I have contributed to this project
Reacted by Christopher MolinLooked at it quickly, and it's not trivial to conditionally import
java.util.Localein themodel.mustache. With #22269 in mind, it might be best to do FQN, otherwise it might be a breaking bug, considering how common it is to have a custom definition ofLocale.Reacted by Andreas Höhmann@Chrimle thanks for referencing my finding in #22269 ... I have the same feeling if I read the code which collects the imports ... almost everywhere there are only simple strings and not full qualified class names. I think this could may avoid such "problems". But yet I don't have enough understanding of the existing code to have a fix in mind or a real proposal. I suggest to extract the whole import-collecting into an own class/service/helper/whatever and replace simple names with fqn's.
Reacted by Christopher MolinGood morning, this issue is currently breaking our builds since 7.16 as we have also "Locale" defined in our models and it's clashing with this unused import.
.../model/Locale.java:18: error: Locale is already defined in this compilation unit import java.util.Locale;Reacted by Christopher MolinGive me the smallest complete example of a breaking open api spec + generator config and I can try to take a look at it.
Bug Report Checklist
Description
Unused import of
Locale. Stems from a change related tosupportUrlQuery, to format strings using Locale. However, the import is most likely not needed except for this use-case.openapi-generator version
Since Version 7.16.0.
OpenAPI declaration file content or url
ANY
type: object, generating a Javaclass.Generation Details
No custom configurations/configOptions.
Steps to reproduce
java.util.Locale.Related issues/PRs
Caused by PR #21871.
Suggest a fix
Wrap the import in
model.mustachewith{{#supportUrlQuery}}.