qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 03/12] qapi/main: handle theoretical None-return from re.matc


From: John Snow
Subject: Re: [PATCH 03/12] qapi/main: handle theoretical None-return from re.match()
Date: Wed, 16 Dec 2020 12:11:46 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1

On 12/16/20 3:23 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:

Mypy cannot understand that this match can never be None, so help it
along.

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

diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index 42517210b805..271d9e84da94 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -23,7 +23,8 @@
def invalid_prefix_char(prefix: str) -> Optional[str]:
      match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', prefix)

@match can't be None because the regexp always matches a prefix,
possibly the empty prefix.

-    if match.end() != len(prefix):
+    # match cannot be None, but mypy cannot infer that.
+    if match and match.end() != len(prefix):
          return prefix[match.end()]
      return None

High-level logic:

        if there is an invalid prefix character:
            return it
        return None

The actual code takes the return None when the match fails.  If this
could happen, it would be wrong.  I can't, so it doesn't matter, but I
dislike it anyway.

Alternative 1: turn "match cannot be None" from comment to code

        match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', prefix)
   +    assert match
        if match.end() != len(prefix):
            return prefix[match.end()]
        return None

Alternative 2: turn empty prefix into a special case

   -    match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', prefix)
   +    match = re.match(r'[A-Za-z_.-][A-Za-z0-9_.-]*', prefix)
   +    if not match:
   +        return prefix[0]
        if match.end() != len(prefix):
            return prefix[match.end()]
        return None

I'd prefer alternative 1.


OK, no problem.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]