Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:92959
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.161.175 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <3de7a02a-0d76-cd48-6001-b1b9456c2690@gmail.com>
References: <CAF+90c_Hg2OsPG6QBjkttqGzP4d413VAWcamDYaAsB1u4uGnVQ@mail.gmail.com>
	<3de7a02a-0d76-cd48-6001-b1b9456c2690@gmail.com>
Date: Fri, 29 Apr 2016 21:32:17 +0200
Message-ID: <CAF+90c_N_d1FcBETrvgeA+qGQ287-fBz-iA3D6C8is85y1_wSA@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a1142d7be6585260531a4b355
Subject: Re: [PHP-DEV] [RFC] Forbid $this being null in instance methods
From: nikita.ppv@gmail.com (Nikita Popov)

--001a1142d7be6585260531a4b355
Content-Type: text/plain; charset=UTF-8

On Fri, Apr 29, 2016 at 9:10 PM, Stanislav Malyshev <smalyshev@gmail.com>
wrote:

> Hi!
>
> > I'd like to submit a small RFC for your consideration, which ensures
> > something that really ought to be a given already: If you use $this in an
> > instance method, you should actually get a $this (and not NULL).
> >
> >     https://wiki.php.net/rfc/forbid_null_this_in_methods
> >
> > As this is targeting PHP 7.1, the RFC is careful to retain compatibility
> > with certain legacy PHP 4 code patterns.
>
> Unfortunately, this does not exactly preserve compatibility. Consider:
>
> public function legacyMethod() {
>   if(is_null($this)) {
>     trigger_error(E_DEPRECATED, "legacyMethod() should be called with an
> object now!");
>     return self::getDefaultInstance()->legacyMethod();
>   }
>   print "I am an ugly legacy method!";
>   $this->doSomeStuff();
>   return true;
> }
>
> Unfortunately, I've seen this pattern not once when dealing with APIs
> which were grown from PHP 4 code and must maintain BC - e.g. have code
> relying on legacyMethod() both being called statically and called on an
> object and legacyMethod() being overridden by extending classes. It is
> an ugly code and an ugly situation to be in, but it is a real pattern
> that happens.
>
> Despite the RFC claiming HHVM does not support this pattern, it does:
> https://3v4l.org/1QrY7
> Unlike PHP, it doesn't even throw deprecated error in that case.
>

Wow, thanks for pointing this out! I'm really stumped as to why I thought
HHVM doesn't support it. I've dropped the remark now.


> Given that this RFC does not seem to enable anyone to do anything
> additional that couldn't be done before, just removes the features from
> language, I see any advantage in doing it so far.
>
> I also do not think enabling using $this as a regular variable is a
> particularly good idea. It does not add anything - there's no real need
> to name your variables specifically "$this", you can choose any other
> name - but it creates potential both for confusion and for weird errors
> when you refactor stuff. Again, don't see any real advantage in this.
>

The thing is that currently we allow using $this as a normal variable in
some ways (accepted as a function argument for example), but then don't
allow it in others (accessing properties on it). However, I agree that this
is probably the wrong direction to go here, instead we should make sure
that $this cannot be used at all. I will consider this.

Thanks,
Nikita

--001a1142d7be6585260531a4b355--