gh-104050: Improve some typing around `default`s and sentinel values (#104626)

- Convert `unspecified` and `unknown` to be members of a `Sentinels` enum, rather than instances of bespoke classes.
  - An enum feels more idiomatic here, and works better with type checkers.
  - Convert some `==` and `!=` checks for these values to identity checks, which are more idiomatic with sentinels.
  - _Don't_ do the same for `Null`, as this needs to be a distinct type due to its usage in `clinic.py`.
- Use `object` as the annotation for `default` across `clinic.py`. `default` can be literally any object, so `object` is the correct annotation here.

---

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
This commit is contained in:
Alex Waygood 2023-05-18 22:58:42 +01:00 committed by GitHub
parent 61027c0211
commit 1c55e8d007
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 24 additions and 19 deletions

View File

@ -12,6 +12,7 @@ import collections
import contextlib import contextlib
import copy import copy
import cpp import cpp
import enum
import functools import functools
import hashlib import hashlib
import inspect import inspect
@ -28,7 +29,7 @@ import traceback
from collections.abc import Callable from collections.abc import Callable
from types import FunctionType, NoneType from types import FunctionType, NoneType
from typing import Any, NamedTuple, NoReturn, Literal, overload from typing import Any, Final, NamedTuple, NoReturn, Literal, overload
# TODO: # TODO:
# #
@ -58,26 +59,27 @@ CLINIC_PREFIXED_ARGS = {
"return_value", "return_value",
} }
class Unspecified:
class Sentinels(enum.Enum):
unspecified = "unspecified"
unknown = "unknown"
def __repr__(self) -> str: def __repr__(self) -> str:
return '<Unspecified>' return f"<{self.value.capitalize()}>"
unspecified = Unspecified()
unspecified: Final = Sentinels.unspecified
unknown: Final = Sentinels.unknown
# This one needs to be a distinct class, unlike the other two
class Null: class Null:
def __repr__(self) -> str: def __repr__(self) -> str:
return '<Null>' return '<Null>'
NULL = Null() NULL = Null()
class Unknown:
def __repr__(self) -> str:
return '<Unknown>'
unknown = Unknown()
sig_end_marker = '--' sig_end_marker = '--'
Appender = Callable[[str], None] Appender = Callable[[str], None]
@ -2600,7 +2602,7 @@ class CConverter(metaclass=CConverterAutoRegister):
# Or the magic value "unknown" if this value is a cannot be evaluated # Or the magic value "unknown" if this value is a cannot be evaluated
# at Argument-Clinic-preprocessing time (but is presumed to be valid # at Argument-Clinic-preprocessing time (but is presumed to be valid
# at runtime). # at runtime).
default: bool | Unspecified = unspecified default: object = unspecified
# If not None, default must be isinstance() of this type. # If not None, default must be isinstance() of this type.
# (You can also specify a tuple of types.) # (You can also specify a tuple of types.)
@ -2686,11 +2688,11 @@ class CConverter(metaclass=CConverterAutoRegister):
name: str, name: str,
py_name: str, py_name: str,
function, function,
default=unspecified, default: object = unspecified,
*, # Keyword only args: *, # Keyword only args:
c_default: str | None = None, c_default: str | None = None,
py_default: str | None = None, py_default: str | None = None,
annotation: str | Unspecified = unspecified, annotation: str | Literal[Sentinels.unspecified] = unspecified,
unused: bool = False, unused: bool = False,
**kwargs **kwargs
): ):
@ -2699,7 +2701,10 @@ class CConverter(metaclass=CConverterAutoRegister):
self.unused = unused self.unused = unused
if default is not unspecified: if default is not unspecified:
if self.default_type and not isinstance(default, (self.default_type, Unknown)): if (self.default_type
and default is not unknown
and not isinstance(default, self.default_type)
):
if isinstance(self.default_type, type): if isinstance(self.default_type, type):
types_str = self.default_type.__name__ types_str = self.default_type.__name__
else: else:
@ -2713,7 +2718,7 @@ class CConverter(metaclass=CConverterAutoRegister):
if py_default: if py_default:
self.py_default = py_default self.py_default = py_default
if annotation != unspecified: if annotation is not unspecified:
fail("The 'annotation' parameter is not currently permitted.") fail("The 'annotation' parameter is not currently permitted.")
# this is deliberate, to prevent you from caching information # this is deliberate, to prevent you from caching information
@ -3967,7 +3972,7 @@ class CReturnConverter(metaclass=CReturnConverterAutoRegister):
# The Python default value for this parameter, as a Python value. # The Python default value for this parameter, as a Python value.
# Or the magic value "unspecified" if there is no default. # Or the magic value "unspecified" if there is no default.
default = None default: object = None
def __init__(self, *, py_default=None, **kwargs): def __init__(self, *, py_default=None, **kwargs):
self.py_default = py_default self.py_default = py_default
@ -4767,7 +4772,7 @@ class DSLParser:
# but at least make an attempt at ensuring it's a valid expression. # but at least make an attempt at ensuring it's a valid expression.
try: try:
value = eval(default) value = eval(default)
if value == unspecified: if value is unspecified:
fail("'unspecified' is not a legal default value!") fail("'unspecified' is not a legal default value!")
except NameError: except NameError:
pass # probably a named constant pass # probably a named constant