Skip to content

[Feature]: Cleanup inconsistent JSpecify public class api聽#1994

Description

馃殌 Feature Request

Hello,
in our project, we recently upgraded to the new v1.63.0 version of playwright-java and noticed some changes in our kotlin-extensions regarding the addition of JSpecify annotations (which we really like in general). While adjusting the code we noticed some inconsistencies in the playwright-api.

If I compare the nullability information on a public field and its getter and setter, the API seems to be inconsistent.
For example if I look at Page.GetByRoleOptions which has the following code

// @NullMarked
class GetByRoleOptions {
    public @Nullable Object name;

    public GetByRoleOptions setName(String name) {
        this.name = name;
        return this;
    }

    public GetByRoleOptions setName(Pattern name) {
        this.name = name;
        return this;
    }
}

The underlying field is nullable while the setter-methods don't accept null-parameters. It's not clear to me what is the intended interaction with this field. The public field access might return null while the set-Methods don't allow to set a null value. But instead I can use the public field access to just set null (or any other type that is not String or Pattern). In Kotlin this is even a little more confusing

val opts: Page.GetByRoleOptions = Locator.getByRole(...)

val name: Any? = opts.name // this accesses the public field directly (no opts.getName())
opts.setName("name")         // calls the set-method
opts.setName(null)               // !error, because setName does not accept null as parameter
opts.name = null                   // allowed because it accesses public field directly

In Kotlin you can use property access that implicitly calls getters and setters and use apply to change the scope of the lambda function to the object its been called on. So instead of using the builder pattern in a chain, you could use apply and configure the object within the lambda. Having a public field and additional setter methods is confusing because in Kotlin calling either looks the same - but they have different nullability constraints.

opts.apply {
    name = null.                       // looks like it calls the setName function, but actually calls the public field
}
// if we had a Java pojo with a private field name with public get/set functions
val pojo = Pojo()
pojo.apply {
    name = null                        // this actually calls the setName function from the pojo
}

I'd be happy to look into it and also get to know nullaway/errorprone a bit better :)

Example

No response

Motivation

I don't really know anything about design decisions and how the programming api for playwright should look like, but in the Java ecosystem I rarely encounter public fields with additional setters. Coming from Kotlin, they are just a bit more confusing to use.
Cleaning up nullability information in the API would help developers to better understand the intention of methods and accessors.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions