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

gh-104050: Run mypy on clinic.py in CI #104421

Merged
merged 16 commits into from
May 15, 2023
Merged
32 changes: 32 additions & 0 deletions .github/workflows/mypy-clinic.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: Run mypy on Tools/clinic
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved

on:
push:
branches:
- main
pull_request:
paths:
- "Tools/clinic/**/*"
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
- ".github/workflows/mypy-clinic.yml"
hugovk marked this conversation as resolved.
Show resolved Hide resolved
workflow_dispatch:

permissions:
contents: read

concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true

jobs:
mypy:
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: "3.11"
cache: pip
cache-dependency-path: .github/workflows/mypy-clinic.yml
- run: pip install mypy==1.3.0
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
- run: mypy --config-file Tools/clinic/mypy.ini
39 changes: 25 additions & 14 deletions Tools/clinic/clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
import traceback
import types

from collections.abc import Callable
from types import *
hugovk marked this conversation as resolved.
Show resolved Hide resolved
from typing import Any, NamedTuple, Type

# TODO:
#
Expand Down Expand Up @@ -67,19 +69,26 @@ def __repr__(self):

sig_end_marker = '--'

Appender = Callable[[str], None]
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
Outputter = Callable[[None], str]

_text_accumulator_nt = collections.namedtuple("_text_accumulator", "text append output")
class _TextAccumulator(NamedTuple):
text: list[str]
append: Appender
output: Outputter

def _text_accumulator():
text = []
def output():
s = ''.join(text)
text.clear()
return s
return _text_accumulator_nt(text, text.append, output)
return _TextAccumulator(text, text.append, output)


text_accumulator_nt = collections.namedtuple("text_accumulator", "text append")
class TextAccumulator(NamedTuple):
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
text: list[str]
append: Appender

def text_accumulator():
"""
Expand All @@ -93,7 +102,7 @@ def text_accumulator():
empties the accumulator.
"""
text, append, output = _text_accumulator()
return text_accumulator_nt(append, output)
return TextAccumulator(append, output)


def warn_or_fail(fail=False, *args, filename=None, line_number=None):
Expand Down Expand Up @@ -1915,8 +1924,10 @@ def dump(self):
# maps strings to Language objects.
# "languages" maps the name of the language ("C", "Python").
# "extensions" maps the file extension ("c", "py").
LangDict = dict[str, Callable[[str], Language]]

languages = { 'C': CLanguage, 'Python': PythonLanguage }
extensions = { name: CLanguage for name in "c cc cpp cxx h hh hpp hxx".split() }
extensions: LangDict = { name: CLanguage for name in "c cc cpp cxx h hh hpp hxx".split() }
extensions['py'] = PythonLanguage


Expand Down Expand Up @@ -2558,15 +2569,15 @@ class CConverter(metaclass=CConverterAutoRegister):
"""

# The C name to use for this variable.
name = None
name: str | None = None
Copy link
Contributor

@erlend-aasland erlend-aasland May 16, 2023

Choose a reason for hiding this comment

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

So, these str | None are now starting to cause frustrations, as I gradually add typing to other parts of clinic.py. Since there are no guards anywhere, I obviously get errors and warnings about str plus None, etc. We could change the defaults to the empty string, but that can also create a lot of havoc.


# The Python name to use for this variable.
py_name = None
py_name: str | None = None

# The C type to use for this variable.
# 'type' should be a Python string specifying the type, e.g. "int".
# If this is a pointer type, the type string should end with ' *'.
type = None
type: str | None = None

# The Python default value for this parameter, as a Python value.
# Or the magic value "unspecified" if there is no default.
Expand All @@ -2577,15 +2588,15 @@ class CConverter(metaclass=CConverterAutoRegister):

# If not None, default must be isinstance() of this type.
# (You can also specify a tuple of types.)
default_type = None
default_type: Type[Any] | tuple[Type[Any], ...] | None = None
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved

# "default" converted into a C value, as a string.
# Or None if there is no default.
c_default = None
c_default: str | None = None

# "default" converted into a Python value, as a string.
# Or None if there is no default.
py_default = None
py_default: str | None = None

# The default value used to initialize the C variable when
# there is no default, but not specifying a default may
Expand All @@ -2597,11 +2608,11 @@ class CConverter(metaclass=CConverterAutoRegister):
#
# This value is specified as a string.
# Every non-abstract subclass should supply a valid value.
c_ignored_default = 'NULL'
c_ignored_default: str = 'NULL'

# The C converter *function* to be used, if any.
# (If this is not None, format_unit must be 'O&'.)
converter = None
converter: str | None = None

# Should Argument Clinic add a '&' before the name of
# the variable when passing it into the _impl function?
Expand Down Expand Up @@ -3414,7 +3425,7 @@ class robuffer: pass
def str_converter_key(types, encoding, zeroes):
return (frozenset(types), bool(encoding), bool(zeroes))

str_converter_argument_map = {}
str_converter_argument_map: dict[str, str] = {}

class str_converter(CConverter):
type = 'const char *'
Expand Down
27 changes: 17 additions & 10 deletions Tools/clinic/cpp.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import re
import sys
from typing import NewType
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
from collections.abc import Callable

def negate(condition):

TokenAndCondition = tuple[str, str]
TokenStack = list[TokenAndCondition]

def negate(condition: str) -> str:
"""
Returns a CPP conditional that is the opposite of the conditional passed in.
"""
Expand All @@ -22,28 +28,29 @@ class Monitor:
Anyway this implementation seems to work well enough for the CPython sources.
"""

is_a_simple_defined: Callable[[str], re.Match[str] | None]
is_a_simple_defined = re.compile(r'^defined\s*\(\s*[A-Za-z0-9_]+\s*\)$').match

def __init__(self, filename=None, *, verbose=False):
self.stack = []
def __init__(self, filename=None, *, verbose: bool = False):
self.stack: TokenStack = []
self.in_comment = False
self.continuation = None
self.continuation: str | None = None
self.line_number = 0
self.filename = filename
self.verbose = verbose

def __repr__(self):
def __repr__(self) -> str:
return ''.join((
'<Monitor ',
str(id(self)),
" line=", str(self.line_number),
" condition=", repr(self.condition()),
">"))

def status(self):
def status(self) -> str:
return str(self.line_number).rjust(4) + ": " + self.condition()

def condition(self):
def condition(self) -> str:
"""
Returns the current preprocessor state, as a single #if condition.
"""
Expand All @@ -62,15 +69,15 @@ def close(self):
if self.stack:
self.fail("Ended file while still in a preprocessor conditional block!")

def write(self, s):
def write(self, s: str):
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
for line in s.split("\n"):
self.writeline(line)

def writeline(self, line):
def writeline(self, line: str):
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
self.line_number += 1
line = line.strip()

def pop_stack():
def pop_stack() -> TokenAndCondition:
if not self.stack:
self.fail("#" + token + " without matching #if / #ifdef / #ifndef!")
return self.stack.pop()
Expand Down
9 changes: 9 additions & 0 deletions Tools/clinic/mypy.ini
Copy link
Member Author

Choose a reason for hiding this comment

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

I usually put my mypy config in a pyproject.toml file these days, but a mypy.ini file feels like it makes more "sense" unless we're planning on adding other linters/formatters to check clinic.py in the future

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[mypy]
pretty = True
enable_error_code = ignore-without-code
disallow_any_generics = True
strict_concatenate = True
warn_redundant_casts = True
warn_unused_ignores = True
warn_unused_configs = True
files = Tools/clinic/