Commit Graph

8 Commits

Author SHA1 Message Date
John Snow
b2b31fdf9b qapi/source: Remove line number from QAPISourceInfo initializer
With the QAPISourceInfo(None, None, None) construct gone, there's no
longer any reason to have to specify that a file starts on the first
line. Remove it from the initializer and default it to 1.

Remove the last vestiges where we check for 'line' being unset, that
can't happen, now.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-4-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-05-20 11:28:27 +02:00
John Snow
3404e57410 qapi/parser: Don't try to handle file errors
Fixes: f5d4361cda
Fixes: 52a474180a
Fixes: 46f49468c6

Remove the try/except block that handles file-opening errors in
QAPISchemaParser.__init__() and add one each to
QAPISchemaParser._include() and QAPISchema.__init__() respectively.

This simultaneously fixes the typing of info.fname (f5d4361cda), A
static typing violation in test-qapi (46f49468c6), and a regression of
an error message (52a474180a).

The short-ish version of what motivates this patch is:

- It's hard to write a good error message in the init method,
  because we need to determine the context of our caller to do so.
  It's easier to just let the caller write the message.
- We don't want to allow QAPISourceInfo(None, None, None) to exist. The
  typing introduced by commit f5d4361cda types the 'fname' field as
  (non-optional) str, which was premature until the removal of this
  construct.
- Errors made using such an object are currently incorrect (since
  52a474180a)
- It's not technically a semantic error if we cannot open the schema.
- There are various typing constraints that make mixing these two cases
  undesirable for a single special case.
- test-qapi's code handling an fname of 'None' is now dead, drop it.
  Additionally, Not all QAPIError objects have an 'info' field (since
  46f49468), so deleting this stanza corrects a typing oversight in
  test-qapi introduced by that commit.

Other considerations:

- open() is moved to a 'with' block to ensure file pointers are
  cleaned up deterministically.
- Python 3.3 deprecated IOError and made it a synonym for OSError.
  Avoid the misleading perception these exception handlers are
  narrower than they really are.

The long version:

The error message here is incorrect (since commit 52a474180a):

> python3 qapi-gen.py 'fake.json'
qapi-gen.py: qapi-gen.py: can't read schema file 'fake.json': No such file or directory

In pursuing it, we find that QAPISourceInfo has a special accommodation
for when there's no filename. Meanwhile, the intent when QAPISourceInfo
was typed (f5d4361cda) was non-optional 'str'. This usage was
overlooked.

To remove this, I'd want to avoid having a "fake" QAPISourceInfo
object. I also don't want to explicitly begin accommodating
QAPISourceInfo itself being None, because we actually want to eventually
prove that this can never happen -- We don't want to confuse "The file
isn't open yet" with "This error stems from a definition that wasn't
defined in any file".

(An earlier series tried to create a dummy info object, but it was tough
to prove in review that it worked correctly without creating new
regressions. This patch avoids that distraction. We would like to first
prove that we never raise QAPISemError for any built-in object before we
add "special" info objects. We aren't ready to do that yet.)

So, which way out of the labyrinth?

Here's one way: Don't try to handle errors at a level with "mixed"
semantic contexts; i.e. don't mix inclusion errors (should report a
source line where the include was triggered) and command line errors
(where we specified a file we couldn't read).

Remove the error handling from the initializer of the parser. Pythonic!
Now it's the caller's job to figure out what to do about it. Handle the
error in QAPISchemaParser._include() instead, where we can write a
targeted error message where we are guaranteed to have an 'info' context
to report with.

The root level error can similarly move to QAPISchema.__init__(), where
we know we'll never have an info context to report with, so we use a
more abstract error type.

Now the error looks sensible again:

> python3 qapi-gen.py 'fake.json'
qapi-gen.py: can't read schema file 'fake.json': No such file or directory

With these error cases separated, QAPISourceInfo can be solidified as
never having placeholder arguments that violate our desired types. Clean
up test-qapi along similar lines.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210519183951.3946870-2-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-05-20 11:28:27 +02: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
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
John Snow
96670e89ec qapi/source.py: delint with pylint
Shush an error and leave a hint for future cleanups when we're allowed
to use Python 3.7+.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20201009161558.107041-26-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10 11:37:48 +02:00
John Snow
f5d4361cda qapi/source.py: add type hint annotations
Annotations do not change runtime behavior.
This commit *only* adds annotations.

A note on typing of __init__: mypy requires init functions with no
parameters to document a return type of None to be considered fully
typed. In the case when there are input parameters, None may be omitted.

Since __init__ may never return any value, it is preferred to omit the
return annotation whenever possible.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20201009161558.107041-25-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10 11:37:48 +02:00
Markus Armbruster
baa310f1bb qapi: Inheriting from object is pointless with Python 3, drop
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200304155932.20452-2-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
2020-03-05 09:24:11 +01: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