[PATCH v4 0/4] [DO-NOT-MERGE] qapi: static typing conversion, "pt0"

[PATCH v4 0/4] [DO-NOT-MERGE] qapi: static typing conversion, "pt0"

[John Snow]

This is a manual testing pre-requisite for other QAPI cleanup
patches. It isn't meant for inclusion, it's included here as an
informative basis for ongoing work only.

(Except for the last two patches, which you should totally merge.)

sphinx-autodoc is enabled for the scripts/qapi package so that docstring
formatting can be validated and inspected to make sure the output looks
"nice". It is not necessarily how we'd want to expose this information,
ultimately, but serves as a good smoke test for base-line docstring
consistency.

Lastly, some scripts/tools for running linters with "canonical"
configurations are provided as a means to verify the linting
base-line. I use these to validate my own work.

V4:
 - Rebased on today's origin/master;
 - Updated config files to work with today's linter tool versions.

John Snow (4):
  [DO-NOT-MERGE] docs: enable sphinx-autodoc for scripts/qapi
  [DO-NOT-MERGE]: Add some ad-hoc linting helpers.
  qapi: Update flake8 config
  qapi: update pylint configuration

 docs/conf.py                          |  3 ++-
 docs/devel/index.rst                  |  1 +
 docs/devel/python/index.rst           |  7 +++++++
 docs/devel/python/qapi.commands.rst   |  7 +++++++
 docs/devel/python/qapi.common.rst     |  7 +++++++
 docs/devel/python/qapi.error.rst      |  7 +++++++
 docs/devel/python/qapi.events.rst     |  7 +++++++
 docs/devel/python/qapi.expr.rst       |  7 +++++++
 docs/devel/python/qapi.gen.rst        |  7 +++++++
 docs/devel/python/qapi.introspect.rst |  7 +++++++
 docs/devel/python/qapi.main.rst       |  7 +++++++
 docs/devel/python/qapi.parser.rst     |  8 ++++++++
 docs/devel/python/qapi.rst            | 26 ++++++++++++++++++++++++
 docs/devel/python/qapi.schema.rst     |  7 +++++++
 docs/devel/python/qapi.source.rst     |  7 +++++++
 docs/devel/python/qapi.types.rst      |  7 +++++++
 docs/devel/python/qapi.visit.rst      |  7 +++++++
 scripts/qapi-lint.sh                  | 29 +++++++++++++++++++++++++++
 scripts/qapi/.flake8                  |  3 ++-
 scripts/qapi/Makefile                 |  5 +++++
 scripts/qapi/pylintrc                 |  1 +
 21 files changed, 165 insertions(+), 2 deletions(-)
 create mode 100644 docs/devel/python/index.rst
 create mode 100644 docs/devel/python/qapi.commands.rst
 create mode 100644 docs/devel/python/qapi.common.rst
 create mode 100644 docs/devel/python/qapi.error.rst
 create mode 100644 docs/devel/python/qapi.events.rst
 create mode 100644 docs/devel/python/qapi.expr.rst
 create mode 100644 docs/devel/python/qapi.gen.rst
 create mode 100644 docs/devel/python/qapi.introspect.rst
 create mode 100644 docs/devel/python/qapi.main.rst
 create mode 100644 docs/devel/python/qapi.parser.rst
 create mode 100644 docs/devel/python/qapi.rst
 create mode 100644 docs/devel/python/qapi.schema.rst
 create mode 100644 docs/devel/python/qapi.source.rst
 create mode 100644 docs/devel/python/qapi.types.rst
 create mode 100644 docs/devel/python/qapi.visit.rst
 create mode 100755 scripts/qapi-lint.sh
 create mode 100644 scripts/qapi/Makefile

-- 
2.39.0

[PATCH v4 1/4] [DO-NOT-MERGE] docs: enable sphinx-autodoc for scripts/qapi

[John Snow]

This is just POC to prove that the docstrings, where they are written,
are correct to some minimum standard. It is included here for
reviewing/testing convenience.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/conf.py                          |  3 ++-
 docs/devel/index.rst                  |  1 +
 docs/devel/python/index.rst           |  7 +++++++
 docs/devel/python/qapi.commands.rst   |  7 +++++++
 docs/devel/python/qapi.common.rst     |  7 +++++++
 docs/devel/python/qapi.error.rst      |  7 +++++++
 docs/devel/python/qapi.events.rst     |  7 +++++++
 docs/devel/python/qapi.expr.rst       |  7 +++++++
 docs/devel/python/qapi.gen.rst        |  7 +++++++
 docs/devel/python/qapi.introspect.rst |  7 +++++++
 docs/devel/python/qapi.main.rst       |  7 +++++++
 docs/devel/python/qapi.parser.rst     |  8 ++++++++
 docs/devel/python/qapi.rst            | 26 ++++++++++++++++++++++++++
 docs/devel/python/qapi.schema.rst     |  7 +++++++
 docs/devel/python/qapi.source.rst     |  7 +++++++
 docs/devel/python/qapi.types.rst      |  7 +++++++
 docs/devel/python/qapi.visit.rst      |  7 +++++++
 17 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 docs/devel/python/index.rst
 create mode 100644 docs/devel/python/qapi.commands.rst
 create mode 100644 docs/devel/python/qapi.common.rst
 create mode 100644 docs/devel/python/qapi.error.rst
 create mode 100644 docs/devel/python/qapi.events.rst
 create mode 100644 docs/devel/python/qapi.expr.rst
 create mode 100644 docs/devel/python/qapi.gen.rst
 create mode 100644 docs/devel/python/qapi.introspect.rst
 create mode 100644 docs/devel/python/qapi.main.rst
 create mode 100644 docs/devel/python/qapi.parser.rst
 create mode 100644 docs/devel/python/qapi.rst
 create mode 100644 docs/devel/python/qapi.schema.rst
 create mode 100644 docs/devel/python/qapi.source.rst
 create mode 100644 docs/devel/python/qapi.types.rst
 create mode 100644 docs/devel/python/qapi.visit.rst

diff --git a/docs/conf.py b/docs/conf.py
index 73a287a4f27..154854d5c15 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -71,7 +71,8 @@
 # Add any Sphinx extension module names here, as strings. They can be
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
 # ones.
-extensions = ['kerneldoc', 'qmp_lexer', 'hxtool', 'depfile', 'qapidoc']
+extensions = ['kerneldoc', 'qmp_lexer', 'hxtool',
+              'depfile', 'qapidoc', 'sphinx.ext.autodoc']
 
 if sphinx.version_info[:3] > (4, 0, 0):
     tags.add('sphinx4')
diff --git a/docs/devel/index.rst b/docs/devel/index.rst
index 09cfb322beb..d4c66e9c8d7 100644
--- a/docs/devel/index.rst
+++ b/docs/devel/index.rst
@@ -14,3 +14,4 @@ modifying QEMU's source code.
    index-api
    index-internals
    index-tcg
+   python/index
diff --git a/docs/devel/python/index.rst b/docs/devel/python/index.rst
new file mode 100644
index 00000000000..31c470154b3
--- /dev/null
+++ b/docs/devel/python/index.rst
@@ -0,0 +1,7 @@
+qapi
+====
+
+.. toctree::
+   :maxdepth: 4
+
+   qapi
diff --git a/docs/devel/python/qapi.commands.rst b/docs/devel/python/qapi.commands.rst
new file mode 100644
index 00000000000..018f7b08a9c
--- /dev/null
+++ b/docs/devel/python/qapi.commands.rst
@@ -0,0 +1,7 @@
+qapi.commands module
+====================
+
+.. automodule:: qapi.commands
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.common.rst b/docs/devel/python/qapi.common.rst
new file mode 100644
index 00000000000..128a90d74be
--- /dev/null
+++ b/docs/devel/python/qapi.common.rst
@@ -0,0 +1,7 @@
+qapi.common module
+==================
+
+.. automodule:: qapi.common
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.error.rst b/docs/devel/python/qapi.error.rst
new file mode 100644
index 00000000000..980e32b63de
--- /dev/null
+++ b/docs/devel/python/qapi.error.rst
@@ -0,0 +1,7 @@
+qapi.error module
+=================
+
+.. automodule:: qapi.error
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.events.rst b/docs/devel/python/qapi.events.rst
new file mode 100644
index 00000000000..1fce85b044e
--- /dev/null
+++ b/docs/devel/python/qapi.events.rst
@@ -0,0 +1,7 @@
+qapi.events module
+==================
+
+.. automodule:: qapi.events
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.expr.rst b/docs/devel/python/qapi.expr.rst
new file mode 100644
index 00000000000..0660270629c
--- /dev/null
+++ b/docs/devel/python/qapi.expr.rst
@@ -0,0 +1,7 @@
+qapi.expr module
+================
+
+.. automodule:: qapi.expr
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.gen.rst b/docs/devel/python/qapi.gen.rst
new file mode 100644
index 00000000000..7b495fd4bf2
--- /dev/null
+++ b/docs/devel/python/qapi.gen.rst
@@ -0,0 +1,7 @@
+qapi.gen module
+===============
+
+.. automodule:: qapi.gen
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.introspect.rst b/docs/devel/python/qapi.introspect.rst
new file mode 100644
index 00000000000..f65ebfccd1b
--- /dev/null
+++ b/docs/devel/python/qapi.introspect.rst
@@ -0,0 +1,7 @@
+qapi.introspect module
+======================
+
+.. automodule:: qapi.introspect
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.main.rst b/docs/devel/python/qapi.main.rst
new file mode 100644
index 00000000000..1255fcda633
--- /dev/null
+++ b/docs/devel/python/qapi.main.rst
@@ -0,0 +1,7 @@
+qapi.main module
+================
+
+.. automodule:: qapi.main
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.parser.rst b/docs/devel/python/qapi.parser.rst
new file mode 100644
index 00000000000..1a8f7b347eb
--- /dev/null
+++ b/docs/devel/python/qapi.parser.rst
@@ -0,0 +1,8 @@
+qapi.parser module
+==================
+
+.. automodule:: qapi.parser
+   :members:
+   :undoc-members:
+   :show-inheritance:
+   :private-members:
diff --git a/docs/devel/python/qapi.rst b/docs/devel/python/qapi.rst
new file mode 100644
index 00000000000..c762019aad3
--- /dev/null
+++ b/docs/devel/python/qapi.rst
@@ -0,0 +1,26 @@
+qapi package
+============
+
+.. automodule:: qapi
+   :members:
+   :undoc-members:
+   :show-inheritance:
+
+Submodules
+----------
+
+.. toctree::
+
+   qapi.commands
+   qapi.common
+   qapi.error
+   qapi.events
+   qapi.expr
+   qapi.gen
+   qapi.introspect
+   qapi.main
+   qapi.parser
+   qapi.schema
+   qapi.source
+   qapi.types
+   qapi.visit
diff --git a/docs/devel/python/qapi.schema.rst b/docs/devel/python/qapi.schema.rst
new file mode 100644
index 00000000000..a08f75ed720
--- /dev/null
+++ b/docs/devel/python/qapi.schema.rst
@@ -0,0 +1,7 @@
+qapi.schema module
+==================
+
+.. automodule:: qapi.schema
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.source.rst b/docs/devel/python/qapi.source.rst
new file mode 100644
index 00000000000..e61e9f60212
--- /dev/null
+++ b/docs/devel/python/qapi.source.rst
@@ -0,0 +1,7 @@
+qapi.source module
+==================
+
+.. automodule:: qapi.source
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.types.rst b/docs/devel/python/qapi.types.rst
new file mode 100644
index 00000000000..6eea827557d
--- /dev/null
+++ b/docs/devel/python/qapi.types.rst
@@ -0,0 +1,7 @@
+qapi.types module
+=================
+
+.. automodule:: qapi.types
+   :members:
+   :undoc-members:
+   :show-inheritance:
diff --git a/docs/devel/python/qapi.visit.rst b/docs/devel/python/qapi.visit.rst
new file mode 100644
index 00000000000..84307cbc236
--- /dev/null
+++ b/docs/devel/python/qapi.visit.rst
@@ -0,0 +1,7 @@
+qapi.visit module
+=================
+
+.. automodule:: qapi.visit
+   :members:
+   :undoc-members:
+   :show-inheritance:
-- 
2.39.0

[PATCH v4 2/4] [DO-NOT-MERGE]: Add some ad-hoc linting helpers.

[John Snow]

These aren't ready for upstream inclusion, because they do not properly
manage version dependencies, execution environment and so on. These are
just the tools I use in my Own Special Environment :tm: for testing and
debugging.

They've been tested only on Fedora 37 for right now, which means:

Python 3.11.1
pylint 2.14.4-3.fc37
mypy 0.982-1.fc37
isort 5.10.1-5.fc37
flake8 5.0.3-1.fc37

The python packaging work does a more exhaustive treatment and
exploration of minimum (and maximum) package requirements; the hope is
that eventually these two series will converge and these dependencies
will all be handled by configuration in qemu.git/python/.

For now, this is just a baseline under one environment to give us some
guidance.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi-lint.sh  | 29 +++++++++++++++++++++++++++++
 scripts/qapi/Makefile |  5 +++++
 2 files changed, 34 insertions(+)
 create mode 100755 scripts/qapi-lint.sh
 create mode 100644 scripts/qapi/Makefile

diff --git a/scripts/qapi-lint.sh b/scripts/qapi-lint.sh
new file mode 100755
index 00000000000..2c2f2808ca6
--- /dev/null
+++ b/scripts/qapi-lint.sh
@@ -0,0 +1,29 @@
+#!/usr/bin/env bash
+set -e
+
+if [[ -f qapi/.flake8 ]]; then
+    echo "flake8 --config=qapi/.flake8 qapi/"
+    flake8 --config=qapi/.flake8 qapi/
+fi
+if [[ -f qapi/pylintrc ]]; then
+    echo "pylint --rcfile=qapi/pylintrc qapi/"
+    pylint --rcfile=qapi/pylintrc qapi/
+fi
+if [[ -f qapi/mypy.ini ]]; then
+    echo "mypy --config-file=qapi/mypy.ini qapi/"
+    mypy --config-file=qapi/mypy.ini qapi/
+fi
+
+if [[ -f qapi/.isort.cfg ]]; then
+    pushd qapi
+    echo "isort -c ."
+    isort -c .
+    popd
+fi
+
+pushd ../bin/git
+make -j9
+make check-qapi-schema
+make docs
+make sphinxdocs
+popd
diff --git a/scripts/qapi/Makefile b/scripts/qapi/Makefile
new file mode 100644
index 00000000000..314e8a5505e
--- /dev/null
+++ b/scripts/qapi/Makefile
@@ -0,0 +1,5 @@
+check:
+	isort -c .
+	flake8 .
+	cd .. && pylint --rcfile=qapi/pylintrc qapi
+	cd .. && mypy -p qapi --config-file=qapi/mypy.ini
-- 
2.39.0

[PATCH v4 3/4] qapi: Update flake8 config

[John Snow]

New versions of flake8 don't like same-line comments. (It's a version
newer than what fc37 ships, but it still makes my life easier to fix it
now.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/.flake8 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8
index 6b158c68b84..a873ff67309 100644
--- a/scripts/qapi/.flake8
+++ b/scripts/qapi/.flake8
@@ -1,2 +1,3 @@
 [flake8]
-extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
+# Prefer pylint's bare-except checks to flake8's
+extend-ignore = E722
-- 
2.39.0

[PATCH v4 4/4] qapi: update pylint configuration

[John Snow]

Newer versions of pylint disable the "no-self-use" message by
default. Older versions don't, though. If we leave the suppressions in,
pylint yelps about useless options. Just tell pylint to shush.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/pylintrc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc
index a7246282030..90546df5345 100644
--- a/scripts/qapi/pylintrc
+++ b/scripts/qapi/pylintrc
@@ -23,6 +23,7 @@ disable=fixme,
         too-many-statements,
         too-many-instance-attributes,
         consider-using-f-string,
+        useless-option-value,
 
 [REPORTS]
 
-- 
2.39.0

Re: [PATCH v4 3/4] qapi: Update flake8 config

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> New versions of flake8 don't like same-line comments. (It's a version
> newer than what fc37 ships, but it still makes my life easier to fix it
> now.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/.flake8 | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8
> index 6b158c68b84..a873ff67309 100644
> --- a/scripts/qapi/.flake8
> +++ b/scripts/qapi/.flake8
> @@ -1,2 +1,3 @@
>  [flake8]
> -extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
> +# Prefer pylint's bare-except checks to flake8's
> +extend-ignore = E722

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

Re: [PATCH v4 4/4] qapi: update pylint configuration

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> Newer versions of pylint disable the "no-self-use" message by
> default. Older versions don't, though. If we leave the suppressions in,
> pylint yelps about useless options. Just tell pylint to shush.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/pylintrc | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc
> index a7246282030..90546df5345 100644
> --- a/scripts/qapi/pylintrc
> +++ b/scripts/qapi/pylintrc
> @@ -23,6 +23,7 @@ disable=fixme,
>          too-many-statements,
>          too-many-instance-attributes,
>          consider-using-f-string,
> +        useless-option-value,
>  
>  [REPORTS]

Pylint docs[*] explains this is "used when a value for an option that is
now deleted from pylint is encountered."  Switching that off makes sense
when you have to deal with a range of versions.  Occasional garbage
collection of old options is advised :)

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


[*] https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/useless-option-value.html

Re: [PATCH v4 4/4] qapi: update pylint configuration

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 1612 bytes --]

On Sat, Feb 4, 2023, 6:34 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > Newer versions of pylint disable the "no-self-use" message by
> > default. Older versions don't, though. If we leave the suppressions in,
> > pylint yelps about useless options. Just tell pylint to shush.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  scripts/qapi/pylintrc | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc
> > index a7246282030..90546df5345 100644
> > --- a/scripts/qapi/pylintrc
> > +++ b/scripts/qapi/pylintrc
> > @@ -23,6 +23,7 @@ disable=fixme,
> >          too-many-statements,
> >          too-many-instance-attributes,
> >          consider-using-f-string,
> > +        useless-option-value,
> >
> >  [REPORTS]
>
> Pylint docs[*] explains this is "used when a value for an option that is
> now deleted from pylint is encountered."  Switching that off makes sense
> when you have to deal with a range of versions.  Occasional garbage
> collection of old options is advised :)
>

Indeed.

For the purposes of CI and repeatability (and maintainer happiness), it'd
be nice to just enforce specific, explicit versions.

For the purposes of developer happiness, I'm willing to accept *some* pain
in supporting a range of versions for the tools.

I'm anticipating re-evaluating suppressions and workarounds when 3.6 gets
dropped.

--js


> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
>
> [*]
> https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/useless-option-value.html
>
>

[-- Attachment #2: Type: text/html, Size: 2914 bytes --]