Re: [PATCH 04/25] qapi: Prefix frontend errors with an "in definition" line

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2949 bytes --]

On 9/24/19 8:28 AM, Markus Armbruster wrote:
> We take pains to include the offending expression in error messages,
> e.g.
> 
>     tests/qapi-schema/alternate-any.json:2: alternate 'Alt' member 'one' cannot use type 'any'
> 
> But not always:
> 
>     tests/qapi-schema/enum-if-invalid.json:2: 'if' condition must be a string or a list of strings
> 
> Instead of improving them one by one, report the offending expression
> whenever it is known, like this:
> 
>     tests/qapi-schema/enum-if-invalid.json: In enum 'TestIfEnum':
>     tests/qapi-schema/enum-if-invalid.json:2: 'if' condition must be a string or a list of strings

Works for me.

> 
> Error messages that mention the offending expression become a bit
> redundant, e.g.
> 
>     tests/qapi-schema/alternate-any.json: In alternate 'Alt':
>     tests/qapi-schema/alternate-any.json:2: alternate 'Alt' member 'one' cannot use type 'any'
> 
> I'll take care of that later in this series.

Temporary verboseness is not a problem.

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi/common.py                            | 15 ++++++++++++++-
>  tests/qapi-schema/alternate-any.err               |  1 +
>  tests/qapi-schema/alternate-array.err             |  1 +

Touches a lot.  But such is refactoring life.

> +++ b/scripts/qapi/common.py
> @@ -64,6 +64,12 @@ class QAPISourceInfo(object):
>          self.fname = fname
>          self.line = line
>          self.parent = parent
> +        self.defn_meta = None
> +        self.defn_name = None
> +
> +    def set_defn(self, meta, name):
> +        self.defn_meta = meta
> +        self.defn_name = name
>  
>      def next_line(self):
>          info = copy.copy(self)
> @@ -73,6 +79,12 @@ class QAPISourceInfo(object):
>      def loc(self):
>          return '%s:%d' % (self.fname, self.line)
>  
> +    def in_defn(self):
> +        if self.defn_name:
> +            return "%s: In %s '%s':\n" % (self.fname,
> +                                          self.defn_meta, self.defn_name)
> +        return ''
> +
>      def include_path(self):
>          ret = ''
>          parent = self.parent
> @@ -82,7 +94,7 @@ class QAPISourceInfo(object):
>          return ret
>  
>      def __str__(self):
> -        return self.include_path() + self.loc()
> +        return self.include_path() + self.in_defn() + self.loc()
>  
>  
>  class QAPIError(Exception):
> @@ -1127,6 +1139,7 @@ def check_exprs(exprs):
>          normalize_if(expr)
>          name = expr[meta]
>          add_name(name, info, meta)
> +        info.set_defn(meta, name)
>          if doc and doc.symbol != name:

Rather simple addition.  Everything else in the patch is fallout.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]