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

[Rule request] Missing return statement #11319

Open
mrcljx opened this issue May 7, 2024 · 2 comments
Open

[Rule request] Missing return statement #11319

mrcljx opened this issue May 7, 2024 · 2 comments
Labels
rule Implementing or modifying a lint rule

Comments

@mrcljx
Copy link

mrcljx commented May 7, 2024

I just stumbled over a bug in our codebase:

def f() -> int:
  return 42

def g() -> int | None:
  f()

The last line should have been return f().

While Mypy reports a Missing return statement, Pyright does not. This is a deliberate decision by the Pyright maintainer (point 2) as there is no type issue here. In this case Mypy actually does some static analysis outside of its main responsibility.

I'm wondering whether there could be a Ruff rule for this. Often, Ruff lacks the type information. But in this case type information isn't needed. If a function:

  1. has a return annotation,
  2. and it's different than -> None or -> NoReturn,
  3. and the body
    1. isn't a stub (... or pass)
    2. doesn't have a return anywhere (in that case RET503 takes over)
    3. and control flow can reach the end of the body
  4. report an error
def ok_1() -> int | None:
  return 2

def ok_2() -> int | None:
  return # < maybe a second rule later could reject `return` without a value in this scenario

def ok_3() -> int | None:
  raise NotImplementedError

def ok_4() -> int | None: ...

def ok_5() -> int | None:
  pass

def ok_6() -> int | None:
  if ...:
    return 42 # RET503 will cause an error here

def ok_7():
  f()
def bad_1() -> int | None:
  f()

def bad_2() -> int | None:
  if ...:
    raise ValueError()
@autinerd
Copy link
Contributor

autinerd commented May 8, 2024

I think it would be a good idea to extend RET503 to take this into account.

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label May 8, 2024
@JonathanPlasse
Copy link
Contributor

I think this could be a good first issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

4 participants