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.

Sign up for GitHub

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

Jump to bottom

Hide derived members that are not public #1459

Merged
merged 1 commit into from Oct 7, 2020
Merged

Hide derived members that are not public #1459

merged 1 commit into from Oct 7, 2020

Conversation

BarkingBad
Copy link
Contributor

@BarkingBad BarkingBad commented Sep 11, 2020 •
edited 

open class A {
    private val shouldBeHidden: Int = 1
    val shouldBeVisible: Int = 2

    private fun privateFun(): Int = 3
    fun publicFun(): Int = 4
}

open class B : A()

open class C : B()

obraz
obraz
obraz

obraz
obraz
obraz

@BarkingBad BarkingBad self-assigned this Sep 11, 2020
@BarkingBad BarkingBad linked an issue Sep 11, 2020 that may be closed by this pull request
Hidden members of base class are documented in derived class #1451
Closed
BarkingBad
BarkingBad commented Sep 11, 2020
View reviewed changes
...ns/base/src/main/kotlin/translators/descriptors/DefaultDescriptorToDocumentableTranslator.kt Outdated
Comment on lines 354 to 436
fun CallableMemberDescriptor.createDRI(wasOverridenBy: DRI? = null): Pair<DRI, DRI?> =
if (kind == CallableMemberDescriptor.Kind.DECLARATION || overriddenDescriptors.isEmpty())
Pair(DRI.from(this), wasOverridenBy)
else
overriddenDescriptors.first().createDRI(DRI.from(this))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was wrong as it was passing only neighbouring DRI to the next level of recursion. We want the most upper and lower DRIs. Moreover it was wrongly interpreted then as an inheritedFrom insted of overridenBy in 361 line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Anyway, I think it is nice to use this function for visitPropertyDescriptor as well. I would also change the extra InheritedFunction to InheritedCallable and then change presentation of property/function row to something like this

obraz

@Kordyjan @sellmair @kamildoleglo what do you think on that?

@BarkingBad BarkingBad force-pushed the fix-hiding-derived-mmebers branch 2 times, most recently from 318d616 to 82d6c56 Compare September 11, 2020 15:06
@BarkingBad
Copy link
Contributor Author

I know it's 528 line, but ~500 are just one test. I would keep it more concise though autoformatter had a different opinion about that

@kamildoleglo kamildoleglo merged commit c2b7412 into master Oct 7, 2020
@kamildoleglo kamildoleglo deleted the fix-hiding-derived-mmebers branch October 7, 2020 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kamildoleglo kamildoleglo kamildoleglo approved these changes

@MarcinAman MarcinAman MarcinAman approved these changes

@Kordyjan Kordyjan Awaiting requested review from Kordyjan

Assignees

@BarkingBad BarkingBad

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Hidden members of base class are documented in derived class
3 participants
@BarkingBad @kamildoleglo @MarcinAman