Re: [PATCH v3 2/3] qapi: improve specificity of type/member descriptions

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> When describing member types always include the context of the
> containing type. Although this is often redundant, in some cases
> it will help to reduce ambiguity.

This is no longer true.  It was in v2.  Suggest:

  Error messages describe object members, enumeration values, features,
  and variants like ROLE 'NAME', where ROLE is "member", "value",
  "feature", or "branch", respectively.  When the member is defined in
  another type, e.g. inherited from a base type, we add "of type
  'TYPE'".  Example: test case struct-base-clash-deep reports a member
  of type 'Sub' clashing with a member of its base type 'Base' as

      struct-base-clash-deep.json: In struct 'Sub':
      struct-base-clash-deep.json:10: member 'name' collides with member 'name' 
of type 'Base'

  Members of implicitly defined types need special treatment.  We don't
  want to add "of type 'TYPE'" for them, because their named are made up
  and mean nothing to the user.  Instead, we describe members of an
  implicitly defined base type as "base member 'NAME'", and command and
  event parameters as "parameter 'NAME'".  Example: test case
  union-bad-base reports member of a variant's type clashing with a
  member of its implicitly defined base type as

      union-bad-base.json: In union 'TestUnion':
      union-bad-base.json:8: member 'string' of type 'TestTypeA' collides with 
base member 'string'

  The next commit will permit unions as variant types.  "base member
  'NAME' would then be ambigious: is it the union's base, or is it the
  union's variant's base?  One of its test cases would report a clash
  between two such bases as "base member 'type' collides with base
  member 'type'".  Confusing.

  Refine the special treatment: add "of TYPE" even for implicitly
  defined types, but massage TYPE and ROLE so they make sense for the
  user.

> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  scripts/qapi/schema.py | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 207e4d71f3..da04b97ded 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -697,6 +697,7 @@ def connect_doc(self, doc):
>  
>      def describe(self, info):
>          role = self.role
> +        meta = 'type'
>          defined_in = self.defined_in
>          assert defined_in
>  
> @@ -708,13 +709,17 @@ def describe(self, info):
>                  # Implicit type created for a command's dict 'data'
>                  assert role == 'member'
>                  role = 'parameter'
> +                meta = 'command'
> +                defined_in = defined_in[:-4]
>              elif defined_in.endswith('-base'):
>                  # Implicit type created for a union's dict 'base'
>                  role = 'base ' + role
> +                defined_in = defined_in[:-5]
>              else:
>                  assert False
> -        elif defined_in != info.defn_name:
> -            return "%s '%s' of type '%s'" % (role, self.name, defined_in)
> +
> +        if defined_in != info.defn_name:
> +            return "%s '%s' of %s '%s'" % (role, self.name, meta, defined_in)
>          return "%s '%s'" % (role, self.name)

Since I rewrote both the patch and the commit message, would you like me
to take the blame and claim authorship?