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

Alternative syntax for unions requires Python 3.10 or newer #513

Closed
stolenzc opened this issue Oct 23, 2020 · 15 comments
Closed

Alternative syntax for unions requires Python 3.10 or newer #513

stolenzc opened this issue Oct 23, 2020 · 15 comments
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@stolenzc
Copy link

Envirement data

  • pylance version: 2020.10.2
  • vscode version: 1.50.1
  • python version: 3.8.5
  • Djagno version: 3.0.3
  • OS platform: MacOs 10.15.7

My django program also alert this and I don't know why, I want to know why and how to ignore this warning?
image

@Zajozor
Copy link

Zajozor commented Oct 23, 2020

Experiencing the same issue, pylance version: 2020.10.2 (pyright d6db7c90), Python 3.7, Django 2.2

Same as the op, we are using Django Rest Framework.

CleanShot 2020-10-23 at 11 30 48@2x

The permission classes which contain the | between them inherit from this class

@six.add_metaclass(BasePermissionMetaclass)
class BasePermission(object):
    """
    A base class from which all permission classes should inherit.
    """

    def has_permission(self, request, view):
        """
        Return `True` if permission is granted, `False` otherwise.
        """
        return True

    def has_object_permission(self, request, view, obj):
        """
        Return `True` if permission is granted, `False` otherwise.
        """
        return True

Further related code can be found at https://github.com/encode/django-rest-framework/blob/master/rest_framework/permissions.py#L97 and near that.

Even manually adding a metaclass with defined __or__ property to the Permission classes does not remove the pylance error. Please let me know if further information is required.

@erictraut
Copy link
Contributor

Thanks for the bug report. I've updated the logic so it detects the case where a class has a custom metaclass that supports the __or__ or __ror__ method and use that for the | operator rather than assuming a misuse of of PEP 614 prior to Python 3.10. This fix will be in the next version of pylance.

@erictraut erictraut added bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed triage labels Oct 25, 2020
@cdce8p
Copy link

cdce8p commented Oct 28, 2020

@erictraut The | operator for type annotations won't be exclusive to 3.10, since postponed evaluation of annotations is enough to support it, as long as the type checker understands it. It can even be used in type comments with Python 2. python/typing#429 (comment) and python/mypy#9647

It might be an idea to limit the warning to isinstance and issubclass calls, or check if from __future__ import annotations is used with type annotations.

@erictraut
Copy link
Contributor

@cdce8p, that's a good point. I've changed the code to suppress that error in any situation where the evaluation of the type annotation is postponed (within quotes and with from __future__ import annotations enabled). The diagnostic will still be reported in cases where you're using <3.10 and the evaluation is not postponed (e.g. in a type alias declaration or an isinstance call).

@jakebailey
Copy link
Member

This issue has been fixed in version 2020.10.3, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/master/CHANGELOG.md#2020103-28-october-2020

@Zajozor
Copy link

Zajozor commented Oct 29, 2020

@jakebailey The problem is not fixed in 2020.10.3

[Info  - 9:19:46 AM] Pylance language server 2020.10.3 (pyright b881f28d) starting

CleanShot 2020-10-29 at 09 22 31@2x

The specific class is from DRF ( details in my comment above ).

Should I create a new issue, or can we reopen this?
Thank you.

@jakebailey jakebailey reopened this Oct 29, 2020
@jakebailey jakebailey added needs investigation Could be an issue - needs investigation and removed fixed in next version (main) A fix has been implemented and will appear in an upcoming version labels Oct 29, 2020
@cdce8p
Copy link

cdce8p commented Oct 29, 2020

Same with type annotations. It seems mostly fixed, however I'm encountering a wired edge case with . notation.
E.g. time | None works, whereas dt.time | None dosen't.

Screen Shot 2020-10-29 at 14 27 02

@erictraut
Copy link
Contributor

@Zajozor, the case involving the Django Permission class will require an update to the Django type stubs so the metaclass declaration properly reflects the fact that it supports a custom __or__ overload method. Please file a separate issue on this, since it's a different issue from the type checker logic.

@cdce8p, I'm able to repro this problem in pylance but not in pyright, which is strange. I suspect it's related to the semantic token highlighting implemented in pylance. I'll investigate further.

@erictraut
Copy link
Contributor

@cdce8p, I found the root cause of the problem you reported. As I suspected, it was a bad interaction between the semantic token provider and the type evaluator. When the semantic token provider asks the type evaluator "what is the type of this token?", there are cases where we need to walk up the parse tree and evaluate the expression in a broader context. In this case, for example, we need to evaluate the entire type annotation expression so we know that the expression has a postponed evaluation.

The fix is in place, and it will be included in the next release.

@erictraut erictraut added fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed needs investigation Could be an issue - needs investigation labels Oct 29, 2020
@cdce8p
Copy link

cdce8p commented Oct 29, 2020

@erictraut Thanks for spending the time investigating and fixing the issue 👍

@jakebailey
Copy link
Member

This issue has been fixed in version 2020.11.0, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/master/CHANGELOG.md#2020110-4-november-2020

@nourselim0
Copy link

For those wondering about the DRF Stubs issue, a very temporary fix is it edit your stubs locally, just go to rest-framework-stubs/permissions.pyi:34 (or just "Go to Definition" of BasePermission) and replace this:
class BasePermission(object):
with this:
class BasePermission(object, metaclass=BasePermissionMetaclass):
Everything else is already there, the metaclass itself is in the stub and it properly inherits from OperationsHolderMixin which has the __or__ method defined.

@cdce8p
Copy link

cdce8p commented Nov 11, 2020

@erictraut There still seems to be an issue when using more the two types.

from __future__ import annotations
var: int | str | list | None = None

This would be a valid syntax: https://www.python.org/dev/peps/pep-0604/#simplified-syntax

Screen Shot 2020-11-11 at 01 28 13

@erictraut
Copy link
Contributor

@cdce8p, this is a different issue. You're using annotations future with a version of Python older than 3.10. This will be fixed in the next release.

@ahnafis
Copy link

ahnafis commented Oct 4, 2021

@erictraut The | operator for type annotations won't be exclusive to 3.10, since postponed evaluation of annotations is enough to support it, as long as the type checker understands it. It can even be used in type comments with Python 2. python/typing#429 (comment) and python/mypy#9647

It might be an idea to limit the warning to isinstance and issubclass calls, or check if from __future__ import annotations is used with type annotations.

Thank you soo much!!! This worked for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 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

7 participants