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

Java generator: Added "java.lang." to all generated instances of type… #1119

Closed
wants to merge 1 commit into from
Closed

Java generator: Added "java.lang." to all generated instances of type… #1119

wants to merge 1 commit into from

Conversation

ne0h
Copy link
Contributor

@ne0h ne0h commented Oct 29, 2016

… java.lang.Object to enable the usage of "Object" as struct name in thrift.

… java.lang.Object to enable the usage of "Object" as struct name in thrift.
@Jens-G
Copy link
Member

Jens-G commented Oct 30, 2016

Hi @ne0h,

could you just create a JIRA ticket for this?

@java devs: Comments anyone?

@ne0h
Copy link
Contributor Author

ne0h commented Oct 30, 2016

Hi Jens-G,

of course: https://issues.apache.org/jira/browse/THRIFT-3954

@bgould
Copy link
Contributor

bgould commented Oct 31, 2016

@Jens-G

IMO this is a valid use case but it is not necessarily a great idea because in general its not very user friendly to design an API that clashes names in java.lang.* ... it is even more complicated in Thrift because other languages may also have constructs called "Object" (javascript for example). There is not a Java-specific technical reason why this shouldn't be allowed however.

That said, there is a related JIRA, not exactly the same problem because java.lang classes do not require import statements, but the idea is the same:

https://issues.apache.org/jira/browse/THRIFT-3301

@jeking3
Copy link
Contributor

jeking3 commented Oct 31, 2016

@bgould Generated code should always be explicitly fully pathed or namespaced and never make any assumptions. Thrift already uses namespaces so I would have no problem with org.king.Object as a thrift structure so long as it derives from java.lang.Object. If the code generator uses "Object" without qualifications that could be ambiguous, and that is a valid defect .. I like the idea of eliminating that. With regards to using a structure named Object, well that's really up to the folks using thrift, it isn't up to the thrift project to tell them how to name their structures and we should stay out of their way where we can.

Copy link
Contributor

@jeking3 jeking3 left a comment

Choose a reason for hiding this comment

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

Generated code should always be fully explicit with namespaces, I like this change.

@asfgit asfgit closed this in 74c99ba Oct 31, 2016
@bgould
Copy link
Contributor

bgould commented Nov 1, 2016

@jeking3

I agree completely. Didn't mean to give the impression that it shouldn't be supported.

bgould pushed a commit to bgould/thrift that referenced this pull request Dec 2, 2016
Client: Java
Patch: Maximilian Hess <mail@ne0h.de>

This closes apache#1119
gadLinux pushed a commit to gadLinux/thrift that referenced this pull request Jan 24, 2017
Client: Java
Patch: Maximilian Hess <mail@ne0h.de>

This closes apache#1119
ngrewe pushed a commit to ngrewe/thrift that referenced this pull request Jan 31, 2017
Client: Java
Patch: Maximilian Hess <mail@ne0h.de>

This closes apache#1119
ngrewe pushed a commit to ngrewe/thrift that referenced this pull request Jan 31, 2017
Client: Java
Patch: Maximilian Hess <mail@ne0h.de>

This closes apache#1119
jeking3 pushed a commit to jeking3/thrift that referenced this pull request Nov 30, 2017
Client: Java
Patch: Maximilian Hess <mail@ne0h.de>

This closes apache#1119
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants