[BUG][JAVA] Unused `Locale` Imports

[Chrimle]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

Unused import of Locale. Stems from a change related to supportUrlQuery, 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 Java class.

Generation Details

No custom configurations/configOptions.

Steps to reproduce

1. Generate any pojo class.
2. The class will have an unused import of java.util.Locale.

Related issues/PRs

Caused by PR #21871.

Suggest a fix

Wrap the import in model.mustache with {{#supportUrlQuery}}.

[Picazsoo]

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.

[Chrimle]

@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 run spotless after 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.

[Picazsoo]

I guess the difference is that I never commit generated code into repo. So I almost never see it.

[Chrimle]

Me neither.

[Picazsoo]

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.

[Mattias-Sehlstedt]

If 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.

[Chrimle]

@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.

[Mattias-Sehlstedt]

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

[Chrimle]

Looked at it quickly, and it's not trivial to conditionally import java.util.Locale in the model.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 of Locale.

[ahoehma]

@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.

[francisco-revoredo-smec]

Good 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;

[Picazsoo]

Give me the smallest complete example of a breaking open api spec + generator config and I can try to take a look at it.