Skip to content

Only define static field constant when inner class collides#6289

Merged
headius merged 2 commits intojruby:jruby-9.2from
headius:prefer_fields_over_inner_classes
Jun 19, 2020
Merged

Only define static field constant when inner class collides#6289
headius merged 2 commits intojruby:jruby-9.2from
headius:prefer_fields_over_inner_classes

Conversation

@headius
Copy link
Member

@headius headius commented Jun 18, 2020

This commit makes two changes to fix #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:

java_import("OuterClass$InnerClass") { "MyWeirdInnerClass" }
MyWeirdInnerClass.whatever()

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()
```
@headius
Copy link
Member Author

headius commented Jun 18, 2020

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.

@headius headius requested review from enebo and kares June 18, 2020 22:45
Copy link
Member

@kares kares left a comment

Choose a reason for hiding this comment

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

👍 nice to see a more specific warning for the corner case

@headius
Copy link
Member Author

headius commented Jun 19, 2020

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.

@headius headius merged commit 48f444c into jruby:jruby-9.2 Jun 19, 2020
@headius headius deleted the prefer_fields_over_inner_classes branch June 19, 2020 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"warning: already initialized constant Companion" with Kotlin companion objects

2 participants