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
Suppress useless companion for Kotlin-As-Java #2681
Conversation
* hide companion object if it contains only const/lateinit/@JvmField properties or @JvmStatic functions * lift the declarations to outer class from companion * mark lifted declarations as static * mark const/JvmField/lateinit properties as static for named objects
* remove getter/setter for lifted from companion properties and named objects * fix visibility modifiers for lifter properties * fix always public top level constants
If a companion object rendered for the class then the static property for accessing the object is shown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've finished with the first review round, nice job 👍
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinCompanion.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinCompanion.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinCompanion.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinCompanion.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinCompanion.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinCompanion.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinToJavaConverter.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done! Left some minor comments to discuss
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinToJavaConverter.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinToJavaConverter.kt
Outdated
Show resolved
Hide resolved
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinToJavaConverter.kt
Show resolved
Hide resolved
plugins/kotlin-as-java/src/main/kotlin/converters/KotlinCompanion.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some minor suggestion, the rest LGTM for the most part 👍
const
/@JvmField
/lateinit
properties or@JvmStatic
functionsconst
/@JvmField
/lateinit
properties as static for named objectsconst
/lateinit
properties in named objects/companionsFix #200, fix #2086