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 check fails: "if None" condition ignored within lambda #261

Closed
fabiorossetto opened this issue Aug 20, 2020 · 4 comments
Closed

Type check fails: "if None" condition ignored within lambda #261

fabiorossetto opened this issue Aug 20, 2020 · 4 comments
Labels
fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@fabiorossetto
Copy link

Environment data

  • Language Server version: v2020.8.1, with basic type checking active
  • OS and version: Ubuntu 18.04
  • Python version (& distribution if applicable, e.g. Anaconda): Python 3.6

Expected behaviour

Pylance correctly does not report any error whithin this function. s_not_none is correctly inferred to be of type str

def get_closure_2(s: Union[str, None]) -> Union[Callable, None]:
    if s is None:
        return None
    s_not_none = s
    return lambda f: "hello".endswith(s_not_none)

However in the following function

def get_closure_1(s: Union[str, None]) -> Union[Callable, None]:
    if s is None:
        return None
    return lambda f: "hello".endswith(s)

Pylance says that Argument of type "str | None" cannot be assigned to parameter "suffix" of type "str | Tuple[Text, ...]" in function "endswith". The expected behavior is that no error should be reported.

Actual behaviour

An error is reported by Pylance saying that s may be None, even though there is a check for it.

Pylance says that Argument of type "str | None" cannot be assigned to parameter "suffix" of type "str | Tuple[Text, ...]" in function "endswith"

@erictraut
Copy link
Contributor

In most cases, Pylance uses code flow analysis to determine whether a type can be narrowed from its original declared type. Using your example above, the type of s is declared as Union[str, None], but after the first if statement, it is narrowed to just str because the code eliminates the possibility that it is a None. However, when s is captured by the lambda, its evaluation is not done in code-flow order. Its evaluation is deferred until the lambda is executed. (The same would be true if you captured s and used it in a locally-defined function.) For captures, it's not safe to use code flow analysis or type narrowing, so the original declared type is used by the type checker. In your particular example, we can examine the code and determine that it's safe, but it's difficult for a type checker to make this determination in general.

This is the same approach I've seen used in other type checkers in other languages. For example, TypeScript uses this same approach for captured variables used in lambdas or locally-defined functions.

Consider this small variation on your example. In this case, s is getting reassigned after the lambda is declared and after s is captured. When the lambda is eventually executed, s contains None, and the code crashes.

def get_closure_1(s: Union[str, None]) -> Union[Callable, None]:
    if s is None:
        return None
    my_lambda = lambda f: "hello".endswith(s)
    s = None
    return my_lambda

In the few cases where I've encountered this pattern in my team's code base, I've easily worked around it using the technique in your first example — by defining a new local variable that has the correct type.

An alternative workaround involves the use of an assert statement, but that requires you to switch from a lambda to a local function, which is definitely more verbose. It does has the advantage of allowing the input parameters to be explicitly annotated, which isn't possible with the lambda syntax.

def get_closure_1(s: Union[str, None]) -> Union[Callable, None]:
    if s is None:
        return None
    
    def my_func(f: str):
        assert s is not None
        return "hello".endswith(s)

    return my_func

And of course, you can always suppress type-related errors using a "# type: ignore" comment or a "cast" call, but I tend to use those as a last resort.

@judej judej added the waiting for user response Requires more information from user label Aug 20, 2020
@github-actions github-actions bot removed the triage label Aug 20, 2020
@fabiorossetto
Copy link
Author

Dear Eric,
thank you very much for your detailed response. I was expecting the issue to be something like that. I'm glad to have confirmation. I understand that it is nearly impossible for the type checker to do the correct determination in this scenario.

I stumbled upon this issue while presenting Pylance to my team. It's really an amazing tool and I'm promoting its usage within the company.

The issue can be closed from my side. Should I close it?

@erictraut erictraut removed the waiting for user response Requires more information from user label Aug 21, 2020
@erictraut
Copy link
Contributor

I found a way to handle type narrowing of local variables and parameters captured by inner-scoped functions and lambdas, so the above workaround will no longer be necessary in this case.

@erictraut erictraut reopened this Sep 27, 2021
@erictraut erictraut added fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed triage labels Sep 27, 2021
@jakebailey
Copy link
Member

This issue has been fixed in version 2021.9.4, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/main/CHANGELOG.md#202194-29-september-2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

4 participants