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

Extension functions and properties defined for base classes and interfaces #1908

Closed
msink opened this issue May 8, 2021 · 11 comments · Fixed by #2625
Closed

Extension functions and properties defined for base classes and interfaces #1908

msink opened this issue May 8, 2021 · 11 comments · Fixed by #2625
Labels
enhancement An issue for a feature or an overall improvement feedback: Kotlin libs Feedback from Kotlin's internal libraries

Comments

@msink
Copy link
Contributor

msink commented May 8, 2021

Describe the bug

Extension functions and properties defined for base classes and interfaces - are lost.
For example (a bit simplified):

/** Container for child controls. */
interface Container {
    fun <T : Control<*>> add(widget: T): T
}

/** Wrapper class for [uiBox] - a container that stack its children horizontally or vertically. */
abstract class Box(alloc: CPointer<uiBox>?) : Control<uiBox>(alloc), Container {
    /** ... */
}

/** DSL builder for a simple button. */
inline fun Container.button(
    text: String,
    init: Button.() -> Unit = {}
): Button = add(Button(text).apply(init))

/** DSL builder for a checkbox widget. */
inline fun Container.checkbox(
    label: String,
    init: Checkbox.() -> Unit = {}
): Checkbox = add(Checkbox(label).apply(init))

/** .... */

Diff for what was generated by Dokka 0.9.17 and Dokka 1.4.32:

 # Box
 
-`abstract class Box : `[`Control`](../-control/README.md)`<`[`uiBox`](../../libui/ui-box.md)`>, `[`Container`](../-container/README.md)
+`abstract class `[`Box`](README.md)`(alloc: CPointer<uiBox>?) : `[`Control`](../-control/README.md)`<uiBox> , `[`Container`](../-container/README.md)
+
+Wrapper class for uiBox - a container that stack its children horizontally or vertically.
 
-Wrapper class for [uiBox](../../libui/ui-box.md) - a container that stack its children horizontally or vertically.
 
 ### Constructors
 
-| Name | Summary |
+| | |
 |---|---|
-| [Box](-box.md) | `Box(alloc: CPointer<`[`uiBox`](../../libui/ui-box.md)`>?)`<br>Wrapper class for [uiBox](../../libui/ui-box.md) - a container that stack its children horizontally or vertically. |
+| [Box](-box.md) | `fun `[`Box`](-box.md)`(alloc: CPointer<uiBox>?)` |
 
 ### Properties
 
 | Name | Summary |
 |---|---|
-| [padded](padded.md) | `var padded: Boolean`<br>If `true`, the container insert some space between children. |
-| [stretchy](stretchy.md) | `var stretchy: Boolean`<br>Next added child should expand to use all available size. |
+| [padded](padded.md) | `var `[`padded`](padded.md)`: Boolean`<br>If `true`, the container insert some space between children. |
+| [stretchy](stretchy.md) | `var `[`stretchy`](stretchy.md)`: Boolean = false`<br>Next added child should expand to use all available size. |
 
 ### Inherited properties
 
 | Name | Summary |
 |---|---|
-| [enabled](../-control/enabled.md) | `var enabled: Boolean`<br>Whether the Control should be enabled or disabled. Defaults to `true`. |
-| [parent](../-control/parent.md) | `var parent: `[`Control`](../-control/README.md)`<*>?`<br>Returns parent of the control or `null` for detached. |
-| [toplevel](../-control/toplevel.md) | `val toplevel: Boolean`<br>Returns whether the control is a top level one or not. |
-| [visible](../-control/visible.md) | `var visible: Boolean`<br>Whether the Control should be visible or hidden. Defaults to `true`. |
+| [disposed](README.md#115031071%2FProperties%2F123127132) | `val `[`disposed`](README.md#115031071%2FProperties%2F123127132)`: Boolean`<br>Returns `true` if object was disposed - in this case [dispose](../-disposable/dispose.md) will do nothing, all other operations are invalid and will `throw Error("Resource is disposed")`. |
+| [enabled](README.md#-2142328875%2FProperties%2F123127132) | `var `[`enabled`](README.md#-2142328875%2FProperties%2F123127132)`: Boolean`<br>Whether the Control should be enabled or disabled. |
+| [parent](README.md#-27209990%2FProperties%2F123127132) | `var `[`parent`](README.md#-27209990%2FProperties%2F123127132)`: `[`Control`](../-control/README.md)`<*>?`<br>Returns parent of the control or `null` for detached. |
+| [toplevel](README.md#405419573%2FProperties%2F123127132) | `val `[`toplevel`](README.md#405419573%2FProperties%2F123127132)`: Boolean`<br>Returns whether the control is a top level one or not. |
+| [visible](README.md#2066795460%2FProperties%2F123127132) | `var `[`visible`](README.md#2066795460%2FProperties%2F123127132)`: Boolean`<br>Whether the Control should be visible or hidden. |
 
 ### Functions
 
 | Name | Summary |
 |---|---|
-| [add](add.md) | `open fun <T : `[`Control`](../-control/README.md)`<*>> add(widget: `[`T`](add.md#T)`): `[`T`](add.md#T)<br>Adds the given widget to the end of the Box. |
-| [delete](delete.md) | `fun delete(index: Int): Unit`<br>Deletes the nth control of the Box. |
+| [add](add.md) | `open override fun <`[`T`](add.md)` : `[`Control`](../-control/README.md)`<*>> `[`add`](add.md)`(widget: `[`T`](add.md)`): `[`T`](add.md)<br>Adds the given widget to the end of the Box. |
+| [delete](delete.md) | `fun `[`delete`](delete.md)`(index: Int)`<br>Deletes the nth control of the Box. |
 
 ### Inherited functions
 
 | Name | Summary |
 |---|---|
-| [disable](../-control/disable.md) | `fun disable(): Unit`<br>Disables the Control. |
-| [dispose](../-control/dispose.md) | `open fun dispose(): Unit`<br>Dispose and free all allocated resources. |
-| [enable](../-control/enable.md) | `fun enable(): Unit`<br>Enables the Control. |
-| [getHandle](../-control/get-handle.md) | `fun getHandle(): ULong`<br>Returns the OS-level handle associated with this Control. |
-| [hide](../-control/hide.md) | `fun hide(): Unit`<br>Hides the Control. Hidden controls do not participate in layout (that is, Box, GridPane, etc. does not reserve space for hidden controls). |
-| [isEnabled](../-control/is-enabled.md) | `fun isEnabled(): Boolean`<br>Whether the Control is enabled. |
-| [isEnabledToUser](../-control/is-enabled-to-user.md) | `fun isEnabledToUser(): Boolean`<br>Whether the Control and all parents are enabled. |
-| [isVisible](../-control/is-visible.md) | `fun isVisible(): Boolean`<br>Whether the Control is visible. |
-| [show](../-control/show.md) | `fun show(): Unit`<br>Shows the Control. |
+| [disable](../-control/disable.md) | `fun `[`disable`](../-control/disable.md)`()`<br>Disables the Control. |
+| [dispose](../-control/dispose.md) | `open override fun `[`dispose`](../-control/dispose.md)`()`<br>Dispose and free all allocated resources. |
+| [enable](../-control/enable.md) | `fun `[`enable`](../-control/enable.md)`()`<br>Enables the Control. |
+| [getHandle](../-control/get-handle.md) | `fun `[`getHandle`](../-control/get-handle.md)`(): ULong`<br>Returns the OS-level handle associated with this Control. |
+| [hide](../-control/hide.md) | `fun `[`hide`](../-control/hide.md)`()`<br>Hides the Control. |
+| [isEnabled](../-control/is-enabled.md) | `fun `[`isEnabled`](../-control/is-enabled.md)`(): Boolean`<br>Whether the Control is enabled. |
+| [isEnabledToUser](../-control/is-enabled-to-user.md) | `fun `[`isEnabledToUser`](../-control/is-enabled-to-user.md)`(): Boolean`<br>Whether the Control and all parents are enabled. |
+| [isVisible](../-control/is-visible.md) | `fun `[`isVisible`](../-control/is-visible.md)`(): Boolean`<br>Whether the Control is visible. |
+| [show](../-control/show.md) | `fun `[`show`](../-control/show.md)`()`<br>Shows the Control. |
-
-### Extension properties
-
-| Name | Summary |
-|---|---|
-| [hbox](../hbox.md) | `val `[`Container`](../-container/README.md)`.hbox: `[`HBox`](../-h-box/README.md)<br>DSL builder for a container that stack its children horizontally. |
-| [vbox](../vbox.md) | `val `[`Container`](../-container/README.md)`.vbox: `[`VBox`](../-v-box/README.md)<br>DSL builder for a container that stack its children vertically. |
-
-### Extension functions
-
-| Name | Summary |
-|---|---|
-| [button](../button.md) | `fun `[`Container`](../-container/README.md)`.button(text: String, init: `[`Button`](../-button/README.md)`.() -> Unit = {}): `[`Button`](../-button/README.md)<br>DSL builder for a simple button. |
-| [checkbox](../checkbox.md) | `fun `[`Container`](../-container/README.md)`.checkbox(label: String, init: `[`Checkbox`](../-checkbox/README.md)`.() -> Unit = {}): `[`Checkbox`](../-checkbox/README.md)<br>DSL builder for a checkbox widget. |
-| [colorbutton](../colorbutton.md) | `fun `[`Container`](../-container/README.md)`.colorbutton(init: `[`ColorButton`](../-color-button/README.md)`.() -> Unit = {}): `[`ColorButton`](../-color-button/README.md)<br>DSL builder for a button that opens a color palette popup. |
-| [combobox](../combobox.md) | `fun `[`Container`](../-container/README.md)`.combobox(init: `[`Combobox`](../-combobox/README.md)`.() -> Unit = {}): `[`Combobox`](../-combobox/README.md)<br>DSL builder for a drop down combo box that allow list selection only. |
-| [datepicker](../datepicker.md) | `fun `[`Container`](../-container/README.md)`.datepicker(init: `[`DatePicker`](../-date-picker/README.md)`.() -> Unit = {}): `[`DatePicker`](../-date-picker/README.md)<br>DSL builder for a widget to edit date. |
-| [datetimepicker](../datetimepicker.md) | `fun `[`Container`](../-container/README.md)`.datetimepicker(init: `[`DateTimePicker`](../-date-time-picker/README.md)`.() -> Unit = {}): `[`DateTimePicker`](../-date-time-picker/README.md)<br>DSL builder for a widget to edit date and time. |
-| [drawarea](../drawarea.md) | `fun `[`Container`](../-container/README.md)`.drawarea(init: `[`DrawArea`](../-draw-area/README.md)`.() -> Unit = {}): `[`DrawArea`](../-draw-area/README.md)<br>DSL builder for a canvas you can draw on. It also receives keyboard and mouse events, is DPI aware, and has several other useful features. |
-| [editablecombobox](../editablecombobox.md) | `fun `[`Container`](../-container/README.md)`.editablecombobox(init: `[`EditableCombobox`](../-editable-combobox/README.md)`.() -> Unit = {}): `[`EditableCombobox`](../-editable-combobox/README.md)<br>DSL builder for a drop down combo box that allow selection from list or free text entry. |
-| [fontbutton](../fontbutton.md) | `fun `[`Container`](../-container/README.md)`.fontbutton(init: `[`FontButton`](../-font-button/README.md)`.() -> Unit = {}): `[`FontButton`](../-font-button/README.md)<br>DSL builder for a button that allows users to choose a font when they click on it. |
-| [form](../form.md) | `fun `[`Container`](../-container/README.md)`.form(padded: Boolean = true, init: `[`Form`](../-form/README.md)`.() -> Unit = {}): `[`Form`](../-form/README.md)<br>DSL builder for a container that organize children as labeled fields. |
-| [gridpane](../gridpane.md) | `fun `[`Container`](../-container/README.md)`.gridpane(padded: Boolean = true, init: `[`GridPane`](../-grid-pane/README.md)`.() -> Unit = {}): `[`GridPane`](../-grid-pane/README.md)<br>DSL builder for a powerful container that allow to specify size and position of each children. |
-| [group](../group.md) | `fun `[`Container`](../-container/README.md)`.group(title: String, margined: Boolean = true, init: `[`Group`](../-group/README.md)`.() -> Unit = {}): `[`Group`](../-group/README.md)<br>DSL builder for a container for a single widget that provide a caption and visually group it's children. |
-| [hbox](../hbox.md) | `fun `[`Container`](../-container/README.md)`.hbox(padded: Boolean = true, init: `[`HBox`](../-h-box/README.md)`.() -> Unit = {}): `[`HBox`](../-h-box/README.md)<br>DSL builder for a container that stack its children horizontally. |
-| [label](../label.md) | `fun `[`Container`](../-container/README.md)`.label(text: String, init: `[`Label`](../-label/README.md)`.() -> Unit = {}): `[`Label`](../-label/README.md)<br>DSL builder for a static text label. |
-| [passwordfield](../passwordfield.md) | `fun `[`Container`](../-container/README.md)`.passwordfield(readonly: Boolean = false, init: `[`PasswordField`](../-password-field/README.md)`.() -> Unit = {}): `[`PasswordField`](../-password-field/README.md)<br>DSL builder for a text entry widget that mask the input, useful to edit passwords or other sensible data. |
-| [progressbar](../progressbar.md) | `fun `[`Container`](../-container/README.md)`.progressbar(init: `[`ProgressBar`](../-progress-bar/README.md)`.() -> Unit = {}): `[`ProgressBar`](../-progress-bar/README.md)<br>DSL builder for a progress bar widget. |
-| [radiobuttons](../radiobuttons.md) | `fun `[`Container`](../-container/README.md)`.radiobuttons(init: `[`RadioButtons`](../-radio-buttons/README.md)`.() -> Unit = {}): `[`RadioButtons`](../-radio-buttons/README.md)<br>DSL builder for a widget that represent a group of radio options. |
-| [scrollingarea](../scrollingarea.md) | `fun `[`Container`](../-container/README.md)`.scrollingarea(width: Int, height: Int, init: `[`ScrollingArea`](../-scrolling-area/README.md)`.() -> Unit = {}): `[`ScrollingArea`](../-scrolling-area/README.md)<br>DSL builder for a canvas with horziontal and vertical scrollbars. |
-| [searchfield](../searchfield.md) | `fun `[`Container`](../-container/README.md)`.searchfield(readonly: Boolean = false, init: `[`SearchField`](../-search-field/README.md)`.() -> Unit = {}): `[`SearchField`](../-search-field/README.md)<br>DSL builder for a text entry widget to search text. |
-| [slider](../slider.md) | `fun `[`Container`](../-container/README.md)`.slider(min: Int, max: Int, init: `[`Slider`](../-slider/README.md)`.() -> Unit = {}): `[`Slider`](../-slider/README.md)<br>DSL builder for an horizontal slide to set numerical values. |
-| [spinbox](../spinbox.md) | `fun `[`Container`](../-container/README.md)`.spinbox(min: Int, max: Int, init: `[`Spinbox`](../-spinbox/README.md)`.() -> Unit = {}): `[`Spinbox`](../-spinbox/README.md)<br>DSL builder for an entry widget for numerical values. |
-| [tableview](../tableview.md) | `fun <T> `[`Container`](../-container/README.md)`.tableview(data: List<`[`T`](../tableview.md#T)`>, init: `[`Table`](../-table/README.md)`<`[`T`](../tableview.md#T)`>.() -> Unit = {}): `[`TableView`](../-table-view/README.md)<br>DSL builder to visualize data in a tabular form. |
-| [tabpane](../tabpane.md) | `fun `[`Container`](../-container/README.md)`.tabpane(init: `[`TabPane`](../-tab-pane/README.md)`.() -> Unit = {}): `[`TabPane`](../-tab-pane/README.md)<br>DSL builder for a container that show each children in a separate tab. |
-| [textarea](../textarea.md) | `fun `[`Container`](../-container/README.md)`.textarea(wrap: Boolean = true, init: `[`TextArea`](../-text-area/README.md)`.() -> Unit = {}): `[`TextArea`](../-text-area/README.md)<br>DSL builder for a multiline plain text editing widget. |
-| [textfield](../textfield.md) | `fun `[`Container`](../-container/README.md)`.textfield(readonly: Boolean = false, init: `[`TextField`](../-text-field/README.md)`.() -> Unit = {}): `[`TextField`](../-text-field/README.md)<br>DSL builder for a simple single line text entry widget. |
-| [timepicker](../timepicker.md) | `fun `[`Container`](../-container/README.md)`.timepicker(init: `[`TimePicker`](../-time-picker/README.md)`.() -> Unit = {}): `[`TimePicker`](../-time-picker/README.md)<br>DSL builder for a widget to edit time. |
-| [vbox](../vbox.md) | `fun `[`Container`](../-container/README.md)`.vbox(padded: Boolean = true, init: `[`VBox`](../-v-box/README.md)`.() -> Unit = {}): `[`VBox`](../-v-box/README.md)<br>DSL builder for a container that stack its children vertically. |
 
 ### Inheritors
 
-| Name | Summary |
-|---|---|
-| [HBox](../-h-box/README.md) | `class HBox : `[`Box`](README.md)<br>Wrapper class for [uiBox](../../libui/ui-box.md) - a container that stack its children horizontally. |
-| [VBox](../-v-box/README.md) | `class VBox : `[`Box`](README.md)<br>Wrapper class for [uiBox](../../libui/ui-box.md) - a container that stack its children vertically. |
+| Name |
+|---|
+| [HBox](../-h-box/README.md) |
+| [VBox](../-v-box/README.md) |
+

Expected behaviour
Not sure, but maybe add new sections "Inherited extension functions" and "Inherited extension properties"
But anyway - not just lose it.

To Reproduce
https://github.com/msink/kotlin-libui/tree/dokka

@msink msink added the bug label May 8, 2021
@MarcinAman
Copy link
Contributor

I have some troubles reproducing this issue. On provided repository i run dokkaGfm and when i go to libui.ktx/index.md i can see extension functions under the functions header. For example the button extension function is present:

image

@msink
Copy link
Contributor Author

msink commented May 11, 2021

Diff here is between https://github.com/msink/kotlin-libui/blob/dokka/docs/libui.ktx/-box/README.md - generated years ago by Dokka 0.9.17, and https://github.com/msink/kotlin-libui/blob/dokka/dokka/libui.ktx/-box/README.md - generated by task ./gradlew dokkaMyGfm

MyGfm is modified version of Gfm renderer, in tools directory.

@MarcinAman
Copy link
Contributor

Yes, but do you mean that we no longer display extension functions and properties on the receiver's page ?
I don't think they are lost, they just got relocated

@msink
Copy link
Contributor Author

msink commented May 17, 2021

I see that extension functions now are displayed only directly for class/interface for what they are defined.
On pages of classes that implement that interface - they are no longer displayed.

@msink
Copy link
Contributor Author

msink commented May 19, 2021

Looks as suppressInheritedMembers by default is set to false for normal functions/properties, and is always true for extension functions.
IMHO they should be consistent.
Or maybe add new suppressInheritedExtensions switch...

@MarcinAman
Copy link
Contributor

No, they are inherited. In the following example:

open class Parent {
    fun String.parentExtensionFun(){

    }
}

class Children : Parent(){

}

Documentation generated for Children contains parentExtensionFun if suppressInheritedMembers is set to false. Otherwise it is hidden as it should be

@msink
Copy link
Contributor Author

msink commented May 20, 2021

Don't know, as was said in first message, in my use case:

interface Parent {
}

class Children(): Parent {
}

fun Parent.parentExtensionFun() {
}

documentation generated for Children does not contain parentExtensionFun.
suppressInheritedMembers is set to false.

@MarcinAman
Copy link
Contributor

Ok, i get the reasoning now. To be fair this is quite an tough topic that i don't have a solution for. In newer dokka we decided to have extensions rendered where they are declared and only add those to the classes that they are for. We didn't include any inheritance there simply because it is quite impossible to draw a line what you should and shouldn't render (extensions on Any, how do you view all of those functions, effective searching, extensions in different modules, visibilites, UX on gfm/html etc).

I'll convert this to an enhancement since current behaviour is an expected one but will try to at least come up with a solution on how to tackle it as it might be needed for stable release

@MarcinAman MarcinAman added enhancement An issue for a feature or an overall improvement and removed bug labels May 24, 2021
@will-ca
Copy link

will-ca commented Dec 14, 2021

Show all functions, but render them further down on the page and weight them lower for searching depending on how far removed they are from the type? Or list them all in one compact block?

@AlexisRiksman-TomTom
Copy link

Extensions that are defined in the application may be relevant for all child classes as well. Are these included in the 'Inherited functions' or 'Inherited properties' tabs, when configuration 'separateInheritedMembers= true' ?

@vmishenev
Copy link
Member

vmishenev commented Apr 25, 2022

It can be useful for Listfrom stdlib. Dokka does not display extensions for its supertypes: Collection and Iterable.
Old Dokka rendered them, see https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-list/#extension-functions

Although for enum it makes no sense to show extensions for Comparable.

@vmishenev vmishenev added the feedback: Kotlin libs Feedback from Kotlin's internal libraries label Apr 25, 2022
@vmishenev vmishenev linked a pull request Aug 16, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An issue for a feature or an overall improvement feedback: Kotlin libs Feedback from Kotlin's internal libraries
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants