From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 67CC8C433E0 for ; Wed, 24 Feb 2021 22:07:17 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E43BE64F03 for ; Wed, 24 Feb 2021 22:07:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E43BE64F03 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:57144 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lF2JP-0007Af-Ud for qemu-devel@archiver.kernel.org; Wed, 24 Feb 2021 17:07:16 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:60714) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lF2Ia-0006Ka-4u for qemu-devel@nongnu.org; Wed, 24 Feb 2021 17:06:24 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:36613) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1lF2IX-0004SL-A6 for qemu-devel@nongnu.org; Wed, 24 Feb 2021 17:06:23 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1614204377; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Rj2FKQeorxrqpDjrMe2+Va5sde6f7oYONr1zr6uHcgo=; b=VcynNM+aNnm8DIHuRCnoPL91Sdj82ooJ7Q54ew6VKc0X9JAYacDX1xnfft6HNPesuX9jDh ijTS0keq6eLWP7HawphOTvGsQAbO4vwqDyRCynbqFg79HsKo38flR/VwvaTzkME9preFhS ekGWq3yAdaN1lcoE9LoiErDXMhe2w1k= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-255-kaFX5agENP2z71IUAIdpMg-1; Wed, 24 Feb 2021 17:06:14 -0500 X-MC-Unique: kaFX5agENP2z71IUAIdpMg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id ADD4E801965; Wed, 24 Feb 2021 22:06:13 +0000 (UTC) Received: from [10.10.112.247] (ovpn-112-247.rdu2.redhat.com [10.10.112.247]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1E6295F9C0; Wed, 24 Feb 2021 22:06:13 +0000 (UTC) Subject: Re: [PATCH v3 06/16] qapi/expr.py: Check type of 'data' member To: Markus Armbruster References: <20210223003408.964543-1-jsnow@redhat.com> <20210223003408.964543-7-jsnow@redhat.com> <87zgzt4ump.fsf@dusky.pond.sub.org> From: John Snow Message-ID: <3da70bbf-bc62-7575-9d70-849384cf6fe6@redhat.com> Date: Wed, 24 Feb 2021 17:06:12 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <87zgzt4ump.fsf@dusky.pond.sub.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=jsnow@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=216.205.24.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Michael Roth , Cleber Rosa , qemu-devel@nongnu.org, Eduardo Habkost Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 2/24/21 5:39 AM, Markus Armbruster wrote: > John Snow writes: > >> Iterating over the members of data isn't going to work if it's not some >> form of dict anyway, but for the sake of mypy, formalize it. >> >> Signed-off-by: John Snow >> Reviewed-by: Eduardo Habkost >> --- >> scripts/qapi/expr.py | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py >> index c97e8ce8a4d..afa6bd07769 100644 >> --- a/scripts/qapi/expr.py >> +++ b/scripts/qapi/expr.py >> @@ -254,6 +254,9 @@ def check_union(expr, info): >> raise QAPISemError(info, "'discriminator' requires 'base'") >> check_name_is_str(discriminator, info, "'discriminator'") >> >> + if not isinstance(members, dict): >> + raise QAPISemError(info, "'data' must be an object") >> + >> for (key, value) in members.items(): >> source = "'data' member '%s'" % key >> check_name_str(key, info, source) >> @@ -267,6 +270,10 @@ def check_alternate(expr, info): >> >> if not members: >> raise QAPISemError(info, "'data' must not be empty") >> + >> + if not isinstance(members, dict): >> + raise QAPISemError(info, "'data' must be an object") >> + >> for (key, value) in members.items(): >> source = "'data' member '%s'" % key >> check_name_str(key, info, source) > > All errors require a negative test. > > If an error is unreachable, it should be an assertion instead. > > If these new errors are reachable, the commit might be a bug fix. > You can, yes: Traceback (most recent call last): File "/home/jsnow/src/qemu/scripts/qapi-gen.py", line 19, in sys.exit(main.main()) File "/home/jsnow/src/qemu/scripts/qapi/main.py", line 89, in main generate(args.schema, File "/home/jsnow/src/qemu/scripts/qapi/main.py", line 51, in generate schema = QAPISchema(schema_file) File "/home/jsnow/src/qemu/scripts/qapi/schema.py", line 853, in __init__ exprs = check_exprs(parser.exprs) File "/home/jsnow/src/qemu/scripts/qapi/expr.py", line 337, in check_exprs check_union(expr, info) File "/home/jsnow/src/qemu/scripts/qapi/expr.py", line 248, in check_union for (key, value) in members.items(): AttributeError: 'list' object has no attribute 'items' from this beauty: ## # @Foo: # # @id: identifier of the backend # # @driver: the backend driver to use # # @timer-period: timer period (in microseconds, 0: use lowest possible) # # Since: 4.0 ## { 'union': 'Foo', 'base': { 'id': 'str', 'driver': 'AudiodevDriver', '*timer-period': 'uint32' }, 'discriminator': 'driver', 'data': ['hello', 'world'] } I'll add some new regression tests for you. Do you want them squished in with this commit, or would you like it done kind of independently, at the beginning of this series instead? --js