diff mbox series

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

Message ID 20210330171844.1197918-8-jsnow@redhat.com
State New
Headers show
Series qapi: static typing conversion, pt4 | expand

Commit Message

John Snow March 30, 2021, 5:18 p.m. UTC
No functional change.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/error.py | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Markus Armbruster April 15, 2021, 7:15 a.m. UTC | #1
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?

> +                 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:
John Snow April 15, 2021, 3:52 p.m. UTC | #2
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:
diff mbox series

Patch

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],
+                 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: