PyCQA / flake8-bugbear

A plugin for Flake8 finding likely bugs and design problems in your program. Contains warnings that don't belong in pyflakes and pycodestyle.
MIT License
1.06k stars 104 forks source link

Couple new rule suggestions #440

Open r-downing opened 9 months ago

r-downing commented 9 months ago

I had a few thoughts for some more linter checks I'd like to have. Would be happy to implement them here if you guys are interested.

1. return <value> or yield <?from> <?value> in __init__()

Example:

class Example:
    def __init__():
        return 1  # bad
        yield  # bad
        yield 1  # bad
        yield from ()  # bad
        return  # ok

Any returning of values, or yielding, in an __init__() would result in a runtime error, but not until you try to instantiate an object of the class. So I think this should be a reasonably safe on-by-default error.

2. assert <constant or fixed value>

Normal asserts:

assert condition
assert condition, "message"

There's already a flake8 check for asserting on a tuple specifically - e.g. accidentally adding parens around (condition, message). And there's a bugbear check for assert False warning that it's disabled by -O. But forgetting the condition (assert "message") or asserting on any other literal value or list/tuple will either always pass or always fail - and is probably unintentional?

3. Inconsistent returns in function

def func(x) -> int | None:
    if x:
        return 0
    # should return None here

This one's more opinionated, but if a function returns something in some case, it should explicitly return something (None) in all other cases.

cooperlees commented 9 months ago

Thanks for asking before spending the time on the PR and us potentially saying nope.

  1. I'd accept it - Thought I do feel cpython/implementations should error on this
  2. I'd accept it ... Anything that is usless / pointless and not covered by a more specific flake8 plugin is welcome here
  3. With this one, are you only checking that there is always explicit returns at the end of a function's body? You're never worrying about the type right as I feel mypy / other type checkers will get that.

    • If so, I'd accept that too.

I don't feel the need for any of these to be optional. They all seems sane to me. But would love another contributor or users views ...

JelleZijlstra commented 9 months ago

Agree 1 and 2 are good.

3 is probably better left to a type checker; mypy already has a similar check. In general, it requires type information to know whether a function always returns. Some examples:

def int_or_die(s: str) -> int:
    try:
        return int(s)
    except ValueError:
        sys.exit(1)

def does_it_return() -> int:
    while True:
        try:
            return int(raw_input())
        except ValueError:
            continue