Do not filter out accessors of synthetic properties - #4515
Conversation
4394495 to
374259f
Compare
| @@ -2342,272 +2342,1637 @@ <h2 class="tableheader">Functions</h2> | |||
| </div> | |||
| </div> | |||
| </div> | |||
| <a data-name="-2108225789%2FFunctions%2F1404203640" anchor-label="getActionBar" id="-2108225789%2FFunctions%2F1404203640" data-filterable-set=":/release"></a> | |||
There was a problem hiding this comment.
I feel that something is wrong here...
When I open this expectedData, I see that we have both properties like actionBar and application there:
As well as get* and set* functions:
So, for me, it feels like if we don't filter out getters and setters coming from Java (this is what this PR does, and I think it's correct), we then need to filter out synthetic Java properties. WDYT?
I mean, the current way, where we show both the synthetic property and its getter and setter, feels very wrong.
There was a problem hiding this comment.
So, for me, it feels like if we don't filter out getters and setters coming from Java (this is what this PR does, and I think it's correct), we then need to filter out synthetic Java properties.
I didn’t quite get your point. What’s the reason for that?
From a code perspective, both the synthetic property and the accessors are available.
// Java
public class A {
public int getA() {
return 0;
}
}
// Kotlin
fun f(a: A) {
a.getA() // resolved
a.a // resolved
}
However, I’m not sure about compatibility - whether we could break links to the accessors in the future.
There was a problem hiding this comment.
Yes, I understand that both are resolvable, but it's a bit strange that we have two separate declarations (getA function and a property) in the generated API reference that represent a single real declaration (getA function).
Also, as far as I understand, if we have documentation for a getter, it will apply to both a property and a function. But if we have both a getter and a setter, we will have documentation only for the getter/setter.
What I would like to understand is how the output will look for something like this:
public class Container {
/** get A doc */
public int getA() { return 0; }
/** get B doc */
public int getB() { return 0; }
/** set B doc */
public void setB(int value) {}
}I understand that it should probably be discussed in #4250 in a broader scope. I'm just a bit skeptical that showing both the synthetic property and its getter separately all the time is the best way forward here, and maybe we need to just show only the getter/setter and ignore the synthetic properties entirely. We could still have a problem: the synthetic property has no documentation, but its getter/setter does.
Maybe there are other solutions here, and we should discuss them again?
There was a problem hiding this comment.
Also, as far as I understand, if we have documentation for a getter, it will apply to both a property and a function.
A synthetic property has no doc in Dokka at all.
In your case, the output will have two properties a and b with no doc.
JIC In this scenario, Java PSI generate not synthetic properties because there are no corresponding backing fields.
Maybe there are other solutions here, and we should discuss them again?
I have no simple solutions here.
1a8b0cd to
69b6fe6
Compare
When Java synthetic properties are formed by "merging" getters and setters, documentation from their accessors can be lost. To preserve this information, we should avoid filtering them out and instead show the original accessors alongside their corresponding synthetic property.
Further, this issue should be addressed.
Original #4502 (comment)