pylint-dev/pylint

E0606: detect validation done in external function

mikez opened this issue · 6 comments

mikez commented

Bug description

def f(offset):
    validate_offset(offset)

    # compute modifier
    number, suffix = int(offset[:-1]), offset[-1]

    if suffix == "d":
        modifier = f"-{number} days"
    elif suffix == "w":
        modifier = f"-{number * 7} days"
    elif suffix == "y":
        modifier = f"-{number} years"
    # else:
    #    raise ValueError() # This is here to make pylint etc. happy

    # do something with `modifier` here (omitted)
    modifier


def validate_offset(offset):
    if not isinstance(argument, str):
        raise ValueError(f"Invalid offset: {offset!r}")

    suffix = argument[-1:]  # slicing here to handle empty strings
    if suffix not in ("d", "w", "y"):
        raise ValueError(
            f"Invalid offset: {offset!r}\n"
            f"Please specify a string of the format 'X[d/w/y]' "
            "where X is a non-negative integer followed by 'd', 'w', or 'y' "
            "that indicates days, weeks, or years."
        )

Configuration

No response

Command used

pylint -sn *.py ${SRC_CORE} ${SRC_TEST}

Pylint output

************* Module things.database
things/database.py:1048:42: E0606: Possibly using variable 'modifier' before assignment (possibly-used-before-assignment)

Expected behavior

It is a common pattern for me to validate input in an external validation function. That keeps the code clean and readable.

In this case, I don't expect to have to write the additional code (commented above):

    else:
        raise ValueError() # This is here to make pylint etc. happy

since the validate_offset already raises if modified won't be set.

A potential workaround would be to explicitly mark modified will be set using some kind of asserts mechanism or comment that exists in the Typescript language (see here).

Pylint version

pylint 3.2.3
astroid 3.2.2
Python 3.11.9

OS / Environment

No response

Additional dependencies

No response

jacobtylerwalls commented

A potential workaround would be to explicitly mark modified will be set using some kind of asserts mechanism or comment that exists in the Typescript language (see here).

This what we've decided to support. Since pylint 3.2.1 you can use from typing import assert_never to assert exhaustiveness.

Will add your example to the docs 👍

mikez commented

@jacobtylerwalls Thanks for the fast response.

To clarify, you'd recommend a pattern like this one:

    if suffix == "d":
        modifier = f"-{number} days"
    elif suffix == "w":
        modifier = f"-{number * 7} days"
    elif suffix == "y":
        modifier = f"-{number} years"
    else:
        typing.assert_never()
jacobtylerwalls commented

Yep. Or slightly more expressively typing.assert_never(suffix) in case it's ever reached at runtime to see what it was called with.

  • 👍1
mikez commented

Yep. Or slightly more expressively typing.assert_never(suffix) in case it's ever reached at runtime to see what it was called with.

Great. Thank you. 😊🙏

Feel free to close this.

mikez commented

@jacobtylerwalls Small follow-up: how do I do this in a backwards compatible way, so it also works on Python 3.8, 3.9, and 3.10? Something like this?

try:
    from typing import assert_never
except ImportError:
    # backwards compatability with Python 3.8, 3.9, 3.10
    def assert_never(arg: NoReturn, /) -> NoReturn:
        """Assert line is unreachable."""
        raise AssertionError(arg)
jacobtylerwalls commented

I think you can import it from typing_extensions, although I haven't tried it myself.

  • 👍1