Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:114097 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 78006 invoked from network); 22 Apr 2021 09:48:27 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 22 Apr 2021 09:48:27 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 9B57B1804CC for ; Thu, 22 Apr 2021 02:51:27 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,HTML_MESSAGE, RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-Virus: No X-Envelope-From: Received: from mail-il1-f177.google.com (mail-il1-f177.google.com [209.85.166.177]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Thu, 22 Apr 2021 02:51:27 -0700 (PDT) Received: by mail-il1-f177.google.com with SMTP id c18so37254886iln.7 for ; Thu, 22 Apr 2021 02:51:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=AzXUGMYba8bb9XjVppZpqNbzRVPKywiuuTI62Y6iWB8=; b=gWgLvtLjwmwTrBw3C0xXlYQSxH7vtgiJ7SyrqOtoPs4Dc5LFvirsxttgqOHeFJXg8a 0+pwC0C4APpe99cc17bOwnpEre90QyOA1IQ7X0YPCDHNmCoY+fwJWVqkm49L85wNAB6R 1gr4a9w0Ui1C/ydAXSe8eA+Dw1HiMvt9ldvdnLV8FeLpks+dyZfwYXnuysZUp0nNVNt4 ZaVCT22jel3RskPSpXuo5+lIo5RCuXe3ewn1kQubr8pqdrP33eU8r4EtUSbdlNIx7+q4 h5D2j6gxgbru1/62IkaIU2BKcPxN20LEf1MNRz0H2J5tJB87719iqe7cwHhMoRrepcin EdmA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=AzXUGMYba8bb9XjVppZpqNbzRVPKywiuuTI62Y6iWB8=; b=nmc0GYZ6XuOItVSPF/AQW5DqmT1LZvcjoNC5uLoJM0GtIw28AQRPYRPqZgF+9/doME JdAVMILgV+0WuCREMIyy5qju1nosms7M0pvxDMJOWeFgEgfFkZU8nt0k1kNVy+GKVPJt 5mCuQMI7qMTEDBVsX0/XQ0GBVJPE9u9IDcJGLPV8SyA2P50dJZ6xawR/5KX2jJLvUeUC DpzR52xhKbuLJvXEiDT5/NP3qOuwgMIrqiKek1KTwLS77kG5xlkQh9vbtm+UYiy/WB5W Gr5zRa0JSdI4uBuPTdYq3AdzaQcnZG5K181JXqUXGdbprdWP2x0cvX+mLKER8eXyKJzw EMSw== X-Gm-Message-State: AOAM530TwFMp6yyzJuw5+yj8oe9VysngBZR3v6I0Wgv1xTRFXqExez5x VR5Itwie7mjH/EyG2acxTNckCntxeSQuv14qpAE= X-Google-Smtp-Source: ABdhPJxrHqt0RnpQFdH3RTicFDs53KlSVp3gBw/Lt7HerHzqIof/IKrZ9Fi0E/Z5BFjb2ZFh6xYF8UNpJzDnf485gG4= X-Received: by 2002:a05:6e02:1d06:: with SMTP id i6mr1996425ila.165.1619085085571; Thu, 22 Apr 2021 02:51:25 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: Date: Thu, 22 Apr 2021 11:51:14 +0200 Message-ID: To: Nikita Popov Cc: =?UTF-8?B?TcOhdMOpIEtvY3Npcw==?= , PHP Internals List Content-Type: multipart/alternative; boundary="00000000000062fde005c08c9fe4" Subject: Re: [PHP-DEV] [RFC] [Vote] Adding return types to internal methods From: ocramius@gmail.com (Marco Pivetta) --00000000000062fde005c08c9fe4 Content-Type: text/plain; charset="UTF-8" On Thu, Apr 22, 2021 at 10:37 AM Nikita Popov wrote: > > To clarify what you're actually suggesting here: Do you want to always > have the type returned from getReturnType() > Correct > and only add an additional bool method that tells you whether that is a > tentative or real type, so that most code can just treat them as being the > same thing? > No, and also not needed, especially since we're converging towards having these types explicitly defined in PHP 9. The use-cases of reflection **on return types** are generally as follows (obviously not exclusive list - it's hard to define "use-cases" in large groups, and be precise): 1. use reflection to generate subtypes (mocks, stubs) 2. use reflection to build a dependency graph of calls (dependency injection containers) 3. use reflection to validate data flows (type-checkers) 4. use reflection for documentation (doc generators, type-checkers) 5. use reflection for data conversion (serializers, ORMs) In the scenario when `ReflectionFunction#getReturnType()` reports the new types immediately: 1. is safe even if `ReflectionFunction#getReturnType()` starts reporting new type the return type is more restrictive, since variance rules allow for subclassing to do this. 2. is safe, because if `mixed` (previous inferred return type) already fits within a dependency graph, the new more precise type already fits too 3. is safe because the **returned** type is still more restrictive than `mixed` 4. is safe because type-checkers already learned to not trust internal php-src declared types (and moved to stubs instead) 5. is safe because the types declared by what is **returned** during deserialization is stricter than the previous `mixed`, and therefore still fits There will be hiccups, like always, but overall there is little space for security issues arising from stricter return type declarations: in fact, the pre-PHP-8.1 scenario (no type declarations) is much more dangerous. In the scenario when `ReflectionFunction#getReturnType()` does **not** report the new types, and instead these types are declared on new "tentative return type" methods (the RFC at vote): 1. 2. 3. 5. until tools consider the new methods, a starker migration to PHP9 is expected. Tools need to be adjusted to consider the new methods for 8.1, and to ignore the new methods for 9.0. This will cause a lot of downstream work 4. generally does not care about internal reflection API (again, these tools learned to hop around it with stubs) In the scenario when `ReflectionFunction#getReturnType()` does report the new types, but we add a new method to state that the return type is "tentative" (approach proposed by NikiC): 1. 2. 3. 5. until tools consider the new methods, a starker migration to PHP9 is expected. Tools need to be adjusted to consider the new methods for 8.1, and to ignore the new methods for 9.0. This will cause a lot of downstream work 4. generally does not care about internal reflection API (again, these tools learned to hop around it with stubs) Overall, I see the change of reported `ReflectionMethod#getReturnType()` as non-problematic, and tooling around reflection would continue working as expected, while adding new API requires: * more work to get the tooling upgraded to PHP 8.1 * more work to get the tooling upgraded to PHP 9.0 (when those methods become obsolete) * more risk that users of tools that are not yet up-to-date with PHP 8.1 experience bugs or weird behavior In fact, tooling will start reporting incompatibilities in types earlier, giving people more runway to fix their subtypes for the upcoming 9.0 change. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/ --00000000000062fde005c08c9fe4--