[Java][clients] remove java.lang prefix from Object

[nielspardon]

Currently the Java generators generate models with equals and toIndentedString methods that use a fully qualified classname java.lang.Object as the parameter type to compare the object to or generate the indented string for.

I would like to introduce a flag to allow disabling the generation of fully qualified classnames since:

• The PMD source code analyzer flags such an unnecessary use of fully qualified classname by default.
• Other equals method generators like the Eclipse generator generate the equals method with the short classname Object by default.

I'm introducing a new additional property called fullyQualifiedClassnames which is set to true by default to keep the current behavior while allowing to optionally disable the generation of fully qualified classnames for POJOs.

Since I'm not sure whether introducing such a flag is permissible for a minor / patch release I'm targeting this PR at master for now and look forward to your feedback.

@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @bkabrda

PR checklist

• Read the contribution guidelines.
• If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
• Run the shell script ./bin/generate-samples.shto update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/config/java*. For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master
• Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

[nielspardon]

Would appreciate it if anyone of the Java technical committee could take a look and at least give some feedback. Thanks!

@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @bkabrda

[jeff9finger]

This seems reasonable. But if this goes into a major version, I think the fully qualified java.lang should be removed by default. And perhaps and option or some other way to preserve the old behavior until it can be removed.

Is there a way to do this without adding another option?

[nielspardon]

This seems reasonable. But if this goes into a major version, I think the fully qualified java.lang should be removed by default. And perhaps and option or some other way to preserve the old behavior until it can be removed.

I agree it probably would make sense to transition to not using the fully qualified class name by default. This could be achieved by changing the default value of the new option to achieve this or by simply not having an option at all and always using the short class name.

I guess the only situation where this could cause a problem would be if someone would have a schema and/or a parent schema with the name Object.

Is there a way to do this without adding another option?

Actually my knowledge of the generator is a bit too limited to make a sound judgement on which other option we could attach it to. I'm still giving it a try though: Looking through the available options the closest match is probably going to be java8 although not requiring the fully qualified class name for is not strictly a Java 8 feature. It seems previously options like legacyDiscriminatorBehavior were introduced. So we could also rename the option to legacyFullyQualifiedLangObjectBehavior to follow this pattern.

Tell me which option you prefer or if you have a better suggestion and I'm happy to rework the PR.

[jeff9finger]

legacyFullyQualifiedLangObjectBehavior or maybe even legacyFullyQualifiedJavaLangObjectBehavior to be very explicit on the meaning, seems reasonable for the java generators.

Are there other opinions by the Java technical committee?

[nielspardon]

legacyFullyQualifiedLangObjectBehavior or maybe even legacyFullyQualifiedJavaLangObjectBehavior to be very explicit on the meaning, seems reasonable for the java generators.

I renamed it to the extra verbose legacyFullyQualifiedJavaLangObjectBehavior and set the default value to false so the legacy behavior has to be enabled and is disabled by default.

Are there other opinions by the Java technical committee?

Seems you're the only member with an opinion on this PR or everybody else is on vacation :-)

[nielspardon]

And pushed another version rebased on the latest master just in case.

[jeff9finger]

LGTM 👍 But I think we should have at least one other member of the committee approve the new option.

[nielspardon]

LGTM 👍 But I think we should have at least one other member of the committee approve the new option.

Sure, makes sense to me. Any volunteers?

@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @karismann @Zomzog @lwlee2608 @bkabrda

[nielspardon]

@jeff9finger are you able to or know someone who is able to re-trigger the Circle CI build? seems it failed with some cache restoration issue which I believe has nothing to do with my code.

Also: What happens when nobody else from the Java tech committee reacts to this PR? Any other means of reaching someone?

[nielspardon]

Hey guys, anyone available for a second review / approval?

@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @karismann @Zomzog @lwlee2608 @bkabrda

[bkabrda]

LGTM 👍

[nielspardon]

Thanks! So what is missing to get this merged? :-)

[cbornet]

Can we avoid adding yet another option ? I'd say put Object as a reserved word so it is "escaped" and just remove java.lang. Next version will be a major so no need to keep compatibility.

[nielspardon]

Can we avoid adding yet another option ? I'd say put Object as a reserved word so it is "escaped" and just remove java.lang. Next version will be a major so no need to keep compatibility.

That's actually a very good point. Seems object is already a reserved word in the AbstractJavaCodeGen so we could just drop the java.lang. prefix and wouldn't need the option.

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java

Lines 136 to 139 in 1bfd86a

	setReservedWordsLowerCase(
	Arrays.asList(
	// special words
	"object",

[nielspardon]

I updated the PR to only remove the java.lang. prefix from the Java / JAX-RS mustache files and regenerated the samples. There is no new option now.

As before, to make it easier for you to review I separated the functional changes from the generated changes by using two commits.

[nielspardon]

If Bitrise wouldn't fail installing Maven and Shippable would become sick of waiting we would probably have a green build.

[nielspardon]

Everything green. Would appreciate it if you guys could take another look and decide whether it can be merged now.

Thanks!

@cbornet @jeff9finger @bkabrda

[nielspardon]

Anyone there who can finally merge this PR? :-)

Thank you!

@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @bkabrda

[nielspardon]

Any chance you can spare 5 minutes and get this merged?

Thank you!

@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @bkabrda

[nielspardon]

Another week. Another reminder.

Any chance you can spare 5 minutes and get this merged?

@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @bkabrda

Also copying the core team in case they can help out:
@wing328 @jimschubert @ackintosh @jmini @etherealjoy @spacether

Thank you!

[nielspardon]

Are you guys well? I'm starting to get concerned for you.

@bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @bkabrda

Also copying the core team in case they can help out:
@wing328 @jimschubert @ackintosh @jmini @etherealjoy @spacether

Thank you!

[jimschubert]

@nielspardon please stop bumping this pull request and be patient.

[nielspardon]

@nielspardon please stop bumping this pull request and be patient.

@jimschubert Thanks for taking the time to write a response. As you can see from the history of this PR I've usually waited between 4 and 7 days before reminding people. If you think this comes across as impatient, then what timelines should I expect as "normal"?

[jimschubert]

@nielspardon we don't have timelines. If you feel that you've addressed @cbornet concerns, which I agree with, then get his feedback on your changes.

I suspect everyone works "bottom up" as I do. So, by bumping the thread you're making it look like there's new activity and discussion which may be doing the complete opposite of what you're trying to accomplish. If I was the reviewer, every time you bump puts the PR last in line unless it was a response to a discussion.

[nielspardon]

Thanks for the explanation.

@nielspardon we don't have timelines. If you feel that you've addressed @cbornet concerns, which I agree with, then get his feedback on your changes.

Which is what I did 24 days ago through this PR. Are there other means by which I could have/should have reached out to them?

I suspect everyone works "bottom up" as I do. So, by bumping the thread you're making it look like there's new activity and discussion which may be doing the complete opposite of what you're trying to accomplish. If I was the reviewer, every time you bump puts the PR last in line unless it was a response to a discussion.

I do fully understand that people might have other tasks and priorities in their lives. As the new guy let me tell you this whole experience was not very motivating so far. I've tried my best to follow the guidelines, provide a small, easy to review PR, implement requested changes, ensure the build stays green, etc. In a nutshell I've also put in a lot of effort to make your lives simpler so you don't have to work bottoms up all the time.

Getting no response for weeks although I've been actually here to try to help you guys is pretty demotivating. Somehow I would expect that the review team would be a bit more responsive at least indicating whether my PR is actually on their list. Currently, there's no visibility for someone like me and all I can do is wait, guess and bump or give up.

[jimschubert]

@nielspardon don't get me wrong, we do appreciate the contribution. Like I said, please stop bumping it and it'll get merged.

For some perspective we have 1700 issues and over 200 pull requests, 150 of which were submitted before yours. Again if others work bottom up like I do, you're unfortunately bumping yourself to the end of the list every time.

For some context, if your PR is merged ahead of 150 other (potentially more complex) pull requests, it may require more work to fix merge conflicts. That work is picked up usually by the core team. As an example, some newer work was merged out of order from other work and I spent around 15 hours trying to resolve merge conflicts in that older work. I eventually had to request the contributor make the fixes.

You've gotten LGTM, nobody has requested more from you. Let us get in the bug fixes and features we're pushing to get into 5.0 and yours will follow. Don't worry.

[jimschubert]

LGTM

Generally we use fully qualified names to avoid conflicts with schema names, but Object is so generic it's unlikely to be used as a schema name. We also declare object as a reserved word, so it would be escaped anyway regardless of the full qualification of the type.

For edge cases where users prefer the fully qualified java.lang.Object, they'll need to customize the templates as needed.

This pull request implementation serves as a good example of providing templates which result in quality code using best practices (linters, static analysis).

[jimschubert]

Getting no response for weeks although I've been actually here to try to help you guys is pretty demotivating. Somehow I would expect that the review team would be a bit more responsive at least indicating whether my PR is actually on their list. Currently, there's no visibility for someone like me and all I can do is wait, guess and bump or give up.

@nielspardon my apologies for any lack of responses. I was personally on hiatus for the entire month of July and into August to celebrate my 40th birthday and focus on family during some hectic months at work. I can only assume that others might be dealing with similar hectic circumstances as the summer months seems to take away from people's free time for open source contributions.

I'll discuss tagging pull requests with a milestone so contributors can have a good idea of when we target the merge. I appreciate your determination to have the pull request addressed, and your candid feedback is very helpful for us to better our practices and interactions.

[jimschubert]

I've opened #7290 to drive some discussion around automatically labeling pull requests for first-time contributors, and assigning someone from the core team as a reviewer or assignee for awareness.