Skip to content

Bug: raising a nested error class causes spurious ‘member is not assignable’ error #1968

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

Closed
sietse opened this issue Jul 31, 2016 · 5 comments
Labels
bug mypy got something wrong priority-2-low

Comments

@sietse
Copy link

sietse commented Jul 31, 2016

First off, thank you all for the delightful mypy tool. I really enjoy it every day I use it. ^_^

As for the bug report, mypy gives a spurious error in this scenario:

I have a class Shift with, nested inside it, the exception class
Shift.AlreadyCovered. The Shift.cover() method, which is annotated to
return None, will raise self.AlreadyCovered under certain circumstances. A
mypy check then complains that Member "AlreadyCovered" is not assignable.
See cover1 in the example below.

Mypy does not give an error in these variants:

  • a method is not annotated to return None (cover2)
  • a method raises an exception defined outside the current class (cover3)
  • a method raises an exception defined outside the current class via a
    reference defined inside the current class. (cover4)

Minimal example of the error:

class Shift:

    # A nested exception class defined inside the current class
    class AlreadyCovered(Exception):
        pass
    # If a method is annotated to return None, then raising the nested
    # exception class gives a mypy error
    #     x.py: note: In member "cover1" of class "Shift":
    #     x.py:11: error: Member "AlreadyCovered" is not assignable
    def cover1(self) -> None:
        raise self.AlreadyCovered  # <-- ERROR HERE

The variants that don't raise errors:

# An exception class defined outside the main (Shift) class
class OuterAlreadyCovered(Exception):
    pass

class Shift:

    # A nested exception class defined inside the current class
    class AlreadyCovered(Exception):
        pass

    # If a method is not annotated to return None, then mypy is okay
    # with raising the nested exception class
    def cover2(self):
        raise self.AlreadyCovered

    # Raising an exception defined outside the current class is fine.
    def cover3(self) -> None:
        raise OuterAlreadyCovered

    # Raising a reference defined inside the current class to an
    # exception defined outside the class is fine.
    def cover4(self) -> None:
        raise self.BorrowedAlreadyCovered
@gvanrossum
Copy link
Member

Thanks for the bug report. There's one more variant that you didn't try -- raise Shift.AlreadyCovered. Does that work?

I understand that that's just a work-around, and mypy really should handle such scoping issues better. Unfortunately, internally class definitions are treated quite specially, and there is also occasionally some confusion between instance and class attributes. So it may be a while before we get to fixing this. (If you want to give it a try please do, but be aware that this part of the mypy code is quite intricate, so I won't hold it against you if you pass.)

@gvanrossum gvanrossum added the bug mypy got something wrong label Aug 1, 2016
@sietse
Copy link
Author

sietse commented Aug 1, 2016

The raise Shift.AlreadyCovered variant causes no errors, so that is indeed a good work-around for now.

class Shift:

    # A nested exception class defined inside the current class
    class AlreadyCovered(Exception):
        pass

    def cover5(self) -> None:
        raise Shift.BorrowedAlreadyCovered

I'll try to have a look at the code; we'll see how far I get. I am quite tentative, so anybody else should also feel free to pick this up, and not wait until/whether I report back.

@gnprice
Copy link
Collaborator

gnprice commented Aug 4, 2016

Great! Will be glad to see a fix if you can make one.

If nothing else it would also be useful just to give it a better error message -- that error message is confusing and erroneous, in addition to this situation probably not being something that should be an error.

@gnprice gnprice added this to the Future milestone Aug 4, 2016
@gvanrossum gvanrossum removed this from the Future milestone Mar 29, 2017
@ilinum
Copy link
Collaborator

ilinum commented Jul 6, 2017

I can't seem to reproduce the issue:

$ cat n.py
class Shift:
    class AlreadyCovered(Exception):
        pass

    def cover1(self) -> None:
        raise self.AlreadyCovered
$ mypy n.py
$

@emmatyping
Copy link
Member

As of 0.511 it was still causing errors. I believe this was fixed in #3636.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong priority-2-low
Projects
None yet
Development

No branches or pull requests

5 participants