On 10/7/20 8:02 AM, Markus Armbruster wrote:
John Snow <js...@redhat.com> writes:

The edge case is that if the name is '', this expression returns a
string instead of a bool, which violates our declared type.

The edge case is impossible, as discussed in review of v2.  I figure the
type checker can't see that, so we need to help it some.  Can we mention
this in the commit message?


Sure. It's quite possible we will be able to better model and constrain these types, but that will come after all of these patches.

Mypy just has no way of knowing that '' is forbidden. That constraint does not exist in the type system we have here at present.

You could, by the way, create a type called NonEmptyString and create a function that casts from str to NonEmptyString by means of an assertion and then annotating the return type.

Anyway, I'll update the commit message for now.


Signed-off-by: John Snow <js...@redhat.com>
Reviewed-by: Cleber Rosa <cr...@redhat.com>
Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
---
  scripts/qapi/gen.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index f2e2746fea5..1bad37fc06b 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -243,7 +243,7 @@ def __init__(self, prefix, what, user_blurb, builtin_blurb, 
pydoc):
@staticmethod
      def _is_user_module(name):
-        return name and not name.startswith('./')
+        return bool(name and not name.startswith('./'))

            return not (name is None or name.startswith('./')

Looks slightly clearer to me.
@staticmethod
      def _is_builtin_module(name):


Reply via email to