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

Type guard is not working properly with walrus conditional #686

Closed
erictraut opened this issue May 23, 2020 · 6 comments
Closed

Type guard is not working properly with walrus conditional #686

erictraut opened this issue May 23, 2020 · 6 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@erictraut
Copy link
Collaborator

erictraut commented May 23, 2020

In issue #474, @alexkoay reported the following bug:

import re
def foo(s: str) -> str:
	if m := re.fullmatch('(test).+', s):
		return m.group(1)
	return 'oops'

group reports a reportOptionalMemberAccess error under Pyright 1.1.38.

@erictraut erictraut added the bug Something isn't working label May 23, 2020
@erictraut
Copy link
Collaborator Author

Looking into this more, the behavior is correct. The bug is in the code, not in Pyright. It was not reported as an error in previous versions of Pyright, but a recent bug fix in 1.1.38 changed that.

It has nothing to do with type guards or the walrus operator. The same error is identified in this simplified version of the code:

def foo(s: str):
    m = re.fullmatch('(test).+', s)
    m.group(1)

The problem here is that re.fullmatch can return None. If it does, your program will crash when it executes m.group(1). Your code needs to deal with the None case. If you're confident that your usage of re.fullmatch will never produce a None result, you can add an assertion to the code, like this:

def foo(s: str):
    m = re.fullmatch('(test).+', s)
    assert m is not None
    m.group(1)

@erictraut erictraut added as designed Not a bug, working as intended and removed bug Something isn't working labels May 23, 2020
@decorator-factory
Copy link

decorator-factory commented May 24, 2020

These two are not equivalent at all (that's the point of the if).

import re
def foo(s: str) -> str:
	if m := re.fullmatch('(test).+', s):
		return m.group(1)
	return 'oops'
def foo(s: str):
    m = re.fullmatch('(test).+', s)
    m.group(1)

The first one will never execute return m.group(1) if m is None.

In [12]: re.Match??
Init signature: re.Match()
Docstring
The result of re.match() and re.search().
Match objects always have a boolean value of True.

I've had the same issue with:

        if rematch := re.match(rf'^\s*{escaped}', string):
            _, end = rematch.span() # <-- "span" is not a known member of "None"

and even

        if (rematch := re.match(rf'^\s*{escaped}', string)) is not None:
            _, end = rematch.span() # <-- "span" is not a known member of "None"

With an assert, it works correctly. But it's redundant here.

        if rematch := re.match(rf'^\s*{escaped}', string):
            assert rematch is not None
            _, end = rematch.span()

@erictraut erictraut added bug Something isn't working and removed as designed Not a bug, working as intended labels May 24, 2020
@erictraut erictraut reopened this May 24, 2020
@erictraut
Copy link
Collaborator Author

You are correct. These aren't equivalent. Thanks for pointing that out. Re-opening the issue.

@decorator-factory
Copy link

Thank you!

@erictraut
Copy link
Collaborator Author

Type guards were already working for some assignment expressions, but it depended on the type of expression on the right-hand side. I've eliminated the restriction and added your example to the test suite.

The fix will be in the next version of Pyright.

@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label May 24, 2020
@erictraut
Copy link
Collaborator Author

This is now fixed in Pyright 1.1.40, which I just published.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants