Skip to content

Do not allow to use Final and ClassVar at same time #10478

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 19, 2021
Merged

Do not allow to use Final and ClassVar at same time #10478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30a95a1
Do not allow use Final and ClassVar at same time
uriyyo May 15, 2021
dd80df8
Add more tests for ClassVar and Final usage
uriyyo May 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions mypy/semanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2295,6 +2295,11 @@ def unwrap_final(self, s: AssignmentStmt) -> bool:
self.fail("Type in Final[...] can only be omitted if there is an initializer", s)
else:
s.type = s.unanalyzed_type.args[0]

if s.type is not None and self.is_classvar(s.type):
self.fail("Variable should not be annotated with both ClassVar and Final", s)
return False

if len(s.lvalues) != 1 or not isinstance(s.lvalues[0], RefExpr):
self.fail("Invalid final declaration", s)
return False
Expand Down
9 changes: 9 additions & 0 deletions test-data/unit/check-final.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1082,3 +1082,12 @@ class A:
self.x = 10 # type: Final
undefined # type: ignore
[builtins fixtures/tuple.pyi]

[case testFinalUsedWithClassVar]
from typing import Final, ClassVar

class A:
a: Final[ClassVar[int]] # E: Variable should not be annotated with both ClassVar and Final
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you test some other variants, such as a: ClassVar[Final] = 1, a: ClassVar[Final[int]]?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such cases will produce an error with message:

Final can be only used as an outermost qualifier in a variable annotation.

Should it be changed to Variable should not be annotated with both ClassVar and Final?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's easy to do, then yes. The existing error message would make users first swap the qualifiers, then get an error anyway from your new check, which is not very user-friendly. But don't worry about changing the error message if it's complicated.

Either way I think it would be useful to test all these variations here. They can all go in the same test case.

Copy link
Member Author

@uriyyo uriyyo May 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added tests that you mention, thanks for pointing this out.

Looks like it is not easy to change message for a ClassVar[Final[int]] case.

I will skip this for this moment.

b: ClassVar[Final[int]] # E: Final can be only used as an outermost qualifier in a variable annotation
c: ClassVar[Final] = 1 # E: Final can be only used as an outermost qualifier in a variable annotation
[out]