Skip to content

Detect exhaustiveness in partially-defined variable checks #13926

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

Open
ilinum opened this issue Oct 20, 2022 · 7 comments
Open

Detect exhaustiveness in partially-defined variable checks #13926

ilinum opened this issue Oct 20, 2022 · 7 comments
Labels
feature topic-possibly-undefined possibly-undefined error code

Comments

@ilinum
Copy link
Collaborator

ilinum commented Oct 20, 2022

For the partially defined check (--enable-error-code partially-defined), mypy generates a false-positive in the following case:

def f1(x: int | str) -> int:
  if isinstance(x, int):
    y = 1
  elif isinstance(x, str): 
    y = 2 
  return y # error: Name "y" may be undefined

The same applies to match statements:

def f(x: int | str) -> int:
  match x: 
    case int():
      y = 1 
    case str():
      y = 2 
  return y # error: Name "y" may be undefined

It's likely that mypy already detects this somewhere, since it doesn't complain about the missing return:

def f3(x: int | str) -> int:
  if isinstance(x, int):
    return 1
  elif isinstance(x, str): 
    return 2
@ilinum
Copy link
Collaborator Author

ilinum commented Nov 22, 2022

@JukkaL (or anyone else), I would appreciate some help here because I'm a bit stuck.

I have discovered has two places where it finds unreachable code:

  • Semantic Analyzer
  • Conditional Type Binder

Semantic Analyzer marks blocks with is_unreachable = True, so it's pretty easy to use (implemented in #14161). However, the exhaustiveness of type checks logic is located Conditional Type Binder, which is where it handles cases as described in the ticket.

Logic using conditional type binder:

self.binder.is_unreachable()

What I would like to do is to store information about unreachable blocks somewhere. My original approach, was to set block.is_unreachable = True in checker.py. This works with cases like:

def f1(x: int | str) -> int:
  if isinstance(x, int):
    y = 1
  elif isinstance(x, str): 
    y = 2 
  else:
    pass
  return y # No error any more.

However, this only works if there is a body but not if the body is missing (so the original examples here don't work). We can add special fields to the IfStmt and MatchStmt classes that indicate reachability of different branches but that sounds a bit iffy to me. Not that I have better ideas :)

Can you think of a good way of storing branch reachability information somewhere?

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 22, 2022

Can you think of a good way of storing branch reachability information somewhere?

Nothing comes to mind immediately, but I'll think about this more tomorrow.

JukkaL pushed a commit that referenced this issue Nov 22, 2022
This adds support for unreachable blocks in `partially-defined` check.

Currently, this only supports blocks that are detected as unreachable
during semantic analysis (so mostly stuff like python version, etc.).
This doesn't support more advanced cases (see #13926 for an example
what's not covered).

Closes #13929
@ilinum
Copy link
Collaborator Author

ilinum commented Feb 24, 2023

@JukkaL would appreciate some pointers here if you have a bit of time! :)

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 24, 2023

Sorry, lost track of this. I'll think about this next week (keep pinging me if you don't hear anything by mid next week).

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 27, 2023

@ilinum I think reusing block.is_unreachable sounds like the best approach. If there is no block, how about just replacing the None value for block with an empty block (Block([]), but only if the flag needs to be set so that this won't increase memory use significantly?

For example, if else_body is None and it's unreachable, we'd assign a Block([]) to else_body during type checking and set is_unreachable = True for this block. If it's reachable (the vast majority of cases), we'd keep the None value, i.e. a None value would implicitly mean "empty but reachable".

@erictraut
Copy link

Was the "partially-defined" error code ever implemented? I don't see any reference to it in the mypy docs or sources. I'm not able to repro the error with the code sample, but perhaps I'm not configuring mypy correctly. Or maybe this issue has been fixed.

@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 25, 2023

Was the "partially-defined" error code ever implemented?

yes, but the name was changed to possibly-undefined. You need to use --enable-error-code=possibly-undefined to enable it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature topic-possibly-undefined possibly-undefined error code
Projects
None yet
Development

No branches or pull requests

4 participants