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

Rhs in the form of "x" in "xxx" cannot infer the type #2010

Closed
WutingjiaX opened this issue Jul 2, 2024 · 6 comments
Closed

Rhs in the form of "x" in "xxx" cannot infer the type #2010

WutingjiaX opened this issue Jul 2, 2024 · 6 comments

Comments

@WutingjiaX
Copy link
Contributor

I found in _infer_comparison_part ,There are the following code snippets:

    elif str_operator in ('in', 'not in'):
        return NO_VALUES

This will result in the inability to infer the type when cursor under the word res:

res = "f" in "foo"

Why not return the following code like the previous branch?:

  return ValueSet([
      _bool_to_value(inference_state, True),
      _bool_to_value(inference_state, False)
  ])
@davidhalter
Copy link
Owner

Can you a bit more specific? What changes are you proposing?

@WutingjiaX
Copy link
Contributor Author

As I mentioned earlier, when I want to infer the type of the following code (the type of res), jedi cannot provide a result:

res = "f" in "foo"

the reason of that is in the function def _infer_comparison_part(inference_state, context, left, operator, right), When encountering a branch of the in operator,it will return NO_VALUES

elif str_operator in ('in', 'not in'):
        return NO_VALUES

So why doesn't this branch return the following content:

  return ValueSet([
      _bool_to_value(inference_state, True),
      _bool_to_value(inference_state, False)
  ])

Like the returned content in the previous COMPARISON_OPERATORSbranch:

    elif str_operator in COMPARISON_OPERATORS:
        if left.is_compiled() and right.is_compiled():
            # Possible, because the return is not an option. Just compare.
            result = left.execute_operation(right, str_operator)
            if result:
                return result
        else:
            # Omit some code

        return ValueSet([
            _bool_to_value(inference_state, True),
            _bool_to_value(inference_state, False)
        ])

After making the modifications, it can infer the correct content and all existing unit tests can still pass

@PeterJCLaw
Copy link
Collaborator

@WutingjiaX thanks for reporting this, you're correct that Jedi currently doesn't infer a type here. I'd guess that the vast majority of uses of x in y expressions are within conditional statements rather than assignments, which is perhaps why this hasn't been an issue before.

Note that __contains__ is something which types can optionally provide, so not all x in y tests are valid. The standard data model does indicate it should return True or False though, so I think assume that that's the return type (rather than e.g: looking up the real signature) could be a reasonable assumption. Given that many of the basic types do provide it, it may also be reasonable to assume that such a test is valid.

There might also be a broader question of why this comparison is handled separately from the binary comparisons -- that could be useful to understand before we change it. (I can't immediately see any hints from the git history)

It might be useful to understand a bit more about the use-case you have in mind here. Are you hitting an issue in a REPL? In a static code environment (perhaps an IDE)? An example test case might be a useful way to express this.

@davidhalter does Jedi have an opinion about whether (in general) it's better to over-infer (i.e: guess a type that might not actually be there) or under-infer (i.e: miss types which are actually present)?
It looks like currently there's under-inference and the suggestion is to flip that -- assume that in is always valid and infer accordingly.

@davidhalter davidhalter added the bug label Jul 9, 2024
@davidhalter
Copy link
Owner

@PeterJCLaw We generally over-infer. There is a simple reason for this: Autocompletion will always work in that case. In my Mypy/Jedi rewrite in Rust I'm trying to be very precise about types, because otherwise type-checking won't work and in this case (there I currently return bool for this case).

@WutingjiaX Can you create a PR for this with a test? I feel like this is not very important, but I would happily merge your proposal. There's probably also a way to return bool instead of True | False, but you don't need to care about that for now.

@WutingjiaX
Copy link
Contributor Author

@PeterJCLaw In a static code environment,but not an IDE。 I use plugins such as jedi, ruff, yapf, etc. to provide a language services at the top level for an AI application building platform (similiar to https://buildship.app/onboard/create-project).
In this scenario, I need the ability to infer expression types like "f" in "foo".Judging the legality of an expression by its return type

@davidhalter Sure,I‘m glad to create a PR for this。By the way,I have another similar type inference question, and I'm not sure if I should raise it in this issue or create a new one

@davidhalter
Copy link
Owner

Feel free to open another issue if it's not related to the same thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants