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.