Commit Graph

38 Commits

Author SHA1 Message Date
John Snow e42648dccd qapi/expr.py: Remove single-letter variable
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421182032.3521476-13-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow 210fd63104 qapi/expr.py: Consolidate check_if_str calls in check_if
This is a small rewrite to address some minor style nits.

Don't compare against the empty list to check for the empty condition, and
move the normalization forward to unify the check on the now-normalized
structure.

With the check unified, the local nested function isn't needed anymore
and can be brought down into the normal flow of the function. With the
nesting level changed, shuffle the error strings around a bit to get
them to fit in 79 columns.

Note: although ifcond is typed as Sequence[str] elsewhere, we *know* that
the parser will produce real, bona-fide lists. It's okay to check
isinstance(ifcond, list) here.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421182032.3521476-12-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow b9ad358aa0 qapi/expr.py: add type hint annotations
Annotations do not change runtime behavior.
This commit *only* adds annotations.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421182032.3521476-11-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow 538cd41065 qapi/expr.py: Modify check_keys to accept any Collection
This is a minor adjustment that lets parameters @required and
@optional take tuple arguments, in particular ().  Later patches will
make use of that.

(Iterable would also have worked, but Iterable also includes things like
generator expressions which are consumed upon iteration, which would
require a rewrite to make sure that each input was only traversed
once. Collection implies the "can re-iterate" property.)

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421182032.3521476-10-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow 7a783ce5b5 qapi/expr.py: Add casts in a few select cases
Casts are instructions to the type checker only, they aren't "safe" and
should probably be avoided in general. In this case, when we perform
type checking on a nested structure, the type of each field does not
"stick".

(See PEP 647 for an example of "type narrowing" that does "stick".
 It is available in Python 3.10, so we can't use it yet.)

We don't need to assert that something is a str if we've already checked
or asserted that it is -- use a cast instead for these cases.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421182032.3521476-9-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow 4918bb7def qapi/expr.py: Check type of union and alternate 'data' member
Prior to this commit, specifying a non-object value here causes the QAPI
parser to crash in expr.py with a stack trace with (likely) an
AttributeError when we attempt to call that value's items() method.

This member needs to be an object (Dict), and not anything else. Add a
check for this with a nicer error message, and formalize that check with
new test cases that exercise that error.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421182032.3521476-8-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow 926bb8add7 qapi/expr.py: move string check upwards in check_type
For readability purposes only, shimmy the early return upwards to the
top of the function, so cases proceed in order from least to most
complex.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20210421182032.3521476-7-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow b66c62a2d3 qapi/expr.py: Add assertion for union type 'check_dict'
mypy isn't fond of allowing you to check for bool membership in a
collection of str elements. Guard this lookup for precisely when we were
given a name.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20210421182032.3521476-6-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow 59b5556ce8 qapi/expr.py: constrain incoming expression types
mypy does not know the types of values stored in Dicts that masquerade
as objects. Help the type checker out by constraining the type.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421182032.3521476-5-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow 0f231dcf29 qapi/expr.py: Check for dict instead of OrderedDict
OrderedDict is a subtype of dict, so we can check for a more general
form. These functions do not themselves depend on it being any
particular type.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20210421182032.3521476-4-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow b7341b89c9 qapi/expr.py: Remove 'info' argument from nested check_if_str
The function can just use the argument from the scope above. Otherwise,
we get shadowed argument errors because the parameter name clashes with
the name of a variable already in-scope.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20210421182032.3521476-3-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
John Snow d0a263cdd0 qapi/expr: Comment cleanup
The linter yaps after 0825f62c84. Fix this trivial issue to restore the
linter baseline.

(Yes, ideally -- and soon -- the linter will be part of CI so we don't
clutter up the log with fixups. For now, though, the baseline is useful
for testing intermediate commits as types are added to the QAPI
library.)

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210421182032.3521476-2-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-04-30 12:59:54 +02:00
Markus Armbruster d83b47646e qapi: Enforce union and alternate branch naming rules
Union branch names should use '-', not '_'.  Enforce this.  The only
offenders are in tests/.  Fix them.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-29-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message typo fixed]
2021-03-23 22:31:53 +01:00
Markus Armbruster 407efbf9e7 qapi: Enforce enum member naming rules
Enum members should use '-', not '_'.  Enforce this.  Fix the fixable
offenders (all in tests/), and add the remainder to pragma
member-name-exceptions.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-28-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2021-03-23 22:31:43 +01:00
Markus Armbruster 5aceeac04d qapi: Enforce struct member naming rules
Struct members, including command arguments, event data, and union
inline base members, should use '-', not '_'.  Enforce this.  Fix the
fixable offenders (all in tests/), and add the remainder to pragma
member-name-exceptions.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-27-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2021-03-23 22:31:43 +01:00
Markus Armbruster 05ebf841ef qapi: Enforce command naming rules
Command names should be lower-case.  Enforce this.  Fix the fixable
offenders (all in tests/), and add the remainder to pragma
command-name-exceptions.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-25-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2021-03-23 22:31:05 +01:00
Markus Armbruster e744708a77 qapi: Enforce feature naming rules
Feature names should use '-', not '_'.  Enforce this.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-24-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2021-03-23 22:31:05 +01:00
Markus Armbruster 9af4b6b9e8 qapi: Prepare for rejecting underscore in command and member names
Command names and member names within a type should be all lower case
with words separated by a hyphen.  We also accept underscore.  Rework
check_name_lower() to optionally reject underscores, but don't use
that option, yet.

Update expected test output for the changed error message.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-23-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2021-03-23 22:31:05 +01:00
Markus Armbruster b86df37478 qapi: Rename pragma *-whitelist to *-exceptions
Rename pragma returns-whitelist to command-returns-exceptions, and
name-case-whitelist to member-name-case-exceptions.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-20-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2021-03-23 22:31:05 +01:00
Markus Armbruster 3e6c8a6331 qapi: Enforce type naming rules
Type names should be CamelCase.  Enforce this.  The only offenders are
in tests/.  Fix them.  Add test type-case to cover the new error.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-15-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Regexp simplified, new test made more robust]
2021-03-23 22:31:05 +01:00
Markus Armbruster d4f4cae8de qapi: Enforce event naming rules
Event names should be ALL_CAPS with words separated by underscore.
Enforce this.  The only offenders are in tests/.  Fix them.  Existing
test event-case covers the new error.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-14-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2021-03-23 22:31:05 +01:00
Markus Armbruster 00ffe242d6 qapi: Consistently permit any case in downstream prefixes
We require lowercase __RFQDN_ downstream prefixes only where we
require the prefixed name to be lowercase.  Don't; permit any case in
__RFQDN_ prefixes anywhere.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-13-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2021-03-23 22:31:05 +01:00
Markus Armbruster d224e0c092 qapi: Move uppercase rejection to check_name_lower()
check_name_lower() is the only user of check_name_str() using
permit_upper=False.  Move the associated code from check_name_str() to
check_name_lower(), and drop the parameter.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-12-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2021-03-23 22:31:05 +01:00
Markus Armbruster eaab06faa5 qapi: Rework name checking in preparation of stricter checking
Naming rules differ for the various kinds of names.  To prepare
enforcing them, define functions to check them: check_name_upper(),
check_name_lower(), and check_name_camel().  For now, these merely
wrap around check_name_str(), but that will change shortly.  Replace
the other uses of check_name_str() by appropriate uses of the
wrappers.  No change in behavior just yet.

check_name_str() now returns the name without downstream and x-
prefix, for use by the wrappers in later patches.  Requires tweaking
regexp @valid_name.  It accepts the same strings as before.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-11-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message improved]
2021-03-23 22:30:51 +01:00
Markus Armbruster 0825f62c84 qapi: Lift enum-specific code out of check_name_str()
check_name_str() masks leading digits when passed enum_member=True.
Only check_enum() does.  Lift the masking into check_enum().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-10-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2021-03-23 22:30:32 +01:00
Markus Armbruster 5fbc78dd36 qapi: Permit flat union members for any tag value
Flat union branch names match the tag enum's member names.  Omitted
branches default to "no members for this tag value".

Branch names starting with a digit get rejected like "'data' member
'0' has an invalid name".  However, omitting the branch works.

This is because flat union tag values get checked twice: as enum
member name, and as union branch name.  The former accepts leading
digits, the latter doesn't.

Branches whose names start with a digit therefore cannot have members.
Feels wrong.  Get rid of the restriction by skipping the latter check.

This can expose c_name() to input it can't handle: a name starting
with a digit.  Improve it to return a valid C identifier for any
input.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-9-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message rewritten]
2021-03-23 22:30:08 +01:00
Markus Armbruster dbfe3c7c28 qapi: Fix to reject optional members with reserved names
check_type() fails to reject optional members with reserved names,
because it neglects to strip off the leading '*'.  Fix that.

The stripping in check_name_str() is now useless.  Drop.

Also drop the "no leading '*'" assertion, because valid_name.match()
ensures it can't fail.

Fixes: 9fb081e0b9
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-8-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
2021-03-23 22:29:37 +01:00
John Snow 67fea57502 qapi: enforce import order/styling with isort
While we're mucking around with imports, we might as well formalize the
style we use. Let's use isort to do it for us.

lines_after_imports=2: Use two lines after imports, to match PEP8's
desire to have "two lines before and after" class definitions, which are
likely to start immediately after imports.

force_sort_within_sections: Intermingles "from x" and "import x" style
statements, such that sorting is always performed strictly on the module
name itself.

force_grid_wrap=4: Four or more imports from a single module will force
the one-per-line style that's more git-friendly. This will generally
happen for 'typing' imports.

multi_line_output=3: Uses the one-per-line indented style for long
imports.

include_trailing_comma: Adds a comma to the last import in a group,
which makes git conflicts nicer to deal with, generally.

line_length: 72 is chosen to match PEP8's "docstrings and comments" line
length limit. If you have a single line import that exceeds 72
characters, your names are too long!

Suggested-by: Cleber Rosa <crosa@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201009161558.107041-8-jsnow@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10 11:37:47 +02:00
John Snow 7137a96099 qapi: Prefer explicit relative imports
All of the QAPI include statements are changed to be package-aware, as
explicit relative imports.

A quirk of Python packages is that the name of the package exists only
*outside* of the package. This means that to a module inside of the qapi
folder, there is inherently no such thing as the "qapi" package. The
reason these imports work is because the "qapi" package exists in the
context of the caller -- the execution shim, where sys.path includes a
directory that has a 'qapi' folder in it.

When we write "from qapi import sibling", we are NOT referencing the folder
'qapi', but rather "any package named qapi in sys.path". If you should
so happen to have a 'qapi' package in your path, it will use *that*
package.

When we write "from .sibling import foo", we always reference explicitly
our sibling module; guaranteeing consistency in *where* we are importing
these modules from.

This can be useful when working with virtual environments and packages
in development mode. In development mode, a package is installed as a
series of symlinks that forwards to your same source files. The problem
arises because code quality checkers will follow "import qapi.x" to the
"installed" version instead of the sibling file and -- even though they
are the same file -- they have different module paths, and this causes
cyclic import problems, false positive type mismatch errors, and more.

It can also be useful when dealing with hierarchical packages, e.g. if
we allow qemu.core.qmp, qemu.qapi.parser, etc.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201009161558.107041-6-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10 11:37:47 +02:00
Kevin Wolf 04f22362f1 qapi: Add a 'coroutine' flag for commands
This patch adds a new 'coroutine' flag to QMP command definitions that
tells the QMP dispatcher that the command handler is safe to be run in a
coroutine.

The documentation of the new flag pretends that this flag is already
used as intended, which it isn't yet after this patch. We'll implement
this in another patch in this series.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20201005155855.256490-9-kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09 07:08:19 +02:00
Markus Armbruster 84ab008687 qapi: Add feature flags to struct members
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20200317115459.31821-21-armbru@redhat.com>
2020-03-17 21:25:47 +01:00
Markus Armbruster 013b4efc9b qapi: Add feature flags to remaining definitions
In v4.1.0, we added feature flags just to struct types (commit
6a8c0b5102^..f3ed93d545), to satisfy an immediate need (commit
c9d4070991 "file-posix: Add dynamic-auto-read-only QAPI feature").  In
v4.2.0, we added them to commands (commit 23394b4c39 "qapi: Add
feature flags to commands") to satisfy another immediate need (commit
d76744e65e "qapi: Allow introspecting fix for savevm's cooperation
with blockdev").

Add them to the remaining definitions: enumeration types, union types,
alternate types, and events.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200317115459.31821-13-armbru@redhat.com>
2020-03-17 19:58:34 +01:00
Markus Armbruster 8ec0e1a4e6 qapi: Brush off some (py)lint
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200304155932.20452-5-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
2020-03-05 09:24:11 +01:00
Markus Armbruster ad1ecfc679 qapi: Fold normalize_enum() into check_enum()
check_features() is always called together with normalize_features().
Fold the latter into the former.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-17-armbru@redhat.com>
2019-10-29 07:35:16 +01:00
Markus Armbruster 2ce51ef68f qapi: Fold normalize_features() into check_features()
check_features() is always called together with normalize_features():
the former in check_struct() and check_command(), the latter in their
caller check_exprs().  Fold the latter into the former.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-16-armbru@redhat.com>
2019-10-29 07:35:16 +01:00
Markus Armbruster c145bfda18 qapi: Fold normalize_if() into check_if()
check_if() is always called together with normalize_if().  Fold the
latter into the former.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191024110237.30963-15-armbru@redhat.com>
2019-10-29 07:35:16 +01:00
Peter Krempa 23394b4c39 qapi: Add feature flags to commands
Similarly to features for struct types introduce the feature flags also
for commands. This will allow notifying management layers of fixes and
compatible changes in the behaviour of a command which may not be
detectable any other way.

The changes were heavily inspired by commit 6a8c0b5102.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191018081454.21369-3-armbru@redhat.com>
2019-10-22 13:54:13 +02:00
Markus Armbruster e6c42b96b9 qapi: Split up scripts/qapi/common.py
The QAPI code generator clocks in at some 3100 SLOC in 8 source files.
Almost 60% of the code is in qapi/common.py.  Split it into more
focused modules:

* Move QAPISchemaPragma and QAPISourceInfo to qapi/source.py.

* Move QAPIError and its sub-classes to qapi/error.py.

* Move QAPISchemaParser and QAPIDoc to parser.py.  Use the opportunity
  to put QAPISchemaParser first.

* Move check_expr() & friends to qapi/expr.py.  Use the opportunity to
  put the code into a more sensible order.

* Move QAPISchema & friends to qapi/schema.py

* Move QAPIGen and its sub-classes, ifcontext,
  QAPISchemaModularCVisitor, and QAPISchemaModularCVisitor to qapi/gen.py

* Delete camel_case(), it's unused since commit e98859a9b9 "qapi:
  Clean up after recent conversions to QAPISchemaVisitor"

A number of helper functions remain in qapi/common.py.  I considered
moving the code generator helpers to qapi/gen.py, but decided not to.
Perhaps we should rewrite them as methods of QAPIGen some day.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20191018074345.24034-7-armbru@redhat.com>
[Add "# -*- coding: utf-8 -*-" lines]
2019-10-22 13:53:55 +02:00