Re: [PATCH v2 7/8] qapi/error: Add type hints

[John Snow]

On 4/15/21 3:15 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > No functional change.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   scripts/qapi/error.py | 11 +++++++++--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/qapi/error.py b/scripts/qapi/error.py
> > index 2183b8c6b7..6ba54821c9 100644
> > --- a/scripts/qapi/error.py
> > +++ b/scripts/qapi/error.py
> > @@ -11,6 +11,10 @@
> >   # This work is licensed under the terms of the GNU GPL, version 2.
> >   # See the COPYING file in the top-level directory.
> >   
> > +from typing import Optional
> > +
> > +from .source import QAPISourceInfo
> > +
> >   
> >   class QAPIError(Exception):
> >       """Base class for all exceptions from the QAPI package."""
> > @@ -18,13 +22,16 @@ class QAPIError(Exception):
> >   
> >   class QAPISourceError(QAPIError):
> >       """Error class for all exceptions identifying a source location."""
> > -    def __init__(self, info, msg, col=None):
> > +    def __init__(self,
> > +                 info: Optional[QAPISourceInfo],
>
> The Optional is a bit surprising.  Mind pointing to the / a reason in
> the commit message?

We don't have a special object representing "No Source Info", so 
schema.py passes around None objects to mean the same.

The problem, ultimately, is that a QAPISchemaEntity may or may not have 
a source info; and various error reporting paths that in practice will 
not involve such a type in its reporting cannot distinguish that using 
the type system, because from the root-down, info is codified as 
Optional[QAPISourceInfo].

There might be a way to introduce a constraint in schema.py later on, 
but for now (prior to part 6, especially), we cannot.

The assertion should help remind a reader that we don't expect it to be 
true.

--js

> > +                 msg: str,
> > +                 col: Optional[int] = None):
> >           super().__init__()
> >           self.info = info
> >           self.msg = msg
> >           self.col = col
> >   
> > -    def __str__(self):
> > +    def __str__(self) -> str:
> >           assert self.info is not None
> >           loc = str(self.info)
> >           if self.col is not None: