[PATCH 06/16] qapi: Improve error position for bogus invalid "Returns" section

Markus Armbruster posted 16 patches 8 months, 2 weeks ago
Maintainers: Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Eric Blake <eblake@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Jason Wang <jasowang@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Yuval Shaia <yuval.shaia.ml@gmail.com>, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>, Jiri Pirko <jiri@resnulli.us>, Stefan Berger <stefanb@linux.vnet.ibm.com>, Stefan Hajnoczi <stefanha@redhat.com>, Mads Ynddal <mads@ynddal.dk>, Gerd Hoffmann <kraxel@redhat.com>, Lukas Straub <lukasstraub2@web.de>
[PATCH 06/16] qapi: Improve error position for bogus invalid "Returns" section
Posted by Markus Armbruster 8 months, 2 weeks ago
When something other than a command has a "Returns" section, the error
message points to the beginning of the definition comment.  Point to
the "Returns" section instead.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/parser.py                   | 10 +++++++---
 tests/qapi-schema/doc-invalid-return.err |  2 +-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 82db595dcf..a771013959 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -759,9 +759,13 @@ def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
         self.features[feature.name].connect(feature)
 
     def check_expr(self, expr: QAPIExpression) -> None:
-        if self.has_section('Returns') and 'command' not in expr:
-            raise QAPISemError(self.info,
-                               "'Returns:' is only valid for commands")
+        if 'command' not in expr:
+            sec = next((sec for sec in self.sections
+                        if sec.name == 'Returns'),
+                       None)
+            if sec:
+                raise QAPISemError(sec.info,
+                                   "'Returns:' is only valid for commands")
 
     def check(self) -> None:
 
diff --git a/tests/qapi-schema/doc-invalid-return.err b/tests/qapi-schema/doc-invalid-return.err
index 2ad89c5941..bc5826de20 100644
--- a/tests/qapi-schema/doc-invalid-return.err
+++ b/tests/qapi-schema/doc-invalid-return.err
@@ -1 +1 @@
-doc-invalid-return.json:3: 'Returns:' is only valid for commands
+doc-invalid-return.json:5: 'Returns:' is only valid for commands
-- 
2.43.0
Re: [PATCH 06/16] qapi: Improve error position for bogus invalid "Returns" section
Posted by Daniel P. Berrangé 8 months, 2 weeks ago
On Fri, Feb 16, 2024 at 03:58:30PM +0100, Markus Armbruster wrote:
> When something other than a command has a "Returns" section, the error
> message points to the beginning of the definition comment.  Point to
> the "Returns" section instead.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi/parser.py                   | 10 +++++++---
>  tests/qapi-schema/doc-invalid-return.err |  2 +-
>  2 files changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|