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

A proposal for supporting references to synthetic Java properties #329

Merged
merged 10 commits into from
Dec 14, 2022

Conversation

strangepleasures
Copy link
Contributor

No description provided.

@nikitabobko
Copy link
Member

nikitabobko commented Nov 2, 2022

This feature has lots of problems. It needs extensive testing. Consider static isFoo, consider Kotlin -> Java inheritance, consider Java field + getter (who wins in resolution), consider references to other Java synthetic functions (Enum.values), consider K2

public class Jaba {
    public String publicField;

    public String getPublicField() {
        return publicField;
    }

    public static String getStaticFoo() {
        return "";
    }

    private String privateField;

    public String getPrivateField() {
        return "";
    }

    public String getALLAREUPPERCASE() {
        return "";
    }

    public String getGetFoo() {
        return "";
    }

    public String getFoo() {
        return "";
    }

    public String getClassClash() {
        return "";
    }

    class getClassClash {
    }

    public static boolean isStaticFoo() {
        return true;
    }
}
import kotlin.reflect.KProperty

fun main() {
    val a = Jaba::publicField // resolved to the field (but should be ambiguity)
    val b = Jaba::privateField // resolved to the method (expected)
    val g = Jaba::getClassClash // Overload resolution ambiguity (expected)

    Jaba().allareuppercase // does this weird Kotlin feature interfere with synthetic properties?

    // Probably unrelated because it's only about functions
    val c = Jaba::staticFoo // Unresolved reference
    val d = Jaba::isStaticFoo // works as KFunction

    val e = Jaba::getFoo // is getGetFoo() or getFoo() ?
    val f: KProperty<String> = Jaba::getFoo // is getGetFoo() or getFoo() ?
    // ^ e and f are different
}

I expect the KEEP document to prove that we cover all the cases

Corner cases are co-authored-by: Simon Ogorodnik, Pavel Mikhailovskii, Nikita Bobko

UPD: covered by Reference resolution strategy section

proposals/references-to-java-synthetic-properties.md Outdated Show resolved Hide resolved
proposals/references-to-java-synthetic-properties.md Outdated Show resolved Hide resolved
proposals/references-to-java-synthetic-properties.md Outdated Show resolved Hide resolved
proposals/references-to-java-synthetic-properties.md Outdated Show resolved Hide resolved
proposals/references-to-java-synthetic-properties.md Outdated Show resolved Hide resolved
proposals/references-to-java-synthetic-properties.md Outdated Show resolved Hide resolved
proposals/references-to-java-synthetic-properties.md Outdated Show resolved Hide resolved
proposals/references-to-java-synthetic-properties.md Outdated Show resolved Hide resolved
proposals/references-to-java-synthetic-properties.md Outdated Show resolved Hide resolved
proposals/references-to-java-synthetic-properties.md Outdated Show resolved Hide resolved
strangepleasures and others added 2 commits November 3, 2022 12:02
Co-authored-by: Alexander Udalov <[email protected]>
Co-authored-by: Alexander Udalov <[email protected]>
Alefas pushed a commit to JetBrains/kotlin that referenced this pull request Nov 3, 2022
EgorKulikov pushed a commit to JetBrains/kotlin that referenced this pull request Nov 15, 2022
As was mentioned above, all features requiring `kotlin-reflect`, such as `KProperty::visibility` worked incorrectly.
Instead of somehow taking into account the synthetic nature of the property, they tried to access a Java **field** of the same name.
If no such field existed a run-time exception would be thrown.
In particular, that meant that calling `propertyReference.seter(value)` would bypass the original setter method and write
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In particular, that meant that calling `propertyReference.seter(value)` would bypass the original setter method and write
In particular, that meant that calling `propertyReference.setter(value)` would bypass the original setter method and write

Co-authored-by: Alexander Udalov <[email protected]>
@strangepleasures strangepleasures merged commit 6cbc590 into master Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants