qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 1c0091: qapi/pylintrc: ignore 'consider-using


From: Richard Henderson
Subject: [Qemu-commits] [qemu/qemu] 1c0091: qapi/pylintrc: ignore 'consider-using-f-string' wa...
Date: Sat, 02 Oct 2021 13:31:37 -0700

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 1c00917409c9604cfc587045ba37395a48337dff
      
https://github.com/qemu/qemu/commit/1c00917409c9604cfc587045ba37395a48337dff
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/pylintrc

  Log Message:
  -----------
  qapi/pylintrc: ignore 'consider-using-f-string' warning

Pylint 2.11.x adds this warning. We're not yet ready to pursue that
conversion, so silence it for now.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-2-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: 2adb988ed4ca31813d237c475a6a327ef16c5432
      
https://github.com/qemu/qemu/commit/2adb988ed4ca31813d237c475a6a327ef16c5432
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/gen.py

  Log Message:
  -----------
  qapi/gen: use dict.items() to iterate over _modules

New pylint warning. I could silence it, but this is the only occurrence
in the entire tree, including everything in iotests/ and python/. Easier
to just change this one instance.

(The warning is emitted in cases where you are fetching the values
anyway, so you may as well just take advantage of the iterator to avoid
redundant lookups.)

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-3-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: 012336a152641b264a65176a388a7fb0118e1781
      
https://github.com/qemu/qemu/commit/012336a152641b264a65176a388a7fb0118e1781
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/parser.py
    M tests/qapi-schema/doc-bad-feature.err

  Log Message:
  -----------
  qapi/parser: fix unused check_args_section arguments

Pylint informs us we're not using these arguments. Oops, it's
right. Correct the error message and remove the remaining unused
parameter.

Fix test output now that the error message is improved.

Fixes: e151941d1b
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-4-jsnow@redhat.com>
[Commit message formatting tweaked]
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: a9e2eb06ed061f37c3ba6ad52722eef20afd713a
      
https://github.com/qemu/qemu/commit/a9e2eb06ed061f37c3ba6ad52722eef20afd713a
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M qapi/block-core.json
    M qga/qapi-schema.json
    M tests/qapi-schema/doc-good.json

  Log Message:
  -----------
  qapi: Add spaces after symbol declaration for consistency

Several QGA definitions omit a blank line after the symbol
declaration. This works OK currently, but it's the only place where we
do this. Adjust it for consistency.

Future commits may wind up enforcing this formatting.

Signed-off-by: John Snow <jsnow@redhat.com>

Message-Id: <20210930205716.1148693-5-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: cd87c14cde5db42a2f13bfdbba1f3cbeb347a411
      
https://github.com/qemu/qemu/commit/cd87c14cde5db42a2f13bfdbba1f3cbeb347a411
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/parser.py
    M tests/qapi-schema/doc-empty-symbol.err

  Log Message:
  -----------
  qapi/parser: remove FIXME comment from _append_body_line

True, we do not check the validity of this symbol -- but we don't check
the validity of definition names during parse, either -- that happens
later, during the expr check. I don't want to introduce a dependency on
expr.py:check_name_str here and introduce a cycle.

Instead, rest assured that a documentation block is required for each
definition. This requirement uses the names of each section to ensure
that we fulfilled this requirement.

e.g., let's say that block-core.json has a comment block for
"Snapshot!Info" by accident. We'll see this error message:

In file included from ../../qapi/block.json:8:
../../qapi/block-core.json: In struct 'SnapshotInfo':
../../qapi/block-core.json:38: documentation comment is for 'Snapshot!Info'

That's a pretty decent error message.

Now, let's say that we actually mangle it twice, identically:

../../qapi/block-core.json: In struct 'Snapshot!Info':
../../qapi/block-core.json:38: struct has an invalid name

That's also pretty decent. If we forget to fix it in both places, we'll
just be back to the first error.

Therefore, let's just drop this FIXME and adjust the error message to
not imply a more thorough check than is actually performed.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-6-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: 1e20a77576dedf1489ce1cdb6abc4b34663637a4
      
https://github.com/qemu/qemu/commit/1e20a77576dedf1489ce1cdb6abc4b34663637a4
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/parser.py

  Log Message:
  -----------
  qapi/parser: clarify _end_section() logic

The "if self._section" clause in end_section is mysterious: In which
circumstances might we end a section when we don't have one?

QAPIDoc always expects there to be a "current section", only except
after a call to end_comment(). This actually *shouldn't* ever be 'None',
so let's remove that logic so I don't wonder why it's like this again in
three months.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-7-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: f4c05aaf148a44d80855eb45b9342feaeeb4764a
      
https://github.com/qemu/qemu/commit/f4c05aaf148a44d80855eb45b9342feaeeb4764a
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/parser.py

  Log Message:
  -----------
  qapi/parser: Introduce NullSection

Here's the weird bit. QAPIDoc generally expects -- virtually everywhere
-- that it will always have a current section. The sole exception to
this is in the case that end_comment() is called, which leaves us with
*no* section. However, in this case, we also don't expect to actually
ever mutate the comment contents ever again.

NullSection is just a Null-object that allows us to maintain the
invariant that we *always* have a current section, enforced by static
typing -- allowing us to type that field as QAPIDoc.Section instead of
the more ambiguous Optional[QAPIDoc.Section].

end_section is renamed to switch_section and now accepts as an argument
the new section to activate, clarifying that no callers ever just
unilaterally end a section; they only do so when starting a new section.

Signed-off-by: John Snow <jsnow@redhat.com>

Message-Id: <20210930205716.1148693-8-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: e7ac60fcd0623fe255f54c33f2bed2e7b2f780f5
      
https://github.com/qemu/qemu/commit/e7ac60fcd0623fe255f54c33f2bed2e7b2f780f5
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/parser.py

  Log Message:
  -----------
  qapi/parser: add import cycle workaround

Adding static types causes a cycle in the QAPI generator:
[schema -> expr -> parser -> schema]. It exists because the QAPIDoc
class needs the names of types defined by the schema module, but the
schema module needs to import both expr.py/parser.py to do its actual
parsing.

Ultimately, the layering violation is that parser.py should not have any
knowledge of specifics of the Schema. QAPIDoc performs double-duty here
both as a parser *and* as a finalized object that is part of the schema.

In this patch, add the offending type hints alongside the workaround to
avoid the cycle becoming a problem at runtime. See
https://mypy.readthedocs.io/en/latest/runtime_troubles.html#import-cycles
for more information on this workaround technique.

I see three ultimate resolutions here:

(1) Just keep this patch and use the TYPE_CHECKING trick to eliminate
    the cycle which is only present during static analysis.

(2) Don't bother to annotate connect_member() et al, give them 'object'
    or 'Any'. I don't particularly like this, because it diminishes the
    usefulness of type hints for documentation purposes. Still, it's an
    extremely quick fix.

(3) Reimplement doc <--> definition correlation directly in schema.py,
    integrating doc fields directly into QAPISchemaMember and relieving
    the QAPIDoc class of the responsibility. Users of the information
    would instead visit the members first and retrieve their
    documentation instead of the inverse operation -- visiting the
    documentation and retrieving their members.

My preference is (3), but in the short-term (1) is the easiest way to
have my cake (strong type hints) and eat it too (Not have import
cycles). Do (1) for now, but plan for (3).

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-9-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: 5f0d9f3bc762fcbb1637b5e257c9cd8b9a8aa9ab
      
https://github.com/qemu/qemu/commit/5f0d9f3bc762fcbb1637b5e257c9cd8b9a8aa9ab
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/parser.py

  Log Message:
  -----------
  qapi/parser: add type hint annotations (QAPIDoc)

Annotations do not change runtime behavior.
This commit consists of only annotations.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-10-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: 15acf48cfed15b37771922093693007d1ad09219
      
https://github.com/qemu/qemu/commit/15acf48cfed15b37771922093693007d1ad09219
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/parser.py

  Log Message:
  -----------
  qapi/parser: Add FIXME for consolidating JSON-related types

The fix for this comment is forthcoming in a future commit, but this
will keep me honest. The linting configuration in ./python/setup.cfg
prohibits 'FIXME' comments. A goal of this long-running series is to
move ./scripts/qapi to ./python/qemu/qapi so that the QAPI generator is
regularly type-checked by GitLab CI.

This comment is a time-bomb to force me to address this issue prior to
that step.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-11-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: 2e28283e419357f0ee03a33a9224f908f9f67b04
      
https://github.com/qemu/qemu/commit/2e28283e419357f0ee03a33a9224f908f9f67b04
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/mypy.ini

  Log Message:
  -----------
  qapi/parser: enable mypy checks

Signed-off-by: John Snow <jsnow@redhat.com>

Message-Id: <20210930205716.1148693-12-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: 18e3673e0f8479e392e55245ad70c1eedb383507
      
https://github.com/qemu/qemu/commit/18e3673e0f8479e392e55245ad70c1eedb383507
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/parser.py

  Log Message:
  -----------
  qapi/parser: Silence too-few-public-methods warning

Eh. Not worth the fuss today. There are bigger fish to fry.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-13-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: d183e0481b1510b253ac94e702c76115f3bb6450
      
https://github.com/qemu/qemu/commit/d183e0481b1510b253ac94e702c76115f3bb6450
  Author: John Snow <jsnow@redhat.com>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M scripts/qapi/pylintrc

  Log Message:
  -----------
  qapi/parser: enable pylint checks

Signed-off-by: John Snow <jsnow@redhat.com>

Message-Id: <20210930205716.1148693-14-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>


  Commit: f50ecf548c7313c27037f7b7fb8ecc5a5e89249c
      
https://github.com/qemu/qemu/commit/f50ecf548c7313c27037f7b7fb8ecc5a5e89249c
  Author: Richard Henderson <richard.henderson@linaro.org>
  Date:   2021-10-02 (Sat, 02 Oct 2021)

  Changed paths:
    M qapi/block-core.json
    M qga/qapi-schema.json
    M scripts/qapi/gen.py
    M scripts/qapi/mypy.ini
    M scripts/qapi/parser.py
    M scripts/qapi/pylintrc
    M tests/qapi-schema/doc-bad-feature.err
    M tests/qapi-schema/doc-empty-symbol.err
    M tests/qapi-schema/doc-good.json

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2021-10-02' into 
staging

QAPI patches patches for 2021-10-02

# gpg: Signature made Sat 02 Oct 2021 01:37:11 AM EDT
# gpg:                using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653
# gpg:                issuer "armbru@redhat.com"
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full]
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>" [full]

* remotes/armbru/tags/pull-qapi-2021-10-02:
  qapi/parser: enable pylint checks
  qapi/parser: Silence too-few-public-methods warning
  qapi/parser: enable mypy checks
  qapi/parser: Add FIXME for consolidating JSON-related types
  qapi/parser: add type hint annotations (QAPIDoc)
  qapi/parser: add import cycle workaround
  qapi/parser: Introduce NullSection
  qapi/parser: clarify _end_section() logic
  qapi/parser: remove FIXME comment from _append_body_line
  qapi: Add spaces after symbol declaration for consistency
  qapi/parser: fix unused check_args_section arguments
  qapi/gen: use dict.items() to iterate over _modules
  qapi/pylintrc: ignore 'consider-using-f-string' warning

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Compare: https://github.com/qemu/qemu/compare/5f992102383e...f50ecf548c73



reply via email to

[Prev in Thread] Current Thread [Next in Thread]