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

Only define static field constant when inner class collides #6289

Merged
merged 2 commits into from Jun 19, 2020

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 Jun 18, 2020
kares
kares approved these changes Jun 19, 2020
Copy link
Member

@kares kares left a comment

👍 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
1 check passed
@headius headius deleted the prefer_fields_over_inner_classes branch Jun 19, 2020
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