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

Relax Y011, Y014 and Y015 to allow default values in many contexts #326

Merged
merged 12 commits into from
Jan 16, 2023
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ Other changes:
banned by Y022. Use `re.Match` and `re.Pattern` instead.
* flake8-pyi no longer supports stub files that aim to support Python 2. If your
stubs need to support Python 2, pin flake8-pyi to 22.11.0 or lower.
* Y011, Y014 and Y015 have all been significantly relaxed. `None`, `bool`s,
`int`s, `float`s, `complex` numbers, strings and `bytes` are all now allowed
as default values for parameter annotations or assignments.

## 22.11.0

Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ currently emitted:
| Y008 | Unrecognized platform. To prevent you from typos, we warn if you use a platform name outside a small set of known platforms (e.g. `"linux"` and `"win32"`).
| Y009 | Empty body should contain `...`, not `pass`. This is just a stylistic choice, but it's the one typeshed made.
| Y010 | Function body must contain only `...`. Stub files should not contain code, so function bodies should be empty.
| Y011 | All default values for typed function arguments must be `...`. Type checkers ignore the default value, so the default value is not useful information in a stub file.
| Y011 | Only simple default values (`int`, `float`, `complex`, `bytes`, `str`, `bool`, `None` or `...`) are allowed for typed function arguments. Type checkers ignore the default value, so the default value is not useful information for type-checking, but it may be useful information for other users of stubs such as IDEs. If you're writing a stub for a function that has a more complex default value, use `...` instead of trying to reproduce the runtime default exactly in the stub.
| Y012 | Class body must not contain `pass`.
| Y013 | Non-empty class body must not contain `...`.
| Y014 | All default values for arguments must be `...`. A stronger version of Y011 that includes arguments without type annotations.
| Y015 | Attribute must not have a default value other than `...`.
| Y014 | Only simple default values are allowed for any function arguments. A stronger version of Y011 that includes arguments without type annotations.
| Y015 | Only simple default values are allowed for assignments. The same as Y011, but for assignments rather than parameter annotations.
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
| Y016 | Unions shouldn't contain duplicates, e.g. `str \| str` is not allowed.
| Y017 | Stubs should not contain assignments with multiple targets or non-name targets.
| Y018 | A private `TypeVar` should be used at least once in the file in which it is defined.
Expand Down
87 changes: 40 additions & 47 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -852,40 +852,6 @@ def _check_for_typevarlike_assignments(
else:
self.error(node, Y001.format(cls_name))

def _Y015_error(self, node: ast.Assign | ast.AnnAssign) -> None:
old_syntax = unparse(node)
copy_of_node = deepcopy(node)
copy_of_node.value = ast.Constant(value=...)
new_syntax = unparse(copy_of_node)
error_message = Y015.format(old_syntax=old_syntax, new_syntax=new_syntax)
self.error(node, error_message)

@staticmethod
def _Y015_violation_detected(node: ast.Assign | ast.AnnAssign) -> bool:
assignment = node.value

if isinstance(node, ast.AnnAssign):
if assignment and not isinstance(assignment, ast.Ellipsis):
return True
return False

if isinstance(assignment, (ast.Num, ast.Str, ast.Bytes)):
return True
if (
isinstance(assignment, ast.UnaryOp)
and isinstance(assignment.op, ast.USub)
and isinstance(assignment.operand, ast.Num)
):
return True
if (
isinstance(assignment, (ast.Constant, ast.NameConstant))
and not isinstance(assignment, ast.Ellipsis)
and assignment.value is not None
):
return True

return False

def visit_Assign(self, node: ast.Assign) -> None:
if self.in_function.active:
# We error for unexpected things within functions separately.
Expand All @@ -904,16 +870,15 @@ def visit_Assign(self, node: ast.Assign) -> None:
is_special_assignment = _is_assignment_which_must_have_a_value(
target_name, in_class=self.in_class.active
)
if is_special_assignment:
assignment = node.value
if is_special_assignment or isinstance(assignment, ast.Str):
with self.string_literals_allowed.enabled():
self.generic_visit(node)
else:
self.generic_visit(node)
if target_name is None:
return
assert isinstance(target, ast.Name)
assignment = node.value

if isinstance(assignment, ast.Call):
function = assignment.func
if _is_TypedDict(function):
Expand All @@ -925,11 +890,12 @@ def visit_Assign(self, node: ast.Assign) -> None:
)
return

if self._Y015_violation_detected(node):
return self._Y015_error(node)

if not is_special_assignment:
self._check_for_type_aliases(node, target, assignment)
if isinstance(
assignment, (ast.Dict, ast.List, ast.Tuple, ast.Set, ast.JoinedStr)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why doesn't this use _is_valid_stub_default? There are a lot of other expression nodes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup. But for assignments, we want to allow much more than just the things we allow in default values for parameters. All of the following are valid in a stub:

T = TypeVar("T") # ast.Call

def foo(): ...
bar = foo # ast.Name

class Spam:
    def eggs(self): ...
ham = Spam.eggs  # ast.Attribute

baz = Literal["foo"]  # ast.Subscript — this should trigger Y026 but not Y015

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, but this still allows a bunch of nodes that have little conceivable use, like Lambda and SetComp. Might be better to go with an allowlist instead of a blocklist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, fair enough, I'll rework.

):
self.error(node, Y015)

def visit_AugAssign(self, node: ast.AugAssign) -> None:
"""Allow `__all__ += ['foo', 'bar']` in a stub file"""
Expand Down Expand Up @@ -962,7 +928,11 @@ def _check_for_type_aliases(
and `X = None` (special-cased because it is so special).
"""
if (
isinstance(assignment, (ast.Subscript, ast.BinOp))
isinstance(assignment, ast.Subscript)
or (
isinstance(assignment, ast.BinOp)
and not self._is_valid_stub_default(assignment)
)
or _is_Any(assignment)
or _is_None(assignment)
):
Expand Down Expand Up @@ -1059,8 +1029,8 @@ def visit_AnnAssign(self, node: ast.AnnAssign) -> None:
return self._check_typealias(node=node, alias_name=target_name)

self.generic_visit(node)
if self._Y015_violation_detected(node):
self._Y015_error(node)
if node.value and not self._is_valid_stub_default(node.value):
self.error(node, Y015)

def _check_union_members(self, members: Sequence[ast.expr]) -> None:
first_union_member = members[0]
Expand Down Expand Up @@ -1695,12 +1665,35 @@ def visit_arguments(self, node: ast.arguments) -> None:
if node.kwarg is not None:
self.visit(node.kwarg)

def _is_valid_stub_default(self, default: ast.expr) -> bool:
# `...`, strings, bytes, ints, floats, complex numbers like `3j`, bools, None
if isinstance(
default, (ast.Ellipsis, ast.Str, ast.Bytes, ast.Num, ast.NameConstant)
):
return True
# Complex numbers such as `4+3j` or `4-3j`
if (
isinstance(default, ast.BinOp)
and isinstance(default.op, (ast.Add, ast.Sub))
and self._is_valid_stub_default(default.left)
and self._is_valid_stub_default(default.right)
):
return True
# Negative ints, negative floats, complex numbers such as `-4+3j` or `-4-3j`
if (
isinstance(default, ast.UnaryOp)
and isinstance(default.op, ast.USub)
and self._is_valid_stub_default(default.operand)
):
return True
return False

def check_arg_default(self, arg: ast.arg, default: ast.expr | None) -> None:
self.visit(arg)
if default is not None:
with self.string_literals_allowed.enabled():
self.visit(default)
if default is not None and not isinstance(default, ast.Ellipsis):
if default is not None and not self._is_valid_stub_default(default):
self.error(default, (Y014 if arg.annotation is None else Y011))

def error(self, node: ast.AST, message: str) -> None:
Expand Down Expand Up @@ -1832,11 +1825,11 @@ def parse_options(
Y008 = 'Y008 Unrecognized platform "{platform}"'
Y009 = 'Y009 Empty body should contain "...", not "pass"'
Y010 = 'Y010 Function body must contain only "..."'
Y011 = 'Y011 Default values for typed arguments must be "..."'
Y011 = "Y011 Only simple default values allowed for typed arguments"
Y012 = 'Y012 Class body must not contain "pass"'
Y013 = 'Y013 Non-empty class body must not contain "..."'
Y014 = 'Y014 Default values for arguments must be "..."'
Y015 = 'Y015 Bad default value. Use "{new_syntax}" instead of "{old_syntax}"'
Y014 = "Y014 Only simple default values allowed for arguments"
Y015 = "Y015 Only simple default values are allowed for assignments"
Y016 = 'Y016 Duplicate union member "{}"'
Y017 = "Y017 Only simple assignments allowed"
Y018 = 'Y018 {typevarlike_cls} "{typevar_name}" is not used'
Expand Down
48 changes: 29 additions & 19 deletions tests/attribute_annotations.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,20 @@ from typing import Final, TypeAlias

import typing_extensions

# We shouldn't emit Y015 for simple default values
field1: int
field2: int = ...
field3 = ... # type: int # Y033 Do not use type comments in stubs (e.g. use "x: int" instead of "x = ... # type: int")
field4: int = 0 # Y015 Bad default value. Use "field4: int = ..." instead of "field4: int = 0"
field5 = 0 # type: int # Y033 Do not use type comments in stubs (e.g. use "x: int" instead of "x = ... # type: int") # Y015 Bad default value. Use "field5 = ..." instead of "field5 = 0"
field6 = 0 # Y015 Bad default value. Use "field6 = ..." instead of "field6 = 0"
field7 = b"" # Y015 Bad default value. Use "field7 = ..." instead of "field7 = b''"
field8 = False # Y015 Bad default value. Use "field8 = ..." instead of "field8 = False"
field81 = -1 # Y015 Bad default value. Use "field81 = ..." instead of "field81 = -1"
field82: float = -98.43 # Y015 Bad default value. Use "field82: float = ..." instead of "field82: float = -98.43"
field83 = -42j # Y015 Bad default value. Use "field83 = ..." instead of "field83 = -42j"

# We don't want this one to trigger Y015 -- it's valid as a TypeAlias
field4: int = 0
field5 = 0 # type: int # Y033 Do not use type comments in stubs (e.g. use "x: int" instead of "x = ... # type: int")
field6 = 0
field7 = b""
field8 = False
field81 = -1
field82: float = -98.43
field83 = -42j
field84 = 5 + 42j
field85 = -5 - 42j
field9 = None # Y026 Use typing_extensions.TypeAlias for type aliases, e.g. "field9: TypeAlias = None"
Field10: TypeAlias = None

Expand All @@ -30,15 +31,20 @@ field16: typing.Final = "foo"
field17: typing_extensions.Final = "foo"
field18: Final = -24j

# We *should* emit Y015 for more complex default values
field19 = [1, 2, 3] # Y015 Only simple default values are allowed for assignments
field20 = (1, 2, 3) # Y015 Only simple default values are allowed for assignments
field21 = {1, 2, 3} # Y015 Only simple default values are allowed for assignments

class Foo:
field1: int
field2: int = ...
field3 = ... # type: int # Y033 Do not use type comments in stubs (e.g. use "x: int" instead of "x = ... # type: int")
field4: int = 0 # Y015 Bad default value. Use "field4: int = ..." instead of "field4: int = 0"
field5 = 0 # type: int # Y033 Do not use type comments in stubs (e.g. use "x: int" instead of "x = ... # type: int") # Y015 Bad default value. Use "field5 = ..." instead of "field5 = 0"
field6 = 0 # Y015 Bad default value. Use "field6 = ..." instead of "field6 = 0"
field7 = b"" # Y015 Bad default value. Use "field7 = ..." instead of "field7 = b''"
field8 = False # Y015 Bad default value. Use "field8 = ..." instead of "field8 = False"
field4: int = 0
field5 = 0 # type: int # Y033 Do not use type comments in stubs (e.g. use "x: int" instead of "x = ... # type: int")
field6 = 0
field7 = b""
field8 = False
# Tests for Final
field9: Final = 1
field10: Final = "foo"
Expand All @@ -48,10 +54,14 @@ class Foo:
field14: typing.Final = "foo"
field15: typing_extensions.Final = "foo"
# Standalone strings used to cause issues
field16 = "x" # Y015 Bad default value. Use "field16 = ..." instead of "field16 = 'x'" # Y020 Quoted annotations should never be used in stubs
field16 = "x"
if sys.platform == "linux":
field17 = "y" # Y015 Bad default value. Use "field17 = ..." instead of "field17 = 'y'" # Y020 Quoted annotations should never be used in stubs
field17 = "y"
elif sys.platform == "win32":
field18 = "z" # Y015 Bad default value. Use "field18 = ..." instead of "field18 = 'z'" # Y020 Quoted annotations should never be used in stubs
field18 = "z"
else:
field19 = "w" # Y015 Bad default value. Use "field19 = ..." instead of "field19 = 'w'" # Y020 Quoted annotations should never be used in stubs
field19 = "w"

field20 = [1, 2, 3] # Y015 Only simple default values are allowed for assignments
field21 = (1, 2, 3) # Y015 Only simple default values are allowed for assignments
field22 = {1, 2, 3} # Y015 Only simple default values are allowed for assignments
26 changes: 20 additions & 6 deletions tests/defaults.pyi
Original file line number Diff line number Diff line change
@@ -1,12 +1,26 @@
import os

def f1(x: int = ...) -> None: ...
def f2(x: int = 3) -> None: ... # Y011 Default values for typed arguments must be "..."
def f2(x: int = 3) -> None: ...
def f201(x: int = -3) -> None: ...
def f202(x: float = 3.14) -> None: ...
def f203(x: type = int) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f3(*, x: int = ...) -> None: ...
def f4(*, x: int = 3) -> None: ... # Y011 Default values for typed arguments must be "..."
def f5(x=3) -> None: ... # Y014 Default values for arguments must be "..."
def f4(*, x: complex = 3j) -> None: ...
def f401(*, x: complex = 5 + 3j) -> None: ...
def f402(*, x: complex = -42 - 42j) -> None: ...
def f405(*, x: int = 3 * 3) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f406(*, x: int = -3 >> 3) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f5(x=3) -> None: ...
def f6(*, x: int) -> None: ...
def f7(x, y: int = 3) -> None: ... # Y011 Default values for typed arguments must be "..."
def f7(x, y: int = 3) -> None: ...
def f8(x, y: int = ...) -> None: ...
def f9(x, y: str = "x") -> None: ... # Y011 Default values for typed arguments must be "..."
def f9(x, y: str = "x") -> None: ...
def f901(x, y: str = os.pathsep) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f10(x, y: str = ..., *args: "int") -> None: ... # Y020 Quoted annotations should never be used in stubs
def f11(*, x: str = "x") -> None: ... # Y011 Default values for typed arguments must be "..."
def f11(*, x: str = "x") -> None: ...
def f12(*, x: str = ..., **kwargs: "int") -> None: ... # Y020 Quoted annotations should never be used in stubs
def f13(x: list[str] = ["foo", "bar", "baz"]) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f14(x: tuple[str, ...] = ("foo", "bar", "baz")) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f15(x: set[str] = {"foo", "bar", "baz"}) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f16(x: frozenset[bytes] = frozenset({b"foo", b"bar", b"baz"})) -> None: ... # Y011 Only simple default values allowed for typed arguments
5 changes: 4 additions & 1 deletion tests/defaults_py38.pyi
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import os

def f(x: "int", /) -> None: ... # Y020 Quoted annotations should never be used in stubs
def f1(x: int, /) -> None: ...
def f2(x: int, /, y: "int") -> None: ... # Y020 Quoted annotations should never be used in stubs
def f3(x: str = "y", /) -> None: ... # Y011 Default values for typed arguments must be "..."
def f3(x: str = "y", /) -> None: ...
def f4(x: str = os.pathsep, /) -> None: ... # Y011 Only simple default values allowed for typed arguments
10 changes: 5 additions & 5 deletions tests/quotes.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ __all__ += ["h"]
__all__.extend(["i"])
__all__.append("j")
__all__.remove("j")
__match_args__ = ('foo',) # Y020 Quoted annotations should never be used in stubs
__slots__ = ('foo',) # Y020 Quoted annotations should never be used in stubs
__match_args__ = ('foo',) # Y015 Only simple default values are allowed for assignments # Y020 Quoted annotations should never be used in stubs
__slots__ = ('foo',) # Y015 Only simple default values are allowed for assignments # Y020 Quoted annotations should never be used in stubs

def f(x: "int"): ... # Y020 Quoted annotations should never be used in stubs
def g(x: list["int"]): ... # Y020 Quoted annotations should never be used in stubs
Expand All @@ -26,7 +26,7 @@ Alias: TypeAlias = list["int"] # Y020 Quoted annotations should never be used i
class Child(list["int"]): # Y020 Quoted annotations should never be used in stubs
"""Documented and guaranteed useful.""" # Y021 Docstrings should not be included in stubs

__all__ = ('foo',) # Y020 Quoted annotations should never be used in stubs
__all__ = ('foo',) # Y015 Only simple default values are allowed for assignments # Y020 Quoted annotations should never be used in stubs
__match_args__ = ('foo', 'bar')
__slots__ = ('foo', 'bar')

Expand Down Expand Up @@ -57,8 +57,8 @@ class DocstringAndPass:
pass # Y012 Class body must not contain "pass"

# These two shouldn't trigger Y020 -- empty strings can't be "quoted annotations"
k = "" # Y015 Bad default value. Use "k = ..." instead of "k = ''"
el = r"" # Y015 Bad default value. Use "el = ..." instead of "el = ''"
k = ""
el = r""

# The following should also pass,
# But we can't test for it in CI, because the error message is *very* slightly different on 3.7
Expand Down