Skip to content

Commit

Permalink
Improve handling of long numbers, strings and bytes (#351)
Browse files Browse the repository at this point in the history
Currently, we allow this:

```py
from typing_extensions import Literal
foo: Literal[1000000000000000000000000000000000000000]
```

But we disallow this:

```python
from typing_extensions import Final
foo: Final = 1000000000000000000000000000000000000000
```

This seems pretty inconsistent; the rationale for disallowing the latter
applies equally well to the former. Fixing this inconsistency also
allows us to clean up the code somewhat.
  • Loading branch information
AlexWaygood committed Mar 14, 2023
1 parent 69df360 commit f611897
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 62 deletions.
15 changes: 15 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,20 @@
# Change Log

## Unreleased

New error codes:
* Y053: Disallow string or bytes literals with length >50 characters.
Previously this rule only applied to parameter default values;
it now applies everywhere.
* Y054: Disallow numeric literals with a string representation >10 characters long.
Previously this rule only applied to parameter default values;
it now applies everywhere.

Other changes:
* 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.

## 23.3.0

* Y011/Y014/Y015: Allow `math` constants `math.inf`, `math.nan`, `math.e`,
Expand Down
2 changes: 2 additions & 0 deletions README.md
Expand Up @@ -85,6 +85,8 @@ currently emitted:
| Y050 | Prefer `typing_extensions.Never` over `typing.NoReturn` for argument annotations. This is a purely stylistic choice in the name of readability.
| Y051 | Y051 detects redundant unions between `Literal` types and builtin supertypes. For example, `Literal[5]` is redundant in the union `int \| Literal[5]`, and `Literal[True]` is redundant in the union `Literal[True] \| bool`.
| Y052 | Y052 disallows assignments to constant values where the assignment does not have a type annotation. For example, `x = 0` in the global namespace is ambiguous in a stub, as there are four different types that could be inferred for the variable `x`: `int`, `Final[int]`, `Literal[0]`, or `Final[Literal[0]]`. Enum members are excluded from this check, as are various special assignments such as `__all__` and `__match_args__`.
| Y053 | Only string and bytes literals <=50 characters long are permitted.
| Y054 | Only numeric literals with a string representation <=10 characters long are permitted.

Note that several error codes recommend using types from `typing_extensions` or
`_typeshed`. Strictly speaking, these packages are not part of the standard
Expand Down
113 changes: 61 additions & 52 deletions pyi.py
Expand Up @@ -726,51 +726,31 @@ def _is_valid_default_value_with_annotation(node: ast.expr) -> bool:
the validity of default values for ast.AnnAssign nodes.
(E.g. `foo: int = 5` is OK, but `foo: TypeVar = TypeVar("foo")` is not.)
"""
# `...`, bools, None
if isinstance(node, (ast.Ellipsis, ast.NameConstant)):
# `...`, 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)):
return True

# strings, bytes
if isinstance(node, (ast.Str, ast.Bytes)):
return len(str(node.s)) <= 50

def _is_valid_Num(node: ast.expr) -> TypeGuard[ast.Num]:
# The maximum character limit is arbitrary, but here's what it's based on:
# Hex representation of 32-bit integers tend to be 10 chars.
# So is the decimal representation of the maximum positive signed 32-bit integer.
# 0xFFFFFFFF --> 4294967295
return isinstance(node, ast.Num) and len(str(node.n)) <= 10

def _is_valid_math_constant(
node: ast.expr, allow_nan: bool = True
) -> TypeGuard[ast.Attribute]:
# math.inf, math.nan, math.e, math.pi, math.tau
return (
isinstance(node, ast.Attribute)
and isinstance(node.value, ast.Name)
and f"{node.value.id}.{node.attr}" in _ALLOWED_MATH_ATTRIBUTES_IN_DEFAULTS
and (allow_nan or f"{node.value.id}.{node.attr}" != "math.nan")
)
# Negative ints, negative floats, negative complex numbers with no real part,
# some constants from the math module
if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.USub):
if isinstance(node.operand, ast.Num):
return True
if isinstance(node.operand, ast.Attribute) and isinstance(
node.operand.value, ast.Name
):
fullname = f"{node.operand.value.id}.{node.operand.attr}"
return (
fullname in _ALLOWED_MATH_ATTRIBUTES_IN_DEFAULTS
and fullname != "math.nan"
)
return False

# Positive ints, positive floats, positive complex numbers with no real part, math constants
if _is_valid_Num(node) or _is_valid_math_constant(node):
return True
# Negative ints, negative floats, negative complex numbers with no real part, math constants
if (
isinstance(node, ast.UnaryOp)
and isinstance(node.op, ast.USub)
and (
_is_valid_Num(node.operand)
# Don't allow -math.nan
or _is_valid_math_constant(node.operand, allow_nan=False)
)
):
return True
# Complex numbers with a real part and an imaginary part...
if (
isinstance(node, ast.BinOp)
and isinstance(node.op, (ast.Add, ast.Sub))
and _is_valid_Num(node.right)
and isinstance(node.right, ast.Num)
and type(node.right.n) is complex
):
left = node.left
Expand All @@ -781,17 +761,19 @@ def _is_valid_math_constant(
if (
isinstance(left, ast.UnaryOp)
and isinstance(left.op, ast.USub)
and _is_valid_Num(left.operand)
and isinstance(left.operand, ast.Num)
and type(left.operand.n) is not complex
):
return True
return False

# Special cases
if (
isinstance(node, ast.Attribute)
and isinstance(node.value, ast.Name)
and f"{node.value.id}.{node.attr}" in _ALLOWED_ATTRIBUTES_IN_DEFAULTS
):
return True
if isinstance(node, ast.Attribute) and isinstance(node.value, ast.Name):
fullname = f"{node.value.id}.{node.attr}"
return (fullname in _ALLOWED_ATTRIBUTES_IN_DEFAULTS) or (
fullname in _ALLOWED_MATH_ATTRIBUTES_IN_DEFAULTS
)

return False


Expand Down Expand Up @@ -1130,20 +1112,42 @@ def visit_Call(self, node: ast.Call) -> None:
for kw in node.keywords:
self.visit(kw)

def _check_for_Y053(self, node: ast.Constant | ast.Str | ast.Bytes) -> None:
if len(node.s) > 50:
self.error(node, Y053)

def _check_for_Y054(self, node: ast.Constant | ast.Num) -> None:
# The maximum character limit is arbitrary, but here's what it's based on:
# Hex representation of 32-bit integers tend to be 10 chars.
# So is the decimal representation of the maximum positive signed 32-bit integer.
# 0xFFFFFFFF --> 4294967295
if len(str(node.n)) > 10:
self.error(node, Y054)

# 3.8+
def visit_Constant(self, node: ast.Constant) -> None:
if (
isinstance(node.value, str)
and node.value
and not self.string_literals_allowed.active
):
if isinstance(node.value, str) and not self.string_literals_allowed.active:
self.error(node, Y020)
elif isinstance(node.value, (str, bytes)):
self._check_for_Y053(node)
elif isinstance(node.value, (int, float, complex)):
self._check_for_Y054(node)

# 3.7 and lower
# 3.7
def visit_Str(self, node: ast.Str) -> None:
if node.s and not self.string_literals_allowed.active:
if self.string_literals_allowed.active:
self._check_for_Y053(node)
else:
self.error(node, Y020)

# 3.7
def visit_Bytes(self, node: ast.Bytes) -> None:
self._check_for_Y053(node)

# 3.7
def visit_Num(self, node: ast.Num) -> None:
self._check_for_Y054(node)

def visit_Expr(self, node: ast.Expr) -> None:
if isinstance(node.value, ast.Str):
self.error(node, Y021)
Expand Down Expand Up @@ -2047,3 +2051,8 @@ def parse_options(
)
Y051 = 'Y051 "{literal_subtype}" is redundant in a union with "{builtin_supertype}"'
Y052 = 'Y052 Need type annotation for "{variable}"'
Y053 = "Y053 String and bytes literals >50 characters long are not permitted"
Y054 = (
"Y054 Numeric literals with a string representation "
">10 characters long are not permitted"
)
16 changes: 8 additions & 8 deletions tests/attribute_annotations.pyi
Expand Up @@ -54,10 +54,10 @@ field22: Final = {"foo": 5} # Y015 Only simple default values are allowed for a
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
field26: int = 0xFFFFFFFFF # Y015 Only simple default values are allowed for assignments
field27: int = 12345678901 # Y015 Only simple default values are allowed for assignments
field28: int = -0xFFFFFFFFF # Y015 Only simple default values are allowed for assignments
field29: int = -12345678901 # Y015 Only simple default values are allowed for assignments
field26: int = 0xFFFFFFFFF # Y054 Numeric literals with a string representation >10 characters long are not permitted
field27: int = 12345678901 # Y054 Numeric literals with a string representation >10 characters long are not permitted
field28: int = -0xFFFFFFFFF # Y054 Numeric literals with a string representation >10 characters long are not permitted
field29: int = -12345678901 # Y054 Numeric literals with a string representation >10 characters long are not permitted

class Foo:
field1: int
Expand Down Expand Up @@ -98,10 +98,10 @@ class Foo:
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
field27 = 0xFFFFFFFFF # Y015 Only simple default values are allowed for assignments
field28 = 12345678901 # Y015 Only simple default values are allowed for assignments
field29 = -0xFFFFFFFFF # Y015 Only simple default values are allowed for assignments
field30 = -12345678901 # Y015 Only simple default values are allowed for assignments
field27 = 0xFFFFFFFFF # Y052 Need type annotation for "field27" # Y054 Numeric literals with a string representation >10 characters long are not permitted
field28 = 12345678901 # Y052 Need type annotation for "field28" # Y054 Numeric literals with a string representation >10 characters long are not permitted
field29 = -0xFFFFFFFFF # Y052 Need type annotation for "field29" # Y054 Numeric literals with a string representation >10 characters long are not permitted
field30 = -12345678901 # Y052 Need type annotation for "field30" # Y054 Numeric literals with a string representation >10 characters long are not permitted

Field95: TypeAlias = None
Field96: TypeAlias = int | None
Expand Down
7 changes: 5 additions & 2 deletions tests/defaults.pyi
Expand Up @@ -62,5 +62,8 @@ def f34(x: str = sys.version) -> None: ...
def f35(x: tuple[int, ...] = sys.version_info) -> None: ...
def f36(x: str = sys.winver) -> None: ...

def f37(x: str = "a_very_long_stringgggggggggggggggggggggggggggggggggggggggggggggg") -> None: ... # Y011 Only simple default values allowed for typed arguments
def f38(x: bytes = b"a_very_long_byte_stringggggggggggggggggggggggggggggggggggggg") -> None: ... # Y011 Only simple default values allowed for typed arguments
def f37(x: str = "a_very_long_stringgggggggggggggggggggggggggggggggggggggggggggggg") -> None: ... # Y053 String and bytes literals >50 characters long are not permitted
def f38(x: bytes = b"a_very_long_byte_stringggggggggggggggggggggggggggggggggggggg") -> None: ... # Y053 String and bytes literals >50 characters long are not permitted

foo: str = "a_very_long_stringgggggggggggggggggggggggggggggggggggggggggggggg" # Y053 String and bytes literals >50 characters long are not permitted
bar: bytes = b"a_very_long_byte_stringggggggggggggggggggggggggggggggggggggg" # Y053 String and bytes literals >50 characters long are not permitted

0 comments on commit f611897

Please sign in to comment.