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

Allow simple container literals as default values #358

Merged
merged 3 commits into from
Mar 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ New error codes:
it now applies everywhere.

Other changes:
* Y011/Y014/Y015: Simple container literals (`list`, `dict`, `tuple` and `set`
literals) are now allowed as default values.
* Y052 is now emitted more consistently.
* Some things that used to result in Y011, Y014 or Y015 being emitted
now result in Y053 or Y054 being emitted.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ currently emitted:
| Y008 | Unrecognized platform in a `sys.platform` check. 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 class or function 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 | 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. Also use `...` for very long numbers, very long strings, very long bytes, or defaults that vary according to the machine Python is being run on.
| Y011 | Only simple default values (`int`, `float`, `complex`, `bytes`, `str`, `bool`, `None`, `...`, or simple container literals) 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. Also use `...` for very long numbers, very long strings, very long bytes, or defaults that vary according to the machine Python is being run on.
| Y012 | Class body must not contain `pass`.
| Y013 | Non-empty class body must not contain `...`.
| Y014 | Only simple default values are allowed for any function arguments. A stronger version of Y011 that includes arguments without type annotations.
Expand Down
31 changes: 30 additions & 1 deletion pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -719,13 +719,42 @@ def _analyse_union(members: Sequence[ast.expr]) -> UnionAnalysis:
)


def _is_valid_default_value_with_annotation(node: ast.expr) -> bool:
def _is_valid_default_value_with_annotation(
node: ast.expr, allow_containers=True
) -> bool:
"""Is `node` valid as a default value for a function or method parameter in a stub?

Note that this function is *also* used to determine
the validity of default values for ast.AnnAssign nodes.
(E.g. `foo: int = 5` is OK, but `foo: TypeVar = TypeVar("foo")` is not.)
"""
# lists, tuples, sets
if isinstance(node, (ast.List, ast.Tuple, ast.Set)):
return (
allow_containers
and len(node.elts) <= 10
and all(
_is_valid_default_value_with_annotation(elt, allow_containers=False)
for elt in node.elts
)
)

# dicts
if isinstance(node, ast.Dict):
return (
allow_containers
and len(node.keys) <= 10
and all(
(
subnode is not None
and _is_valid_default_value_with_annotation(
subnode, allow_containers=False
)
)
for subnode in chain(node.keys, node.values)
)
)

# `...`, bools, None, str, bytes,
# positive ints, positive floats, positive complex numbers with no real part
if isinstance(node, (ast.Ellipsis, ast.NameConstant, ast.Str, ast.Bytes, ast.Num)):
Expand Down
34 changes: 26 additions & 8 deletions tests/attribute_annotations.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ field9 = None # Y026 Use typing_extensions.TypeAlias for type aliases, e.g. "fi
Field95: TypeAlias = None
Field96: TypeAlias = int | None
Field97: TypeAlias = None | typing.SupportsInt | builtins.str | float | bool
field19 = [1, 2, 3] # Y052 Need type annotation for "field19"
field191: list[int] = [1, 2, 3]
field20 = (1, 2, 3) # Y052 Need type annotation for "field20"
field201: tuple[int, ...] = (1, 2, 3)
field21 = {1, 2, 3} # Y052 Need type annotation for "field21"
field211: set[int] = {1, 2, 3}
field212 = {"foo": "bar"} # Y052 Need type annotation for "field212"
field213: dict[str, str] = {"foo": "bar"}
field22: Final = {"foo": 5}

# Tests for Final
field11: Final = 1
Expand All @@ -47,10 +56,16 @@ field182: Final = os.pathsep
field183: Final = None

# 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
field22: Final = {"foo": 5} # Y015 Only simple default values are allowed for assignments
field221: list[int] = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] # Y015 Only simple default values are allowed for assignments
field222: list[int] = [100000000000000000000000000000] # Y054 Numeric literals with a string representation >10 characters long are not permitted
field223: list[int] = [*range(10)] # Y015 Only simple default values are allowed for assignments
field224: list[int] = list(range(10)) # Y015 Only simple default values are allowed for assignments
field225: list[object] = [{}, 1, 2] # Y015 Only simple default values are allowed for assignments
field226: tuple[str | tuple[str, ...], ...] = ("foo", ("foo", "bar")) # Y015 Only simple default values are allowed for assignments
field227: dict[str, object] = {"foo": {"foo": "bar"}} # Y015 Only simple default values are allowed for assignments
Comment on lines +64 to +65
Copy link
Collaborator

@srittau srittau Mar 15, 2023

Choose a reason for hiding this comment

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

I think these are decent examples, why we shouldn't forbid nesting. It's extra code and might occasionally be useful. If it becomes too complex, a manual reviewer can always point this out.

That said, this is not a show stopper.

Copy link
Collaborator Author

@AlexWaygood AlexWaygood Mar 15, 2023

Choose a reason for hiding this comment

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

I thought you might say that :)

I guess my worry is that we've added most of our defaults up till now using stubdefaulter, which can result in a huge PR that we can't really review manually. I wouldn't really want a default such as {(("foo", "bar"), (1, 2): {("foo", "bar"): {1, 2, 3}}} added automatically by stubdefaulter in some bizarre third-party library to slip through human review by being bundled in a huge PR. But I guess that's pretty unlikely; I don't feel too strongly about disallowing nesting.

field228: dict[str, list[object]] = {"foo": []} # Y015 Only simple default values are allowed for assignments
# When parsed, this case results in `None` being placed in the `.keys` list for the `ast.Dict` node
field229: dict[int, int] = {1: 2, **{3: 4}} # Y015 Only simple default values are allowed for assignments
field23 = "foo" + "bar" # Y015 Only simple default values are allowed for assignments
field24 = b"foo" + b"bar" # Y015 Only simple default values are allowed for assignments
field25 = 5 * 5 # Y015 Only simple default values are allowed for assignments
Expand Down Expand Up @@ -91,10 +106,13 @@ class Foo:
else:
field19 = "w" # Y052 Need type annotation for "field19"

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
field23: Final = {"foo": 5} # Y015 Only simple default values are allowed for assignments
field20 = [1, 2, 3] # Y052 Need type annotation for "field20"
field201: list[int] = [1, 2, 3]
field21 = (1, 2, 3) # Y052 Need type annotation for "field21"
field211: tuple[int, ...] = (1, 2, 3)
field22 = {1, 2, 3} # Y052 Need type annotation for "field22"
field221: set[int] = {1, 2, 3}
field23: Final = {"foo": 5}
field24 = "foo" + "bar" # Y015 Only simple default values are allowed for assignments
field25 = b"foo" + b"bar" # Y015 Only simple default values are allowed for assignments
field26 = 5 * 5 # Y015 Only simple default values are allowed for assignments
Expand Down
13 changes: 10 additions & 3 deletions tests/defaults.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,16 @@ def f901(x, y: str = os.pathsep) -> None: ... # Y011 Only simple default values
def f10(x, y: str = ..., *args: "int") -> None: ... # Y020 Quoted annotations should never be used in stubs
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 f13(x: list[str] = ["foo", "bar", "baz"]) -> None: ...
def f14(x: tuple[str, ...] = ("foo", "bar", "baz")) -> None: ...
def f15(x: set[str] = {"foo", "bar", "baz"}) -> None: ...
def f151(x: dict[int, int] = {1: 2}) -> None: ...
# When parsed, this case results in `None` being placed in the `.keys` list for the `ast.Dict` node
def f152(x: dict[int, int] = {1: 2, **{3: 4}}) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f153(x: list[int] = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f154(x: tuple[str, ...] = ("foo", ("bar", "baz"))) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f141(x: list[int] = [*range(10)]) -> None: ... # Y011 Only simple default values allowed for typed arguments
def f142(x: list[int] = list(range(10))) -> 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
def f17(x: str = "foo" + "bar") -> None: ... # Y011 Only simple default values allowed for typed arguments
def f18(x: str = b"foo" + b"bar") -> None: ... # Y011 Only simple default values allowed for typed arguments
Expand Down
6 changes: 3 additions & 3 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',) # Y015 Only simple default values are allowed for assignments
__slots__ = ('foo',) # Y015 Only simple default values are allowed for assignments
__match_args__ = ('foo',) # Y052 Need type annotation for "__match_args__"
__slots__ = ('foo',) # Y052 Need type annotation for "__slots__"

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',) # Y015 Only simple default values are allowed for assignments
__all__ = ('foo',) # Y052 Need type annotation for "__all__"
__match_args__ = ('foo', 'bar')
__slots__ = ('foo', 'bar')

Expand Down