[PATCH v6 00/36] qapi: static typing conversion, pt1

John Snow posted 36 patches 3 years, 6 months ago
Test checkpatch passed
Failed in applying to current master (apply log)
docs/devel/multi-thread-tcg.rst |   2 +-
docs/devel/testing.rst          |   2 +-
scripts/qapi-gen.py             |  57 ++--------
scripts/qapi/.flake8            |   2 +
scripts/qapi/.isort.cfg         |   7 ++
scripts/qapi/commands.py        |  90 +++++++++++-----
scripts/qapi/common.py          | 164 ++++++++++++++++-------------
scripts/qapi/events.py          |  58 +++++++---
scripts/qapi/expr.py            |   7 +-
scripts/qapi/gen.py             | 180 ++++++++++++++++++++------------
scripts/qapi/introspect.py      |  16 ++-
scripts/qapi/main.py            |  95 +++++++++++++++++
scripts/qapi/mypy.ini           |  30 ++++++
scripts/qapi/parser.py          |   6 +-
scripts/qapi/pylintrc           |  70 +++++++++++++
scripts/qapi/schema.py          |  33 +++---
scripts/qapi/source.py          |  35 ++++---
scripts/qapi/types.py           | 125 +++++++++++++++-------
scripts/qapi/visit.py           | 116 ++++++++++++++------
19 files changed, 759 insertions(+), 336 deletions(-)
create mode 100644 scripts/qapi/.flake8
create mode 100644 scripts/qapi/.isort.cfg
create mode 100644 scripts/qapi/main.py
create mode 100644 scripts/qapi/mypy.ini
create mode 100644 scripts/qapi/pylintrc
[PATCH v6 00/36] qapi: static typing conversion, pt1
Posted by John Snow 3 years, 6 months ago
Hi, this series adds static type hints to the QAPI module.
This is part one!

Part 1: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt1
Everything: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt6

- Requires Python 3.6+
- Requires mypy 0.770 or newer (for type analysis only)
- Requires pylint 2.6.0 or newer (for lint checking only)

In general, this series tackles the cleanup of one individual QAPI
module at a time. Once it passes pylint or mypy checks, those checks are
enabled for that file.

Type hints are added in patches that add *only* type hints and change no
other behavior. Any necessary changes to behavior to accommodate typing
are split out into their own tiny patches.

Notes:

- After patch 07, `isort -c` should pass 100% on this and every
  future commit.

- After patch 08, `flake8 qapi/` should pass 100% on this and every
  future commit.

- After patch 09, `pylint --rcfile=qapi/pylintrc qapi/` should pass 100%
  on this and every future commit.

- After patch 18, `mypy --config-file=qapi/mypy.ini qapi/` should pass
  100% on this and every future commit.

Review Status:

[01] docs-repair-broken-references  #
[02] qapi-modify-docstrings-to-be   #
[03] qapi-gen-separate-arg-parsing  # [RB] CR,EH [TB] CR [SOB] JS
[04] qapi-move-generator-entrypoint # [RB] CR,EH [TB] CR [SOB] JS
[05] qapi-prefer-explicit-relative  # [RB] CR,EH,PM [SOB] JS
[06] qapi-remove-wildcard-includes  # [RB] CR,EH,PM [SOB] JS
[07] qapi-enforce-import-order      # [RB] CR,MA [TB] CR [SOB] JS
[08] qapi-delint-using-flake8       # [RB] CR,EH [SOB] JS
[09] qapi-add-pylintrc              # [RB] CR [TB] CR,EH [SOB] JS
[10] qapi-common-py-remove-python   # [RB] CR,EH [SOB] JS
[11] qapi-common-add-indent-manager # [RB] CR,EH [SOB] JS
[12] qapi-common-py-delint-with     # [RB] CR,EH [SOB] JS
[13] replace-c-by-char              # [RB] CR,EH,PM [SOB] JS
[14] qapi-common-py-check-with      # [RB] CR [TB] CR,EH [SOB] JS
[15] qapi-common-py-add-notational  # [RB] CR,EH [SOB] JS
[16] qapi-common-move-comments-into # [RB] CR,EH [SOB] JS
[17] qapi-split-build_params-into   # [RB] CR,EH [SOB] JS
[18] qapi-establish-mypy-type       # [RB] CR [TB] CR,EH [SOB] JS
[19] qapi-events-py-add-notational  # [RB] CR,EH [SOB] JS
[20] qapi-events-move-comments-into # [RB] CR,EH [SOB] JS
[21] qapi-commands-py-don-t-re-bind # [RB] CR,EH,MA [SOB] JS
[22] qapi-commands-py-add           # [RB] CR,EH [SOB] JS
[23] qapi-commands-py-enable        # [RB] CR,EH [SOB] JS
[24] qapi-source-py-add-notational  # [RB] CR,EH [TB] CR [SOB] JS
[25] qapi-source-py-delint-with     # [RB] CR,EH [TB] CR [SOB] JS
[26] qapi-gen-py-fix-edge-case-of   # [RB] CR,EH,PM [SOB] JS
[27] qapi-gen-py-add-notational     # [RB] CR,EH [SOB] JS
[28] qapi-gen-py-enable-checking    # [RB] CR,EH [TB] CR [SOB] JS
[29] qapi-gen-py-remove-unused      # [RB] CR,EH [SOB] JS
[30] qapi-gen-py-update-write-to-be # [RB] CR,EH,MA [SOB] JS
[31] qapi-gen-py-delint-with-pylint # [RB] CR,EH [SOB] JS
[32] qapi-types-py-add-type-hint    # [RB] CR,EH [SOB] JS
[33] qapi-types-py-remove-one       # [RB] CR,EH [SOB] JS
[34] qapi-visit-py-assert           # [RB] CR,EH [SOB] JS
[35] qapi-visit-py-remove-unused    # [RB] CR,EH,PM [TB] CR [SOB] JS
[36] qapi-visit-py-add-notational   # [RB] CR,EH [TB] CR [SOB] JS

Changelog:

002/36:[0001] [FC] 'qapi: modify docstrings to be sphinx-compatible'
003/36:[0030] [FC] 'qapi-gen: Separate arg-parsing from generation'
004/36:[down] 'qapi: move generator entrypoint into package'
008/36:[0008] [FC] 'qapi: delint using flake8'
016/36:[0017] [FC] 'qapi/common.py: Convert comments into docstrings, and elaborate'
017/36:[0004] [FC] 'qapi/common.py: move build_params into gen.py'
022/36:[0011] [FC] 'qapi/commands.py: add type hint annotations'
024/36:[0003] [FC] 'qapi/source.py: add type hint annotations'
033/36:[0022] [FC] 'qapi/types.py: remove one-letter variables'

V6:
 - Rebase
 - 02: Dropped changes not *strictly* required for sphinx building.
 - 03: Split prefix check into helper function,
       Removed default constants,
       Moved error message back to main() function, changed wording
 - 04: Changes from 003; fixed commit message ("package", not "module")
 - 08: Line wrapping changes.
 - 16: Sphinx formatting changes, docstring change.
 - 17: Copyright string changes
 - 22: Reflow and add type for new 'coroutine' parameter
 - 24: Simpler QAPISourceInfo() typing
 - 33: "memb" and "var" instead of "member" and "variant".

V5:
 - Remove DO-NOT-MERGE patches (Now in Part2)
 - Remove introspect.py patches (Now in Part2)
 - 02: Docstring formatting, more commit message (Markus)

V4:
 - Rebase on Peter Maydell's work;
  - 05: Context differences from Peter Maydell's work
  - 06: Remove qapi.doc
  - 07: Remove qapi.doc, remove superfluous "if match"
  - 09: Remove qapi.doc
  - 13: remove qapi.doc
  - 18: remove qapi.doc
  - 22: remove qapi.doc
  - 31: remove QAPIGenDoc changes

 - Minor adjustments from list feedback;
  - 01: More backticks for QAPI json files, now that they are in Sphinx-exe
  - 02: Use :ref: role for the `docker-ref` cross-reference
  - 04: Remove doc.py work changes; add references for start_if/end_if
  - 10: Changes to accommodate isort
  - 11: Added lines_after_imports=2
  - 16: isort whitespace changes
  - 24: Take Markus's docstring phrasing
  - 34: add comment explaining os.open()
  - 37: isort differences

V3:
 - Use isort to enforce import consistency
 - Use sphinx apidoc to check docstring format

V2:
 - Removed Python3.6 patch in favor of Thomas Huth's
 - Addressed (most) feedback from Markus
 - Renamed type hint annotation commits

John Snow (36):
  docs: repair broken references
  qapi: modify docstrings to be sphinx-compatible
  qapi-gen: Separate arg-parsing from generation
  qapi: move generator entrypoint into package
  qapi: Prefer explicit relative imports
  qapi: Remove wildcard includes
  qapi: enforce import order/styling with isort
  qapi: delint using flake8
  qapi: add pylintrc
  qapi/common.py: Remove python compatibility workaround
  qapi/common.py: Add indent manager
  qapi/common.py: delint with pylint
  qapi/common.py: Replace one-letter 'c' variable
  qapi/common.py: check with pylint
  qapi/common.py: add type hint annotations
  qapi/common.py: Convert comments into docstrings, and elaborate
  qapi/common.py: move build_params into gen.py
  qapi: establish mypy type-checking baseline
  qapi/events.py: add type hint annotations
  qapi/events.py: Move comments into docstrings
  qapi/commands.py: Don't re-bind to variable of different type
  qapi/commands.py: add type hint annotations
  qapi/commands.py: enable checking with mypy
  qapi/source.py: add type hint annotations
  qapi/source.py: delint with pylint
  qapi/gen.py: Fix edge-case of _is_user_module
  qapi/gen.py: add type hint annotations
  qapi/gen.py: Enable checking with mypy
  qapi/gen.py: Remove unused parameter
  qapi/gen.py: update write() to be more idiomatic
  qapi/gen.py: delint with pylint
  qapi/types.py: add type hint annotations
  qapi/types.py: remove one-letter variables
  qapi/visit.py: assert tag_member contains a QAPISchemaEnumType
  qapi/visit.py: remove unused parameters from gen_visit_object
  qapi/visit.py: add type hint annotations

 docs/devel/multi-thread-tcg.rst |   2 +-
 docs/devel/testing.rst          |   2 +-
 scripts/qapi-gen.py             |  57 ++--------
 scripts/qapi/.flake8            |   2 +
 scripts/qapi/.isort.cfg         |   7 ++
 scripts/qapi/commands.py        |  90 +++++++++++-----
 scripts/qapi/common.py          | 164 ++++++++++++++++-------------
 scripts/qapi/events.py          |  58 +++++++---
 scripts/qapi/expr.py            |   7 +-
 scripts/qapi/gen.py             | 180 ++++++++++++++++++++------------
 scripts/qapi/introspect.py      |  16 ++-
 scripts/qapi/main.py            |  95 +++++++++++++++++
 scripts/qapi/mypy.ini           |  30 ++++++
 scripts/qapi/parser.py          |   6 +-
 scripts/qapi/pylintrc           |  70 +++++++++++++
 scripts/qapi/schema.py          |  33 +++---
 scripts/qapi/source.py          |  35 ++++---
 scripts/qapi/types.py           | 125 +++++++++++++++-------
 scripts/qapi/visit.py           | 116 ++++++++++++++------
 19 files changed, 759 insertions(+), 336 deletions(-)
 create mode 100644 scripts/qapi/.flake8
 create mode 100644 scripts/qapi/.isort.cfg
 create mode 100644 scripts/qapi/main.py
 create mode 100644 scripts/qapi/mypy.ini
 create mode 100644 scripts/qapi/pylintrc

-- 
2.26.2



Re: [PATCH v6 00/36] qapi: static typing conversion, pt1
Posted by Markus Armbruster 3 years, 6 months ago
John Snow <jsnow@redhat.com> writes:

> Hi, this series adds static type hints to the QAPI module.
> This is part one!
>
> Part 1: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt1
> Everything: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt6
>
> - Requires Python 3.6+
> - Requires mypy 0.770 or newer (for type analysis only)
> - Requires pylint 2.6.0 or newer (for lint checking only)
>
> In general, this series tackles the cleanup of one individual QAPI
> module at a time. Once it passes pylint or mypy checks, those checks are
> enabled for that file.
>
> Type hints are added in patches that add *only* type hints and change no
> other behavior. Any necessary changes to behavior to accommodate typing
> are split out into their own tiny patches.
>
> Notes:
>
> - After patch 07, `isort -c` should pass 100% on this and every
>   future commit.
>
> - After patch 08, `flake8 qapi/` should pass 100% on this and every
>   future commit.
>
> - After patch 09, `pylint --rcfile=qapi/pylintrc qapi/` should pass 100%
>   on this and every future commit.
>
> - After patch 18, `mypy --config-file=qapi/mypy.ini qapi/` should pass
>   100% on this and every future commit.

Series
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Queued, thanks!


Re: [PATCH v6 00/36] qapi: static typing conversion, pt1
Posted by John Snow 3 years, 6 months ago
On 10/10/20 5:43 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> Hi, this series adds static type hints to the QAPI module.
>> This is part one!
>>
>> Part 1: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt1
>> Everything: https://gitlab.com/jsnow/qemu/-/tree/python-qapi-cleanup-pt6
>>
>> - Requires Python 3.6+
>> - Requires mypy 0.770 or newer (for type analysis only)
>> - Requires pylint 2.6.0 or newer (for lint checking only)
>>
>> In general, this series tackles the cleanup of one individual QAPI
>> module at a time. Once it passes pylint or mypy checks, those checks are
>> enabled for that file.
>>
>> Type hints are added in patches that add *only* type hints and change no
>> other behavior. Any necessary changes to behavior to accommodate typing
>> are split out into their own tiny patches.
>>
>> Notes:
>>
>> - After patch 07, `isort -c` should pass 100% on this and every
>>    future commit.
>>
>> - After patch 08, `flake8 qapi/` should pass 100% on this and every
>>    future commit.
>>
>> - After patch 09, `pylint --rcfile=qapi/pylintrc qapi/` should pass 100%
>>    on this and every future commit.
>>
>> - After patch 18, `mypy --config-file=qapi/mypy.ini qapi/` should pass
>>    100% on this and every future commit.
> 
> Series
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> Queued, thanks!
> 

Thanks for putting up with me!

Only five left to go! Enjoy your PTO.

--js