Only define static field constant when inner class collides#6289
Merged
headius merged 2 commits intojruby:jruby-9.2from Jun 19, 2020
Merged
Only define static field constant when inner class collides#6289headius merged 2 commits intojruby:jruby-9.2from
headius merged 2 commits intojruby:jruby-9.2from
Conversation
This commit makes two changes to fix jruby#6196: * When both an inner class and a static final field have the same name, only define the field. Typically this is used for a singleton pattern where the field points at a singleton instance of the inner class, as with Kotlin companion classes. Since we can't define both, we prefer the field. * Do not define the inner class constant as a side effect of initializing that class's proxy, like we do for package constants for normal classes. Defer that constant definition to the initialization of the declaring class, which will make the decision whether to define (or not) the constant at that point. If both the field and the class are needed, it's still possible to import the inner class under a specific name using `java_import`: ```ruby java_import("OuterClass$InnerClass") { "MyWeirdInnerClass" } MyWeirdInnerClass.whatever() ```
Member
Author
|
I will add a trivial mechanical test case using Java code, and I believe @kalenp will try to come up with some actual Kotlin test cases we can incorporate. |
kares
approved these changes
Jun 19, 2020
Member
kares
left a comment
There was a problem hiding this comment.
👍 nice to see a more specific warning for the corner case
Member
Author
|
Note that this PR really just avoids the warning, and does nothing special with the companion classes. We may want to consider a bit more magic, similar to Scala singleton objects where make the object's methods appear to be on the class like they would appear from Scala code. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit makes two changes to fix #6196:
name, only define the field. Typically this is used for a
singleton pattern where the field points at a singleton instance
of the inner class, as with Kotlin companion classes. Since we
can't define both, we prefer the field.
initializing that class's proxy, like we do for package
constants for normal classes. Defer that constant definition to
the initialization of the declaring class, which will make the
decision whether to define (or not) the constant at that point.
If both the field and the class are needed, it's still possible to
import the inner class under a specific name using
java_import: