reconcile ruff rule flake8-boolean-trap with current actions

[bandophahita]

When enabled, ruff rule FBT : flake8-boolean-trap triggers the following errors.

screenpy_selenium/actions/enter.py:154:46: FBT001 Boolean-typed positional argument in function definition
screenpy_selenium/actions/enter.py:154:46: FBT002 Boolean default positional argument in function definition
screenpy_selenium/actions/hold_down.py:90:62: FBT001 Boolean-typed positional argument in function definition
screenpy_selenium/actions/hold_down.py:90:62: FBT002 Boolean default positional argument in function definition
screenpy_selenium/actions/release.py:77:61: FBT001 Boolean-typed positional argument in function definition
screenpy_selenium/actions/release.py:77:61: FBT002 Boolean default positional argument in function definition
screenpy_selenium/questions/selected.py:92:54: FBT001 Boolean-typed positional argument in function definition
screenpy_selenium/questions/selected.py:92:54: FBT002 Boolean default positional argument in function definition
screenpy_selenium/questions/text.py:72:50: FBT001 Boolean-typed positional argument in function definition
screenpy_selenium/questions/text.py:72:50: FBT002 Boolean default positional argument in function definition

The simple solution would be to ignore the errors in these cases. However, if there is value in following this rule, we should consider what could be done.

---

I have yet to find a suggestion that does not break backwards compatibility.

Multiple function/class

Enter could be split up and a new class created like EnterCensored.
This would absolutely cause existing users to re-write portions of their code.

Mutually Exclusive Flags

Enter("text", mask=True) vs Enter("text", unmask=True)
Any existing users would need to add the keyword if they were calling the action without the classmethods.

Enum Argument

Enter("text", mask=MaskStyle.CENSORED)
Existing users with calls like Enter("text", True) would need to update their code.

I had thought it might be possible to build an enum using 0 and 1....

class MaskStyle(Enum):
    UNCENSORED = 0
    CENSORED = 1

...but alas bool(MaskStyle.UNCENSORED) returns True.

String Argument with Literal

Enter("text", "censored")
Just like the Enum solution, users would need to update their call.

keyword argument only

Enter("text", mask=True)
Again, any users that called the action Enter("text", True) would be required to add the keyword.

[bandophahita]

My thoughts are a little conflicted.

On the one hand, I agree with the basis of the rule. If we don't take compatibility into account, I would strongly lean towards multiple classes or keyword argument only. Though going with multiple classes incurs problems with natural language.

actor.will(EnterCensored.the_text('text') doesn't quite make sense.

On the other hand backwards compatibility is a golden rule we should not break without a new major version. I don't know why, but releasing a major version just for this feels... lackluster?

My gut says the smart move is to punt on this for now. Set # noqa: FBT001, FBT002 on the current actions but still enable the rule for any possible new code that is added.

If we are serious about adhering the rule for the current code, we ought to put a warning to users letting them know future changes will break their code.

[perrygoy]

I think we can just add a * to Enter's __init__ before the mask parameter and force keyword-only. I don't know if anyone is using Enter like:

the_actor.will(Enter(PASSWORD, True))

... but if they are, i would like them to stop, forcibly. :P

If we do this:

class Enter:
    ...
    def __init__(self: SelfEnter, text: str, *, mask: bool = False) -> None:
        ...

then the folks doing that will get this error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Enter.__init__() takes 1 positional argument but 2 were given

[perrygoy]

As for releasing a new major version, we could just have it simmer while we add some of the other stuff in here?

[bandophahita]

I came up with this decorator if we want to give folks a little time before we force their hand.

def pos_args_deprecated(*keywords: str) -> Function:
    """Warn users which positional arguments should be called via keyword."""

    def deprecated(func: Function) -> Function:
        argnames = func.__code__.co_varnames[: func.__code__.co_argcount]
        i = min([argnames.index(kw) for kw in keywords])
        kw_argnames = argnames[i:]

        @wraps(func)
        def wrapper(*args: P.args, **kwargs: P.kwargs) -> Function:
            # call the function first, to make sure the signature matches
            ret_value = func(*args, **kwargs)

            args_that_should_be_kw = args[i:]
            if args_that_should_be_kw:
                posargnames = ", ".join(kw_argnames)

                msg = (
                    f"Warning: positional arguments `{posargnames}` for "
                    f"`{func.__qualname__}` are deprecated.  "
                    f"Please use keyword arguments instead."
                )
                warnings.warn(msg, DeprecationWarning, stacklevel=2)

            return ret_value

        return wrapper

    return deprecated

It gets used like this:

class Enter:
    @pos_args_deprecated("mask")
    def __init__(
        self: SelfEnter, text: str, mask: bool = False  # noqa: FBT001, FBT002
    ) -> None:
    ...